Ingress resolution of uncommon virtio net hdr checksum metadata (software checksum completion at ingress) adds performance overhead but is currently invisible: no counter tracks it, so a hypothetical workload relying on uncommon metadata cannot be distinguished from none at all, and packets quietly pay the host-CPU checksum overhead instead of offload.
Have virtio_net_hdr_to_skb() return 1 when it completes an uncommon checksum in software, and flip every converter call site to test for negative errno. Have tun count these events per device as rx_csum_sw and expose the counter via ethtool -S. Signed-off-by: Michael S. Tsirkin <[email protected]> Assisted-by: LLM Fixes: f43798c27684 ("tun: Allow GSO using virtio_net_hdr") --- drivers/net/tap.c | 4 ++-- drivers/net/tun.c | 44 ++++++++++++++++++++++++++++++++++++-- drivers/net/virtio_net.c | 2 +- include/linux/virtio_net.h | 12 +++++++++-- net/packet/af_packet.c | 4 ++-- 5 files changed, 57 insertions(+), 9 deletions(-) diff --git a/drivers/net/tap.c b/drivers/net/tap.c index ff67d99deb39e..73e8833944463 100644 --- a/drivers/net/tap.c +++ b/drivers/net/tap.c @@ -649,7 +649,7 @@ static ssize_t tap_get_user(struct tap_queue *q, void *msg_control, if (vnet_hdr_len) { err = tun_vnet_hdr_to_skb(q->flags, skb, &vnet_hdr); - if (err) { + if (err < 0) { rcu_read_unlock(); drop_reason = SKB_DROP_REASON_DEV_HDR; goto err_kfree; @@ -1084,7 +1084,7 @@ static int tap_get_user_xdp(struct tap_queue *q, struct xdp_buff *xdp) if (vnet_hdr_len) { err = tun_vnet_hdr_to_skb(q->flags, skb, gso); - if (err) { + if (err < 0) { rcu_read_unlock(); goto err_kfree; } diff --git a/drivers/net/tun.c b/drivers/net/tun.c index 242899f7fd071..138ca8987dfc6 100644 --- a/drivers/net/tun.c +++ b/drivers/net/tun.c @@ -210,6 +210,7 @@ struct tun_struct { u32 flow_count; u32 rx_batched; atomic_long_t rx_frame_errors; + atomic_long_t rx_csum_sw; struct bpf_prog __rcu *xdp_prog; struct tun_prog __rcu *steering_prog; struct tun_prog __rcu *filter_prog; @@ -1935,11 +1936,19 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile, break; } - if (tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, &hdr)) { + err = tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, &hdr); + if (err < 0) { atomic_long_inc(&tun->rx_frame_errors); err = -EINVAL; goto free_skb; } + if (err > 0) + atomic_long_inc(&tun->rx_csum_sw); + /* The status is consumed: keep err clear so later exit paths + * (XDP drop via unlock_frags) do not return it to write() as + * a short count. + */ + err = 0; if ((tun->flags & TUN_TYPE_MASK) == IFF_TAP) skb->protocol = eth_type_trans(skb, tun->dev); @@ -2605,12 +2614,15 @@ static int tun_xdp_one(struct tun_struct *tun, tnl_hdr = (struct virtio_net_hdr_v1_hash_tunnel *)gso; skb->dev = tun->dev; skb_set_network_header(skb, ETH_HLEN); - if (tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, tnl_hdr)) { + ret = tun_vnet_hdr_tnl_to_skb(tun->flags, features, skb, tnl_hdr); + if (ret < 0) { atomic_long_inc(&tun->rx_frame_errors); kfree_skb(skb); ret = -EINVAL; goto out; } + if (ret > 0) + atomic_long_inc(&tun->rx_csum_sw); skb->protocol = eth_type_trans(skb, tun->dev); skb_reset_network_header(skb); @@ -3739,7 +3751,35 @@ static void tun_get_channels(struct net_device *dev, channels->max_combined = tun->flags & IFF_MULTI_QUEUE ? MAX_TAP_QUEUES : 1; } +#define TUN_STATS_LEN 1 +static const char tun_ethtool_strings[TUN_STATS_LEN][ETH_GSTRING_LEN] = { + "rx_csum_sw", +}; + +static int tun_get_sset_count(struct net_device *dev, int sset) +{ + return sset == ETH_SS_STATS ? TUN_STATS_LEN : -EOPNOTSUPP; +} + +static void tun_get_strings(struct net_device *dev, u32 stringset, u8 *data) +{ + if (stringset == ETH_SS_STATS) + memcpy(data, tun_ethtool_strings, + sizeof(tun_ethtool_strings)); +} + +static void tun_get_ethtool_stats(struct net_device *dev, + struct ethtool_stats *stats, u64 *data) +{ + struct tun_struct *tun = netdev_priv(dev); + + data[0] = atomic_long_read(&tun->rx_csum_sw); +} + static const struct ethtool_ops tun_ethtool_ops = { + .get_sset_count = tun_get_sset_count, + .get_strings = tun_get_strings, + .get_ethtool_stats = tun_get_ethtool_stats, .supported_coalesce_params = ETHTOOL_COALESCE_RX_MAX_FRAMES, .get_drvinfo = tun_get_drvinfo, .get_msglevel = tun_get_msglevel, diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index daab43ac92ce4..2e6fd81df9c41 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -2519,7 +2519,7 @@ static void virtnet_receive_done(struct virtnet_info *vi, struct receive_queue * skb_set_network_header(skb, ETH_HLEN); if (virtio_net_hdr_tnl_to_skb(skb, &hdr->tnl_hdr, vi->rx_tnl, vi->rx_tnl_csum, - virtio_is_little_endian(vi->vdev))) { + virtio_is_little_endian(vi->vdev)) < 0) { net_warn_ratelimited("%s: bad gso: type: %x, size: %u, flags %x tunnel %d tnl csum %d\n", dev->name, hdr->hdr.gso_type, hdr->hdr.gso_size, hdr->hdr.flags, diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h index b181183118b3a..d8447db58ac83 100644 --- a/include/linux/virtio_net.h +++ b/include/linux/virtio_net.h @@ -14,6 +14,14 @@ #include <uapi/linux/tcp.h> #include <uapi/linux/virtio_net.h> +/* + * virtio_net_hdr_to_skb() and virtio_net_hdr_tnl_to_skb() return 0 on + * success and a negative errno on failure. On success they may also + * return 1, indicating that an uncommon checksum was completed in + * software at ingress rather than deferred to the egress path; + * drivers can use this to account for such packets. + */ + static inline bool virtio_net_hdr_match_proto(__be16 protocol, __u8 gso_type) { switch (gso_type & ~VIRTIO_NET_HDR_GSO_ECN) { @@ -292,7 +300,7 @@ static inline int __virtio_net_hdr_to_skb(struct sk_buff *skb, err = skb_checksum_help(skb); skb->transport_header = old_transport; - return err; + return err ?: 1; } return 0; @@ -441,7 +449,7 @@ virtio_net_hdr_tnl_to_skb(struct sk_buff *skb, /* Let the basic parsing deal with plain GSO features. */ ret = __virtio_net_hdr_to_skb(skb, hdr, true, hdr->gso_type & ~gso_tunnel_type, false); - if (ret) + if (ret < 0) return ret; /* In case of USO, the inner protocol is still unknown and diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c index 52ae3d875e971..e6aec9ecfa994 100644 --- a/net/packet/af_packet.c +++ b/net/packet/af_packet.c @@ -2913,7 +2913,7 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg) } if (has_vnet_hdr) { - if (virtio_net_hdr_to_skb(skb, &vnet_hdr, vio_le())) { + if (virtio_net_hdr_to_skb(skb, &vnet_hdr, vio_le()) < 0) { tp_len = -EINVAL; goto tpacket_error; } @@ -3136,7 +3136,7 @@ static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len) if (vnet_hdr_sz) { err = virtio_net_hdr_to_skb(skb, &vnet_hdr, vio_le()); - if (err) + if (err < 0) goto out_free; len += vnet_hdr_sz; } -- MST
