Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210511044253.469034-1-yuri.benditovich@daynix.com>
 <20210511044253.469034-2-yuri.benditovich@daynix.com> <40938c20-5851-089b-c3c0-074bbd636970@redhat.com>
 <CAOEp5OdgYtP+W1thGsTGnvEPWrJ02s1HemskQpnMTUyYbsX4jQ@mail.gmail.com>
In-Reply-To: <CAOEp5OdgYtP+W1thGsTGnvEPWrJ02s1HemskQpnMTUyYbsX4jQ@mail.gmail.com>
From:   Jason Wang <jasowang@redhat.com>
Date:   Tue, 11 May 2021 16:24:39 +0800
Message-ID: <CACGkMEuk3-iP+AxsvhT16t+5dXXtVMGoWPovM=Msm0kvo3LR2Q@mail.gmail.com>
Subject: Re: [PATCH 1/4] virtio-net: add definitions for host USO feature
To:     Yuri Benditovich <yuri.benditovich@daynix.com>
Cc:     "David S. Miller" <davem@davemloft.net>,
        Jakub Kicinski <kuba@kernel.org>,
        "Michael S . Tsirkin" <mst@redhat.com>,
        Network Development <netdev@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        virtualization <virtualization@lists.linux-foundation.org>,
        Yan Vugenfirer <yan@daynix.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Tue, May 11, 2021 at 4:12 PM Yuri Benditovich
<yuri.benditovich@daynix.com> wrote:
>
> On Tue, May 11, 2021 at 9:47 AM Jason Wang <jasowang@redhat.com> wrote:
> >
> >
> > =E5=9C=A8 2021/5/11 =E4=B8=8B=E5=8D=8812:42, Yuri Benditovich =E5=86=99=
=E9=81=93:
> > > Define feature bit and GSO type according to the VIRTIO
> > > specification.
> > >
> > > Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
> > > ---
> > >   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/vir=
tio_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, I=
Pv4 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 */
> >
>