Re: [PATCH net-next v2 2/7] um: vector: honor virtio net hdr conversion failures

["Michael S. Tsirkin" <mst@redhat.com>]

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