* [PATCH net v4 1/7] net: account for ingress-resolved virtio net hdr state [not found] <cover.v4.git.mst@redhat.com> @ 2026-10-21 7:49 ` Michael S. Tsirkin 2026-10-21 7:49 ` [PATCH net v4 2/7] um: vector: trim skbs before verifying the header Michael S. Tsirkin 2026-10-21 16:08 ` [PATCH net v4 1/7] net: account for ingress-resolved virtio net hdr state Willem de Bruijn 0 siblings, 2 replies; 5+ messages in thread From: Michael S. Tsirkin @ 2026-10-21 7:49 UTC (permalink / raw) To: linux-kernel Cc: Willem de Bruijn, Chris Arges, Björn Töpel, Phil Sutter, Jason Wang, Eugenio Pérez, Xuan Zhuo, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, netdev, virtualization, Max Krasnyansky, Rusty Russell, Richard Weinberger, Anton Ivanov, Johannes Berg, Simon Horman, linux-um The following patch will add ingress resolution of uncommon virtio net hdr checksum metadata (software checksum completion at ingress) which adds performance overhead to the packet processing. We should make it visible, so a hypothetical workload relying on uncommon metadata can be distinguished from none at all. This way issues in the parsing will be reported and not quietly pay the host-CPU checksum overhead instead of offload. Flip every call to virtio_net_hdr_to_skb to test for negative errno only. The um vector raw transport even ignored the return value altogether: rejected packets went on into the stack with their metadata unvalidated, so check it there as well. Have tun, tap and virtio_net count these events per device as rx_csum_sw, exposed via ethtool -S. Signed-off-by: Michael S. Tsirkin <mst@redhat.com> --- arch/um/drivers/vector_transports.c | 4 ++- drivers/net/ipvlan/ipvtap.c | 36 +++++++++++++++++++++++++ drivers/net/macvtap.c | 36 +++++++++++++++++++++++++ drivers/net/tap.c | 8 ++++-- drivers/net/tun.c | 41 +++++++++++++++++++++++++++-- drivers/net/virtio_net.c | 15 ++++++++--- include/linux/if_tap.h | 1 + net/packet/af_packet.c | 4 +-- 8 files changed, 135 insertions(+), 10 deletions(-) diff --git a/arch/um/drivers/vector_transports.c b/arch/um/drivers/vector_transports.c index e1fb2a76fdff..8b7548458663 100644 --- a/arch/um/drivers/vector_transports.c +++ b/arch/um/drivers/vector_transports.c @@ -210,7 +210,9 @@ static int raw_verify_header( return 1; skb_set_network_header(skb, ETH_HLEN); - virtio_net_hdr_to_skb(skb, vheader, virtio_legacy_is_little_endian()); + if (virtio_net_hdr_to_skb(skb, vheader, + virtio_legacy_is_little_endian()) < 0) + return -1; return 0; } diff --git a/drivers/net/ipvlan/ipvtap.c b/drivers/net/ipvlan/ipvtap.c index 66c949d94261..9e2f1c7073d2 100644 --- a/drivers/net/ipvlan/ipvtap.c +++ b/drivers/net/ipvlan/ipvtap.c @@ -1,5 +1,6 @@ // SPDX-License-Identifier: GPL-2.0-only #include <linux/etherdevice.h> +#include <linux/ethtool.h> #include "ipvlan.h" #include <linux/if_vlan.h> #include <linux/if_tap.h> @@ -74,6 +75,32 @@ static void ipvtap_update_features(struct tap_dev *tap, netdev_update_features(vlan->dev); } +static int ipvtap_get_sset_count(struct net_device *dev, int sset) +{ + return sset == ETH_SS_STATS ? 1 : -EOPNOTSUPP; +} + +static void ipvtap_get_strings(struct net_device *dev, u32 stringset, + u8 *data) +{ + if (stringset == ETH_SS_STATS) + ethtool_puts(&data, "rx_csum_sw"); +} + +static void ipvtap_get_ethtool_stats(struct net_device *dev, + struct ethtool_stats *stats, + u64 *data) +{ + struct ipvtap_dev *vlantap = netdev_priv(dev); + + data[0] = atomic64_read(&vlantap->tap.rx_csum_sw); +} + +/* checkpatch warns but this can't be const: cloned from ipvlan's + * ops at newlink time, with the statistics above added. + */ +static struct ethtool_ops ipvtap_ethtool_ops; + static int ipvtap_newlink(struct net_device *dev, struct rtnl_newlink_params *params, struct netlink_ext_ack *extack) @@ -91,6 +118,15 @@ static int ipvtap_newlink(struct net_device *dev, vlantap->tap.update_features = ipvtap_update_features; vlantap->tap.count_rx_dropped = ipvtap_count_rx_dropped; + /* ipvlan's ethtool ops are already installed at this point; + * clone them and add the tap statistics on top. + */ + ipvtap_ethtool_ops = *dev->ethtool_ops; + ipvtap_ethtool_ops.get_sset_count = ipvtap_get_sset_count; + ipvtap_ethtool_ops.get_strings = ipvtap_get_strings; + ipvtap_ethtool_ops.get_ethtool_stats = ipvtap_get_ethtool_stats; + dev->ethtool_ops = &ipvtap_ethtool_ops; + err = netdev_rx_handler_register(dev, tap_handle_frame, &vlantap->tap); if (err) return err; diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c index cc975dfb7380..3288cc6ae6c2 100644 --- a/drivers/net/macvtap.c +++ b/drivers/net/macvtap.c @@ -1,5 +1,6 @@ // SPDX-License-Identifier: GPL-2.0-only #include <linux/etherdevice.h> +#include <linux/ethtool.h> #include <linux/if_macvlan.h> #include <linux/if_tap.h> #include <linux/if_vlan.h> @@ -78,6 +79,32 @@ static void macvtap_update_features(struct tap_dev *tap, netdev_update_features(vlan->dev); } +static int macvtap_get_sset_count(struct net_device *dev, int sset) +{ + return sset == ETH_SS_STATS ? 1 : -EOPNOTSUPP; +} + +static void macvtap_get_strings(struct net_device *dev, u32 stringset, + u8 *data) +{ + if (stringset == ETH_SS_STATS) + ethtool_puts(&data, "rx_csum_sw"); +} + +static void macvtap_get_ethtool_stats(struct net_device *dev, + struct ethtool_stats *stats, + u64 *data) +{ + struct macvtap_dev *vlantap = netdev_priv(dev); + + data[0] = atomic64_read(&vlantap->tap.rx_csum_sw); +} + +/* checkpatch warns but this can't be const: cloned from macvlan's + * ops at newlink time, with the statistics above added. + */ +static struct ethtool_ops macvtap_ethtool_ops; + static int macvtap_newlink(struct net_device *dev, struct rtnl_newlink_params *params, struct netlink_ext_ack *extack) @@ -99,6 +126,15 @@ static int macvtap_newlink(struct net_device *dev, vlantap->tap.count_rx_dropped = macvtap_count_rx_dropped; vlantap->tap.update_features = macvtap_update_features; + /* macvlan's ethtool ops are already installed at this point; + * clone them and add the tap statistics on top. + */ + macvtap_ethtool_ops = *dev->ethtool_ops; + macvtap_ethtool_ops.get_sset_count = macvtap_get_sset_count; + macvtap_ethtool_ops.get_strings = macvtap_get_strings; + macvtap_ethtool_ops.get_ethtool_stats = macvtap_get_ethtool_stats; + dev->ethtool_ops = &macvtap_ethtool_ops; + err = netdev_rx_handler_register(dev, tap_handle_frame, &vlantap->tap); if (err) return err; diff --git a/drivers/net/tap.c b/drivers/net/tap.c index ff67d99deb39..d7520622e809 100644 --- a/drivers/net/tap.c +++ b/drivers/net/tap.c @@ -649,11 +649,13 @@ 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; } + if (err > 0) + atomic64_inc(&tap->rx_csum_sw); } skb_probe_transport_header(skb); @@ -1084,10 +1086,12 @@ 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; } + if (err > 0) + atomic64_inc(&tap->rx_csum_sw); } skb_probe_transport_header(skb); diff --git a/drivers/net/tun.c b/drivers/net/tun.c index 242899f7fd07..19cb443c89b0 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; + atomic64_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,16 @@ 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) { + atomic64_inc(&tun->rx_csum_sw); + err = 0; + } if ((tun->flags & TUN_TYPE_MASK) == IFF_TAP) skb->protocol = eth_type_trans(skb, tun->dev); @@ -2605,12 +2611,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) + atomic64_inc(&tun->rx_csum_sw); skb->protocol = eth_type_trans(skb, tun->dev); skb_reset_network_header(skb); @@ -3739,7 +3748,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] = atomic64_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 daab43ac92ce..b0441bf8e875 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -125,6 +125,7 @@ struct virtnet_rq_stats { u64_stats_t packets; u64_stats_t bytes; u64_stats_t drops; + u64_stats_t rx_csum_sw; u64_stats_t xdp_packets; u64_stats_t xdp_tx; u64_stats_t xdp_redirects; @@ -158,6 +159,7 @@ static const struct virtnet_stat_desc virtnet_sq_stats_desc[] = { static const struct virtnet_stat_desc virtnet_rq_stats_desc[] = { VIRTNET_RQ_STAT("drops", drops), + VIRTNET_RQ_STAT("rx_csum_sw", rx_csum_sw), VIRTNET_RQ_STAT("xdp_packets", xdp_packets), VIRTNET_RQ_STAT("xdp_tx", xdp_tx), VIRTNET_RQ_STAT("xdp_redirects", xdp_redirects), @@ -2502,6 +2504,7 @@ static void virtnet_receive_done(struct virtnet_info *vi, struct receive_queue * { struct virtio_net_common_hdr *hdr; struct net_device *dev = vi->dev; + int ret; hdr = skb_vnet_common_hdr(skb); if (dev->features & NETIF_F_RXHASH && vi->has_rss_hash_report) @@ -2517,15 +2520,21 @@ static void virtnet_receive_done(struct virtnet_info *vi, struct receive_queue * skb->dev = dev; 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))) { + ret = virtio_net_hdr_tnl_to_skb(skb, &hdr->tnl_hdr, vi->rx_tnl, + vi->rx_tnl_csum, + virtio_is_little_endian(vi->vdev)); + if (ret < 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, vi->rx_tnl, vi->rx_tnl_csum); goto frame_err; } + if (ret > 0) { + u64_stats_update_begin(&rq->stats.syncp); + u64_stats_inc(&rq->stats.rx_csum_sw); + u64_stats_update_end(&rq->stats.syncp); + } skb_record_rx_queue(skb, vq2rxq(rq->vq)); skb->protocol = eth_type_trans(skb, dev); diff --git a/include/linux/if_tap.h b/include/linux/if_tap.h index 553552fa635c..4eac24bb1df7 100644 --- a/include/linux/if_tap.h +++ b/include/linux/if_tap.h @@ -41,6 +41,7 @@ struct tap_dev { struct list_head queue_list; int numvtaps; int numqueues; + atomic64_t rx_csum_sw; netdev_features_t tap_features; int minor; diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c index 52ae3d875e97..e6aec9ecfa99 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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH net v4 2/7] um: vector: trim skbs before verifying the header 2026-10-21 7:49 ` [PATCH net v4 1/7] net: account for ingress-resolved virtio net hdr state Michael S. Tsirkin @ 2026-10-21 7:49 ` Michael S. Tsirkin 2026-10-21 16:08 ` Willem de Bruijn 2026-10-21 16:08 ` [PATCH net v4 1/7] net: account for ingress-resolved virtio net hdr state Willem de Bruijn 1 sibling, 1 reply; 5+ messages in thread From: Michael S. Tsirkin @ 2026-10-21 7:49 UTC (permalink / raw) To: linux-kernel Cc: Willem de Bruijn, Chris Arges, Björn Töpel, Phil Sutter, Jason Wang, Eugenio Pérez, Xuan Zhuo, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, netdev, virtualization, Max Krasnyansky, Rusty Russell, Richard Weinberger, Anton Ivanov, Johannes Berg, linux-um The vector receive path over-allocates the skb and only trims it to the received length after the transport header is verified. A follow up patch will complete the packet checksum in software within virtio_net_hdr_to_skb so we need the packet trimmed (otherwise the checksum would be computed incorrectly over the unreceived tail of the buffer). Trim first, so the import sees the packet as it was received. Signed-off-by: Michael S. Tsirkin <mst@redhat.com> --- arch/um/drivers/vector_kern.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/arch/um/drivers/vector_kern.c b/arch/um/drivers/vector_kern.c index 8a70b3a62553..1afbef78df72 100644 --- a/arch/um/drivers/vector_kern.c +++ b/arch/um/drivers/vector_kern.c @@ -865,6 +865,7 @@ static int vector_legacy_rx(struct vector_private *vp) if (skb != NULL) { if (pkt_len > vp->header_size) { + pskb_trim(skb, pkt_len - vp->rx_header_size); if (vp->header_size > 0) { header_check = vp->verify_header( vp->header_rxbuffer, skb, vp); @@ -879,7 +880,6 @@ static int vector_legacy_rx(struct vector_private *vp) skb->ip_summed = CHECKSUM_UNNECESSARY; } } - pskb_trim(skb, pkt_len - vp->rx_header_size); skb->protocol = eth_type_trans(skb, skb->dev); vp->dev->stats.rx_bytes += skb->len; vp->dev->stats.rx_packets++; @@ -981,6 +981,8 @@ static int vector_mmsg_rx(struct vector_private *vp, int budget) for (i = 0; i < packet_count; i++) { skb = (*skbuff_vector); if (mmsg_vector->msg_len > vp->header_size) { + pskb_trim(skb, + mmsg_vector->msg_len - vp->rx_header_size); if (vp->header_size > 0) { header_check = vp->verify_header( mmsg_vector->msg_hdr.msg_iov->iov_base, @@ -1005,8 +1007,6 @@ static int vector_mmsg_rx(struct vector_private *vp, int budget) skb->ip_summed = CHECKSUM_UNNECESSARY; } } - pskb_trim(skb, - mmsg_vector->msg_len - vp->rx_header_size); skb->protocol = eth_type_trans(skb, skb->dev); /* * We do not need to lock on updating stats here -- MST ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net v4 2/7] um: vector: trim skbs before verifying the header 2026-10-21 7:49 ` [PATCH net v4 2/7] um: vector: trim skbs before verifying the header Michael S. Tsirkin @ 2026-10-21 16:08 ` Willem de Bruijn 0 siblings, 0 replies; 5+ messages in thread From: Willem de Bruijn @ 2026-10-21 16:08 UTC (permalink / raw) To: Michael S. Tsirkin, linux-kernel Cc: Willem de Bruijn, Chris Arges, Björn Töpel, Phil Sutter, Jason Wang, Eugenio Pérez, Xuan Zhuo, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, netdev, virtualization, Max Krasnyansky, Rusty Russell, Richard Weinberger, Anton Ivanov, Johannes Berg, linux-um Michael S. Tsirkin wrote: > The vector receive path over-allocates the skb and only trims it > to the received length after the transport header is verified. > > A follow up patch will complete the packet checksum in software within > virtio_net_hdr_to_skb so we need the packet trimmed (otherwise the > checksum would be computed incorrectly over the unreceived tail of the > buffer). > > Trim first, so the import sees the packet as it was received. > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> Reviewed-by: Willem de Bruijn <willemb@google.com> ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net v4 1/7] net: account for ingress-resolved virtio net hdr state 2026-10-21 7:49 ` [PATCH net v4 1/7] net: account for ingress-resolved virtio net hdr state Michael S. Tsirkin 2026-10-21 7:49 ` [PATCH net v4 2/7] um: vector: trim skbs before verifying the header Michael S. Tsirkin @ 2026-10-21 16:08 ` Willem de Bruijn 2026-10-21 16:28 ` Michael S. Tsirkin 1 sibling, 1 reply; 5+ messages in thread From: Willem de Bruijn @ 2026-10-21 16:08 UTC (permalink / raw) To: Michael S. Tsirkin, linux-kernel Cc: Willem de Bruijn, Chris Arges, Björn Töpel, Phil Sutter, Jason Wang, Eugenio Pérez, Xuan Zhuo, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, netdev, virtualization, Max Krasnyansky, Rusty Russell, Richard Weinberger, Anton Ivanov, Johannes Berg, Simon Horman, linux-um Michael S. Tsirkin wrote: > The following patch will add ingress resolution of uncommon virtio net > hdr checksum metadata (software checksum completion at ingress) which > adds performance overhead to the packet processing. > > We should make it visible, so a hypothetical workload relying on > uncommon metadata can be distinguished from none at all. This way issues > in the parsing will be reported and not quietly pay the host-CPU > checksum overhead instead of offload. > > Flip every call to virtio_net_hdr_to_skb to test for negative errno > only. The um vector raw transport even ignored the return value > altogether: rejected packets went on into the stack with their > metadata unvalidated, so check it there as well. > > Have tun, tap and virtio_net count these events per device as > rx_csum_sw, exposed via ethtool -S. > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > --- > arch/um/drivers/vector_transports.c | 4 ++- > drivers/net/ipvlan/ipvtap.c | 36 +++++++++++++++++++++++++ > drivers/net/macvtap.c | 36 +++++++++++++++++++++++++ > drivers/net/tap.c | 8 ++++-- > drivers/net/tun.c | 41 +++++++++++++++++++++++++++-- > drivers/net/virtio_net.c | 15 ++++++++--- > include/linux/if_tap.h | 1 + > net/packet/af_packet.c | 4 +-- > 8 files changed, 135 insertions(+), 10 deletions(-) > > diff --git a/arch/um/drivers/vector_transports.c b/arch/um/drivers/vector_transports.c > index e1fb2a76fdff..8b7548458663 100644 > --- a/arch/um/drivers/vector_transports.c > +++ b/arch/um/drivers/vector_transports.c > @@ -210,7 +210,9 @@ static int raw_verify_header( > return 1; > > skb_set_network_header(skb, ETH_HLEN); > - virtio_net_hdr_to_skb(skb, vheader, virtio_legacy_is_little_endian()); > + if (virtio_net_hdr_to_skb(skb, vheader, > + virtio_legacy_is_little_endian()) < 0) > + return -1; > return 0; > } > > diff --git a/drivers/net/ipvlan/ipvtap.c b/drivers/net/ipvlan/ipvtap.c > index 66c949d94261..9e2f1c7073d2 100644 > --- a/drivers/net/ipvlan/ipvtap.c > +++ b/drivers/net/ipvlan/ipvtap.c > @@ -1,5 +1,6 @@ > // SPDX-License-Identifier: GPL-2.0-only > #include <linux/etherdevice.h> > +#include <linux/ethtool.h> > #include "ipvlan.h" > #include <linux/if_vlan.h> > #include <linux/if_tap.h> > @@ -74,6 +75,32 @@ static void ipvtap_update_features(struct tap_dev *tap, > netdev_update_features(vlan->dev); > } > > +static int ipvtap_get_sset_count(struct net_device *dev, int sset) > +{ > + return sset == ETH_SS_STATS ? 1 : -EOPNOTSUPP; > +} > + > +static void ipvtap_get_strings(struct net_device *dev, u32 stringset, > + u8 *data) > +{ > + if (stringset == ETH_SS_STATS) > + ethtool_puts(&data, "rx_csum_sw"); > +} > + > +static void ipvtap_get_ethtool_stats(struct net_device *dev, > + struct ethtool_stats *stats, > + u64 *data) > +{ > + struct ipvtap_dev *vlantap = netdev_priv(dev); > + > + data[0] = atomic64_read(&vlantap->tap.rx_csum_sw); > +} > + > +/* checkpatch warns but this can't be const: cloned from ipvlan's > + * ops at newlink time, with the statistics above added. > + */ > +static struct ethtool_ops ipvtap_ethtool_ops; > + > static int ipvtap_newlink(struct net_device *dev, > struct rtnl_newlink_params *params, > struct netlink_ext_ack *extack) > @@ -91,6 +118,15 @@ static int ipvtap_newlink(struct net_device *dev, > vlantap->tap.update_features = ipvtap_update_features; > vlantap->tap.count_rx_dropped = ipvtap_count_rx_dropped; > > + /* ipvlan's ethtool ops are already installed at this point; > + * clone them and add the tap statistics on top. > + */ > + ipvtap_ethtool_ops = *dev->ethtool_ops; > + ipvtap_ethtool_ops.get_sset_count = ipvtap_get_sset_count; > + ipvtap_ethtool_ops.get_strings = ipvtap_get_strings; > + ipvtap_ethtool_ops.get_ethtool_stats = ipvtap_get_ethtool_stats; > + dev->ethtool_ops = &ipvtap_ethtool_ops; > + LLM: This cloning pattern in ipvtap_newlink (and identically in macvtap_newlink below) has serious concurrency and hardening issues: 1. Concurrent Data Race: ipvtap_ethtool_ops is a single static global struct. Although newlink runs under rtnl_lock(), ethtool readers on existing devices (e.g. netlink stats_prepare_data() and strset_prepare_data()) run under rcu_read_lock() without RTNL. Whenever a new ipvtap/macvtap device is created, ipvtap_ethtool_ops = *dev->ethtool_ops overwrites the global ops table in-place, temporarily clearing get_sset_count and get_strings to NULL before re-populating them. Any concurrent ethtool query on an existing device can observe NULL or torn function pointers. 2. Writable Function Pointer Table: ethtool_ops contains function pointers and should reside in .rodata (const) to prevent kernel exploit targets. 3. Hardcoded Set Count: ipvtap_get_sset_count returns 1 unconditionally; if the lower device ever adds ethtool stats in the future, they are completely masked. Instead of dynamically mutating a non-const global ops struct on every newlink, either: • Export the base ops from macvlan/ipvlan so tap can declare a const struct ethtool_ops directly, or • At the very least, initialize it once (e.g. guarded by a check or at module init) so it is not re-clobbered while readers are active. Additionally, in general, ipvlan/macvlan/tun/tap changes are preferred to be individual patches. I don't think this is as important as the main patches 4 and 5. If you want to submit quickly, I would drop/postpone these second order features. Maybe go back to the previous tun-only counter for now, if you consider that important enough. > err = netdev_rx_handler_register(dev, tap_handle_frame, &vlantap->tap); > if (err) > return err; > diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c > index cc975dfb7380..3288cc6ae6c2 100644 > --- a/drivers/net/macvtap.c > +++ b/drivers/net/macvtap.c > @@ -1,5 +1,6 @@ > // SPDX-License-Identifier: GPL-2.0-only > #include <linux/etherdevice.h> > +#include <linux/ethtool.h> > #include <linux/if_macvlan.h> > #include <linux/if_tap.h> > #include <linux/if_vlan.h> > @@ -78,6 +79,32 @@ static void macvtap_update_features(struct tap_dev *tap, > netdev_update_features(vlan->dev); > } > > +static int macvtap_get_sset_count(struct net_device *dev, int sset) > +{ > + return sset == ETH_SS_STATS ? 1 : -EOPNOTSUPP; > +} > + > +static void macvtap_get_strings(struct net_device *dev, u32 stringset, > + u8 *data) > +{ > + if (stringset == ETH_SS_STATS) > + ethtool_puts(&data, "rx_csum_sw"); > +} > + > +static void macvtap_get_ethtool_stats(struct net_device *dev, > + struct ethtool_stats *stats, > + u64 *data) > +{ > + struct macvtap_dev *vlantap = netdev_priv(dev); > + > + data[0] = atomic64_read(&vlantap->tap.rx_csum_sw); > +} > + > +/* checkpatch warns but this can't be const: cloned from macvlan's > + * ops at newlink time, with the statistics above added. > + */ > +static struct ethtool_ops macvtap_ethtool_ops; > + > static int macvtap_newlink(struct net_device *dev, > struct rtnl_newlink_params *params, > struct netlink_ext_ack *extack) > @@ -99,6 +126,15 @@ static int macvtap_newlink(struct net_device *dev, > vlantap->tap.count_rx_dropped = macvtap_count_rx_dropped; > vlantap->tap.update_features = macvtap_update_features; > > + /* macvlan's ethtool ops are already installed at this point; > + * clone them and add the tap statistics on top. > + */ > + macvtap_ethtool_ops = *dev->ethtool_ops; > + macvtap_ethtool_ops.get_sset_count = macvtap_get_sset_count; > + macvtap_ethtool_ops.get_strings = macvtap_get_strings; > + macvtap_ethtool_ops.get_ethtool_stats = macvtap_get_ethtool_stats; > + dev->ethtool_ops = &macvtap_ethtool_ops; > + > err = netdev_rx_handler_register(dev, tap_handle_frame, &vlantap->tap); > if (err) > return err; > diff --git a/drivers/net/tap.c b/drivers/net/tap.c > index ff67d99deb39..d7520622e809 100644 > --- a/drivers/net/tap.c > +++ b/drivers/net/tap.c > @@ -649,11 +649,13 @@ 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; > } > + if (err > 0) > + atomic64_inc(&tap->rx_csum_sw); > } > > skb_probe_transport_header(skb); > @@ -1084,10 +1086,12 @@ 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; > } > + if (err > 0) > + atomic64_inc(&tap->rx_csum_sw); (repeat) reset err to 0 in all paths. > } > > skb_probe_transport_header(skb); > diff --git a/drivers/net/tun.c b/drivers/net/tun.c > index 242899f7fd07..19cb443c89b0 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; > + atomic64_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,16 @@ 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) { > + atomic64_inc(&tun->rx_csum_sw); > + err = 0; > + } > > if ((tun->flags & TUN_TYPE_MASK) == IFF_TAP) > skb->protocol = eth_type_trans(skb, tun->dev); > @@ -2605,12 +2611,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) > + atomic64_inc(&tun->rx_csum_sw); > > skb->protocol = eth_type_trans(skb, tun->dev); > skb_reset_network_header(skb); > @@ -3739,7 +3748,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] = atomic64_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 daab43ac92ce..b0441bf8e875 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -125,6 +125,7 @@ struct virtnet_rq_stats { > u64_stats_t packets; > u64_stats_t bytes; > u64_stats_t drops; > + u64_stats_t rx_csum_sw; > u64_stats_t xdp_packets; > u64_stats_t xdp_tx; > u64_stats_t xdp_redirects; > @@ -158,6 +159,7 @@ static const struct virtnet_stat_desc virtnet_sq_stats_desc[] = { > > static const struct virtnet_stat_desc virtnet_rq_stats_desc[] = { > VIRTNET_RQ_STAT("drops", drops), > + VIRTNET_RQ_STAT("rx_csum_sw", rx_csum_sw), > VIRTNET_RQ_STAT("xdp_packets", xdp_packets), > VIRTNET_RQ_STAT("xdp_tx", xdp_tx), > VIRTNET_RQ_STAT("xdp_redirects", xdp_redirects), > @@ -2502,6 +2504,7 @@ static void virtnet_receive_done(struct virtnet_info *vi, struct receive_queue * > { > struct virtio_net_common_hdr *hdr; > struct net_device *dev = vi->dev; > + int ret; > > hdr = skb_vnet_common_hdr(skb); > if (dev->features & NETIF_F_RXHASH && vi->has_rss_hash_report) > @@ -2517,15 +2520,21 @@ static void virtnet_receive_done(struct virtnet_info *vi, struct receive_queue * > > skb->dev = dev; > 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))) { > + ret = virtio_net_hdr_tnl_to_skb(skb, &hdr->tnl_hdr, vi->rx_tnl, > + vi->rx_tnl_csum, > + virtio_is_little_endian(vi->vdev)); > + if (ret < 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, > vi->rx_tnl, vi->rx_tnl_csum); > goto frame_err; > } > + if (ret > 0) { > + u64_stats_update_begin(&rq->stats.syncp); > + u64_stats_inc(&rq->stats.rx_csum_sw); > + u64_stats_update_end(&rq->stats.syncp); > + } Because rx_csum_sw was added to virtnet_rq_stats_desc[], virtnet_poll() already iterates through virtnet_rq_stats_desc[] and syncs each field from the local stack stats into rq->stats at the end of the NAPI polling cycle inside a single u64_stats_update_begin/end(&rq- >stats.syncp). Calling u64_stats_update_begin/end(&rq->stats.syncp) directly per-packet in virtnet_receive_done() bypasses that batching pattern. While SW csum is off the fast path, it is inconsistent with how other rq stats are handled. > > skb_record_rx_queue(skb, vq2rxq(rq->vq)); > skb->protocol = eth_type_trans(skb, dev); > diff --git a/include/linux/if_tap.h b/include/linux/if_tap.h > index 553552fa635c..4eac24bb1df7 100644 > --- a/include/linux/if_tap.h > +++ b/include/linux/if_tap.h > @@ -41,6 +41,7 @@ struct tap_dev { > struct list_head queue_list; > int numvtaps; > int numqueues; > + atomic64_t rx_csum_sw; > netdev_features_t tap_features; > int minor; > > diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c > index 52ae3d875e97..e6aec9ecfa99 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 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net v4 1/7] net: account for ingress-resolved virtio net hdr state 2026-10-21 16:08 ` [PATCH net v4 1/7] net: account for ingress-resolved virtio net hdr state Willem de Bruijn @ 2026-10-21 16:28 ` Michael S. Tsirkin 0 siblings, 0 replies; 5+ messages in thread From: Michael S. Tsirkin @ 2026-10-21 16:28 UTC (permalink / raw) To: Willem de Bruijn Cc: linux-kernel, Chris Arges, Björn Töpel, Phil Sutter, Jason Wang, Eugenio Pérez, Xuan Zhuo, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, netdev, virtualization, Max Krasnyansky, Rusty Russell, Richard Weinberger, Anton Ivanov, Johannes Berg, Simon Horman, linux-um On Wed, Oct 21, 2026 at 12:08:06PM -0400, Willem de Bruijn wrote: > Michael S. Tsirkin wrote: > > The following patch will add ingress resolution of uncommon virtio net > > hdr checksum metadata (software checksum completion at ingress) which > > adds performance overhead to the packet processing. > > > > We should make it visible, so a hypothetical workload relying on > > uncommon metadata can be distinguished from none at all. This way issues > > in the parsing will be reported and not quietly pay the host-CPU > > checksum overhead instead of offload. > > > > Flip every call to virtio_net_hdr_to_skb to test for negative errno > > only. The um vector raw transport even ignored the return value > > altogether: rejected packets went on into the stack with their > > metadata unvalidated, so check it there as well. > > > > Have tun, tap and virtio_net count these events per device as > > rx_csum_sw, exposed via ethtool -S. > > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > --- > > arch/um/drivers/vector_transports.c | 4 ++- > > drivers/net/ipvlan/ipvtap.c | 36 +++++++++++++++++++++++++ > > drivers/net/macvtap.c | 36 +++++++++++++++++++++++++ > > drivers/net/tap.c | 8 ++++-- > > drivers/net/tun.c | 41 +++++++++++++++++++++++++++-- > > drivers/net/virtio_net.c | 15 ++++++++--- > > include/linux/if_tap.h | 1 + > > net/packet/af_packet.c | 4 +-- > > 8 files changed, 135 insertions(+), 10 deletions(-) > > > > diff --git a/arch/um/drivers/vector_transports.c b/arch/um/drivers/vector_transports.c > > index e1fb2a76fdff..8b7548458663 100644 > > --- a/arch/um/drivers/vector_transports.c > > +++ b/arch/um/drivers/vector_transports.c > > @@ -210,7 +210,9 @@ static int raw_verify_header( > > return 1; > > > > skb_set_network_header(skb, ETH_HLEN); > > - virtio_net_hdr_to_skb(skb, vheader, virtio_legacy_is_little_endian()); > > + if (virtio_net_hdr_to_skb(skb, vheader, > > + virtio_legacy_is_little_endian()) < 0) > > + return -1; > > return 0; > > } > > > > diff --git a/drivers/net/ipvlan/ipvtap.c b/drivers/net/ipvlan/ipvtap.c > > index 66c949d94261..9e2f1c7073d2 100644 > > --- a/drivers/net/ipvlan/ipvtap.c > > +++ b/drivers/net/ipvlan/ipvtap.c > > @@ -1,5 +1,6 @@ > > // SPDX-License-Identifier: GPL-2.0-only > > #include <linux/etherdevice.h> > > +#include <linux/ethtool.h> > > #include "ipvlan.h" > > #include <linux/if_vlan.h> > > #include <linux/if_tap.h> > > @@ -74,6 +75,32 @@ static void ipvtap_update_features(struct tap_dev *tap, > > netdev_update_features(vlan->dev); > > } > > > > +static int ipvtap_get_sset_count(struct net_device *dev, int sset) > > +{ > > + return sset == ETH_SS_STATS ? 1 : -EOPNOTSUPP; > > +} > > + > > +static void ipvtap_get_strings(struct net_device *dev, u32 stringset, > > + u8 *data) > > +{ > > + if (stringset == ETH_SS_STATS) > > + ethtool_puts(&data, "rx_csum_sw"); > > +} > > + > > +static void ipvtap_get_ethtool_stats(struct net_device *dev, > > + struct ethtool_stats *stats, > > + u64 *data) > > +{ > > + struct ipvtap_dev *vlantap = netdev_priv(dev); > > + > > + data[0] = atomic64_read(&vlantap->tap.rx_csum_sw); > > +} > > + > > +/* checkpatch warns but this can't be const: cloned from ipvlan's > > + * ops at newlink time, with the statistics above added. > > + */ > > +static struct ethtool_ops ipvtap_ethtool_ops; > > + > > static int ipvtap_newlink(struct net_device *dev, > > struct rtnl_newlink_params *params, > > struct netlink_ext_ack *extack) > > @@ -91,6 +118,15 @@ static int ipvtap_newlink(struct net_device *dev, > > vlantap->tap.update_features = ipvtap_update_features; > > vlantap->tap.count_rx_dropped = ipvtap_count_rx_dropped; > > > > + /* ipvlan's ethtool ops are already installed at this point; > > + * clone them and add the tap statistics on top. > > + */ > > + ipvtap_ethtool_ops = *dev->ethtool_ops; > > + ipvtap_ethtool_ops.get_sset_count = ipvtap_get_sset_count; > > + ipvtap_ethtool_ops.get_strings = ipvtap_get_strings; > > + ipvtap_ethtool_ops.get_ethtool_stats = ipvtap_get_ethtool_stats; > > + dev->ethtool_ops = &ipvtap_ethtool_ops; > > + > > LLM: > > This cloning pattern in ipvtap_newlink (and identically in macvtap_newlink below) has serious concurrency and hardening issues: > 1. Concurrent Data Race: ipvtap_ethtool_ops is a single static global struct. Although newlink runs under rtnl_lock(), ethtool readers on > existing devices (e.g. netlink stats_prepare_data() and strset_prepare_data()) run under rcu_read_lock() without RTNL. Whenever a new > ipvtap/macvtap device is created, ipvtap_ethtool_ops = *dev->ethtool_ops overwrites the global ops table in-place, temporarily clearing > get_sset_count and get_strings to NULL before re-populating them. Any concurrent ethtool query on an existing device can observe NULL or > torn function pointers. > 2. Writable Function Pointer Table: ethtool_ops contains function pointers and should reside in .rodata (const) to prevent kernel exploit > targets. > 3. Hardcoded Set Count: ipvtap_get_sset_count returns 1 unconditionally; if the lower device ever adds ethtool stats in the future, they > are completely masked. > > Instead of dynamically mutating a non-const global ops struct on every newlink, either: > • Export the base ops from macvlan/ipvlan so tap can declare a const struct ethtool_ops directly, or > • At the very least, initialize it once (e.g. guarded by a check or at module init) so it is not re-clobbered while readers are active. > > Additionally, in general, ipvlan/macvlan/tun/tap changes are preferred > to be individual patches. > > I don't think this is as important as the main patches 4 and 5. If you > want to submit quickly, I would drop/postpone these second order > features. Maybe go back to the previous tun-only counter for now, if > you consider that important enough. ok I will do tun only. > > err = netdev_rx_handler_register(dev, tap_handle_frame, &vlantap->tap); > > if (err) > > return err; > > diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c > > index cc975dfb7380..3288cc6ae6c2 100644 > > --- a/drivers/net/macvtap.c > > +++ b/drivers/net/macvtap.c > > @@ -1,5 +1,6 @@ > > // SPDX-License-Identifier: GPL-2.0-only > > #include <linux/etherdevice.h> > > +#include <linux/ethtool.h> > > #include <linux/if_macvlan.h> > > #include <linux/if_tap.h> > > #include <linux/if_vlan.h> > > @@ -78,6 +79,32 @@ static void macvtap_update_features(struct tap_dev *tap, > > netdev_update_features(vlan->dev); > > } > > > > +static int macvtap_get_sset_count(struct net_device *dev, int sset) > > +{ > > + return sset == ETH_SS_STATS ? 1 : -EOPNOTSUPP; > > +} > > + > > +static void macvtap_get_strings(struct net_device *dev, u32 stringset, > > + u8 *data) > > +{ > > + if (stringset == ETH_SS_STATS) > > + ethtool_puts(&data, "rx_csum_sw"); > > +} > > + > > +static void macvtap_get_ethtool_stats(struct net_device *dev, > > + struct ethtool_stats *stats, > > + u64 *data) > > +{ > > + struct macvtap_dev *vlantap = netdev_priv(dev); > > + > > + data[0] = atomic64_read(&vlantap->tap.rx_csum_sw); > > +} > > + > > +/* checkpatch warns but this can't be const: cloned from macvlan's > > + * ops at newlink time, with the statistics above added. > > + */ > > +static struct ethtool_ops macvtap_ethtool_ops; > > + > > static int macvtap_newlink(struct net_device *dev, > > struct rtnl_newlink_params *params, > > struct netlink_ext_ack *extack) > > @@ -99,6 +126,15 @@ static int macvtap_newlink(struct net_device *dev, > > vlantap->tap.count_rx_dropped = macvtap_count_rx_dropped; > > vlantap->tap.update_features = macvtap_update_features; > > > > + /* macvlan's ethtool ops are already installed at this point; > > + * clone them and add the tap statistics on top. > > + */ > > + macvtap_ethtool_ops = *dev->ethtool_ops; > > + macvtap_ethtool_ops.get_sset_count = macvtap_get_sset_count; > > + macvtap_ethtool_ops.get_strings = macvtap_get_strings; > > + macvtap_ethtool_ops.get_ethtool_stats = macvtap_get_ethtool_stats; > > + dev->ethtool_ops = &macvtap_ethtool_ops; > > + > > err = netdev_rx_handler_register(dev, tap_handle_frame, &vlantap->tap); > > if (err) > > return err; > > diff --git a/drivers/net/tap.c b/drivers/net/tap.c > > index ff67d99deb39..d7520622e809 100644 > > --- a/drivers/net/tap.c > > +++ b/drivers/net/tap.c > > @@ -649,11 +649,13 @@ 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; > > } > > + if (err > 0) > > + atomic64_inc(&tap->rx_csum_sw); > > } > > > > skb_probe_transport_header(skb); > > @@ -1084,10 +1086,12 @@ 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; > > } > > + if (err > 0) > > + atomic64_inc(&tap->rx_csum_sw); > > (repeat) reset err to 0 in all paths. > > > } > > > > skb_probe_transport_header(skb); > > diff --git a/drivers/net/tun.c b/drivers/net/tun.c > > index 242899f7fd07..19cb443c89b0 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; > > + atomic64_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,16 @@ 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) { > > + atomic64_inc(&tun->rx_csum_sw); > > + err = 0; > > + } > > > > if ((tun->flags & TUN_TYPE_MASK) == IFF_TAP) > > skb->protocol = eth_type_trans(skb, tun->dev); > > @@ -2605,12 +2611,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) > > + atomic64_inc(&tun->rx_csum_sw); > > > > skb->protocol = eth_type_trans(skb, tun->dev); > > skb_reset_network_header(skb); > > @@ -3739,7 +3748,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] = atomic64_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 daab43ac92ce..b0441bf8e875 100644 > > --- a/drivers/net/virtio_net.c > > +++ b/drivers/net/virtio_net.c > > @@ -125,6 +125,7 @@ struct virtnet_rq_stats { > > u64_stats_t packets; > > u64_stats_t bytes; > > u64_stats_t drops; > > + u64_stats_t rx_csum_sw; > > u64_stats_t xdp_packets; > > u64_stats_t xdp_tx; > > u64_stats_t xdp_redirects; > > @@ -158,6 +159,7 @@ static const struct virtnet_stat_desc virtnet_sq_stats_desc[] = { > > > > static const struct virtnet_stat_desc virtnet_rq_stats_desc[] = { > > VIRTNET_RQ_STAT("drops", drops), > > + VIRTNET_RQ_STAT("rx_csum_sw", rx_csum_sw), > > VIRTNET_RQ_STAT("xdp_packets", xdp_packets), > > VIRTNET_RQ_STAT("xdp_tx", xdp_tx), > > VIRTNET_RQ_STAT("xdp_redirects", xdp_redirects), > > @@ -2502,6 +2504,7 @@ static void virtnet_receive_done(struct virtnet_info *vi, struct receive_queue * > > { > > struct virtio_net_common_hdr *hdr; > > struct net_device *dev = vi->dev; > > + int ret; > > > > hdr = skb_vnet_common_hdr(skb); > > if (dev->features & NETIF_F_RXHASH && vi->has_rss_hash_report) > > @@ -2517,15 +2520,21 @@ static void virtnet_receive_done(struct virtnet_info *vi, struct receive_queue * > > > > skb->dev = dev; > > 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))) { > > + ret = virtio_net_hdr_tnl_to_skb(skb, &hdr->tnl_hdr, vi->rx_tnl, > > + vi->rx_tnl_csum, > > + virtio_is_little_endian(vi->vdev)); > > + if (ret < 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, > > vi->rx_tnl, vi->rx_tnl_csum); > > goto frame_err; > > } > > + if (ret > 0) { > > + u64_stats_update_begin(&rq->stats.syncp); > > + u64_stats_inc(&rq->stats.rx_csum_sw); > > + u64_stats_update_end(&rq->stats.syncp); > > + } > > Because rx_csum_sw was added to virtnet_rq_stats_desc[], virtnet_poll() already iterates through virtnet_rq_stats_desc[] and syncs each > field from the local stack stats into rq->stats at the end of the NAPI polling cycle inside a single u64_stats_update_begin/end(&rq- > >stats.syncp). > Calling u64_stats_update_begin/end(&rq->stats.syncp) directly per-packet in virtnet_receive_done() bypasses that batching pattern. While SW > csum is off the fast path, it is inconsistent with how other rq stats are handled. > > > > > skb_record_rx_queue(skb, vq2rxq(rq->vq)); > > skb->protocol = eth_type_trans(skb, dev); > > diff --git a/include/linux/if_tap.h b/include/linux/if_tap.h > > index 553552fa635c..4eac24bb1df7 100644 > > --- a/include/linux/if_tap.h > > +++ b/include/linux/if_tap.h > > @@ -41,6 +41,7 @@ struct tap_dev { > > struct list_head queue_list; > > int numvtaps; > > int numqueues; > > + atomic64_t rx_csum_sw; > > netdev_features_t tap_features; > > int minor; > > > > diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c > > index 52ae3d875e97..e6aec9ecfa99 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 > > > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-10-21 16:28 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <cover.v4.git.mst@redhat.com>
2026-10-21 7:49 ` [PATCH net v4 1/7] net: account for ingress-resolved virtio net hdr state Michael S. Tsirkin
2026-10-21 7:49 ` [PATCH net v4 2/7] um: vector: trim skbs before verifying the header Michael S. Tsirkin
2026-10-21 16:08 ` Willem de Bruijn
2026-10-21 16:08 ` [PATCH net v4 1/7] net: account for ingress-resolved virtio net hdr state Willem de Bruijn
2026-10-21 16:28 ` Michael S. Tsirkin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox