| Message ID | 20231220014747.1508581-3-dw@davidwei.uk (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | netdevsim: link and forward skbs between ports | expand |
Wed, Dec 20, 2023 at 02:47:44AM CET, dw@davidwei.uk wrote: >Add a debugfs file in >/sys/kernel/debug/netdevsim/netdevsimN/ports/A/peer > >Writing "M B" to this file will link port A of netdevsim N with port B >of netdevsim M. Reading this file will return the linked netdevsim id >and port, if any. > >nsim_dev_list_lock and rtnl_lock are held during read/write of peer to >prevent concurrent modification of netdevsims or their ports. > >Signed-off-by: David Wei <dw@davidwei.uk> >--- > drivers/net/netdevsim/dev.c | 127 +++++++++++++++++++++++++++--- > drivers/net/netdevsim/netdev.c | 6 ++ > drivers/net/netdevsim/netdevsim.h | 1 + > 3 files changed, 121 insertions(+), 13 deletions(-) > >diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c >index e30a12130e07..e4621861c70b 100644 >--- a/drivers/net/netdevsim/dev.c >+++ b/drivers/net/netdevsim/dev.c >@@ -391,6 +391,117 @@ static const struct file_operations nsim_dev_rate_parent_fops = { > .owner = THIS_MODULE, > }; > >+static struct nsim_dev *nsim_dev_find_by_id(unsigned int id) >+{ >+ struct nsim_dev *dev; >+ >+ list_for_each_entry(dev, &nsim_dev_list, list) >+ if (dev->nsim_bus_dev->dev.id == id) >+ return dev; >+ >+ return NULL; >+} >+ >+static struct nsim_dev_port * >+__nsim_dev_port_lookup(struct nsim_dev *nsim_dev, enum nsim_dev_port_type type, >+ unsigned int port_index) >+{ >+ struct nsim_dev_port *nsim_dev_port; >+ >+ port_index = nsim_dev_port_index(type, port_index); >+ list_for_each_entry(nsim_dev_port, &nsim_dev->port_list, list) >+ if (nsim_dev_port->port_index == port_index) >+ return nsim_dev_port; >+ return NULL; >+} >+ >+static ssize_t nsim_dev_peer_read(struct file *file, char __user *data, >+ size_t count, loff_t *ppos) >+{ >+ struct nsim_dev_port *nsim_dev_port; >+ struct netdevsim *peer; >+ unsigned int id, port; >+ char buf[23]; >+ ssize_t ret; >+ >+ mutex_lock(&nsim_dev_list_lock); >+ rtnl_lock(); >+ nsim_dev_port = file->private_data; >+ peer = rtnl_dereference(nsim_dev_port->ns->peer); >+ if (!peer) >+ goto out; >+ >+ id = peer->nsim_bus_dev->dev.id; >+ port = peer->nsim_dev_port->port_index; >+ ret = scnprintf(buf, sizeof(buf), "%u %u\n", id, port); >+ ret = simple_read_from_buffer(data, count, ppos, buf, ret); >+ >+out: >+ rtnl_unlock(); >+ mutex_unlock(&nsim_dev_list_lock); >+ >+ return ret; >+} >+ >+static ssize_t nsim_dev_peer_write(struct file *file, >+ const char __user *data, >+ size_t count, loff_t *ppos) >+{ >+ struct nsim_dev_port *nsim_dev_port, *peer_dev_port; >+ struct nsim_dev *peer_dev; >+ unsigned int id, port; >+ char buf[22]; >+ ssize_t ret; >+ >+ if (count >= sizeof(buf)) >+ return -ENOSPC; >+ >+ ret = copy_from_user(buf, data, count); >+ if (ret) >+ return -EFAULT; >+ buf[count] = '\0'; >+ >+ ret = sscanf(buf, "%u %u", &id, &port); >+ if (ret != 2) { >+ pr_err("Format for adding a peer is \"id port\" (uint uint)"); >+ return -EINVAL; >+ } >+ >+ ret = -EINVAL; >+ mutex_lock(&nsim_dev_list_lock); >+ rtnl_lock(); >+ peer_dev = nsim_dev_find_by_id(id); >+ if (!peer_dev) >+ goto out; Tell the user what's wrong please. >+ >+ peer_dev_port = __nsim_dev_port_lookup(peer_dev, NSIM_DEV_PORT_TYPE_PF, >+ port); >+ if (!peer_dev_port) >+ goto out; Tell the user what's wrong please. >+ >+ nsim_dev_port = file->private_data; >+ if (nsim_dev_port == peer_dev_port) Why fail here? IDK, success sounds better to me. >+ goto out; >+ >+ rcu_assign_pointer(nsim_dev_port->ns->peer, peer_dev_port->ns); >+ rcu_assign_pointer(peer_dev_port->ns->peer, nsim_dev_port->ns); >+ ret = count; >+ >+out: >+ rtnl_unlock(); >+ mutex_unlock(&nsim_dev_list_lock); >+ >+ return ret; >+} >+ >+static const struct file_operations nsim_dev_peer_fops = { >+ .open = simple_open, >+ .read = nsim_dev_peer_read, >+ .write = nsim_dev_peer_write, >+ .llseek = generic_file_llseek, >+ .owner = THIS_MODULE, >+}; >+ > static int nsim_dev_port_debugfs_init(struct nsim_dev *nsim_dev, > struct nsim_dev_port *nsim_dev_port) > { >@@ -421,6 +532,9 @@ static int nsim_dev_port_debugfs_init(struct nsim_dev *nsim_dev, > } > debugfs_create_symlink("dev", nsim_dev_port->ddir, dev_link_name); > >+ debugfs_create_file("peer", 0600, nsim_dev_port->ddir, >+ nsim_dev_port, &nsim_dev_peer_fops); >+ > return 0; > } > >@@ -1702,19 +1816,6 @@ void nsim_drv_remove(struct nsim_bus_dev *nsim_bus_dev) > dev_set_drvdata(&nsim_bus_dev->dev, NULL); > } > >-static struct nsim_dev_port * >-__nsim_dev_port_lookup(struct nsim_dev *nsim_dev, enum nsim_dev_port_type type, >- unsigned int port_index) >-{ >- struct nsim_dev_port *nsim_dev_port; >- >- port_index = nsim_dev_port_index(type, port_index); >- list_for_each_entry(nsim_dev_port, &nsim_dev->port_list, list) >- if (nsim_dev_port->port_index == port_index) >- return nsim_dev_port; >- return NULL; >-} >- > int nsim_drv_port_add(struct nsim_bus_dev *nsim_bus_dev, enum nsim_dev_port_type type, > unsigned int port_index) > { >diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c >index aecaf5f44374..434322f6a565 100644 >--- a/drivers/net/netdevsim/netdev.c >+++ b/drivers/net/netdevsim/netdev.c >@@ -388,6 +388,7 @@ nsim_create(struct nsim_dev *nsim_dev, struct nsim_dev_port *nsim_dev_port) > ns->nsim_dev = nsim_dev; > ns->nsim_dev_port = nsim_dev_port; > ns->nsim_bus_dev = nsim_dev->nsim_bus_dev; >+ RCU_INIT_POINTER(ns->peer, NULL); > SET_NETDEV_DEV(dev, &ns->nsim_bus_dev->dev); > SET_NETDEV_DEVLINK_PORT(dev, &nsim_dev_port->devlink_port); > nsim_ethtool_init(ns); >@@ -407,8 +408,13 @@ nsim_create(struct nsim_dev *nsim_dev, struct nsim_dev_port *nsim_dev_port) > void nsim_destroy(struct netdevsim *ns) > { > struct net_device *dev = ns->netdev; >+ struct netdevsim *peer; > > rtnl_lock(); >+ peer = rtnl_dereference(ns->peer); >+ if (peer) >+ RCU_INIT_POINTER(peer->peer, NULL); >+ RCU_INIT_POINTER(ns->peer, NULL); > unregister_netdevice(dev); > if (nsim_dev_port_is_pf(ns->nsim_dev_port)) { > nsim_macsec_teardown(ns); >diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h >index babb61d7790b..24fc3fbda791 100644 >--- a/drivers/net/netdevsim/netdevsim.h >+++ b/drivers/net/netdevsim/netdevsim.h >@@ -125,6 +125,7 @@ struct netdevsim { > } udp_ports; > > struct nsim_ethtool ethtool; >+ struct netdevsim __rcu *peer; > }; > > struct netdevsim * >-- >2.39.3 >
Hi David, kernel test robot noticed the following build warnings: [auto build test WARNING on net-next/main] url: https://github.com/intel-lab-lkp/linux/commits/David-Wei/netdevsim-maintain-a-list-of-probed-netdevsims/20231220-095238 base: net-next/main patch link: https://lore.kernel.org/r/20231220014747.1508581-3-dw%40davidwei.uk patch subject: [PATCH net-next v4 2/5] netdevsim: allow two netdevsim ports to be connected config: arm64-allyesconfig (https://download.01.org/0day-ci/archive/20231220/202312202036.QxZ1fdee-lkp@intel.com/config) compiler: clang version 18.0.0git (https://github.com/llvm/llvm-project 10056c821a56a19cef732129e4e0c5883ae1ee49) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231220/202312202036.QxZ1fdee-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202312202036.QxZ1fdee-lkp@intel.com/ All warnings (new ones prefixed by >>): >> drivers/net/netdevsim/dev.c:431:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized] 431 | if (!peer) | ^~~~~ drivers/net/netdevsim/dev.c:443:9: note: uninitialized use occurs here 443 | return ret; | ^~~ drivers/net/netdevsim/dev.c:431:2: note: remove the 'if' if its condition is always false 431 | if (!peer) | ^~~~~~~~~~ 432 | goto out; | ~~~~~~~~ drivers/net/netdevsim/dev.c:425:13: note: initialize the variable 'ret' to silence this warning 425 | ssize_t ret; | ^ | = 0 1 warning generated. vim +431 drivers/net/netdevsim/dev.c 417 418 static ssize_t nsim_dev_peer_read(struct file *file, char __user *data, 419 size_t count, loff_t *ppos) 420 { 421 struct nsim_dev_port *nsim_dev_port; 422 struct netdevsim *peer; 423 unsigned int id, port; 424 char buf[23]; 425 ssize_t ret; 426 427 mutex_lock(&nsim_dev_list_lock); 428 rtnl_lock(); 429 nsim_dev_port = file->private_data; 430 peer = rtnl_dereference(nsim_dev_port->ns->peer); > 431 if (!peer) 432 goto out; 433 434 id = peer->nsim_bus_dev->dev.id; 435 port = peer->nsim_dev_port->port_index; 436 ret = scnprintf(buf, sizeof(buf), "%u %u\n", id, port); 437 ret = simple_read_from_buffer(data, count, ppos, buf, ret); 438 439 out: 440 rtnl_unlock(); 441 mutex_unlock(&nsim_dev_list_lock); 442 443 return ret; 444 } 445
On 2023-12-20 01:09, Jiri Pirko wrote: > Wed, Dec 20, 2023 at 02:47:44AM CET, dw@davidwei.uk wrote: >> Add a debugfs file in >> /sys/kernel/debug/netdevsim/netdevsimN/ports/A/peer >> >> Writing "M B" to this file will link port A of netdevsim N with port B >> of netdevsim M. Reading this file will return the linked netdevsim id >> and port, if any. >> >> nsim_dev_list_lock and rtnl_lock are held during read/write of peer to >> prevent concurrent modification of netdevsims or their ports. >> >> Signed-off-by: David Wei <dw@davidwei.uk> >> --- >> drivers/net/netdevsim/dev.c | 127 +++++++++++++++++++++++++++--- >> drivers/net/netdevsim/netdev.c | 6 ++ >> drivers/net/netdevsim/netdevsim.h | 1 + >> 3 files changed, 121 insertions(+), 13 deletions(-) >> >> diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c >> index e30a12130e07..e4621861c70b 100644 >> --- a/drivers/net/netdevsim/dev.c >> +++ b/drivers/net/netdevsim/dev.c >> @@ -391,6 +391,117 @@ static const struct file_operations nsim_dev_rate_parent_fops = { >> .owner = THIS_MODULE, >> }; >> >> +static struct nsim_dev *nsim_dev_find_by_id(unsigned int id) >> +{ >> + struct nsim_dev *dev; >> + >> + list_for_each_entry(dev, &nsim_dev_list, list) >> + if (dev->nsim_bus_dev->dev.id == id) >> + return dev; >> + >> + return NULL; >> +} >> + >> +static struct nsim_dev_port * >> +__nsim_dev_port_lookup(struct nsim_dev *nsim_dev, enum nsim_dev_port_type type, >> + unsigned int port_index) >> +{ >> + struct nsim_dev_port *nsim_dev_port; >> + >> + port_index = nsim_dev_port_index(type, port_index); >> + list_for_each_entry(nsim_dev_port, &nsim_dev->port_list, list) >> + if (nsim_dev_port->port_index == port_index) >> + return nsim_dev_port; >> + return NULL; >> +} >> + >> +static ssize_t nsim_dev_peer_read(struct file *file, char __user *data, >> + size_t count, loff_t *ppos) >> +{ >> + struct nsim_dev_port *nsim_dev_port; >> + struct netdevsim *peer; >> + unsigned int id, port; >> + char buf[23]; >> + ssize_t ret; >> + >> + mutex_lock(&nsim_dev_list_lock); >> + rtnl_lock(); >> + nsim_dev_port = file->private_data; >> + peer = rtnl_dereference(nsim_dev_port->ns->peer); >> + if (!peer) >> + goto out; >> + >> + id = peer->nsim_bus_dev->dev.id; >> + port = peer->nsim_dev_port->port_index; >> + ret = scnprintf(buf, sizeof(buf), "%u %u\n", id, port); >> + ret = simple_read_from_buffer(data, count, ppos, buf, ret); >> + >> +out: >> + rtnl_unlock(); >> + mutex_unlock(&nsim_dev_list_lock); >> + >> + return ret; >> +} >> + >> +static ssize_t nsim_dev_peer_write(struct file *file, >> + const char __user *data, >> + size_t count, loff_t *ppos) >> +{ >> + struct nsim_dev_port *nsim_dev_port, *peer_dev_port; >> + struct nsim_dev *peer_dev; >> + unsigned int id, port; >> + char buf[22]; >> + ssize_t ret; >> + >> + if (count >= sizeof(buf)) >> + return -ENOSPC; >> + >> + ret = copy_from_user(buf, data, count); >> + if (ret) >> + return -EFAULT; >> + buf[count] = '\0'; >> + >> + ret = sscanf(buf, "%u %u", &id, &port); >> + if (ret != 2) { >> + pr_err("Format for adding a peer is \"id port\" (uint uint)"); >> + return -EINVAL; >> + } >> + >> + ret = -EINVAL; >> + mutex_lock(&nsim_dev_list_lock); >> + rtnl_lock(); >> + peer_dev = nsim_dev_find_by_id(id); >> + if (!peer_dev) >> + goto out; > > Tell the user what's wrong please. Okay. > >> + >> + peer_dev_port = __nsim_dev_port_lookup(peer_dev, NSIM_DEV_PORT_TYPE_PF, >> + port); >> + if (!peer_dev_port) >> + goto out; > > Tell the user what's wrong please. Ditto. > > >> + >> + nsim_dev_port = file->private_data; >> + if (nsim_dev_port == peer_dev_port) > > Why fail here? IDK, success sounds better to me. I don't want to link a port to itself. In my mental model a port is e.g. a port on a switch. It doesn't make sense to connect it to itself. > > >> + goto out; >> + >> + rcu_assign_pointer(nsim_dev_port->ns->peer, peer_dev_port->ns); >> + rcu_assign_pointer(peer_dev_port->ns->peer, nsim_dev_port->ns); >> + ret = count; >> + >> +out: >> + rtnl_unlock(); >> + mutex_unlock(&nsim_dev_list_lock); >> + >> + return ret; >> +} >> + >> +static const struct file_operations nsim_dev_peer_fops = { >> + .open = simple_open, >> + .read = nsim_dev_peer_read, >> + .write = nsim_dev_peer_write, >> + .llseek = generic_file_llseek, >> + .owner = THIS_MODULE, >> +}; >> + >> static int nsim_dev_port_debugfs_init(struct nsim_dev *nsim_dev, >> struct nsim_dev_port *nsim_dev_port) >> { >> @@ -421,6 +532,9 @@ static int nsim_dev_port_debugfs_init(struct nsim_dev *nsim_dev, >> } >> debugfs_create_symlink("dev", nsim_dev_port->ddir, dev_link_name); >> >> + debugfs_create_file("peer", 0600, nsim_dev_port->ddir, >> + nsim_dev_port, &nsim_dev_peer_fops); >> + >> return 0; >> } >> >> @@ -1702,19 +1816,6 @@ void nsim_drv_remove(struct nsim_bus_dev *nsim_bus_dev) >> dev_set_drvdata(&nsim_bus_dev->dev, NULL); >> } >> >> -static struct nsim_dev_port * >> -__nsim_dev_port_lookup(struct nsim_dev *nsim_dev, enum nsim_dev_port_type type, >> - unsigned int port_index) >> -{ >> - struct nsim_dev_port *nsim_dev_port; >> - >> - port_index = nsim_dev_port_index(type, port_index); >> - list_for_each_entry(nsim_dev_port, &nsim_dev->port_list, list) >> - if (nsim_dev_port->port_index == port_index) >> - return nsim_dev_port; >> - return NULL; >> -} >> - >> int nsim_drv_port_add(struct nsim_bus_dev *nsim_bus_dev, enum nsim_dev_port_type type, >> unsigned int port_index) >> { >> diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c >> index aecaf5f44374..434322f6a565 100644 >> --- a/drivers/net/netdevsim/netdev.c >> +++ b/drivers/net/netdevsim/netdev.c >> @@ -388,6 +388,7 @@ nsim_create(struct nsim_dev *nsim_dev, struct nsim_dev_port *nsim_dev_port) >> ns->nsim_dev = nsim_dev; >> ns->nsim_dev_port = nsim_dev_port; >> ns->nsim_bus_dev = nsim_dev->nsim_bus_dev; >> + RCU_INIT_POINTER(ns->peer, NULL); >> SET_NETDEV_DEV(dev, &ns->nsim_bus_dev->dev); >> SET_NETDEV_DEVLINK_PORT(dev, &nsim_dev_port->devlink_port); >> nsim_ethtool_init(ns); >> @@ -407,8 +408,13 @@ nsim_create(struct nsim_dev *nsim_dev, struct nsim_dev_port *nsim_dev_port) >> void nsim_destroy(struct netdevsim *ns) >> { >> struct net_device *dev = ns->netdev; >> + struct netdevsim *peer; >> >> rtnl_lock(); >> + peer = rtnl_dereference(ns->peer); >> + if (peer) >> + RCU_INIT_POINTER(peer->peer, NULL); >> + RCU_INIT_POINTER(ns->peer, NULL); >> unregister_netdevice(dev); >> if (nsim_dev_port_is_pf(ns->nsim_dev_port)) { >> nsim_macsec_teardown(ns); >> diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h >> index babb61d7790b..24fc3fbda791 100644 >> --- a/drivers/net/netdevsim/netdevsim.h >> +++ b/drivers/net/netdevsim/netdevsim.h >> @@ -125,6 +125,7 @@ struct netdevsim { >> } udp_ports; >> >> struct nsim_ethtool ethtool; >> + struct netdevsim __rcu *peer; >> }; >> >> struct netdevsim * >> -- >> 2.39.3 >>
Fri, Dec 22, 2023 at 01:47:59AM CET, dw@davidwei.uk wrote: >On 2023-12-20 01:09, Jiri Pirko wrote: >> Wed, Dec 20, 2023 at 02:47:44AM CET, dw@davidwei.uk wrote: >>> Add a debugfs file in >>> /sys/kernel/debug/netdevsim/netdevsimN/ports/A/peer >>> >>> Writing "M B" to this file will link port A of netdevsim N with port B >>> of netdevsim M. Reading this file will return the linked netdevsim id >>> and port, if any. >>> >>> nsim_dev_list_lock and rtnl_lock are held during read/write of peer to >>> prevent concurrent modification of netdevsims or their ports. >>> >>> Signed-off-by: David Wei <dw@davidwei.uk> >>> --- >>> drivers/net/netdevsim/dev.c | 127 +++++++++++++++++++++++++++--- >>> drivers/net/netdevsim/netdev.c | 6 ++ >>> drivers/net/netdevsim/netdevsim.h | 1 + >>> 3 files changed, 121 insertions(+), 13 deletions(-) >>> >>> diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c >>> index e30a12130e07..e4621861c70b 100644 >>> --- a/drivers/net/netdevsim/dev.c >>> +++ b/drivers/net/netdevsim/dev.c >>> @@ -391,6 +391,117 @@ static const struct file_operations nsim_dev_rate_parent_fops = { >>> .owner = THIS_MODULE, >>> }; >>> >>> +static struct nsim_dev *nsim_dev_find_by_id(unsigned int id) >>> +{ >>> + struct nsim_dev *dev; >>> + >>> + list_for_each_entry(dev, &nsim_dev_list, list) >>> + if (dev->nsim_bus_dev->dev.id == id) >>> + return dev; >>> + >>> + return NULL; >>> +} >>> + >>> +static struct nsim_dev_port * >>> +__nsim_dev_port_lookup(struct nsim_dev *nsim_dev, enum nsim_dev_port_type type, >>> + unsigned int port_index) >>> +{ >>> + struct nsim_dev_port *nsim_dev_port; >>> + >>> + port_index = nsim_dev_port_index(type, port_index); >>> + list_for_each_entry(nsim_dev_port, &nsim_dev->port_list, list) >>> + if (nsim_dev_port->port_index == port_index) >>> + return nsim_dev_port; >>> + return NULL; >>> +} >>> + >>> +static ssize_t nsim_dev_peer_read(struct file *file, char __user *data, >>> + size_t count, loff_t *ppos) >>> +{ >>> + struct nsim_dev_port *nsim_dev_port; >>> + struct netdevsim *peer; >>> + unsigned int id, port; >>> + char buf[23]; >>> + ssize_t ret; >>> + >>> + mutex_lock(&nsim_dev_list_lock); >>> + rtnl_lock(); >>> + nsim_dev_port = file->private_data; >>> + peer = rtnl_dereference(nsim_dev_port->ns->peer); >>> + if (!peer) >>> + goto out; >>> + >>> + id = peer->nsim_bus_dev->dev.id; >>> + port = peer->nsim_dev_port->port_index; >>> + ret = scnprintf(buf, sizeof(buf), "%u %u\n", id, port); >>> + ret = simple_read_from_buffer(data, count, ppos, buf, ret); >>> + >>> +out: >>> + rtnl_unlock(); >>> + mutex_unlock(&nsim_dev_list_lock); >>> + >>> + return ret; >>> +} >>> + >>> +static ssize_t nsim_dev_peer_write(struct file *file, >>> + const char __user *data, >>> + size_t count, loff_t *ppos) >>> +{ >>> + struct nsim_dev_port *nsim_dev_port, *peer_dev_port; >>> + struct nsim_dev *peer_dev; >>> + unsigned int id, port; >>> + char buf[22]; >>> + ssize_t ret; >>> + >>> + if (count >= sizeof(buf)) >>> + return -ENOSPC; >>> + >>> + ret = copy_from_user(buf, data, count); >>> + if (ret) >>> + return -EFAULT; >>> + buf[count] = '\0'; >>> + >>> + ret = sscanf(buf, "%u %u", &id, &port); >>> + if (ret != 2) { >>> + pr_err("Format for adding a peer is \"id port\" (uint uint)"); >>> + return -EINVAL; >>> + } >>> + >>> + ret = -EINVAL; >>> + mutex_lock(&nsim_dev_list_lock); >>> + rtnl_lock(); >>> + peer_dev = nsim_dev_find_by_id(id); >>> + if (!peer_dev) >>> + goto out; >> >> Tell the user what's wrong please. > >Okay. > >> >>> + >>> + peer_dev_port = __nsim_dev_port_lookup(peer_dev, NSIM_DEV_PORT_TYPE_PF, >>> + port); >>> + if (!peer_dev_port) >>> + goto out; >> >> Tell the user what's wrong please. > >Ditto. > >> >> >>> + >>> + nsim_dev_port = file->private_data; >>> + if (nsim_dev_port == peer_dev_port) >> >> Why fail here? IDK, success sounds better to me. > >I don't want to link a port to itself. In my mental model a port is e.g. >a port on a switch. It doesn't make sense to connect it to itself. Okay, I misread. > >> >> >>> + goto out; >>> + >>> + rcu_assign_pointer(nsim_dev_port->ns->peer, peer_dev_port->ns); >>> + rcu_assign_pointer(peer_dev_port->ns->peer, nsim_dev_port->ns); >>> + ret = count; >>> + >>> +out: >>> + rtnl_unlock(); >>> + mutex_unlock(&nsim_dev_list_lock); >>> + >>> + return ret; >>> +} >>> + >>> +static const struct file_operations nsim_dev_peer_fops = { >>> + .open = simple_open, >>> + .read = nsim_dev_peer_read, >>> + .write = nsim_dev_peer_write, >>> + .llseek = generic_file_llseek, >>> + .owner = THIS_MODULE, >>> +}; >>> + >>> static int nsim_dev_port_debugfs_init(struct nsim_dev *nsim_dev, >>> struct nsim_dev_port *nsim_dev_port) >>> { >>> @@ -421,6 +532,9 @@ static int nsim_dev_port_debugfs_init(struct nsim_dev *nsim_dev, >>> } >>> debugfs_create_symlink("dev", nsim_dev_port->ddir, dev_link_name); >>> >>> + debugfs_create_file("peer", 0600, nsim_dev_port->ddir, >>> + nsim_dev_port, &nsim_dev_peer_fops); >>> + >>> return 0; >>> } >>> >>> @@ -1702,19 +1816,6 @@ void nsim_drv_remove(struct nsim_bus_dev *nsim_bus_dev) >>> dev_set_drvdata(&nsim_bus_dev->dev, NULL); >>> } >>> >>> -static struct nsim_dev_port * >>> -__nsim_dev_port_lookup(struct nsim_dev *nsim_dev, enum nsim_dev_port_type type, >>> - unsigned int port_index) >>> -{ >>> - struct nsim_dev_port *nsim_dev_port; >>> - >>> - port_index = nsim_dev_port_index(type, port_index); >>> - list_for_each_entry(nsim_dev_port, &nsim_dev->port_list, list) >>> - if (nsim_dev_port->port_index == port_index) >>> - return nsim_dev_port; >>> - return NULL; >>> -} >>> - >>> int nsim_drv_port_add(struct nsim_bus_dev *nsim_bus_dev, enum nsim_dev_port_type type, >>> unsigned int port_index) >>> { >>> diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c >>> index aecaf5f44374..434322f6a565 100644 >>> --- a/drivers/net/netdevsim/netdev.c >>> +++ b/drivers/net/netdevsim/netdev.c >>> @@ -388,6 +388,7 @@ nsim_create(struct nsim_dev *nsim_dev, struct nsim_dev_port *nsim_dev_port) >>> ns->nsim_dev = nsim_dev; >>> ns->nsim_dev_port = nsim_dev_port; >>> ns->nsim_bus_dev = nsim_dev->nsim_bus_dev; >>> + RCU_INIT_POINTER(ns->peer, NULL); >>> SET_NETDEV_DEV(dev, &ns->nsim_bus_dev->dev); >>> SET_NETDEV_DEVLINK_PORT(dev, &nsim_dev_port->devlink_port); >>> nsim_ethtool_init(ns); >>> @@ -407,8 +408,13 @@ nsim_create(struct nsim_dev *nsim_dev, struct nsim_dev_port *nsim_dev_port) >>> void nsim_destroy(struct netdevsim *ns) >>> { >>> struct net_device *dev = ns->netdev; >>> + struct netdevsim *peer; >>> >>> rtnl_lock(); >>> + peer = rtnl_dereference(ns->peer); >>> + if (peer) >>> + RCU_INIT_POINTER(peer->peer, NULL); >>> + RCU_INIT_POINTER(ns->peer, NULL); >>> unregister_netdevice(dev); >>> if (nsim_dev_port_is_pf(ns->nsim_dev_port)) { >>> nsim_macsec_teardown(ns); >>> diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h >>> index babb61d7790b..24fc3fbda791 100644 >>> --- a/drivers/net/netdevsim/netdevsim.h >>> +++ b/drivers/net/netdevsim/netdevsim.h >>> @@ -125,6 +125,7 @@ struct netdevsim { >>> } udp_ports; >>> >>> struct nsim_ethtool ethtool; >>> + struct netdevsim __rcu *peer; >>> }; >>> >>> struct netdevsim * >>> -- >>> 2.39.3 >>>
diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c index e30a12130e07..e4621861c70b 100644 --- a/drivers/net/netdevsim/dev.c +++ b/drivers/net/netdevsim/dev.c @@ -391,6 +391,117 @@ static const struct file_operations nsim_dev_rate_parent_fops = { .owner = THIS_MODULE, }; +static struct nsim_dev *nsim_dev_find_by_id(unsigned int id) +{ + struct nsim_dev *dev; + + list_for_each_entry(dev, &nsim_dev_list, list) + if (dev->nsim_bus_dev->dev.id == id) + return dev; + + return NULL; +} + +static struct nsim_dev_port * +__nsim_dev_port_lookup(struct nsim_dev *nsim_dev, enum nsim_dev_port_type type, + unsigned int port_index) +{ + struct nsim_dev_port *nsim_dev_port; + + port_index = nsim_dev_port_index(type, port_index); + list_for_each_entry(nsim_dev_port, &nsim_dev->port_list, list) + if (nsim_dev_port->port_index == port_index) + return nsim_dev_port; + return NULL; +} + +static ssize_t nsim_dev_peer_read(struct file *file, char __user *data, + size_t count, loff_t *ppos) +{ + struct nsim_dev_port *nsim_dev_port; + struct netdevsim *peer; + unsigned int id, port; + char buf[23]; + ssize_t ret; + + mutex_lock(&nsim_dev_list_lock); + rtnl_lock(); + nsim_dev_port = file->private_data; + peer = rtnl_dereference(nsim_dev_port->ns->peer); + if (!peer) + goto out; + + id = peer->nsim_bus_dev->dev.id; + port = peer->nsim_dev_port->port_index; + ret = scnprintf(buf, sizeof(buf), "%u %u\n", id, port); + ret = simple_read_from_buffer(data, count, ppos, buf, ret); + +out: + rtnl_unlock(); + mutex_unlock(&nsim_dev_list_lock); + + return ret; +} + +static ssize_t nsim_dev_peer_write(struct file *file, + const char __user *data, + size_t count, loff_t *ppos) +{ + struct nsim_dev_port *nsim_dev_port, *peer_dev_port; + struct nsim_dev *peer_dev; + unsigned int id, port; + char buf[22]; + ssize_t ret; + + if (count >= sizeof(buf)) + return -ENOSPC; + + ret = copy_from_user(buf, data, count); + if (ret) + return -EFAULT; + buf[count] = '\0'; + + ret = sscanf(buf, "%u %u", &id, &port); + if (ret != 2) { + pr_err("Format for adding a peer is \"id port\" (uint uint)"); + return -EINVAL; + } + + ret = -EINVAL; + mutex_lock(&nsim_dev_list_lock); + rtnl_lock(); + peer_dev = nsim_dev_find_by_id(id); + if (!peer_dev) + goto out; + + peer_dev_port = __nsim_dev_port_lookup(peer_dev, NSIM_DEV_PORT_TYPE_PF, + port); + if (!peer_dev_port) + goto out; + + nsim_dev_port = file->private_data; + if (nsim_dev_port == peer_dev_port) + goto out; + + rcu_assign_pointer(nsim_dev_port->ns->peer, peer_dev_port->ns); + rcu_assign_pointer(peer_dev_port->ns->peer, nsim_dev_port->ns); + ret = count; + +out: + rtnl_unlock(); + mutex_unlock(&nsim_dev_list_lock); + + return ret; +} + +static const struct file_operations nsim_dev_peer_fops = { + .open = simple_open, + .read = nsim_dev_peer_read, + .write = nsim_dev_peer_write, + .llseek = generic_file_llseek, + .owner = THIS_MODULE, +}; + static int nsim_dev_port_debugfs_init(struct nsim_dev *nsim_dev, struct nsim_dev_port *nsim_dev_port) { @@ -421,6 +532,9 @@ static int nsim_dev_port_debugfs_init(struct nsim_dev *nsim_dev, } debugfs_create_symlink("dev", nsim_dev_port->ddir, dev_link_name); + debugfs_create_file("peer", 0600, nsim_dev_port->ddir, + nsim_dev_port, &nsim_dev_peer_fops); + return 0; } @@ -1702,19 +1816,6 @@ void nsim_drv_remove(struct nsim_bus_dev *nsim_bus_dev) dev_set_drvdata(&nsim_bus_dev->dev, NULL); } -static struct nsim_dev_port * -__nsim_dev_port_lookup(struct nsim_dev *nsim_dev, enum nsim_dev_port_type type, - unsigned int port_index) -{ - struct nsim_dev_port *nsim_dev_port; - - port_index = nsim_dev_port_index(type, port_index); - list_for_each_entry(nsim_dev_port, &nsim_dev->port_list, list) - if (nsim_dev_port->port_index == port_index) - return nsim_dev_port; - return NULL; -} - int nsim_drv_port_add(struct nsim_bus_dev *nsim_bus_dev, enum nsim_dev_port_type type, unsigned int port_index) { diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c index aecaf5f44374..434322f6a565 100644 --- a/drivers/net/netdevsim/netdev.c +++ b/drivers/net/netdevsim/netdev.c @@ -388,6 +388,7 @@ nsim_create(struct nsim_dev *nsim_dev, struct nsim_dev_port *nsim_dev_port) ns->nsim_dev = nsim_dev; ns->nsim_dev_port = nsim_dev_port; ns->nsim_bus_dev = nsim_dev->nsim_bus_dev; + RCU_INIT_POINTER(ns->peer, NULL); SET_NETDEV_DEV(dev, &ns->nsim_bus_dev->dev); SET_NETDEV_DEVLINK_PORT(dev, &nsim_dev_port->devlink_port); nsim_ethtool_init(ns); @@ -407,8 +408,13 @@ nsim_create(struct nsim_dev *nsim_dev, struct nsim_dev_port *nsim_dev_port) void nsim_destroy(struct netdevsim *ns) { struct net_device *dev = ns->netdev; + struct netdevsim *peer; rtnl_lock(); + peer = rtnl_dereference(ns->peer); + if (peer) + RCU_INIT_POINTER(peer->peer, NULL); + RCU_INIT_POINTER(ns->peer, NULL); unregister_netdevice(dev); if (nsim_dev_port_is_pf(ns->nsim_dev_port)) { nsim_macsec_teardown(ns); diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h index babb61d7790b..24fc3fbda791 100644 --- a/drivers/net/netdevsim/netdevsim.h +++ b/drivers/net/netdevsim/netdevsim.h @@ -125,6 +125,7 @@ struct netdevsim { } udp_ports; struct nsim_ethtool ethtool; + struct netdevsim __rcu *peer; }; struct netdevsim *
Add a debugfs file in /sys/kernel/debug/netdevsim/netdevsimN/ports/A/peer Writing "M B" to this file will link port A of netdevsim N with port B of netdevsim M. Reading this file will return the linked netdevsim id and port, if any. nsim_dev_list_lock and rtnl_lock are held during read/write of peer to prevent concurrent modification of netdevsims or their ports. Signed-off-by: David Wei <dw@davidwei.uk> --- drivers/net/netdevsim/dev.c | 127 +++++++++++++++++++++++++++--- drivers/net/netdevsim/netdev.c | 6 ++ drivers/net/netdevsim/netdevsim.h | 1 + 3 files changed, 121 insertions(+), 13 deletions(-)