@@ -39,6 +39,11 @@ static struct dentry *nsim_dev_ddir;
#define NSIM_DEV_DUMMY_REGION_SIZE (1024 * 32)
+static int nsim_dev_devlink_port_param_set(struct devlink_port *port, u32 id,
+ struct devlink_param_gset_ctx *ctx);
+static int nsim_dev_devlink_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,17 @@ 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_MAX,
+};
+
+#define NSIM_DEV_DEVLINK_PORT_PARAM_NUM \
+ (NSIM_DEVLINK_PORT_PARAM_ID_MAX - NSIM_DEVLINK_PORT_PARAM_ID_BASE - 1)
+
+enum nsim_devlink_port_param_id {
+ NSIM_DEVLINK_PORT_PARAM_ID_BASE = NSIM_DEVLINK_PARAM_ID_MAX,
+ NSIM_DEVLINK_PORT_PARAM_ID_TEST_STR,
+ NSIM_DEVLINK_PORT_PARAM_ID_TEST_BOOL,
+ NSIM_DEVLINK_PORT_PARAM_ID_MAX,
};
static const struct devlink_param nsim_devlink_params[] = {
@@ -892,6 +908,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_devlink_port_param_get,
+ .set = nsim_dev_devlink_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 +926,24 @@ 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,
+};
+
+struct devlink_param nsim_dev_port_params[] = {
+ {
+ .id = NSIM_DEVLINK_PORT_PARAM_ID_TEST_STR,
+ .name = "port_param_test_str",
+ .generic = false,
+ .type = DEVLINK_PARAM_TYPE_STRING,
+ .supported_cmodes = BIT(DEVLINK_PARAM_CMODE_RUNTIME),
+ },
+ {
+ .id = NSIM_DEVLINK_PORT_PARAM_ID_TEST_BOOL,
+ .name = "port_param_test_bool",
+ .generic = false,
+ .type = DEVLINK_PARAM_TYPE_BOOL,
+ .supported_cmodes = BIT(DEVLINK_PARAM_CMODE_RUNTIME),
+ },
};
#define NSIM_DEV_MAX_MACS_DEFAULT 32
@@ -934,10 +973,15 @@ static int __nsim_dev_port_add(struct nsim_dev *nsim_dev,
if (err)
goto err_port_free;
- err = nsim_dev_port_debugfs_init(nsim_dev, nsim_dev_port);
+ err = devlink_port_params_register(devlink_port, nsim_dev_port_params,
+ ARRAY_SIZE(nsim_dev_port_params));
if (err)
goto err_dl_port_unregister;
+ err = nsim_dev_port_debugfs_init(nsim_dev, nsim_dev_port);
+ if (err)
+ goto err_port_params_unregister;
+
nsim_dev_port->ns = nsim_create(nsim_dev, nsim_dev_port);
if (IS_ERR(nsim_dev_port->ns)) {
err = PTR_ERR(nsim_dev_port->ns);
@@ -951,6 +995,9 @@ static int __nsim_dev_port_add(struct nsim_dev *nsim_dev,
err_port_debugfs_exit:
nsim_dev_port_debugfs_exit(nsim_dev_port);
+err_port_params_unregister:
+ devlink_port_params_unregister(devlink_port, nsim_dev_port_params,
+ ARRAY_SIZE(nsim_dev_port_params));
err_dl_port_unregister:
devlink_port_unregister(devlink_port);
err_port_free:
@@ -1239,6 +1286,42 @@ int nsim_dev_port_del(struct nsim_bus_dev *nsim_bus_dev,
return err;
}
+static int nsim_dev_devlink_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_PORT_PARAM_ID_TEST_STR) {
+ strcpy(ctx->val.vstr, nsim_port->devlink_test_param_str);
+ return 0;
+ } else if (id == NSIM_DEVLINK_PORT_PARAM_ID_TEST_BOOL) {
+ ctx->val.vbool = nsim_port->devlink_test_param_bool;
+ return 0;
+ }
+
+ return -EINVAL;
+}
+
+static int nsim_dev_devlink_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_PORT_PARAM_ID_TEST_STR) {
+ strcpy(nsim_port->devlink_test_param_str, ctx->val.vstr);
+ return 0;
+ } else if (id == NSIM_DEVLINK_PORT_PARAM_ID_TEST_BOOL) {
+ nsim_port->devlink_test_param_bool = ctx->val.vbool;
+ return 0;
+ }
+
+ return -EINVAL;
+}
+
int nsim_dev_init(void)
{
nsim_dev_ddir = debugfs_create_dir(DRV_NAME, NULL);
@@ -203,6 +203,8 @@ struct nsim_dev_port {
unsigned int port_index;
struct dentry *ddir;
struct netdevsim *ns;
+ char devlink_test_param_str[__DEVLINK_PARAM_MAX_STRING_VALUE];
+ bool devlink_test_param_bool;
};
struct nsim_dev {
@@ -1191,6 +1191,28 @@ enum devlink_trap_group_generic_id {
.min_burst = _min_burst, \
}
+/**
+ * struct devlink_port_param_ops - devlink port parameters-specific ops
+ * @get: get port parameter value, used for runtime and permanent
+ * configuration modes
+ * @set: set port parameter value, used for runtime and permanent
+ * configuration modes
+ * @validate: validate input value is applicable (within value range, etc.)
+ *
+ * This struct should be used by the driver register port parameters ops,
+ * that would be used by the devlink subsystem to fetch (get) or alter (set)
+ * devlink port parameter.
+ */
+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:
@@ -1453,6 +1475,7 @@ struct devlink_ops {
struct devlink_port *port,
enum devlink_port_fn_state state,
struct netlink_ext_ack *extack);
+ const struct devlink_port_param_ops *port_param_ops;
};
static inline void *devlink_priv(struct devlink *devlink)
@@ -3923,6 +3923,7 @@ static int devlink_nl_param_fill(struct sk_buff *msg, struct devlink *devlink,
const struct devlink_param *param = param_item->param;
struct devlink_param_gset_ctx ctx;
struct nlattr *param_values_list;
+ struct devlink_port *dl_port;
struct nlattr *param_attr;
int nla_type;
void *hdr;
@@ -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 &&
+ devlink->ops->port_param_ops->get) {
+ 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;
@@ -4201,6 +4215,7 @@ static int __devlink_nl_cmd_param_set_doit(struct devlink *devlink,
struct devlink_param_item *param_item;
const struct devlink_param *param;
union devlink_param_value value;
+ struct devlink_port *dl_port;
int err = 0;
param_item = devlink_param_get_from_info(param_list, info);
@@ -4234,13 +4249,26 @@ static int __devlink_nl_cmd_param_set_doit(struct devlink *devlink,
param_item->driverinit_value = value;
param_item->driverinit_value_valid = true;
} else {
- if (!param->set)
- return -EOPNOTSUPP;
- ctx.val = value;
- ctx.cmode = cmode;
- err = devlink_param_set(devlink, param, &ctx);
- if (err)
- return err;
+ if (cmd == DEVLINK_CMD_PORT_PARAM_SET &&
+ devlink->ops->port_param_ops &&
+ devlink->ops->port_param_ops->set) {
+ dl_port = devlink_port_get_by_index(devlink,
+ port_index);
+ err = devlink->ops->port_param_ops->set(dl_port,
+ param->id,
+ &ctx);
+ if (err)
+ return err;
+ } else {
+ if (!param->set)
+ return -EOPNOTSUPP;
+
+ ctx.val = value;
+ ctx.cmode = cmode;
+ err = devlink_param_set(devlink, param, &ctx);
+ if (err)
+ return err;
+ }
}
devlink_param_notify(devlink, port_index, param_item, cmd);
@@ -4269,8 +4297,6 @@ static int devlink_param_register_one(struct devlink *devlink,
if (param->supported_cmodes == BIT(DEVLINK_PARAM_CMODE_DRIVERINIT))
WARN_ON(param->get || param->set);
- else
- WARN_ON(!param->get || !param->set);
param_item = kzalloc(sizeof(*param_item), GFP_KERNEL);
if (!param_item)
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@plvision.eu> Suggested-by: Vadym Kochan <vadym.kochan@plvision.eu> -- V2: 1) devlink: - add commentary description over devlink_port_param_ops struct. - add port param ops 'set' usage. - remove WARN_ON upon registering devlink params in case of get/set not set. 2) netdevsim: - dynamically allocate port params, as well as their values. - add base / max values for port params and devlink params enums to ease-out iteration over them. - separate devlink port params IDs and devlink param IDs. - add statically allocated devlink port params array. --- drivers/net/netdevsim/dev.c | 85 ++++++++++++++++++++++++++++++- drivers/net/netdevsim/netdevsim.h | 2 + include/net/devlink.h | 23 +++++++++ net/core/devlink.c | 46 +++++++++++++---- 4 files changed, 145 insertions(+), 11 deletions(-)