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

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

在 2023/5/23 上午11:58, Heng Qi 写道:
> On Mon, May 22, 2023 at 03:19:16PM -0400, Michael S. Tsirkin wrote:
>> On Mon, May 22, 2023 at 01:02:36PM +0800, Heng Qi wrote:
>>> 1. Currently, a received encapsulated packet has an outer and an inner header, but
>>> the virtio device is unable to calculate the hash for the inner header. The same
>>> flow can traverse through different tunnels, resulting in the encapsulated
>>> packets being spread across multiple receive queues (refer to the figure below).
>>> However, in certain scenarios, we may need to direct these encapsulated packets of
>>> the same flow to a single receive queue. This facilitates the processing
>>> of the flow by the same CPU to improve performance (warm caches, less locking, etc.).
>>>
>>>                 client1                    client2
>>>                    |        +-------+         |
>>>                    +------->|tunnels|<--------+
>>>                             +-------+
>>>                                |  |
>>>                                v  v
>>>                        +-----------------+
>>>                        | monitoring host |
>>>                        +-----------------+
>>>
>>> To achieve this, the device can calculate a symmetric hash based on the inner headers
>>> of the same flow.
>>>
>>> 2. For legacy systems, they may lack entropy fields which modern protocols have in
>>> the outer header, resulting in multiple flows with the same outer header but
>>> different inner headers being directed to the same receive queue. This results in
>>> poor receive performance.
>>>
>>> To address this limitation, inner header hash can be used to enable the device to advertise
>>> the capability to calculate the hash for the inner packet, regaining better receive performance.
>>>
>>> Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
>>> Reviewed-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
>>> ---
>>> v13->v14:
>>> 	1. Move supported_hash_tunnel_types from config space into cvq command. @Parav Pandit
>>> 	2. Rebase to master branch.
>>> 	3. Some minor modifications.
>> So, I proposed adding a "generic UDP tunnel" option which simply uses UDP source
>> port for hash. I think it will help us not having to chaise future tunnels as
>> more and more are added.
> I agree, but I thought we'd do this in another thread, sorry.
> Following your suggestion, we should add a field similar to
> \field{generic_udp_tunnel_option} in the virtnet_hash_tunnel_config_set
> structure.
>
> \field{generic_udp_tunnel_option} should be 0, 1 or 2.
>
> \field{hash_tunnel_types} is still useful, but for more general purpose we need
> to use it together with \field{generic_udp_tunnel_option}.
>
> When \field{generic_udp_tunnel_option} is 0, all tunneling protocols included in
> \field{hash_tunnel_types} use the inner header for hashing. For other tunnel
> protocols not included in \field{hash_tunnel_types}, the hash is calculated as if
> VIRTIO_NET_F_TUNNEL_HASH is not negotiated.
>
> When \field{generic_udp_tunnel_option} is 1, all tunneling protocols included in
> \field{hash_tunnel_types} use the inner header for hashing. For other tunnel
> protocols not included in \field{hash_tunnel_types}, if their outer headers are
> based on UDP protocol, the device use the outer UDP source port for hashing.
> For the rest of the tunnel protocols, the hash is calculated as if VIRTIO_NET_F_TUNNEL_HASH
> was not negotiated.
>
> When \field{generic_udp_tunnel_option} is 2, for all UDP tunneling protocols,
> the outer udp source port is used for hashing, otherwise if the tunneling protocol
> is included in \field{hash_tunnel_types}, the inner header is used for hashing.
> For the rest of the tunnel protocols, the hash is calculated as if VIRTIO_NET_F_TUNNEL_HASH
> was not negotiated.
>
> And for this option, we need to add a reminder:
> Although the \field{generic_udp_tunnel_option} helps us adapt to more new
> tunneling protocols, it is still an unreliable option, especially for
> tunneling protocols that use "SHOULD" "Recommended" in their own
> specifications, because it means the udp source port does not
> always fully identify a stream.
>

Hi, Michael.

Do you agree with this plan? Please let me know if you have any comments.:)

If there are no comments, I can start a new version to make progress.

Thanks.

>> I also suggested dropping some tunnels which are less common and where
>> the specification is unambiguous enough that source port should include
>> inner hash.
> OK, I'll re-screen and update the tunneling protocols we already include
> (e.g. remove STT since it fits what you said).
>
> Thanks.
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org