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

[Johannes Berg <johannes@sipsolutions.net>]

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