[PATCH net-next v2 0/3] netdevsim: link and forward skbs between ports

[PATCH net-next v2 0/3] netdevsim: link and forward skbs between ports

[David Wei]

This patchset adds the ability to link two netdevsim ports together and
forward skbs between them, similar to veth. The goal is to use netdevsim
for testing features e.g. zero copy Rx using io_uring.

This feature was tested locally on QEMU, and a selftest is included.

---
v1->v2:
- renamed debugfs file from "link" to "peer"
- replaced strstep() with sscanf() for consistency
- increased char[] buf sz to 22 for copying id + port from user
- added err msg w/ expected fmt when linking as a hint to user
- prevent linking port to itself
- protect peer ptr using RCU

David Wei (3):
  netdevsim: allow two netdevsim ports to be connected
  netdevsim: forward skbs from one connected port to another
  netdevsim: add selftest for forwarding skb between connected ports

 drivers/net/netdevsim/bus.c                   |  10 ++
 drivers/net/netdevsim/dev.c                   |  86 ++++++++++++++
 drivers/net/netdevsim/netdev.c                |  29 ++++-
 drivers/net/netdevsim/netdevsim.h             |   3 +
 .../selftests/drivers/net/netdevsim/peer.sh   | 111 ++++++++++++++++++
 5 files changed, 234 insertions(+), 5 deletions(-)
 create mode 100755 tools/testing/selftests/drivers/net/netdevsim/peer.sh

-- 
2.39.3

[PATCH net-next v2 1/3] netdevsim: allow two netdevsim ports to be connected

[David Wei]

From: David Wei <davidhwei@meta.com>

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.

Signed-off-by: David Wei <dw@davidwei.uk>
---
 drivers/net/netdevsim/bus.c       | 10 ++++
 drivers/net/netdevsim/dev.c       | 85 +++++++++++++++++++++++++++++++
 drivers/net/netdevsim/netdev.c    |  6 +++
 drivers/net/netdevsim/netdevsim.h |  3 ++
 4 files changed, 104 insertions(+)

diff --git a/drivers/net/netdevsim/bus.c b/drivers/net/netdevsim/bus.c
index bcbc1e19edde..3e4378e9dbee 100644
--- a/drivers/net/netdevsim/bus.c
+++ b/drivers/net/netdevsim/bus.c
@@ -364,3 +364,13 @@ void nsim_bus_exit(void)
 	driver_unregister(&nsim_driver);
 	bus_unregister(&nsim_bus);
 }
+
+struct nsim_bus_dev *nsim_bus_dev_get(unsigned int id)
+{
+	struct nsim_bus_dev *nsim_bus_dev;
+	list_for_each_entry(nsim_bus_dev, &nsim_bus_dev_list, list) {
+		if (nsim_bus_dev->dev.id == id)
+			return nsim_bus_dev;
+	}
+	return NULL;
+}
diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index b4d3b9cde8bd..7af219ff6fa9 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -388,6 +388,88 @@ static const struct file_operations nsim_dev_rate_parent_fops = {
 	.owner = THIS_MODULE,
 };
 
+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 len;
+
+	nsim_dev_port = file->private_data;
+	rcu_read_lock();
+	peer = rcu_dereference(nsim_dev_port->ns->peer);
+	if (!peer) {
+		len = scnprintf(buf, sizeof(buf), "\n");
+		goto out;
+	}
+
+	id = peer->nsim_bus_dev->dev.id;
+	port = peer->nsim_dev_port->port_index;
+	len = scnprintf(buf, sizeof(buf), "%u %u\n", id, port);
+
+out:
+	rcu_read_unlock();
+	return simple_read_from_buffer(data, count, ppos, buf, len);
+}
+
+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_bus_dev *peer_bus_dev;
+	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;
+	}
+
+	/* invalid netdevsim id */
+	peer_bus_dev = nsim_bus_dev_get(id);
+	if (!peer_bus_dev)
+		return -EINVAL;
+
+	/* cannot link to self */
+	nsim_dev_port = file->private_data;
+	if (nsim_dev_port->ns->nsim_bus_dev == peer_bus_dev &&
+	    nsim_dev_port->port_index == port)
+		return -EINVAL;
+
+	peer_dev = dev_get_drvdata(&peer_bus_dev->dev);
+	list_for_each_entry(peer_dev_port, &peer_dev->port_list, list) {
+		if (peer_dev_port->port_index != port)
+			continue;
+		rcu_assign_pointer(nsim_dev_port->ns->peer, peer_dev_port->ns);
+		rcu_assign_pointer(peer_dev_port->ns->peer, nsim_dev_port->ns);
+		return count;
+	}
+
+	return -EINVAL;
+}
+
+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)
 {
@@ -418,6 +500,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;
 }
 
diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
index aecaf5f44374..e290c54b0e70 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,9 +408,14 @@ 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);
+	RCU_INIT_POINTER(ns->peer, NULL);
 	unregister_netdevice(dev);
+	if (peer)
+		RCU_INIT_POINTER(peer->peer, NULL);
 	if (nsim_dev_port_is_pf(ns->nsim_dev_port)) {
 		nsim_macsec_teardown(ns);
 		nsim_ipsec_teardown(ns);
diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h
index 028c825b86db..2601c3ad1d17 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 *
@@ -417,3 +418,5 @@ struct nsim_bus_dev {
 
 int nsim_bus_init(void);
 void nsim_bus_exit(void);
+
+struct nsim_bus_dev *nsim_bus_dev_get(unsigned int id);
-- 
2.39.3

[PATCH net-next v2 2/3] netdevsim: forward skbs from one connected port to another

[David Wei]

From: David Wei <davidhwei@meta.com>

Forward skbs sent from one netdevsim port to its connected netdevsim
port using dev_forward_skb, in a spirit similar to veth.

Signed-off-by: David Wei <dw@davidwei.uk>
---
 drivers/net/netdevsim/netdev.c | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
index e290c54b0e70..c5f53b1dbdcc 100644
--- a/drivers/net/netdevsim/netdev.c
+++ b/drivers/net/netdevsim/netdev.c
@@ -29,19 +29,33 @@
 static netdev_tx_t nsim_start_xmit(struct sk_buff *skb, struct net_device *dev)
 {
 	struct netdevsim *ns = netdev_priv(dev);
+	struct netdevsim *peer_ns;
+	int ret = NETDEV_TX_OK;
 
+	rcu_read_lock();
 	if (!nsim_ipsec_tx(ns, skb))
-		goto out;
+		goto err;
 
 	u64_stats_update_begin(&ns->syncp);
 	ns->tx_packets++;
 	ns->tx_bytes += skb->len;
 	u64_stats_update_end(&ns->syncp);
 
-out:
-	dev_kfree_skb(skb);
+	peer_ns = rcu_dereference(ns->peer);
+	if (!peer_ns)
+		goto err;
+
+	skb_tx_timestamp(skb);
+	if (unlikely(dev_forward_skb(peer_ns->netdev, skb) == NET_RX_DROP))
+		ret = NET_XMIT_DROP;
 
-	return NETDEV_TX_OK;
+	rcu_read_unlock();
+	return ret;
+
+err:
+	rcu_read_unlock();
+	dev_kfree_skb(skb);
+	return ret;
 }
 
 static void nsim_set_rx_mode(struct net_device *dev)
@@ -302,7 +316,6 @@ static void nsim_setup(struct net_device *dev)
 	eth_hw_addr_random(dev);
 
 	dev->tx_queue_len = 0;
-	dev->flags |= IFF_NOARP;
 	dev->flags &= ~IFF_MULTICAST;
 	dev->priv_flags |= IFF_LIVE_ADDR_CHANGE |
 			   IFF_NO_QUEUE;
-- 
2.39.3

[PATCH net-next v2 3/3] netdevsim: add selftest for forwarding skb between connected ports

[David Wei]

From: David Wei <davidhwei@meta.com>

Connect two netdevsim ports in different namespaces together, then send
packets between them using socat.

Signed-off-by: David Wei <dw@davidwei.uk>
---
 .../selftests/drivers/net/netdevsim/peer.sh   | 111 ++++++++++++++++++
 1 file changed, 111 insertions(+)
 create mode 100755 tools/testing/selftests/drivers/net/netdevsim/peer.sh

diff --git a/tools/testing/selftests/drivers/net/netdevsim/peer.sh b/tools/testing/selftests/drivers/net/netdevsim/peer.sh
new file mode 100755
index 000000000000..d1d59a932174
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/netdevsim/peer.sh
@@ -0,0 +1,111 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0-only
+
+NSIM_DEV_SYS=/sys/bus/netdevsim
+NSIM_DEV_DFS=/sys/kernel/debug/netdevsim/netdevsim
+
+socat_check()
+{
+	if [ ! -x "$(command -v socat)" ]; then
+		echo "socat command not found. Skipping test"
+		return 1
+	fi
+
+	return 0
+}
+
+setup_ns()
+{
+	set -e
+	ip netns add nssv
+	ip netns add nscl
+
+	ip link set eni1np1 netns nssv
+	ip link set eni2np1 netns nscl
+
+	ip netns exec nssv ip addr add '192.168.1.1/24' dev eni1np1
+	ip netns exec nscl ip addr add '192.168.1.2/24' dev eni2np1
+
+	ip netns exec nssv ip link set dev eni1np1 up
+	ip netns exec nscl ip link set dev eni2np1 up
+	set +e
+}
+
+cleanup_ns()
+{
+	ip netns del nscl
+	ip netns del nssv
+}
+
+###
+### Code start
+###
+
+modprobe netdevsim
+
+# linking
+
+echo 1 > ${NSIM_DEV_SYS}/new_device
+
+echo "2 0" > ${NSIM_DEV_DFS}1/ports/0/peer 2>/dev/null
+if [ $? -eq 0 ]; then
+	echo "linking with non-existent netdevsim should fail"
+	exit 1
+fi
+
+echo 2 > /sys/bus/netdevsim/new_device
+
+echo "2 0" > ${NSIM_DEV_DFS}1/ports/0/peer
+if [ $? -ne 0 ]; then
+	echo "linking netdevsim1 port0 with netdevsim2 port0 should succeed"
+	exit 1
+fi
+
+# argument error checking
+
+echo "2 1" > ${NSIM_DEV_DFS}1/ports/0/peer 2>/dev/null
+if [ $? -eq 0 ]; then
+	echo "linking with non-existent port in a netdevsim should fail"
+	exit 1
+fi
+
+echo "1 0" > ${NSIM_DEV_DFS}1/ports/0/peer 2>/dev/null
+if [ $? -eq 0 ]; then
+	echo "linking with self should fail"
+	exit 1
+fi
+
+echo "2 a" > ${NSIM_DEV_DFS}1/ports/0/peer 2>/dev/null
+if [ $? -eq 0 ]; then
+	echo "invalid arg should fail"
+	exit 1
+fi
+
+# send/recv packets
+
+socat_check || exit 1
+
+setup_ns
+
+tmp_file=$(mktemp)
+ip netns exec nssv socat TCP-LISTEN:1234,fork $tmp_file &
+pid=$!
+
+echo "HI" | ip netns exec nscl socat STDIN TCP:192.168.1.1:1234
+
+count=$(cat $tmp_file | wc -c)
+if [[ $count -ne 3 ]]; then
+	echo "expected 3 bytes, got $count"
+	exit 1
+fi
+
+echo 2 > ${NSIM_DEV_SYS}/del_device
+
+kill $pid
+echo 1 > ${NSIM_DEV_SYS}/del_device
+
+cleanup_ns
+
+modprobe -r netdevsim
+
+exit 0
-- 
2.39.3

Re: [PATCH net-next v2 1/3] netdevsim: allow two netdevsim ports to be connected

[Jakub Kicinski]

On Sat,  9 Dec 2023 17:04:46 -0800 David Wei wrote:
> diff --git a/drivers/net/netdevsim/bus.c b/drivers/net/netdevsim/bus.c
> index bcbc1e19edde..3e4378e9dbee 100644
> --- a/drivers/net/netdevsim/bus.c
> +++ b/drivers/net/netdevsim/bus.c
> @@ -364,3 +364,13 @@ void nsim_bus_exit(void)
>  	driver_unregister(&nsim_driver);
>  	bus_unregister(&nsim_bus);
>  }
> +
> +struct nsim_bus_dev *nsim_bus_dev_get(unsigned int id)

nit: s/get/find/ get sometimes implied taking a reference 

> +{
> +	struct nsim_bus_dev *nsim_bus_dev;

new line here, please checkpatch --strict

> +	list_for_each_entry(nsim_bus_dev, &nsim_bus_dev_list, list) {
> +		if (nsim_bus_dev->dev.id == id)
> +			return nsim_bus_dev;

You must assume some lock is being held so that you can walk the list
and return a meaningful value? :) Please figure out what caller has to
hold and add an appropriate lockdep assert here.

> +	}
> +	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 len;
> +
> +	nsim_dev_port = file->private_data;
> +	rcu_read_lock();
> +	peer = rcu_dereference(nsim_dev_port->ns->peer);
> +	if (!peer) {
> +		len = scnprintf(buf, sizeof(buf), "\n");

Why not return 0?

> +		goto out;
> +	}
> +
> +	id = peer->nsim_bus_dev->dev.id;
> +	port = peer->nsim_dev_port->port_index;
> +	len = scnprintf(buf, sizeof(buf), "%u %u\n", id, port);
> +
> +out:
> +	rcu_read_unlock();
> +	return simple_read_from_buffer(data, count, ppos, buf, len);
> +}

> @@ -417,3 +418,5 @@ struct nsim_bus_dev {
>  
>  int nsim_bus_init(void);
>  void nsim_bus_exit(void);
> +
> +struct nsim_bus_dev *nsim_bus_dev_get(unsigned int id);

nit: let this go before the module init/exit funcs, 3 lines up

Re: [PATCH net-next v2 3/3] netdevsim: add selftest for forwarding skb between connected ports

[Jakub Kicinski]

On Sat,  9 Dec 2023 17:04:48 -0800 David Wei wrote:
> From: David Wei <davidhwei@meta.com>
> 
> Connect two netdevsim ports in different namespaces together, then send
> packets between them using socat.
> 
> Signed-off-by: David Wei <dw@davidwei.uk>
> ---
>  .../selftests/drivers/net/netdevsim/peer.sh   | 111 ++++++++++++++++++
>  1 file changed, 111 insertions(+)
>  create mode 100755 tools/testing/selftests/drivers/net/netdevsim/peer.sh

You need to include this script in the Makefile so it gets run

> diff --git a/tools/testing/selftests/drivers/net/netdevsim/peer.sh b/tools/testing/selftests/drivers/net/netdevsim/peer.sh
> new file mode 100755
> index 000000000000..d1d59a932174
> --- /dev/null
> +++ b/tools/testing/selftests/drivers/net/netdevsim/peer.sh
> @@ -0,0 +1,111 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0-only
> +
> +NSIM_DEV_SYS=/sys/bus/netdevsim
> +NSIM_DEV_DFS=/sys/kernel/debug/netdevsim/netdevsim
> +
> +socat_check()
> +{
> +	if [ ! -x "$(command -v socat)" ]; then
> +		echo "socat command not found. Skipping test"
> +		return 1
> +	fi
> +
> +	return 0
> +}
> +
> +setup_ns()
> +{
> +	set -e
> +	ip netns add nssv
> +	ip netns add nscl
> +
> +	ip link set eni1np1 netns nssv
> +	ip link set eni2np1 netns nscl

This assumes you have systemd renaming interfaces.
I can find out what the netdev is called from sysfs.
After you create the nsim device in its sysfs dir
there will be a dir "net" and in it you'll have
a subdir with the same name as the netdev.

> +	ip netns exec nssv ip addr add '192.168.1.1/24' dev eni1np1
> +	ip netns exec nscl ip addr add '192.168.1.2/24' dev eni2np1
> +
> +	ip netns exec nssv ip link set dev eni1np1 up
> +	ip netns exec nscl ip link set dev eni2np1 up
> +	set +e
> +}
> +
> +cleanup_ns()
> +{
> +	ip netns del nscl
> +	ip netns del nssv
> +}
> +
> +###
> +### Code start
> +###
> +
> +modprobe netdevsim
> +
> +# linking
> +
> +echo 1 > ${NSIM_DEV_SYS}/new_device

Use $RANDOM to generate a random device ID.

> +echo "2 0" > ${NSIM_DEV_DFS}1/ports/0/peer 2>/dev/null
> +if [ $? -eq 0 ]; then
> +	echo "linking with non-existent netdevsim should fail"
> +	exit 1
> +fi
> +
> +echo 2 > /sys/bus/netdevsim/new_device
> +
> +echo "2 0" > ${NSIM_DEV_DFS}1/ports/0/peer
> +if [ $? -ne 0 ]; then
> +	echo "linking netdevsim1 port0 with netdevsim2 port0 should succeed"
> +	exit 1
> +fi
> +
> +# argument error checking
> +
> +echo "2 1" > ${NSIM_DEV_DFS}1/ports/0/peer 2>/dev/null
> +if [ $? -eq 0 ]; then
> +	echo "linking with non-existent port in a netdevsim should fail"
> +	exit 1
> +fi
> +
> +echo "1 0" > ${NSIM_DEV_DFS}1/ports/0/peer 2>/dev/null
> +if [ $? -eq 0 ]; then
> +	echo "linking with self should fail"
> +	exit 1
> +fi
> +
> +echo "2 a" > ${NSIM_DEV_DFS}1/ports/0/peer 2>/dev/null
> +if [ $? -eq 0 ]; then
> +	echo "invalid arg should fail"
> +	exit 1
> +fi
> +
> +# send/recv packets
> +
> +socat_check || exit 1

There's a magic return value for skipping kselftests, 1 means fail.
-- 
pw-bot: cr

Re: [PATCH net-next v2 1/3] netdevsim: allow two netdevsim ports to be connected

[David Wei]

On 2023-12-12 12:24, Jakub Kicinski wrote:
> On Sat,  9 Dec 2023 17:04:46 -0800 David Wei wrote:
>> diff --git a/drivers/net/netdevsim/bus.c b/drivers/net/netdevsim/bus.c
>> index bcbc1e19edde..3e4378e9dbee 100644
>> --- a/drivers/net/netdevsim/bus.c
>> +++ b/drivers/net/netdevsim/bus.c
>> @@ -364,3 +364,13 @@ void nsim_bus_exit(void)
>>  	driver_unregister(&nsim_driver);
>>  	bus_unregister(&nsim_bus);
>>  }
>> +
>> +struct nsim_bus_dev *nsim_bus_dev_get(unsigned int id)
> 
> nit: s/get/find/ get sometimes implied taking a reference 

Will do.

> 
>> +{
>> +	struct nsim_bus_dev *nsim_bus_dev;
> 
> new line here, please checkpatch --strict

Sorry, will remember to do this next time.

> 
>> +	list_for_each_entry(nsim_bus_dev, &nsim_bus_dev_list, list) {
>> +		if (nsim_bus_dev->dev.id == id)
>> +			return nsim_bus_dev;
> 
> You must assume some lock is being held so that you can walk the list
> and return a meaningful value? :) Please figure out what caller has to
> hold and add an appropriate lockdep assert here.

Will address.

> 
>> +	}
>> +	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 len;
>> +
>> +	nsim_dev_port = file->private_data;
>> +	rcu_read_lock();
>> +	peer = rcu_dereference(nsim_dev_port->ns->peer);
>> +	if (!peer) {
>> +		len = scnprintf(buf, sizeof(buf), "\n");
> 
> Why not return 0?

No reason, wasn't sure what the norm is.

> 
>> +		goto out;
>> +	}
>> +
>> +	id = peer->nsim_bus_dev->dev.id;
>> +	port = peer->nsim_dev_port->port_index;
>> +	len = scnprintf(buf, sizeof(buf), "%u %u\n", id, port);
>> +
>> +out:
>> +	rcu_read_unlock();
>> +	return simple_read_from_buffer(data, count, ppos, buf, len);
>> +}
> 
>> @@ -417,3 +418,5 @@ struct nsim_bus_dev {
>>  
>>  int nsim_bus_init(void);
>>  void nsim_bus_exit(void);
>> +
>> +struct nsim_bus_dev *nsim_bus_dev_get(unsigned int id);
> 
> nit: let this go before the module init/exit funcs, 3 lines up

Will address.

Re: [PATCH net-next v2 3/3] netdevsim: add selftest for forwarding skb between connected ports

[David Wei]

On 2023-12-12 12:29, Jakub Kicinski wrote:
> On Sat,  9 Dec 2023 17:04:48 -0800 David Wei wrote:
>> From: David Wei <davidhwei@meta.com>
>>
>> Connect two netdevsim ports in different namespaces together, then send
>> packets between them using socat.
>>
>> Signed-off-by: David Wei <dw@davidwei.uk>
>> ---
>>  .../selftests/drivers/net/netdevsim/peer.sh   | 111 ++++++++++++++++++
>>  1 file changed, 111 insertions(+)
>>  create mode 100755 tools/testing/selftests/drivers/net/netdevsim/peer.sh
> 
> You need to include this script in the Makefile so it gets run

Thanks, will address.

> 
>> diff --git a/tools/testing/selftests/drivers/net/netdevsim/peer.sh b/tools/testing/selftests/drivers/net/netdevsim/peer.sh
>> new file mode 100755
>> index 000000000000..d1d59a932174
>> --- /dev/null
>> +++ b/tools/testing/selftests/drivers/net/netdevsim/peer.sh
>> @@ -0,0 +1,111 @@
>> +#!/bin/bash
>> +# SPDX-License-Identifier: GPL-2.0-only
>> +
>> +NSIM_DEV_SYS=/sys/bus/netdevsim
>> +NSIM_DEV_DFS=/sys/kernel/debug/netdevsim/netdevsim
>> +
>> +socat_check()
>> +{
>> +	if [ ! -x "$(command -v socat)" ]; then
>> +		echo "socat command not found. Skipping test"
>> +		return 1
>> +	fi
>> +
>> +	return 0
>> +}
>> +
>> +setup_ns()
>> +{
>> +	set -e
>> +	ip netns add nssv
>> +	ip netns add nscl
>> +
>> +	ip link set eni1np1 netns nssv
>> +	ip link set eni2np1 netns nscl
> 
> This assumes you have systemd renaming interfaces.
> I can find out what the netdev is called from sysfs.
> After you create the nsim device in its sysfs dir
> there will be a dir "net" and in it you'll have
> a subdir with the same name as the netdev.

Thanks, will fix.

> 
>> +	ip netns exec nssv ip addr add '192.168.1.1/24' dev eni1np1
>> +	ip netns exec nscl ip addr add '192.168.1.2/24' dev eni2np1
>> +
>> +	ip netns exec nssv ip link set dev eni1np1 up
>> +	ip netns exec nscl ip link set dev eni2np1 up
>> +	set +e
>> +}
>> +
>> +cleanup_ns()
>> +{
>> +	ip netns del nscl
>> +	ip netns del nssv
>> +}
>> +
>> +###
>> +### Code start
>> +###
>> +
>> +modprobe netdevsim
>> +
>> +# linking
>> +
>> +echo 1 > ${NSIM_DEV_SYS}/new_device
> 
> Use $RANDOM to generate a random device ID.

Will do.

> 
>> +echo "2 0" > ${NSIM_DEV_DFS}1/ports/0/peer 2>/dev/null
>> +if [ $? -eq 0 ]; then
>> +	echo "linking with non-existent netdevsim should fail"
>> +	exit 1
>> +fi
>> +
>> +echo 2 > /sys/bus/netdevsim/new_device
>> +
>> +echo "2 0" > ${NSIM_DEV_DFS}1/ports/0/peer
>> +if [ $? -ne 0 ]; then
>> +	echo "linking netdevsim1 port0 with netdevsim2 port0 should succeed"
>> +	exit 1
>> +fi
>> +
>> +# argument error checking
>> +
>> +echo "2 1" > ${NSIM_DEV_DFS}1/ports/0/peer 2>/dev/null
>> +if [ $? -eq 0 ]; then
>> +	echo "linking with non-existent port in a netdevsim should fail"
>> +	exit 1
>> +fi
>> +
>> +echo "1 0" > ${NSIM_DEV_DFS}1/ports/0/peer 2>/dev/null
>> +if [ $? -eq 0 ]; then
>> +	echo "linking with self should fail"
>> +	exit 1
>> +fi
>> +
>> +echo "2 a" > ${NSIM_DEV_DFS}1/ports/0/peer 2>/dev/null
>> +if [ $? -eq 0 ]; then
>> +	echo "invalid arg should fail"
>> +	exit 1
>> +fi
>> +
>> +# send/recv packets
>> +
>> +socat_check || exit 1
> 
> There's a magic return value for skipping kselftests, 1 means fail.

Thanks, will fix.