qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Akihiko Odaki <akihiko.odaki@daynix.com>
To: Yuri Benditovich <yuri.benditovich@daynix.com>,
	Dmitry Fleytman <dmitry.fleytman@gmail.com>,
	Jason Wang <jasowang@redhat.com>,
	Sriram Yagnaraman <sriram.yagnaraman@est.tech>,
	mst@redhat.com, Stefan Weil <sw@weilnetz.de>,
	qemu-devel@nongnu.org
Cc: yan@daynix.com, andrew@daynix.com
Subject: Re: [PATCH 1/4] tap: Added USO support to tap device.
Date: Thu, 20 Jul 2023 09:31:33 +0900	[thread overview]
Message-ID: <d6906dd9-cb4a-065d-1755-c8cf9b5dcda8@daynix.com> (raw)
In-Reply-To: <20230719152139.1316570-2-yuri.benditovich@daynix.com>

Nitpicking: the subject of this patch is somewhat unconventional. What 
about: "tap: Add USO support to tap device"?

On 2023/07/20 0:21, Yuri Benditovich wrote:
> From: Andrew Melnychenko <andrew@daynix.com>
> 
> Passing additional parameters (USOv4 and USOv6 offloads) when
> setting TAP offloads
> 
> Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
> Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
> ---
>   hw/net/e1000e_core.c |  2 +-
>   hw/net/igb_core.c    |  2 +-
>   hw/net/virtio-net.c  |  4 +++-
>   hw/net/vmxnet3.c     |  2 ++
>   include/net/net.h    |  4 ++--
>   net/net.c            |  4 ++--
>   net/tap-bsd.c        |  2 +-
>   net/tap-linux.c      | 15 ++++++++++++---
>   net/tap-linux.h      |  2 ++
>   net/tap-solaris.c    |  2 +-
>   net/tap-stub.c       |  2 +-
>   net/tap-win32.c      |  2 +-
>   net/tap.c            |  6 +++---
>   net/tap_int.h        |  3 ++-
>   14 files changed, 34 insertions(+), 18 deletions(-)
> 
> diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c
> index f8aeafa16b..d4055956ad 100644
> --- a/hw/net/e1000e_core.c
> +++ b/hw/net/e1000e_core.c
> @@ -2852,7 +2852,7 @@ e1000e_update_rx_offloads(E1000ECore *core)
>   
>       if (core->has_vnet) {
>           qemu_set_offload(qemu_get_queue(core->owner_nic)->peer,
> -                         cso_state, 0, 0, 0, 0);
> +                         cso_state, 0, 0, 0, 0, 0, 0);
>       }
>   }
>   
> diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c
> index 8b6b75c522..389eef1549 100644
> --- a/hw/net/igb_core.c
> +++ b/hw/net/igb_core.c
> @@ -2753,7 +2753,7 @@ igb_update_rx_offloads(IGBCore *core)
>   
>       if (core->has_vnet) {
>           qemu_set_offload(qemu_get_queue(core->owner_nic)->peer,
> -                         cso_state, 0, 0, 0, 0);
> +                         cso_state, 0, 0, 0, 0, 0, 0);
>       }
>   }
>   
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 7102ec4817..d2311e7d6e 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -859,7 +859,9 @@ static void virtio_net_apply_guest_offloads(VirtIONet *n)
>               !!(n->curr_guest_offloads & (1ULL << VIRTIO_NET_F_GUEST_TSO4)),
>               !!(n->curr_guest_offloads & (1ULL << VIRTIO_NET_F_GUEST_TSO6)),
>               !!(n->curr_guest_offloads & (1ULL << VIRTIO_NET_F_GUEST_ECN)),
> -            !!(n->curr_guest_offloads & (1ULL << VIRTIO_NET_F_GUEST_UFO)));
> +            !!(n->curr_guest_offloads & (1ULL << VIRTIO_NET_F_GUEST_UFO)),
> +            !!(n->curr_guest_offloads & (1ULL << VIRTIO_NET_F_GUEST_USO4)),
> +            !!(n->curr_guest_offloads & (1ULL << VIRTIO_NET_F_GUEST_USO6)));
>   }
>   
>   static uint64_t virtio_net_guest_offloads_by_features(uint32_t features)
> diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
> index 5dfacb1098..886adae42b 100644
> --- a/hw/net/vmxnet3.c
> +++ b/hw/net/vmxnet3.c
> @@ -1341,6 +1341,8 @@ static void vmxnet3_update_features(VMXNET3State *s)
>                            s->lro_supported,
>                            s->lro_supported,
>                            0,
> +                         0,
> +                         0,
>                            0);
>       }
>   }
> diff --git a/include/net/net.h b/include/net/net.h
> index 1448d00afb..b5ccfbbffb 100644
> --- a/include/net/net.h
> +++ b/include/net/net.h
> @@ -58,7 +58,7 @@ typedef bool (HasVnetHdr)(NetClientState *);
>   typedef bool (HasVnetHdrLen)(NetClientState *, int);
>   typedef bool (GetUsingVnetHdr)(NetClientState *);
>   typedef void (UsingVnetHdr)(NetClientState *, bool);
> -typedef void (SetOffload)(NetClientState *, int, int, int, int, int);
> +typedef void (SetOffload)(NetClientState *, int, int, int, int, int, int, int);
>   typedef int (GetVnetHdrLen)(NetClientState *);
>   typedef void (SetVnetHdrLen)(NetClientState *, int);
>   typedef int (SetVnetLE)(NetClientState *, bool);
> @@ -192,7 +192,7 @@ bool qemu_has_vnet_hdr_len(NetClientState *nc, int len);
>   bool qemu_get_using_vnet_hdr(NetClientState *nc);
>   void qemu_using_vnet_hdr(NetClientState *nc, bool enable);
>   void qemu_set_offload(NetClientState *nc, int csum, int tso4, int tso6,
> -                      int ecn, int ufo);
> +                      int ecn, int ufo, int uso4, int uso6);
>   int qemu_get_vnet_hdr_len(NetClientState *nc);
>   void qemu_set_vnet_hdr_len(NetClientState *nc, int len);
>   int qemu_set_vnet_le(NetClientState *nc, bool is_le);
> diff --git a/net/net.c b/net/net.c
> index 6492ad530e..543e6dec43 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -532,13 +532,13 @@ void qemu_using_vnet_hdr(NetClientState *nc, bool enable)
>   }
>   
>   void qemu_set_offload(NetClientState *nc, int csum, int tso4, int tso6,
> -                          int ecn, int ufo)
> +                          int ecn, int ufo, int uso4, int uso6)
>   {
>       if (!nc || !nc->info->set_offload) {
>           return;
>       }
>   
> -    nc->info->set_offload(nc, csum, tso4, tso6, ecn, ufo);
> +    nc->info->set_offload(nc, csum, tso4, tso6, ecn, ufo, uso4, uso6);
>   }
>   
>   int qemu_get_vnet_hdr_len(NetClientState *nc)
> diff --git a/net/tap-bsd.c b/net/tap-bsd.c
> index 4c98fdd337..abd16a2ad2 100644
> --- a/net/tap-bsd.c
> +++ b/net/tap-bsd.c
> @@ -232,7 +232,7 @@ int tap_fd_set_vnet_be(int fd, int is_be)
>   }
>   
>   void tap_fd_set_offload(int fd, int csum, int tso4,
> -                        int tso6, int ecn, int ufo)
> +                        int tso6, int ecn, int ufo, int uso4, int uso6)
>   {
>   }
>   
> diff --git a/net/tap-linux.c b/net/tap-linux.c
> index f54f308d35..30fcca1bc2 100644
> --- a/net/tap-linux.c
> +++ b/net/tap-linux.c
> @@ -237,7 +237,7 @@ int tap_fd_set_vnet_be(int fd, int is_be)
>   }
>   
>   void tap_fd_set_offload(int fd, int csum, int tso4,
> -                        int tso6, int ecn, int ufo)
> +                        int tso6, int ecn, int ufo, int uso4, int uso6)
>   {
>       unsigned int offload = 0;
>   
> @@ -256,13 +256,22 @@ void tap_fd_set_offload(int fd, int csum, int tso4,
>               offload |= TUN_F_TSO_ECN;
>           if (ufo)
>               offload |= TUN_F_UFO;
> +        if (uso4) {
> +            offload |= TUN_F_USO4;
> +        }
> +        if (uso6) {
> +            offload |= TUN_F_USO6;
> +        }
>       }
>   
>       if (ioctl(fd, TUNSETOFFLOAD, offload) != 0) {
> -        offload &= ~TUN_F_UFO;
> +        offload &= ~(TUN_F_USO4 | TUN_F_USO6);

Shouldn't we just report an error when USO is requested on a system that 
does not support it?

>           if (ioctl(fd, TUNSETOFFLOAD, offload) != 0) {
> -            fprintf(stderr, "TUNSETOFFLOAD ioctl() failed: %s\n",
> +            offload &= ~TUN_F_UFO;
> +            if (ioctl(fd, TUNSETOFFLOAD, offload) != 0) {
> +                fprintf(stderr, "TUNSETOFFLOAD ioctl() failed: %s\n",
>                       strerror(errno));
> +            }
>           }
>       }
>   }
> diff --git a/net/tap-linux.h b/net/tap-linux.h
> index bbbb62c2a7..9a58cecb7f 100644
> --- a/net/tap-linux.h
> +++ b/net/tap-linux.h
> @@ -50,5 +50,7 @@
>   #define TUN_F_TSO6    0x04    /* I can handle TSO for IPv6 packets */
>   #define TUN_F_TSO_ECN 0x08    /* I can handle TSO with ECN bits. */
>   #define TUN_F_UFO     0x10    /* I can handle UFO packets */
> +#define TUN_F_USO4    0x20    /* I can handle USO for IPv4 packets */
> +#define TUN_F_USO6    0x40    /* I can handle USO for IPv6 packets */
>   
>   #endif /* QEMU_TAP_LINUX_H */
> diff --git a/net/tap-solaris.c b/net/tap-solaris.c
> index 38e15028bf..a617a10e5c 100644
> --- a/net/tap-solaris.c
> +++ b/net/tap-solaris.c
> @@ -236,7 +236,7 @@ int tap_fd_set_vnet_be(int fd, int is_be)
>   }
>   
>   void tap_fd_set_offload(int fd, int csum, int tso4,
> -                        int tso6, int ecn, int ufo)
> +                        int tso6, int ecn, int ufo, int uso4, int uso6)
>   {
>   }
>   
> diff --git a/net/tap-stub.c b/net/tap-stub.c
> index a0fa25804b..ac8dfc03b4 100644
> --- a/net/tap-stub.c
> +++ b/net/tap-stub.c
> @@ -67,7 +67,7 @@ int tap_fd_set_vnet_be(int fd, int is_be)
>   }
>   
>   void tap_fd_set_offload(int fd, int csum, int tso4,
> -                        int tso6, int ecn, int ufo)
> +                        int tso6, int ecn, int ufo, int uso4, int uso6)
>   {
>   }
>   
> diff --git a/net/tap-win32.c b/net/tap-win32.c
> index f327d62ab0..7b8b4be02c 100644
> --- a/net/tap-win32.c
> +++ b/net/tap-win32.c
> @@ -741,7 +741,7 @@ static void tap_using_vnet_hdr(NetClientState *nc, bool using_vnet_hdr)
>   }
>   
>   static void tap_set_offload(NetClientState *nc, int csum, int tso4,
> -                     int tso6, int ecn, int ufo)
> +                     int tso6, int ecn, int ufo, int uso4, int uso6)
>   {
>   }
>   
> diff --git a/net/tap.c b/net/tap.c
> index 1bf085d422..14ea4ef26f 100644
> --- a/net/tap.c
> +++ b/net/tap.c
> @@ -307,14 +307,14 @@ static int tap_set_vnet_be(NetClientState *nc, bool is_be)
>   }
>   
>   static void tap_set_offload(NetClientState *nc, int csum, int tso4,
> -                     int tso6, int ecn, int ufo)
> +                     int tso6, int ecn, int ufo, int uso4, int uso6)
>   {
>       TAPState *s = DO_UPCAST(TAPState, nc, nc);
>       if (s->fd < 0) {
>           return;
>       }
>   
> -    tap_fd_set_offload(s->fd, csum, tso4, tso6, ecn, ufo);
> +    tap_fd_set_offload(s->fd, csum, tso4, tso6, ecn, ufo, uso4, uso6);
>   }
>   
>   static void tap_exit_notify(Notifier *notifier, void *data)
> @@ -414,7 +414,7 @@ static TAPState *net_tap_fd_init(NetClientState *peer,
>       s->using_vnet_hdr = false;
>       s->has_ufo = tap_probe_has_ufo(s->fd);
>       s->enabled = true;
> -    tap_set_offload(&s->nc, 0, 0, 0, 0, 0);
> +    tap_set_offload(&s->nc, 0, 0, 0, 0, 0, 0, 0);
>       /*
>        * Make sure host header length is set correctly in tap:
>        * it might have been modified by another instance of qemu.
> diff --git a/net/tap_int.h b/net/tap_int.h
> index 547f8a5a28..d8861d81ba 100644
> --- a/net/tap_int.h
> +++ b/net/tap_int.h
> @@ -37,7 +37,8 @@ void tap_set_sndbuf(int fd, const NetdevTapOptions *tap, Error **errp);
>   int tap_probe_vnet_hdr(int fd, Error **errp);
>   int tap_probe_vnet_hdr_len(int fd, int len);
>   int tap_probe_has_ufo(int fd);
> -void tap_fd_set_offload(int fd, int csum, int tso4, int tso6, int ecn, int ufo);
> +void tap_fd_set_offload(int fd, int csum, int tso4, int tso6, int ecn, int ufo,
> +                        int uso4, int uso6);
>   void tap_fd_set_vnet_hdr_len(int fd, int len);
>   int tap_fd_set_vnet_le(int fd, int vnet_is_le);
>   int tap_fd_set_vnet_be(int fd, int vnet_is_be);


  reply	other threads:[~2023-07-20  0:32 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-19 15:21 [PATCH 0/4] virtio-net: add USO feature (UDP segmentation offload) Yuri Benditovich
2023-07-19 15:21 ` [PATCH 1/4] tap: Added USO support to tap device Yuri Benditovich
2023-07-20  0:31   ` Akihiko Odaki [this message]
2023-07-20 15:53     ` Yuri Benditovich
2023-07-19 15:21 ` [PATCH 2/4] virtio-net: Added USO flags to vhost support Yuri Benditovich
2023-07-19 15:21 ` [PATCH 3/4] virtio-net: added USO support Yuri Benditovich
2023-07-20  0:37   ` Akihiko Odaki
2023-07-20 16:05     ` Yuri Benditovich
2023-07-20 17:17       ` Michael S. Tsirkin
2023-07-19 15:21 ` [PATCH 4/4] virtio-net: Added uso check Yuri Benditovich
2023-07-20  0:41   ` Akihiko Odaki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d6906dd9-cb4a-065d-1755-c8cf9b5dcda8@daynix.com \
    --to=akihiko.odaki@daynix.com \
    --cc=andrew@daynix.com \
    --cc=dmitry.fleytman@gmail.com \
    --cc=jasowang@redhat.com \
    --cc=mst@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=sriram.yagnaraman@est.tech \
    --cc=sw@weilnetz.de \
    --cc=yan@daynix.com \
    --cc=yuri.benditovich@daynix.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).