[patch net-next] net: hns: add skb_reset_mac_header() after skb being alloc

From: Kejian Yan <redacted>

HNS receives a packet without doing anything, but it should call
skb_reset_mac_header() to initialize the header before using
eth_hdr().

Fixes: 0d6b425a3773c3445b0f51b2f333821beaacb619
Signed-off-by: Kejian Yan <redacted>
Signed-off-by: Yisen Zhuang <redacted>
---
 drivers/net/ethernet/hisilicon/hns/hns_enet.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/hisilicon/hns/hns_enet.c b/drivers/net/ethernet/hisilicon/hns/hns_enet.c
index ad742a6..15200e4 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_enet.c

@@ -600,6 +600,7 @@ static int hns_nic_poll_rx_skb(struct hns_nic_ring_data *ring_data,
 		ring->stats.sw_err_cnt++;
 		return -ENOMEM;
 	}
+	skb_reset_mac_header(skb);
 
 	prefetchw(skb->data);
 	length = le16_to_cpu(desc->rx.pkt_len);

-- 
1.9.1

From: Yisen Zhuang <redacted>
Date: Mon, 13 Jun 2016 20:41:22 +0800

From: Kejian Yan <redacted>

HNS receives a packet without doing anything, but it should call
skb_reset_mac_header() to initialize the header before using
eth_hdr().

Fixes: 0d6b425a3773c3445b0f51b2f333821beaacb619
Signed-off-by: Kejian Yan <redacted>
Signed-off-by: Yisen Zhuang <redacted>

Well, this patch made me look at this function.

You really shouldn't be filtering packets looped back, that is
the stack's job.  It shouldn't be happening in the driver.

And once you remove that code, this patch here is no longer
necessary.

Second of all, unless you card supports every protocol that
exists in the past, present, and _future_ you cannot set
skb->ip_summed to CHECKSUM_UNNECSSARY unconditionally like
that.

You can only set that for protocols your chip actually supports.

Hi David,

Thanks for your suggestions.

Please see my comments below.

Thanks,

Yisen

在 2016/6/15 13:41, David Miller 写道:

From: Yisen Zhuang <redacted>
Date: Mon, 13 Jun 2016 20:41:22 +0800

quoted

From: Kejian Yan <redacted>

HNS receives a packet without doing anything, but it should call
skb_reset_mac_header() to initialize the header before using
eth_hdr().

Fixes: 0d6b425a3773c3445b0f51b2f333821beaacb619
Signed-off-by: Kejian Yan <redacted>
Signed-off-by: Yisen Zhuang <redacted>

Well, this patch made me look at this function.

You really shouldn't be filtering packets looped back, that is
the stack's job.  It shouldn't be happening in the driver.

If we use ping6 to test if it is connected to network, CPUs would send out the NS packets
and these packets will be looped back to CPUs. If driver does not drop these packets,
they will be sent to protocol stack and protocol stack consider that there is a device
with the same address and it is not available address. It will show us the log like
"connect: Cannot assign requested address". Then it can not connect to the network enviroment.
Thus, we drop these packets looped back in HNS driver.

And once you remove that code, this patch here is no longer
necessary.

Second of all, unless you card supports every protocol that
exists in the past, present, and _future_ you cannot set
skb->ip_summed to CHECKSUM_UNNECSSARY unconditionally like
that.

You can only set that for protocols your chip actually supports.

Thanks for your suggestions. I will prepare a new patch to fix it.

.

在 2016/6/15 18:30, Yisen Zhuang 写道:

Hi David,

Thanks for your suggestions.

Please see my comments below.

Thanks,

Yisen

在 2016/6/15 13:41, David Miller 写道:

quoted

From: Yisen Zhuang <redacted>
Date: Mon, 13 Jun 2016 20:41:22 +0800

quoted

From: Kejian Yan <redacted>

HNS receives a packet without doing anything, but it should call
skb_reset_mac_header() to initialize the header before using
eth_hdr().

Fixes: 0d6b425a3773c3445b0f51b2f333821beaacb619
Signed-off-by: Kejian Yan <redacted>
Signed-off-by: Yisen Zhuang <redacted>

Well, this patch made me look at this function.

You really shouldn't be filtering packets looped back, that is
the stack's job.  It shouldn't be happening in the driver.

If we use ping6 to test if it is connected to network, CPUs would send out the NS packets
and these packets will be looped back to CPUs. If driver does not drop these packets,
they will be sent to protocol stack and protocol stack consider that there is a device
with the same address and it is not available address. It will show us the log like
"connect: Cannot assign requested address". Then it can not connect to the network enviroment.
Thus, we drop these packets looped back in HNS driver.

Hi David,

What is the opinion about this issue? We need this patch to resolve the issue.

Hoping for your suggestions.

Many thanks,

Yisen

quoted

And once you remove that code, this patch here is no longer
necessary.

Second of all, unless you card supports every protocol that
exists in the past, present, and _future_ you cannot set
skb->ip_summed to CHECKSUM_UNNECSSARY unconditionally like
that.

You can only set that for protocols your chip actually supports.

Thanks for your suggestions. I will prepare a new patch to fix it.

quoted

.

_______________________________________________
linuxarm mailing list
linuxarm@huawei.com
http://rnd-openeuler.huawei.com/mailman/listinfo/linuxarm

From: Yisen Zhuang <redacted>
Date: Mon, 20 Jun 2016 15:12:34 +0800

在 2016/6/15 18:30, Yisen Zhuang 写道:

quoted

Hi David,

Thanks for your suggestions.

Please see my comments below.

Thanks,

Yisen

在 2016/6/15 13:41, David Miller 写道:

quoted

From: Yisen Zhuang <redacted>
Date: Mon, 13 Jun 2016 20:41:22 +0800

quoted

From: Kejian Yan <redacted>

HNS receives a packet without doing anything, but it should call
skb_reset_mac_header() to initialize the header before using
eth_hdr().

Fixes: 0d6b425a3773c3445b0f51b2f333821beaacb619
Signed-off-by: Kejian Yan <redacted>
Signed-off-by: Yisen Zhuang <redacted>

Well, this patch made me look at this function.

You really shouldn't be filtering packets looped back, that is
the stack's job.  It shouldn't be happening in the driver.

If we use ping6 to test if it is connected to network, CPUs would send out the NS packets
and these packets will be looped back to CPUs. If driver does not drop these packets,
they will be sent to protocol stack and protocol stack consider that there is a device
with the same address and it is not available address. It will show us the log like
"connect: Cannot assign requested address". Then it can not connect to the network enviroment.
Thus, we drop these packets looped back in HNS driver.

Hi David,

What is the opinion about this issue? We need this patch to resolve the issue.

I guess this is fine if your chip behaves this way.