Hi, David
On 2017/7/4 18:28, David Miller wrote:
> From: Lin Yun Sheng <linyunsh...@huawei.com>
> Date: Tue, 4 Jul 2017 18:47:31 +0800
>
>> From: Yunsheng Lin <linyunsh...@huawei.com>
>>
>> If driver support checksum offload, should check netdev feature
>> before fill TX description and get CSUM err bit from RX
>> description. HNS driver do the check in RX derction but it doesn't
>> do the check in TX direction.
>>
>> Signed-off-by: lipeng <lipeng...@huawei.com>
>> Reviewed-by: Daode Huang <huangda...@hisilicon.com>
>> Reviewed-by: Yunsheng Lin <linyunsh...@huawei.com>
>
> This is not correct.
>
> You should be checking the skb->checksum field to decide if you should
> offload the TX checksum of the packet or not.
>
> Correct drivers, as far as I am aware, do not check the feature flags
> so I wonder where you got this idea from. Always use other well
> established existing drivers as a model for how to handle things like
> this.
>
> And this makes sense. An SKB can have it's checksumming determination
> made first, then the netdev feature change is made afterwards. For
> correctness you still need to TX checksum offload that SKB otherwise
> it will be emitted without a correctly computed checksum.
>
You are right, thanks for pointing out.
driver/net/ethernet/hisilicon/hns/hns_enet.c
fill_desc:
if (skb->ip_summed == CHECKSUM_PARTIAL) {
protocol = skb->protocol;
ip_offset = ETH_HLEN;
/*if it is a SW VLAN check the next protocol*/
if (protocol == htons(ETH_P_8021Q)) {
ip_offset += VLAN_HLEN;
protocol = vlan_get_protocol(skb);
skb->protocol = protocol;
}
if (skb->protocol == htons(ETH_P_IP)) {
flag_ipoffset |= 1 << HNS_TXD_L3CS_B;
/* check for tcp/udp header */
flag_ipoffset |= 1 << HNS_TXD_L4CS_B;
} else if (skb->protocol == htons(ETH_P_IPV6)) {
/* ipv6 has not l3 cs, check for L4 header */
flag_ipoffset |= 1 << HNS_TXD_L4CS_B;
}
flag_ipoffset |= ip_offset << HNS_TXD_IPOFFSET_S;
}
the hns driver has already check the skb->ip_summed, and checking the
ndev->feature is
not correct.
Will remove this patch next revision.
Best Regards
Yunsheng Lin
