Re: [virtio-dev] Re: [virtio-comment] Re: [PATCH v14] virtio-net: support inner header hash
On Thu, Jun 01, 2023 at 01:17:08PM +0800, Heng Qi wrote:
> On Thu, Jun 01, 2023 at 12:30:30AM -0400, Michael S. Tsirkin wrote:
> > On Tue, May 30, 2023 at 03:40:18PM -0400, Michael S. Tsirkin wrote:
> > > On Fri, May 26, 2023 at 04:04:18PM +0800, Heng Qi wrote:
> > > >
> > > >
> > > > 在 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.).
> > > > > > >
> > > > > > > client1client2
> > > > > > >|+---+ |
> > > > > > >+--->|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
> > > > > > > Reviewed-by: Xuan Zhuo
> > > > > > > ---
> > > > > > > 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
> > > > >
Re: [virtio-dev] Re: [virtio-comment] Re: [PATCH v14] virtio-net: support inner header hash
On Tue, May 30, 2023 at 03:40:18PM -0400, Michael S. Tsirkin wrote:
> On Fri, May 26, 2023 at 04:04:18PM +0800, Heng Qi wrote:
> >
> >
> > 在 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.).
> > > > >
> > > > > client1client2
> > > > >|+---+ |
> > > > >+--->|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
> > > > > Reviewed-by: Xuan Zhuo
> > > > > ---
> > > > > 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
Re: [virtio-dev] Re: [virtio-comment] Re: [PATCH v14] virtio-net: support inner header hash
On Fri, May 26, 2023 at 04:04:18PM +0800, Heng Qi wrote:
>
>
> 在 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.).
> > > >
> > > > client1client2
> > > >|+---+ |
> > > >+--->|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
> > > > Reviewed-by: Xuan Zhuo
> > > > ---
> > > > 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.
How are "tunneling protocols" defined though?
Maybe pass a mask of destination UDP ports for which this applies?
Then we don't need options, if port is set in mask then
generic udp tunnel inner hash
