public inbox for virtio-comment@lists.linux.dev
 help / color / mirror / Atom feed
From: Cornelia Huck <cohuck@redhat.com>
To: Parav Pandit <parav@nvidia.com>, Paolo Abeni <pabeni@redhat.com>,
	"virtio-comment@lists.linux.dev" <virtio-comment@lists.linux.dev>
Cc: "maxime.coquelin@redhat.com" <maxime.coquelin@redhat.com>,
	Eelco Chaudron <echaudro@redhat.com>,
	Jason Wang <jasowang@redhat.com>,
	Stefano Garzarella <sgarzare@redhat.com>,
	Willem de Bruijn <willemb@google.com>,
	"kshankar@marvell.com" <kshankar@marvell.com>
Subject: RE: [PATCH v11 3/4] virtio-net: define UDP tunnel segmentation offload feature
Date: Mon, 20 Jan 2025 10:03:42 +0100	[thread overview]
Message-ID: <87ldv597rl.fsf@redhat.com> (raw)
In-Reply-To: <CY8PR12MB7195D914B9EF00FF62C5E1DFDCE42@CY8PR12MB7195.namprd12.prod.outlook.com>

On Sun, Jan 19 2025, Parav Pandit <parav@nvidia.com> wrote:

> Hi Paolo, Michael,
>
>> From: Paolo Abeni <pabeni@redhat.com>
>> Sent: Wednesday, November 27, 2024 3:07 PM
>> 
>> 
>> +\item[VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO (46)] Driver can receive
>> GSO
>> +packets
>> +  carried by a UDP tunnel.
>> +
>> +\item[VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO (48)] Device can receive
>> GSO
>> +packets
>> +  carried by a UDP tunnel.
>
> Feature bits 42 to 49 are not applicable to device specific type. They are reserved.
> Please see the note. 
>
> Current spec description is:
>
> 0 to 23, and 50 to 127 Feature bits for the specific device type
> 24 to 41 Feature bits reserved for extensions to the queue and feature negotiation mechanisms, see 6
> 42 to 49, and 128 and above Feature bits reserved for future extensions.
>
> So 2 bits of these patch and of patch 4, 46 to 49 to be shifted to bits 65 to 68.
>
> Or an alternative would be to amend,
>
> Bits 46 to 127 for specific device type,
> And 42 to 45 and 128 reserved for future extensions.
>
> Michael,
> Please let me know so that I can have the follow up patch to this one as voting is started.
> And the fix is required for it.
>
> Is there any historic reason for 42-49 as reserved, or it can be used as proposed in this patch?
> If it was only kept as elastic buffer for device generic, and device type to grow it is probably ok.
> Using 46 to 49 has small advantage to reuse in the offloads bit as below.
> Please confirm.

See the reasoning given in 88895f56e642 ("Reserve more feature bits for
device type usage") -- the idea was to push out the point in time when
generic feature bits will have to be in the bit area beyond 63. Given
that we only used one of those generic bits in the meantime, I assume it
will still be some time before we run out of bits, even if we let the
net driver grab some bits there. Still, I'd probably prefer that the net
device used different bits. [And issues like this are easy to miss
during review unfortunately.]

However, if the device was to use different bits, the clean solution
would be to withdraw, respin, and revote. If we only changed the
reserved ranges, we'd be fine with a change on top, so that would be
less hassle in the short team. If we decide that we can live with the
smaller feature bit space below 63, I'd be fine with that solution.

>
>
>> @@ -1862,6 +2068,7 @@ \subsubsection{Control
>> Virtqueue}\label{sec:Device Types / Network Device / Devi
>>  #define VIRTIO_NET_F_GUEST_TSO6       8
>>  #define VIRTIO_NET_F_GUEST_ECN        9
>>  #define VIRTIO_NET_F_GUEST_UFO        10
>> +#define VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO  46
>>  #define VIRTIO_NET_F_GUEST_USO4       54
>>  #define VIRTIO_NET_F_GUEST_USO6       55


  parent reply	other threads:[~2025-01-20  9:03 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-11-27  9:36 [PATCH v11 0/4] virtio-net: define UDP tunnel offload Paolo Abeni
2024-11-27  9:36 ` [PATCH v11 1/4] virtio-net: clarify NEEDS_CSUM semantic for GSO packats Paolo Abeni
2024-11-28  3:00   ` Jason Wang
2025-01-19  7:56   ` Parav Pandit
2024-11-27  9:36 ` [PATCH v11 2/4] virtio-net: clarify DATA_VALID semantic for encap protos Paolo Abeni
2024-11-28  3:01   ` Jason Wang
2025-01-19  7:57   ` Parav Pandit
2024-11-27  9:36 ` [PATCH v11 3/4] virtio-net: define UDP tunnel segmentation offload feature Paolo Abeni
2024-11-28  3:01   ` Jason Wang
2025-01-19  8:06   ` Parav Pandit
2025-01-20  8:44     ` Paolo Abeni
2025-01-20  8:57       ` Parav Pandit
2025-01-20  9:03     ` Cornelia Huck [this message]
2025-01-20 11:32       ` Parav Pandit
2025-01-20 12:20         ` Cornelia Huck
2025-01-20 14:19           ` Parav Pandit
2025-01-20 14:50             ` Cornelia Huck
2025-01-21 18:40             ` Paolo Abeni
2025-01-21 19:38               ` Parav Pandit
2025-01-22 14:43                 ` Paolo Abeni
2025-01-26  6:24               ` Parav Pandit
2024-11-27  9:36 ` [PATCH v11 4/4] virtio-net: define UDP tunnel checksum " Paolo Abeni
2024-11-28  3:01   ` Jason Wang
2024-11-27  9:48 ` [PATCH v11 0/4] virtio-net: define UDP tunnel offload Paolo Abeni
2025-01-14 15:04   ` Paolo Abeni
2025-01-14 15:16     ` Michael S. Tsirkin
2025-01-14 15:57       ` Paolo Abeni

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=87ldv597rl.fsf@redhat.com \
    --to=cohuck@redhat.com \
    --cc=echaudro@redhat.com \
    --cc=jasowang@redhat.com \
    --cc=kshankar@marvell.com \
    --cc=maxime.coquelin@redhat.com \
    --cc=pabeni@redhat.com \
    --cc=parav@nvidia.com \
    --cc=sgarzare@redhat.com \
    --cc=virtio-comment@lists.linux.dev \
    --cc=willemb@google.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