netdev - Re: [PATCH net v2] net: lantiq_xrx200: increase buffer reservation

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]

Date:   Mon, 13 Dec 2021 00:05:16 +0100
From:   Aleksander Bajkowski <olek2@...pl>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     hauke@...ke-m.de, davem@...emloft.net, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH net v2] net: lantiq_xrx200: increase buffer reservation

Hi,

Sorry for the late reply, but recently I haven't had access to
hardware to test different MTU values and packet lengths on the
hardware.

On 12/8/21 5:54 AM, Jakub Kicinski wrote:
> On Mon,  6 Dec 2021 23:39:09 +0100 Aleksander Jan Bajkowski wrote:
>> +static int xrx200_max_frame_len(int mtu)
>> +{
>> +	return VLAN_ETH_HLEN + mtu + ETH_FCS_LEN;
> 
> You sure the problem is not that this doesn't include ETH_HLEN? 
> MTU is the length of the L2 _payload_.
> 

VLAN_ETH_HLEN (14 + 4) contains ETH_HLEN (14). This function returns
the length of the frame that is written to the RX descriptor. Maybe
I don't understand the question and you are asking something else? 

>> +}
>> +
>> +static int xrx200_buffer_size(int mtu)
>> +{
>> +	return round_up(xrx200_max_frame_len(mtu) - 1, 4 * XRX200_DMA_BURST_LEN);
> 
> Why the - 1 ? 🤔
> 

This is how the hardware behaves. I don't really know where the -1
comes from. Unfortunately, I do not have access to TRM. 

> For a frame size 101 => max_frame_len 109 you'll presumably want 
> the buffer to be 116, not 108?
> 

For a frame size 101 => max_frame_len is 123 (18 + 101 + 4). Infact, PMAC strips FCS and ETH_FCS_LEN may not be needed. This behavior
is controlled by the PMAC_HD_CTL_RC bit. This bit is enabled from
the beginning of this driver. Ethtool has the option to enable
FCS reception, but the ethtool interface is not yet supported
by this driver. 

>> +}
>> +

Experiments show that the hardware starts to split the frame at
max_frame_len() - 1. Some examples:

pkt len		MTU	max_frame_size()	buffer_size()	desc1	desc2	desc3	desc4
----------------------------------------------------------------------------------------------
1506		1483		1505		1504		1502	4	X	X
1505		1483		1505		1504		1502	3	X	X
1504		1483		1505		1504		1504	X	X	X
1503		1483		1505		1504		1503	X	X	X
1502		1483		1505		1504		1502	X	X	X
1501		1483		1505		1504		1501	X	X	X
----------------------------------------------------------------------------------------------
1249		380		402		416		414	416	416	3
1248		380		402		416		414	416	416	2
1247		380		402		416		414	416	416	1
1246		380		402		416		414	416	416	X
1245		380		402		416		414	416	415	X
----------------------------------------------------------------------------------------------

In fact, this patch is a preparation for SG DMA support, which
I wrote some time ago.