Re: [PATCH net v3 6/8] net: add a sysctl to opt out of virtio net hdr checksum validation

[Willem de Bruijn <willemdebruijn.kernel@gmail.com>]

Michael S. Tsirkin wrote:
> On Tue, Oct 20, 2026 at 04:19:41PM -0400, Willem de Bruijn wrote:
> > Michael S. Tsirkin wrote:
> > > On Tue, Oct 20, 2026 at 03:54:23PM -0400, Willem de Bruijn wrote:
> > > > Michael S. Tsirkin wrote:
> > > > > 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 | 19 ++++++++
> > > > >  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               | 60 ++++++++++++++++++++++++
> > > > >  6 files changed, 110 insertions(+)
> > > > > 
> > > > > diff --git a/Documentation/admin-guide/sysctl/net.rst b/Documentation/admin-guide/sysctl/net.rst
> > > > > index e586e17fc7a5..b1a1c4714b22 100644
> > > > > --- a/Documentation/admin-guide/sysctl/net.rst
> > > > > +++ b/Documentation/admin-guide/sysctl/net.rst
> > > > > @@ -246,6 +246,25 @@ processes are dropped unless socket option SOF_TIMESTAMPING_OPT_TSONLY is set.
> > > > >  Default: 1 (on)
> > > > >  
> > > > >  
> > > > > +virtio_net_hdr_csum_check
> > > > > +-------------------------
> > > > 
> > > > virtio_net_hdr_validate? In case we do have to add more validation
> > > > later.
> > > 
> > > If you insist. My worry would be what if that validation is again
> > > user-visible somehow and we need to distinguish users who want
> > > to disable that future thing and users who want to disable
> > > just this current thing.
> > > 
> > > Hard to predict the future. But I will go with your intuition.
> > 
> > Thanks, but definitely feel free to ignore my suggestions.
> > 
> > My intuition would be to define as
> > 
> > 0: off
> > 1: everything one
> > 2+: sadly needed fudge values because we keep having to tighten,
> >     but some crazy workloads need to opt-out to avoid breakage.
> 
> yes you said i just do not get how it will work for userspace,
> maybe u can explain?
> 
> and what specifically is needed right now, while we
> just have 1.

Right now the sysctl is boolean.

If we define it as a narrow, very precisely defined sysctl, then its
exact behavior is set in stone.

If we then add additional validation later, that can similarly have
unintended breakage of weird users, we need to add another sysctl.

And if that validation is not entirely independent, i.e., builds on
this, you get a weird set of interdependent sysctls.

While in general systems will run with all validation enabled, unless
they do something weird that gets hit by validation.

A single broadly defined sysctl ("validate input at the source") can
be refined over time, including to expand to validation of other
virtio_net_hdr fields or protocol combinations.

Even if defining it more broadly we can still leave it a boolean:
all validation on or off.

Or, optionally, we can also define additional validation levels,
e.g, 0:off, 1:everything-on, 2:validate-csum-only.