lkml.org 
[lkml]   [2020]   [May]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2 net-next 2/4] vmxnet3: add support to get/set rx flow hash
On Thu, May 28, 2020 at 07:29:34PM +0000, Ronak Doshi wrote:
>
> On 5/28/20, 12:21 PM, "Michal Kubecek" <mkubecek@suse.cz> wrote:
>
> > On Thu, May 28, 2020 at 11:36:13AM -0700, Ronak Doshi wrote:
> > > With vmxnet3 version 4, the emulation supports multiqueue(RSS) for
> > > UDP and ESP traffic. A guest can enable/disable RSS for UDP/ESP over
> > > IPv4/IPv6 by issuing commands introduced in this patch. ESP ipv6 is
> > > not yet supported in this patch.
> > >
> > > This patch implements get_rss_hash_opts and set_rss_hash_opts
> > > methods to allow querying and configuring different Rx flow hash
> > > configurations.
> > >
> > > Signed-off-by: Ronak Doshi <doshir@vmware.com>
> > > ---
> >
> > This still suffers from the inconsistency between get and set handler
> > I already pointed out in v1:
> >
> > - there is no way to change VMXNET3_RSS_FIELDS_TCPIP{4,6} bits
> > - get_rxnfc() may return value that set_rxnfc() won't accept
> > - get_rxnfc() may return different value than set_rxnfc() set
> >
> > Above, vmxnet3_get_rss_hash_opts() returns 0 or
> > RXH_L4_B_0_1 | RXH_L4_B_2_3 | RXH_IP_SRC | RXH_IP_DST for any of
> > {TCP,UDP}_V{4,6}_FLOW, depending on corresponding bit in rss_fields. But
> > here you accept only all four bits for TCP (both v4 and v6) and either
> > the two RXH_IP_* bits or all four for UDP.
> >
> > Michal
>
> Hi Michal,
>
> That is intentional as vmxnet3 device always expects TCP rss to be enabled
> if rss is supported. If RSS is enabled, by default rss_fields has TCP/IP RSS
> supported and cannot be disabled. Its only for UDP/ESP flows the config
> can change. Hence, get_rss always reports TCP/IP RSS enabled, and set_rss
> does not accept disabling TCP RSS. Hope this answers your concern.

Maybe it will be easier to understand what I'm talking about if I show
it in a table. Let's use shortcuts

L3 = RXH_IP_SRC | RXH_IP_DST
L4 = RXH_L4_B_0_1 | RXH_L4_B_2_3

Then vmxnet3_get_rss_hash_opts() translates rss_fields bits to
info->data like this:
0 1
---------------------------------------------
VMXNET3_RSS_FIELDS_TCPIP4 0 L3 | L4
VMXNET3_RSS_FIELDS_TCPIP6 0 L3 | L4
VMXNET3_RSS_FIELDS_UDPIP4 0 L3 | L4
VMXNET3_RSS_FIELDS_UDPIP6 0 L3 | L4
VMXNET3_RSS_FIELDS_ESPIP4 L3 L3 | L4
VMXNET3_RSS_FIELDS_ESPIP6 L3 L3

But the translation from info->data to bits of rss_fields which should
be the inverse of above, actually works like ("err" means -EINVAL error
and "noop" that nothing is done):

0 L3 L3 | L4
---------------------------------------------------
VMXNET3_RSS_FIELDS_TCPIP4 err err noop
VMXNET3_RSS_FIELDS_TCPIP6 err err noop
VMXNET3_RSS_FIELDS_UDPIP4 err 0 1
VMXNET3_RSS_FIELDS_UDPIP6 err 0 1
VMXNET3_RSS_FIELDS_ESPIP4 err 0 1
VMXNET3_RSS_FIELDS_ESPIP6 err noop err

This means that for both TCP and UDP, you have cases where get handler
will return value which will cause an error if it's fed back to set
handler. And for UDP, accepted values for set are L3 and L3 | L4 but get
handler returns 0 or L3 | L4.

So UDP part is wrong and if TCP always hashes by all four fields, it
would be cleaner to return that information unconditionally, like you do
e.g. for ESPv6 (with a different value).

Michal

\
 
 \ /
  Last update: 2020-05-28 22:16    [W:0.045 / U:0.716 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site