diff mbox series

[net-next,RFC,v1,1/4] net: devlink: Add support for port regions

Message ID 20200919144332.3665538-2-andrew@lunn.ch
State RFC
Delegated to: David Miller
Headers show
Series Add per port devlink regions | expand

Commit Message

Andrew Lunn Sept. 19, 2020, 2:43 p.m. UTC 
Allow regions to be registered to a devlink port. The same netlink API
is used, but the port index is provided to indicate when a region is a
port region as opposed to a device region.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 include/net/devlink.h |  27 +++++
 net/core/devlink.c    | 251 +++++++++++++++++++++++++++++++++++++-----
 2 files changed, 252 insertions(+), 26 deletions(-)

Comments

Vladimir Oltean Sept. 20, 2020, 11:45 p.m. UTC | #1 
On Sat, Sep 19, 2020 at 04:43:29PM +0200, Andrew Lunn wrote:
> Allow regions to be registered to a devlink port. The same netlink API
> is used, but the port index is provided to indicate when a region is a
> port region as opposed to a device region.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  include/net/devlink.h |  27 +++++
>  net/core/devlink.c    | 251 +++++++++++++++++++++++++++++++++++++-----
>  2 files changed, 252 insertions(+), 26 deletions(-)
> 
> diff --git a/net/core/devlink.c b/net/core/devlink.c
> index 045468390480..66469cdcdc1e 100644
> --- a/net/core/devlink.c
> +++ b/net/core/devlink.c
> @@ -4198,16 +4225,30 @@ static int devlink_nl_cmd_region_get_doit(struct sk_buff *skb,
>  					  struct genl_info *info)
>  {
>  	struct devlink *devlink = info->user_ptr[0];
> +	struct devlink_port *port = NULL;
>  	struct devlink_region *region;
>  	const char *region_name;
>  	struct sk_buff *msg;
> +	unsigned int index;
>  	int err;
>  
>  	if (!info->attrs[DEVLINK_ATTR_REGION_NAME])
>  		return -EINVAL;
>  
> +	if (info->attrs[DEVLINK_ATTR_PORT_INDEX]) {
> +		index = nla_get_u32(info->attrs[DEVLINK_ATTR_PORT_INDEX]);
> +
> +		port = devlink_port_get_by_index(devlink, index);
> +		if (!port)
> +			return -ENODEV;
> +	}
> +
>  	region_name = nla_data(info->attrs[DEVLINK_ATTR_REGION_NAME]);
> -	region = devlink_region_get_by_name(devlink, region_name);
> +	if (port)
> +		region = devlink_port_region_get_by_name(port, region_name);
> +	else
> +		region = devlink_region_get_by_name(devlink, region_name);
> +

This looks like a simple enough solution, but am I right that old
kernels, which ignore this new DEVLINK_ATTR_PORT_INDEX netlink
attribute, will consequently interpret any devlink command for a port as
being for a global region? Sure, in the end, that kernel will probably
fail anyway, due to the region name mismatch. And at the moment there
isn't any driver that registers a global and a port region with the same
name. But when that will happen, the user space tools of the future will
trigger incorrect behavior into the kernel of today, instead of it
reporting an unsupported operation as it should. Or am I
misunderstanding?

>  	if (!region)
>  		return -EINVAL;
>  

Thanks,
-Vladimir
Vladimir Oltean Sept. 21, 2020, 12:23 a.m. UTC | #2 
On Mon, Sep 21, 2020 at 02:45:39AM +0300, Vladimir Oltean wrote:
> This looks like a simple enough solution, but am I right that old
> kernels, which ignore this new DEVLINK_ATTR_PORT_INDEX netlink
> attribute, will consequently interpret any devlink command for a port as
> being for a global region? Sure, in the end, that kernel will probably
> fail anyway, due to the region name mismatch. And at the moment there
> isn't any driver that registers a global and a port region with the same
> name. But when that will happen, the user space tools of the future will
> trigger incorrect behavior into the kernel of today, instead of it
> reporting an unsupported operation as it should. Or am I
> misunderstanding?

Thinking about this more, I believe that the only conditions that need
to be avoided are:
- mlx4 should never create a port region called "cr-space" or "fw-health"
- ice should never create a port region called "nvm-flash" or
  "device-caps"
- netdevsim should never create a port region called "dummy"
- mv88e6xxx should never create a port region called "global1",
  "global2" or "atu"

Because these are the only region names supported by kernels that don't
parse DEVLINK_ATTR_PORT_INDEX, I think we don't need to complicate the
solution, and go with DEVLINK_ATTR_PORT_INDEX.

-Vladimir
Andrew Lunn Sept. 21, 2020, 3:02 a.m. UTC | #3 
On Mon, Sep 21, 2020 at 12:23:18AM +0000, Vladimir Oltean wrote:
> On Mon, Sep 21, 2020 at 02:45:39AM +0300, Vladimir Oltean wrote:
> > This looks like a simple enough solution, but am I right that old
> > kernels, which ignore this new DEVLINK_ATTR_PORT_INDEX netlink
> > attribute, will consequently interpret any devlink command for a port as
> > being for a global region? Sure, in the end, that kernel will probably
> > fail anyway, due to the region name mismatch. And at the moment there
> > isn't any driver that registers a global and a port region with the same
> > name. But when that will happen, the user space tools of the future will
> > trigger incorrect behavior into the kernel of today, instead of it
> > reporting an unsupported operation as it should. Or am I
> > misunderstanding?
> 
> Thinking about this more, I believe that the only conditions that need
> to be avoided are:
> - mlx4 should never create a port region called "cr-space" or "fw-health"
> - ice should never create a port region called "nvm-flash" or
>   "device-caps"
> - netdevsim should never create a port region called "dummy"
> - mv88e6xxx should never create a port region called "global1",
>   "global2" or "atu"
> 
> Because these are the only region names supported by kernels that don't
> parse DEVLINK_ATTR_PORT_INDEX, I think we don't need to complicate the
> solution, and go with DEVLINK_ATTR_PORT_INDEX.

It would be easy to check when adding a per port region if a global
region of the same name already exists. Checking when adding a global
region to see if there is a port region with the same name is a bit
more work, but doable.

     Andrew
Vladimir Oltean Sept. 21, 2020, 10:09 a.m. UTC | #4 
On Mon, Sep 21, 2020 at 05:02:13AM +0200, Andrew Lunn wrote:
> On Mon, Sep 21, 2020 at 12:23:18AM +0000, Vladimir Oltean wrote:
> > On Mon, Sep 21, 2020 at 02:45:39AM +0300, Vladimir Oltean wrote:
> > > This looks like a simple enough solution, but am I right that old
> > > kernels, which ignore this new DEVLINK_ATTR_PORT_INDEX netlink
> > > attribute, will consequently interpret any devlink command for a port as
> > > being for a global region? Sure, in the end, that kernel will probably
> > > fail anyway, due to the region name mismatch. And at the moment there
> > > isn't any driver that registers a global and a port region with the same
> > > name. But when that will happen, the user space tools of the future will
> > > trigger incorrect behavior into the kernel of today, instead of it
> > > reporting an unsupported operation as it should. Or am I
> > > misunderstanding?
> >
> > Thinking about this more, I believe that the only conditions that need
> > to be avoided are:
> > - mlx4 should never create a port region called "cr-space" or "fw-health"
> > - ice should never create a port region called "nvm-flash" or
> >   "device-caps"
> > - netdevsim should never create a port region called "dummy"
> > - mv88e6xxx should never create a port region called "global1",
> >   "global2" or "atu"
> >
> > Because these are the only region names supported by kernels that don't
> > parse DEVLINK_ATTR_PORT_INDEX, I think we don't need to complicate the
> > solution, and go with DEVLINK_ATTR_PORT_INDEX.
>
> It would be easy to check when adding a per port region if a global
> region of the same name already exists. Checking when adding a global
> region to see if there is a port region with the same name is a bit
> more work, but doable.

See, I don't think that the check would be useful. By the time the new
kernel understands DEVLINK_ATTR_PORT_INDEX, the global and port regions
are already properly namespaced. So future drivers can have a port and
a global region of the same name. The problem is only with kernels that
don't understand DEVLINK_ATTR_PORT_INDEX. These only support global
regions. The concern is that future port regions for these specific 4
drivers might confuse them.

-Vladimir
diff mbox series

Patch

diff --git a/include/net/devlink.h b/include/net/devlink.h
index 4883dbae7faf..d9ce317ae060 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -110,6 +110,7 @@  struct devlink_port_attrs {
 struct devlink_port {
 	struct list_head list;
 	struct list_head param_list;
+	struct list_head region_list;
 	struct devlink *devlink;
 	unsigned int index;
 	bool registered;
@@ -573,6 +574,26 @@  struct devlink_region_ops {
 	void *priv;
 };
 
+/**
+ * struct devlink_port_region_ops - Region operations for a port
+ * @name: region name
+ * @destructor: callback used to free snapshot memory when deleting
+ * @snapshot: callback to request an immediate snapshot. On success,
+ *            the data variable must be updated to point to the snapshot data.
+ *            The function will be called while the devlink instance lock is
+ *            held.
+ * @priv: Pointer to driver private data for the region operation
+ */
+struct devlink_port_region_ops {
+	const char *name;
+	void (*destructor)(const void *data);
+	int (*snapshot)(struct devlink_port *port,
+			const struct devlink_port_region_ops *ops,
+			struct netlink_ext_ack *extack,
+			u8 **data);
+	void *priv;
+};
+
 struct devlink_fmsg;
 struct devlink_health_reporter;
 
@@ -1336,7 +1357,13 @@  struct devlink_region *
 devlink_region_create(struct devlink *devlink,
 		      const struct devlink_region_ops *ops,
 		      u32 region_max_snapshots, u64 region_size);
+struct devlink_region *
+devlink_port_region_create(struct devlink_port *port,
+			   const struct devlink_port_region_ops *ops,
+			   u32 region_max_snapshots, u64 region_size);
 void devlink_region_destroy(struct devlink_region *region);
+void devlink_port_region_destroy(struct devlink_region *region);
+
 int devlink_region_snapshot_id_get(struct devlink *devlink, u32 *id);
 void devlink_region_snapshot_id_put(struct devlink *devlink, u32 id);
 int devlink_region_snapshot_create(struct devlink_region *region,
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 045468390480..66469cdcdc1e 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -347,8 +347,12 @@  devlink_sb_tc_index_get_from_info(struct devlink_sb *devlink_sb,
 
 struct devlink_region {
 	struct devlink *devlink;
+	struct devlink_port *port;
 	struct list_head list;
-	const struct devlink_region_ops *ops;
+	union {
+		const struct devlink_region_ops *ops;
+		const struct devlink_port_region_ops *port_ops;
+	};
 	struct list_head snapshot_list;
 	u32 max_snapshots;
 	u32 cur_snapshots;
@@ -374,6 +378,19 @@  devlink_region_get_by_name(struct devlink *devlink, const char *region_name)
 	return NULL;
 }
 
+static struct devlink_region *
+devlink_port_region_get_by_name(struct devlink_port *port,
+				const char *region_name)
+{
+	struct devlink_region *region;
+
+	list_for_each_entry(region, &port->region_list, list)
+		if (!strcmp(region->ops->name, region_name))
+			return region;
+
+	return NULL;
+}
+
 static struct devlink_snapshot *
 devlink_region_snapshot_get_by_id(struct devlink_region *region, u32 id)
 {
@@ -3905,6 +3922,11 @@  static int devlink_nl_region_fill(struct sk_buff *msg, struct devlink *devlink,
 	if (err)
 		goto nla_put_failure;
 
+	if (region->port)
+		if (nla_put_u32(msg, DEVLINK_ATTR_PORT_INDEX,
+				region->port->index))
+			goto nla_put_failure;
+
 	err = nla_put_string(msg, DEVLINK_ATTR_REGION_NAME, region->ops->name);
 	if (err)
 		goto nla_put_failure;
@@ -3952,6 +3974,11 @@  devlink_nl_region_notify_build(struct devlink_region *region,
 	if (err)
 		goto out_cancel_msg;
 
+	if (region->port)
+		if (nla_put_u32(msg, DEVLINK_ATTR_PORT_INDEX,
+				region->port->index))
+			goto out_cancel_msg;
+
 	err = nla_put_string(msg, DEVLINK_ATTR_REGION_NAME,
 			     region->ops->name);
 	if (err)
@@ -4198,16 +4225,30 @@  static int devlink_nl_cmd_region_get_doit(struct sk_buff *skb,
 					  struct genl_info *info)
 {
 	struct devlink *devlink = info->user_ptr[0];
+	struct devlink_port *port = NULL;
 	struct devlink_region *region;
 	const char *region_name;
 	struct sk_buff *msg;
+	unsigned int index;
 	int err;
 
 	if (!info->attrs[DEVLINK_ATTR_REGION_NAME])
 		return -EINVAL;
 
+	if (info->attrs[DEVLINK_ATTR_PORT_INDEX]) {
+		index = nla_get_u32(info->attrs[DEVLINK_ATTR_PORT_INDEX]);
+
+		port = devlink_port_get_by_index(devlink, index);
+		if (!port)
+			return -ENODEV;
+	}
+
 	region_name = nla_data(info->attrs[DEVLINK_ATTR_REGION_NAME]);
-	region = devlink_region_get_by_name(devlink, region_name);
+	if (port)
+		region = devlink_port_region_get_by_name(port, region_name);
+	else
+		region = devlink_region_get_by_name(devlink, region_name);
+
 	if (!region)
 		return -EINVAL;
 
@@ -4226,10 +4267,76 @@  static int devlink_nl_cmd_region_get_doit(struct sk_buff *skb,
 	return genlmsg_reply(msg, info);
 }
 
+static int devlink_nl_cmd_region_get_port_dumpit(struct sk_buff *msg,
+						 struct netlink_callback *cb,
+						 struct devlink_port *port,
+						 int *idx,
+						 int start)
+{
+	struct devlink_region *region;
+	int err = 0;
+
+	list_for_each_entry(region, &port->region_list, list) {
+		if (*idx < start) {
+			(*idx)++;
+			continue;
+		}
+		err = devlink_nl_region_fill(msg, port->devlink,
+					     DEVLINK_CMD_REGION_GET,
+					     NETLINK_CB(cb->skb).portid,
+					     cb->nlh->nlmsg_seq,
+					     NLM_F_MULTI, region);
+		if (err)
+			goto out;
+		(*idx)++;
+	}
+
+out:
+	mutex_unlock(&devlink_mutex);
+	return err;
+}
+
+static int devlink_nl_cmd_region_get_devlink_dumpit(struct sk_buff *msg,
+						    struct netlink_callback *cb,
+						    struct devlink *devlink,
+						    int *idx,
+						    int start)
+{
+	struct devlink_region *region;
+	struct devlink_port *port;
+	int err = 0;
+
+	mutex_lock(&devlink->lock);
+	list_for_each_entry(region, &devlink->region_list, list) {
+		if (*idx < start) {
+			(*idx)++;
+			continue;
+		}
+		err = devlink_nl_region_fill(msg, devlink,
+					     DEVLINK_CMD_REGION_GET,
+					     NETLINK_CB(cb->skb).portid,
+					     cb->nlh->nlmsg_seq,
+					     NLM_F_MULTI, region);
+		if (err)
+			goto out;
+		(*idx)++;
+	}
+
+	list_for_each_entry(port, &devlink->port_list, list) {
+		err = devlink_nl_cmd_region_get_port_dumpit(msg, cb, port, idx,
+							    start);
+		if (err)
+			goto out;
+	}
+
+out:
+	mutex_unlock(&devlink_mutex);
+	return err;
+}
+
 static int devlink_nl_cmd_region_get_dumpit(struct sk_buff *msg,
 					    struct netlink_callback *cb)
 {
-	struct devlink_region *region;
 	struct devlink *devlink;
 	int start = cb->args[0];
 	int idx = 0;
@@ -4239,25 +4346,10 @@  static int devlink_nl_cmd_region_get_dumpit(struct sk_buff *msg,
 	list_for_each_entry(devlink, &devlink_list, list) {
 		if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
 			continue;
-
-		mutex_lock(&devlink->lock);
-		list_for_each_entry(region, &devlink->region_list, list) {
-			if (idx < start) {
-				idx++;
-				continue;
-			}
-			err = devlink_nl_region_fill(msg, devlink,
-						     DEVLINK_CMD_REGION_GET,
-						     NETLINK_CB(cb->skb).portid,
-						     cb->nlh->nlmsg_seq,
-						     NLM_F_MULTI, region);
-			if (err) {
-				mutex_unlock(&devlink->lock);
-				goto out;
-			}
-			idx++;
-		}
-		mutex_unlock(&devlink->lock);
+		err = devlink_nl_cmd_region_get_devlink_dumpit(msg, cb, devlink,
+							       &idx, start);
+		if (err)
+			goto out;
 	}
 out:
 	mutex_unlock(&devlink_mutex);
@@ -4270,8 +4362,10 @@  static int devlink_nl_cmd_region_del(struct sk_buff *skb,
 {
 	struct devlink *devlink = info->user_ptr[0];
 	struct devlink_snapshot *snapshot;
+	struct devlink_port *port = NULL;
 	struct devlink_region *region;
 	const char *region_name;
+	unsigned int index;
 	u32 snapshot_id;
 
 	if (!info->attrs[DEVLINK_ATTR_REGION_NAME] ||
@@ -4281,7 +4375,19 @@  static int devlink_nl_cmd_region_del(struct sk_buff *skb,
 	region_name = nla_data(info->attrs[DEVLINK_ATTR_REGION_NAME]);
 	snapshot_id = nla_get_u32(info->attrs[DEVLINK_ATTR_REGION_SNAPSHOT_ID]);
 
-	region = devlink_region_get_by_name(devlink, region_name);
+	if (info->attrs[DEVLINK_ATTR_PORT_INDEX]) {
+		index = nla_get_u32(info->attrs[DEVLINK_ATTR_PORT_INDEX]);
+
+		port = devlink_port_get_by_index(devlink, index);
+		if (!port)
+			return -ENODEV;
+	}
+
+	if (port)
+		region = devlink_port_region_get_by_name(port, region_name);
+	else
+		region = devlink_region_get_by_name(devlink, region_name);
+
 	if (!region)
 		return -EINVAL;
 
@@ -4298,9 +4404,11 @@  devlink_nl_cmd_region_new(struct sk_buff *skb, struct genl_info *info)
 {
 	struct devlink *devlink = info->user_ptr[0];
 	struct devlink_snapshot *snapshot;
+	struct devlink_port *port = NULL;
 	struct nlattr *snapshot_id_attr;
 	struct devlink_region *region;
 	const char *region_name;
+	unsigned int index;
 	u32 snapshot_id;
 	u8 *data;
 	int err;
@@ -4311,7 +4419,20 @@  devlink_nl_cmd_region_new(struct sk_buff *skb, struct genl_info *info)
 	}
 
 	region_name = nla_data(info->attrs[DEVLINK_ATTR_REGION_NAME]);
-	region = devlink_region_get_by_name(devlink, region_name);
+
+	if (info->attrs[DEVLINK_ATTR_PORT_INDEX]) {
+		index = nla_get_u32(info->attrs[DEVLINK_ATTR_PORT_INDEX]);
+
+		port = devlink_port_get_by_index(devlink, index);
+		if (!port)
+			return -ENODEV;
+	}
+
+	if (port)
+		region = devlink_port_region_get_by_name(port, region_name);
+	else
+		region = devlink_region_get_by_name(devlink, region_name);
+
 	if (!region) {
 		NL_SET_ERR_MSG_MOD(info->extack, "The requested region does not exist");
 		return -EINVAL;
@@ -4347,7 +4468,12 @@  devlink_nl_cmd_region_new(struct sk_buff *skb, struct genl_info *info)
 		}
 	}
 
-	err = region->ops->snapshot(devlink, region->ops, info->extack, &data);
+	if (port)
+		err = region->port_ops->snapshot(port, region->port_ops,
+						 info->extack, &data);
+	else
+		err = region->ops->snapshot(devlink, region->ops,
+					    info->extack, &data);
 	if (err)
 		goto err_snapshot_capture;
 
@@ -4469,10 +4595,12 @@  static int devlink_nl_cmd_region_read_dumpit(struct sk_buff *skb,
 	const struct genl_dumpit_info *info = genl_dumpit_info(cb);
 	u64 ret_offset, start_offset, end_offset = U64_MAX;
 	struct nlattr **attrs = info->attrs;
+	struct devlink_port *port = NULL;
 	struct devlink_region *region;
 	struct nlattr *chunks_attr;
 	const char *region_name;
 	struct devlink *devlink;
+	unsigned int index;
 	void *hdr;
 	int err;
 
@@ -4493,8 +4621,21 @@  static int devlink_nl_cmd_region_read_dumpit(struct sk_buff *skb,
 		goto out_unlock;
 	}
 
+	if (info->attrs[DEVLINK_ATTR_PORT_INDEX]) {
+		index = nla_get_u32(info->attrs[DEVLINK_ATTR_PORT_INDEX]);
+
+		port = devlink_port_get_by_index(devlink, index);
+		if (!port)
+			return -ENODEV;
+	}
+
 	region_name = nla_data(attrs[DEVLINK_ATTR_REGION_NAME]);
-	region = devlink_region_get_by_name(devlink, region_name);
+
+	if (port)
+		region = devlink_port_region_get_by_name(port, region_name);
+	else
+		region = devlink_region_get_by_name(devlink, region_name);
+
 	if (!region) {
 		err = -EINVAL;
 		goto out_unlock;
@@ -4531,6 +4672,11 @@  static int devlink_nl_cmd_region_read_dumpit(struct sk_buff *skb,
 	if (err)
 		goto nla_put_failure;
 
+	if (region->port)
+		if (nla_put_u32(skb, DEVLINK_ATTR_PORT_INDEX,
+				region->port->index))
+			goto nla_put_failure;
+
 	err = nla_put_string(skb, DEVLINK_ATTR_REGION_NAME, region_name);
 	if (err)
 		goto nla_put_failure;
@@ -7622,6 +7768,7 @@  int devlink_port_register(struct devlink *devlink,
 	mutex_init(&devlink_port->reporters_lock);
 	list_add_tail(&devlink_port->list, &devlink->port_list);
 	INIT_LIST_HEAD(&devlink_port->param_list);
+	INIT_LIST_HEAD(&devlink_port->region_list);
 	mutex_unlock(&devlink->lock);
 	INIT_DELAYED_WORK(&devlink_port->type_warn_dw, &devlink_port_type_warn);
 	devlink_port_type_warn_schedule(devlink_port);
@@ -7645,6 +7792,7 @@  void devlink_port_unregister(struct devlink_port *devlink_port)
 	list_del(&devlink_port->list);
 	mutex_unlock(&devlink->lock);
 	WARN_ON(!list_empty(&devlink_port->reporter_list));
+	WARN_ON(!list_empty(&devlink_port->region_list));
 	mutex_destroy(&devlink_port->reporters_lock);
 }
 EXPORT_SYMBOL_GPL(devlink_port_unregister);
@@ -8723,6 +8871,57 @@  devlink_region_create(struct devlink *devlink,
 }
 EXPORT_SYMBOL_GPL(devlink_region_create);
 
+/**
+ *	devlink_port_region_create - create a new address region for a port
+ *
+ *	@port: devlink port
+ *	@ops: region operations and name
+ *	@region_max_snapshots: Maximum supported number of snapshots for region
+ *	@region_size: size of region
+ */
+struct devlink_region *
+devlink_port_region_create(struct devlink_port *port,
+			   const struct devlink_port_region_ops *ops,
+			   u32 region_max_snapshots, u64 region_size)
+{
+	struct devlink *devlink = port->devlink;
+	struct devlink_region *region;
+	int err = 0;
+
+	if (WARN_ON(!ops) || WARN_ON(!ops->destructor))
+		return ERR_PTR(-EINVAL);
+
+	mutex_lock(&devlink->lock);
+
+	if (devlink_port_region_get_by_name(port, ops->name)) {
+		err = -EEXIST;
+		goto unlock;
+	}
+
+	region = kzalloc(sizeof(*region), GFP_KERNEL);
+	if (!region) {
+		err = -ENOMEM;
+		goto unlock;
+	}
+
+	region->devlink = devlink;
+	region->port = port;
+	region->max_snapshots = region_max_snapshots;
+	region->port_ops = ops;
+	region->size = region_size;
+	INIT_LIST_HEAD(&region->snapshot_list);
+	list_add_tail(&region->list, &port->region_list);
+	devlink_nl_region_notify(region, NULL, DEVLINK_CMD_REGION_NEW);
+
+	mutex_unlock(&devlink->lock);
+	return region;
+
+unlock:
+	mutex_unlock(&devlink->lock);
+	return ERR_PTR(err);
+}
+EXPORT_SYMBOL_GPL(devlink_port_region_create);
+
 /**
  *	devlink_region_destroy - destroy address region
  *