Re: Fw: [Bug 201423] New: eth0: hw csum failure

2018-10-15 Thread Dave Stevenson
Hi Eric.

On Mon, 15 Oct 2018 at 16:42, Eric Dumazet  wrote:
>
> On Mon, Oct 15, 2018 at 8:15 AM Stephen Hemminger
>  wrote:
> >
> >
> >
> > Begin forwarded message:
> >
> > Date: Sun, 14 Oct 2018 10:42:48 +
> > From: bugzilla-dae...@bugzilla.kernel.org
> > To: step...@networkplumber.org
> > Subject: [Bug 201423] New: eth0: hw csum failure
> >
> >
> > https://bugzilla.kernel.org/show_bug.cgi?id=201423
> >
> > Bug ID: 201423
> >Summary: eth0: hw csum failure
> >Product: Networking
> >Version: 2.5
> > Kernel Version: 4.19.0-rc7
> >   Hardware: Intel
> > OS: Linux
> >   Tree: Mainline
> > Status: NEW
> >   Severity: normal
> >   Priority: P1
> >  Component: Other
> >   Assignee: step...@networkplumber.org
> >   Reporter: ross...@inwind.it
> > Regression: No
> >
> > I have a P6T DELUXE V2 motherboard and using the sky2 driver for the 
> > ethernet
> > ports. I get the following error message:
> >
> > [  433.727397] eth0: hw csum failure
> > [  433.727406] CPU: 4 PID: 0 Comm: swapper/4 Not tainted 4.19.0-rc7 #19
> > [  433.727406] Hardware name: System manufacturer System Product Name/P6T
> > DELUXE V2, BIOS 120212/22/2010
> > [  433.727407] Call Trace:
> > [  433.727409]  
> > [  433.727415]  dump_stack+0x46/0x5b
> > [  433.727419]  __skb_checksum_complete+0xb0/0xc0
> > [  433.727423]  tcp_v4_rcv+0x528/0xb60
> > [  433.727426]  ? ipt_do_table+0x2d0/0x400
> > [  433.727429]  ip_local_deliver_finish+0x5a/0x110
> > [  433.727430]  ip_local_deliver+0xe1/0xf0
> > [  433.727431]  ? ip_sublist_rcv_finish+0x60/0x60
> > [  433.727432]  ip_rcv+0xca/0xe0
> > [  433.727434]  ? ip_rcv_finish_core.isra.0+0x300/0x300
> > [  433.727436]  __netif_receive_skb_one_core+0x4b/0x70
> > [  433.727438]  netif_receive_skb_internal+0x4e/0x130
> > [  433.727439]  napi_gro_receive+0x6a/0x80
> > [  433.727442]  sky2_poll+0x707/0xd20
> > [  433.727446]  ? rcu_check_callbacks+0x1b4/0x900
> > [  433.727447]  net_rx_action+0x237/0x380
> > [  433.727449]  __do_softirq+0xdc/0x1e0
> > [  433.727452]  irq_exit+0xa9/0xb0
> > [  433.727453]  do_IRQ+0x45/0xc0
> > [  433.727455]  common_interrupt+0xf/0xf
> > [  433.727456]  
> > [  433.727459] RIP: 0010:cpuidle_enter_state+0x124/0x200
> > [  433.727461] Code: 53 60 89 c3 e8 dd 90 ad ff 65 8b 3d 96 58 a7 7e e8 d1 
> > 8f
> > ad ff 31 ff 49 89 c4 e8 27 99 ad ff fb 48 ba cf f7 53 e3 a5 9b c4 20 <4c> 
> > 89 e1
> > 4c 29 e9 48 89 c8 48 c1 f9 3f 48 f7 ea b8 ff ff ff 7f 48
> > [  433.727462] RSP: :c90a3e98 EFLAGS: 0282 ORIG_RAX:
> > ffde
> > [  433.727463] RAX: 880237b1f280 RBX: 0004 RCX:
> > 001f
> > [  433.727464] RDX: 20c49ba5e353f7cf RSI: 2fe419c1 RDI:
> > 
> > [  433.727465] RBP: 880237b263a0 R08: 0714 R09:
> > 00650512105d
> > [  433.727465] R10:  R11: 0342 R12:
> > 0064fc2a8b1c
> > [  433.727466] R13: 0064fc25b35f R14: 0004 R15:
> > 8204af20
> > [  433.727468]  ? cpuidle_enter_state+0x119/0x200
> > [  433.727471]  do_idle+0x1bf/0x200
> > [  433.727473]  cpu_startup_entry+0x6a/0x70
> > [  433.727475]  start_secondary+0x17f/0x1c0
> > [  433.727476]  secondary_startup_64+0xa4/0xb0
> > [  441.662954] eth0: hw csum failure
> > [  441.662959] CPU: 4 PID: 4347 Comm: radeon_cs:0 Not tainted 4.19.0-rc7 #19
> > [  441.662960] Hardware name: System manufacturer System Product Name/P6T
> > DELUXE V2, BIOS 120212/22/2010
> > [  441.662960] Call Trace:
> > [  441.662963]  
> > [  441.662968]  dump_stack+0x46/0x5b
> > [  441.662972]  __skb_checksum_complete+0xb0/0xc0
> > [  441.662975]  tcp_v4_rcv+0x528/0xb60
> > [  441.662979]  ? ipt_do_table+0x2d0/0x400
> > [  441.662981]  ip_local_deliver_finish+0x5a/0x110
> > [  441.662983]  ip_local_deliver+0xe1/0xf0
> > [  441.662985]  ? ip_sublist_rcv_finish+0x60/0x60
> > [  441.662986]  ip_rcv+0xca/0xe0
> > [  441.662988]  ? ip_rcv_finish_core.isra.0+0x300/0x300
> > [  441.662990]  __netif_receive_skb_one_core+0x4b/0x70
> > [  441.662993]  netif_receive_skb_internal+0x4e/0x130
> > [  441.662994]  napi_gro_receive+0x6a/0x80
> > [  441.662998]  sky2_poll+0x707/0xd20
> > [  441.663000]  net_rx_action+0x237/0x380
> > [  441.663002]  __do_softirq+0xdc/0x1e0
> > [  441.663005]  irq_exit+0xa9/0xb0
> > [  441.663007]  do_IRQ+0x45/0xc0
> > [  441.663009]  common_interrupt+0xf/0xf
> > [  441.663010]  
> > [  441.663012] RIP: 0010:merge+0x22/0xb0
> > [  441.663014] Code: c3 31 c0 c3 90 90 90 90 41 56 41 55 41 54 55 48 89 d5 
> > 53
> > 48 89 cb 48 83 ec 18 65 48 8b 04 25 28 00 00 00 48 89 44 24 10 31 c0 <48> 
> > 85 c9
> > 74 70 48 85 d2 74 6b 49 89 fd 49 89 f6 49 89 e4 eb 14 48
> > [  441.663015] RSP: 0018:c990b988 EFLAGS: 0246 ORIG_RAX:
> > ffde
> > [  441.663017] RAX:  RBX: 88021ab2d408 RCX:
> > 88021ab2d408
> > [  

[PATCH 3/4] net: lan78xx: Add support for VLAN tag stripping.

2018-06-25 Thread Dave Stevenson
The chip supports stripping the VLAN tag and reporting it
in metadata.
Complete the support for this.

Signed-off-by: Dave Stevenson 
---
 drivers/net/usb/lan78xx.c | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index afe7fa3..f72a8f5 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -64,6 +64,7 @@
 #define DEFAULT_RX_CSUM_ENABLE (true)
 #define DEFAULT_TSO_CSUM_ENABLE(true)
 #define DEFAULT_VLAN_FILTER_ENABLE (true)
+#define DEFAULT_VLAN_RX_OFFLOAD(true)
 #define TX_OVERHEAD(8)
 #define RXW_PADDING2
 
@@ -2363,6 +2364,11 @@ static int lan78xx_set_features(struct net_device 
*netdev,
pdata->rfe_ctl &= ~(RFE_CTL_ICMP_COE_ | RFE_CTL_IGMP_COE_);
}
 
+   if (features & NETIF_F_HW_VLAN_CTAG_RX)
+   pdata->rfe_ctl |= RFE_CTL_VLAN_STRIP_;
+   else
+   pdata->rfe_ctl &= ~RFE_CTL_VLAN_STRIP_;
+
if (features & NETIF_F_HW_VLAN_CTAG_FILTER)
pdata->rfe_ctl |= RFE_CTL_VLAN_FILTER_;
else
@@ -2976,6 +2982,9 @@ static int lan78xx_bind(struct lan78xx_net *dev, struct 
usb_interface *intf)
if (DEFAULT_TSO_CSUM_ENABLE)
dev->net->features |= NETIF_F_TSO | NETIF_F_TSO6 | NETIF_F_SG;
 
+   if (DEFAULT_VLAN_RX_OFFLOAD)
+   dev->net->features |= NETIF_F_HW_VLAN_CTAG_RX;
+
if (DEFAULT_VLAN_FILTER_ENABLE)
dev->net->features |= NETIF_F_HW_VLAN_CTAG_FILTER;
 
@@ -3052,6 +3061,16 @@ static void lan78xx_rx_csum_offload(struct lan78xx_net 
*dev,
}
 }
 
+static void lan78xx_rx_vlan_offload(struct lan78xx_net *dev,
+   struct sk_buff *skb,
+   u32 rx_cmd_a, u32 rx_cmd_b)
+{
+   if ((dev->net->features & NETIF_F_HW_VLAN_CTAG_RX) &&
+   (rx_cmd_a & RX_CMD_A_FVTG_))
+   __vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q),
+  (rx_cmd_b & 0x));
+}
+
 static void lan78xx_skb_return(struct lan78xx_net *dev, struct sk_buff *skb)
 {
int status;
@@ -3116,6 +3135,8 @@ static int lan78xx_rx(struct lan78xx_net *dev, struct 
sk_buff *skb)
if (skb->len == size) {
lan78xx_rx_csum_offload(dev, skb,
rx_cmd_a, rx_cmd_b);
+   lan78xx_rx_vlan_offload(dev, skb,
+   rx_cmd_a, rx_cmd_b);
 
skb_trim(skb, skb->len - 4); /* remove fcs */
skb->truesize = size + sizeof(struct sk_buff);
@@ -3134,6 +3155,7 @@ static int lan78xx_rx(struct lan78xx_net *dev, struct 
sk_buff *skb)
skb_set_tail_pointer(skb2, size);
 
lan78xx_rx_csum_offload(dev, skb2, rx_cmd_a, rx_cmd_b);
+   lan78xx_rx_vlan_offload(dev, skb2, rx_cmd_a, rx_cmd_b);
 
skb_trim(skb2, skb2->len - 4); /* remove fcs */
skb2->truesize = size + sizeof(struct sk_buff);
-- 
2.7.4



[PATCH 0/4] lan78xx minor fixes

2018-06-25 Thread Dave Stevenson
This is a small set of patches for the Microchip LAN78xx chip,
as used in the Raspberry Pi 3B+.
The main debug/discussion was on
https://github.com/raspberrypi/linux/issues/2458

Initial symptoms were that VLANs were very unreliable.
A couple of things were found:
- firstly that the hardware timeout value set failed to 
  take into account the VLAN tag, so a full MTU packet
  would be timed out.
- second was that regular checksum failures were being
  reported. Disabling checksum offload confirmed that
  the checksums were valid, and further experimentation
  identified that it was only if the VLAN tags were being
  passed through to the kernel that there were issues.
  The hardware supports VLAN filtering and tag stripping,
  therefore those have been implemented (much of the work
  was already done), and the driver drops back to s/w
  checksums should the choice be made not to use the h/w
  VLAN stripping.

Dave Stevenson (4):
  net: lan78xx: Allow for VLAN headers in timeout calcs
  net: lan78xx: Add support for VLAN filtering.
  net: lan78xx: Add support for VLAN tag stripping.
  net: lan78xx: Use s/w csum check on VLANs without tag stripping

 drivers/net/usb/lan78xx.c | 37 ++---
 1 file changed, 34 insertions(+), 3 deletions(-)

-- 
2.7.4



[PATCH 1/4] net: lan78xx: Allow for VLAN headers in timeout calcs

2018-06-25 Thread Dave Stevenson
The frame abort timeout being set by lan78xx_set_rx_max_frame_length
didn't account for any VLAN headers, resulting in very low
throughput if used with tagged VLANs.

Use VLAN_ETH_HLEN instead of ETH_HLEN to correct for this.

Signed-off-by: Dave Stevenson 
---
 drivers/net/usb/lan78xx.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index a89570f..2f793d4 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -2298,7 +2298,7 @@ static int lan78xx_change_mtu(struct net_device *netdev, 
int new_mtu)
if ((ll_mtu % dev->maxpacket) == 0)
return -EDOM;
 
-   ret = lan78xx_set_rx_max_frame_length(dev, new_mtu + ETH_HLEN);
+   ret = lan78xx_set_rx_max_frame_length(dev, new_mtu + VLAN_ETH_HLEN);
 
netdev->mtu = new_mtu;
 
@@ -2587,7 +2587,8 @@ static int lan78xx_reset(struct lan78xx_net *dev)
buf |= FCT_TX_CTL_EN_;
ret = lan78xx_write_reg(dev, FCT_TX_CTL, buf);
 
-   ret = lan78xx_set_rx_max_frame_length(dev, dev->net->mtu + ETH_HLEN);
+   ret = lan78xx_set_rx_max_frame_length(dev,
+ dev->net->mtu + VLAN_ETH_HLEN);
 
ret = lan78xx_read_reg(dev, MAC_RX, );
buf |= MAC_RX_RXEN_;
-- 
2.7.4



[PATCH 4/4] net: lan78xx: Use s/w csum check on VLANs without tag stripping

2018-06-25 Thread Dave Stevenson
Observations of VLANs dropping packets due to invalid
checksums when not offloading VLAN tag receive.
With VLAN tag stripping enabled no issue is observed.

Drop back to s/w checksums if VLAN offload is disabled.

Signed-off-by: Dave Stevenson 
---
 drivers/net/usb/lan78xx.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index f72a8f5..6f2ea84 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -3052,8 +3052,13 @@ static void lan78xx_rx_csum_offload(struct lan78xx_net 
*dev,
struct sk_buff *skb,
u32 rx_cmd_a, u32 rx_cmd_b)
 {
+   /* HW Checksum offload appears to be flawed if used when not stripping
+* VLAN headers. Drop back to S/W checksums under these conditions.
+*/
if (!(dev->net->features & NETIF_F_RXCSUM) ||
-   unlikely(rx_cmd_a & RX_CMD_A_ICSM_)) {
+   unlikely(rx_cmd_a & RX_CMD_A_ICSM_) ||
+   ((rx_cmd_a & RX_CMD_A_FVTG_) &&
+!(dev->net->features & NETIF_F_HW_VLAN_CTAG_RX))) {
skb->ip_summed = CHECKSUM_NONE;
} else {
skb->csum = ntohs((u16)(rx_cmd_b >> RX_CMD_B_CSUM_SHIFT_));
-- 
2.7.4



[PATCH 2/4] net: lan78xx: Add support for VLAN filtering.

2018-06-25 Thread Dave Stevenson
HW_VLAN_CTAG_FILTER was partially implemented, but not advertised
to Linux.

Complete the implementation of this.

Signed-off-by: Dave Stevenson 
---
 drivers/net/usb/lan78xx.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index 2f793d4..afe7fa3 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -2363,7 +2363,7 @@ static int lan78xx_set_features(struct net_device *netdev,
pdata->rfe_ctl &= ~(RFE_CTL_ICMP_COE_ | RFE_CTL_IGMP_COE_);
}
 
-   if (features & NETIF_F_HW_VLAN_CTAG_RX)
+   if (features & NETIF_F_HW_VLAN_CTAG_FILTER)
pdata->rfe_ctl |= RFE_CTL_VLAN_FILTER_;
else
pdata->rfe_ctl &= ~RFE_CTL_VLAN_FILTER_;
@@ -2976,6 +2976,9 @@ static int lan78xx_bind(struct lan78xx_net *dev, struct 
usb_interface *intf)
if (DEFAULT_TSO_CSUM_ENABLE)
dev->net->features |= NETIF_F_TSO | NETIF_F_TSO6 | NETIF_F_SG;
 
+   if (DEFAULT_VLAN_FILTER_ENABLE)
+   dev->net->features |= NETIF_F_HW_VLAN_CTAG_FILTER;
+
dev->net->hw_features = dev->net->features;
 
ret = lan78xx_setup_irq_domain(dev);
-- 
2.7.4