Re: [PATCH net-next] eth: remove neterion/vxge

Re: [PATCH net-next] eth: remove neterion/vxge

[Jiri Pirko]

Fri, Jul 01, 2022 at 06:42:34AM CEST, kuba@kernel.org wrote:
>The last meaningful change to this driver was made by Jon in 2011.
>As much as we'd like to believe that this is because the code is
>perfect the chances are nobody is using this hardware.

Hmm, I can understand what for driver for HW that is no longer
developed, the driver changes might be very minimal. The fact that the
code does not change for years does not mean that there are users of
this NIC which this patch would break :/

Isn't there some obsoletion scheme globally applied to kernel device
support? I would expect something like that.

Re: [PATCH net-next] eth: remove neterion/vxge

[David Lamparter]

[culled Cc:]

On Fri, Jul 01, 2022 at 12:34:13PM +0200, Jiri Pirko wrote:
> Fri, Jul 01, 2022 at 06:42:34AM CEST, kuba@kernel.org wrote:
> >The last meaningful change to this driver was made by Jon in 2011.
> >As much as we'd like to believe that this is because the code is
> >perfect the chances are nobody is using this hardware.
> 
> Hmm, I can understand what for driver for HW that is no longer
> developed, the driver changes might be very minimal. The fact that the
> code does not change for years does not mean that there are users of
> this NIC which this patch would break :/

As a "reference datapoint", I'm a user that was affected by the removal
of the Mellanox SwitchX-2 driver about a year ago.  But that was a bit
different since the driver was apparently rather incomplete (I don't
know the details, was still messing around to even get things going.)

(FWIW my use case is in giving old hardware a second life, in this case
completely throwing away the PowerPC control board from Mellanox SX6000
series switches and replacing it with a new custom CPU board...  I might
well be the only person interested in that driver.

> Isn't there some obsoletion scheme globally applied to kernel device
> support? I would expect something like that.

I have the same question - didn't see any such policy but didn't look
particularly hard.  But would like to avoid putting time into making
something work just to have the kernel driver yanked shortly after :)


-David

Re: [PATCH net-next] eth: remove neterion/vxge

[Jiri Pirko]

Fri, Jul 01, 2022 at 03:17:32PM CEST, equinox@diac24.net wrote:
>[culled Cc:]
>
>On Fri, Jul 01, 2022 at 12:34:13PM +0200, Jiri Pirko wrote:
>> Fri, Jul 01, 2022 at 06:42:34AM CEST, kuba@kernel.org wrote:
>> >The last meaningful change to this driver was made by Jon in 2011.
>> >As much as we'd like to believe that this is because the code is
>> >perfect the chances are nobody is using this hardware.
>> 
>> Hmm, I can understand what for driver for HW that is no longer
>> developed, the driver changes might be very minimal. The fact that the
>> code does not change for years does not mean that there are users of
>> this NIC which this patch would break :/
>
>As a "reference datapoint", I'm a user that was affected by the removal
>of the Mellanox SwitchX-2 driver about a year ago.  But that was a bit

You could not be. There was really no functionality implemented in
switchx2 driver. I doubt you used 32x40G port switch with slow-path
forwarding through kernel with total max bandwidth of like 1-2G for the
whole switch :)


>different since the driver was apparently rather incomplete (I don't
>know the details, was still messing around to even get things going.)
>
>(FWIW my use case is in giving old hardware a second life, in this case
>completely throwing away the PowerPC control board from Mellanox SX6000
>series switches and replacing it with a new custom CPU board...  I might
>well be the only person interested in that driver.
>
>> Isn't there some obsoletion scheme globally applied to kernel device
>> support? I would expect something like that.
>
>I have the same question - didn't see any such policy but didn't look
>particularly hard.  But would like to avoid putting time into making
>something work just to have the kernel driver yanked shortly after :)
>
>
>-David

Re: [PATCH net-next] eth: remove neterion/vxge

[David Lamparter]

On Fri, Jul 01, 2022 at 04:06:11PM +0200, Jiri Pirko wrote:
> Fri, Jul 01, 2022 at 03:17:32PM CEST, equinox@diac24.net wrote:
> >As a "reference datapoint", I'm a user that was affected by the removal
> >of the Mellanox SwitchX-2 driver about a year ago.  But that was a bit
> 
> You could not be. There was really no functionality implemented in
> switchx2 driver. I doubt you used 32x40G port switch with slow-path
> forwarding through kernel with total max bandwidth of like 1-2G for the
> whole switch :)

Funny you would mention that, that's exactly as far as I got - on some
attempts at least, when I didn't get an initialization failure after a
15 minute reset timer ;D

Re: [PATCH net-next] eth: remove neterion/vxge

[Jakub Kicinski]

On Fri, 1 Jul 2022 15:17:32 +0200 David Lamparter wrote:
> > Hmm, I can understand what for driver for HW that is no longer
> > developed, the driver changes might be very minimal. The fact that the
> > code does not change for years does not mean that there are users of
> > this NIC which this patch would break :/  

Nah, bugs will be discovered. Look at mlx4 or ixgbe, those are
similarly old yet we still occasionally get a fix for a 10 year old
bug. The only bug report I could find for vxge is RH bugzilla filed
likely by RH QA themselves, 11 years ago.

> > Isn't there some obsoletion scheme globally applied to kernel device
> > support? I would expect something like that.  
> 
> I have the same question - didn't see any such policy but didn't look
> particularly hard.

I don't know of any one that works, that's the problem. I think
previous discussions were about more serious stuff like uAPI.

I don't really care about vxge in particular, I was already looking for
something to delete and the bad patch I mention in the commit msg came
up. What I'm mostly interested in is getting some experience to inform
a deletion policy. We can't come up with one by just talking. I'm
hoping to make this a topic for the maintainer's summit as well.

We are pretty open to taking in new drivers, (necessarily) even without
users, I think the flip side of that coin has to be that we delete unused
stuff. We're not a code storage service.

Here are some facts:
 - driver is not actively maintained (Jon did not nack the bad patch)
 - driver has no known users (it's unlikely they exist)
 - driver is not of great quality (constant stream of bot fixes)
 - driver is of significant complexity and needs to be adjusted each
   time we change core APIs

It's been over a decade of no development, let's delete this code.

If someone complains we can quickly revert the deletion in stable
(CCing Greg to keep me honest, I haven't actually talked to him).
I'm obviously responsible for the deletion so I'll prepare the revert.

Re: [PATCH net-next] eth: remove neterion/vxge

[Martin Habets]

On Fri, Jul 01, 2022 at 12:34:13PM +0200, Jiri Pirko wrote:
> Isn't there some obsoletion scheme globally applied to kernel device
> support? I would expect something like that.

The only thing I've found is that stuff gets marked with DEPRECATED
in Kconfig.
Having an official CONFIG_DEPRECATED option could be a way to
formalise this more, but I'm not sure if that is needed.

Martin

Re: [PATCH net-next] eth: remove neterion/vxge

[Francois Romieu]

Jakub Kicinski <kuba@kernel.org> :
> The last meaningful change to this driver was made by Jon in 2011.
> As much as we'd like to believe that this is because the code is
> perfect the chances are nobody is using this hardware.

It was used with some success in 2017:

https://bugzilla.kernel.org/show_bug.cgi?id=197881

> Because of the size of this driver there is a nontrivial maintenance
> cost to keeping this code around, in the last 2 years we're averaging
> more than 1 change a month. Some of which require nontrivial review
> effort, see commit 877fe9d49b74 ("Revert "drivers/net/ethernet/neterion/vxge:
> Fix a use-after-free bug in vxge-main.c"") for example.

vxge_remove() calls vxge_device_unregister().

vxge_device_unregister() does unregister_netdev() + ... + free_netdev().

vxge_remove() keeps using netdev_priv() pointer... :o/

Imho it is not nontrivial enough that top-level maintainers must handle it
but it is just mvho that maintainers handle too much low-value stuff.

Regarding the unused hardware side of the problem, it's a bit sad that
there still is no centralized base of interested users for a given piece
of hardware in 2022.

-- 
Ueimor

Re: [PATCH net-next] eth: remove neterion/vxge

[Jakub Kicinski]

On Fri, 1 Jul 2022 19:12:43 +0200 Francois Romieu wrote:
> Jakub Kicinski <kuba@kernel.org> :
> > The last meaningful change to this driver was made by Jon in 2011.
> > As much as we'd like to believe that this is because the code is
> > perfect the chances are nobody is using this hardware.  
> 
> It was used with some success in 2017:
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=197881

Nice find! Quoting for the list:

  vxge.ko can work nicely for kernel version 4.1 (I tried 4.1.44)
  However, for any version beyond that (I tried 4.4, 4.8, 4.13)
  the card can be initiated - but when I tried to do some network
  transfer (for example, ssh) I saw something like.... 

  [Tx queue timeout stack trace follows]

I didn't see any fixes since 2017 so the problem must still be there.
Could be just a particular version of FW that's broken, tho.

> > Because of the size of this driver there is a nontrivial maintenance
> > cost to keeping this code around, in the last 2 years we're averaging
> > more than 1 change a month. Some of which require nontrivial review
> > effort, see commit 877fe9d49b74 ("Revert "drivers/net/ethernet/neterion/vxge:
> > Fix a use-after-free bug in vxge-main.c"") for example.  
> 
> vxge_remove() calls vxge_device_unregister().
> 
> vxge_device_unregister() does unregister_netdev() + ... + free_netdev().
> 
> vxge_remove() keeps using netdev_priv() pointer... :o/
> 
> Imho it is not nontrivial enough that top-level maintainers must handle it
> but it is just mvho that maintainers handle too much low-value stuff.

Ack, this particular bug is just an excuse, it can be fixed.

> Regarding the unused hardware side of the problem, it's a bit sad that
> there still is no centralized base of interested users for a given piece
> of hardware in 2022.

100%, I really wish something like that existed. I have a vague memory
of Fedora or some other distro collecting HW data. Maybe it died because
of privacy issues?

Knowing that stuff gets used would be a great motivation. Handling all
the academic / bot patches for stuff I think goes completely unused is
weighing down my psyche.

Re: [PATCH net-next] eth: remove neterion/vxge

[Paolo Abeni]

On Fri, 2022-07-01 at 14:40 -0700, Jakub Kicinski wrote:
> 100%, I really wish something like that existed. I have a vague memory
> of Fedora or some other distro collecting HW data. Maybe it died because
> of privacy issues?

AFAICS that database still exists and is active:

https://linux-hardware.org/?view=search&vendor=neterion&d=All

It shows no usage at all for the relevant vendor.

On the flip side, it looks like the data points come mostly/exclusively
from desktop systems, not very relevant in this specific case.


/P

Re: [PATCH net-next] eth: remove neterion/vxge

[Jakub Kicinski]

On Tue, 05 Jul 2022 08:17:24 +0200 Paolo Abeni wrote:
> On Fri, 2022-07-01 at 14:40 -0700, Jakub Kicinski wrote:
> > 100%, I really wish something like that existed. I have a vague memory
> > of Fedora or some other distro collecting HW data. Maybe it died because
> > of privacy issues?  
> 
> AFAICS that database still exists and is active:
> 
> https://linux-hardware.org/?view=search&vendor=neterion&d=All
> 
> It shows no usage at all for the relevant vendor.
> 
> On the flip side, it looks like the data points come mostly/exclusively
> from desktop systems, not very relevant in this specific case.

GTK! There is a whole bunch of old Mellanox NICs reported so I think
there is _some_ server coverage. I'm leaning towards applying the patch.

Re: [PATCH net-next] eth: remove neterion/vxge

[Stephen Hemminger]

On Tue, 5 Jul 2022 11:06:34 -0700
Jakub Kicinski <kuba@kernel.org> wrote:

> On Tue, 05 Jul 2022 08:17:24 +0200 Paolo Abeni wrote:
> > On Fri, 2022-07-01 at 14:40 -0700, Jakub Kicinski wrote:  
> > > 100%, I really wish something like that existed. I have a vague memory
> > > of Fedora or some other distro collecting HW data. Maybe it died because
> > > of privacy issues?    
> > 
> > AFAICS that database still exists and is active:
> > 
> > https://linux-hardware.org/?view=search&vendor=neterion&d=All
> > 
> > It shows no usage at all for the relevant vendor.
> > 
> > On the flip side, it looks like the data points come mostly/exclusively
> > from desktop systems, not very relevant in this specific case.  
> 
> GTK! There is a whole bunch of old Mellanox NICs reported so I think
> there is _some_ server coverage. I'm leaning towards applying the patch.

Looks like S2IO became Neterion and then was acquired by Exar in 2010.
Then MaxLinear acquired Exar in 2017.
Looks like they dropped out of NIC business and only do switches??

Re: [PATCH net-next] eth: remove neterion/vxge

[Francois Romieu]

Stephen Hemminger <stephen@networkplumber.org> :
> On Tue, 5 Jul 2022 11:06:34 -0700
> Jakub Kicinski <kuba@kernel.org> wrote:
> > On Tue, 05 Jul 2022 08:17:24 +0200 Paolo Abeni wrote:
> > > On Fri, 2022-07-01 at 14:40 -0700, Jakub Kicinski wrote:  
> > > > 100%, I really wish something like that existed. I have a vague memory
> > > > of Fedora or some other distro collecting HW data. Maybe it died because
> > > > of privacy issues?    
> > > 
> > > AFAICS that database still exists and is active:
> > > 
> > > https://linux-hardware.org/?view=search&vendor=neterion&d=AllRC
> > > 
> > > It shows no usage at all for the relevant vendor.
> > > 
> > > On the flip side, it looks like the data points come mostly/exclusively
> > > from desktop systems, not very relevant in this specific case.  
> > 
> > GTK! There is a whole bunch of old Mellanox NICs reported so I think
> > there is _some_ server coverage. I'm leaning towards applying the patch.

There are 1182 servers per
https://linux-hardware.org/?view=computers&type=Server

> Looks like S2IO became Neterion and then was acquired by Exar in 2010.
> Then MaxLinear acquired Exar in 2017.
> Looks like they dropped out of NIC business and only do switches??

Maxlinear keeps some archive material around:
https://www.maxlinear.com/Files/Documents/X3100Linux_GeneralInfo-FAQ.pdf

A search on linux-hardware with S2io vendor id from above (17d5) does
not find any part.

Patches aside, MARC does not find recent vxge material.

The driver is stable and the systems wherein it is used are (too) stable
as well. See for instance:

https://bugzilla.redhat.com/buglist.cgi?bug_status=__all__&content=vxge

Active maintenance seems useless.

-- 
Ueimor

Re: [PATCH net-next] eth: remove neterion/vxge

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net-next.git (master)
by Jakub Kicinski <kuba@kernel.org>:

On Thu, 30 Jun 2022 21:42:34 -0700 you wrote:
> The last meaningful change to this driver was made by Jon in 2011.
> As much as we'd like to believe that this is because the code is
> perfect the chances are nobody is using this hardware.
> 
> Because of the size of this driver there is a nontrivial maintenance
> cost to keeping this code around, in the last 2 years we're averaging
> more than 1 change a month. Some of which require nontrivial review
> effort, see commit 877fe9d49b74 ("Revert "drivers/net/ethernet/neterion/vxge:
> Fix a use-after-free bug in vxge-main.c"") for example.
> 
> [...]

Here is the summary with links:
  - [net-next] eth: remove neterion/vxge
    https://git.kernel.org/netdev/net-next/c/f05643a0f60b

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html