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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <Y7bOQDKrUrEC9S/t@nataraja>
Date:   Thu, 5 Jan 2023 14:18:56 +0100
From:   Harald Welte <laforge@...monks.org>
To:     Krzysztof Hałasa <khalasa@...p.pl>
Cc:     Andrew Lunn <andrew@...n.ch>, netdev@...r.kernel.org
Subject: Re: [PATCH] net: hdlc: Increase maximum HDLC MTU

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