[virtio-comment] Re: [virtio-dev] RE: [virtio-comment] RE: [PATCH v9] virtio-net: support inner header hash

[Heng Qi <hengqi@linux.alibaba.com>]

在 2023/2/21 下午11:32, Parav Pandit 写道:
>> From: Heng Qi <hengqi@linux.alibaba.com>
>> Sent: Tuesday, February 21, 2023 8:34 AM
>>> Even without a queue overflow, this shared receive queue may not have a
>> balanced number of packets.
>>> For example, tunnel-2 occupied 90% of the queue and left only 10% for
>> tunnel-1.
>>> So, your example is right (and extreme), a generic mention of QoS covers
>> both aspects.
>>> Secondly "user inside the tunnel" is challenging to explain.
>>> In above sentence talks specifically about the "receive queue" as an object.
>>>
>> "overflow" in my example. If we only use a one-sentence summary to describe
>> tunnel risks, then I think this subparagraph is called "QoS problem for tunnel
>> hash".
>>
> Yes, Tunnel Qos limitations
>
>>>> struct virtio_net_rss_config {
>>>>>>         le32 hash_types;
>>>>>> +    le32 hash_tunnel_types;
>>>>> This field is not needed as device config space advertisement for
>>>>> the support
>>>> is enough.
>>>>
>>>> If so, virtio_net_hash_config does not require hash_tunnel_types when
>>>> it does not need to configure the specific tunnel(s).
>>>>
>>>>> If the intent is to enable hashing for the specific tunnel(s), an
>>>>> individual
>>>> command is better.
>>>>
>>>> Drivers MAY filter out some tunneling types when negotiating this feature.
>>>> Do you think it would be better for us to add a separate command? I
>>>> don't see tools like ethtool that can configure specific tunnels in userspace.
>>>>
>>> The reason I proposed different command is, Let's say we have only
>>> single command.
>>> Rss config command has many other fields unrelated to the inner hash.
>> Yes. For inner hash, fields such as indirection table are irrelevant.
>>
> Right.
>
>>> Hence, to enable inner hash, the driver needs to supply the exact same value
>> for unrelated fields to the same value.
>>> And device needs to compare it with the old value and maintain some sort of
>> cache to derive that nothing changes from the previous hash config, hence,
>> ignore hw configuration for rss.
>>> This mechanism slows down the command for the unrelated task.
>> Totally agree.
>>
>>> Hence, I am considering a separate command that would be simple for the
>> device and driver to implement.
>>
>> I agree with you. Do you want me to do it in this patch, or should we do it in
>> another patch?
>>
> It should be in this patch set because enablement in the device is linked to this capability exposed in this patch.
>
>  From patch split point of view,
>
> Patch-1 to introduce the feature bit, description, and link to CVQ dependency.
> Patch-2 for its link in virtio_net_config structure and description.
> Patch-3 for new command touching control VQ pieces.

Yes, and you seem to have missed my other replies in this thread:), I 
rephrased it here:
virtio_net_hash_config seems to be reusable, as the v9 patch is doing, 
why don't we re-use it?
The reason I can think of is to not expand the virtio_net_hash_config 
structure, just set a separate
structure for VIRTIO_NET_F_HASH_TUNNEL and include hash_tunnel_types?

Thanks.
>
> We can always squash the patch to single when/if it is hard to understand in multiple patches.


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/