Re: [PATCH v1] virtio-net: Fix to avoid using reserved feature bits

On 5/19/25 11:04 AM, Parav Pandit wrote:
>> From: Paolo Abeni <[email protected]>
>> Sent: Monday, May 19, 2025 2:28 PM
>> On 5/7/25 11:57 AM, Paolo Abeni wrote:
>>> On 5/6/25 6:20 PM, Parav Pandit wrote:
>>>> From: Paolo Abeni <[email protected] Sent: Tuesday, May 6, 2025 9:10
>>>> PM
>>>>> On 5/6/25 5:00 PM, Parav Pandit wrote:
>>>>>> From: Paolo Abeni <[email protected] Sent: Tuesday, May 6, 2025
>>>>>> 8:09 PM
>>>>>>> On 5/6/25 10:56 AM, Parav Pandit wrote:
>>>>>>>> Are you good with #3?
>>>>>>>
>>>>>>> I'm sorry for the latency. Let me double check to avoid possible
>>>>>>> misunderstanding; #3 means:
>>>>>>>
>>>>>>> - 0 to 23, and 50 to 127 Feature bits for the specific device type
>>>>>>> + 0 to 23, and 45 to 127 Feature bits for the specific device type
>>>>>>>
>>>>>> No change in above feature bits.
>>>>>>
>>>>>>> using bits 46-39 for UDP tunnel offloads and likely bit 45 for
>>>>>>> VIRTIO_NET_F_OUT_NET_HEADER.
>>>>>>>
>>>>>> This also to use bit 69 as proposed.
>>>>>>
>>>>>>> The VIRTIO_NET_F_CTRL_GUEST_OFFLOADS mapping should be
>> specified
>>>>>>> after eventually a new offload feature will be defined using a bit >= 64.
>>>>>>>
>>>>>> No. UDP tunnel feature bits 65 to 68 maps to command bits
>> 46,47,48,49.
>>>>>> This is the only description change in
>>>>> VIRTIO_NET_F_CTRL_GUEST_OFFLOADS command.
>>>>>> Would it work?
>>>>>
>>>>> AFAICT, yes, it should work.
>>>>>
>>>>> But it will not avoid the immediate need to expand the virtio
>>>>> features negotiation above 64 bits, with the already mentioned
>> complexity.
>>>>>
>>>>> I would preferably avoid that, if possible: I restarted this thread
>>>>> with such a goal.
>>>>>
>>>> In that case we should adopt #2.
>>>
>>> Do we have quorum? Should I send a patch?
>>
>> As per off-list discussion with Michel, there is no agreement on reserved bits
>> re-use.
>>
>> That means the only available option is #3 above.
>>
>> @Parav, would you send a patch to fix the offloads <> features mapping, or do
>> you prefer I'll do it?
>>
> If its urgent, please do it.
> If it can wait till 22 May this week, I will prepare one.

I'll do, because I want to move this thing forward. Note that I think
it's better to change the offload bit name definition for UDP related
offload, to avoid multiple definitions using specifying the string and
different value. i.e.:

---
diff --git a/device-types/net/description.tex
b/device-types/net/description.tex
index 1b6b54d..5d869bc 100644
--- a/device-types/net/description.tex
+++ b/device-types/net/description.tex
@@ -2186,8 +2186,8 @@ \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_UDP_TUNNEL_GSO_CSUM 47
+#define VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO_MAPPED  46
+#define VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO_CSUM_MAPPED 47
 #define VIRTIO_NET_F_GUEST_USO4       54
 #define VIRTIO_NET_F_GUEST_USO6       55

@@ -2205,6 +2205,9 @@ \subsubsection{Control Virtqueue}\label{sec:Device
Types / Network Device / Devi
 negotiation corresponding offload gets enabled to preserve backward
 compatibility.

+Note that device features bit in the [65 to 68] range correspond to
+offloads bits in the [46 to 49] range.
+
 \drivernormative{\subparagraph}{Setting Offloads State}{Device Types /
Network Device / Device Operation / Control Virtqueue / Offloads State
Configuration / Setting Offloads State}

 A driver MUST NOT enable an offload for which the appropriate feature