Re: [PATCH 1/4] virtio-net: add definitions for host USO feature

[Jason Wang]

On Tue, May 11, 2021 at 4:12 PM Yuri Benditovich
<yuri.benditovich@xxxxxxxxxx> wrote:
>
> On Tue, May 11, 2021 at 9:47 AM Jason Wang <jasowang@xxxxxxxxxx> wrote:
> >
> >
> > 在 2021/5/11 下午12:42, Yuri Benditovich 写道:
> > > Define feature bit and GSO type according to the VIRTIO
> > > specification.
> > >
> > > Signed-off-by: Yuri Benditovich <yuri.benditovich@xxxxxxxxxx>
> > > ---
> > >   include/uapi/linux/virtio_net.h | 2 ++
> > >   1 file changed, 2 insertions(+)
> > >
> > > diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
> > > index 3f55a4215f11..a556ac735d7f 100644
> > > --- a/include/uapi/linux/virtio_net.h
> > > +++ b/include/uapi/linux/virtio_net.h
> > > @@ -57,6 +57,7 @@
> > >                                        * Steering */
> > >   #define VIRTIO_NET_F_CTRL_MAC_ADDR 23       /* Set MAC address */
> > >
> > > +#define VIRTIO_NET_F_HOST_USO     56 /* Host can handle USO packets */
>
> This is the virtio-net feature

Right, I miss this part.

>
> > >   #define VIRTIO_NET_F_HASH_REPORT  57        /* Supports hash report */
> > >   #define VIRTIO_NET_F_RSS      60    /* Supports RSS RX steering */
> > >   #define VIRTIO_NET_F_RSC_EXT          61    /* extended coalescing info */
> > > @@ -130,6 +131,7 @@ struct virtio_net_hdr_v1 {
> > >   #define VIRTIO_NET_HDR_GSO_TCPV4    1       /* GSO frame, IPv4 TCP (TSO) */
> > >   #define VIRTIO_NET_HDR_GSO_UDP              3       /* GSO frame, IPv4 UDP (UFO) */
> > >   #define VIRTIO_NET_HDR_GSO_TCPV6    4       /* GSO frame, IPv6 TCP */
> > > +#define VIRTIO_NET_HDR_GSO_UDP_L4    5       /* GSO frame, IPv4 UDP (USO) */
>
> This is respective GSO type
>
> >
> >
> > This is the gso_type not the feature actually.
> >
> > I wonder what's the reason for not
> >
> > 1) introducing a dedicated virtio-net feature bit for this
> > (VIRTIO_NET_F_GUEST_GSO_UDP_L4.
>
> This series is not for GUEST's feature, it is only for host feature.
>
> > 2) toggle the NETIF_F_GSO_UDP_L4  feature for tuntap based on the
> > negotiated feature.
>
> The NETIF_F_GSO_UDP_L4 would be required for the guest RX path.
> The guest TX path does not require any flags to be propagated, it only
> allows the guest to transmit large UDP packets and have them
> automatically splitted.
> (This is similar to HOST_UFO but does packet segmentation instead of
> fragmentation. GUEST_UFO indeed requires a respective NETIF flag, as
> it is unclear whether the guest is capable of receiving such packets).

So I think it's better to implement TX/RX in the same series unless
there's something missed:

For Guest TX, NETIF_F_GSO_UDP_L4 needs to be enabled in the guest
virtio-net only when VIRTIO_NET_F_HOST_USO is negotiated.
For guest RX, NETIF_F_GSO_UDP_L4 needs to be enabled on the host
tuntap only when VIRTIO_NET_F_GUEST_USO is neogiated.

Thanks

>
> >
> > Thanks
> >
> >
> > >   #define VIRTIO_NET_HDR_GSO_ECN              0x80    /* TCP has ECN set */
> > >       __u8 gso_type;
> > >       __virtio16 hdr_len;     /* Ethernet + IP + tcp/udp hdrs */
> >
>