From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4F18DC53210 for ; Thu, 5 Jan 2023 13:30:58 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230010AbjAENa0 (ORCPT ); Thu, 5 Jan 2023 08:30:26 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:49390 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233545AbjAENaJ (ORCPT ); Thu, 5 Jan 2023 08:30:09 -0500 Received: from ganesha.gnumonks.org (ganesha.gnumonks.org [IPv6:2001:780:45:1d:225:90ff:fe52:c662]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id BEAB624C for ; Thu, 5 Jan 2023 05:30:07 -0800 (PST) Received: from uucp by ganesha.gnumonks.org with local-bsmtp (Exim 4.94.2) (envelope-from ) id 1pDQJo-00CauE-Pp; Thu, 05 Jan 2023 14:30:04 +0100 Received: from laforge by localhost.localdomain with local (Exim 4.96) (envelope-from ) id 1pDQ92-00FPC6-1N; Thu, 05 Jan 2023 14:18:56 +0100 Date: Thu, 5 Jan 2023 14:18:56 +0100 From: Harald Welte To: Krzysztof =?utf-8?Q?Ha=C5=82asa?= Cc: Andrew Lunn , netdev@vger.kernel.org Subject: Re: [PATCH] net: hdlc: Increase maximum HDLC MTU Message-ID: References: <20230104125724.3587015-1-laforge@osmocom.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org 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 https://laforge.gnumonks.org/ ============================================================================ "Privacy in residential applications is a desirable marketing option." (ETSI EN 300 175-7 Ch. A6)