[PATCH RFC v2 iproute2-next] devlink: Add eswitch attr option for shared descriptors

[PATCH RFC v2 iproute2-next] devlink: Add eswitch attr option for shared descriptors

[William Tu]

Add two eswitch attrs: shrdesc_mode and shrdesc_count.
shrdesc_mode: to enable a sharing memory buffer for
representor's rx buffer, and shrdesc_count: to control the
number of buffers in this shared memory pool.

An example use case:
  $ devlink dev eswitch show pci/0000:08:00.0
    pci/0000:08:00.0: mode legacy inline-mode none encap-mode basic \
    shrdesc-mode none shrdesc-count 0
  $ devlink dev eswitch set pci/0000:08:00.0 mode switchdev \
    shrdesc-mode basic shrdesc-count 1024
  $ devlink dev eswitch show pci/0000:08:00.0
    pci/0000:08:00.0: mode switchdev inline-mode none encap-mode basic \
    shrdesc-mode basic shrdesc-count 1024

Signed-off-by: William Tu <witu@nvidia.com>
---
Discussions:
https://lore.kernel.org/netdev/20240201193050.3b19111b@kernel.org/

Corresponding kernel code:
https://lore.kernel.org/netdev/20240228015954.11981-1-witu@nvidia.com/

v2: feedback from Stephen
- add man page, send to iproute2-next
---
 devlink/devlink.c            | 83 +++++++++++++++++++++++++++++++++++-
 include/uapi/linux/devlink.h |  7 +++
 man/man8/devlink-dev.8       | 18 ++++++++
 3 files changed, 106 insertions(+), 2 deletions(-)

diff --git a/devlink/devlink.c b/devlink/devlink.c
index dbeb6e397e8e..affc29eb7cad 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -53,6 +53,9 @@
 #define ESWITCH_ENCAP_MODE_NONE "none"
 #define ESWITCH_ENCAP_MODE_BASIC "basic"
 
+#define ESWITCH_SHRDESC_MODE_NONE "none"
+#define ESWITCH_SHRDESC_MODE_BASIC "basic"
+
 #define PARAM_CMODE_RUNTIME_STR "runtime"
 #define PARAM_CMODE_DRIVERINIT_STR "driverinit"
 #define PARAM_CMODE_PERMANENT_STR "permanent"
@@ -309,6 +312,8 @@ static int ifname_map_update(struct ifname_map *ifname_map, const char *ifname)
 #define DL_OPT_PORT_FN_RATE_TX_PRIORITY	BIT(55)
 #define DL_OPT_PORT_FN_RATE_TX_WEIGHT	BIT(56)
 #define DL_OPT_PORT_FN_CAPS	BIT(57)
+#define DL_OPT_ESWITCH_SHRDESC_MODE	BIT(58)
+#define DL_OPT_ESWITCH_SHRDESC_COUNT	BIT(59)
 
 struct dl_opts {
 	uint64_t present; /* flags of present items */
@@ -375,6 +380,8 @@ struct dl_opts {
 	const char *linecard_type;
 	bool selftests_opt[DEVLINK_ATTR_SELFTEST_ID_MAX + 1];
 	struct nla_bitfield32 port_fn_caps;
+	enum devlink_eswitch_shrdesc_mode eswitch_shrdesc_mode;
+	uint32_t eswitch_shrdesc_count;
 };
 
 struct dl {
@@ -630,6 +637,8 @@ static const enum mnl_attr_data_type devlink_policy[DEVLINK_ATTR_MAX + 1] = {
 	[DEVLINK_ATTR_ESWITCH_MODE] = MNL_TYPE_U16,
 	[DEVLINK_ATTR_ESWITCH_INLINE_MODE] = MNL_TYPE_U8,
 	[DEVLINK_ATTR_ESWITCH_ENCAP_MODE] = MNL_TYPE_U8,
+	[DEVLINK_ATTR_ESWITCH_SHRDESC_MODE] = MNL_TYPE_U8,
+	[DEVLINK_ATTR_ESWITCH_SHRDESC_COUNT] = MNL_TYPE_U32,
 	[DEVLINK_ATTR_DPIPE_TABLES] = MNL_TYPE_NESTED,
 	[DEVLINK_ATTR_DPIPE_TABLE] = MNL_TYPE_NESTED,
 	[DEVLINK_ATTR_DPIPE_TABLE_NAME] = MNL_TYPE_STRING,
@@ -1464,6 +1473,21 @@ eswitch_encap_mode_get(const char *typestr,
 	return 0;
 }
 
+static int
+eswitch_shrdesc_mode_get(const char *typestr,
+			 enum devlink_eswitch_shrdesc_mode *p_shrdesc_mode)
+{
+	if (strcmp(typestr, ESWITCH_SHRDESC_MODE_NONE) == 0) {
+		*p_shrdesc_mode = DEVLINK_ESWITCH_SHRDESC_MODE_NONE;
+	} else if (strcmp(typestr, ESWITCH_SHRDESC_MODE_BASIC) == 0) {
+		*p_shrdesc_mode = DEVLINK_ESWITCH_SHRDESC_MODE_BASIC;
+	} else {
+		pr_err("Unknown eswitch shrdesc mode \"%s\"\n", typestr);
+		return -EINVAL;
+	}
+	return 0;
+}
+
 static int flash_overwrite_section_get(const char *sectionstr, uint32_t *mask)
 {
 	if (strcmp(sectionstr, "settings") == 0) {
@@ -1672,6 +1696,8 @@ static const struct dl_args_metadata dl_args_required[] = {
 	{DL_OPT_LINECARD,	      "Linecard index expected."},
 	{DL_OPT_LINECARD_TYPE,	      "Linecard type expected."},
 	{DL_OPT_SELFTESTS,            "Test name is expected"},
+	{DL_OPT_ESWITCH_SHRDESC_MODE, "E-Switch shared descriptors option expected."},
+	{DL_OPT_ESWITCH_SHRDESC_COUNT,"E-Switch shared descriptors count expected."},
 };
 
 static int dl_args_finding_required_validate(uint64_t o_required,
@@ -1895,6 +1921,26 @@ static int dl_argv_parse(struct dl *dl, uint64_t o_required,
 			if (err)
 				return err;
 			o_found |= DL_OPT_ESWITCH_ENCAP_MODE;
+		} else if ((dl_argv_match(dl, "shrdesc-mode")) &&
+			   (o_all & DL_OPT_ESWITCH_SHRDESC_MODE)) {
+			const char *typestr;
+
+			dl_arg_inc(dl);
+			err = dl_argv_str(dl, &typestr);
+			if (err)
+				return err;
+			err = eswitch_shrdesc_mode_get(typestr,
+						       &opts->eswitch_shrdesc_mode);
+			if (err)
+				return err;
+			o_found |= DL_OPT_ESWITCH_SHRDESC_MODE;
+		} else if (dl_argv_match(dl, "shrdesc-count") &&
+			   (o_all & DL_OPT_ESWITCH_SHRDESC_COUNT)) {
+			dl_arg_inc(dl);
+			err = dl_argv_uint32_t(dl, &opts->eswitch_shrdesc_count);
+			if (err)
+				return err;
+			o_found |= DL_OPT_ESWITCH_SHRDESC_COUNT;
 		} else if (dl_argv_match(dl, "path") &&
 			   (o_all & DL_OPT_RESOURCE_PATH)) {
 			dl_arg_inc(dl);
@@ -2547,6 +2593,12 @@ static void dl_opts_put(struct nlmsghdr *nlh, struct dl *dl)
 	if (opts->present & DL_OPT_ESWITCH_ENCAP_MODE)
 		mnl_attr_put_u8(nlh, DEVLINK_ATTR_ESWITCH_ENCAP_MODE,
 				opts->eswitch_encap_mode);
+	if (opts->present & DL_OPT_ESWITCH_SHRDESC_MODE)
+		mnl_attr_put_u8(nlh, DEVLINK_ATTR_ESWITCH_SHRDESC_MODE,
+				opts->eswitch_shrdesc_mode);
+	if (opts->present & DL_OPT_ESWITCH_SHRDESC_COUNT)
+		mnl_attr_put_u32(nlh, DEVLINK_ATTR_ESWITCH_SHRDESC_COUNT,
+				 opts->eswitch_shrdesc_count);
 	if ((opts->present & DL_OPT_RESOURCE_PATH) && opts->resource_id_valid)
 		mnl_attr_put_u64(nlh, DEVLINK_ATTR_RESOURCE_ID,
 				 opts->resource_id);
@@ -2707,6 +2759,8 @@ static void cmd_dev_help(void)
 	pr_err("       devlink dev eswitch set DEV [ mode { legacy | switchdev } ]\n");
 	pr_err("                               [ inline-mode { none | link | network | transport } ]\n");
 	pr_err("                               [ encap-mode { none | basic } ]\n");
+	pr_err("                               [ shrdesc-mode { none | basic } ]\n");
+	pr_err("                               [ shrdesc-count { VALUE } ]\n");
 	pr_err("       devlink dev eswitch show DEV\n");
 	pr_err("       devlink dev param set DEV name PARAMETER value VALUE cmode { permanent | driverinit | runtime }\n");
 	pr_err("       devlink dev param show [DEV name PARAMETER]\n");
@@ -3172,6 +3226,18 @@ static const char *eswitch_encap_mode_name(uint32_t mode)
 	}
 }
 
+static const char *eswitch_shrdesc_mode_name(uint8_t mode)
+{
+	switch (mode) {
+	case DEVLINK_ESWITCH_SHRDESC_MODE_NONE:
+		return ESWITCH_SHRDESC_MODE_NONE;
+	case DEVLINK_ESWITCH_SHRDESC_MODE_BASIC:
+		return ESWITCH_SHRDESC_MODE_BASIC;
+	default:
+		return "<unknown mode>";
+	}
+}
+
 static void pr_out_eswitch(struct dl *dl, struct nlattr **tb)
 {
 	__pr_out_handle_start(dl, tb, true, false);
@@ -3194,7 +3260,18 @@ static void pr_out_eswitch(struct dl *dl, struct nlattr **tb)
 			     eswitch_encap_mode_name(mnl_attr_get_u8(
 				    tb[DEVLINK_ATTR_ESWITCH_ENCAP_MODE])));
 	}
-
+	if (tb[DEVLINK_ATTR_ESWITCH_SHRDESC_MODE]) {
+		check_indent_newline(dl);
+		print_string(PRINT_ANY, "shrdesc-mode", "shrdesc-mode %s",
+			     eswitch_shrdesc_mode_name(mnl_attr_get_u8(
+				    tb[DEVLINK_ATTR_ESWITCH_SHRDESC_MODE])));
+	}
+	if (tb[DEVLINK_ATTR_ESWITCH_SHRDESC_COUNT]) {
+		check_indent_newline(dl);
+		print_uint(PRINT_ANY, "shrdesc-count", "shrdesc-count %u",
+			   mnl_attr_get_u32(
+				    tb[DEVLINK_ATTR_ESWITCH_SHRDESC_COUNT]));
+	}
 	pr_out_handle_end(dl);
 }
 
@@ -3239,7 +3316,9 @@ static int cmd_dev_eswitch_set(struct dl *dl)
 	err = dl_argv_parse(dl, DL_OPT_HANDLE,
 			    DL_OPT_ESWITCH_MODE |
 			    DL_OPT_ESWITCH_INLINE_MODE |
-			    DL_OPT_ESWITCH_ENCAP_MODE);
+			    DL_OPT_ESWITCH_ENCAP_MODE |
+			    DL_OPT_ESWITCH_SHRDESC_MODE |
+			    DL_OPT_ESWITCH_SHRDESC_COUNT);
 	if (err)
 		return err;
 
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index e77170199815..494a4b61f917 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -195,6 +195,11 @@ enum devlink_eswitch_encap_mode {
 	DEVLINK_ESWITCH_ENCAP_MODE_BASIC,
 };
 
+enum devlink_eswitch_shrdesc_mode {
+	DEVLINK_ESWITCH_SHRDESC_MODE_NONE,
+	DEVLINK_ESWITCH_SHRDESC_MODE_BASIC,
+};
+
 enum devlink_port_flavour {
 	DEVLINK_PORT_FLAVOUR_PHYSICAL, /* Any kind of a port physically
 					* facing the user.
@@ -614,6 +619,8 @@ enum devlink_attr {
 
 	DEVLINK_ATTR_REGION_DIRECT,		/* flag */
 
+	DEVLINK_ATTR_ESWITCH_SHRDESC_MODE,      /* u8 */
+	DEVLINK_ATTR_ESWITCH_SHRDESC_COUNT,     /* u32 */
 	/* add new attributes above here, update the policy in devlink.c */
 
 	__DEVLINK_ATTR_MAX,
diff --git a/man/man8/devlink-dev.8 b/man/man8/devlink-dev.8
index e9d091df48d8..4df055778868 100644
--- a/man/man8/devlink-dev.8
+++ b/man/man8/devlink-dev.8
@@ -34,6 +34,10 @@ devlink-dev \- devlink device configuration
 .BR inline-mode " { " none " | " link " | " network " | " transport " } "
 ] [
 .BR encap-mode " { " none " | " basic " } "
+] [
+.BR shrdesc-mode " { " none " | " basic " } "
+] [
+.BR shrdesc-count " { COUNT } "
 ]
 
 .ti -8
@@ -151,6 +155,20 @@ Set eswitch encapsulation support
 .I basic
 - Enable encapsulation support
 
+.TP
+.BR shrdesc-mode " { " none " | " basic " } "
+Set eswitch shared descriptor support for eswitch representors.
+
+.I none
+- Disable shared descriptor support for eswitch representor's rx ring.
+
+.I basic
+- Enable shared descriptor support for eswitch representor's rx ring.
+
+.TP
+.BR shrdesc-count " COUNT"
+Set the number of entries in shared descriptor pool.
+
 .SS devlink dev param set  - set new value to devlink device configuration parameter
 
 .TP
-- 
2.38.1

Re: [PATCH RFC v2 iproute2-next] devlink: Add eswitch attr option for shared descriptors

[Stephen Hemminger]

On Fri, 1 Mar 2024 04:02:14 +0200
William Tu <witu@nvidia.com> wrote:

> +		} else if ((dl_argv_match(dl, "shrdesc-mode")) &&

So devlink has its own match problem.

We stopped allowing new match arguments because they create
ordering conflicts. For example in your new code if "shrdesc" is
passed it matches the mode field.

Re: [PATCH RFC v2 iproute2-next] devlink: Add eswitch attr option for shared descriptors

[William Tu]

On 2/29/24 7:39 PM, Stephen Hemminger wrote:
> External email: Use caution opening links or attachments
>
>
> On Fri, 1 Mar 2024 04:02:14 +0200
> William Tu <witu@nvidia.com> wrote:
>
>> +             } else if ((dl_argv_match(dl, "shrdesc-mode")) &&
> So devlink has its own match problem.
>
> We stopped allowing new match arguments because they create
> ordering conflicts. For example in your new code if "shrdesc" is
> passed it matches the mode field.
Hi Stephen,

Thanks! in this case how do I introduce new argments?

Should I introduce dl_argv_exact_match like below, or should I handle it 
specifically in "cmd_dev_eswitch_set(struct dl *dl)".
Thanks
William

diff --git a/devlink/devlink.c b/devlink/devlink.c
index affc29eb7cad..1521c2c24acf 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -466,6 +466,14 @@ static bool dl_argv_match(struct dl *dl, const char 
*pattern)
         return strcmpx(dl_argv(dl), pattern) == 0;
  }

+static bool dl_argv_exact_match(struct dl *dl, const char *pattern)
+{
+       if (dl_argc(dl) == 0 || strlen(dl_argv(dl)) != strlen(pattern))
+               return false;
+
+       return strcmpx(dl_argv(dl), pattern) == 0;
+}
+
  static bool dl_no_arg(struct dl *dl)
  {
         return dl_argc(dl) == 0;
@@ -1921,7 +1929,7 @@ static int dl_argv_parse(struct dl *dl, uint64_t 
o_required,
                         if (err)
                                 return err;
                         o_found |= DL_OPT_ESWITCH_ENCAP_MODE;
-               } else if ((dl_argv_match(dl, "shrdesc-mode")) &&
+               } else if ((dl_argv_exact_match(dl, "shrdesc-mode")) &&
                            (o_all & DL_OPT_ESWITCH_SHRDESC_MODE)) {
                         const char *typestr;

@@ -1934,7 +1942,7 @@ static int dl_argv_parse(struct dl *dl, uint64_t 
o_required,
                         if (err)
                                 return err;
                         o_found |= DL_OPT_ESWITCH_SHRDESC_MODE;
-               } else if (dl_argv_match(dl, "shrdesc-count") &&
+               } else if (dl_argv_exact_match(dl, "shrdesc-count") &&
                            (o_all & DL_OPT_ESWITCH_SHRDESC_COUNT)) {
                         dl_arg_inc(dl);
                         err = dl_argv_uint32_t(dl, 
&opts->eswitch_shrdesc_count);