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]

Date:   Mon, 10 Dec 2018 11:44:03 -0800 (PST)
From:   David Miller <davem@...emloft.net>
To:     horms+renesas@...ge.net.au
Cc:     sergei.shtylyov@...entembedded.com, 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

From: Simon Horman <horms+renesas@...ge.net.au>
Date: Mon, 10 Dec 2018 09:59:17 +0100

> +#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.

Even in the most fundamental way, the macro is required to satisfy
the "no magic constants" rule for kernel code.