All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next v2 0/5] netdevsim: improve separation between device and bus
@ 2021-10-30 23:15 Jakub Kicinski
  2021-10-30 23:15 ` [PATCH net-next v2 1/5] netdevsim: take rtnl_lock when assigning num_vfs Jakub Kicinski
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: Jakub Kicinski @ 2021-10-30 23:15 UTC (permalink / raw)
  To: davem; +Cc: netdev, Jakub Kicinski

VF config falls strangely in between device and bus
responsibilities today. Because of this bus.c sticks fingers
directly into struct nsim_dev and we look at nsim_bus_dev
in many more places than necessary.

Make bus.c contain pure interface code, and move
the particulars of the logic (which touch on eswitch,
devlink reloads etc) to dev.c. Rename the functions
at the boundary of the interface to make the separation
clearer.

v2: add missing statics after functions were un-exposed

Jakub Kicinski (5):
  netdevsim: take rtnl_lock when assigning num_vfs
  netdevsim: move vfconfig to nsim_dev
  netdevsim: move details of vf config to dev
  netdevsim: move max vf config to dev
  netdevsim: rename 'driver' entry points

 drivers/net/netdevsim/bus.c       | 155 ++----------------------
 drivers/net/netdevsim/dev.c       | 188 ++++++++++++++++++++++++++----
 drivers/net/netdevsim/netdev.c    |  72 ++++++------
 drivers/net/netdevsim/netdevsim.h |  55 ++++-----
 4 files changed, 235 insertions(+), 235 deletions(-)

-- 
2.31.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH net-next v2 1/5] netdevsim: take rtnl_lock when assigning num_vfs
  2021-10-30 23:15 [PATCH net-next v2 0/5] netdevsim: improve separation between device and bus Jakub Kicinski
@ 2021-10-30 23:15 ` Jakub Kicinski
  2021-10-30 23:15 ` [PATCH net-next v2 2/5] netdevsim: move vfconfig to nsim_dev Jakub Kicinski
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Jakub Kicinski @ 2021-10-30 23:15 UTC (permalink / raw)
  To: davem; +Cc: netdev, Jakub Kicinski

Legacy VF NDOs look at num_vfs and then based on that
index into vfconfig. If we don't rtnl_lock() num_vfs
may get set to 0 and vfconfig freed/replaced while
the NDO is running.

We don't need to protect replacing vfconfig since it's
only done when num_vfs is 0.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 drivers/net/netdevsim/bus.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/net/netdevsim/bus.c b/drivers/net/netdevsim/bus.c
index 29f5627d11e6..284223108d25 100644
--- a/drivers/net/netdevsim/bus.c
+++ b/drivers/net/netdevsim/bus.c
@@ -24,6 +24,14 @@ static struct nsim_bus_dev *to_nsim_bus_dev(struct device *dev)
 	return container_of(dev, struct nsim_bus_dev, dev);
 }
 
+static void
+nsim_bus_dev_set_vfs(struct nsim_bus_dev *nsim_bus_dev, unsigned int num_vfs)
+{
+	rtnl_lock();
+	nsim_bus_dev->num_vfs = num_vfs;
+	rtnl_unlock();
+}
+
 static int nsim_bus_dev_vfs_enable(struct nsim_bus_dev *nsim_bus_dev,
 				   unsigned int num_vfs)
 {
@@ -35,13 +43,13 @@ static int nsim_bus_dev_vfs_enable(struct nsim_bus_dev *nsim_bus_dev,
 
 	if (!nsim_bus_dev->vfconfigs)
 		return -ENOMEM;
-	nsim_bus_dev->num_vfs = num_vfs;
+	nsim_bus_dev_set_vfs(nsim_bus_dev, num_vfs);
 
 	nsim_dev = dev_get_drvdata(&nsim_bus_dev->dev);
 	if (nsim_esw_mode_is_switchdev(nsim_dev)) {
 		err = nsim_esw_switchdev_enable(nsim_dev, NULL);
 		if (err)
-			nsim_bus_dev->num_vfs = 0;
+			nsim_bus_dev_set_vfs(nsim_bus_dev, 0);
 	}
 
 	return err;
@@ -51,7 +59,7 @@ void nsim_bus_dev_vfs_disable(struct nsim_bus_dev *nsim_bus_dev)
 {
 	struct nsim_dev *nsim_dev;
 
-	nsim_bus_dev->num_vfs = 0;
+	nsim_bus_dev_set_vfs(nsim_bus_dev, 0);
 	nsim_dev = dev_get_drvdata(&nsim_bus_dev->dev);
 	if (nsim_esw_mode_is_switchdev(nsim_dev))
 		nsim_esw_legacy_enable(nsim_dev, NULL);
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH net-next v2 2/5] netdevsim: move vfconfig to nsim_dev
  2021-10-30 23:15 [PATCH net-next v2 0/5] netdevsim: improve separation between device and bus Jakub Kicinski
  2021-10-30 23:15 ` [PATCH net-next v2 1/5] netdevsim: take rtnl_lock when assigning num_vfs Jakub Kicinski
@ 2021-10-30 23:15 ` Jakub Kicinski
  2021-10-30 23:15 ` [PATCH net-next v2 3/5] netdevsim: move details of vf config to dev Jakub Kicinski
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Jakub Kicinski @ 2021-10-30 23:15 UTC (permalink / raw)
  To: davem; +Cc: netdev, Jakub Kicinski

When netdevsim got split into the faux bus vfconfig ended
up in the bus device (think pci_dev) which is strange because
it contains very networky not to say netdevy information.
Move it to nsim_dev, which is the driver "priv" structure
for the device.

To make sure we don't race with probe/remove take
the device lock (much like PCI).

While at it remove the NULL-checking of vfconfigs.
It appears to be pointless.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 drivers/net/netdevsim/bus.c       | 43 ++++++++----------
 drivers/net/netdevsim/dev.c       | 53 ++++++++++++++++-------
 drivers/net/netdevsim/netdev.c    | 72 +++++++++++++++----------------
 drivers/net/netdevsim/netdevsim.h | 34 ++++++++-------
 4 files changed, 111 insertions(+), 91 deletions(-)

diff --git a/drivers/net/netdevsim/bus.c b/drivers/net/netdevsim/bus.c
index 284223108d25..1e7df184419d 100644
--- a/drivers/net/netdevsim/bus.c
+++ b/drivers/net/netdevsim/bus.c
@@ -40,9 +40,6 @@ static int nsim_bus_dev_vfs_enable(struct nsim_bus_dev *nsim_bus_dev,
 
 	if (nsim_bus_dev->max_vfs < num_vfs)
 		return -ENOMEM;
-
-	if (!nsim_bus_dev->vfconfigs)
-		return -ENOMEM;
 	nsim_bus_dev_set_vfs(nsim_bus_dev, num_vfs);
 
 	nsim_dev = dev_get_drvdata(&nsim_bus_dev->dev);
@@ -70,6 +67,7 @@ nsim_bus_dev_numvfs_store(struct device *dev, struct device_attribute *attr,
 			  const char *buf, size_t count)
 {
 	struct nsim_bus_dev *nsim_bus_dev = to_nsim_bus_dev(dev);
+	struct nsim_dev *nsim_dev = dev_get_drvdata(dev);
 	unsigned int num_vfs;
 	int ret;
 
@@ -77,7 +75,13 @@ nsim_bus_dev_numvfs_store(struct device *dev, struct device_attribute *attr,
 	if (ret)
 		return ret;
 
-	mutex_lock(&nsim_bus_dev->vfs_lock);
+	device_lock(dev);
+	if (!nsim_dev) {
+		ret = -ENOENT;
+		goto exit_unlock;
+	}
+
+	mutex_lock(&nsim_dev->vfs_lock);
 	if (nsim_bus_dev->num_vfs == num_vfs)
 		goto exit_good;
 	if (nsim_bus_dev->num_vfs && num_vfs) {
@@ -95,7 +99,8 @@ nsim_bus_dev_numvfs_store(struct device *dev, struct device_attribute *attr,
 exit_good:
 	ret = count;
 exit_unlock:
-	mutex_unlock(&nsim_bus_dev->vfs_lock);
+	mutex_unlock(&nsim_dev->vfs_lock);
+	device_unlock(dev);
 
 	return ret;
 }
@@ -117,7 +122,8 @@ ssize_t nsim_bus_dev_max_vfs_read(struct file *file,
 				  char __user *data,
 				  size_t count, loff_t *ppos)
 {
-	struct nsim_bus_dev *nsim_bus_dev = file->private_data;
+	struct nsim_dev *nsim_dev = file->private_data;
+	struct nsim_bus_dev *nsim_bus_dev = nsim_dev->nsim_bus_dev;
 	char buf[11];
 	ssize_t len;
 
@@ -132,7 +138,8 @@ ssize_t nsim_bus_dev_max_vfs_write(struct file *file,
 				   const char __user *data,
 				   size_t count, loff_t *ppos)
 {
-	struct nsim_bus_dev *nsim_bus_dev = file->private_data;
+	struct nsim_dev *nsim_dev = file->private_data;
+	struct nsim_bus_dev *nsim_bus_dev = nsim_dev->nsim_bus_dev;
 	struct nsim_vf_config *vfconfigs;
 	ssize_t ret;
 	char buf[10];
@@ -144,7 +151,7 @@ ssize_t nsim_bus_dev_max_vfs_write(struct file *file,
 	if (count >= sizeof(buf))
 		return -ENOSPC;
 
-	mutex_lock(&nsim_bus_dev->vfs_lock);
+	mutex_lock(&nsim_dev->vfs_lock);
 	/* Reject if VFs are configured */
 	if (nsim_bus_dev->num_vfs) {
 		ret = -EBUSY;
@@ -176,13 +183,13 @@ ssize_t nsim_bus_dev_max_vfs_write(struct file *file,
 		goto unlock;
 	}
 
-	kfree(nsim_bus_dev->vfconfigs);
-	nsim_bus_dev->vfconfigs = vfconfigs;
+	kfree(nsim_dev->vfconfigs);
+	nsim_dev->vfconfigs = vfconfigs;
 	nsim_bus_dev->max_vfs = val;
 	*ppos += count;
 	ret = count;
 unlock:
-	mutex_unlock(&nsim_bus_dev->vfs_lock);
+	mutex_unlock(&nsim_dev->vfs_lock);
 	return ret;
 }
 
@@ -428,26 +435,15 @@ nsim_bus_dev_new(unsigned int id, unsigned int port_count, unsigned int num_queu
 	nsim_bus_dev->initial_net = current->nsproxy->net_ns;
 	nsim_bus_dev->max_vfs = NSIM_BUS_DEV_MAX_VFS;
 	mutex_init(&nsim_bus_dev->nsim_bus_reload_lock);
-	mutex_init(&nsim_bus_dev->vfs_lock);
 	/* Disallow using nsim_bus_dev */
 	smp_store_release(&nsim_bus_dev->init, false);
 
-	nsim_bus_dev->vfconfigs = kcalloc(nsim_bus_dev->max_vfs,
-					  sizeof(struct nsim_vf_config),
-					  GFP_KERNEL | __GFP_NOWARN);
-	if (!nsim_bus_dev->vfconfigs) {
-		err = -ENOMEM;
-		goto err_nsim_bus_dev_id_free;
-	}
-
 	err = device_register(&nsim_bus_dev->dev);
 	if (err)
-		goto err_nsim_vfs_free;
+		goto err_nsim_bus_dev_id_free;
 
 	return nsim_bus_dev;
 
-err_nsim_vfs_free:
-	kfree(nsim_bus_dev->vfconfigs);
 err_nsim_bus_dev_id_free:
 	ida_free(&nsim_bus_dev_ids, nsim_bus_dev->dev.id);
 err_nsim_bus_dev_free:
@@ -461,7 +457,6 @@ static void nsim_bus_dev_del(struct nsim_bus_dev *nsim_bus_dev)
 	smp_store_release(&nsim_bus_dev->init, false);
 	device_unregister(&nsim_bus_dev->dev);
 	ida_free(&nsim_bus_dev_ids, nsim_bus_dev->dev.id);
-	kfree(nsim_bus_dev->vfconfigs);
 	kfree(nsim_bus_dev);
 }
 
diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index 6c906deca71c..8157d28b32e4 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -56,6 +56,14 @@ static inline unsigned int nsim_dev_port_index_to_vf_index(unsigned int port_ind
 
 static struct dentry *nsim_dev_ddir;
 
+unsigned int nsim_dev_get_vfs(struct nsim_dev *nsim_dev)
+{
+	WARN_ON(!lockdep_rtnl_is_held() &&
+		!lockdep_is_held(&nsim_dev->vfs_lock));
+
+	return nsim_dev->nsim_bus_dev->num_vfs;
+}
+
 #define NSIM_DEV_DUMMY_REGION_SIZE (1024 * 32)
 
 static int
@@ -260,7 +268,7 @@ static int nsim_dev_debugfs_init(struct nsim_dev *nsim_dev)
 			    nsim_dev->ddir,
 			    &nsim_dev->fail_trap_policer_counter_get);
 	debugfs_create_file("max_vfs", 0600, nsim_dev->ddir,
-			    nsim_dev->nsim_bus_dev, &nsim_dev_max_vfs_fops);
+			    nsim_dev, &nsim_dev_max_vfs_fops);
 
 	nsim_dev->nodes_ddir = debugfs_create_dir("rate_nodes", nsim_dev->ddir);
 	if (IS_ERR(nsim_dev->nodes_ddir)) {
@@ -326,9 +334,9 @@ static int nsim_dev_port_debugfs_init(struct nsim_dev *nsim_dev,
 		unsigned int vf_id = nsim_dev_port_index_to_vf_index(port_index);
 
 		debugfs_create_u16("tx_share", 0400, nsim_dev_port->ddir,
-				   &nsim_bus_dev->vfconfigs[vf_id].min_tx_rate);
+				   &nsim_dev->vfconfigs[vf_id].min_tx_rate);
 		debugfs_create_u16("tx_max", 0400, nsim_dev_port->ddir,
-				   &nsim_bus_dev->vfconfigs[vf_id].max_tx_rate);
+				   &nsim_dev->vfconfigs[vf_id].max_tx_rate);
 		nsim_dev_port->rate_parent = debugfs_create_file("rate_parent",
 								 0400,
 								 nsim_dev_port->ddir,
@@ -508,7 +516,7 @@ int nsim_esw_switchdev_enable(struct nsim_dev *nsim_dev, struct netlink_ext_ack
 	struct nsim_bus_dev *nsim_bus_dev = nsim_dev->nsim_bus_dev;
 	int i, err;
 
-	for (i = 0; i < nsim_bus_dev->num_vfs; i++) {
+	for (i = 0; i < nsim_dev_get_vfs(nsim_dev); i++) {
 		err = nsim_dev_port_add(nsim_bus_dev, NSIM_DEV_PORT_TYPE_VF, i);
 		if (err) {
 			NL_SET_ERR_MSG_MOD(extack, "Failed to initialize VFs' netdevsim ports");
@@ -531,7 +539,7 @@ static int nsim_devlink_eswitch_mode_set(struct devlink *devlink, u16 mode,
 	struct nsim_dev *nsim_dev = devlink_priv(devlink);
 	int err = 0;
 
-	mutex_lock(&nsim_dev->nsim_bus_dev->vfs_lock);
+	mutex_lock(&nsim_dev->vfs_lock);
 	if (mode == nsim_dev->esw_mode)
 		goto unlock;
 
@@ -543,7 +551,7 @@ static int nsim_devlink_eswitch_mode_set(struct devlink *devlink, u16 mode,
 		err = -EINVAL;
 
 unlock:
-	mutex_unlock(&nsim_dev->nsim_bus_dev->vfs_lock);
+	mutex_unlock(&nsim_dev->vfs_lock);
 	return err;
 }
 
@@ -1091,7 +1099,7 @@ static int nsim_leaf_tx_share_set(struct devlink_rate *devlink_rate, void *priv,
 				  u64 tx_share, struct netlink_ext_ack *extack)
 {
 	struct nsim_dev_port *nsim_dev_port = priv;
-	struct nsim_bus_dev *nsim_bus_dev = nsim_dev_port->ns->nsim_bus_dev;
+	struct nsim_dev *nsim_dev = nsim_dev_port->ns->nsim_dev;
 	int vf_id = nsim_dev_port_index_to_vf_index(nsim_dev_port->port_index);
 	int err;
 
@@ -1099,7 +1107,7 @@ static int nsim_leaf_tx_share_set(struct devlink_rate *devlink_rate, void *priv,
 	if (err)
 		return err;
 
-	nsim_bus_dev->vfconfigs[vf_id].min_tx_rate = tx_share;
+	nsim_dev->vfconfigs[vf_id].min_tx_rate = tx_share;
 	return 0;
 }
 
@@ -1107,7 +1115,7 @@ static int nsim_leaf_tx_max_set(struct devlink_rate *devlink_rate, void *priv,
 				u64 tx_max, struct netlink_ext_ack *extack)
 {
 	struct nsim_dev_port *nsim_dev_port = priv;
-	struct nsim_bus_dev *nsim_bus_dev = nsim_dev_port->ns->nsim_bus_dev;
+	struct nsim_dev *nsim_dev = nsim_dev_port->ns->nsim_dev;
 	int vf_id = nsim_dev_port_index_to_vf_index(nsim_dev_port->port_index);
 	int err;
 
@@ -1115,7 +1123,7 @@ static int nsim_leaf_tx_max_set(struct devlink_rate *devlink_rate, void *priv,
 	if (err)
 		return err;
 
-	nsim_bus_dev->vfconfigs[vf_id].max_tx_rate = tx_max;
+	nsim_dev->vfconfigs[vf_id].max_tx_rate = tx_max;
 	return 0;
 }
 
@@ -1271,13 +1279,12 @@ static const struct devlink_ops nsim_dev_devlink_ops = {
 static int __nsim_dev_port_add(struct nsim_dev *nsim_dev, enum nsim_dev_port_type type,
 			       unsigned int port_index)
 {
-	struct nsim_bus_dev *nsim_bus_dev = nsim_dev->nsim_bus_dev;
 	struct devlink_port_attrs attrs = {};
 	struct nsim_dev_port *nsim_dev_port;
 	struct devlink_port *devlink_port;
 	int err;
 
-	if (type == NSIM_DEV_PORT_TYPE_VF && !nsim_bus_dev->num_vfs)
+	if (type == NSIM_DEV_PORT_TYPE_VF && !nsim_dev_get_vfs(nsim_dev))
 		return -EINVAL;
 
 	nsim_dev_port = kzalloc(sizeof(*nsim_dev_port), GFP_KERNEL);
@@ -1455,6 +1462,7 @@ int nsim_dev_probe(struct nsim_bus_dev *nsim_bus_dev)
 	nsim_dev->switch_id.id_len = sizeof(nsim_dev->switch_id.id);
 	get_random_bytes(nsim_dev->switch_id.id, nsim_dev->switch_id.id_len);
 	INIT_LIST_HEAD(&nsim_dev->port_list);
+	mutex_init(&nsim_dev->vfs_lock);
 	mutex_init(&nsim_dev->port_list_lock);
 	nsim_dev->fw_update_status = true;
 	nsim_dev->fw_update_overwrite_mask = 0;
@@ -1464,9 +1472,17 @@ int nsim_dev_probe(struct nsim_bus_dev *nsim_bus_dev)
 
 	dev_set_drvdata(&nsim_bus_dev->dev, nsim_dev);
 
+	nsim_dev->vfconfigs = kcalloc(nsim_bus_dev->max_vfs,
+				      sizeof(struct nsim_vf_config),
+				      GFP_KERNEL | __GFP_NOWARN);
+	if (!nsim_dev->vfconfigs) {
+		err = -ENOMEM;
+		goto err_devlink_free;
+	}
+
 	err = nsim_dev_resources_register(devlink);
 	if (err)
-		goto err_devlink_free;
+		goto err_vfc_free;
 
 	err = devlink_params_register(devlink, nsim_devlink_params,
 				      ARRAY_SIZE(nsim_devlink_params));
@@ -1532,8 +1548,11 @@ int nsim_dev_probe(struct nsim_bus_dev *nsim_bus_dev)
 				  ARRAY_SIZE(nsim_devlink_params));
 err_dl_unregister:
 	devlink_resources_unregister(devlink, NULL);
+err_vfc_free:
+	kfree(nsim_dev->vfconfigs);
 err_devlink_free:
 	devlink_free(devlink);
+	dev_set_drvdata(&nsim_bus_dev->dev, NULL);
 	return err;
 }
 
@@ -1545,10 +1564,10 @@ static void nsim_dev_reload_destroy(struct nsim_dev *nsim_dev)
 		return;
 	debugfs_remove(nsim_dev->take_snapshot);
 
-	mutex_lock(&nsim_dev->nsim_bus_dev->vfs_lock);
-	if (nsim_dev->nsim_bus_dev->num_vfs)
+	mutex_lock(&nsim_dev->vfs_lock);
+	if (nsim_dev_get_vfs(nsim_dev))
 		nsim_bus_dev_vfs_disable(nsim_dev->nsim_bus_dev);
-	mutex_unlock(&nsim_dev->nsim_bus_dev->vfs_lock);
+	mutex_unlock(&nsim_dev->vfs_lock);
 
 	nsim_dev_port_del_all(nsim_dev);
 	nsim_dev_psample_exit(nsim_dev);
@@ -1572,7 +1591,9 @@ void nsim_dev_remove(struct nsim_bus_dev *nsim_bus_dev)
 	devlink_params_unregister(devlink, nsim_devlink_params,
 				  ARRAY_SIZE(nsim_devlink_params));
 	devlink_resources_unregister(devlink, NULL);
+	kfree(nsim_dev->vfconfigs);
 	devlink_free(devlink);
+	dev_set_drvdata(&nsim_bus_dev->dev, NULL);
 }
 
 static struct nsim_dev_port *
diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
index 50572e0f1f52..e470e3398abc 100644
--- a/drivers/net/netdevsim/netdev.c
+++ b/drivers/net/netdevsim/netdev.c
@@ -82,12 +82,12 @@ nsim_setup_tc_block_cb(enum tc_setup_type type, void *type_data, void *cb_priv)
 static int nsim_set_vf_mac(struct net_device *dev, int vf, u8 *mac)
 {
 	struct netdevsim *ns = netdev_priv(dev);
-	struct nsim_bus_dev *nsim_bus_dev = ns->nsim_bus_dev;
+	struct nsim_dev *nsim_dev = ns->nsim_dev;
 
 	/* Only refuse multicast addresses, zero address can mean unset/any. */
-	if (vf >= nsim_bus_dev->num_vfs || is_multicast_ether_addr(mac))
+	if (vf >= nsim_dev_get_vfs(nsim_dev) || is_multicast_ether_addr(mac))
 		return -EINVAL;
-	memcpy(nsim_bus_dev->vfconfigs[vf].vf_mac, mac, ETH_ALEN);
+	memcpy(nsim_dev->vfconfigs[vf].vf_mac, mac, ETH_ALEN);
 
 	return 0;
 }
@@ -96,14 +96,14 @@ static int nsim_set_vf_vlan(struct net_device *dev, int vf,
 			    u16 vlan, u8 qos, __be16 vlan_proto)
 {
 	struct netdevsim *ns = netdev_priv(dev);
-	struct nsim_bus_dev *nsim_bus_dev = ns->nsim_bus_dev;
+	struct nsim_dev *nsim_dev = ns->nsim_dev;
 
-	if (vf >= nsim_bus_dev->num_vfs || vlan > 4095 || qos > 7)
+	if (vf >= nsim_dev_get_vfs(nsim_dev) || vlan > 4095 || qos > 7)
 		return -EINVAL;
 
-	nsim_bus_dev->vfconfigs[vf].vlan = vlan;
-	nsim_bus_dev->vfconfigs[vf].qos = qos;
-	nsim_bus_dev->vfconfigs[vf].vlan_proto = vlan_proto;
+	nsim_dev->vfconfigs[vf].vlan = vlan;
+	nsim_dev->vfconfigs[vf].qos = qos;
+	nsim_dev->vfconfigs[vf].vlan_proto = vlan_proto;
 
 	return 0;
 }
@@ -111,18 +111,18 @@ static int nsim_set_vf_vlan(struct net_device *dev, int vf,
 static int nsim_set_vf_rate(struct net_device *dev, int vf, int min, int max)
 {
 	struct netdevsim *ns = netdev_priv(dev);
-	struct nsim_bus_dev *nsim_bus_dev = ns->nsim_bus_dev;
+	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");
 		return -EOPNOTSUPP;
 	}
 
-	if (vf >= nsim_bus_dev->num_vfs)
+	if (vf >= nsim_dev_get_vfs(nsim_dev))
 		return -EINVAL;
 
-	nsim_bus_dev->vfconfigs[vf].min_tx_rate = min;
-	nsim_bus_dev->vfconfigs[vf].max_tx_rate = max;
+	nsim_dev->vfconfigs[vf].min_tx_rate = min;
+	nsim_dev->vfconfigs[vf].max_tx_rate = max;
 
 	return 0;
 }
@@ -130,11 +130,11 @@ static int nsim_set_vf_rate(struct net_device *dev, int vf, int min, int max)
 static int nsim_set_vf_spoofchk(struct net_device *dev, int vf, bool val)
 {
 	struct netdevsim *ns = netdev_priv(dev);
-	struct nsim_bus_dev *nsim_bus_dev = ns->nsim_bus_dev;
+	struct nsim_dev *nsim_dev = ns->nsim_dev;
 
-	if (vf >= nsim_bus_dev->num_vfs)
+	if (vf >= nsim_dev_get_vfs(nsim_dev))
 		return -EINVAL;
-	nsim_bus_dev->vfconfigs[vf].spoofchk_enabled = val;
+	nsim_dev->vfconfigs[vf].spoofchk_enabled = val;
 
 	return 0;
 }
@@ -142,11 +142,11 @@ static int nsim_set_vf_spoofchk(struct net_device *dev, int vf, bool val)
 static int nsim_set_vf_rss_query_en(struct net_device *dev, int vf, bool val)
 {
 	struct netdevsim *ns = netdev_priv(dev);
-	struct nsim_bus_dev *nsim_bus_dev = ns->nsim_bus_dev;
+	struct nsim_dev *nsim_dev = ns->nsim_dev;
 
-	if (vf >= nsim_bus_dev->num_vfs)
+	if (vf >= nsim_dev_get_vfs(nsim_dev))
 		return -EINVAL;
-	nsim_bus_dev->vfconfigs[vf].rss_query_enabled = val;
+	nsim_dev->vfconfigs[vf].rss_query_enabled = val;
 
 	return 0;
 }
@@ -154,11 +154,11 @@ static int nsim_set_vf_rss_query_en(struct net_device *dev, int vf, bool val)
 static int nsim_set_vf_trust(struct net_device *dev, int vf, bool val)
 {
 	struct netdevsim *ns = netdev_priv(dev);
-	struct nsim_bus_dev *nsim_bus_dev = ns->nsim_bus_dev;
+	struct nsim_dev *nsim_dev = ns->nsim_dev;
 
-	if (vf >= nsim_bus_dev->num_vfs)
+	if (vf >= nsim_dev_get_vfs(nsim_dev))
 		return -EINVAL;
-	nsim_bus_dev->vfconfigs[vf].trusted = val;
+	nsim_dev->vfconfigs[vf].trusted = val;
 
 	return 0;
 }
@@ -167,22 +167,22 @@ static int
 nsim_get_vf_config(struct net_device *dev, int vf, struct ifla_vf_info *ivi)
 {
 	struct netdevsim *ns = netdev_priv(dev);
-	struct nsim_bus_dev *nsim_bus_dev = ns->nsim_bus_dev;
+	struct nsim_dev *nsim_dev = ns->nsim_dev;
 
-	if (vf >= nsim_bus_dev->num_vfs)
+	if (vf >= nsim_dev_get_vfs(nsim_dev))
 		return -EINVAL;
 
 	ivi->vf = vf;
-	ivi->linkstate = nsim_bus_dev->vfconfigs[vf].link_state;
-	ivi->min_tx_rate = nsim_bus_dev->vfconfigs[vf].min_tx_rate;
-	ivi->max_tx_rate = nsim_bus_dev->vfconfigs[vf].max_tx_rate;
-	ivi->vlan = nsim_bus_dev->vfconfigs[vf].vlan;
-	ivi->vlan_proto = nsim_bus_dev->vfconfigs[vf].vlan_proto;
-	ivi->qos = nsim_bus_dev->vfconfigs[vf].qos;
-	memcpy(&ivi->mac, nsim_bus_dev->vfconfigs[vf].vf_mac, ETH_ALEN);
-	ivi->spoofchk = nsim_bus_dev->vfconfigs[vf].spoofchk_enabled;
-	ivi->trusted = nsim_bus_dev->vfconfigs[vf].trusted;
-	ivi->rss_query_en = nsim_bus_dev->vfconfigs[vf].rss_query_enabled;
+	ivi->linkstate = nsim_dev->vfconfigs[vf].link_state;
+	ivi->min_tx_rate = nsim_dev->vfconfigs[vf].min_tx_rate;
+	ivi->max_tx_rate = nsim_dev->vfconfigs[vf].max_tx_rate;
+	ivi->vlan = nsim_dev->vfconfigs[vf].vlan;
+	ivi->vlan_proto = nsim_dev->vfconfigs[vf].vlan_proto;
+	ivi->qos = nsim_dev->vfconfigs[vf].qos;
+	memcpy(&ivi->mac, nsim_dev->vfconfigs[vf].vf_mac, ETH_ALEN);
+	ivi->spoofchk = nsim_dev->vfconfigs[vf].spoofchk_enabled;
+	ivi->trusted = nsim_dev->vfconfigs[vf].trusted;
+	ivi->rss_query_en = nsim_dev->vfconfigs[vf].rss_query_enabled;
 
 	return 0;
 }
@@ -190,9 +190,9 @@ nsim_get_vf_config(struct net_device *dev, int vf, struct ifla_vf_info *ivi)
 static int nsim_set_vf_link_state(struct net_device *dev, int vf, int state)
 {
 	struct netdevsim *ns = netdev_priv(dev);
-	struct nsim_bus_dev *nsim_bus_dev = ns->nsim_bus_dev;
+	struct nsim_dev *nsim_dev = ns->nsim_dev;
 
-	if (vf >= nsim_bus_dev->num_vfs)
+	if (vf >= nsim_dev_get_vfs(nsim_dev))
 		return -EINVAL;
 
 	switch (state) {
@@ -204,7 +204,7 @@ static int nsim_set_vf_link_state(struct net_device *dev, int vf, int state)
 		return -EINVAL;
 	}
 
-	nsim_bus_dev->vfconfigs[vf].link_state = state;
+	nsim_dev->vfconfigs[vf].link_state = state;
 
 	return 0;
 }
diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h
index ec9939fba534..b4b287cdfe77 100644
--- a/drivers/net/netdevsim/netdevsim.h
+++ b/drivers/net/netdevsim/netdevsim.h
@@ -217,6 +217,19 @@ struct nsim_dev_port {
 	struct netdevsim *ns;
 };
 
+struct nsim_vf_config {
+	int link_state;
+	u16 min_tx_rate;
+	u16 max_tx_rate;
+	u16 vlan;
+	__be16 vlan_proto;
+	u16 qos;
+	u8 vf_mac[ETH_ALEN];
+	bool spoofchk_enabled;
+	bool trusted;
+	bool rss_query_enabled;
+};
+
 struct nsim_dev {
 	struct nsim_bus_dev *nsim_bus_dev;
 	struct nsim_fib_data *fib_data;
@@ -225,6 +238,10 @@ struct nsim_dev {
 	struct dentry *ports_ddir;
 	struct dentry *take_snapshot;
 	struct dentry *nodes_ddir;
+
+	struct mutex vfs_lock;  /* Protects vfconfigs */
+	struct nsim_vf_config *vfconfigs;
+
 	struct bpf_offload_dev *bpf_dev;
 	bool bpf_bind_accept;
 	bool bpf_bind_verifier_accept;
@@ -293,6 +310,8 @@ int nsim_dev_port_del(struct nsim_bus_dev *nsim_bus_dev,
 		      enum nsim_dev_port_type type,
 		      unsigned int port_index);
 
+unsigned int nsim_dev_get_vfs(struct nsim_dev *nsim_dev);
+
 struct nsim_fib_data *nsim_fib_create(struct devlink *devlink,
 				      struct netlink_ext_ack *extack);
 void nsim_fib_destroy(struct devlink *devlink, struct nsim_fib_data *fib_data);
@@ -335,19 +354,6 @@ static inline bool nsim_ipsec_tx(struct netdevsim *ns, struct sk_buff *skb)
 }
 #endif
 
-struct nsim_vf_config {
-	int link_state;
-	u16 min_tx_rate;
-	u16 max_tx_rate;
-	u16 vlan;
-	__be16 vlan_proto;
-	u16 qos;
-	u8 vf_mac[ETH_ALEN];
-	bool spoofchk_enabled;
-	bool trusted;
-	bool rss_query_enabled;
-};
-
 struct nsim_bus_dev {
 	struct device dev;
 	struct list_head list;
@@ -358,8 +364,6 @@ struct nsim_bus_dev {
 				  */
 	unsigned int max_vfs;
 	unsigned int num_vfs;
-	struct mutex vfs_lock;  /* Protects vfconfigs */
-	struct nsim_vf_config *vfconfigs;
 	/* Lock for devlink->reload_enabled in netdevsim module */
 	struct mutex nsim_bus_reload_lock;
 	bool in_reload;
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH net-next v2 3/5] netdevsim: move details of vf config to dev
  2021-10-30 23:15 [PATCH net-next v2 0/5] netdevsim: improve separation between device and bus Jakub Kicinski
  2021-10-30 23:15 ` [PATCH net-next v2 1/5] netdevsim: take rtnl_lock when assigning num_vfs Jakub Kicinski
  2021-10-30 23:15 ` [PATCH net-next v2 2/5] netdevsim: move vfconfig to nsim_dev Jakub Kicinski
@ 2021-10-30 23:15 ` Jakub Kicinski
  2021-10-30 23:15 ` [PATCH net-next v2 4/5] netdevsim: move max " Jakub Kicinski
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Jakub Kicinski @ 2021-10-30 23:15 UTC (permalink / raw)
  To: davem; +Cc: netdev, Jakub Kicinski

Since "eswitch" configuration was added bus.c contains
a lot of device details which really belong to dev.c.

Restructure the code while moving it.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 drivers/net/netdevsim/bus.c       | 69 ++-----------------------------
 drivers/net/netdevsim/dev.c       | 61 +++++++++++++++++++++++++--
 drivers/net/netdevsim/netdevsim.h |  6 +--
 3 files changed, 63 insertions(+), 73 deletions(-)

diff --git a/drivers/net/netdevsim/bus.c b/drivers/net/netdevsim/bus.c
index 1e7df184419d..d037600c0f0c 100644
--- a/drivers/net/netdevsim/bus.c
+++ b/drivers/net/netdevsim/bus.c
@@ -8,7 +8,6 @@
 #include <linux/kernel.h>
 #include <linux/list.h>
 #include <linux/mutex.h>
-#include <linux/rtnetlink.h>
 #include <linux/slab.h>
 #include <linux/sysfs.h>
 
@@ -24,50 +23,11 @@ static struct nsim_bus_dev *to_nsim_bus_dev(struct device *dev)
 	return container_of(dev, struct nsim_bus_dev, dev);
 }
 
-static void
-nsim_bus_dev_set_vfs(struct nsim_bus_dev *nsim_bus_dev, unsigned int num_vfs)
-{
-	rtnl_lock();
-	nsim_bus_dev->num_vfs = num_vfs;
-	rtnl_unlock();
-}
-
-static int nsim_bus_dev_vfs_enable(struct nsim_bus_dev *nsim_bus_dev,
-				   unsigned int num_vfs)
-{
-	struct nsim_dev *nsim_dev;
-	int err = 0;
-
-	if (nsim_bus_dev->max_vfs < num_vfs)
-		return -ENOMEM;
-	nsim_bus_dev_set_vfs(nsim_bus_dev, num_vfs);
-
-	nsim_dev = dev_get_drvdata(&nsim_bus_dev->dev);
-	if (nsim_esw_mode_is_switchdev(nsim_dev)) {
-		err = nsim_esw_switchdev_enable(nsim_dev, NULL);
-		if (err)
-			nsim_bus_dev_set_vfs(nsim_bus_dev, 0);
-	}
-
-	return err;
-}
-
-void nsim_bus_dev_vfs_disable(struct nsim_bus_dev *nsim_bus_dev)
-{
-	struct nsim_dev *nsim_dev;
-
-	nsim_bus_dev_set_vfs(nsim_bus_dev, 0);
-	nsim_dev = dev_get_drvdata(&nsim_bus_dev->dev);
-	if (nsim_esw_mode_is_switchdev(nsim_dev))
-		nsim_esw_legacy_enable(nsim_dev, NULL);
-}
-
 static ssize_t
 nsim_bus_dev_numvfs_store(struct device *dev, struct device_attribute *attr,
 			  const char *buf, size_t count)
 {
 	struct nsim_bus_dev *nsim_bus_dev = to_nsim_bus_dev(dev);
-	struct nsim_dev *nsim_dev = dev_get_drvdata(dev);
 	unsigned int num_vfs;
 	int ret;
 
@@ -76,33 +36,12 @@ nsim_bus_dev_numvfs_store(struct device *dev, struct device_attribute *attr,
 		return ret;
 
 	device_lock(dev);
-	if (!nsim_dev) {
-		ret = -ENOENT;
-		goto exit_unlock;
-	}
-
-	mutex_lock(&nsim_dev->vfs_lock);
-	if (nsim_bus_dev->num_vfs == num_vfs)
-		goto exit_good;
-	if (nsim_bus_dev->num_vfs && num_vfs) {
-		ret = -EBUSY;
-		goto exit_unlock;
-	}
-
-	if (num_vfs) {
-		ret = nsim_bus_dev_vfs_enable(nsim_bus_dev, num_vfs);
-		if (ret)
-			goto exit_unlock;
-	} else {
-		nsim_bus_dev_vfs_disable(nsim_bus_dev);
-	}
-exit_good:
-	ret = count;
-exit_unlock:
-	mutex_unlock(&nsim_dev->vfs_lock);
+	ret = -ENOENT;
+	if (dev_get_drvdata(dev))
+		ret = nsim_drv_configure_vfs(nsim_bus_dev, num_vfs);
 	device_unlock(dev);
 
-	return ret;
+	return ret ? ret : count;
 }
 
 static ssize_t
diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index 8157d28b32e4..c19f36c9e0a1 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -64,6 +64,14 @@ unsigned int nsim_dev_get_vfs(struct nsim_dev *nsim_dev)
 	return nsim_dev->nsim_bus_dev->num_vfs;
 }
 
+static void
+nsim_bus_dev_set_vfs(struct nsim_bus_dev *nsim_bus_dev, unsigned int num_vfs)
+{
+	rtnl_lock();
+	nsim_bus_dev->num_vfs = num_vfs;
+	rtnl_unlock();
+}
+
 #define NSIM_DEV_DUMMY_REGION_SIZE (1024 * 32)
 
 static int
@@ -496,7 +504,9 @@ static void nsim_dev_dummy_region_exit(struct nsim_dev *nsim_dev)
 }
 
 static void __nsim_dev_port_del(struct nsim_dev_port *nsim_dev_port);
-int nsim_esw_legacy_enable(struct nsim_dev *nsim_dev, struct netlink_ext_ack *extack)
+
+static int nsim_esw_legacy_enable(struct nsim_dev *nsim_dev,
+				  struct netlink_ext_ack *extack)
 {
 	struct devlink *devlink = priv_to_devlink(nsim_dev);
 	struct nsim_dev_port *nsim_dev_port, *tmp;
@@ -511,7 +521,8 @@ int nsim_esw_legacy_enable(struct nsim_dev *nsim_dev, struct netlink_ext_ack *ex
 	return 0;
 }
 
-int nsim_esw_switchdev_enable(struct nsim_dev *nsim_dev, struct netlink_ext_ack *extack)
+static int nsim_esw_switchdev_enable(struct nsim_dev *nsim_dev,
+				     struct netlink_ext_ack *extack)
 {
 	struct nsim_bus_dev *nsim_bus_dev = nsim_dev->nsim_bus_dev;
 	int i, err;
@@ -1565,8 +1576,11 @@ static void nsim_dev_reload_destroy(struct nsim_dev *nsim_dev)
 	debugfs_remove(nsim_dev->take_snapshot);
 
 	mutex_lock(&nsim_dev->vfs_lock);
-	if (nsim_dev_get_vfs(nsim_dev))
-		nsim_bus_dev_vfs_disable(nsim_dev->nsim_bus_dev);
+	if (nsim_dev_get_vfs(nsim_dev)) {
+		nsim_bus_dev_set_vfs(nsim_dev->nsim_bus_dev, 0);
+		if (nsim_esw_mode_is_switchdev(nsim_dev))
+			nsim_esw_legacy_enable(nsim_dev, NULL);
+	}
 	mutex_unlock(&nsim_dev->vfs_lock);
 
 	nsim_dev_port_del_all(nsim_dev);
@@ -1641,6 +1655,45 @@ int nsim_dev_port_del(struct nsim_bus_dev *nsim_bus_dev, enum nsim_dev_port_type
 	return err;
 }
 
+int nsim_drv_configure_vfs(struct nsim_bus_dev *nsim_bus_dev,
+			   unsigned int num_vfs)
+{
+	struct nsim_dev *nsim_dev = dev_get_drvdata(&nsim_bus_dev->dev);
+	int ret;
+
+	mutex_lock(&nsim_dev->vfs_lock);
+	if (nsim_bus_dev->num_vfs == num_vfs) {
+		ret = 0;
+		goto exit_unlock;
+	}
+	if (nsim_bus_dev->num_vfs && num_vfs) {
+		ret = -EBUSY;
+		goto exit_unlock;
+	}
+	if (nsim_bus_dev->max_vfs < num_vfs) {
+		ret = -ENOMEM;
+		goto exit_unlock;
+	}
+
+	nsim_bus_dev_set_vfs(nsim_bus_dev, num_vfs);
+	if (nsim_esw_mode_is_switchdev(nsim_dev)) {
+		if (num_vfs) {
+			ret = nsim_esw_switchdev_enable(nsim_dev, NULL);
+			if (ret) {
+				nsim_bus_dev_set_vfs(nsim_bus_dev, 0);
+				goto exit_unlock;
+			}
+		} else {
+			nsim_esw_legacy_enable(nsim_dev, NULL);
+		}
+	}
+
+exit_unlock:
+	mutex_unlock(&nsim_dev->vfs_lock);
+
+	return ret;
+}
+
 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 b4b287cdfe77..8da5f82e5cfc 100644
--- a/drivers/net/netdevsim/netdevsim.h
+++ b/drivers/net/netdevsim/netdevsim.h
@@ -281,9 +281,6 @@ struct nsim_dev {
 	u16 esw_mode;
 };
 
-int nsim_esw_legacy_enable(struct nsim_dev *nsim_dev, struct netlink_ext_ack *extack);
-int nsim_esw_switchdev_enable(struct nsim_dev *nsim_dev, struct netlink_ext_ack *extack);
-
 static inline bool nsim_esw_mode_is_legacy(struct nsim_dev *nsim_dev)
 {
 	return nsim_dev->esw_mode == DEVLINK_ESWITCH_MODE_LEGACY;
@@ -309,6 +306,8 @@ int nsim_dev_port_add(struct nsim_bus_dev *nsim_bus_dev,
 int nsim_dev_port_del(struct nsim_bus_dev *nsim_bus_dev,
 		      enum nsim_dev_port_type type,
 		      unsigned int port_index);
+int nsim_drv_configure_vfs(struct nsim_bus_dev *nsim_bus_dev,
+			   unsigned int num_vfs);
 
 unsigned int nsim_dev_get_vfs(struct nsim_dev *nsim_dev);
 
@@ -324,7 +323,6 @@ ssize_t nsim_bus_dev_max_vfs_read(struct file *file,
 ssize_t nsim_bus_dev_max_vfs_write(struct file *file,
 				   const char __user *data,
 				   size_t count, loff_t *ppos);
-void nsim_bus_dev_vfs_disable(struct nsim_bus_dev *nsim_bus_dev);
 
 static inline bool nsim_dev_port_is_pf(struct nsim_dev_port *nsim_dev_port)
 {
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH net-next v2 4/5] netdevsim: move max vf config to dev
  2021-10-30 23:15 [PATCH net-next v2 0/5] netdevsim: improve separation between device and bus Jakub Kicinski
                   ` (2 preceding siblings ...)
  2021-10-30 23:15 ` [PATCH net-next v2 3/5] netdevsim: move details of vf config to dev Jakub Kicinski
@ 2021-10-30 23:15 ` Jakub Kicinski
  2021-10-30 23:15 ` [PATCH net-next v2 5/5] netdevsim: rename 'driver' entry points Jakub Kicinski
  2021-11-01 13:40 ` [PATCH net-next v2 0/5] netdevsim: improve separation between device and bus patchwork-bot+netdevbpf
  5 siblings, 0 replies; 7+ messages in thread
From: Jakub Kicinski @ 2021-10-30 23:15 UTC (permalink / raw)
  To: davem; +Cc: netdev, Jakub Kicinski

max_vfs is a strange little beast because the file
hangs off of nsim's debugfs, but it configures a field
in the bus device. Move it to dev.c, let's look at it
as if the device driver was imposing VF limit based
on FW info (like pci_sriov_set_totalvfs()).

Again, when moving refactor the function not to hold
the vfs lock pointlessly while parsing the input.
Wrap the access from the read side in READ_ONCE()
to appease concurrency checkers. Do not check if
return value from snprintf() is negative...

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 drivers/net/netdevsim/bus.c       | 75 -------------------------------
 drivers/net/netdevsim/dev.c       | 64 ++++++++++++++++++++++++++
 drivers/net/netdevsim/netdevsim.h |  7 ---
 3 files changed, 64 insertions(+), 82 deletions(-)

diff --git a/drivers/net/netdevsim/bus.c b/drivers/net/netdevsim/bus.c
index d037600c0f0c..0b41f1625db9 100644
--- a/drivers/net/netdevsim/bus.c
+++ b/drivers/net/netdevsim/bus.c
@@ -57,81 +57,6 @@ static struct device_attribute nsim_bus_dev_numvfs_attr =
 	__ATTR(sriov_numvfs, 0664, nsim_bus_dev_numvfs_show,
 	       nsim_bus_dev_numvfs_store);
 
-ssize_t nsim_bus_dev_max_vfs_read(struct file *file,
-				  char __user *data,
-				  size_t count, loff_t *ppos)
-{
-	struct nsim_dev *nsim_dev = file->private_data;
-	struct nsim_bus_dev *nsim_bus_dev = nsim_dev->nsim_bus_dev;
-	char buf[11];
-	ssize_t len;
-
-	len = snprintf(buf, sizeof(buf), "%u\n", nsim_bus_dev->max_vfs);
-	if (len < 0)
-		return len;
-
-	return simple_read_from_buffer(data, count, ppos, buf, len);
-}
-
-ssize_t nsim_bus_dev_max_vfs_write(struct file *file,
-				   const char __user *data,
-				   size_t count, loff_t *ppos)
-{
-	struct nsim_dev *nsim_dev = file->private_data;
-	struct nsim_bus_dev *nsim_bus_dev = nsim_dev->nsim_bus_dev;
-	struct nsim_vf_config *vfconfigs;
-	ssize_t ret;
-	char buf[10];
-	u32 val;
-
-	if (*ppos != 0)
-		return 0;
-
-	if (count >= sizeof(buf))
-		return -ENOSPC;
-
-	mutex_lock(&nsim_dev->vfs_lock);
-	/* Reject if VFs are configured */
-	if (nsim_bus_dev->num_vfs) {
-		ret = -EBUSY;
-		goto unlock;
-	}
-
-	ret = copy_from_user(buf, data, count);
-	if (ret) {
-		ret = -EFAULT;
-		goto unlock;
-	}
-
-	buf[count] = '\0';
-	ret = kstrtouint(buf, 10, &val);
-	if (ret) {
-		ret = -EIO;
-		goto unlock;
-	}
-
-	/* max_vfs limited by the maximum number of provided port indexes */
-	if (val > NSIM_DEV_VF_PORT_INDEX_MAX - NSIM_DEV_VF_PORT_INDEX_BASE) {
-		ret = -ERANGE;
-		goto unlock;
-	}
-
-	vfconfigs = kcalloc(val, sizeof(struct nsim_vf_config), GFP_KERNEL | __GFP_NOWARN);
-	if (!vfconfigs) {
-		ret = -ENOMEM;
-		goto unlock;
-	}
-
-	kfree(nsim_dev->vfconfigs);
-	nsim_dev->vfconfigs = vfconfigs;
-	nsim_bus_dev->max_vfs = val;
-	*ppos += count;
-	ret = count;
-unlock:
-	mutex_unlock(&nsim_dev->vfs_lock);
-	return ret;
-}
-
 static ssize_t
 new_port_store(struct device *dev, struct device_attribute *attr,
 	       const char *buf, size_t count)
diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index c19f36c9e0a1..040531fe0879 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -227,6 +227,70 @@ static const struct file_operations nsim_dev_trap_fa_cookie_fops = {
 	.owner = THIS_MODULE,
 };
 
+static ssize_t nsim_bus_dev_max_vfs_read(struct file *file, char __user *data,
+					 size_t count, loff_t *ppos)
+{
+	struct nsim_dev *nsim_dev = file->private_data;
+	char buf[11];
+	ssize_t len;
+
+	len = scnprintf(buf, sizeof(buf), "%u\n",
+			READ_ONCE(nsim_dev->nsim_bus_dev->max_vfs));
+
+	return simple_read_from_buffer(data, count, ppos, buf, len);
+}
+
+static ssize_t nsim_bus_dev_max_vfs_write(struct file *file,
+					  const char __user *data,
+					  size_t count, loff_t *ppos)
+{
+	struct nsim_vf_config *vfconfigs;
+	struct nsim_dev *nsim_dev;
+	char buf[10];
+	ssize_t ret;
+	u32 val;
+
+	if (*ppos != 0)
+		return 0;
+
+	if (count >= sizeof(buf))
+		return -ENOSPC;
+
+	ret = copy_from_user(buf, data, count);
+	if (ret)
+		return -EFAULT;
+	buf[count] = '\0';
+
+	ret = kstrtouint(buf, 10, &val);
+	if (ret)
+		return -EINVAL;
+
+	/* max_vfs limited by the maximum number of provided port indexes */
+	if (val > NSIM_DEV_VF_PORT_INDEX_MAX - NSIM_DEV_VF_PORT_INDEX_BASE)
+		return -ERANGE;
+
+	vfconfigs = kcalloc(val, sizeof(struct nsim_vf_config),
+			    GFP_KERNEL | __GFP_NOWARN);
+	if (!vfconfigs)
+		return -ENOMEM;
+
+	nsim_dev = file->private_data;
+	mutex_lock(&nsim_dev->vfs_lock);
+	/* Reject if VFs are configured */
+	if (nsim_dev_get_vfs(nsim_dev)) {
+		ret = -EBUSY;
+	} else {
+		swap(nsim_dev->vfconfigs, vfconfigs);
+		WRITE_ONCE(nsim_dev->nsim_bus_dev->max_vfs, val);
+		*ppos += count;
+		ret = count;
+	}
+	mutex_unlock(&nsim_dev->vfs_lock);
+
+	kfree(vfconfigs);
+	return ret;
+}
+
 static const struct file_operations nsim_dev_max_vfs_fops = {
 	.open = simple_open,
 	.read = nsim_bus_dev_max_vfs_read,
diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h
index 8da5f82e5cfc..fd7133407f05 100644
--- a/drivers/net/netdevsim/netdevsim.h
+++ b/drivers/net/netdevsim/netdevsim.h
@@ -317,13 +317,6 @@ void nsim_fib_destroy(struct devlink *devlink, struct nsim_fib_data *fib_data);
 u64 nsim_fib_get_val(struct nsim_fib_data *fib_data,
 		     enum nsim_resource_id res_id, bool max);
 
-ssize_t nsim_bus_dev_max_vfs_read(struct file *file,
-				  char __user *data,
-				  size_t count, loff_t *ppos);
-ssize_t nsim_bus_dev_max_vfs_write(struct file *file,
-				   const char __user *data,
-				   size_t count, loff_t *ppos);
-
 static inline bool nsim_dev_port_is_pf(struct nsim_dev_port *nsim_dev_port)
 {
 	return nsim_dev_port->port_type == NSIM_DEV_PORT_TYPE_PF;
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH net-next v2 5/5] netdevsim: rename 'driver' entry points
  2021-10-30 23:15 [PATCH net-next v2 0/5] netdevsim: improve separation between device and bus Jakub Kicinski
                   ` (3 preceding siblings ...)
  2021-10-30 23:15 ` [PATCH net-next v2 4/5] netdevsim: move max " Jakub Kicinski
@ 2021-10-30 23:15 ` Jakub Kicinski
  2021-11-01 13:40 ` [PATCH net-next v2 0/5] netdevsim: improve separation between device and bus patchwork-bot+netdevbpf
  5 siblings, 0 replies; 7+ messages in thread
From: Jakub Kicinski @ 2021-10-30 23:15 UTC (permalink / raw)
  To: davem; +Cc: netdev, Jakub Kicinski

Rename functions serving as driver entry points
from nsim_dev_... to nsim_drv_... this makes the
API boundary between bus and dev clearer.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 drivers/net/netdevsim/bus.c       |  8 ++++----
 drivers/net/netdevsim/dev.c       | 12 ++++++------
 drivers/net/netdevsim/netdevsim.h |  8 ++++----
 3 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/net/netdevsim/bus.c b/drivers/net/netdevsim/bus.c
index 0b41f1625db9..25cb2e600d53 100644
--- a/drivers/net/netdevsim/bus.c
+++ b/drivers/net/netdevsim/bus.c
@@ -80,7 +80,7 @@ new_port_store(struct device *dev, struct device_attribute *attr,
 		return -EBUSY;
 	}
 
-	ret = nsim_dev_port_add(nsim_bus_dev, NSIM_DEV_PORT_TYPE_PF, port_index);
+	ret = nsim_drv_port_add(nsim_bus_dev, NSIM_DEV_PORT_TYPE_PF, port_index);
 	mutex_unlock(&nsim_bus_dev->nsim_bus_reload_lock);
 	return ret ? ret : count;
 }
@@ -110,7 +110,7 @@ del_port_store(struct device *dev, struct device_attribute *attr,
 		return -EBUSY;
 	}
 
-	ret = nsim_dev_port_del(nsim_bus_dev, NSIM_DEV_PORT_TYPE_PF, port_index);
+	ret = nsim_drv_port_del(nsim_bus_dev, NSIM_DEV_PORT_TYPE_PF, port_index);
 	mutex_unlock(&nsim_bus_dev->nsim_bus_reload_lock);
 	return ret ? ret : count;
 }
@@ -250,14 +250,14 @@ static int nsim_bus_probe(struct device *dev)
 {
 	struct nsim_bus_dev *nsim_bus_dev = to_nsim_bus_dev(dev);
 
-	return nsim_dev_probe(nsim_bus_dev);
+	return nsim_drv_probe(nsim_bus_dev);
 }
 
 static void nsim_bus_remove(struct device *dev)
 {
 	struct nsim_bus_dev *nsim_bus_dev = to_nsim_bus_dev(dev);
 
-	nsim_dev_remove(nsim_bus_dev);
+	nsim_drv_remove(nsim_bus_dev);
 }
 
 static int nsim_num_vf(struct device *dev)
diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index 040531fe0879..5db40d713d2a 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -592,7 +592,7 @@ static int nsim_esw_switchdev_enable(struct nsim_dev *nsim_dev,
 	int i, err;
 
 	for (i = 0; i < nsim_dev_get_vfs(nsim_dev); i++) {
-		err = nsim_dev_port_add(nsim_bus_dev, NSIM_DEV_PORT_TYPE_VF, i);
+		err = nsim_drv_port_add(nsim_bus_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);
@@ -604,7 +604,7 @@ static int nsim_esw_switchdev_enable(struct nsim_dev *nsim_dev,
 
 err_port_add_vfs:
 	for (i--; i >= 0; i--)
-		nsim_dev_port_del(nsim_bus_dev, NSIM_DEV_PORT_TYPE_VF, i);
+		nsim_drv_port_del(nsim_bus_dev, NSIM_DEV_PORT_TYPE_VF, i);
 	return err;
 }
 
@@ -1522,7 +1522,7 @@ static int nsim_dev_reload_create(struct nsim_dev *nsim_dev,
 	return err;
 }
 
-int nsim_dev_probe(struct nsim_bus_dev *nsim_bus_dev)
+int nsim_drv_probe(struct nsim_bus_dev *nsim_bus_dev)
 {
 	struct nsim_dev *nsim_dev;
 	struct devlink *devlink;
@@ -1656,7 +1656,7 @@ static void nsim_dev_reload_destroy(struct nsim_dev *nsim_dev)
 	mutex_destroy(&nsim_dev->port_list_lock);
 }
 
-void nsim_dev_remove(struct nsim_bus_dev *nsim_bus_dev)
+void nsim_drv_remove(struct nsim_bus_dev *nsim_bus_dev)
 {
 	struct nsim_dev *nsim_dev = dev_get_drvdata(&nsim_bus_dev->dev);
 	struct devlink *devlink = priv_to_devlink(nsim_dev);
@@ -1687,7 +1687,7 @@ __nsim_dev_port_lookup(struct nsim_dev *nsim_dev, enum nsim_dev_port_type type,
 	return NULL;
 }
 
-int nsim_dev_port_add(struct nsim_bus_dev *nsim_bus_dev, enum nsim_dev_port_type type,
+int nsim_drv_port_add(struct nsim_bus_dev *nsim_bus_dev, enum nsim_dev_port_type type,
 		      unsigned int port_index)
 {
 	struct nsim_dev *nsim_dev = dev_get_drvdata(&nsim_bus_dev->dev);
@@ -1702,7 +1702,7 @@ int nsim_dev_port_add(struct nsim_bus_dev *nsim_bus_dev, enum nsim_dev_port_type
 	return err;
 }
 
-int nsim_dev_port_del(struct nsim_bus_dev *nsim_bus_dev, enum nsim_dev_port_type type,
+int nsim_drv_port_del(struct nsim_bus_dev *nsim_bus_dev, enum nsim_dev_port_type type,
 		      unsigned int port_index)
 {
 	struct nsim_dev *nsim_dev = dev_get_drvdata(&nsim_bus_dev->dev);
diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h
index fd7133407f05..c49771f27f17 100644
--- a/drivers/net/netdevsim/netdevsim.h
+++ b/drivers/net/netdevsim/netdevsim.h
@@ -298,12 +298,12 @@ static inline struct net *nsim_dev_net(struct nsim_dev *nsim_dev)
 
 int nsim_dev_init(void);
 void nsim_dev_exit(void);
-int nsim_dev_probe(struct nsim_bus_dev *nsim_bus_dev);
-void nsim_dev_remove(struct nsim_bus_dev *nsim_bus_dev);
-int nsim_dev_port_add(struct nsim_bus_dev *nsim_bus_dev,
+int nsim_drv_probe(struct nsim_bus_dev *nsim_bus_dev);
+void nsim_drv_remove(struct nsim_bus_dev *nsim_bus_dev);
+int nsim_drv_port_add(struct nsim_bus_dev *nsim_bus_dev,
 		      enum nsim_dev_port_type type,
 		      unsigned int port_index);
-int nsim_dev_port_del(struct nsim_bus_dev *nsim_bus_dev,
+int nsim_drv_port_del(struct nsim_bus_dev *nsim_bus_dev,
 		      enum nsim_dev_port_type type,
 		      unsigned int port_index);
 int nsim_drv_configure_vfs(struct nsim_bus_dev *nsim_bus_dev,
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH net-next v2 0/5] netdevsim: improve separation between device and bus
  2021-10-30 23:15 [PATCH net-next v2 0/5] netdevsim: improve separation between device and bus Jakub Kicinski
                   ` (4 preceding siblings ...)
  2021-10-30 23:15 ` [PATCH net-next v2 5/5] netdevsim: rename 'driver' entry points Jakub Kicinski
@ 2021-11-01 13:40 ` patchwork-bot+netdevbpf
  5 siblings, 0 replies; 7+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-11-01 13:40 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: davem, netdev

Hello:

This series was applied to netdev/net-next.git (master)
by David S. Miller <davem@davemloft.net>:

On Sat, 30 Oct 2021 16:15:00 -0700 you wrote:
> VF config falls strangely in between device and bus
> responsibilities today. Because of this bus.c sticks fingers
> directly into struct nsim_dev and we look at nsim_bus_dev
> in many more places than necessary.
> 
> Make bus.c contain pure interface code, and move
> the particulars of the logic (which touch on eswitch,
> devlink reloads etc) to dev.c. Rename the functions
> at the boundary of the interface to make the separation
> clearer.
> 
> [...]

Here is the summary with links:
  - [net-next,v2,1/5] netdevsim: take rtnl_lock when assigning num_vfs
    https://git.kernel.org/netdev/net-next/c/26c37d89f61d
  - [net-next,v2,2/5] netdevsim: move vfconfig to nsim_dev
    https://git.kernel.org/netdev/net-next/c/5e388f3dc38c
  - [net-next,v2,3/5] netdevsim: move details of vf config to dev
    https://git.kernel.org/netdev/net-next/c/1c401078bcf3
  - [net-next,v2,4/5] netdevsim: move max vf config to dev
    https://git.kernel.org/netdev/net-next/c/a3353ec32554
  - [net-next,v2,5/5] netdevsim: rename 'driver' entry points
    https://git.kernel.org/netdev/net-next/c/a66f64b80815

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-11-01 13:40 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-30 23:15 [PATCH net-next v2 0/5] netdevsim: improve separation between device and bus Jakub Kicinski
2021-10-30 23:15 ` [PATCH net-next v2 1/5] netdevsim: take rtnl_lock when assigning num_vfs Jakub Kicinski
2021-10-30 23:15 ` [PATCH net-next v2 2/5] netdevsim: move vfconfig to nsim_dev Jakub Kicinski
2021-10-30 23:15 ` [PATCH net-next v2 3/5] netdevsim: move details of vf config to dev Jakub Kicinski
2021-10-30 23:15 ` [PATCH net-next v2 4/5] netdevsim: move max " Jakub Kicinski
2021-10-30 23:15 ` [PATCH net-next v2 5/5] netdevsim: rename 'driver' entry points Jakub Kicinski
2021-11-01 13:40 ` [PATCH net-next v2 0/5] netdevsim: improve separation between device and bus patchwork-bot+netdevbpf

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.