netdev - Re: [PATCH net-next v5 3/5] netdevsim: forward skbs from one connected port to another

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Tue, 2 Jan 2024 12:20:46 +0100
From: Eric Dumazet <edumazet@...gle.com>
To: David Wei <dw@...idwei.uk>
Cc: Jakub Kicinski <kuba@...nel.org>, Jiri Pirko <jiri@...nulli.us>, 
	Sabrina Dubroca <sd@...asysnail.net>, netdev@...r.kernel.org, 
	"David S. Miller" <davem@...emloft.net>, Paolo Abeni <pabeni@...hat.com>
Subject: Re: [PATCH net-next v5 3/5] netdevsim: forward skbs from one
 connected port to another

On Thu, Dec 28, 2023 at 2:46 AM David Wei <dw@...idwei.uk> wrote:
>
> Forward skbs sent from one netdevsim port to its connected netdevsim
> port using dev_forward_skb, in a spirit similar to veth.
>
> Add a tx_dropped variable to struct netdevsim, tracking the number of
> skbs that could not be forwarded using dev_forward_skb().
>
> The xmit() function accessing the peer ptr is protected by an RCU read
> critical section. The rcu_read_lock() is functionally redundant as since
> v5.0 all softirqs are implicitly RCU read critical sections; but it is
> useful for human readers.
>
> If another CPU is concurrently in nsim_destroy(), then it will first set
> the peer ptr to NULL. This does not affect any existing readers that
> dereferenced a non-NULL peer. Then, in unregister_netdevice(), there is
> a synchronize_rcu() before the netdev is actually unregistered and
> freed. This ensures that any readers i.e. xmit() that got a non-NULL
> peer will complete before the netdev is freed.
>
> Any readers after the RCU_INIT_POINTER() but before synchronize_rcu()
> will dereference NULL, making it safe.
>
> The codepath to nsim_destroy() and nsim_create() takes both the newly
> added nsim_dev_list_lock and rtnl_lock. This makes it safe with
> concurrent calls to linking two netdevsims together.
>
> Signed-off-by: David Wei <dw@...idwei.uk>
> ---
>  drivers/net/netdevsim/netdev.c    | 21 ++++++++++++++++++---
>  drivers/net/netdevsim/netdevsim.h |  1 +
>  2 files changed, 19 insertions(+), 3 deletions(-)
>


> @@ -302,7 +318,6 @@ static void nsim_setup(struct net_device *dev)
>         eth_hw_addr_random(dev);
>
>         dev->tx_queue_len = 0;
> -       dev->flags |= IFF_NOARP;

This part seems to be unrelated to this patch ?

>         dev->flags &= ~IFF_MULTICAST;
>         dev->priv_flags |= IFF_LIVE_ADDR_CHANGE |
>                            IFF_NO_QUEUE;