[REGRESSION][BISECTED] virtio_net CSUM broken with Cloud Hypervisor

[REGRESSION][BISECTED] virtio_net CSUM broken with Cloud Hypervisor

[Alyssa Ross]

[-- Attachment #1: Type: text/plain, Size: 1546 bytes --]

Since 56a06bd40fab ("virtio_net: enable gso over UDP tunnel support."),
networking in Cloud Hypervisor is no longer working for me.

I've narrowed down the problem to here:

> @@ -2555,14 +2567,21 @@ static void virtnet_receive_done(struct virtnet_info *vi, struct receive_queue *
>  	if (dev->features & NETIF_F_RXHASH && vi->has_rss_hash_report)
>  		virtio_skb_set_hash(&hdr->hash_v1_hdr, skb);
>  
> -	if (flags & VIRTIO_NET_HDR_F_DATA_VALID)
> -		skb->ip_summed = CHECKSUM_UNNECESSARY;
> +	hdr->hdr.flags = flags;

It looks like this was added because virtio_net_handle_csum_offload()
looks at the flags from the hdr it's given, rather than having it passed
separately, but it appears something later on relies on the previous
value of hdr->hdr.flags.

From my tracing, hdr->hdr.flags is set to either 0 or
VIRTIO_NET_HDR_F_NEEDS_CSUM before this assignment, and flags is always
0, so in some cases VIRTIO_NET_HDR_F_NEEDS_CSUM now ends up being unset.

> +	if (virtio_net_handle_csum_offload(skb, &hdr->hdr, vi->rx_tnl_csum)) {
> +		net_warn_ratelimited("%s: bad csum: flags: %x, gso_type: %x rx_tnl_csum %d\n",
> +				     dev->name, hdr->hdr.flags,
> +				     hdr->hdr.gso_type, vi->rx_tnl_csum);
> +		goto frame_err;
> +	}

If I change it to save the previous value of hdr->hdr.flags, and restore
it again here, everything works again.

Disabling offload_csum in Cloud Hypervisor is a usable workaround,
because then hdr->hdr.flags is always 0 to begin with anyway.

#regzbot introduced: 56a06bd40fab

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

Re: [REGRESSION][BISECTED] virtio_net CSUM broken with Cloud Hypervisor

[Jason Wang]

On Thu, Oct 30, 2025 at 4:03 AM Alyssa Ross <hi@alyssa.is> wrote:
>
> Since 56a06bd40fab ("virtio_net: enable gso over UDP tunnel support."),
> networking in Cloud Hypervisor is no longer working for me.
>
> I've narrowed down the problem to here:
>
> > @@ -2555,14 +2567,21 @@ static void virtnet_receive_done(struct virtnet_info *vi, struct receive_queue *
> >       if (dev->features & NETIF_F_RXHASH && vi->has_rss_hash_report)
> >               virtio_skb_set_hash(&hdr->hash_v1_hdr, skb);
> >
> > -     if (flags & VIRTIO_NET_HDR_F_DATA_VALID)
> > -             skb->ip_summed = CHECKSUM_UNNECESSARY;
> > +     hdr->hdr.flags = flags;
>
> It looks like this was added because virtio_net_handle_csum_offload()
> looks at the flags from the hdr it's given, rather than having it passed
> separately, but it appears something later on relies on the previous
> value of hdr->hdr.flags.
>
> From my tracing, hdr->hdr.flags is set to either 0 or
> VIRTIO_NET_HDR_F_NEEDS_CSUM before this assignment, and flags is always
> 0, so in some cases VIRTIO_NET_HDR_F_NEEDS_CSUM now ends up being unset.

Are you using XDP, if not there should be no change.

>
> > +     if (virtio_net_handle_csum_offload(skb, &hdr->hdr, vi->rx_tnl_csum)) {
> > +             net_warn_ratelimited("%s: bad csum: flags: %x, gso_type: %x rx_tnl_csum %d\n",
> > +                                  dev->name, hdr->hdr.flags,
> > +                                  hdr->hdr.gso_type, vi->rx_tnl_csum);
> > +             goto frame_err;
> > +     }
>
> If I change it to save the previous value of hdr->hdr.flags, and restore
> it again here, everything works again.
>
> Disabling offload_csum in Cloud Hypervisor is a usable workaround,
> because then hdr->hdr.flags is always 0 to begin with anyway.
>
> #regzbot introduced: 56a06bd40fab

Is mergeable rx buffer enabled, if not, I wonder if this can be fixed by:

https://marc.info/?l=linux-netdev&m=176170721926346&w=2

Thanks

Re: [REGRESSION][BISECTED] virtio_net CSUM broken with Cloud Hypervisor

[Alyssa Ross]

[-- Attachment #1: Type: text/plain, Size: 2190 bytes --]

Jason Wang <jasowang@redhat.com> writes:

> On Thu, Oct 30, 2025 at 4:03 AM Alyssa Ross <hi@alyssa.is> wrote:
>>
>> Since 56a06bd40fab ("virtio_net: enable gso over UDP tunnel support."),
>> networking in Cloud Hypervisor is no longer working for me.
>>
>> I've narrowed down the problem to here:
>>
>> > @@ -2555,14 +2567,21 @@ static void virtnet_receive_done(struct virtnet_info *vi, struct receive_queue *
>> >       if (dev->features & NETIF_F_RXHASH && vi->has_rss_hash_report)
>> >               virtio_skb_set_hash(&hdr->hash_v1_hdr, skb);
>> >
>> > -     if (flags & VIRTIO_NET_HDR_F_DATA_VALID)
>> > -             skb->ip_summed = CHECKSUM_UNNECESSARY;
>> > +     hdr->hdr.flags = flags;
>>
>> It looks like this was added because virtio_net_handle_csum_offload()
>> looks at the flags from the hdr it's given, rather than having it passed
>> separately, but it appears something later on relies on the previous
>> value of hdr->hdr.flags.
>>
>> From my tracing, hdr->hdr.flags is set to either 0 or
>> VIRTIO_NET_HDR_F_NEEDS_CSUM before this assignment, and flags is always
>> 0, so in some cases VIRTIO_NET_HDR_F_NEEDS_CSUM now ends up being unset.
>
> Are you using XDP, if not there should be no change.
>
>>
>> > +     if (virtio_net_handle_csum_offload(skb, &hdr->hdr, vi->rx_tnl_csum)) {
>> > +             net_warn_ratelimited("%s: bad csum: flags: %x, gso_type: %x rx_tnl_csum %d\n",
>> > +                                  dev->name, hdr->hdr.flags,
>> > +                                  hdr->hdr.gso_type, vi->rx_tnl_csum);
>> > +             goto frame_err;
>> > +     }
>>
>> If I change it to save the previous value of hdr->hdr.flags, and restore
>> it again here, everything works again.
>>
>> Disabling offload_csum in Cloud Hypervisor is a usable workaround,
>> because then hdr->hdr.flags is always 0 to begin with anyway.
>>
>> #regzbot introduced: 56a06bd40fab
>
> Is mergeable rx buffer enabled, if not, I wonder if this can be fixed by:
>
> https://marc.info/?l=linux-netdev&m=176170721926346&w=2

That patch does fix it, thank you!

#regzbot fix: virtio-net: fix incorrect flags recording in big mode

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]