diff mbox series

[RFC,1/4] net/core: Introduce master_xmit_slave_get

Message ID 20200126132126.9981-2-maorg@mellanox.com (mailing list archive)
State RFC
Headers show
Series Introduce master_xmit_slave_get | expand

Commit Message

Maor Gottlieb Jan. 26, 2020, 1:21 p.m. UTC
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@mellanox.com>
---
 include/linux/netdevice.h |  3 +++
 include/net/lag.h         | 19 +++++++++++++++++++
 2 files changed, 22 insertions(+)

Comments

Gal Pressman Jan. 29, 2020, 9:57 a.m. UTC | #1
On 26/01/2020 15:21, 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@mellanox.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);

Hey Maor,
Should lag be named flags?
Also, better to use unsigned type for it.
Maor Gottlieb Jan. 30, 2020, 3:40 p.m. UTC | #2
On 1/29/2020 11:57 AM, Gal Pressman wrote:
> On 26/01/2020 15:21, 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@mellanox.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);
> Hey Maor,
> Should lag be named flags?
> Also, better to use unsigned type for it.

Yeah, will change it.
Saeed Mahameed Feb. 12, 2020, 8:33 p.m. UTC | #3
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@mellanox.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))
diff mbox series

Patch

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);
+	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))