diff mbox series

[RFC,net-next,10/19] pds_core: devlink params for enabling VIF support

Message ID 20221118225656.48309-11-snelson@pensando.io (mailing list archive)
State RFC
Delegated to: Netdev Maintainers
Headers show
Series pds core and vdpa drivers | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count fail Series longer than 15 patches (and no cover letter)
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers warning 2 maintainers not CCed: pabeni@redhat.com edumazet@google.com
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch warning WARNING: line length of 82 exceeds 80 columns WARNING: line length of 85 exceeds 80 columns
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Shannon Nelson Nov. 18, 2022, 10:56 p.m. UTC
Now that we have the code to start and stop the VFs and
set up the auxiliary_bus devices, let's add the devlink
parameter switches so the user can enable the features.

Signed-off-by: Shannon Nelson <snelson@pensando.io>
---
 .../net/ethernet/pensando/pds_core/devlink.c  | 99 +++++++++++++++++++
 1 file changed, 99 insertions(+)

Comments

Jakub Kicinski Nov. 28, 2022, 6:29 p.m. UTC | #1
On Fri, 18 Nov 2022 14:56:47 -0800 Shannon Nelson wrote:
> +	DEVLINK_PARAM_DRIVER(PDSC_DEVLINK_PARAM_ID_LM,
> +			     "enable_lm",
> +			     DEVLINK_PARAM_TYPE_BOOL,
> +			     BIT(DEVLINK_PARAM_CMODE_RUNTIME),
> +			     pdsc_dl_enable_get,
> +			     pdsc_dl_enable_set,
> +			     pdsc_dl_enable_validate),

Terrible name, not vendor specific.
Shannon Nelson Nov. 28, 2022, 10:26 p.m. UTC | #2
On 11/28/22 10:29 AM, Jakub Kicinski wrote:
> On Fri, 18 Nov 2022 14:56:47 -0800 Shannon Nelson wrote:
>> +     DEVLINK_PARAM_DRIVER(PDSC_DEVLINK_PARAM_ID_LM,
>> +                          "enable_lm",
>> +                          DEVLINK_PARAM_TYPE_BOOL,
>> +                          BIT(DEVLINK_PARAM_CMODE_RUNTIME),
>> +                          pdsc_dl_enable_get,
>> +                          pdsc_dl_enable_set,
>> +                          pdsc_dl_enable_validate),
> 
> Terrible name, not vendor specific.

... but useful for starting a conversation.

How about we add
	DEVLINK_PARAM_GENERIC_ID_ENABLE_LM,

to live along with the existing
	DEVLINK_PARAM_GENERIC_ID_ENABLE_RDMA,
	DEVLINK_PARAM_GENERIC_ID_ENABLE_VNET,

By the way, thanks for your time looking through these patches.

Cheers,
sln
Andrew Lunn Nov. 28, 2022, 10:57 p.m. UTC | #3
On Mon, Nov 28, 2022 at 02:26:26PM -0800, Shannon Nelson wrote:
> On 11/28/22 10:29 AM, Jakub Kicinski wrote:
> > On Fri, 18 Nov 2022 14:56:47 -0800 Shannon Nelson wrote:
> > > +     DEVLINK_PARAM_DRIVER(PDSC_DEVLINK_PARAM_ID_LM,
> > > +                          "enable_lm",
> > > +                          DEVLINK_PARAM_TYPE_BOOL,
> > > +                          BIT(DEVLINK_PARAM_CMODE_RUNTIME),
> > > +                          pdsc_dl_enable_get,
> > > +                          pdsc_dl_enable_set,
> > > +                          pdsc_dl_enable_validate),
> > 
> > Terrible name, not vendor specific.
> 
> ... but useful for starting a conversation.
> 
> How about we add
> 	DEVLINK_PARAM_GENERIC_ID_ENABLE_LM,

I know we are running short of short acronyms and we have to recycle
them, rfc5513 and all, so could you actually use
DEVLINK_PARAM_GENERIC_ID_ENABLE_LIST_MANAGER making it clear your
Smart NIC is running majordomo and will soon replace vger.

      Andrew
Shannon Nelson Nov. 28, 2022, 11:07 p.m. UTC | #4
On 11/28/22 2:57 PM, Andrew Lunn wrote:
> On Mon, Nov 28, 2022 at 02:26:26PM -0800, Shannon Nelson wrote:
>> On 11/28/22 10:29 AM, Jakub Kicinski wrote:
>>> On Fri, 18 Nov 2022 14:56:47 -0800 Shannon Nelson wrote:
>>>> +     DEVLINK_PARAM_DRIVER(PDSC_DEVLINK_PARAM_ID_LM,
>>>> +                          "enable_lm",
>>>> +                          DEVLINK_PARAM_TYPE_BOOL,
>>>> +                          BIT(DEVLINK_PARAM_CMODE_RUNTIME),
>>>> +                          pdsc_dl_enable_get,
>>>> +                          pdsc_dl_enable_set,
>>>> +                          pdsc_dl_enable_validate),
>>>
>>> Terrible name, not vendor specific.
>>
>> ... but useful for starting a conversation.
>>
>> How about we add
>>        DEVLINK_PARAM_GENERIC_ID_ENABLE_LM,
> 
> I know we are running short of short acronyms and we have to recycle
> them, rfc5513 and all, so could you actually use
> DEVLINK_PARAM_GENERIC_ID_ENABLE_LIST_MANAGER making it clear your
> Smart NIC is running majordomo and will soon replace vger.
> 
>        Andrew

Oh, hush, someone might hear you speak of our plan to take over the 
email world!  You never know who might be listening...

On the other hand, "LM" could be expanded to "LIVE_MIGRATION", but that 
is soooo many letters to type... perhaps I'll need to set up some 
additional vim macros.

How about:
	DEVLINK_PARAM_GENERIC_ID_ENABLE_LIVE_MIGRATION

Cheers,
sln
Andrew Lunn Nov. 28, 2022, 11:29 p.m. UTC | #5
> > I know we are running short of short acronyms and we have to recycle
> > them, rfc5513 and all, so could you actually use
> > DEVLINK_PARAM_GENERIC_ID_ENABLE_LIST_MANAGER making it clear your
> > Smart NIC is running majordomo and will soon replace vger.
> > 
> >        Andrew
> 
> Oh, hush, someone might hear you speak of our plan to take over the email
> world!

It seems like something a Smart NIC would be ideal to do. Here is an
email body and 10,000 email addresses i recently acquired, go send
spam to them at line rate.

> How about:
> 	DEVLINK_PARAM_GENERIC_ID_ENABLE_LIVE_MIGRATION

Much better.

     Andrew
Jakub Kicinski Nov. 28, 2022, 11:39 p.m. UTC | #6
On Tue, 29 Nov 2022 00:29:42 +0100 Andrew Lunn wrote:
> > How about:
> > 	DEVLINK_PARAM_GENERIC_ID_ENABLE_LIVE_MIGRATION  
> 
> Much better.

+1, although I care much less about the define name which is stupidly
long anyway and more about the actual value that the user will see
Leon Romanovsky Nov. 29, 2022, 9 a.m. UTC | #7
On Mon, Nov 28, 2022 at 03:39:22PM -0800, Jakub Kicinski wrote:
> On Tue, 29 Nov 2022 00:29:42 +0100 Andrew Lunn wrote:
> > > How about:
> > > 	DEVLINK_PARAM_GENERIC_ID_ENABLE_LIVE_MIGRATION  
> > 
> > Much better.
> 
> +1, although I care much less about the define name which is stupidly
> long anyway and more about the actual value that the user will see

We have enable/disable devlink live migration knob in our queue. Saeed
thought to send it next week.

Thanks
Jiri Pirko Nov. 29, 2022, 9:13 a.m. UTC | #8
Tue, Nov 29, 2022 at 12:39:22AM CET, kuba@kernel.org wrote:
>On Tue, 29 Nov 2022 00:29:42 +0100 Andrew Lunn wrote:
>> > How about:
>> > 	DEVLINK_PARAM_GENERIC_ID_ENABLE_LIVE_MIGRATION  
>> 
>> Much better.
>
>+1, although I care much less about the define name which is stupidly
>long anyway and more about the actual value that the user will see

We have patches that introduce live migration as a generic port function
capability bit. It is an attribute of the function.
Shannon Nelson Nov. 29, 2022, 5:16 p.m. UTC | #9
On 11/29/22 1:13 AM, Jiri Pirko wrote:
> Tue, Nov 29, 2022 at 12:39:22AM CET, kuba@kernel.org wrote:
>> On Tue, 29 Nov 2022 00:29:42 +0100 Andrew Lunn wrote:
>>>> How about:
>>>>     DEVLINK_PARAM_GENERIC_ID_ENABLE_LIVE_MIGRATION
>>>
>>> Much better.
>>
>> +1, although I care much less about the define name which is stupidly
>> long anyway and more about the actual value that the user will see
> 
> We have patches that introduce live migration as a generic port function
> capability bit. It is an attribute of the function.
> 

Thanks Leon and Jiri, we'll keep an eye out for it.

sln
diff mbox series

Patch

diff --git a/drivers/net/ethernet/pensando/pds_core/devlink.c b/drivers/net/ethernet/pensando/pds_core/devlink.c
index 0568e8b7391c..2d09643b9add 100644
--- a/drivers/net/ethernet/pensando/pds_core/devlink.c
+++ b/drivers/net/ethernet/pensando/pds_core/devlink.c
@@ -8,6 +8,75 @@ 
 
 #include "core.h"
 
+static struct pdsc_viftype *pdsc_dl_find_viftype_by_id(struct pdsc *pdsc,
+						       enum devlink_param_type dl_id)
+{
+	int vt;
+
+	for (vt = 0; vt < PDS_DEV_TYPE_MAX; vt++) {
+		if (pdsc->viftype_status[vt].dl_id == dl_id)
+			return &pdsc->viftype_status[vt];
+	}
+
+	return NULL;
+}
+
+static int pdsc_dl_enable_get(struct devlink *dl, u32 id,
+			      struct devlink_param_gset_ctx *ctx)
+{
+	struct pdsc *pdsc = devlink_priv(dl);
+	struct pdsc_viftype *vt_entry;
+
+	vt_entry = pdsc_dl_find_viftype_by_id(pdsc, id);
+	if (!vt_entry)
+		return -ENOENT;
+
+	ctx->val.vbool = vt_entry->enabled;
+
+	return 0;
+}
+
+static int pdsc_dl_enable_set(struct devlink *dl, u32 id,
+			      struct devlink_param_gset_ctx *ctx)
+{
+	struct pdsc *pdsc = devlink_priv(dl);
+	struct pdsc_viftype *vt_entry;
+	int err = 0;
+	int vf;
+
+	vt_entry = pdsc_dl_find_viftype_by_id(pdsc, id);
+	if (!vt_entry || !vt_entry->supported)
+		return -EOPNOTSUPP;
+
+	if (vt_entry->enabled == ctx->val.vbool)
+		return 0;
+
+	vt_entry->enabled = ctx->val.vbool;
+	for (vf = 0; vf < pdsc->num_vfs; vf++) {
+		err = ctx->val.vbool ? pdsc_auxbus_dev_add_vf(pdsc, vf) :
+				       pdsc_auxbus_dev_del_vf(pdsc, vf);
+	}
+
+	return err;
+}
+
+static int pdsc_dl_enable_validate(struct devlink *dl, u32 id,
+				   union devlink_param_value val,
+				   struct netlink_ext_ack *extack)
+{
+	struct pdsc *pdsc = devlink_priv(dl);
+	struct pdsc_viftype *vt_entry;
+
+	vt_entry = pdsc_dl_find_viftype_by_id(pdsc, id);
+	if (!vt_entry || !vt_entry->supported)
+		return -EOPNOTSUPP;
+
+	if (!pdsc->viftype_status[vt_entry->vif_id].supported)
+		return -ENODEV;
+
+	return 0;
+}
+
 static char *slot_labels[] = { "fw.gold", "fw.mainfwa", "fw.mainfwb" };
 
 static int pdsc_dl_fw_boot_get(struct devlink *dl, u32 id,
@@ -84,6 +153,18 @@  static int pdsc_dl_fw_boot_validate(struct devlink *dl, u32 id,
 }
 
 static const struct devlink_param pdsc_dl_params[] = {
+	DEVLINK_PARAM_GENERIC(ENABLE_VNET,
+			      BIT(DEVLINK_PARAM_CMODE_RUNTIME),
+			      pdsc_dl_enable_get,
+			      pdsc_dl_enable_set,
+			      pdsc_dl_enable_validate),
+	DEVLINK_PARAM_DRIVER(PDSC_DEVLINK_PARAM_ID_LM,
+			     "enable_lm",
+			     DEVLINK_PARAM_TYPE_BOOL,
+			     BIT(DEVLINK_PARAM_CMODE_RUNTIME),
+			     pdsc_dl_enable_get,
+			     pdsc_dl_enable_set,
+			     pdsc_dl_enable_validate),
 	DEVLINK_PARAM_DRIVER(PDSC_DEVLINK_PARAM_ID_FW_BOOT,
 			     "boot_fw",
 			     DEVLINK_PARAM_TYPE_STRING,
@@ -93,6 +174,23 @@  static const struct devlink_param pdsc_dl_params[] = {
 			     pdsc_dl_fw_boot_validate),
 };
 
+static void pdsc_dl_set_params_init_values(struct devlink *dl)
+{
+	struct pdsc *pdsc = devlink_priv(dl);
+	union devlink_param_value value;
+	int vt;
+
+	for (vt = 0; vt < PDS_DEV_TYPE_MAX; vt++) {
+		if (!pdsc->viftype_status[vt].dl_id)
+			continue;
+
+		value.vbool = pdsc->viftype_status[vt].enabled;
+		devlink_param_driverinit_value_set(dl,
+						   pdsc->viftype_status[vt].dl_id,
+						   value);
+	}
+}
+
 static int pdsc_dl_flash_update(struct devlink *dl,
 				struct devlink_flash_update_params *params,
 				struct netlink_ext_ack *extack)
@@ -195,6 +293,7 @@  int pdsc_dl_register(struct pdsc *pdsc)
 				      ARRAY_SIZE(pdsc_dl_params));
 	if (err)
 		return err;
+	pdsc_dl_set_params_init_values(dl);
 
 	devlink_register(dl);