| Message ID | 20190702152056.31728-1-skalluru@marvell.com |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net-next,1/1] devlink: Add APIs to publish/unpublish the port parameters. | expand |
On Tue, Jul 02, 2019 at 08:20:56AM -0700, Sudarsana Reddy Kalluru wrote: > The patch adds devlink interfaces for drivers to publish/unpublish the > devlink port parameters. Hi Sudarsana A good commit message says more about 'why' than 'what'. I can see the 'what' by reading the code. But the 'why' is often not so clear. Why would i want to unpublish port parameters? Thanks Andrew
> -----Original Message----- > From: netdev-owner@vger.kernel.org <netdev-owner@vger.kernel.org> On > Behalf Of Andrew Lunn > Sent: Tuesday, July 2, 2019 9:42 PM > To: Sudarsana Reddy Kalluru <skalluru@marvell.com> > Cc: davem@davemloft.net; netdev@vger.kernel.org; mkalderon@marvell.com; > aelior@marvell.com; jiri@resnulli.us > Subject: Re: [PATCH net-next 1/1] devlink: Add APIs to publish/unpublish the > port parameters. > > On Tue, Jul 02, 2019 at 08:20:56AM -0700, Sudarsana Reddy Kalluru wrote: > > The patch adds devlink interfaces for drivers to publish/unpublish the > > devlink port parameters. > > Hi Sudarsana > > A good commit message says more about 'why' than 'what'. I can see the > 'what' by reading the code. But the 'why' is often not so clear. > > Why would i want to unpublish port parameters? > > Thanks > Andrew A vendor driver calling these APIs is needed at minimum.
> A vendor driver calling these APIs is needed at minimum.
Not a vendor driver, but a mainline driver.
But yes, a new API should not be added without at least one user.
Andrew
> -----Original Message----- > From: Andrew Lunn <andrew@lunn.ch> > Sent: Tuesday, July 2, 2019 10:19 PM > To: Parav Pandit <parav@mellanox.com> > Cc: Sudarsana Reddy Kalluru <skalluru@marvell.com>; davem@davemloft.net; > netdev@vger.kernel.org; mkalderon@marvell.com; aelior@marvell.com; > jiri@resnulli.us > Subject: Re: [PATCH net-next 1/1] devlink: Add APIs to publish/unpublish the > port parameters. > > > A vendor driver calling these APIs is needed at minimum. > > Not a vendor driver, but a mainline driver. > My apologies for terminology. I meant to say that a NIC/hw driver from the kernel tree that created the devlink port should call this API (as user) in the patch. You said it rightly below. Thanks. > But yes, a new API should not be added without at least one user. > > Andrew
> -----Original Message----- > From: Parav Pandit <parav@mellanox.com> > Sent: Tuesday, July 2, 2019 10:49 PM > To: Andrew Lunn <andrew@lunn.ch> > Cc: Sudarsana Reddy Kalluru <skalluru@marvell.com>; > davem@davemloft.net; netdev@vger.kernel.org; Michal Kalderon > <mkalderon@marvell.com>; Ariel Elior <aelior@marvell.com>; > jiri@resnulli.us > Subject: [EXT] RE: [PATCH net-next 1/1] devlink: Add APIs to > publish/unpublish the port parameters. > > External Email > > ---------------------------------------------------------------------- > > > > -----Original Message----- > > From: Andrew Lunn <andrew@lunn.ch> > > Sent: Tuesday, July 2, 2019 10:19 PM > > To: Parav Pandit <parav@mellanox.com> > > Cc: Sudarsana Reddy Kalluru <skalluru@marvell.com>; > > davem@davemloft.net; netdev@vger.kernel.org; > mkalderon@marvell.com; > > aelior@marvell.com; jiri@resnulli.us > > Subject: Re: [PATCH net-next 1/1] devlink: Add APIs to > > publish/unpublish the port parameters. > > > > > A vendor driver calling these APIs is needed at minimum. > > > > Not a vendor driver, but a mainline driver. > > > My apologies for terminology. > I meant to say that a NIC/hw driver from the kernel tree that created the > devlink port should call this API (as user) in the patch. > You said it rightly below. Thanks. > > > But yes, a new API should not be added without at least one user. > > > > Andrew Thanks a lot for your reviews. Marvell NIC driver has a requirement to support the display/configuration of device attributes. Sent the proposed changes with following 'subject line', [PATCH net-next 4/4] qed: Add devlink support for configuration attributes. Have received a comment (from community) suggesting to move some of the attributes to devlink-port interface, which requires the proposed APIs. Will update the commit message and send it with the Marvel driver patch series which use this functionality.
diff --git a/include/net/devlink.h b/include/net/devlink.h index 6c51e86..2e2d7fc 100644 --- a/include/net/devlink.h +++ b/include/net/devlink.h @@ -651,6 +651,8 @@ int devlink_port_params_register(struct devlink_port *devlink_port, void devlink_port_params_unregister(struct devlink_port *devlink_port, const struct devlink_param *params, size_t params_count); +void devlink_port_params_publish(struct devlink_port *devlink_port); +void devlink_port_params_unpublish(struct devlink_port *ddevlink_port); int devlink_param_driverinit_value_get(struct devlink *devlink, u32 param_id, union devlink_param_value *init_val); int devlink_param_driverinit_value_set(struct devlink *devlink, u32 param_id, diff --git a/net/core/devlink.c b/net/core/devlink.c index 4baf716..c06c23f 100644 --- a/net/core/devlink.c +++ b/net/core/devlink.c @@ -6378,6 +6378,48 @@ void devlink_port_params_unregister(struct devlink_port *devlink_port, } EXPORT_SYMBOL_GPL(devlink_port_params_unregister); +/** + * devlink_port_params_publish - publish port configuration parameters + * + * @devlink_port: devlink port + * + * Publish previously registered port configuration parameters. + */ +void devlink_port_params_publish(struct devlink_port *devlink_port) +{ + struct devlink_param_item *param_item; + + list_for_each_entry(param_item, &devlink_port->param_list, list) { + if (param_item->published) + continue; + param_item->published = true; + devlink_param_notify(devlink_port->devlink, devlink_port->index, + param_item, DEVLINK_CMD_PORT_PARAM_NEW); + } +} +EXPORT_SYMBOL_GPL(devlink_port_params_publish); + +/** + * devlink_port_params_unpublish - unpublish port configuration parameters + * + * @devlink_port: devlink port + * + * Unpublish previously registered port configuration parameters. + */ +void devlink_port_params_unpublish(struct devlink_port *devlink_port) +{ + struct devlink_param_item *param_item; + + list_for_each_entry(param_item, &devlink_port->param_list, list) { + if (!param_item->published) + continue; + param_item->published = false; + devlink_param_notify(devlink_port->devlink, devlink_port->index, + param_item, DEVLINK_CMD_PORT_PARAM_DEL); + } +} +EXPORT_SYMBOL_GPL(devlink_port_params_unpublish); + static int __devlink_param_driverinit_value_get(struct list_head *param_list, u32 param_id, union devlink_param_value *init_val)