[PATCH net-next v1] netdevsim: replace pr_err with {dev,netdev,}_err wherever possible

[PATCH net-next v1] netdevsim: replace pr_err with {dev,netdev,}_err wherever possible

[David Wei]

Replace pr_err() in netdevsim with {dev,netdev,}_err if possible,
preferring the most specific device available.

Not all instances of pr_err() can be replaced however, as there may not
be a device to associate the error with, or a device might not be
available.

Tested by building and running netdevsim/peer.sh selftest.

Signed-off-by: David Wei <dw@davidwei.uk>
---
 drivers/net/netdevsim/bus.c    | 22 +++++++++++-----------
 drivers/net/netdevsim/dev.c    | 23 +++++++++++++----------
 drivers/net/netdevsim/fib.c    |  4 ++--
 drivers/net/netdevsim/netdev.c |  2 +-
 4 files changed, 27 insertions(+), 24 deletions(-)

diff --git a/drivers/net/netdevsim/bus.c b/drivers/net/netdevsim/bus.c
index 64c0cdd31bf8..84ad23db15d7 100644
--- a/drivers/net/netdevsim/bus.c
+++ b/drivers/net/netdevsim/bus.c
@@ -270,8 +270,8 @@ static ssize_t link_device_store(const struct bus_type *bus, const char *buf, si
 	}
 
 	if (!netdev_is_nsim(dev_a)) {
-		pr_err("Device with ifindex %u in netnsfd %d is not a netdevsim\n",
-		       ifidx_a, netnsfd_a);
+		netdev_err(dev_a, "Device with ifindex %u in netnsfd %d is not a netdevsim\n",
+			   ifidx_a, netnsfd_a);
 		goto out_err;
 	}
 
@@ -283,13 +283,13 @@ static ssize_t link_device_store(const struct bus_type *bus, const char *buf, si
 	}
 
 	if (!netdev_is_nsim(dev_b)) {
-		pr_err("Device with ifindex %u in netnsfd %d is not a netdevsim\n",
-		       ifidx_b, netnsfd_b);
+		netdev_err(dev_b, "Device with ifindex %u in netnsfd %d is not a netdevsim\n",
+			   ifidx_b, netnsfd_b);
 		goto out_err;
 	}
 
 	if (dev_a == dev_b) {
-		pr_err("Cannot link a netdevsim to itself\n");
+		netdev_err(dev_a, "Cannot link a netdevsim to itself\n");
 		goto out_err;
 	}
 
@@ -297,16 +297,16 @@ static ssize_t link_device_store(const struct bus_type *bus, const char *buf, si
 	nsim_a = netdev_priv(dev_a);
 	peer = rtnl_dereference(nsim_a->peer);
 	if (peer) {
-		pr_err("Netdevsim %d:%u is already linked\n", netnsfd_a,
-		       ifidx_a);
+		netdev_err(dev_a, "Netdevsim %d:%u is already linked\n",
+			   netnsfd_a, ifidx_a);
 		goto out_err;
 	}
 
 	nsim_b = netdev_priv(dev_b);
 	peer = rtnl_dereference(nsim_b->peer);
 	if (peer) {
-		pr_err("Netdevsim %d:%u is already linked\n", netnsfd_b,
-		       ifidx_b);
+		netdev_err(dev_b, "Netdevsim %d:%u is already linked\n",
+			   netnsfd_b, ifidx_b);
 		goto out_err;
 	}
 
@@ -353,8 +353,8 @@ static ssize_t unlink_device_store(const struct bus_type *bus, const char *buf,
 	}
 
 	if (!netdev_is_nsim(dev)) {
-		pr_err("Device with ifindex %u in netnsfd %d is not a netdevsim\n",
-		       ifidx, netnsfd);
+		netdev_err(dev, "Device with ifindex %u in netnsfd %d is not a netdevsim\n",
+			   ifidx, netnsfd);
 		goto out_put_netns;
 	}
 
diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index 92a7a36b93ac..b675660b37e6 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -111,7 +111,7 @@ static ssize_t nsim_dev_take_snapshot_write(struct file *file,
 
 	err = devlink_region_snapshot_id_get(devlink, &id);
 	if (err) {
-		pr_err("Failed to get snapshot id\n");
+		dev_err(&nsim_dev->nsim_bus_dev->dev, "Failed to get snapshot id\n");
 		kfree(dummy_data);
 		return err;
 	}
@@ -119,7 +119,7 @@ static ssize_t nsim_dev_take_snapshot_write(struct file *file,
 					     dummy_data, id);
 	devlink_region_snapshot_id_put(devlink, id);
 	if (err) {
-		pr_err("Failed to create region snapshot\n");
+		dev_err(&nsim_dev->nsim_bus_dev->dev, "Failed to create region snapshot\n");
 		kfree(dummy_data);
 		return err;
 	}
@@ -428,6 +428,8 @@ static void nsim_dev_port_debugfs_exit(struct nsim_dev_port *nsim_dev_port)
 
 static int nsim_dev_resources_register(struct devlink *devlink)
 {
+	struct nsim_dev *nsim_dev = devlink_priv(devlink);
+
 	struct devlink_resource_size_params params = {
 		.size_max = (u64)-1,
 		.size_granularity = 1,
@@ -441,7 +443,7 @@ static int nsim_dev_resources_register(struct devlink *devlink)
 				     DEVLINK_RESOURCE_ID_PARENT_TOP,
 				     &params);
 	if (err) {
-		pr_err("Failed to register IPv4 top resource\n");
+		dev_err(&nsim_dev->nsim_bus_dev->dev, "Failed to register IPv4 top resource\n");
 		goto err_out;
 	}
 
@@ -449,7 +451,7 @@ static int nsim_dev_resources_register(struct devlink *devlink)
 				     NSIM_RESOURCE_IPV4_FIB,
 				     NSIM_RESOURCE_IPV4, &params);
 	if (err) {
-		pr_err("Failed to register IPv4 FIB resource\n");
+		dev_err(&nsim_dev->nsim_bus_dev->dev, "Failed to register IPv4 FIB resource\n");
 		goto err_out;
 	}
 
@@ -457,7 +459,7 @@ static int nsim_dev_resources_register(struct devlink *devlink)
 				     NSIM_RESOURCE_IPV4_FIB_RULES,
 				     NSIM_RESOURCE_IPV4, &params);
 	if (err) {
-		pr_err("Failed to register IPv4 FIB rules resource\n");
+		dev_err(&nsim_dev->nsim_bus_dev->dev, "Failed to register IPv4 FIB rules resource\n");
 		goto err_out;
 	}
 
@@ -467,7 +469,7 @@ static int nsim_dev_resources_register(struct devlink *devlink)
 				     DEVLINK_RESOURCE_ID_PARENT_TOP,
 				     &params);
 	if (err) {
-		pr_err("Failed to register IPv6 top resource\n");
+		dev_err(&nsim_dev->nsim_bus_dev->dev, "Failed to register IPv6 top resource\n");
 		goto err_out;
 	}
 
@@ -475,7 +477,7 @@ static int nsim_dev_resources_register(struct devlink *devlink)
 				     NSIM_RESOURCE_IPV6_FIB,
 				     NSIM_RESOURCE_IPV6, &params);
 	if (err) {
-		pr_err("Failed to register IPv6 FIB resource\n");
+		dev_err(&nsim_dev->nsim_bus_dev->dev, "Failed to register IPv6 FIB resource\n");
 		goto err_out;
 	}
 
@@ -483,7 +485,7 @@ static int nsim_dev_resources_register(struct devlink *devlink)
 				     NSIM_RESOURCE_IPV6_FIB_RULES,
 				     NSIM_RESOURCE_IPV6, &params);
 	if (err) {
-		pr_err("Failed to register IPv6 FIB rules resource\n");
+		dev_err(&nsim_dev->nsim_bus_dev->dev, "Failed to register IPv6 FIB rules resource\n");
 		goto err_out;
 	}
 
@@ -493,7 +495,7 @@ static int nsim_dev_resources_register(struct devlink *devlink)
 				     DEVLINK_RESOURCE_ID_PARENT_TOP,
 				     &params);
 	if (err) {
-		pr_err("Failed to register NEXTHOPS resource\n");
+		dev_err(&nsim_dev->nsim_bus_dev->dev, "Failed to register NEXTHOPS resource\n");
 		goto err_out;
 	}
 	return 0;
@@ -603,7 +605,8 @@ static int nsim_esw_switchdev_enable(struct nsim_dev *nsim_dev,
 		err = __nsim_dev_port_add(nsim_dev, NSIM_DEV_PORT_TYPE_VF, i);
 		if (err) {
 			NL_SET_ERR_MSG_MOD(extack, "Failed to initialize VFs' netdevsim ports");
-			pr_err("Failed to initialize VF id=%d. %d.\n", i, err);
+			dev_err(&nsim_dev->nsim_bus_dev->dev,
+				"Failed to initialize VF id=%d. %d.\n", i, err);
 			goto err_port_add_vfs;
 		}
 	}
diff --git a/drivers/net/netdevsim/fib.c b/drivers/net/netdevsim/fib.c
index a1f91ff8ec56..991087a47ba0 100644
--- a/drivers/net/netdevsim/fib.c
+++ b/drivers/net/netdevsim/fib.c
@@ -1587,7 +1587,7 @@ struct nsim_fib_data *nsim_fib_create(struct devlink *devlink,
 	err = register_nexthop_notifier(devlink_net(devlink), &data->nexthop_nb,
 					extack);
 	if (err) {
-		pr_err("Failed to register nexthop notifier\n");
+		dev_err(&nsim_dev->nsim_bus_dev->dev, "Failed to register nexthop notifier\n");
 		goto err_rhashtable_fib_destroy;
 	}
 
@@ -1595,7 +1595,7 @@ struct nsim_fib_data *nsim_fib_create(struct devlink *devlink,
 	err = register_fib_notifier(devlink_net(devlink), &data->fib_nb,
 				    nsim_fib_dump_inconsistent, extack);
 	if (err) {
-		pr_err("Failed to register fib notifier\n");
+		dev_err(&nsim_dev->nsim_bus_dev->dev, "Failed to register fib notifier\n");
 		goto err_nexthop_nb_unregister;
 	}
 
diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
index 8330bc0bcb7e..a31e3b50859e 100644
--- a/drivers/net/netdevsim/netdev.c
+++ b/drivers/net/netdevsim/netdev.c
@@ -132,7 +132,7 @@ static int nsim_set_vf_rate(struct net_device *dev, int vf, int min, int max)
 	struct nsim_dev *nsim_dev = ns->nsim_dev;
 
 	if (nsim_esw_mode_is_switchdev(ns->nsim_dev)) {
-		pr_err("Not supported in switchdev mode. Please use devlink API.\n");
+		netdev_err(dev, "Not supported in switchdev mode. Please use devlink API.\n");
 		return -EOPNOTSUPP;
 	}
 
-- 
2.43.0

Re: [PATCH net-next v1] netdevsim: replace pr_err with {dev,netdev,}_err wherever possible

[Simon Horman]

On Sat, Mar 09, 2024 at 05:52:15PM -0800, David Wei wrote:
> Replace pr_err() in netdevsim with {dev,netdev,}_err if possible,
> preferring the most specific device available.
> 
> Not all instances of pr_err() can be replaced however, as there may not
> be a device to associate the error with, or a device might not be
> available.
> 
> Tested by building and running netdevsim/peer.sh selftest.
> 
> Signed-off-by: David Wei <dw@davidwei.uk>

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH net-next v1] netdevsim: replace pr_err with {dev,netdev,}_err wherever possible

[Jakub Kicinski]

On Sat,  9 Mar 2024 17:52:15 -0800 David Wei wrote:
> -		pr_err("Failed to get snapshot id\n");
> +		dev_err(&nsim_dev->nsim_bus_dev->dev, "Failed to get snapshot id\n");

We seem to use dev_err(&nsim_dev->nsim_bus_dev->dev, ...
in quite a few places after this patch, how about we add a wrapper

#define nsim_err(ns_dev, args...) \
	dev_err(&(ns_dev)->nsim_bus_dev->dev(dev), ##args)
?
-- 
pw-bot: cr

Re: [PATCH net-next v1] netdevsim: replace pr_err with {dev,netdev,}_err wherever possible

[David Wei]

On 2024-03-11 13:07, Jakub Kicinski wrote:
> On Sat,  9 Mar 2024 17:52:15 -0800 David Wei wrote:
>> -		pr_err("Failed to get snapshot id\n");
>> +		dev_err(&nsim_dev->nsim_bus_dev->dev, "Failed to get snapshot id\n");
> 
> We seem to use dev_err(&nsim_dev->nsim_bus_dev->dev, ...
> in quite a few places after this patch, how about we add a wrapper
> 
> #define nsim_err(ns_dev, args...) \
> 	dev_err(&(ns_dev)->nsim_bus_dev->dev(dev), ##args)
> ?

Yeah SG, I'll define it in netdevsim.h.

I'll re-send once net-next is open again.