netdev - Re: [PATCH/RFT net] ravb: expand rx descriptor data to accommodate hw checksum

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: <c0e7450e-3d15-e7bd-d0a6-125b725f6870@cogentembedded.com>
Date:   Mon, 10 Dec 2018 23:11:57 +0300
From:   Sergei Shtylyov <sergei.shtylyov@...entembedded.com>
To:     David Miller <davem@...emloft.net>, horms+renesas@...ge.net.au
Cc:     magnus.damm@...il.com, netdev@...r.kernel.org,
        linux-renesas-soc@...r.kernel.org
Subject: Re: [PATCH/RFT net] ravb: expand rx descriptor data to accommodate hw
 checksum

On 12/10/2018 10:51 PM, Sergei Shtylyov wrote:

>>> +#define RAVB_CSUM_LEN 2
>>> +
>>  ...
>>>  	priv->rx_buf_sz = (ndev->mtu <= 1492 ? PKT_BUF_SZ : ndev->mtu) +
>>> -		ETH_HLEN + VLAN_HLEN;
>>> +		ETH_HLEN + VLAN_HLEN + RAVB_CSUM_LEN;
>>  ...
>>> +	if (unlikely(skb->len < RAVB_CSUM_LEN))
>>  ...
>>> -	hw_csum = skb_tail_pointer(skb) - 2;
>>> +	hw_csum = skb_tail_pointer(skb) - RAVB_CSUM_LEN;
>>  ...
>>> -	skb_trim(skb, skb->len - 2);
>>> +	skb_trim(skb, skb->len - RAVB_CSUM_LEN);
>>
>> Unlike Sergei, I think this macro define should be kept in the fix.
>>
>> It is absolutely crucial for anyone reading this code to understand
>> what this value is all about.
>>
>> People reading the code aren't able to go automatically back to a
>> commit to learn what this value means, and even if they could they
>> shouldn't have to do so for a bunch of magic '2' constants placed all
>> over.
> 
>    We already have a comment in ravb_tx_csum(), I only asked for another one

   Sorry, ravb_rx_csum(), of course. Those keys are dangerously close to each other. :-)

> (the place where we fix up the packet size). 

[...]

MBR. Sergei