[dpdk-dev] doc: announce KNI ethtool removal

Message ID 1469030873-5966-1-git-send-email-thomas.monjalon@6wind.com (mailing list archive)
State Rejected, archived
Headers

Commit Message

Thomas Monjalon July 20, 2016, 4:07 p.m. UTC
  The out-of-tree kernel code must be avoided.
Moreover there is no good reason to keep this legacy feature
which is only partially supported.

As described earlier in this plan:
	http://dpdk.org/ml/archives/dev/2016-July/043606.html
it will help to keep PCI ids in PMD code.

Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
---
 doc/guides/rel_notes/deprecation.rst | 7 +++++++
 1 file changed, 7 insertions(+)
  

Comments

Ferruh Yigit July 21, 2016, 3:33 p.m. UTC | #1
On 7/20/2016 5:07 PM, Thomas Monjalon wrote:
> The out-of-tree kernel code must be avoided.
> Moreover there is no good reason to keep this legacy feature
> which is only partially supported.
> 
> As described earlier in this plan:
> 	http://dpdk.org/ml/archives/dev/2016-July/043606.html
> it will help to keep PCI ids in PMD code.
> 
> Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> ---
>  doc/guides/rel_notes/deprecation.rst | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> index f502f86..9cadf6a 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -41,3 +41,10 @@ Deprecation Notices
>  * The mempool functions for single/multi producer/consumer are deprecated and
>    will be removed in 16.11.
>    It is replaced by rte_mempool_generic_get/put functions.
> +
> +* The ethtool support will be removed from KNI in 16.11.
> +  It is implemented only for igb and ixgbe.
> +  It is really hard to maintain because it requires some out-of-tree kernel
> +  code to be duplicated in this kernel module.
> +  Removing this partial support will help to restrict the PCI id definitions
> +  to the PMD code.
> 

KNI ethtool is functional and maintained, and it may have users!

Why just removing it, specially without providing an alternative?
Is is good time to discuss KCP again?

Regards,
ferruh
  
Igor Ryzhov July 21, 2016, 3:41 p.m. UTC | #2
Hello.

I think good alternative is rte_ethtool library from ethtool sample
application.
But I am wondering why this code is only in app, not in lib.

Best regards,
Igor

On Thu, Jul 21, 2016 at 4:33 PM, Ferruh Yigit <ferruh.yigit@intel.com>
wrote:

> On 7/20/2016 5:07 PM, Thomas Monjalon wrote:
> > The out-of-tree kernel code must be avoided.
> > Moreover there is no good reason to keep this legacy feature
> > which is only partially supported.
> >
> > As described earlier in this plan:
> >       http://dpdk.org/ml/archives/dev/2016-July/043606.html
> > it will help to keep PCI ids in PMD code.
> >
> > Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> > ---
> >  doc/guides/rel_notes/deprecation.rst | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/doc/guides/rel_notes/deprecation.rst
> b/doc/guides/rel_notes/deprecation.rst
> > index f502f86..9cadf6a 100644
> > --- a/doc/guides/rel_notes/deprecation.rst
> > +++ b/doc/guides/rel_notes/deprecation.rst
> > @@ -41,3 +41,10 @@ Deprecation Notices
> >  * The mempool functions for single/multi producer/consumer are
> deprecated and
> >    will be removed in 16.11.
> >    It is replaced by rte_mempool_generic_get/put functions.
> > +
> > +* The ethtool support will be removed from KNI in 16.11.
> > +  It is implemented only for igb and ixgbe.
> > +  It is really hard to maintain because it requires some out-of-tree
> kernel
> > +  code to be duplicated in this kernel module.
> > +  Removing this partial support will help to restrict the PCI id
> definitions
> > +  to the PMD code.
> >
>
> KNI ethtool is functional and maintained, and it may have users!
>
> Why just removing it, specially without providing an alternative?
> Is is good time to discuss KCP again?
>
> Regards,
> ferruh
>
  
Thomas Monjalon July 21, 2016, 4:41 p.m. UTC | #3
2016-07-21 16:41, Igor Ryzhov:
> On Thu, Jul 21, 2016 at 4:33 PM, Ferruh Yigit <ferruh.yigit@intel.com>
> wrote:
> > On 7/20/2016 5:07 PM, Thomas Monjalon wrote:
> > > The out-of-tree kernel code must be avoided.
> > > Moreover there is no good reason to keep this legacy feature
> > > which is only partially supported.
> > >
> > > As described earlier in this plan:
> > >       http://dpdk.org/ml/archives/dev/2016-July/043606.html
> > > it will help to keep PCI ids in PMD code.
> > >
> > > Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
[...]
> > > +
> > > +* The ethtool support will be removed from KNI in 16.11.
> > > +  It is implemented only for igb and ixgbe.
> > > +  It is really hard to maintain because it requires some out-of-tree kernel
> > > +  code to be duplicated in this kernel module.
> > > +  Removing this partial support will help to restrict the PCI id definitions
> > > +  to the PMD code.
> >
> > KNI ethtool is functional and maintained, and it may have users!
> >
> > Why just removing it, specially without providing an alternative?

Because
1/ It is using the shared PCI ids that we want to move
2/ It has a poor support (igb/ixgbe) and makes users confused
3/ It is a big import of another version of igb/ixgbe drivers

About the point 1, if we decide to keep KNI ethtool, please could you
duplicate the igb/ixgbe PCI ids in KNI?

> > Is is good time to discuss KCP again?
> 
> I think good alternative is rte_ethtool library from ethtool sample
> application.

Yes I think so.

> But I am wondering why this code is only in app, not in lib.

It is an example lib because we were not sure wether we wanted to
support it. But maybe it is time to discuss its status and check
if it can be integrated with other DPDK libs?
  
Jay Rolette July 21, 2016, 6:20 p.m. UTC | #4
On Thu, Jul 21, 2016 at 10:33 AM, Ferruh Yigit <ferruh.yigit@intel.com>
wrote:

> On 7/20/2016 5:07 PM, Thomas Monjalon wrote:
> > The out-of-tree kernel code must be avoided.
> > Moreover there is no good reason to keep this legacy feature
> > which is only partially supported.
> >
> > As described earlier in this plan:
> >       http://dpdk.org/ml/archives/dev/2016-July/043606.html
> > it will help to keep PCI ids in PMD code.
> >
> > Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> > ---
> >  doc/guides/rel_notes/deprecation.rst | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/doc/guides/rel_notes/deprecation.rst
> b/doc/guides/rel_notes/deprecation.rst
> > index f502f86..9cadf6a 100644
> > --- a/doc/guides/rel_notes/deprecation.rst
> > +++ b/doc/guides/rel_notes/deprecation.rst
> > @@ -41,3 +41,10 @@ Deprecation Notices
> >  * The mempool functions for single/multi producer/consumer are
> deprecated and
> >    will be removed in 16.11.
> >    It is replaced by rte_mempool_generic_get/put functions.
> > +
> > +* The ethtool support will be removed from KNI in 16.11.
> > +  It is implemented only for igb and ixgbe.
> > +  It is really hard to maintain because it requires some out-of-tree
> kernel
> > +  code to be duplicated in this kernel module.
> > +  Removing this partial support will help to restrict the PCI id
> definitions
> > +  to the PMD code.
> >
>
> KNI ethtool is functional and maintained, and it may have users!
>
> Why just removing it, specially without providing an alternative?
> Is is good time to discuss KCP again?
>

Yes, my product uses it. Seems like we are back to the same discussion we
had a few months ago about the KNI situation...

It shouldn't be removed unless there is a replacement, ideally one that
works with the normal Linux tools like every other network device. While
the code wasn't ready at the time, it was a definite improvement over what
we have with KNI today.

Jay
  
Thomas Monjalon July 21, 2016, 8:32 p.m. UTC | #5
2016-07-21 13:20, Jay Rolette:
> On Thu, Jul 21, 2016 at 10:33 AM, Ferruh Yigit <ferruh.yigit@intel.com>
> wrote:
> > KNI ethtool is functional and maintained, and it may have users!
> >
> > Why just removing it, specially without providing an alternative?
> > Is is good time to discuss KCP again?
> 
> Yes, my product uses it.

Your product uses what? KCP? KNI? KNI ethtool?

> Seems like we are back to the same discussion we
> had a few months ago about the KNI situation...
> 
> It shouldn't be removed unless there is a replacement, ideally one that
> works with the normal Linux tools like every other network device.

This ethtool module works only for igb and ixgbe!
There is already no replacement for other drivers.
Who works on a replacement?

> While the code wasn't ready at the time, it was a definite improvement over what
> we have with KNI today.
  
Jay Rolette July 21, 2016, 8:54 p.m. UTC | #6
On Thu, Jul 21, 2016 at 3:32 PM, Thomas Monjalon <thomas.monjalon@6wind.com>
wrote:

> 2016-07-21 13:20, Jay Rolette:
> > On Thu, Jul 21, 2016 at 10:33 AM, Ferruh Yigit <ferruh.yigit@intel.com>
> > wrote:
> > > KNI ethtool is functional and maintained, and it may have users!
> > >
> > > Why just removing it, specially without providing an alternative?
> > > Is is good time to discuss KCP again?
> >
> > Yes, my product uses it.
>
> Your product uses what? KCP? KNI? KNI ethtool?
>

Sorry, that wasn't very clear. It uses KNI + ifconfig to configure the
device/interface in Linux. I'm assuming the "ethtool" bits under discussion
are the same things that make ifconfig work with KNI to the limited extent
it does.

> Seems like we are back to the same discussion we
> > had a few months ago about the KNI situation...
> >
> > It shouldn't be removed unless there is a replacement, ideally one that
> > works with the normal Linux tools like every other network device.
>
> This ethtool module works only for igb and ixgbe!
> There is already no replacement for other drivers.
> Who works on a replacement?
>

Ferruh submitted KCP previously, but you guys didn't like the fact that it
was a kernel module. IIRC, one of the gains from that was simplified
maintenance because you didn't need driver specific support for KNI.
Assuming he's still willing to beat it into shape, we have something that
is already most of the way there.

If people are going to continue to block it because it is a kernel module,
then IMO, it's better to leave the existing support on igx / ixgbe in place
instead of stepping backwards to zero support for ethtool.

> While the code wasn't ready at the time, it was a definite improvement
> over what
> > we have with KNI today.
>
  
Thomas Monjalon July 21, 2016, 11:15 p.m. UTC | #7
2016-07-21 15:54, Jay Rolette:
> On Thu, Jul 21, 2016 at 3:32 PM, Thomas Monjalon <thomas.monjalon@6wind.com>
> wrote:
> > 2016-07-21 13:20, Jay Rolette:
> > > On Thu, Jul 21, 2016 at 10:33 AM, Ferruh Yigit <ferruh.yigit@intel.com>
> > > wrote:
> > > > KNI ethtool is functional and maintained, and it may have users!
> > > >
> > > > Why just removing it, specially without providing an alternative?
> > > > Is is good time to discuss KCP again?
> > >
> > > Yes, my product uses it.
> >
> > Your product uses what? KCP? KNI? KNI ethtool?
> 
> Sorry, that wasn't very clear. It uses KNI + ifconfig to configure the
> device/interface in Linux. I'm assuming the "ethtool" bits under discussion
> are the same things that make ifconfig work with KNI to the limited extent
> it does.

What are you configuring with ifconfig?

> > Seems like we are back to the same discussion we
> > > had a few months ago about the KNI situation...
> > >
> > > It shouldn't be removed unless there is a replacement, ideally one that
> > > works with the normal Linux tools like every other network device.
> >
> > This ethtool module works only for igb and ixgbe!
> > There is already no replacement for other drivers.
> > Who works on a replacement?
> 
> Ferruh submitted KCP previously, but you guys didn't like the fact that it
> was a kernel module.

No we didn't like having yet another out-of-tree kernel module.
We'd love having KCP in Linux.

> IIRC, one of the gains from that was simplified
> maintenance because you didn't need driver specific support for KNI.
> Assuming he's still willing to beat it into shape, we have something that
> is already most of the way there.
> 
> If people are going to continue to block it because it is a kernel module,
> then IMO, it's better to leave the existing support on igx / ixgbe in place
> instead of stepping backwards to zero support for ethtool.

Maybe that one day you will want to use something else than ixgbe in your
product (and have more than 10 Gb). This day you'll find other solutions
than using KNI ethtool.

> > While the code wasn't ready at the time, it was a definite improvement
> > over what we have with KNI today.
  
Andriy Berestovskyy July 22, 2016, 5:20 p.m. UTC | #8
Hi folks,
Just to clarify. Thomas is talking about removing just the KNI ethtool
(i.e. lib/librte_eal/linuxapp/kni/ethtool/*). The major functionality
of those 45K lines of code is to get the same MAC address on the KNI
interface and the underlying igb/ixgbe NIC.

At the moment the rest of the DPDK eth devices work fine without the
KNI ethtool. The workaround is very simple: use ifconfig or ip tool to
set the same MAC you have on your NIC. Put it into your network
configuration to make it permanent.

Examples:
ifconfig vEth0_0 hw ether <your NIC's MAC>
or
ip link set vEth0_0 address <your NIC's MAC>
or
in /etc/network/interfaces under the "iface vEth0_0" section add the following:
    hwaddress <your NIC's MAC>


Andriy

On Thu, Jul 21, 2016 at 10:54 PM, Jay Rolette <rolette@infinite.io> wrote:
> On Thu, Jul 21, 2016 at 3:32 PM, Thomas Monjalon <thomas.monjalon@6wind.com>
> wrote:
>
>> 2016-07-21 13:20, Jay Rolette:
>> > On Thu, Jul 21, 2016 at 10:33 AM, Ferruh Yigit <ferruh.yigit@intel.com>
>> > wrote:
>> > > KNI ethtool is functional and maintained, and it may have users!
>> > >
>> > > Why just removing it, specially without providing an alternative?
>> > > Is is good time to discuss KCP again?
>> >
>> > Yes, my product uses it.
>>
>> Your product uses what? KCP? KNI? KNI ethtool?
>>
>
> Sorry, that wasn't very clear. It uses KNI + ifconfig to configure the
> device/interface in Linux. I'm assuming the "ethtool" bits under discussion
> are the same things that make ifconfig work with KNI to the limited extent
> it does.
>
>> Seems like we are back to the same discussion we
>> > had a few months ago about the KNI situation...
>> >
>> > It shouldn't be removed unless there is a replacement, ideally one that
>> > works with the normal Linux tools like every other network device.
>>
>> This ethtool module works only for igb and ixgbe!
>> There is already no replacement for other drivers.
>> Who works on a replacement?
>>
>
> Ferruh submitted KCP previously, but you guys didn't like the fact that it
> was a kernel module. IIRC, one of the gains from that was simplified
> maintenance because you didn't need driver specific support for KNI.
> Assuming he's still willing to beat it into shape, we have something that
> is already most of the way there.
>
> If people are going to continue to block it because it is a kernel module,
> then IMO, it's better to leave the existing support on igx / ixgbe in place
> instead of stepping backwards to zero support for ethtool.
>
>> While the code wasn't ready at the time, it was a definite improvement
>> over what
>> > we have with KNI today.
>>
  
Ferruh Yigit July 26, 2016, 1:18 p.m. UTC | #9
On 7/22/2016 6:20 PM, Andriy Berestovskyy wrote:
> Hi folks,
> Just to clarify. Thomas is talking about removing just the KNI ethtool
> (i.e. lib/librte_eal/linuxapp/kni/ethtool/*). The major functionality
> of those 45K lines of code is to get the same MAC address on the KNI
> interface and the underlying igb/ixgbe NIC.

Yes what we talk here is "KNI ethtool" but its major functionality is
NOT getting MAC address from device. Basically it lets issuing ethtool
commands to the device via virtual interface.
Like for a device that controlled by DPDK, it is possible to read device
registers, or read/write eeprom of the device via this feature.
So as name suggests, it enables ethtool support for device.

> 
> At the moment the rest of the DPDK eth devices work fine without the
> KNI ethtool. 

Of course, this is an optional feature, drivers doesn't support this
feature will work fine.
So keeping this feature, doesn't cost any to drivers that doesn't
support this feature.

> The workaround is very simple: use ifconfig or ip tool to
> set the same MAC you have on your NIC. Put it into your network
> configuration to make it permanent.
> 
> Examples:
> ifconfig vEth0_0 hw ether <your NIC's MAC>
> or
> ip link set vEth0_0 address <your NIC's MAC>
> or
> in /etc/network/interfaces under the "iface vEth0_0" section add the following:
>     hwaddress <your NIC's MAC>
> 
> 
> Andriy
> 
> On Thu, Jul 21, 2016 at 10:54 PM, Jay Rolette <rolette@infinite.io> wrote:
>> On Thu, Jul 21, 2016 at 3:32 PM, Thomas Monjalon <thomas.monjalon@6wind.com>
>> wrote:
>>
>>> 2016-07-21 13:20, Jay Rolette:
>>>> On Thu, Jul 21, 2016 at 10:33 AM, Ferruh Yigit <ferruh.yigit@intel.com>
>>>> wrote:
>>>>> KNI ethtool is functional and maintained, and it may have users!
>>>>>
>>>>> Why just removing it, specially without providing an alternative?
>>>>> Is is good time to discuss KCP again?
>>>>
>>>> Yes, my product uses it.
>>>
>>> Your product uses what? KCP? KNI? KNI ethtool?
>>>
>>
>> Sorry, that wasn't very clear. It uses KNI + ifconfig to configure the
>> device/interface in Linux. I'm assuming the "ethtool" bits under discussion
>> are the same things that make ifconfig work with KNI to the limited extent
>> it does.
>>
>>> Seems like we are back to the same discussion we
>>>> had a few months ago about the KNI situation...
>>>>
>>>> It shouldn't be removed unless there is a replacement, ideally one that
>>>> works with the normal Linux tools like every other network device.
>>>
>>> This ethtool module works only for igb and ixgbe!
>>> There is already no replacement for other drivers.
>>> Who works on a replacement?
>>>
>>
>> Ferruh submitted KCP previously, but you guys didn't like the fact that it
>> was a kernel module. IIRC, one of the gains from that was simplified
>> maintenance because you didn't need driver specific support for KNI.
>> Assuming he's still willing to beat it into shape, we have something that
>> is already most of the way there.
>>
>> If people are going to continue to block it because it is a kernel module,
>> then IMO, it's better to leave the existing support on igx / ixgbe in place
>> instead of stepping backwards to zero support for ethtool.
>>
>>> While the code wasn't ready at the time, it was a definite improvement
>>> over what
>>>> we have with KNI today.
>>>
> 
> 
>
  
Ferruh Yigit July 26, 2016, 1:23 p.m. UTC | #10
On 7/21/2016 5:41 PM, Thomas Monjalon wrote:
> 2016-07-21 16:41, Igor Ryzhov:
>> On Thu, Jul 21, 2016 at 4:33 PM, Ferruh Yigit <ferruh.yigit@intel.com>
>> wrote:
>>> On 7/20/2016 5:07 PM, Thomas Monjalon wrote:
>>>> The out-of-tree kernel code must be avoided.
>>>> Moreover there is no good reason to keep this legacy feature
>>>> which is only partially supported.
>>>>
>>>> As described earlier in this plan:
>>>>       http://dpdk.org/ml/archives/dev/2016-July/043606.html
>>>> it will help to keep PCI ids in PMD code.
>>>>
>>>> Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> [...]
>>>> +
>>>> +* The ethtool support will be removed from KNI in 16.11.
>>>> +  It is implemented only for igb and ixgbe.
>>>> +  It is really hard to maintain because it requires some out-of-tree kernel
>>>> +  code to be duplicated in this kernel module.
>>>> +  Removing this partial support will help to restrict the PCI id definitions
>>>> +  to the PMD code.
>>>
>>> KNI ethtool is functional and maintained, and it may have users!
>>>
>>> Why just removing it, specially without providing an alternative?
> 
> Because
> 1/ It is using the shared PCI ids that we want to move
> 2/ It has a poor support (igb/ixgbe) and makes users confused
> 3/ It is a big import of another version of igb/ixgbe drivers

I agree it is not the best design, but this is a functional piece of
code, and as long it keep maintained or completely replaced I am for
keeping it.

> About the point 1, if we decide to keep KNI ethtool, please could you
> duplicate the igb/ixgbe PCI ids in KNI?

Sure, I am not aware of exactly what needs to be done, please show me.

>>> Is is good time to discuss KCP again?
>>
>> I think good alternative is rte_ethtool library from ethtool sample
>> application.
> 
> Yes I think so.
> 
>> But I am wondering why this code is only in app, not in lib.
> 
> It is an example lib because we were not sure wether we wanted to
> support it. But maybe it is time to discuss its status and check
> if it can be integrated with other DPDK libs?
>
  
Thomas Monjalon July 26, 2016, 1:42 p.m. UTC | #11
2016-07-26 14:23, Ferruh Yigit:
> On 7/21/2016 5:41 PM, Thomas Monjalon wrote:
> > About the point 1, if we decide to keep KNI ethtool, please could you
> > duplicate the igb/ixgbe PCI ids in KNI?
> 
> Sure, I am not aware of exactly what needs to be done, please show me.

We just need to remove the use of the file rte_pci_dev_ids.h:
	http://dpdk.org/ml/archives/dev/2016-July/043606.html
  

Patch

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index f502f86..9cadf6a 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -41,3 +41,10 @@  Deprecation Notices
 * The mempool functions for single/multi producer/consumer are deprecated and
   will be removed in 16.11.
   It is replaced by rte_mempool_generic_get/put functions.
+
+* The ethtool support will be removed from KNI in 16.11.
+  It is implemented only for igb and ixgbe.
+  It is really hard to maintain because it requires some out-of-tree kernel
+  code to be duplicated in this kernel module.
+  Removing this partial support will help to restrict the PCI id definitions
+  to the PMD code.