On Fri, Oct 16, 2026 at 11:44:16AM +0200, Johannes Berg wrote:
> On Fri, 2026-10-16 at 05:43 -0400, Michael S. Tsirkin wrote:
> >
> > > > > > - 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;
> > > > > >
> > > > >
> > > > > Seems it'd be simpler to just
> > > > >
> > > > > return virtio_net_hdr_to_skb(...);
> > > > >
> > > > > As far as I can tell, the function only returns -EINVAL or 0 (never a
> > > > > positive value), and the caller only cares about negative (error) or
> > > > > positive (no checksum needed), so that'd be equivalent?
> > > > >
> > > > > johannes
> > > >
> > > > if raw_verify_header can safely return any errno - i did not want to
> > > > audit it.
> > >
> > > You already used an LLM and still didn't want to? ;-)
> > >
> > > I checked briefly (by hand) and it seems fine, it's assigned to the
> > > function pointer, which only has two callers on the _kern.c side.
> > >
> > > johannes
> >
> >
> > header_check = vp->verify_header(
> >
> > mmsg_vector->msg_hdr.msg_iov->iov_base,
> > skb,
> > vp
> > );
> > if (header_check < 0) {
> > /* Overlay header failed to verify -
> > discard.
> > * We can actually keep this skb and reuse
> > it,
> > * but that will make the prep logic too
> > * complex.
> > */
> > dev_kfree_skb_irq(skb);
> > vp->estats.rx_encaps_errors++;
> > (*skbuff_vector) = NULL;
> > mmsg_vector++;
> > skbuff_vector++;
> > continue;
> > }
> > if (header_check > 0) {
> > vp->estats.rx_csum_offload_good++;
> > skb->ip_summed =
> > CHECKSUM_UNNECESSARY;
> > }
> >
> >
> >
> > so any negative is fine, but if we return 1 then it will force to
> > CHECKSUM_UNNECESSARY
> > which I am not sure is what was intended?
> >
> Right, but virtio_net_hdr_to_skb() cannot return 1, only 0 on success
> and -ERROR on failure?
>
> johannes
After this patchset, it can, to signal slow path software checksum.
I'm changing callers first so that changing the function itself
does not break bisect. I should mention this in the commit log.
--
MST
