Re: [PATCH net-next 2/4] vmxnet3: add support to get/set rx flow hash
Hi Ronak,
I love your patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
[also build test WARNING on sparc-next/master linus/master ipvs/master v5.7-rc7
next-20200526]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url:
https://github.com/0day-ci/linux/commits/Ronak-Doshi/vmxnet3-upgrade-to-version-4/20200528-112935
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git
50ce4c099bebf56be86c9448f7f4bcd34f33663c
config: alpha-allyesconfig (attached as .config)
compiler: alpha-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross
ARCH=alpha
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot
All warnings (new ones prefixed by >>, old ones prefixed by <<):
drivers/net/vmxnet3/vmxnet3_ethtool.c: In function 'vmxnet3_get_rss_hash_opts':
>> drivers/net/vmxnet3/vmxnet3_ethtool.c:704:6: warning: this statement may
>> fall through [-Wimplicit-fallthrough=]
704 | if (rss_fields & VMXNET3_RSS_FIELDS_ESPIP4)
| ^
drivers/net/vmxnet3/vmxnet3_ethtool.c:706:2: note: here
706 | case SCTP_V4_FLOW:
| ^~~~
vim +704 drivers/net/vmxnet3/vmxnet3_ethtool.c
667
668 static int
669 vmxnet3_get_rss_hash_opts(struct vmxnet3_adapter *adapter,
670struct ethtool_rxnfc *info)
671 {
672 enum Vmxnet3_RSSField rss_fields;
673
674 if (netif_running(adapter->netdev)) {
675 unsigned long flags;
676
677 spin_lock_irqsave(&adapter->cmd_lock, flags);
678
679 VMXNET3_WRITE_BAR1_REG(adapter, VMXNET3_REG_CMD,
680 VMXNET3_CMD_GET_RSS_FIELDS);
681 rss_fields = VMXNET3_READ_BAR1_REG(adapter,
VMXNET3_REG_CMD);
682 spin_unlock_irqrestore(&adapter->cmd_lock, flags);
683 } else {
684 rss_fields = adapter->rss_fields;
685 }
686
687 info->data = 0;
688
689 /* Report default options for RSS on vmxnet3 */
690 switch (info->flow_type) {
691 case TCP_V4_FLOW:
692 if (rss_fields & VMXNET3_RSS_FIELDS_TCPIP4)
693 info->data |= RXH_L4_B_0_1 | RXH_L4_B_2_3 |
694RXH_IP_SRC | RXH_IP_DST;
695 break;
696 case UDP_V4_FLOW:
697 if (rss_fields & VMXNET3_RSS_FIELDS_UDPIP4)
698 info->data |= RXH_L4_B_0_1 | RXH_L4_B_2_3 |
699RXH_IP_SRC | RXH_IP_DST;
700 break;
701 case AH_ESP_V4_FLOW:
702 case AH_V4_FLOW:
703 case ESP_V4_FLOW:
> 704 if (rss_fields & VMXNET3_RSS_FIELDS_ESPIP4)
705 info->data |= RXH_L4_B_0_1 | RXH_L4_B_2_3;
706 case SCTP_V4_FLOW:
707 case IPV4_FLOW:
708 info->data |= RXH_IP_SRC | RXH_IP_DST;
709 break;
710 case TCP_V6_FLOW:
711 if (rss_fields & VMXNET3_RSS_FIELDS_TCPIP6)
712 info->data |= RXH_L4_B_0_1 | RXH_L4_B_2_3 |
713RXH_IP_SRC | RXH_IP_DST;
714 break;
715 case UDP_V6_FLOW:
716 if (rss_fields & VMXNET3_RSS_FIELDS_UDPIP6)
717 info->data |= RXH_L4_B_0_1 | RXH_L4_B_2_3 |
718RXH_IP_SRC | RXH_IP_DST;
719 break;
720 case AH_ESP_V6_FLOW:
721 case AH_V6_FLOW:
722 case ESP_V6_FLOW:
723 case SCTP_V6_FLOW:
724 case IPV6_FLOW:
725 info->data |= RXH_IP_SRC | RXH_IP_DST;
726 break;
727 default:
728 return -EINVAL;
729 }
730
731 return 0;
732 }
733
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
Re: [PATCH net-next 2/4] vmxnet3: add support to get/set rx flow hash
On Wed, May 27, 2020 at 07:07:04PM -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
> ---
[...]
> diff --git a/drivers/net/vmxnet3/vmxnet3_ethtool.c
> b/drivers/net/vmxnet3/vmxnet3_ethtool.c
> index 1163eca7aba5..ceedf63020cb 100644
> --- a/drivers/net/vmxnet3/vmxnet3_ethtool.c
> +++ b/drivers/net/vmxnet3/vmxnet3_ethtool.c
> @@ -665,18 +665,236 @@ vmxnet3_set_ringparam(struct net_device *netdev,
> return err;
> }
>
> +static int
> +vmxnet3_get_rss_hash_opts(struct vmxnet3_adapter *adapter,
> + struct ethtool_rxnfc *info)
> +{
> + enum Vmxnet3_RSSField rss_fields;
> +
> + if (netif_running(adapter->netdev)) {
> + unsigned long flags;
> +
> + spin_lock_irqsave(&adapter->cmd_lock, flags);
> +
> + VMXNET3_WRITE_BAR1_REG(adapter, VMXNET3_REG_CMD,
> +VMXNET3_CMD_GET_RSS_FIELDS);
> + rss_fields = VMXNET3_READ_BAR1_REG(adapter, VMXNET3_REG_CMD);
> + spin_unlock_irqrestore(&adapter->cmd_lock, flags);
> + } else {
> + rss_fields = adapter->rss_fields;
> + }
> +
> + info->data = 0;
> +
> + /* Report default options for RSS on vmxnet3 */
> + switch (info->flow_type) {
> + case TCP_V4_FLOW:
> + if (rss_fields & VMXNET3_RSS_FIELDS_TCPIP4)
> + info->data |= RXH_L4_B_0_1 | RXH_L4_B_2_3 |
> + RXH_IP_SRC | RXH_IP_DST;
> + break;
> + case UDP_V4_FLOW:
> + if (rss_fields & VMXNET3_RSS_FIELDS_UDPIP4)
> + info->data |= RXH_L4_B_0_1 | RXH_L4_B_2_3 |
> + RXH_IP_SRC | RXH_IP_DST;
> + break;
In both cases above (and also in the two for IPv6 below) you set
info->data to either 0 or all four bits, depending on the value of
corresponding flag in rss_fields. But in vmxnet3_set_rss_hash_opt()
you have different mapping:
- for TCP, you only accept all four bits (no other value) and don't
touch rss_fields at all
- for UDP, you allow either all four bits (and set the flag) or the
two IP related bits (and clear the flag)
The UDPv4/UDPv6 behaviour of vmxnet3_set_rss_hash_opt() seems to be the
correct one but you should be consistent between get and set handlers.
> + case AH_ESP_V4_FLOW:
> + case AH_V4_FLOW:
> + case ESP_V4_FLOW:
> + if (rss_fields & VMXNET3_RSS_FIELDS_ESPIP4)
> + info->data |= RXH_L4_B_0_1 | RXH_L4_B_2_3;
If this fallthrough is intentional (it seems to be), it should be
marked.
Michal
> + case SCTP_V4_FLOW:
> + case IPV4_FLOW:
> + info->data |= RXH_IP_SRC | RXH_IP_DST;
> + break;
> + case TCP_V6_FLOW:
> + if (rss_fields & VMXNET3_RSS_FIELDS_TCPIP6)
> + info->data |= RXH_L4_B_0_1 | RXH_L4_B_2_3 |
> + RXH_IP_SRC | RXH_IP_DST;
> + break;
> + case UDP_V6_FLOW:
> + if (rss_fields & VMXNET3_RSS_FIELDS_UDPIP6)
> + info->data |= RXH_L4_B_0_1 | RXH_L4_B_2_3 |
> + RXH_IP_SRC | RXH_IP_DST;
> + break;
> + case AH_ESP_V6_FLOW:
> + case AH_V6_FLOW:
> + case ESP_V6_FLOW:
> + case SCTP_V6_FLOW:
> + case IPV6_FLOW:
> + info->data |= RXH_IP_SRC | RXH_IP_DST;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int
> +vmxnet3_set_rss_hash_opt(struct net_device *netdev,
> + struct vmxnet3_adapter *adapter,
> + struct ethtool_rxnfc *nfc)
> +{
> + enum Vmxnet3_RSSField rss_fields = adapter->rss_fields;
> +
> + /* RSS does not support anything other than hashing
> + * to queues on src and dst IPs and ports
> + */
> + if (nfc->data & ~(RXH_IP_SRC | RXH_IP_DST |
> + RXH_L4_B_0_1 | RXH_L4_B_2_3))
> + return -EINVAL;
> +
> + switch (nfc->flow_type) {
> + case TCP_V4_FLOW:
> + case TCP_V6_FLOW:
> + if (!(nfc->data & RXH_IP_SRC) ||
> + !(nfc->data & RXH_IP_DST) ||
> + !(nfc->data & RXH_L4_B_0_1) ||
> + !(nfc->data & RXH_L4_B_2_3))
> + return -EINVAL;
> + break;
> + case UDP_V4_FLOW:
> + if (!(nfc->data & RXH_IP_SRC) ||
> + !(nfc->data & RXH_IP_DST))
> + return -EINVAL;
> + switch (nfc->data & (RXH_L4_B_0_1 | RXH_L
[PATCH net-next 2/4] vmxnet3: add support to get/set rx flow hash
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
---
drivers/net/vmxnet3/vmxnet3_defs.h| 12 ++
drivers/net/vmxnet3/vmxnet3_drv.c | 39 ++
drivers/net/vmxnet3/vmxnet3_ethtool.c | 223 +-
drivers/net/vmxnet3/vmxnet3_int.h | 4 +
4 files changed, 276 insertions(+), 2 deletions(-)
diff --git a/drivers/net/vmxnet3/vmxnet3_defs.h
b/drivers/net/vmxnet3/vmxnet3_defs.h
index c77274228a3e..aac97fac1186 100644
--- a/drivers/net/vmxnet3/vmxnet3_defs.h
+++ b/drivers/net/vmxnet3/vmxnet3_defs.h
@@ -82,6 +82,7 @@ enum {
VMXNET3_CMD_RESERVED3,
VMXNET3_CMD_SET_COALESCE,
VMXNET3_CMD_REGISTER_MEMREGS,
+ VMXNET3_CMD_SET_RSS_FIELDS,
VMXNET3_CMD_FIRST_GET = 0xF00D,
VMXNET3_CMD_GET_QUEUE_STATUS = VMXNET3_CMD_FIRST_GET,
@@ -96,6 +97,7 @@ enum {
VMXNET3_CMD_GET_RESERVED1,
VMXNET3_CMD_GET_TXDATA_DESC_SIZE,
VMXNET3_CMD_GET_COALESCE,
+ VMXNET3_CMD_GET_RSS_FIELDS,
};
/*
@@ -685,12 +687,22 @@ struct Vmxnet3_MemRegs {
struct Vmxnet3_MemoryRegion memRegs[1];
};
+enum Vmxnet3_RSSField {
+ VMXNET3_RSS_FIELDS_TCPIP4 = 0x0001,
+ VMXNET3_RSS_FIELDS_TCPIP6 = 0x0002,
+ VMXNET3_RSS_FIELDS_UDPIP4 = 0x0004,
+ VMXNET3_RSS_FIELDS_UDPIP6 = 0x0008,
+ VMXNET3_RSS_FIELDS_ESPIP4 = 0x0010,
+ VMXNET3_RSS_FIELDS_ESPIP6 = 0x0020,
+};
+
/* If the command data <= 16 bytes, use the shared memory directly.
* otherwise, use variable length configuration descriptor.
*/
union Vmxnet3_CmdInfo {
struct Vmxnet3_VariableLenConfDesc varConf;
struct Vmxnet3_SetPolling setPolling;
+ enum Vmxnet3_RSSField setRssFields;
__le64 data[2];
};
diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c
b/drivers/net/vmxnet3/vmxnet3_drv.c
index ec2878f8c1f6..4ea7a40ada88 100644
--- a/drivers/net/vmxnet3/vmxnet3_drv.c
+++ b/drivers/net/vmxnet3/vmxnet3_drv.c
@@ -2554,6 +2554,39 @@ vmxnet3_init_coalesce(struct vmxnet3_adapter *adapter)
spin_unlock_irqrestore(&adapter->cmd_lock, flags);
}
+static void
+vmxnet3_init_rssfields(struct vmxnet3_adapter *adapter)
+{
+ struct Vmxnet3_DriverShared *shared = adapter->shared;
+ union Vmxnet3_CmdInfo *cmdInfo = &shared->cu.cmdInfo;
+ unsigned long flags;
+
+ if (!VMXNET3_VERSION_GE_4(adapter))
+ return;
+
+ spin_lock_irqsave(&adapter->cmd_lock, flags);
+
+ if (adapter->default_rss_fields) {
+ VMXNET3_WRITE_BAR1_REG(adapter, VMXNET3_REG_CMD,
+ VMXNET3_CMD_GET_RSS_FIELDS);
+ adapter->rss_fields =
+ VMXNET3_READ_BAR1_REG(adapter, VMXNET3_REG_CMD);
+ } else {
+ cmdInfo->setRssFields = adapter->rss_fields;
+ VMXNET3_WRITE_BAR1_REG(adapter, VMXNET3_REG_CMD,
+ VMXNET3_CMD_SET_RSS_FIELDS);
+ /* Not all requested RSS may get applied, so get and
+* cache what was actually applied.
+*/
+ VMXNET3_WRITE_BAR1_REG(adapter, VMXNET3_REG_CMD,
+ VMXNET3_CMD_GET_RSS_FIELDS);
+ adapter->rss_fields =
+ VMXNET3_READ_BAR1_REG(adapter, VMXNET3_REG_CMD);
+ }
+
+ spin_unlock_irqrestore(&adapter->cmd_lock, flags);
+}
+
int
vmxnet3_activate_dev(struct vmxnet3_adapter *adapter)
{
@@ -2603,6 +2636,7 @@ vmxnet3_activate_dev(struct vmxnet3_adapter *adapter)
}
vmxnet3_init_coalesce(adapter);
+ vmxnet3_init_rssfields(adapter);
for (i = 0; i < adapter->num_rx_queues; i++) {
VMXNET3_WRITE_BAR0_REG(adapter,
@@ -3430,6 +3464,11 @@ vmxnet3_probe_device(struct pci_dev *pdev,
adapter->default_coal_mode = true;
}
+ if (VMXNET3_VERSION_GE_4(adapter)) {
+ adapter->default_rss_fields = true;
+ adapter->rss_fields = VMXNET3_RSS_FIELDS_DEFAULT;
+ }
+
SET_NETDEV_DEV(netdev, &pdev->dev);
vmxnet3_declare_features(adapter, dma64);
diff --git a/drivers/net/vmxnet3/vmxnet3_ethtool.c
b/drivers/net/vmxnet3/vmxnet3_ethtool.c
index 1163eca7aba5..ceedf63020cb 100644
--- a/drivers/net/vmxnet3/vmxnet3_ethtool.c
+++ b/drivers/net/vmxnet3/vmxnet3_ethtool.c
@@ -665,18 +665,236 @@ vmxnet3_set_ringparam(struct net_device *netdev,
return err;
}
+static int
+vmxnet3_get_rss_hash_opts(struct vmxnet3_adapter *adapter,
+ struc
[PATCH net-next 2/4] vmxnet3: add support to get/set rx flow hash
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
---
drivers/net/vmxnet3/vmxnet3_defs.h| 12 ++
drivers/net/vmxnet3/vmxnet3_drv.c | 39 ++
drivers/net/vmxnet3/vmxnet3_ethtool.c | 223 +-
drivers/net/vmxnet3/vmxnet3_int.h | 4 +
4 files changed, 276 insertions(+), 2 deletions(-)
diff --git a/drivers/net/vmxnet3/vmxnet3_defs.h
b/drivers/net/vmxnet3/vmxnet3_defs.h
index c77274228a3e..aac97fac1186 100644
--- a/drivers/net/vmxnet3/vmxnet3_defs.h
+++ b/drivers/net/vmxnet3/vmxnet3_defs.h
@@ -82,6 +82,7 @@ enum {
VMXNET3_CMD_RESERVED3,
VMXNET3_CMD_SET_COALESCE,
VMXNET3_CMD_REGISTER_MEMREGS,
+ VMXNET3_CMD_SET_RSS_FIELDS,
VMXNET3_CMD_FIRST_GET = 0xF00D,
VMXNET3_CMD_GET_QUEUE_STATUS = VMXNET3_CMD_FIRST_GET,
@@ -96,6 +97,7 @@ enum {
VMXNET3_CMD_GET_RESERVED1,
VMXNET3_CMD_GET_TXDATA_DESC_SIZE,
VMXNET3_CMD_GET_COALESCE,
+ VMXNET3_CMD_GET_RSS_FIELDS,
};
/*
@@ -685,12 +687,22 @@ struct Vmxnet3_MemRegs {
struct Vmxnet3_MemoryRegion memRegs[1];
};
+enum Vmxnet3_RSSField {
+ VMXNET3_RSS_FIELDS_TCPIP4 = 0x0001,
+ VMXNET3_RSS_FIELDS_TCPIP6 = 0x0002,
+ VMXNET3_RSS_FIELDS_UDPIP4 = 0x0004,
+ VMXNET3_RSS_FIELDS_UDPIP6 = 0x0008,
+ VMXNET3_RSS_FIELDS_ESPIP4 = 0x0010,
+ VMXNET3_RSS_FIELDS_ESPIP6 = 0x0020,
+};
+
/* If the command data <= 16 bytes, use the shared memory directly.
* otherwise, use variable length configuration descriptor.
*/
union Vmxnet3_CmdInfo {
struct Vmxnet3_VariableLenConfDesc varConf;
struct Vmxnet3_SetPolling setPolling;
+ enum Vmxnet3_RSSField setRssFields;
__le64 data[2];
};
diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c
b/drivers/net/vmxnet3/vmxnet3_drv.c
index ec2878f8c1f6..4ea7a40ada88 100644
--- a/drivers/net/vmxnet3/vmxnet3_drv.c
+++ b/drivers/net/vmxnet3/vmxnet3_drv.c
@@ -2554,6 +2554,39 @@ vmxnet3_init_coalesce(struct vmxnet3_adapter *adapter)
spin_unlock_irqrestore(&adapter->cmd_lock, flags);
}
+static void
+vmxnet3_init_rssfields(struct vmxnet3_adapter *adapter)
+{
+ struct Vmxnet3_DriverShared *shared = adapter->shared;
+ union Vmxnet3_CmdInfo *cmdInfo = &shared->cu.cmdInfo;
+ unsigned long flags;
+
+ if (!VMXNET3_VERSION_GE_4(adapter))
+ return;
+
+ spin_lock_irqsave(&adapter->cmd_lock, flags);
+
+ if (adapter->default_rss_fields) {
+ VMXNET3_WRITE_BAR1_REG(adapter, VMXNET3_REG_CMD,
+ VMXNET3_CMD_GET_RSS_FIELDS);
+ adapter->rss_fields =
+ VMXNET3_READ_BAR1_REG(adapter, VMXNET3_REG_CMD);
+ } else {
+ cmdInfo->setRssFields = adapter->rss_fields;
+ VMXNET3_WRITE_BAR1_REG(adapter, VMXNET3_REG_CMD,
+ VMXNET3_CMD_SET_RSS_FIELDS);
+ /* Not all requested RSS may get applied, so get and
+* cache what was actually applied.
+*/
+ VMXNET3_WRITE_BAR1_REG(adapter, VMXNET3_REG_CMD,
+ VMXNET3_CMD_GET_RSS_FIELDS);
+ adapter->rss_fields =
+ VMXNET3_READ_BAR1_REG(adapter, VMXNET3_REG_CMD);
+ }
+
+ spin_unlock_irqrestore(&adapter->cmd_lock, flags);
+}
+
int
vmxnet3_activate_dev(struct vmxnet3_adapter *adapter)
{
@@ -2603,6 +2636,7 @@ vmxnet3_activate_dev(struct vmxnet3_adapter *adapter)
}
vmxnet3_init_coalesce(adapter);
+ vmxnet3_init_rssfields(adapter);
for (i = 0; i < adapter->num_rx_queues; i++) {
VMXNET3_WRITE_BAR0_REG(adapter,
@@ -3430,6 +3464,11 @@ vmxnet3_probe_device(struct pci_dev *pdev,
adapter->default_coal_mode = true;
}
+ if (VMXNET3_VERSION_GE_4(adapter)) {
+ adapter->default_rss_fields = true;
+ adapter->rss_fields = VMXNET3_RSS_FIELDS_DEFAULT;
+ }
+
SET_NETDEV_DEV(netdev, &pdev->dev);
vmxnet3_declare_features(adapter, dma64);
diff --git a/drivers/net/vmxnet3/vmxnet3_ethtool.c
b/drivers/net/vmxnet3/vmxnet3_ethtool.c
index 1163eca7aba5..ceedf63020cb 100644
--- a/drivers/net/vmxnet3/vmxnet3_ethtool.c
+++ b/drivers/net/vmxnet3/vmxnet3_ethtool.c
@@ -665,18 +665,236 @@ vmxnet3_set_ringparam(struct net_device *netdev,
return err;
}
+static int
+vmxnet3_get_rss_hash_opts(struct vmxnet3_adapter *adapter,
+ struc
