* [PATCH V3 net 0/3] Fix checksum issues with Q-in-Q vlans
@ 2017-05-23 17:38 Vladislav Yasevich
2017-05-23 17:38 ` [PATCH V3 net 1/3] vlan: Fix tcp checksum offloads in " Vladislav Yasevich
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Vladislav Yasevich @ 2017-05-23 17:38 UTC (permalink / raw)
To: netdev; +Cc: alexander.duyck, makita.toshiaki, Vladislav Yasevich
TCP checksum appear broken on a lot of devices that
advertise NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM. This problem
becomes very visible/reproducable since the series
commit afb0bc972b526 ("Merge branch 'stacked_vlan_tso'").
In particular, the issue appeared consistently on bnx2 and be2net
drivers (not all drivers were tested).
This short series corrects this by disabling checksum offload
support on packets sent through Q-in-Q vlans if the underlying HW only
enables IP specific checksum features. We currently 'assume' that
any drivers setting NETIF_F_HW_CSUM can correclty pass checksum offsets
to HW. It is up to individual drivers to enable it properly through
ndo_features_check if they have some support for Q-in-Q vlans.
Additionally, be2net driver was fixed to make the proper call.
While looking at the drivers, it was also found that virtio-net ended
up disabling accelerations, which is unnecessary.
V3: Fixed checkpatch errors.
V2: Instead of disabling checksuming for all devices, only devices using
NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM are now affected by this change.
For drivers using NETIF_F_HW_CSUM, we will continue to use checksum
offloading. If any drivers are found to be broken, they would need
be fixed individually.
Vladislav Yasevich (3):
vlan: Fix tcp checksum offloads in Q-in-Q vlans
be2net: Fix offload features for Q-in-Q packets
virtio-net: enable TSO/checksum offloads for Q-in-Q vlans
drivers/net/ethernet/emulex/benet/be_main.c | 4 +++-
drivers/net/virtio_net.c | 1 +
include/linux/if_vlan.h | 10 ++++++++--
3 files changed, 12 insertions(+), 3 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH V3 net 1/3] vlan: Fix tcp checksum offloads in Q-in-Q vlans
2017-05-23 17:38 [PATCH V3 net 0/3] Fix checksum issues with Q-in-Q vlans Vladislav Yasevich
@ 2017-05-23 17:38 ` Vladislav Yasevich
2017-05-24 0:48 ` Toshiaki Makita
2017-05-23 17:38 ` [PATCH V3 net 2/3] be2net: Fix offload features for Q-in-Q packets Vladislav Yasevich
` (2 subsequent siblings)
3 siblings, 1 reply; 6+ messages in thread
From: Vladislav Yasevich @ 2017-05-23 17:38 UTC (permalink / raw)
To: netdev
Cc: alexander.duyck, makita.toshiaki, Vladislav Yasevich, Michal Kubecek
It appears that TCP checksum offloading has been broken for
Q-in-Q vlans. The behavior was execerbated by the
series
commit afb0bc972b52 ("Merge branch 'stacked_vlan_tso'")
that that enabled accleleration features on stacked vlans.
However, event without that series, it is possible to trigger
this issue. It just requires a lot more specialized configuration.
The root cause is the interaction between how
netdev_intersect_features() works, the features actually set on
the vlan devices and HW having the ability to run checksum with
longer headers.
The issue starts when netdev_interesect_features() replaces
NETIF_F_HW_CSUM with a combination of NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM,
if the HW advertises IP|IPV6 specific checksums. This happens
for tagged and multi-tagged packets. However, HW that enables
IP|IPV6 checksum offloading doesn't gurantee that packets with
arbitrarily long headers can be checksummed.
This patch disables IP|IPV6 checksums on the packet for multi-tagged
packets.
CC: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
CC: Michal Kubecek <mkubecek@suse.cz>
Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
---
include/linux/if_vlan.h | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index 8d5fcd6..6686d0f 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -614,14 +614,16 @@ static inline bool skb_vlan_tagged_multi(const struct sk_buff *skb)
static inline netdev_features_t vlan_features_check(const struct sk_buff *skb,
netdev_features_t features)
{
- if (skb_vlan_tagged_multi(skb))
- features = netdev_intersect_features(features,
- NETIF_F_SG |
- NETIF_F_HIGHDMA |
- NETIF_F_FRAGLIST |
- NETIF_F_HW_CSUM |
- NETIF_F_HW_VLAN_CTAG_TX |
- NETIF_F_HW_VLAN_STAG_TX);
+ if (skb_vlan_tagged_multi(skb)) {
+ /* In the case of multi-tagged packets, use a direct mask
+ * instead of using netdev_interesect_features(), to make
+ * sure that only devices supporting NETIF_F_HW_CSUM will
+ * have checksum offloading support.
+ */
+ features &= NETIF_F_SG | NETIF_F_HIGHDMA | NETIF_F_HW_CSUM |
+ NETIF_F_FRAGLIST | NETIF_F_HW_VLAN_CTAG_TX |
+ NETIF_F_HW_VLAN_STAG_TX;
+ }
return features;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH V3 net 2/3] be2net: Fix offload features for Q-in-Q packets
2017-05-23 17:38 [PATCH V3 net 0/3] Fix checksum issues with Q-in-Q vlans Vladislav Yasevich
2017-05-23 17:38 ` [PATCH V3 net 1/3] vlan: Fix tcp checksum offloads in " Vladislav Yasevich
@ 2017-05-23 17:38 ` Vladislav Yasevich
2017-05-23 17:38 ` [PATCH V3 net 3/3] virtio-net: enable TSO/checksum offloads for Q-in-Q vlans Vladislav Yasevich
2017-05-24 20:25 ` [PATCH V3 net 0/3] Fix checksum issues with " David Miller
3 siblings, 0 replies; 6+ messages in thread
From: Vladislav Yasevich @ 2017-05-23 17:38 UTC (permalink / raw)
To: netdev
Cc: alexander.duyck, makita.toshiaki, Vladislav Yasevich,
Sathya Perla, Ajit Khaparde, Sriharsha Basavapatna,
Somnath Kotur
At least some of the be2net cards do not seem to be capabled
of performing checksum offload computions on Q-in-Q packets.
In these case, the recevied checksum on the remote is invalid
and TCP syn packets are dropped.
This patch adds a call to check disbled acceleration features
on Q-in-Q tagged traffic.
CC: Sathya Perla <sathya.perla@broadcom.com>
CC: Ajit Khaparde <ajit.khaparde@broadcom.com>
CC: Sriharsha Basavapatna <sriharsha.basavapatna@broadcom.com>
CC: Somnath Kotur <somnath.kotur@broadcom.com>
Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
---
drivers/net/ethernet/emulex/benet/be_main.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index f3a09ab..4eee18c 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -5078,9 +5078,11 @@ static netdev_features_t be_features_check(struct sk_buff *skb,
struct be_adapter *adapter = netdev_priv(dev);
u8 l4_hdr = 0;
- /* The code below restricts offload features for some tunneled packets.
+ /* The code below restricts offload features for some tunneled and
+ * Q-in-Q packets.
* Offload features for normal (non tunnel) packets are unchanged.
*/
+ features = vlan_features_check(skb, features);
if (!skb->encapsulation ||
!(adapter->flags & BE_FLAGS_VXLAN_OFFLOADS))
return features;
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH V3 net 3/3] virtio-net: enable TSO/checksum offloads for Q-in-Q vlans
2017-05-23 17:38 [PATCH V3 net 0/3] Fix checksum issues with Q-in-Q vlans Vladislav Yasevich
2017-05-23 17:38 ` [PATCH V3 net 1/3] vlan: Fix tcp checksum offloads in " Vladislav Yasevich
2017-05-23 17:38 ` [PATCH V3 net 2/3] be2net: Fix offload features for Q-in-Q packets Vladislav Yasevich
@ 2017-05-23 17:38 ` Vladislav Yasevich
2017-05-24 20:25 ` [PATCH V3 net 0/3] Fix checksum issues with " David Miller
3 siblings, 0 replies; 6+ messages in thread
From: Vladislav Yasevich @ 2017-05-23 17:38 UTC (permalink / raw)
To: netdev; +Cc: alexander.duyck, makita.toshiaki, Vladislav Yasevich
Since virtio does not provide it's own ndo_features_check handler,
TSO, and now checksum offload, are disabled for stacked vlans.
Re-enable the support and let the host take care of it. This
restores/improves Guest-to-Guest performance over Q-in-Q vlans.
Acked-by: Jason Wang <jasowang@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
---
drivers/net/virtio_net.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 665627c..ead7a58 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2020,6 +2020,7 @@ static const struct net_device_ops virtnet_netdev = {
.ndo_poll_controller = virtnet_netpoll,
#endif
.ndo_xdp = virtnet_xdp,
+ .ndo_features_check = passthru_features_check,
};
static void virtnet_config_changed_work(struct work_struct *work)
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH V3 net 1/3] vlan: Fix tcp checksum offloads in Q-in-Q vlans
2017-05-23 17:38 ` [PATCH V3 net 1/3] vlan: Fix tcp checksum offloads in " Vladislav Yasevich
@ 2017-05-24 0:48 ` Toshiaki Makita
0 siblings, 0 replies; 6+ messages in thread
From: Toshiaki Makita @ 2017-05-24 0:48 UTC (permalink / raw)
To: Vladislav Yasevich, netdev
Cc: alexander.duyck, Vladislav Yasevich, Michal Kubecek
On 2017/05/24 2:38, Vladislav Yasevich wrote:
> It appears that TCP checksum offloading has been broken for
> Q-in-Q vlans. The behavior was execerbated by the
> series
> commit afb0bc972b52 ("Merge branch 'stacked_vlan_tso'")
> that that enabled accleleration features on stacked vlans.
>
> However, event without that series, it is possible to trigger
> this issue. It just requires a lot more specialized configuration.
>
> The root cause is the interaction between how
> netdev_intersect_features() works, the features actually set on
> the vlan devices and HW having the ability to run checksum with
> longer headers.
>
> The issue starts when netdev_interesect_features() replaces
> NETIF_F_HW_CSUM with a combination of NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM,
> if the HW advertises IP|IPV6 specific checksums. This happens
> for tagged and multi-tagged packets. However, HW that enables
> IP|IPV6 checksum offloading doesn't gurantee that packets with
> arbitrarily long headers can be checksummed.
>
> This patch disables IP|IPV6 checksums on the packet for multi-tagged
> packets.
>
> CC: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
> CC: Michal Kubecek <mkubecek@suse.cz>
> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
> ---
Thank you for fixing it.
Acked-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
Toshiaki Makita
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH V3 net 0/3] Fix checksum issues with Q-in-Q vlans
2017-05-23 17:38 [PATCH V3 net 0/3] Fix checksum issues with Q-in-Q vlans Vladislav Yasevich
` (2 preceding siblings ...)
2017-05-23 17:38 ` [PATCH V3 net 3/3] virtio-net: enable TSO/checksum offloads for Q-in-Q vlans Vladislav Yasevich
@ 2017-05-24 20:25 ` David Miller
3 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2017-05-24 20:25 UTC (permalink / raw)
To: vyasevich; +Cc: netdev, alexander.duyck, makita.toshiaki, vyasevic
From: Vladislav Yasevich <vyasevich@gmail.com>
Date: Tue, 23 May 2017 13:38:40 -0400
> TCP checksum appear broken on a lot of devices that
> advertise NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM. This problem
> becomes very visible/reproducable since the series
> commit afb0bc972b526 ("Merge branch 'stacked_vlan_tso'").
>
> In particular, the issue appeared consistently on bnx2 and be2net
> drivers (not all drivers were tested).
>
> This short series corrects this by disabling checksum offload
> support on packets sent through Q-in-Q vlans if the underlying HW only
> enables IP specific checksum features. We currently 'assume' that
> any drivers setting NETIF_F_HW_CSUM can correclty pass checksum offsets
> to HW. It is up to individual drivers to enable it properly through
> ndo_features_check if they have some support for Q-in-Q vlans.
>
> Additionally, be2net driver was fixed to make the proper call.
>
> While looking at the drivers, it was also found that virtio-net ended
> up disabling accelerations, which is unnecessary.
>
> V3: Fixed checkpatch errors.
>
> V2: Instead of disabling checksuming for all devices, only devices using
> NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM are now affected by this change.
> For drivers using NETIF_F_HW_CSUM, we will continue to use checksum
> offloading. If any drivers are found to be broken, they would need
> be fixed individually.
Series applied and queued up for -stable, thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-05-24 20:25 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-23 17:38 [PATCH V3 net 0/3] Fix checksum issues with Q-in-Q vlans Vladislav Yasevich
2017-05-23 17:38 ` [PATCH V3 net 1/3] vlan: Fix tcp checksum offloads in " Vladislav Yasevich
2017-05-24 0:48 ` Toshiaki Makita
2017-05-23 17:38 ` [PATCH V3 net 2/3] be2net: Fix offload features for Q-in-Q packets Vladislav Yasevich
2017-05-23 17:38 ` [PATCH V3 net 3/3] virtio-net: enable TSO/checksum offloads for Q-in-Q vlans Vladislav Yasevich
2017-05-24 20:25 ` [PATCH V3 net 0/3] Fix checksum issues with " David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.