netdev - Re: [RFC PATCH 1/4] net/core: Introduce master_xmit_slave

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] [day] [month] [year] [list]

Message-ID: <ddd3dccc215db626e6e3195555a6dece39422ce1.camel@mellanox.com>
Date:   Wed, 12 Feb 2020 20:33:08 +0000
From:   Saeed Mahameed <saeedm@...lanox.com>
To:     "vfalico@...il.com" <vfalico@...il.com>,
        Maor Gottlieb <maorg@...lanox.com>,
        "andy@...yhouse.net" <andy@...yhouse.net>,
        "j.vosburgh@...il.com" <j.vosburgh@...il.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        Jiri Pirko <jiri@...lanox.com>
CC:     Jason Gunthorpe <jgg@...lanox.com>,
        "linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
        Leon Romanovsky <leonro@...lanox.com>,
        Mark Zhang <markz@...lanox.com>,
        Parav Pandit <parav@...lanox.com>,
        Eran Ben Elisha <eranbe@...lanox.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Alex Rosenbaum <alexr@...lanox.com>
Subject: Re: [RFC PATCH 1/4] net/core: Introduce master_xmit_slave_get

On Sun, 2020-01-26 at 15:21 +0200, Maor Gottlieb wrote:
> Add new ndo to get the xmit slave of master device.
> When slave selection method is based on hash, then the user can ask
> to
> get the xmit slave assume all the slaves can transmit by setting the
> LAG_FLAGS_HASH_ALL_SLAVES bit in the flags argument.
> 
> Signed-off-by: Maor Gottlieb <maorg@...lanox.com>
> ---
>  include/linux/netdevice.h |  3 +++
>  include/net/lag.h         | 19 +++++++++++++++++++
>  2 files changed, 22 insertions(+)
> 
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 11bdf6cb30bd..faba4aa094e5 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -1379,6 +1379,9 @@ struct net_device_ops {
>  						 struct netlink_ext_ack
> *extack);
>  	int			(*ndo_del_slave)(struct net_device
> *dev,
>  						 struct net_device
> *slave_dev);
> +	struct net_device*	(*ndo_xmit_slave_get)(struct
> net_device *master_dev,
> +						      struct sk_buff
> *skb,
> +						      int lag);
>  	netdev_features_t	(*ndo_fix_features)(struct net_device *dev,
>  						    netdev_features_t
> features);
>  	int			(*ndo_set_features)(struct net_device
> *dev,
> diff --git a/include/net/lag.h b/include/net/lag.h
> index 95b880e6fdde..c710daf8f57a 100644
> --- a/include/net/lag.h
> +++ b/include/net/lag.h
> @@ -6,6 +6,25 @@
>  #include <linux/if_team.h>
>  #include <net/bonding.h>
>  
> +enum lag_get_slaves_flags {
> +	LAG_FLAGS_HASH_ALL_SLAVES = 1<<0
> +};
> +
> +static inline
> +struct net_device *master_xmit_slave_get(struct net_device
> *master_dev,
> +					 struct sk_buff *skb,
> +					 int flags)
> +{
> +	const struct net_device_ops *ops = master_dev->netdev_ops;
> +	struct net_device *slave = NULL;
> +
> +	rcu_read_lock();
> +	if (ops->ndo_xmit_slave_get)
> +		slave = ops->ndo_xmit_slave_get(master_dev, skb,
> flags);

what is the purpose of the rcu ? Aren't you supposed to dev_hold(slave)
under the rcu ?

and the caller should be responsible to issue the dev_put() .. 

otherwise slave is not guaranteed to stick around after this ndo
returns. or i am missing some assumptions that are not listed in this
patchset commit messages or cover letter.

Please clarify.


> +	rcu_read_unlock();
> +	return slave;
> +}
> +
>  static inline bool net_lag_port_dev_txable(const struct net_device
> *port_dev)
>  {
>  	if (netif_is_team_port(port_dev))