Re: [PATCH net-next v2 5/7] net: add a sysctl to opt out of virtio net hdr checksum validation

On Fri, Oct 16, 2026 at 11:30:17AM -0400, Willem de Bruijn wrote:

Michael S. Tsirkin wrote:

quoted

On Thu, Oct 15, 2026 at 01:50:54PM -0400, Willem de Bruijn wrote:

quoted

Michael S. Tsirkin wrote:

quoted

The ingress validation as implemented by the previous
patch is stricter than what it replaced: GSO
packets whose checksum metadata matches no transport header are
dropped, and non-GSO ones get their checksum completed in place.
Such packets used to pass with the metadata unvalidated,
and with no overhead.

I judge that this should not affect any deployments, but there is still
some risk that it does.  In case a deployment depends on them anyway,
provide a per-netns switch to get the old behavior back:

echo 0 > /proc/sys/net/core/virtio_net_hdr_csum_check

The default is validating.

Assisted-by: LLM
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 Documentation/admin-guide/sysctl/net.rst | 15 +++++++
 include/linux/virtio_net.h               | 23 ++++++++++
 include/net/netns/core.h                 |  1 +
 net/core/dev.c                           |  6 +++
 net/core/net_namespace.c                 |  1 +
 net/core/sysctl_net_core.c               | 54 ++++++++++++++++++++++++
 6 files changed, 100 insertions(+)

diff --git a/Documentation/admin-guide/sysctl/net.rst b/Documentation/admin-guide/sysctl/net.rst
index e586e17fc7a5..7f230929c37e 100644
--- a/Documentation/admin-guide/sysctl/net.rst
+++ b/Documentation/admin-guide/sysctl/net.rst

@@ -246,6 +246,21 @@ processes are dropped unless socket option SOF_TIMESTAMPING_OPT_TSONLY is set.
 Default: 1 (on)
 
 
+virtio_net_hdr_csum_check
+-------------------------

Make more generic, virtio_net_hdr_validate?

In case we add more validation in the future (as we have in the past),
without having to introduce multiple such knobs.

But it's uapi, are you sure we'll be able to change the meaning
of an existing knob, even if we keep the name vague?

quoted

quoted

+
+When processing packets with a virtio net header (through tun, tap, a
+packet socket or a virtio network device) that request checksum offload
+(VIRTIO_NET_HDR_F_NEEDS_CSUM), validate the packet: complete the checksum
+in software, or drop the packet where the header does not match it.
+
+When disabled, the header is trusted not to corrupt the packet, and the
+checksum is offloaded to hardware (or not verified if it never reaches
+hardware). This is only safe if no protocol is processing the packets.
+
+Default: 1 (on)
+
+
 wmem_default
 ------------
 

diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index 357115ff0ec2..aa2f8ac4657c 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h

@@ -5,6 +5,7 @@
 #include <linux/if_vlan.h>
 #include <linux/ip.h>
 #include <linux/ipv6.h>
+#include <linux/jump_label.h>
 #include <linux/netdevice.h>
 #include <linux/udp.h>
 #include <net/gso.h>

@@ -78,6 +79,19 @@ static inline struct iphdr *virtio_net_ip4_parse(struct sk_buff *skb,
 	*thoff = nhoff + iph->ihl * sizeof(u32);
 	return iph;
 }
+/* Defined in net/core/dev.c, enabled while any network namespace
+ * has net.core.virtio_net_hdr_csum_check set to 0.
+ */

Should this be a global sysctl, rather than per netns?

For customers I work with, it would be much less useful.

Third option: make it an integer rather than a boolean.

0: no validation
1: current level of validation
2 and beyond: if we have to extend it later

Not pretty, I'll admit. So fine to keep as is too if you prefer.
But I do foresee more changes in this area, if history is a guide.
 

quoted

quoted

Even if per-netns, most likely do not want it writable by netns root
with user namespaces.

That is the equivalent of ns_capable(CAP_NET_RAW), which is the cause
of many crashes due to bad packets ingested by, e.g., a packet socket.

Hmm, yes i get that. Two knobs? a. verify b. the per netns value ro? Both
on by default?

A single system wide field is simplest.

Is there a benefit to making this configurable on a per netns basis?

As I tried to explain: a specific container breaks, you decide to trust it,
you do not need to trust all.

If so, that's fine too. As long as root-in-namespace cannot modify it,
so readonly indeed.

Right. So I envision along the lines of:

net.core.virtio_net_hdr_validate_all - validate packets from any netns,
netns root can not change it, only host root

net.core.virtio_net_hdr_validate - per netns


both set by default, validation runs if either is set.

-- 
MST