diff mbox series

[net-next,v4,02/12] eth: mvpp2: implement new RSS context API

Message ID 20240809031827.2373341-3-kuba@kernel.org (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series ethtool: rss: driver tweaks and netlink context dumps | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; GEN HAS DIFF 2 files changed, 119 insertions(+);
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit fail Errors and warnings before: 29 this patch: 18
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 6 of 6 maintainers
netdev/build_clang fail Errors and warnings before: 29 this patch: 18
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn fail Errors and warnings before: 35 this patch: 18
netdev/checkpatch warning WARNING: line length of 85 exceeds 80 columns
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Jakub Kicinski Aug. 9, 2024, 3:18 a.m. UTC 
Implement the separate create/modify/delete ops for RSS.

No problems with IDs - even tho RSS tables are per device
the driver already seems to allocate IDs linearly per port.
There's a translation table from per-port context ID
to device context ID.

mvpp2 doesn't have a key for the hash, it defaults to
an empty/previous indir table.

Note that there is no key at all, so we don't have to be
concerned with reporting the wrong one (which is addressed
by a patch later in the series).

Compile-tested only.

Reviewed-by: Edward Cree <ecree.xilinx@gmail.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
v4:
 - adjust to meaning of max_context_id from net
v3:
 - fix max_context_id to be inclusive
 - extend commit message with why this is different than patch 6
v2:
 - copy the default indir table
 - use ID from the core
 - fix build issues

CC: marcin.s.wojtas@gmail.com
CC: linux@armlinux.org.uk
---
 .../net/ethernet/marvell/mvpp2/mvpp2_cls.c    |  18 +---
 .../net/ethernet/marvell/mvpp2/mvpp2_cls.h    |   2 +-
 .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 102 +++++++++++++-----
 3 files changed, 79 insertions(+), 43 deletions(-)

Comments

Simon Horman Aug. 9, 2024, 2:56 p.m. UTC | #1 
On Thu, Aug 08, 2024 at 08:18:17PM -0700, Jakub Kicinski wrote:
> Implement the separate create/modify/delete ops for RSS.
> 
> No problems with IDs - even tho RSS tables are per device
> the driver already seems to allocate IDs linearly per port.
> There's a translation table from per-port context ID
> to device context ID.
> 
> mvpp2 doesn't have a key for the hash, it defaults to
> an empty/previous indir table.
> 
> Note that there is no key at all, so we don't have to be
> concerned with reporting the wrong one (which is addressed
> by a patch later in the series).
> 
> Compile-tested only.
> 
> Reviewed-by: Edward Cree <ecree.xilinx@gmail.com>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> v4:
>  - adjust to meaning of max_context_id from net

Hi Jakub,

Should rxfh_max_num_contexts be used instead?

I'm seeing:

  CC [M]  .../mvpp2_main.o
.../mvpp2_main.c:5794:10: error: ‘const struct ethtool_ops’ has no member named ‘rxfh_max_context_id’; did you mean ‘rxfh_max_num_contexts’?
 5794 |         .rxfh_max_context_id    = MVPP22_N_RSS_TABLES,
      |          ^~~~~~~~~~~~~~~~~~~
      |          rxfh_max_num_contexts


Which seems to make sense in the context of:

- net: ethtool: fix off-by-one error in max RSS context IDs
  https://git.kernel.org/netdev/net-next/c/b54de55990b0

...
kernel test robot Aug. 9, 2024, 6:05 p.m. UTC | #2 
Hi Jakub,

kernel test robot noticed the following build errors:

[auto build test ERROR on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Jakub-Kicinski/selftests-drv-net-rss_ctx-add-identifier-to-traffic-comments/20240809-143446
base:   net-next/main
patch link:    https://lore.kernel.org/r/20240809031827.2373341-3-kuba%40kernel.org
patch subject: [PATCH net-next v4 02/12] eth: mvpp2: implement new RSS context API
config: m68k-allmodconfig (https://download.01.org/0day-ci/archive/20240810/202408100156.UZVR13Wy-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 14.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240810/202408100156.UZVR13Wy-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408100156.UZVR13Wy-lkp@intel.com/

All error/warnings (new ones prefixed by >>):

>> drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c:5795:10: error: 'const struct ethtool_ops' has no member named 'rxfh_max_context_id'; did you mean 'rxfh_max_num_contexts'?
    5795 |         .rxfh_max_context_id    = MVPP22_N_RSS_TABLES,
         |          ^~~~~~~~~~~~~~~~~~~
         |          rxfh_max_num_contexts
   In file included from drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c:42:
>> drivers/net/ethernet/marvell/mvpp2/mvpp2.h:859:41: warning: unsigned conversion from 'int' to 'unsigned char:1' changes value from '8' to '0' [-Woverflow]
     859 | #define MVPP22_N_RSS_TABLES             8
         |                                         ^
   drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c:5795:35: note: in expansion of macro 'MVPP22_N_RSS_TABLES'
    5795 |         .rxfh_max_context_id    = MVPP22_N_RSS_TABLES,
         |                                   ^~~~~~~~~~~~~~~~~~~


vim +5795 drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c

  5792	
  5793	static const struct ethtool_ops mvpp2_eth_tool_ops = {
  5794		.cap_rss_ctx_supported	= true,
> 5795		.rxfh_max_context_id	= MVPP22_N_RSS_TABLES,
  5796		.supported_coalesce_params = ETHTOOL_COALESCE_USECS |
  5797					     ETHTOOL_COALESCE_MAX_FRAMES,
  5798		.nway_reset		= mvpp2_ethtool_nway_reset,
  5799		.get_link		= ethtool_op_get_link,
  5800		.get_ts_info		= mvpp2_ethtool_get_ts_info,
  5801		.set_coalesce		= mvpp2_ethtool_set_coalesce,
  5802		.get_coalesce		= mvpp2_ethtool_get_coalesce,
  5803		.get_drvinfo		= mvpp2_ethtool_get_drvinfo,
  5804		.get_ringparam		= mvpp2_ethtool_get_ringparam,
  5805		.set_ringparam		= mvpp2_ethtool_set_ringparam,
  5806		.get_strings		= mvpp2_ethtool_get_strings,
  5807		.get_ethtool_stats	= mvpp2_ethtool_get_stats,
  5808		.get_sset_count		= mvpp2_ethtool_get_sset_count,
  5809		.get_pauseparam		= mvpp2_ethtool_get_pause_param,
  5810		.set_pauseparam		= mvpp2_ethtool_set_pause_param,
  5811		.get_link_ksettings	= mvpp2_ethtool_get_link_ksettings,
  5812		.set_link_ksettings	= mvpp2_ethtool_set_link_ksettings,
  5813		.get_rxnfc		= mvpp2_ethtool_get_rxnfc,
  5814		.set_rxnfc		= mvpp2_ethtool_set_rxnfc,
  5815		.get_rxfh_indir_size	= mvpp2_ethtool_get_rxfh_indir_size,
  5816		.get_rxfh		= mvpp2_ethtool_get_rxfh,
  5817		.set_rxfh		= mvpp2_ethtool_set_rxfh,
  5818		.create_rxfh_context	= mvpp2_create_rxfh_context,
  5819		.modify_rxfh_context	= mvpp2_modify_rxfh_context,
  5820		.remove_rxfh_context	= mvpp2_remove_rxfh_context,
  5821	};
  5822
kernel test robot Aug. 9, 2024, 7:47 p.m. UTC | #3 
Hi Jakub,

kernel test robot noticed the following build errors:

[auto build test ERROR on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Jakub-Kicinski/selftests-drv-net-rss_ctx-add-identifier-to-traffic-comments/20240809-143446
base:   net-next/main
patch link:    https://lore.kernel.org/r/20240809031827.2373341-3-kuba%40kernel.org
patch subject: [PATCH net-next v4 02/12] eth: mvpp2: implement new RSS context API
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20240810/202408100303.d5TA9nQp-lkp@intel.com/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240810/202408100303.d5TA9nQp-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408100303.d5TA9nQp-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c:5795:3: error: field designator 'rxfh_max_context_id' does not refer to any field in type 'const struct ethtool_ops'
    5795 |         .rxfh_max_context_id    = MVPP22_N_RSS_TABLES,
         |         ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   1 error generated.


vim +5795 drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c

  5792	
  5793	static const struct ethtool_ops mvpp2_eth_tool_ops = {
  5794		.cap_rss_ctx_supported	= true,
> 5795		.rxfh_max_context_id	= MVPP22_N_RSS_TABLES,
  5796		.supported_coalesce_params = ETHTOOL_COALESCE_USECS |
  5797					     ETHTOOL_COALESCE_MAX_FRAMES,
  5798		.nway_reset		= mvpp2_ethtool_nway_reset,
  5799		.get_link		= ethtool_op_get_link,
  5800		.get_ts_info		= mvpp2_ethtool_get_ts_info,
  5801		.set_coalesce		= mvpp2_ethtool_set_coalesce,
  5802		.get_coalesce		= mvpp2_ethtool_get_coalesce,
  5803		.get_drvinfo		= mvpp2_ethtool_get_drvinfo,
  5804		.get_ringparam		= mvpp2_ethtool_get_ringparam,
  5805		.set_ringparam		= mvpp2_ethtool_set_ringparam,
  5806		.get_strings		= mvpp2_ethtool_get_strings,
  5807		.get_ethtool_stats	= mvpp2_ethtool_get_stats,
  5808		.get_sset_count		= mvpp2_ethtool_get_sset_count,
  5809		.get_pauseparam		= mvpp2_ethtool_get_pause_param,
  5810		.set_pauseparam		= mvpp2_ethtool_set_pause_param,
  5811		.get_link_ksettings	= mvpp2_ethtool_get_link_ksettings,
  5812		.set_link_ksettings	= mvpp2_ethtool_set_link_ksettings,
  5813		.get_rxnfc		= mvpp2_ethtool_get_rxnfc,
  5814		.set_rxnfc		= mvpp2_ethtool_set_rxnfc,
  5815		.get_rxfh_indir_size	= mvpp2_ethtool_get_rxfh_indir_size,
  5816		.get_rxfh		= mvpp2_ethtool_get_rxfh,
  5817		.set_rxfh		= mvpp2_ethtool_set_rxfh,
  5818		.create_rxfh_context	= mvpp2_create_rxfh_context,
  5819		.modify_rxfh_context	= mvpp2_modify_rxfh_context,
  5820		.remove_rxfh_context	= mvpp2_remove_rxfh_context,
  5821	};
  5822
diff mbox series

Patch

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_cls.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_cls.c
index 40aeaa7bd739..1641791a2d5b 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_cls.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_cls.c
@@ -1522,29 +1522,19 @@  static int mvpp22_rss_context_create(struct mvpp2_port *port, u32 *rss_ctx)
 	return 0;
 }
 
-int mvpp22_port_rss_ctx_create(struct mvpp2_port *port, u32 *port_ctx)
+int mvpp22_port_rss_ctx_create(struct mvpp2_port *port, u32 port_ctx)
 {
 	u32 rss_ctx;
-	int ret, i;
+	int ret;
 
 	ret = mvpp22_rss_context_create(port, &rss_ctx);
 	if (ret)
 		return ret;
 
-	/* Find the first available context number in the port, starting from 1.
-	 * Context 0 on each port is reserved for the default context.
-	 */
-	for (i = 1; i < MVPP22_N_RSS_TABLES; i++) {
-		if (port->rss_ctx[i] < 0)
-			break;
-	}
-
-	if (i == MVPP22_N_RSS_TABLES)
+	if (WARN_ON_ONCE(port->rss_ctx[port_ctx] >= 0))
 		return -EINVAL;
 
-	port->rss_ctx[i] = rss_ctx;
-	*port_ctx = i;
-
+	port->rss_ctx[port_ctx] = rss_ctx;
 	return 0;
 }
 
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_cls.h b/drivers/net/ethernet/marvell/mvpp2/mvpp2_cls.h
index 663157dc8062..85c9c6e80678 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_cls.h
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_cls.h
@@ -264,7 +264,7 @@  int mvpp22_port_rss_init(struct mvpp2_port *port);
 int mvpp22_port_rss_enable(struct mvpp2_port *port);
 int mvpp22_port_rss_disable(struct mvpp2_port *port);
 
-int mvpp22_port_rss_ctx_create(struct mvpp2_port *port, u32 *rss_ctx);
+int mvpp22_port_rss_ctx_create(struct mvpp2_port *port, u32 rss_ctx);
 int mvpp22_port_rss_ctx_delete(struct mvpp2_port *port, u32 rss_ctx);
 
 int mvpp22_port_rss_ctx_indir_set(struct mvpp2_port *port, u32 rss_ctx,
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index 0d62a33afa80..90182f6fd9a1 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -5696,38 +5696,80 @@  static int mvpp2_ethtool_get_rxfh(struct net_device *dev,
 	return ret;
 }
 
+static bool mvpp2_ethtool_rxfh_okay(struct mvpp2_port *port,
+				    const struct ethtool_rxfh_param *rxfh)
+{
+	if (!mvpp22_rss_is_supported(port))
+		return false;
+
+	if (rxfh->hfunc != ETH_RSS_HASH_NO_CHANGE &&
+	    rxfh->hfunc != ETH_RSS_HASH_CRC32)
+		return false;
+
+	if (rxfh->key)
+		return false;
+
+	return true;
+}
+
+static int mvpp2_create_rxfh_context(struct net_device *dev,
+				     struct ethtool_rxfh_context *ctx,
+				     const struct ethtool_rxfh_param *rxfh,
+				     struct netlink_ext_ack *extack)
+{
+	struct mvpp2_port *port = netdev_priv(dev);
+	int ret = 0;
+
+	if (!mvpp2_ethtool_rxfh_okay(port, rxfh))
+		return -EOPNOTSUPP;
+
+	ctx->hfunc = ETH_RSS_HASH_CRC32;
+
+	ret = mvpp22_port_rss_ctx_create(port, rxfh->rss_context);
+	if (ret)
+		return ret;
+
+	if (!rxfh->indir)
+		ret = mvpp22_port_rss_ctx_indir_get(port, rxfh->rss_context,
+						    ethtool_rxfh_context_indir(ctx));
+	else
+		ret = mvpp22_port_rss_ctx_indir_set(port, rxfh->rss_context,
+						    rxfh->indir);
+	return ret;
+}
+
+static int mvpp2_modify_rxfh_context(struct net_device *dev,
+				     struct ethtool_rxfh_context *ctx,
+				     const struct ethtool_rxfh_param *rxfh,
+				     struct netlink_ext_ack *extack)
+{
+	struct mvpp2_port *port = netdev_priv(dev);
+	int ret = 0;
+
+	if (!mvpp2_ethtool_rxfh_okay(port, rxfh))
+		return -EOPNOTSUPP;
+
+	if (rxfh->indir)
+		ret = mvpp22_port_rss_ctx_indir_set(port, rxfh->rss_context,
+						    rxfh->indir);
+	return ret;
+}
+
+static int mvpp2_remove_rxfh_context(struct net_device *dev,
+				     struct ethtool_rxfh_context *ctx,
+				     u32 rss_context,
+				     struct netlink_ext_ack *extack)
+{
+	struct mvpp2_port *port = netdev_priv(dev);
+
+	return mvpp22_port_rss_ctx_delete(port, rss_context);
+}
+
 static int mvpp2_ethtool_set_rxfh(struct net_device *dev,
 				  struct ethtool_rxfh_param *rxfh,
 				  struct netlink_ext_ack *extack)
 {
-	struct mvpp2_port *port = netdev_priv(dev);
-	u32 *rss_context = &rxfh->rss_context;
-	int ret = 0;
-
-	if (!mvpp22_rss_is_supported(port))
-		return -EOPNOTSUPP;
-
-	if (rxfh->hfunc != ETH_RSS_HASH_NO_CHANGE &&
-	    rxfh->hfunc != ETH_RSS_HASH_CRC32)
-		return -EOPNOTSUPP;
-
-	if (rxfh->key)
-		return -EOPNOTSUPP;
-
-	if (*rss_context && rxfh->rss_delete)
-		return mvpp22_port_rss_ctx_delete(port, *rss_context);
-
-	if (*rss_context == ETH_RXFH_CONTEXT_ALLOC) {
-		ret = mvpp22_port_rss_ctx_create(port, rss_context);
-		if (ret)
-			return ret;
-	}
-
-	if (rxfh->indir)
-		ret = mvpp22_port_rss_ctx_indir_set(port, *rss_context,
-						    rxfh->indir);
-
-	return ret;
+	return mvpp2_modify_rxfh_context(dev, NULL, rxfh, extack);
 }
 
 /* Device ops */
@@ -5750,6 +5792,7 @@  static const struct net_device_ops mvpp2_netdev_ops = {
 
 static const struct ethtool_ops mvpp2_eth_tool_ops = {
 	.cap_rss_ctx_supported	= true,
+	.rxfh_max_context_id	= MVPP22_N_RSS_TABLES,
 	.supported_coalesce_params = ETHTOOL_COALESCE_USECS |
 				     ETHTOOL_COALESCE_MAX_FRAMES,
 	.nway_reset		= mvpp2_ethtool_nway_reset,
@@ -5772,6 +5815,9 @@  static const struct ethtool_ops mvpp2_eth_tool_ops = {
 	.get_rxfh_indir_size	= mvpp2_ethtool_get_rxfh_indir_size,
 	.get_rxfh		= mvpp2_ethtool_get_rxfh,
 	.set_rxfh		= mvpp2_ethtool_set_rxfh,
+	.create_rxfh_context	= mvpp2_create_rxfh_context,
+	.modify_rxfh_context	= mvpp2_modify_rxfh_context,
+	.remove_rxfh_context	= mvpp2_remove_rxfh_context,
 };
 
 /* Used for PPv2.1, or PPv2.2 with the old Device Tree binding that