[RFC] net: core: devlink: add port_params_ops for devlink port parameters altering

From: Oleksandr Mazur
Date: Fri Apr 09 2021 - 12:23:12 EST


I'd like to discuss a possibility of handling devlink port parameters
with devlink port pointer supplied.

Current design makes it impossible to distinguish which port's parameter
should get altered (set) or retrieved (get) whenever there's a single
parameter registered within a few ports.

This patch aims to show how this can be changed:
- introduce structure port_params_ops that has callbacks for get/set/validate;
- if devlink has registered port_params_ops, then upon every devlink
port parameter get/set call invoke port parameters callback

Signed-off-by: Oleksandr Mazur <oleksandr.mazur@xxxxxxxxxxx>
---
drivers/net/netdevsim/dev.c | 46 +++++++++++++++++++++++++++++++
drivers/net/netdevsim/netdevsim.h | 1 +
include/net/devlink.h | 11 ++++++++
net/core/devlink.c | 16 ++++++++++-
4 files changed, 73 insertions(+), 1 deletion(-)

diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index 6189a4c0d39e..4f9a3104ca46 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -39,6 +39,11 @@ static struct dentry *nsim_dev_ddir;

#define NSIM_DEV_DUMMY_REGION_SIZE (1024 * 32)

+static int nsim_dev_port_param_set(struct devlink_port *port, u32 id,
+ struct devlink_param_gset_ctx *ctx);
+static int nsim_dev_port_param_get(struct devlink_port *port, u32 id,
+ struct devlink_param_gset_ctx *ctx);
+
static int
nsim_dev_take_snapshot(struct devlink *devlink,
const struct devlink_region_ops *ops,
@@ -339,6 +344,7 @@ static int nsim_dev_resources_register(struct devlink *devlink)
enum nsim_devlink_param_id {
NSIM_DEVLINK_PARAM_ID_BASE = DEVLINK_PARAM_GENERIC_ID_MAX,
NSIM_DEVLINK_PARAM_ID_TEST1,
+ NSIM_DEVLINK_PARAM_ID_TEST2,
};

static const struct devlink_param nsim_devlink_params[] = {
@@ -349,6 +355,10 @@ static const struct devlink_param nsim_devlink_params[] = {
"test1", DEVLINK_PARAM_TYPE_BOOL,
BIT(DEVLINK_PARAM_CMODE_DRIVERINIT),
NULL, NULL, NULL),
+ DEVLINK_PARAM_DRIVER(NSIM_DEVLINK_PARAM_ID_TEST2,
+ "test1", DEVLINK_PARAM_TYPE_U32,
+ BIT(DEVLINK_PARAM_CMODE_DRIVERINIT),
+ NULL, NULL, NULL),
};

static void nsim_devlink_set_params_init_values(struct nsim_dev *nsim_dev,
@@ -892,6 +902,11 @@ nsim_dev_devlink_trap_policer_counter_get(struct devlink *devlink,
return 0;
}

+static const struct devlink_port_param_ops nsim_dev_port_param_ops = {
+ .get = nsim_dev_port_param_get,
+ .set = nsim_dev_port_param_set,
+};
+
static const struct devlink_ops nsim_dev_devlink_ops = {
.supported_flash_update_params = DEVLINK_SUPPORT_FLASH_UPDATE_COMPONENT |
DEVLINK_SUPPORT_FLASH_UPDATE_OVERWRITE_MASK,
@@ -905,6 +920,7 @@ static const struct devlink_ops nsim_dev_devlink_ops = {
.trap_group_set = nsim_dev_devlink_trap_group_set,
.trap_policer_set = nsim_dev_devlink_trap_policer_set,
.trap_policer_counter_get = nsim_dev_devlink_trap_policer_counter_get,
+ .port_param_ops = &nsim_dev_port_param_ops,
};

#define NSIM_DEV_MAX_MACS_DEFAULT 32
@@ -1239,6 +1255,36 @@ int nsim_dev_port_del(struct nsim_bus_dev *nsim_bus_dev,
return err;
}

+static int nsim_dev_port_param_get(struct devlink_port *port, u32 id,
+ struct devlink_param_gset_ctx *ctx)
+{
+ struct nsim_dev *nsim_dev = devlink_priv(port->devlink);
+ struct nsim_dev_port *nsim_port =
+ __nsim_dev_port_lookup(nsim_dev, port->index);
+
+ if (id == NSIM_DEVLINK_PARAM_ID_TEST2) {
+ ctx->val.vu32 = nsim_port->test_parameter_value;
+ return 0;
+ }
+
+ return -EINVAL;
+}
+
+static int nsim_dev_port_param_set(struct devlink_port *port, u32 id,
+ struct devlink_param_gset_ctx *ctx)
+{
+ struct nsim_dev *nsim_dev = devlink_priv(port->devlink);
+ struct nsim_dev_port *nsim_port =
+ __nsim_dev_port_lookup(nsim_dev, port->index);
+
+ if (id == NSIM_DEVLINK_PARAM_ID_TEST2) {
+ nsim_port->test_parameter_value = ctx->val.vu32;
+ return 0;
+ }
+
+ return -EINVAL;
+}
+
int nsim_dev_init(void)
{
nsim_dev_ddir = debugfs_create_dir(DRV_NAME, NULL);
diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h
index 7ff24e03577b..4f5fc491c8d6 100644
--- a/drivers/net/netdevsim/netdevsim.h
+++ b/drivers/net/netdevsim/netdevsim.h
@@ -203,6 +203,7 @@ struct nsim_dev_port {
unsigned int port_index;
struct dentry *ddir;
struct netdevsim *ns;
+ u32 test_parameter_value;
};

struct nsim_dev {
diff --git a/include/net/devlink.h b/include/net/devlink.h
index 853420db5d32..85a7b9970496 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -1189,6 +1189,16 @@ enum devlink_trap_group_generic_id {
.min_burst = _min_burst, \
}

+struct devlink_port_param_ops {
+ int (*get)(struct devlink_port *port, u32 id,
+ struct devlink_param_gset_ctx *ctx);
+ int (*set)(struct devlink_port *port, u32 id,
+ struct devlink_param_gset_ctx *ctx);
+ int (*validate)(struct devlink_port *port, u32 id,
+ union devlink_param_value val,
+ struct netlink_ext_ack *extack);
+};
+
struct devlink_ops {
/**
* @supported_flash_update_params:
@@ -1451,6 +1461,7 @@ struct devlink_ops {
struct devlink_port *port,
enum devlink_port_fn_state state,
struct netlink_ext_ack *extack);
+ struct devlink_port_param_ops *port_param_ops;
};

static inline void *devlink_priv(struct devlink *devlink)
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 737b61c2976e..20f3545f4e7b 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -3918,6 +3918,7 @@ static int devlink_nl_param_fill(struct sk_buff *msg, struct devlink *devlink,
enum devlink_command cmd,
u32 portid, u32 seq, int flags)
{
+ struct devlink_port *dl_port;
union devlink_param_value param_value[DEVLINK_PARAM_CMODE_MAX + 1];
bool param_value_set[DEVLINK_PARAM_CMODE_MAX + 1] = {};
const struct devlink_param *param = param_item->param;
@@ -3941,7 +3942,20 @@ static int devlink_nl_param_fill(struct sk_buff *msg, struct devlink *devlink,
if (!param_item->published)
continue;
ctx.cmode = i;
- err = devlink_param_get(devlink, param, &ctx);
+ if ((cmd == DEVLINK_CMD_PORT_PARAM_GET ||
+ cmd == DEVLINK_CMD_PORT_PARAM_NEW ||
+ cmd == DEVLINK_CMD_PORT_PARAM_DEL) &&
+ devlink->ops->port_param_ops) {
+
+ dl_port = devlink_port_get_by_index(devlink,
+ port_index);
+ err = devlink->ops->port_param_ops->get(dl_port,
+ param->id,
+ &ctx);
+ } else {
+ err = devlink_param_get(devlink, param, &ctx);
+ }
+
if (err)
return err;
param_value[i] = ctx.val;
--
2.17.1