diff mbox series

[net-next] net: x25: Remove unimplemented X.25-over-LLC code stubs

Message ID 20201209033346.83742-1-xie.he.0141@gmail.com (mailing list archive)
State Accepted
Commit 13458ffe0a953e17587f172a8e5059c243e6850a
Delegated to: Netdev Maintainers
Headers show
Series [net-next] net: x25: Remove unimplemented X.25-over-LLC code stubs | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for net-next
netdev/subject_prefix success Link
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 74 lines checked
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/header_inline success Link
netdev/stable success Stable not CCed

Commit Message

Xie He Dec. 9, 2020, 3:33 a.m. UTC
According to the X.25 documentation, there was a plan to implement
X.25-over-802.2-LLC. It never finished but left various code stubs in the
X.25 code. At this time it is unlikely that it would ever finish so it
may be better to remove those code stubs.

Also change the documentation to make it clear that this is not a ongoing
plan anymore. Change words like "will" to "could", "would", etc.

Cc: Martin Schiller <ms@dev.tdt.de>
Signed-off-by: Xie He <xie.he.0141@gmail.com>
---
 Documentation/networking/x25.rst | 12 +++++-------
 net/x25/af_x25.c                 |  6 +-----
 net/x25/x25_dev.c                | 13 -------------
 net/x25/x25_route.c              |  7 +------
 4 files changed, 7 insertions(+), 31 deletions(-)

Comments

David Laight Dec. 9, 2020, 9:21 p.m. UTC | #1
From: Xie He
> Sent: 09 December 2020 03:34
> 
> According to the X.25 documentation, there was a plan to implement
> X.25-over-802.2-LLC. It never finished but left various code stubs in the
> X.25 code. At this time it is unlikely that it would ever finish so it
> may be better to remove those code stubs.

I always wondered about running Class 2 transport directly over LLC2
(rather than Class 4 over LLC1).
But the only LLC2 user was netbios - and microsoft's LLC2 was broken.
Not to mention the window probing needed to handle systems that
said they supported a window of (IIRC) 15 but would discard the
5th back to back frame.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
Xie He Dec. 9, 2020, 10:53 p.m. UTC | #2
On Wed, Dec 9, 2020 at 1:21 PM David Laight <David.Laight@aculab.com> wrote:
>
> I always wondered about running Class 2 transport directly over LLC2
> (rather than Class 4 over LLC1).
> But the only LLC2 user was netbios - and microsoft's LLC2 was broken.
> Not to mention the window probing needed to handle systems that
> said they supported a window of (IIRC) 15 but would discard the
> 5th back to back frame.

To me, LLC1 and LLC2 are to Ethernet what UDP and TCP are to IP
networks. I think we can use LLC1 and LLC2 wherever UDP and TCP can be
used, as long as we are in the same LAN and are willing to use MAC
addresses as the addresses. X.25 layer 3 certainly can also run over
LLC2.

Linux actually has support for LLC1 and LLC2. User space programs can
transmit data directly over LLC1 and LLC2 using "AF_LLC" sockets.
David Laight Dec. 10, 2020, 9:14 a.m. UTC | #3
From: Xie He
> Sent: 09 December 2020 22:54
> 
> On Wed, Dec 9, 2020 at 1:21 PM David Laight <David.Laight@aculab.com> wrote:
> >
> > I always wondered about running Class 2 transport directly over LLC2
> > (rather than Class 4 over LLC1).
> > But the only LLC2 user was netbios - and microsoft's LLC2 was broken.
> > Not to mention the window probing needed to handle systems that
> > said they supported a window of (IIRC) 15 but would discard the
> > 5th back to back frame.
> 
> To me, LLC1 and LLC2 are to Ethernet what UDP and TCP are to IP
> networks. I think we can use LLC1 and LLC2 wherever UDP and TCP can be
> used, as long as we are in the same LAN and are willing to use MAC
> addresses as the addresses.

Except that you don't have any where near enough 'ports' so you need
something to demultiplex messages to different applications.

We (ICL) always ran class 4 transport (which does error recovery)
directly over LLC1 using MAC address (a NUL byte for the network layer).
This requires a bridged network and globally unique MAC addresses.
Sending out an LLC reflect packet to the broadcast MAC address used to
generate a couple of thousand responses (many would get discarded
because the bridges got overloaded).

> X.25 layer 3 certainly can also run over LLC2.

You don't need X.25 layer 3.
X.25 layer 2 does error recovery over a point-to-point link.
X.25 layer 3 does switching between machines.
Class 2 transport does multiplexing over a reliable lower layer.
So you normally need all three.

However LLC2 gives you a reliable connection between two machines
(selected by MAC address).
So you should be able to run Class 2 transport (well one of its
4 variants!) directly over LL2.

The advantage over Class 4 transport over LLC1 is that there is
only one set of retransmit buffers (etc) regardless of the number
of connections.

But this is all 30 year old history...

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
Xie He Dec. 10, 2020, 10:17 a.m. UTC | #4
On Thu, Dec 10, 2020 at 1:14 AM David Laight <David.Laight@aculab.com> wrote:
>
> > To me, LLC1 and LLC2 are to Ethernet what UDP and TCP are to IP
> > networks. I think we can use LLC1 and LLC2 wherever UDP and TCP can be
> > used, as long as we are in the same LAN and are willing to use MAC
> > addresses as the addresses.
>
> Except that you don't have any where near enough 'ports' so you need
> something to demultiplex messages to different applications.

Yes, LLC only has 256 "ports" compared to more than 60000 for UDP/TCP.

> We (ICL) always ran class 4 transport (which does error recovery)
> directly over LLC1 using MAC address (a NUL byte for the network layer).
> This requires a bridged network and globally unique MAC addresses.
> Sending out an LLC reflect packet to the broadcast MAC address used to
> generate a couple of thousand responses (many would get discarded
> because the bridges got overloaded).

Wow, You have a really big LAN!

> > X.25 layer 3 certainly can also run over LLC2.
>
> You don't need X.25 layer 3.
> X.25 layer 2 does error recovery over a point-to-point link.
> X.25 layer 3 does switching between machines.
> Class 2 transport does multiplexing over a reliable lower layer.
> So you normally need all three.

Yes, I was just saying X.25 layer 3 can run over any reliable
point-to-point links, including X.25 layer 2, LLC2 and TCP.

> However LLC2 gives you a reliable connection between two machines
> (selected by MAC address).
> So you should be able to run Class 2 transport (well one of its
> 4 variants!) directly over LL2.

Yes.

> The advantage over Class 4 transport over LLC1 is that there is
> only one set of retransmit buffers (etc) regardless of the number
> of connections.

Right. But nowadays we have big enough memories for many buffers, so
it may be preferable to make connections operate independent of each
other. This way one lost frame wouldn't affect all connections. This
is also why HTTP3 moved to QUIC instead of using TCP.

> But this is all 30 year old history...

Haha, we are talking about really old technologies.
David Laight Dec. 10, 2020, 10:34 p.m. UTC | #5
From: Xie He
> Sent: 10 December 2020 10:17
> 
> On Thu, Dec 10, 2020 at 1:14 AM David Laight <David.Laight@aculab.com> wrote:
> >
> > > To me, LLC1 and LLC2 are to Ethernet what UDP and TCP are to IP
> > > networks. I think we can use LLC1 and LLC2 wherever UDP and TCP can be
> > > used, as long as we are in the same LAN and are willing to use MAC
> > > addresses as the addresses.
> >
> > Except that you don't have any where near enough 'ports' so you need
> > something to demultiplex messages to different applications.
> 
> Yes, LLC only has 256 "ports" compared to more than 60000 for UDP/TCP.

And ISO transport separates out the address from the connection-id.
The TSAP (used to select the listening application) is 32 bytes.
If you run the ISO Network layer (which isn't X.25 level 3) on a LAN
you have an additional 24 byte NSAP.

For X.25 level 3 we routed calls to applications using any of (IIRC):
- called number sub-address.
- CUG (closed user group number)
- Some other L3 parameters I can't remember :-)
- TSAP if transport layer also in use.
The only way to pass that down was in a TLV format.
Fortunately we weren't even trying to use BSD style sockets.

> > We (ICL) always ran class 4 transport (which does error recovery)
> > directly over LLC1 using MAC address (a NUL byte for the network layer).
> > This requires a bridged network and globally unique MAC addresses.
> > Sending out an LLC reflect packet to the broadcast MAC address used to
> > generate a couple of thousand responses (many would get discarded
> > because the bridges got overloaded).
> 
> Wow, You have a really big LAN!

I think it 'only' stretched from London to Manchester.
But it might have gone up to Edinburgh.
It wasn't a single collision domain, there were bridges doing
MAC filtering - but they had to be open to broadcast traffic.

It was actually a bad IP broadcast packet that took out all the
unix servers in several cities!
(Zero length in a IP options field caused the code trying to skip
the options to generate the ICMP error to spin.
By the time the corporate network guys came storming into our lab
we'd already got a dump from one system and had found the bad packet.
We never did find out why it got sent - the originating system
wasn't doing anything 'odd'.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
patchwork-bot+netdevbpf@kernel.org Dec. 13, 2020, 1:20 a.m. UTC | #6
Hello:

This patch was applied to netdev/net-next.git (refs/heads/master):

On Tue,  8 Dec 2020 19:33:46 -0800 you wrote:
> According to the X.25 documentation, there was a plan to implement
> X.25-over-802.2-LLC. It never finished but left various code stubs in the
> X.25 code. At this time it is unlikely that it would ever finish so it
> may be better to remove those code stubs.
> 
> Also change the documentation to make it clear that this is not a ongoing
> plan anymore. Change words like "will" to "could", "would", etc.
> 
> [...]

Here is the summary with links:
  - [net-next] net: x25: Remove unimplemented X.25-over-LLC code stubs
    https://git.kernel.org/netdev/net-next/c/13458ffe0a95

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

Patch

diff --git a/Documentation/networking/x25.rst b/Documentation/networking/x25.rst
index 00e45d384ba0..e11d9ebdf9a3 100644
--- a/Documentation/networking/x25.rst
+++ b/Documentation/networking/x25.rst
@@ -19,13 +19,11 @@  implementation of LAPB. Therefore the LAPB modules would be called by
 unintelligent X.25 card drivers and not by intelligent ones, this would
 provide a uniform device driver interface, and simplify configuration.
 
-To confuse matters a little, an 802.2 LLC implementation for Linux is being
-written which will allow X.25 to be run over an Ethernet (or Token Ring) and
-conform with the JNT "Pink Book", this will have a different interface to
-the Packet Layer but there will be no confusion since the class of device
-being served by the LLC will be completely separate from LAPB. The LLC
-implementation is being done as part of another protocol project (SNA) and
-by a different author.
+To confuse matters a little, an 802.2 LLC implementation is also possible
+which could allow X.25 to be run over an Ethernet (or Token Ring) and
+conform with the JNT "Pink Book", this would have a different interface to
+the Packet Layer but there would be no confusion since the class of device
+being served by the LLC would be completely separate from LAPB.
 
 Just when you thought that it could not become more confusing, another
 option appeared, XOT. This allows X.25 Packet Layer frames to operate over
diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c
index d41fffb2507b..ff687b97b2d9 100644
--- a/net/x25/af_x25.c
+++ b/net/x25/af_x25.c
@@ -211,11 +211,7 @@  static int x25_device_event(struct notifier_block *this, unsigned long event,
 	if (!net_eq(dev_net(dev), &init_net))
 		return NOTIFY_DONE;
 
-	if (dev->type == ARPHRD_X25
-#if IS_ENABLED(CONFIG_LLC)
-	 || dev->type == ARPHRD_ETHER
-#endif
-	 ) {
+	if (dev->type == ARPHRD_X25) {
 		switch (event) {
 		case NETDEV_REGISTER:
 		case NETDEV_POST_TYPE_CHANGE:
diff --git a/net/x25/x25_dev.c b/net/x25/x25_dev.c
index 25bf72ee6cad..5259ef8f5242 100644
--- a/net/x25/x25_dev.c
+++ b/net/x25/x25_dev.c
@@ -160,10 +160,6 @@  void x25_establish_link(struct x25_neigh *nb)
 		*ptr = X25_IFACE_CONNECT;
 		break;
 
-#if IS_ENABLED(CONFIG_LLC)
-	case ARPHRD_ETHER:
-		return;
-#endif
 	default:
 		return;
 	}
@@ -179,10 +175,6 @@  void x25_terminate_link(struct x25_neigh *nb)
 	struct sk_buff *skb;
 	unsigned char *ptr;
 
-#if IS_ENABLED(CONFIG_LLC)
-	if (nb->dev->type == ARPHRD_ETHER)
-		return;
-#endif
 	if (nb->dev->type != ARPHRD_X25)
 		return;
 
@@ -212,11 +204,6 @@  void x25_send_frame(struct sk_buff *skb, struct x25_neigh *nb)
 		*dptr = X25_IFACE_DATA;
 		break;
 
-#if IS_ENABLED(CONFIG_LLC)
-	case ARPHRD_ETHER:
-		kfree_skb(skb);
-		return;
-#endif
 	default:
 		kfree_skb(skb);
 		return;
diff --git a/net/x25/x25_route.c b/net/x25/x25_route.c
index ec2a39e9b3e6..9fbe4bb38d94 100644
--- a/net/x25/x25_route.c
+++ b/net/x25/x25_route.c
@@ -124,12 +124,7 @@  struct net_device *x25_dev_get(char *devname)
 {
 	struct net_device *dev = dev_get_by_name(&init_net, devname);
 
-	if (dev &&
-	    (!(dev->flags & IFF_UP) || (dev->type != ARPHRD_X25
-#if IS_ENABLED(CONFIG_LLC)
-					&& dev->type != ARPHRD_ETHER
-#endif
-					))){
+	if (dev && (!(dev->flags & IFF_UP) || dev->type != ARPHRD_X25)) {
 		dev_put(dev);
 		dev = NULL;
 	}