[PATCH net-next] net: dlci: Deprecate the DLCI driver (aka the Frame Relay layer)

[PATCH net-next] net: dlci: Deprecate the DLCI driver (aka the Frame Relay layer)

[Xie He]

I wish to deprecate the Frame Relay layer (dlci.c) in the kernel because
we already have a newer and better "HDLC Frame Relay" layer (hdlc_fr.c).

Reasons why hdlc_fr.c is better than dlci.c include:

1.
dlci.c is dated 1997, while hdlc_fr.c is dated 1999 - 2006, so the later
is newer than the former.

2.
hdlc_fr.c is working well (tested by me). For dlci.c, I cannot even find
the user space problem needed to use it. The link provided in
Documentation/networking/framerelay.rst (in the last paragraph) is no
longer valid.

3.
dlci.c supports only one hardware driver - sdla.c, while hdlc_fr.c
supports many hardware drivers through the generic HDLC layer (hdlc.c).

WAN hardware devices are usually able to support several L2 protocols
at the same time, so the HDLC layer is more suitable for these devices.

The hardware devices that sdla.c supports are also multi-protocol
(according to drivers/net/wan/Kconfig), so the HDLC layer is more
suitable for these devices, too.

4.
hdlc_fr.c supports LMI and supports Ethernet emulation. dlci.c supports
neither according to its code.

5.
include/uapi/linux/if_frad.h, which comes with dlci.c, contains two
structs for ioctl configs (dlci_conf and frad_conf). According to the
comments, these two structs are specially crafted for sdla.c (the only
hardware driver dlci.c supports). I think this makes dlci.c not generic
enough to support other hardware drivers.

Signed-off-by: Xie He <xie.he.0141@gmail.com>
---
 Documentation/networking/framerelay.rst | 3 +++
 drivers/net/wan/dlci.c                  | 2 ++
 drivers/net/wan/sdla.c                  | 3 +++
 3 files changed, 8 insertions(+)

diff --git a/Documentation/networking/framerelay.rst b/Documentation/networking/framerelay.rst
index 6d904399ec6d..92e66fc3dffc 100644
--- a/Documentation/networking/framerelay.rst
+++ b/Documentation/networking/framerelay.rst
@@ -4,6 +4,9 @@
 Frame Relay (FR)
 ================
 
+(Note that this Frame Relay layer is deprecated. New drivers should use the
+HDLC Frame Relay layer instead.)
+
 Frame Relay (FR) support for linux is built into a two tiered system of device
 drivers.  The upper layer implements RFC1490 FR specification, and uses the
 Data Link Connection Identifier (DLCI) as its hardware address.  Usually these
diff --git a/drivers/net/wan/dlci.c b/drivers/net/wan/dlci.c
index 3ca4daf63389..1f0eee10c13f 100644
--- a/drivers/net/wan/dlci.c
+++ b/drivers/net/wan/dlci.c
@@ -514,6 +514,8 @@ static int __init init_dlci(void)
 	register_netdevice_notifier(&dlci_notifier);
 
 	printk("%s.\n", version);
+	pr_warn("The DLCI driver (the Frame Relay layer) is deprecated.\n"
+		"Please move your driver to the HDLC Frame Relay layer.\n");
 
 	return 0;
 }
diff --git a/drivers/net/wan/sdla.c b/drivers/net/wan/sdla.c
index bc2c1c7fb1a4..21d602f698fc 100644
--- a/drivers/net/wan/sdla.c
+++ b/drivers/net/wan/sdla.c
@@ -1623,6 +1623,9 @@ static int __init init_sdla(void)
 	int err;
 
 	printk("%s.\n", version);
+	pr_warn("The SDLA driver is deprecated.\n"
+		"If you are still using the hardware,\n"
+		"please help move this driver to the HDLC Frame Relay layer.\n");
 
 	sdla = alloc_netdev(sizeof(struct frad_local), "sdla0",
 			    NET_NAME_UNKNOWN, setup_sdla);
-- 
2.25.1

Re: [PATCH net-next] net: dlci: Deprecate the DLCI driver (aka the Frame Relay layer)

[Jakub Kicinski]

On Wed, 28 Oct 2020 00:05:04 -0700 Xie He wrote:
> I wish to deprecate the Frame Relay layer (dlci.c) in the kernel because
> we already have a newer and better "HDLC Frame Relay" layer (hdlc_fr.c).
> 
> Reasons why hdlc_fr.c is better than dlci.c include:
> 
> 1.
> dlci.c is dated 1997, while hdlc_fr.c is dated 1999 - 2006, so the later
> is newer than the former.
> 
> 2.
> hdlc_fr.c is working well (tested by me). For dlci.c, I cannot even find
> the user space problem needed to use it. The link provided in
> Documentation/networking/framerelay.rst (in the last paragraph) is no
> longer valid.
> 
> 3.
> dlci.c supports only one hardware driver - sdla.c, while hdlc_fr.c
> supports many hardware drivers through the generic HDLC layer (hdlc.c).
> 
> WAN hardware devices are usually able to support several L2 protocols
> at the same time, so the HDLC layer is more suitable for these devices.
> 
> The hardware devices that sdla.c supports are also multi-protocol
> (according to drivers/net/wan/Kconfig), so the HDLC layer is more
> suitable for these devices, too.
> 
> 4.
> hdlc_fr.c supports LMI and supports Ethernet emulation. dlci.c supports
> neither according to its code.
> 
> 5.
> include/uapi/linux/if_frad.h, which comes with dlci.c, contains two
> structs for ioctl configs (dlci_conf and frad_conf). According to the
> comments, these two structs are specially crafted for sdla.c (the only
> hardware driver dlci.c supports). I think this makes dlci.c not generic
> enough to support other hardware drivers.
> 
> Signed-off-by: Xie He <xie.he.0141@gmail.com>

This code has only seen cleanup patches since the git era begun -
do you think that it may still have users? Or is it completely unused?

The usual way of getting rid of old code is to move it to staging/
for a few releases then delete it, like Arnd just did with wimax.

> diff --git a/Documentation/networking/framerelay.rst b/Documentation/networking/framerelay.rst
> index 6d904399ec6d..92e66fc3dffc 100644
> --- a/Documentation/networking/framerelay.rst
> +++ b/Documentation/networking/framerelay.rst
> @@ -4,6 +4,9 @@
>  Frame Relay (FR)
>  ================
>  
> +(Note that this Frame Relay layer is deprecated. New drivers should use the
> +HDLC Frame Relay layer instead.)
> +
>  Frame Relay (FR) support for linux is built into a two tiered system of device
>  drivers.  The upper layer implements RFC1490 FR specification, and uses the
>  Data Link Connection Identifier (DLCI) as its hardware address.  Usually these
> diff --git a/drivers/net/wan/dlci.c b/drivers/net/wan/dlci.c
> index 3ca4daf63389..1f0eee10c13f 100644
> --- a/drivers/net/wan/dlci.c
> +++ b/drivers/net/wan/dlci.c
> @@ -514,6 +514,8 @@ static int __init init_dlci(void)
>  	register_netdevice_notifier(&dlci_notifier);
>  
>  	printk("%s.\n", version);
> +	pr_warn("The DLCI driver (the Frame Relay layer) is deprecated.\n"
> +		"Please move your driver to the HDLC Frame Relay layer.\n");
>  
>  	return 0;
>  }
> diff --git a/drivers/net/wan/sdla.c b/drivers/net/wan/sdla.c
> index bc2c1c7fb1a4..21d602f698fc 100644
> --- a/drivers/net/wan/sdla.c
> +++ b/drivers/net/wan/sdla.c
> @@ -1623,6 +1623,9 @@ static int __init init_sdla(void)
>  	int err;
>  
>  	printk("%s.\n", version);
> +	pr_warn("The SDLA driver is deprecated.\n"
> +		"If you are still using the hardware,\n"
> +		"please help move this driver to the HDLC Frame Relay layer.\n");
>  
>  	sdla = alloc_netdev(sizeof(struct frad_local), "sdla0",
>  			    NET_NAME_UNKNOWN, setup_sdla);

Re: [PATCH net-next] net: dlci: Deprecate the DLCI driver (aka the Frame Relay layer)

[Xie He]

On Fri, Oct 30, 2020 at 8:07 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> This code has only seen cleanup patches since the git era begun -
> do you think that it may still have users? Or is it completely unused?

I don't think it is still used. But I don't have solid evidence. So I
asked people in the warning messages to move to the newer HDLC Frame
Relay layer.

Some evidence indicating this is not in use might be:

1. In "Documentation/networking/framerelay.rst", in the last
paragraph, the link for the user space programs needed for
configuration has been broken.

2. The Frame Relay layer supports only one hardware driver in the
kernel. I looked up the hardware on the vendor's website, and it
seemed they had their own drivers for the hardware. I guess most users
would use the driver provided by the vendor instead.

The names of the hardware supported by our sdla.c driver appear in this page:
    http://ftp.sangoma.com/technote/INDEX
According to this file in the same directory, the driver provided by
the vendor is named WANPIPE and supports multi-protocol (unlike sdla.c
/ dlci.c but similar to hdlc_fr.c):
    http://ftp.sangoma.com/technote/tn0015.txt
According to the WANPIPE installation guide here, the vendor
explicitly requested users to disable dlci.c in the kernel, by
answering NO to CONFIG_DLCI:
    http://ftp.sangoma.com/linux/current_wanpipe/doc/WanpipeInstallation.pdf

3. According to drivers/net/wan/Kconfig, sdla.c (the only hardware
driver dlci.c supports) depends on CONFIG_ISA. According to Wikipedia,
ISA is an old 16-bit bus system. I think this makes users of this
hardware driver rare.

4. Frame Relay itself is an old technology. I'm not clear about its
overall usage but if it's still used its usage must be declining.

> The usual way of getting rid of old code is to move it to staging/
> for a few releases then delete it, like Arnd just did with wimax.

Oh. OK. But I see "include/linux/if_frad.h" is included in
"net/socket.c", and there's still some code in "net/socket.c" related
to it. If we move all these files to "staging/", we need to change the
"include" line in "net/socket.c" to point to the new location, and we
still need to keep a little code in "net/socket.c". So I think if we
move it to "staging/", we can't do this in a clean way.

Re: [PATCH net-next] net: dlci: Deprecate the DLCI driver (aka the Frame Relay layer)

[Jakub Kicinski]

On Fri, 30 Oct 2020 22:10:42 -0700 Xie He wrote:
> > The usual way of getting rid of old code is to move it to staging/
> > for a few releases then delete it, like Arnd just did with wimax.  
> 
> Oh. OK. But I see "include/linux/if_frad.h" is included in
> "net/socket.c", and there's still some code in "net/socket.c" related
> to it. If we move all these files to "staging/", we need to change the
> "include" line in "net/socket.c" to point to the new location, and we
> still need to keep a little code in "net/socket.c". So I think if we
> move it to "staging/", we can't do this in a clean way.

I'd just place that code under appropriate #ifdef CONFIG_ so we don't
forget to remove it later.  It's just the dlci_ioctl_hook, right?

Maybe others have better ideas, Arnd?

Re: [PATCH net-next] net: dlci: Deprecate the DLCI driver (aka the Frame Relay layer)

[Xie He]

On Sat, Oct 31, 2020 at 9:51 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> > > The usual way of getting rid of old code is to move it to staging/
> > > for a few releases then delete it, like Arnd just did with wimax.
> >
> > Oh. OK. But I see "include/linux/if_frad.h" is included in
> > "net/socket.c", and there's still some code in "net/socket.c" related
> > to it. If we move all these files to "staging/", we need to change the
> > "include" line in "net/socket.c" to point to the new location, and we
> > still need to keep a little code in "net/socket.c". So I think if we
> > move it to "staging/", we can't do this in a clean way.
>
> I'd just place that code under appropriate #ifdef CONFIG_ so we don't
> forget to remove it later.  It's just the dlci_ioctl_hook, right?

Searching "dlci" (case insensitive) in "net/socket.c", I found there
were 3 areas with "dlci.c" related code. I also found there were 2
macro definitions in "include/uapi/linux/sockios.h".

> Maybe others have better ideas, Arnd?

Re: [PATCH net-next] net: dlci: Deprecate the DLCI driver (aka the Frame Relay layer)

[Arnd Bergmann]

On Sat, Oct 31, 2020 at 5:53 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Fri, 30 Oct 2020 22:10:42 -0700 Xie He wrote:
> > > The usual way of getting rid of old code is to move it to staging/
> > > for a few releases then delete it, like Arnd just did with wimax.
> >
> > Oh. OK. But I see "include/linux/if_frad.h" is included in
> > "net/socket.c", and there's still some code in "net/socket.c" related
> > to it. If we move all these files to "staging/", we need to change the
> > "include" line in "net/socket.c" to point to the new location, and we
> > still need to keep a little code in "net/socket.c". So I think if we
> > move it to "staging/", we can't do this in a clean way.
>
> I'd just place that code under appropriate #ifdef CONFIG_ so we don't
> forget to remove it later.  It's just the dlci_ioctl_hook, right?
>
> Maybe others have better ideas, Arnd?

I think it can just go in the bin directly. I actually submitted a couple of
patches to clean up drivers/net/wan last year but didn't follow up
with a new version after we decided that x.25 is still needed, see
https://lore.kernel.org/netdev/20191209151256.2497534-1-arnd@arndb.de/

I can resubmit if you like.

      Arnd

Re: [PATCH net-next] net: dlci: Deprecate the DLCI driver (aka the Frame Relay layer)

[Jakub Kicinski]

On Sat, 31 Oct 2020 22:41:30 +0100 Arnd Bergmann wrote:
> On Sat, Oct 31, 2020 at 5:53 PM Jakub Kicinski <kuba@kernel.org> wrote:
> >
> > On Fri, 30 Oct 2020 22:10:42 -0700 Xie He wrote:  
> > > > The usual way of getting rid of old code is to move it to staging/
> > > > for a few releases then delete it, like Arnd just did with wimax.  
> > >
> > > Oh. OK. But I see "include/linux/if_frad.h" is included in
> > > "net/socket.c", and there's still some code in "net/socket.c" related
> > > to it. If we move all these files to "staging/", we need to change the
> > > "include" line in "net/socket.c" to point to the new location, and we
> > > still need to keep a little code in "net/socket.c". So I think if we
> > > move it to "staging/", we can't do this in a clean way.  
> >
> > I'd just place that code under appropriate #ifdef CONFIG_ so we don't
> > forget to remove it later.  It's just the dlci_ioctl_hook, right?
> >
> > Maybe others have better ideas, Arnd?  
> 
> I think it can just go in the bin directly.

Ack, fine by me.

> I actually submitted a couple of patches to clean up drivers/net/wan
> last year but didn't follow up with a new version after we decided
> that x.25 is still needed, see
> https://lore.kernel.org/netdev/20191209151256.2497534-1-arnd@arndb.de/
> 
> I can resubmit if you like.

Let's just leave it at DLCI/SDLA for now, we can revisit once Dave 
is back :)

Re: [PATCH net-next] net: dlci: Deprecate the DLCI driver (aka the Frame Relay layer)

[Xie He]

On Sat, Oct 31, 2020 at 2:41 PM Arnd Bergmann <arnd@kernel.org> wrote:
>
> I think it can just go in the bin directly. I actually submitted a couple of
> patches to clean up drivers/net/wan last year but didn't follow up
> with a new version after we decided that x.25 is still needed, see
> https://lore.kernel.org/netdev/20191209151256.2497534-1-arnd@arndb.de/
>
> I can resubmit if you like.

Should we also remove the two macro definitions in
"include/uapi/linux/sockios.h" (SIOCADDDLCI / SIOCDELDLCI), too? It
seems to be not included in your original patch.

Re: [PATCH net-next] net: dlci: Deprecate the DLCI driver (aka the Frame Relay layer)

[Arnd Bergmann]

On Sun, Nov 1, 2020 at 12:37 AM Xie He <xie.he.0141@gmail.com> wrote:
>
> On Sat, Oct 31, 2020 at 2:41 PM Arnd Bergmann <arnd@kernel.org> wrote:
> >
> > I think it can just go in the bin directly. I actually submitted a couple of
> > patches to clean up drivers/net/wan last year but didn't follow up
> > with a new version after we decided that x.25 is still needed, see
> > https://lore.kernel.org/netdev/20191209151256.2497534-1-arnd@arndb.de/
> >
> > I can resubmit if you like.
>
> Should we also remove the two macro definitions in
> "include/uapi/linux/sockios.h" (SIOCADDDLCI / SIOCDELDLCI), too? It
> seems to be not included in your original patch.

Not sure, it should probably at least be marked as 'obsolete' in the header
like SIOCGIFDIVERT, but removing the definitions might risk that someone
later reuses the numbers for a new command. I don't know if there is an
official policy for this. I see a couple of other definitions in the same file
that have no apparent implementation:
SIOCGIFCOUNT, SIOCDRARP, SIOCGRARP and SIOCSRARP. These
were still referenced in 2.6.12, but only in dead code that has since
been removed.

      arnd

Re: [PATCH net-next] net: dlci: Deprecate the DLCI driver (aka the Frame Relay layer)

[Xie He]

On Sat, Oct 31, 2020 at 3:04 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Sat, 31 Oct 2020 22:41:30 +0100 Arnd Bergmann wrote:
> >
> > I think it can just go in the bin directly.
>
> Ack, fine by me.
>
> > I actually submitted a couple of patches to clean up drivers/net/wan
> > last year but didn't follow up with a new version after we decided
> > that x.25 is still needed, see
> > https://lore.kernel.org/netdev/20191209151256.2497534-1-arnd@arndb.de/
> >
> > I can resubmit if you like.
>
> Let's just leave it at DLCI/SDLA for now, we can revisit once Dave
> is back :)

Hi Arnd,

Can you resubmit your patch to delete the DLCI / SDLA drivers? I
really want them to be deleted. Thank you so much!