s2io: only arm hardware LSO for GSO skbs
Subsystems (2)
PATCH 1/1 s2io: only arm hardware LSO for GSO skbs
s2io_xmit() enables the Xframe/Xframe-II hardware LSO (TCP segmentation) engine whenever the skb's gso_type carries SKB_GSO_TCPV4/TCPV6, and programs the segment size from gso_size:
offload_type = s2io_offload_type(skb);
if (offload_type & (SKB_GSO_TCPV4 | SKB_GSO_TCPV6)) {
txdp->Control_1 |= TXD_TCP_LSO_EN;
txdp->Control_1 |= TXD_TCP_LSO_MSS(s2io_tcp_mss(skb));
}Since commit 51466a7545b7 ("tcp: fill shinfo->gso_type at last moment") tcp_transmit_skb() sets skb_shinfo(skb)->gso_type unconditionally on every TCP skb, including non-GSO frames where gso_size is 0. The driver therefore arms the LSO engine with MSS == 0 for ordinary TCP segments such as the connection's SYN. The Xframe-II LSO engine treats an MSS of 0 as an illegal descriptor and aborts the transmit (lso_err_reg reports LSO6_ABORT), so the frame is dropped before it reaches the MAC. The result is that no TCP can be transmitted on these adapters since v4.2; UDP and ICMP (which never carry SKB_GSO_TCPV4) are unaffected.
Only arm the LSO engine when the skb is actually GSO (gso_size > 0), restoring the pre-4.2 behaviour. Non-GSO TCP frames take the normal transmit path.
Reproduced and fixed on Linux 6.6.67 with an Xframe-II adapter (PCI 17d5:5832); bisected to good v4.1.6 / bad v4.2.2.
---
[ Not upstream and cannot be: the s2io driver was removed from mainline in
commit aba0138eb7d7 ("net: ethernet: neterion: s2io: remove unused driver"). It still ships in the 6.6.y and 6.12.y stable trees, where this bug is present and the patch applies cleanly. Please apply there. ]
drivers/net/ethernet/neterion/s2io.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/ethernet/neterion/s2io.c +1 -1
@@ -4105,7 +4105,7 @@ static netdev_tx_t s2io_xmit(struct sk_buff *skb, struct net_device *dev) } offload_type =s2io_offload_type (skb); -if (offload_type & (SKB_GSO_TCPV4 |SKB_GSO_TCPV6 )) { +if ((offload_type & (SKB_GSO_TCPV4 |SKB_GSO_TCPV6 )) &&skb_is_gso (skb)) { txdp->Control_1 |=TXD_TCP_LSO_EN ; txdp->Control_1 |=TXD_TCP_LSO_MSS (s2io_tcp_mss (skb)); }
Signature · 1 line
2.54.0
Greg Kroah-Hartman
maintainerQuoted 27 lines · Michael Pratte
s2io_xmit()enables the Xframe/Xframe-II hardware LSO (TCP segmentation) engine whenever the skb'sgso_typecarries SKB_GSO_TCPV4/TCPV6, and programs the segment size fromgso_size:offload_type = s2io_offload_type(skb); if (offload_type & (SKB_GSO_TCPV4 | SKB_GSO_TCPV6)) { txdp->Control_1 |= TXD_TCP_LSO_EN; txdp->Control_1 |= TXD_TCP_LSO_MSS(s2io_tcp_mss(skb)); }Since commit
51466a7545b7("tcp: fillshinfo->gso_typeat last moment")tcp_transmit_skb()sets skb_shinfo(skb)->gso_type unconditionally on every TCP skb, including non-GSO frames wheregso_sizeis 0. The driver therefore arms the LSO engine with MSS == 0 for ordinary TCP segments such as the connection's SYN. The Xframe-II LSO engine treats an MSS of 0 as an illegal descriptor and aborts the transmit (lso_err_regreportsLSO6_ABORT), so the frame is dropped before it reaches the MAC. The result is that no TCP can be transmitted on these adapters since v4.2; UDP and ICMP (which never carrySKB_GSO_TCPV4) are unaffected.Only arm the LSO engine when the skb is actually GSO (
gso_size> 0), restoring the pre-4.2 behaviour. Non-GSO TCP frames take the normal transmit path.Reproduced and fixed on Linux 6.6.67 with an Xframe-II adapter (PCI 17d5:5832); bisected to good v4.1.6 / bad v4.2.2.
Quoted 7 lines
Fixes 51466a7545b7 ("tcp: fill shinfo->gso_type at last moment")Signed-off-by Michael Pratte <[email protected]>---
[ Not upstream and cannot be: the s2io driver was removed from mainline in
commitaba0138eb7d7("net: ethernet: neterion: s2io: remove unused driver"). It still ships in the 6.6.y and 6.12.y stable trees, where this bug is present and the patch applies cleanly. Please apply there. ]
Why not just remove the driver in older kernels as well if it is not being used?
And if it's not being used, why is this patch needed at all?
thanks,
greg k-h
Quoted 2 lines · Greg Kroah-Hartman
Why not just remove the driver in older kernels as well if it is not being used?
It is being used - Xframe-II (17d5:5832) in a Supermicro X5DA8 on 6.6. Please keep it in stable.
Quoted 1 line
And if it's not being used, why is this patch needed at all?
It's used and broken: since v4.2 (51466a7545b7) s2io arms LSO with MSS=0 on every non-GSO TCP frame, so the card aborts all TCP TX - links fine, UDP/ICMP ok, but no TCP at all. The one-liner restores it.
Thanks,
Michael
Quoted 45 lines · Greg Kroah-Hartman
Quoted 27 lines · Michael Pratte
s2io_xmit()enables the Xframe/Xframe-II hardware LSO (TCP segmentation) engine whenever the skb'sgso_typecarries SKB_GSO_TCPV4/TCPV6, and programs the segment size fromgso_size:offload_type = s2io_offload_type(skb); if (offload_type & (SKB_GSO_TCPV4 | SKB_GSO_TCPV6)) { txdp->Control_1 |= TXD_TCP_LSO_EN; txdp->Control_1 |= TXD_TCP_LSO_MSS(s2io_tcp_mss(skb)); }Since commit
51466a7545b7("tcp: fillshinfo->gso_typeat last moment")tcp_transmit_skb()sets skb_shinfo(skb)->gso_type unconditionally on every TCP skb, including non-GSO frames wheregso_sizeis 0. The driver therefore arms the LSO engine with MSS == 0 for ordinary TCP segments such as the connection's SYN. The Xframe-II LSO engine treats an MSS of 0 as an illegal descriptor and aborts the transmit (lso_err_regreportsLSO6_ABORT), so the frame is dropped before it reaches the MAC. The result is that no TCP can be transmitted on these adapters since v4.2; UDP and ICMP (which never carrySKB_GSO_TCPV4) are unaffected.Only arm the LSO engine when the skb is actually GSO (
gso_size> 0), restoring the pre-4.2 behaviour. Non-GSO TCP frames take the normal transmit path.Reproduced and fixed on Linux 6.6.67 with an Xframe-II adapter (PCI 17d5:5832); bisected to good v4.1.6 / bad v4.2.2.
Quoted 7 lines
Fixes 51466a7545b7 ("tcp: fill shinfo->gso_type at last moment")Signed-off-by Michael Pratte <[email protected]>---
[ Not upstream and cannot be: the s2io driver was removed from mainline in
commitaba0138eb7d7("net: ethernet: neterion: s2io: remove unused driver"). It still ships in the 6.6.y and 6.12.y stable trees, where this bug is present and the patch applies cleanly. Please apply there. ]Why not just remove the driver in older kernels as well if it is not being used?
And if it's not being used, why is this patch needed at all?
thanks,
greg k-h
Greg Kroah-Hartman
maintainerQuoted 4 lines · Michael Pratte
Quoted 2 lines · Greg Kroah-Hartman
Why not just remove the driver in older kernels as well if it is not being used?
Quoted 8 lines
It is being used - Xframe-II (17d5:5832) in a Supermicro X5DA8 on 6.6. Please keep it in stable.
Quoted 1 line
And if it's not being used, why is this patch needed at all?
It's used and broken: since v4.2 (
51466a7545b7) s2io arms LSO with MSS=0 on every non-GSO TCP frame, so the card aborts all TCP TX - links fine, UDP/ICMP ok, but no TCP at all. The one-liner restores it.
So what happens when 6.6.y goes end-of-life? Why is this driver removed if people are actually using it and willing to fix bugs in it?
I really don't want to take non-upstream patches for obvious reasons. And for code that has been removed already, that goes doubly so.
thanks,
greg k-h
Quoted 2 lines · Greg Kroah-Hartman
So what happens when 6.6.y goes end-of-life? Why is this driver removed if people are actually using it and willing to fix bugs in it?
It was removed as "unused," but I use it (Xframe-II in a Supermicro X5DA8) - which is what prompted this. I'll pursue getting it back in mainline instead: a revert of aba0138eb7d7 with the fix on top, which also addresses the EOL question.
Quoted 2 lines
I really don't want to take non-upstream patches for obvious reasons. And for code that has been removed already, that goes doubly so.
Understood - withdrawing the stable request; I'll send it to net-next as a revert + fix instead. Happy to test and fix issues as they come up.
Thanks,
Michael
Quoted 24 lines · Greg Kroah-Hartman
Quoted 4 lines · Michael Pratte
Quoted 2 lines · Greg Kroah-Hartman
Why not just remove the driver in older kernels as well if it is not being used?
Quoted 8 lines
It is being used - Xframe-II (17d5:5832) in a Supermicro X5DA8 on 6.6. Please keep it in stable.
Quoted 1 line
And if it's not being used, why is this patch needed at all?
It's used and broken: since v4.2 (
51466a7545b7) s2io arms LSO with MSS=0 on every non-GSO TCP frame, so the card aborts all TCP TX - links fine, UDP/ICMP ok, but no TCP at all. The one-liner restores it.So what happens when 6.6.y goes end-of-life? Why is this driver removed if people are actually using it and willing to fix bugs in it?
I really don't want to take non-upstream patches for obvious reasons. And for code that has been removed already, that goes doubly so.
thanks,
greg k-h