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] [thread-next>] [day] [month] [year] [list]

Message-ID: <m3wn614g0k.fsf@t19.piap.pl>
Date:   Thu, 05 Jan 2023 08:06:19 +0100
From:   Krzysztof Hałasa <khalasa@...p.pl>
To:     Harald Welte <laforge@...ocom.org>
Cc:     Andrew Lunn <andrew@...n.ch>, netdev@...r.kernel.org
Subject: Re: [PATCH] net: hdlc: Increase maximum HDLC MTU

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@...ocom.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