[PATCH] net: hdlc: Increase maximum HDLC MTU

[PATCH] net: hdlc: Increase maximum HDLC MTU

[Harald Welte]

The FRF 1.2 specification clearly states:

> A maximum frame relay information field size of 1600 octets shall be
> supported by the network and the user.

The linux kernel hdlc/fr code has so far had a maximum MTU of 1500
octets.  This may have been sufficient to transport "regular" Ethernet
frames of MTU 1500 via frame relay net-devices, but there are other
use cases than Ethernet.

One such use case is the 3GPP Gb interface (TS 48.014, 48.016, 48.018)
operated over Frame Relay.  There is open source software [2]
implementing those interfaces by means of AF_PACKET sockets over
Linux kernel hdlcX devices.

And before anyone asks: Even in 2023 there are real-world deployments of
those interfaces over Frame Relay in production use.

This patch doesn't change the default hdlcX netdev MTU, but permits
userspace to configure a higher MTU, in those cases needed.

[1] https://www.broadband-forum.org/technical/download/FRF.1.2.pdf
[2] https://osmocom.org/projects/osmo-gbproxy/wiki
[3] https://gitea.osmocom.org/osmocom/libosmocore/src/branch/master/src/gb/gprs_ns2_fr.c

Signed-off-by: Harald Welte <laforge@osmocom.org>
---
 include/uapi/linux/hdlc.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/hdlc.h b/include/uapi/linux/hdlc.h
index d89cb3ee7c70..f6b01e883cb5 100644
--- a/include/uapi/linux/hdlc.h
+++ b/include/uapi/linux/hdlc.h
@@ -13,7 +13,9 @@
 #define _UAPI__HDLC_H
 
 
-#define HDLC_MAX_MTU 1500	/* Ethernet 1500 bytes */
+/* FRF 1.2 states the information field should be 1600 bytes. So in case of
+ * a 4-byte header of Q.922, this results in a MTU of 1604 bytes */
+#define HDLC_MAX_MTU 1604	/* as required for FR network (e.g. carrying GPRS-NS) */
 #if 0
 #define HDLC_MAX_MRU (HDLC_MAX_MTU + 10 + 14 + 4) /* for ETH+VLAN over FR */
 #else
-- 
2.39.0

Re: [PATCH] net: hdlc: Increase maximum HDLC MTU

[Andrew Lunn]

On Wed, Jan 04, 2023 at 01:57:24PM +0100, Harald Welte wrote:
> The FRF 1.2 specification clearly states:
> 
> > A maximum frame relay information field size of 1600 octets shall be
> > supported by the network and the user.
> 
> The linux kernel hdlc/fr code has so far had a maximum MTU of 1500
> octets.  This may have been sufficient to transport "regular" Ethernet
> frames of MTU 1500 via frame relay net-devices, but there are other
> use cases than Ethernet.
> 
> One such use case is the 3GPP Gb interface (TS 48.014, 48.016, 48.018)
> operated over Frame Relay.  There is open source software [2]
> implementing those interfaces by means of AF_PACKET sockets over
> Linux kernel hdlcX devices.
> 
> And before anyone asks: Even in 2023 there are real-world deployments of
> those interfaces over Frame Relay in production use.
> 
> This patch doesn't change the default hdlcX netdev MTU, but permits
> userspace to configure a higher MTU, in those cases needed.

I could be reading the code wrong, but:

https://elixir.bootlin.com/linux/latest/source/drivers/net/wan/hdlc.c#L231

static void hdlc_setup_dev(struct net_device *dev)
{
	/* Re-init all variables changed by HDLC protocol drivers,
	 * including ether_setup() called from hdlc_raw_eth.c.
	 */
	dev->flags		 = IFF_POINTOPOINT | IFF_NOARP;
	dev->priv_flags		 = IFF_WAN_HDLC;
	dev->mtu		 = HDLC_MAX_MTU;
	dev->min_mtu		 = 68;
	dev->max_mtu		 = HDLC_MAX_MTU;
	dev->type		 = ARPHRD_RAWHDLC;
	dev->hard_header_len	 = 0;
	dev->needed_headroom	 = 0;
	dev->addr_len		 = 0;
	dev->header_ops		 = &hdlc_null_ops;
}

This change does appear to change dev->mtu, not just dev->max_mtu?
So i'm not sure the commit message is correct?

   Andrew

Re: [PATCH] net: hdlc: Increase maximum HDLC MTU

[Harald Welte]

Dear Andrew,

On Wed, Jan 04, 2023 at 06:52:32PM +0100, Andrew Lunn wrote:
 
> This change does appear to change dev->mtu, not just dev->max_mtu?
> So i'm not sure the commit message is correct?

Thanks for your feedback, which seems correct. 

We've been using this patch in production for more than a year, but our
use cases are all using larger MTU so this might have been unnoticed.

I'll investigate and likely we have to introduce a new HDLC_DEFAULT_MTU
of 1500 to achieve backwards compatibility.

-- 
- Harald Welte <laforge@gnumonks.org>          https://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
                                                  (ETSI EN 300 175-7 Ch. A6)

Re: [PATCH] net: hdlc: Increase maximum HDLC MTU

[Krzysztof Hałasa]

Harald,

Andrew is right. The default setting makes sure the packets fit into
regular Ethernet on the other side of the link (which is, or was, the
most common situation). I guess the mtu could be set trivially to 1500
with the max being 1600 or 16k or whatever.

Now there is a second thing, the HDLC_MAX_MRU (which is set to 1600).
This is the (fixed) size of RX (and TX) memory buffers on certain old
cards (some of whom are ISA and maybe even use 8-bit XT-BUS transfer
mode). I guess it doesn't concern you directly, but the MTU on those
cards must be kept at most at HDLC_MAX_MRU - max size of the headers
(= 10 + 14 + 4 or so, maybe more) or the packets generated by the IP
stack won't go out correctly.

Harald Welte <laforge@osmocom.org> writes:

> +/* FRF 1.2 states the information field should be 1600 bytes. So in case of
> + * a 4-byte header of Q.922, this results in a MTU of 1604 bytes */
> +#define HDLC_MAX_MTU 1604	/* as required for FR network (e.g.
> carrying GPRS-NS) */

I think the "FR information field" is the data portion, without 2-byte
Q.922 address, and without the 2-byte frame check sequence, but
including e.g. UI and NLPID. This means, in the simplest case of IPv4/v6,
max MTU of 1598 bytes (by default), and less than that with 802.1q
(8-byte "snap" DLCI header format + 14-byte bridged Ethernet header +
4 byte .1q header). This was never very straightforward.

I think maybe we change HDLC_MAX_MRU from 1600 to 1602 (2 bytes for the
Q.922 address and 1600 for the "FR information field"), this shouldn't
break anything and would IMHO make the code compliant with the FRF 1.2.

Then we drop the HDLC_MAX_MTU completely and use ETH_MAX_MTU (which is
0xFFFF) for dev->max_mtu instead. Devices using fixed buffer sizes
should override this to, I guess, the limit - 10 - 14 - 4.
For dev->mtu we could, by default, use ETH_DATA_LEN which is 1500 bytes.
Also the assignments in fr_add_pvc() should be changed to account for
the hdlcX master device parameters.

What do you think?
-- 
Krzysztof "Chris" Hałasa

Sieć Badawcza Łukasiewicz
Przemysłowy Instytut Automatyki i Pomiarów PIAP
Al. Jerozolimskie 202, 02-486 Warszawa

Re: [PATCH] net: hdlc: Increase maximum HDLC MTU

[Harald Welte]

Hi Krzysztof,

thanks for your detailed analyiss.

On Thu, Jan 05, 2023 at 08:06:19AM +0100, Krzysztof Hałasa wrote:

> Andrew is right. The default setting makes sure the packets fit into
> regular Ethernet on the other side of the link (which is, or was, the
> most common situation). I guess the mtu could be set trivially to 1500
> with the max being 1600 or 16k or whatever.

great.

> Now there is a second thing, the HDLC_MAX_MRU (which is set to 1600).
> This is the (fixed) size of RX (and TX) memory buffers on certain old
> cards (some of whom are ISA and maybe even use 8-bit XT-BUS transfer
> mode). I guess it doesn't concern you directly, but the MTU on those
> cards must be kept at most at HDLC_MAX_MRU - max size of the headers
> (= 10 + 14 + 4 or so, maybe more) or the packets generated by the IP
> stack won't go out correctly.

Understood. Indeed it is not my immediate concern, as I don't have any of
those old ISA or even 8-bit XT-bus cards.  If anyone had some spares of
such retronetworking hardware, I'd be very interested.  In the 
actual "present day" production deployments we're using either the
Osmocom icE1usb (2-port E1 USB) or Digium TE820 (8-port E1 PCIe) with
DAHDI as and CONFIG_DAHDI_NET, which then uses the kernel HDLC.

> > +/* FRF 1.2 states the information field should be 1600 bytes. So in case of
> > + * a 4-byte header of Q.922, this results in a MTU of 1604 bytes */
> > +#define HDLC_MAX_MTU 1604	/* as required for FR network (e.g.
> > carrying GPRS-NS) */
> 
> I think the "FR information field" is the data portion, without 2-byte
> Q.922 address, and without the 2-byte frame check sequence, but
> including e.g. UI and NLPID. 

In my understanding of Q.922/Q.921, the information field does not include
the control field or the address field.  So in your example, the UI would
be the control field.  So we have a FR frame consisting of:

* flag
* address field: typically 2 octets, in theory also 3 or 4-octet formats
* control field: 2 octets for I/S format, 1 octet for U format
* FCS: 2 octets
* flag

> This means, in the simplest case of IPv4/v6,
> max MTU of 1598 bytes (by default), and less than that with 802.1q
> (8-byte "snap" DLCI header format + 14-byte bridged Ethernet header +
> 4 byte .1q header). This was never very straightforward.

As indicated, I unfortunately lack any in-depth experience with
deployments of Ethernet or IP over Frame Relay or HDLC.

> I think maybe we change HDLC_MAX_MRU from 1600 to 1602 (2 bytes for the
> Q.922 address and 1600 for the "FR information field"), this shouldn't
> break anything and would IMHO make the code compliant with the FRF 1.2.

I would argue it should be at least 1604 (2 bytes address and 2 bytes
control + 1600 byte information field.

> Then we drop the HDLC_MAX_MTU completely and use ETH_MAX_MTU (which is
> 0xFFFF) for dev->max_mtu instead. Devices using fixed buffer sizes
> should override this to, I guess, the limit - 10 - 14 - 4.
> For dev->mtu we could, by default, use ETH_DATA_LEN which is 1500 bytes.
> Also the assignments in fr_add_pvc() should be changed to account for
> the hdlcX master device parameters.
> 
> What do you think?

Sounds all good to me, though as stated I cannot really say much on the
IP/Eth encapsuiation formats due to lack of experience with those.

btw: In case you're wondering why in osmocom we are using "raw" hdlc
netdev and implementing FR in userspace: That's because we typically
need to implement the network side of FR towards an external user,  and
we in general need more control over the Q.933 layer.  It's easier to do
this in userspace, given that all of the PVC are handled by a single
application anyway.

Regards,
	Harald

-- 
- Harald Welte <laforge@gnumonks.org>          https://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
                                                  (ETSI EN 300 175-7 Ch. A6)