mbox series

[RFC,00/13,net-next] drivers/net/Space.c cleanup

Message ID 20210515221320.1255291-1-arnd@kernel.org (mailing list archive)
Headers show
Series drivers/net/Space.c cleanup | expand

Message

Arnd Bergmann May 15, 2021, 10:13 p.m. UTC
From: Arnd Bergmann <arnd@arndb.de>

I discovered that there are still a couple of drivers that rely on
beiong statically initialized from drivers/net/Space.c the way
we did in the last century. As it turns out, there are a couple
of simplifications that can be made here, as well as some minor
bugfixes.

There are four classes of drivers that use this:

- most 10mbit ISA bus ethernet drivers (and one 100mbit one)
- both ISA localtalk drivers
- several m68k ethernet drivers
- one obsolete WAN driver

I found that the drivers using in arch/m68k/ don't actually benefit
from being probed this way as they do not rely on the netdev= command
line arguments, they have simply never been changed to work like a
modern driver.

I had previously sent a patch to remove the sbni/granch driver, and
there were no objections to this patch but forgot to resend it after
some discussion about another patch in the same series.

For the ISA drivers, there is usually no way to probe multiple devices
at boot time other than the netdev= arguments, so all that logic is left
in place for the moment, but centralized in a single file that only gets
included in the kernel build if one or more of the drivers are built-in.

I'm also changing the old-style init_module() functions in these drivers
to static functions with a module_init() annotation, to more closely
resemble modern drivers. There are only a few users of the init_module()
interface remaining, and removing that would likely allow some cleanups
in the module loader code.

There are a couple of possible follow-ups:

* Most of ISA drivers could be trivially converted to use the module_init()
  entry point, which would slightly change the command line syntax and
  still support a single device of that type, but not more than one. We
  could decide that this is fine, as few users remain that have any of
  these devices, let alone more than one.

* Alternatively, the fact that the ISA drivers have never been cleaned
  up can be seen as an indication that there isn't really much remaining
  interest in them. We could move them to drivers/staging along with the
  consolidated contents of drivers/net/Space.c and see if anyone still
  uses them and eventually remove the ones that nobody has.
  I can see that Paul Gortmaker removed a number of less common ISA
  ethernet drivers in 2013, but at the time left these because they
  were possibly still relevant.

* If we end up moving the cops localtalk driver to staging, support
  for localtalk devices (though probably not appletalk over ethernet)
  can arguably meet the same fate.

If someone wants to work on those follow-ups or thinks they are a
good idea, let me know, otherwise I'd leave it at this cleanup.

       Arnd

Arnd Bergmann (13):
  [net-next] bcmgenet: remove call to netdev_boot_setup_check
  [net-next] natsemi: sonic: stop calling netdev_boot_setup_check
  [net-next] appletalk: ltpc: remove static probing
  [net-next] 3c509: stop calling netdev_boot_setup_check
  [net-next] cs89x0: rework driver configuration
  [net-next] m68k: remove legacy probing
  [net-next] move netdev_boot_setup into Space.c
  [net-next] make legacy ISA probe optional
  [net-next] wan: remove stale Kconfig entries
  [net-next] wan: remove sbni/granch driver
  [net-next] wan: hostess_sv11: use module_init/module_exit helpers
  [net-next] ethernet: isa: convert to module_init/module_exit
  [net-next] 8390: xsurf100: avoid including lib8390.c

 .../admin-guide/kernel-parameters.txt         |    2 -
 drivers/net/Kconfig                           |    7 +
 drivers/net/Makefile                          |    3 +-
 drivers/net/Space.c                           |  178 +-
 drivers/net/appletalk/Kconfig                 |    4 +-
 drivers/net/appletalk/ltpc.c                  |    7 +-
 drivers/net/ethernet/3com/3c509.c             |    3 -
 drivers/net/ethernet/3com/3c515.c             |    3 +-
 drivers/net/ethernet/3com/Kconfig             |    1 +
 drivers/net/ethernet/8390/Kconfig             |    3 +
 drivers/net/ethernet/8390/Makefile            |    2 +-
 drivers/net/ethernet/8390/apne.c              |   11 +-
 drivers/net/ethernet/8390/ne.c                |    5 +-
 drivers/net/ethernet/8390/smc-ultra.c         |    9 +-
 drivers/net/ethernet/8390/wd.c                |    7 +-
 drivers/net/ethernet/8390/xsurf100.c          |    7 +-
 drivers/net/ethernet/amd/Kconfig              |    2 +
 drivers/net/ethernet/amd/atarilance.c         |   11 +-
 drivers/net/ethernet/amd/lance.c              |    6 +-
 drivers/net/ethernet/amd/mvme147.c            |   16 +-
 drivers/net/ethernet/amd/ni65.c               |    6 +-
 drivers/net/ethernet/amd/sun3lance.c          |   19 +-
 .../net/ethernet/broadcom/genet/bcmgenet.c    |    2 -
 drivers/net/ethernet/cirrus/Kconfig           |   27 +-
 drivers/net/ethernet/cirrus/cs89x0.c          |   31 +-
 drivers/net/ethernet/i825xx/82596.c           |   24 +-
 drivers/net/ethernet/i825xx/sun3_82586.c      |   17 +-
 drivers/net/ethernet/natsemi/jazzsonic.c      |    2 -
 drivers/net/ethernet/natsemi/xtsonic.c        |    1 -
 drivers/net/ethernet/smsc/Kconfig             |    1 +
 drivers/net/ethernet/smsc/smc9194.c           |    6 +-
 drivers/net/wan/Kconfig                       |   51 -
 drivers/net/wan/Makefile                      |    1 -
 drivers/net/wan/hostess_sv11.c                |    6 +-
 drivers/net/wan/sbni.c                        | 1638 -----------------
 drivers/net/wan/sbni.h                        |  147 --
 include/linux/netdevice.h                     |   13 -
 include/net/Space.h                           |   10 -
 net/core/dev.c                                |  125 --
 net/ethernet/eth.c                            |    2 -
 40 files changed, 259 insertions(+), 2157 deletions(-)
 delete mode 100644 drivers/net/wan/sbni.c
 delete mode 100644 drivers/net/wan/sbni.h

Comments

Maciej W. Rozycki May 16, 2021, 12:06 a.m. UTC | #1
On Sun, 16 May 2021, Arnd Bergmann wrote:

> For the ISA drivers, there is usually no way to probe multiple devices
> at boot time other than the netdev= arguments, so all that logic is left
> in place for the moment, but centralized in a single file that only gets
> included in the kernel build if one or more of the drivers are built-in.

 As I recall at least some ISA drivers did probe multiple interfaces in 
their monolithic configuration; I used a configuration with as many as 
five ne2k clones for a bridge machine for a fairly large network (~300 
machines) and as I recall it required no command-line parameters (but then 
it was some 25 years ago, so I may well not remember correctly anymore).  
It may have been with ISA PnP though (damn!).

 For modular drivers it was deemed too dangerous, for obvious reasons, and 
explicit parameters were the only way.

> * Most of ISA drivers could be trivially converted to use the module_init()
>   entry point, which would slightly change the command line syntax and
>   still support a single device of that type, but not more than one. We
>   could decide that this is fine, as few users remain that have any of
>   these devices, let alone more than one.
> 
> * Alternatively, the fact that the ISA drivers have never been cleaned
>   up can be seen as an indication that there isn't really much remaining
>   interest in them. We could move them to drivers/staging along with the
>   consolidated contents of drivers/net/Space.c and see if anyone still
>   uses them and eventually remove the ones that nobody has.

 I have a 3c509b interface in active use (although in the EISA mode, so no 
need for weird probing, but it can be reconfigured), and I have an ne2k 
clone in storage, so I could do some run-time verification if there is no 
one else available.  I'll see if I can do some driver polishing for these 
devices, but given the number of other tasks on my table this is somewhat 
low priority for me.

  Maciej
Arnd Bergmann May 16, 2021, 9:27 a.m. UTC | #2
On Sun, May 16, 2021 at 2:06 AM Maciej W. Rozycki <macro@orcam.me.uk> wrote:
>
> On Sun, 16 May 2021, Arnd Bergmann wrote:
>
> > For the ISA drivers, there is usually no way to probe multiple devices
> > at boot time other than the netdev= arguments, so all that logic is left
> > in place for the moment, but centralized in a single file that only gets
> > included in the kernel build if one or more of the drivers are built-in.
>
>  As I recall at least some ISA drivers did probe multiple interfaces in
> their monolithic configuration; I used a configuration with as many as
> five ne2k clones for a bridge machine for a fairly large network (~300
> machines) and as I recall it required no command-line parameters (but then
> it was some 25 years ago, so I may well not remember correctly anymore).
> It may have been with ISA PnP though (damn!).
>
>  For modular drivers it was deemed too dangerous, for obvious reasons, and
> explicit parameters were the only way.

I did use two ne2k compatible pre-PnP cards in a router, by loading two
copies of the same driver module, and I'm fairly sure that would still work,
but it's obviously not how it was meant to be used. ;-)

To clarify: the code drivers/net/Space.c logic can probe these drivers
both with and without explicit configuration. Using the netdev= arguments
will result in reliable assignment of device names and prevent drivers
from poking ports that may be used by other devices, but drivers can
also implement their own device detection, in case of
drivers/net/ethernet/8390/ne.c, it can find devices at up to six addresses:
static unsigned int netcard_portlist[] __initdata = {
    0x300, 0x280, 0x320, 0x340, 0x360, 0x380, 0
};

> > * Most of ISA drivers could be trivially converted to use the module_init()
> >   entry point, which would slightly change the command line syntax and
> >   still support a single device of that type, but not more than one. We
> >   could decide that this is fine, as few users remain that have any of
> >   these devices, let alone more than one.
> >
> > * Alternatively, the fact that the ISA drivers have never been cleaned
> >   up can be seen as an indication that there isn't really much remaining
> >   interest in them. We could move them to drivers/staging along with the
> >   consolidated contents of drivers/net/Space.c and see if anyone still
> >   uses them and eventually remove the ones that nobody has.
>
>  I have a 3c509b interface in active use (although in the EISA mode, so no
> need for weird probing, but it can be reconfigured),

The 3c509 driver already doesn't use the drivers/net/Space.c probing,
Marc Zyngier addressed this when he added EISA support in 2003.
It also supports multiple non-PnP cards with module parameters
as well as pre-standard jumperless autoconfiguration. My only change
to this driver was to remove a few lines of dead code that I guess Marc
missed at the time.

I probably should have listed the drivers that still use Space.c after
my series. Here is the list I'm talking about:

3com:
    3c515/corkscrew: supports PnP mode, ignores command line and
        module parameters but scans for devices on all ports
8390:
   wd80x3: supports module parameters up to four cards, no PnP
   smc-ultra: supports up to four devices with module parameters, plus PnP
   ne2k/iosa: supports up to four devices with module parameters, plus PnP
amd
   lance: supports up to eight devices with module parameters, no PnP
   ni65: supports only one device as loadable module, no PnP
smsc
   smc9194: supports only one device as loadable module, no PnP
cirrus
   cs89x0 (supports only one device as loadable module, plus custom
        PnP-like probing in built-in mode)

> and I have an ne2k clone in storage, so I could do some run-time
> verification if there is no one else available.  I'll see if I can do some driver
> polishing for these devices, but given the number of other tasks on my
> table this is somewhat low priority for me.

I don't think it's necessary to validate the changes I did to the
ne2k driver specifically, though it might be nice to see that I didn't
break the overall logic in Space.c.

        Arnd
Paul Gortmaker May 17, 2021, 2:38 p.m. UTC | #3
[[RFC 00/13] [net-next] drivers/net/Space.c cleanup] On 16/05/2021 (Sun 00:13) Arnd Bergmann wrote:

> From: Arnd Bergmann <arnd@arndb.de>

[...]

> * Alternatively, the fact that the ISA drivers have never been cleaned
>   up can be seen as an indication that there isn't really much remaining
>   interest in them. We could move them to drivers/staging along with the
>   consolidated contents of drivers/net/Space.c and see if anyone still
>   uses them and eventually remove the ones that nobody has.
>   I can see that Paul Gortmaker removed a number of less common ISA
>   ethernet drivers in 2013, but at the time left these because they
>   were possibly still relevant.

It was a thin/remote possibility, even then:  From v3.9-rc1~139^2~288

  In theory, it is possible that someone could still be running one of these
  12+ year old P3 machines and want 3.9+ bleeding edge kernels (but unlikely).
  In light of the above (remote) possibility, we can defer the removal of some
  ISA network drivers that were highly popular and well tested.  Typically
  that means the stuff more from the mid to late '90s, some with ISA PnP
  support, like the 3c509, the wd/SMC 8390 based stuff, PCnet/lance etc.

Now those 12y old machines are 20y old, and bleeding edge for this discussion
will be the the v5.14+ kernel.

Leaving the more popular cards was a concession to making progress vs.
having the whole cleanup blocked by individuals who haven't yet realized
that using ancient hardware is best done (only done?) with ancient kernels.

Maybe things are better now; people are putting more consideration to
the future of the kernel, rather than clinging to times long past?
We've since managed to delete several complete old arch dirs; which I
had previously thought impossible.  I couldn't even git rid of the x86
EISA support code six years ago.[1]

I'd be willing to do a "Phase 2" of 930d52c012b8 ISA net delete;  I'm
not sure the bounce through stable on the way out does much other than
muddy the git history.  I'd be tempted to just propose the individual
deletes and see where that goes....

Paul.
--

[1] https://lwn.net/Articles/630115/

> 
> * If we end up moving the cops localtalk driver to staging, support
>   for localtalk devices (though probably not appletalk over ethernet)
>   can arguably meet the same fate.
> 
> If someone wants to work on those follow-ups or thinks they are a
> good idea, let me know, otherwise I'd leave it at this cleanup.
> 
>        Arnd
> 
> Arnd Bergmann (13):
>   [net-next] bcmgenet: remove call to netdev_boot_setup_check
>   [net-next] natsemi: sonic: stop calling netdev_boot_setup_check
>   [net-next] appletalk: ltpc: remove static probing
>   [net-next] 3c509: stop calling netdev_boot_setup_check
>   [net-next] cs89x0: rework driver configuration
>   [net-next] m68k: remove legacy probing
>   [net-next] move netdev_boot_setup into Space.c
>   [net-next] make legacy ISA probe optional
>   [net-next] wan: remove stale Kconfig entries
>   [net-next] wan: remove sbni/granch driver
>   [net-next] wan: hostess_sv11: use module_init/module_exit helpers
>   [net-next] ethernet: isa: convert to module_init/module_exit
>   [net-next] 8390: xsurf100: avoid including lib8390.c
> 
>  .../admin-guide/kernel-parameters.txt         |    2 -
>  drivers/net/Kconfig                           |    7 +
>  drivers/net/Makefile                          |    3 +-
>  drivers/net/Space.c                           |  178 +-
>  drivers/net/appletalk/Kconfig                 |    4 +-
>  drivers/net/appletalk/ltpc.c                  |    7 +-
>  drivers/net/ethernet/3com/3c509.c             |    3 -
>  drivers/net/ethernet/3com/3c515.c             |    3 +-
>  drivers/net/ethernet/3com/Kconfig             |    1 +
>  drivers/net/ethernet/8390/Kconfig             |    3 +
>  drivers/net/ethernet/8390/Makefile            |    2 +-
>  drivers/net/ethernet/8390/apne.c              |   11 +-
>  drivers/net/ethernet/8390/ne.c                |    5 +-
>  drivers/net/ethernet/8390/smc-ultra.c         |    9 +-
>  drivers/net/ethernet/8390/wd.c                |    7 +-
>  drivers/net/ethernet/8390/xsurf100.c          |    7 +-
>  drivers/net/ethernet/amd/Kconfig              |    2 +
>  drivers/net/ethernet/amd/atarilance.c         |   11 +-
>  drivers/net/ethernet/amd/lance.c              |    6 +-
>  drivers/net/ethernet/amd/mvme147.c            |   16 +-
>  drivers/net/ethernet/amd/ni65.c               |    6 +-
>  drivers/net/ethernet/amd/sun3lance.c          |   19 +-
>  .../net/ethernet/broadcom/genet/bcmgenet.c    |    2 -
>  drivers/net/ethernet/cirrus/Kconfig           |   27 +-
>  drivers/net/ethernet/cirrus/cs89x0.c          |   31 +-
>  drivers/net/ethernet/i825xx/82596.c           |   24 +-
>  drivers/net/ethernet/i825xx/sun3_82586.c      |   17 +-
>  drivers/net/ethernet/natsemi/jazzsonic.c      |    2 -
>  drivers/net/ethernet/natsemi/xtsonic.c        |    1 -
>  drivers/net/ethernet/smsc/Kconfig             |    1 +
>  drivers/net/ethernet/smsc/smc9194.c           |    6 +-
>  drivers/net/wan/Kconfig                       |   51 -
>  drivers/net/wan/Makefile                      |    1 -
>  drivers/net/wan/hostess_sv11.c                |    6 +-
>  drivers/net/wan/sbni.c                        | 1638 -----------------
>  drivers/net/wan/sbni.h                        |  147 --
>  include/linux/netdevice.h                     |   13 -
>  include/net/Space.h                           |   10 -
>  net/core/dev.c                                |  125 --
>  net/ethernet/eth.c                            |    2 -
>  40 files changed, 259 insertions(+), 2157 deletions(-)
>  delete mode 100644 drivers/net/wan/sbni.c
>  delete mode 100644 drivers/net/wan/sbni.h
> 
> -- 
> 2.29.2
> 
> Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
> Cc: "Maciej W. Rozycki" <macro@orcam.me.uk>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Doug Berger <opendmb@gmail.com>
> Cc: Florian Fainelli <f.fainelli@gmail.com>
> Cc: Sam Creasey <sammy@sammy.net>
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Finn Thain <fthain@telegraphics.com.au>
> Cc: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Andrii Nakryiko <andriin@fb.com>
> Cc: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> Cc: netdev@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: bcm-kernel-feedback-list@broadcom.com
Maciej W. Rozycki May 17, 2021, 3:40 p.m. UTC | #4
On Mon, 17 May 2021, Paul Gortmaker wrote:

> Leaving the more popular cards was a concession to making progress vs.
> having the whole cleanup blocked by individuals who haven't yet realized
> that using ancient hardware is best done (only done?) with ancient kernels.

 Hmm, it depends on what you want to achieve, although I think it will be 
fair if you require anyone caring about old hardware to keep any relevant 
code base, such as drivers, board support, etc. up to date as our internal 
interfaces evolve.  Otherwise if it works, then I fail to see a reason to 
remove it just because you can.

> Maybe things are better now; people are putting more consideration to
> the future of the kernel, rather than clinging to times long past?
> We've since managed to delete several complete old arch dirs; which I
> had previously thought impossible.  I couldn't even git rid of the x86
> EISA support code six years ago.[1]

 Heh, that machine I raised my objection for is now back in service, after 
a failure of its industrial PSU and the installation of a replacement one 
(which was a bit of a challenge to track down), serving the maintenance of 
the defxx driver for the DEFEA (EISA) variant of the DEC FDDI network 
adapter:

platform eisa.0: Probing EISA bus 0
eisa 00:00: EISA: Mainboard AEI0401 detected
eisa 00:05: EISA: slot 5: DEC3002 detected
defxx: v1.12 2021/03/10  Lawrence V. Stefani and others
00:05: DEFEA at I/O addr = 0x5000, IRQ = 10, Hardware addr = 00-00-f8-c8-b3-b6
00:05: registered as fddi0
eisa 00:06: EISA: slot 6: NPI0303 detected
eisa 00:08: EISA: slot 8: TCM5094 detected
eth0: 3c5x9 found at 0x8000, 10baseT port, address 00:a0:24:b6:8b:db, IRQ 12.
platform eisa.0: EISA: Detected 3 cards

I just need to upgrade it with more DRAM (256MiB supported; not too bad 
for an i486) so that it runs a tad more smoothly.  An alternative would be 
switching to an EISA Alpha machine, however I don't have any at the moment 
and chasing one would be a bit of an issue.

 FAOD I keep getting contacted about the FDDI stuff as it remains in use 
in various industrial and scientific installations and the typical use 
case nowadays is getting whatever old gear, which has fallen apart, they 
have used to communicate with their equipment over FDDI with a modern 
Linux machine, preferably running out of the box with one of the readily 
available distributions, and one of those FDDI cards (normally the PCI 
variant though), which are reasonably available still.

> I'd be willing to do a "Phase 2" of 930d52c012b8 ISA net delete;  I'm
> not sure the bounce through stable on the way out does much other than
> muddy the git history.  I'd be tempted to just propose the individual
> deletes and see where that goes....

 Sounds fair to me.

  Maciej
Arnd Bergmann May 17, 2021, 3:49 p.m. UTC | #5
On Mon, May 17, 2021 at 4:38 PM Paul Gortmaker
<paul.gortmaker@windriver.com> wrote:
> > From: Arnd Bergmann <arnd@arndb.de>
>
> I'd be willing to do a "Phase 2" of 930d52c012b8 ISA net delete;  I'm
> not sure the bounce through stable on the way out does much other than
> muddy the git history.  I'd be tempted to just propose the individual
> deletes and see where that goes....

I think the main benefit of going through drivers/staging would be that
it could be done somewhat more aggressively by moving more of the
presumed-unused hardware out at once, and waiting for media
coverage to wake up any remaining users. Then again, reverting
a removal isn't that different from reverting the move to staging.

If we do it one driver at a time, a good start might be the ones that
don't already have support for probing multiple devices in a loadable
module (ni65,smc9194, cs89x0-isa, cops, ltpc), which would let us
kill off drivers/net/Space.c by always using the module_init() logic.
These are all non-ISAPnP devices, and a second step could be
to remove support for all non-PNP devices (lance, wd80x3,
corkscrew and maybe the ISA WAN devices).

Unless we can agree to remove CONFIG_ISA completely, it's probably
best to leave ISA support for those drivers that also work on PCI, EISA
or m68k hardware, as those have the best chance to still get tested.
3c509 is probably the best maintained, given that it's the only network
driver that got converted to isa_register_driver() from the linux-2.0
style static probing. 3c515 has the distinction of being the only
remaining 100mbit one, and ne2k is obviously the one that one can
still find on ebay or the parent's attic.

       Arnd
Finn Thain May 17, 2021, 11:04 p.m. UTC | #6
On Mon, 17 May 2021, Paul Gortmaker wrote:

> Leaving the more popular cards was a concession to making progress vs. 
> having the whole cleanup blocked by individuals who haven't yet realized 
> that using ancient hardware is best done (only done?) with ancient 
> kernels.
> 

By extension, using ancient kernels is best done with ancient userland. 
And now the time has come to remove all the 'compat' nonsense.

Oh, wait, all of that old software seems to be riddled with bugs and 
vulnerabilities.

And who would have thought that all those developers writing new code for 
emulators and their users were holding up "progress"?

> Maybe things are better now; people are putting more consideration to
> the future of the kernel, rather than clinging to times long past?

If you've been doing engineering for a while you'll start to notice a 
pattern: old designs that work get reused. That's partly why the latest 
silicon contains ancient logic blocks.

When that changes, perhaps we can talk about "progress"...