Merge branch 'netdevsim-device-and-bus'

Jakub Kicinski says:

====================
netdevsim: improve separation between device and bus

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.
====================
Signed-off-by: David S. Miller <davem@davemloft.net>

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,39 +23,6 @@ static struct nsim_bus_dev *to_nsim_bus_dev(struct device *dev)
 	return container_of(dev, struct nsim_bus_dev, dev);
 }
 
-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;
-
-	if (!nsim_bus_dev->vfconfigs)
-		return -ENOMEM;
-	nsim_bus_dev->num_vfs = 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;
-	}
-
-	return err;
-}
-
-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_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)
@@ -69,27 +35,13 @@ nsim_bus_dev_numvfs_store(struct device *dev, struct device_attribute *attr,
 	if (ret)
 		return ret;
 
-	mutex_lock(&nsim_bus_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_bus_dev->vfs_lock);
+	device_lock(dev);
+	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
@@ -105,79 +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_bus_dev *nsim_bus_dev = file->private_data;
-	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_bus_dev *nsim_bus_dev = file->private_data;
-	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_bus_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_bus_dev->vfconfigs);
-	nsim_bus_dev->vfconfigs = vfconfigs;
-	nsim_bus_dev->max_vfs = val;
-	*ppos += count;
-	ret = count;
-unlock:
-	mutex_unlock(&nsim_bus_dev->vfs_lock);
-	return ret;
-}
-
 static ssize_t
 new_port_store(struct device *dev, struct device_attribute *attr,
 	       const char *buf, size_t count)
@@ -201,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;
 }
@@ -231,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;
 }
@@ -371,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)
@@ -420,26 +299,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:
@@ -453,7 +321,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);
 }
 

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;
 }

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;
@@ -264,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;
@@ -284,14 +298,18 @@ 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,
+			   unsigned int num_vfs);
+
+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);
@@ -299,14 +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);
-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)
 {
 	return nsim_dev_port->port_type == NSIM_DEV_PORT_TYPE_PF;
@@ -335,19 +345,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 +355,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;