Re: [EXT] Re: [net-next] net: dsa: felix: disable always guard band bit for TAS config

From: Michael Walle <michael@walle.cc>
To: Xiaoliang Yang <xiaoliang.yang_1@nxp.com>
Cc: Vladimir Oltean <vladimir.oltean@nxp.com>,
	Vladimir Oltean <olteanv@gmail.com>,
	UNGLinuxDriver@microchip.com, alexandre.belloni@bootlin.com,
	allan.nielsen@microchip.com,
	Claudiu Manoil <claudiu.manoil@nxp.com>,
	davem@davemloft.net, idosch@mellanox.com,
	joergen.andreasen@microchip.com, linux-kernel@vger.kernel.org,
	netdev@vger.kernel.org, Po Liu <po.liu@nxp.com>,
	vinicius.gomes@intel.com
Subject: Re: [EXT] Re: [net-next] net: dsa: felix: disable always guard band bit for TAS config
Date: Wed, 09 Jun 2021 10:41:55 +0200	[thread overview]
Message-ID: <32f1854fdc0fda86627371bb82f2c873@walle.cc> (raw)
In-Reply-To: <DB8PR04MB5785C5BDBDD51401362563D6F0369@DB8PR04MB5785.eurprd04.prod.outlook.com>

Am 2021-06-09 10:06, schrieb Xiaoliang Yang:
> On 2021-06-07 19:26, Michael Walle wrote:
>> 
>> Hi Vladimir, Hi Xiaoliang,
>> 
>> Am 2021-05-07 14:19, schrieb Vladimir Oltean:
>> > Devices like Felix need the per-queue max SDU from the user - if that
>> > isn's specified in the netlink message they'll have to default to the
>> > interface's MTU.
>> 
>> Btw. just to let you and Xiaoliang know:
>> 
>> It appears that PORT_MAX_SDU isn't working as expected. It is used as 
>> a
>> fallback if QMAXSDU_CFG_n isn't set for the guard band calculation. 
>> But it
>> appears to be _not_ used for discarding any frames. E.g. if you set
>> PORT_MAX_SDU to 500 the port will still happily send frames larger 
>> than 500
>> bytes. (Unless of course you hit the guard band of 500 bytes). OTOH
>> QMAXSDU_CFG_n works as expected, it will discard oversized frames - 
>> and
>> presumly will set the guard band accordingly, I haven't tested this 
>> explicitly.
>> 
>> Thus, I wonder what sense PORT_MAX_SDU makes at all. If you set the 
>> guard
>> band to a smaller value than the MTU, you'll also need to make sure, 
>> there will
>> be no larger frames scheduled on that port.
>> 
>> In any case, the workaround is to set QMAXSDU_CFG_n (for all
>> n=0..7) to the desired max_sdu value instead of using PORT_MAX_SDU.
>> 
>> It might also make sense to check with the IP supplier.
>> 
>> In the case anyone wants to implement that for (upstream) linux ;)
>> 
>> -michael
> 
> Yes, PORT_MAX_SDU is only used for guard band calculation. DEV_GMII:
> MAC_MAXLEN_CFG
> limited the frame length accepted by the MAC.

But MAC_MAXLEN_CFG is for ingress handling while you want egress 
handling,
for example think two ingress ports sending to one egress port. The
limitation is on the egress side. Or two queues with different guard
bands/maxsdu settings.

> I am worried that
> QMAXSDU is not a universal
> setting, it may just be set on Felix, so there is no suitable place to
> add this configuration.

I can't follow you here. I'm talkling about felix and its quirks. Eg. 
the
static guard band handling there, the reason why we need the maxsdu
setting in the first place.

Or do you think about how to communicate that setting from user space to
the kernel? In this case, I'd say we'll need some kind of parameter for
this kind of devices which doesn't have a dynamic guard band mechanism.
Felix won't be the only one.

-michael