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
