Mon, Apr 20, 2020 at 09:54:17AM CEST, maorg@xxxxxxxxxxxx wrote:
>Add new ndo to get the xmit slave of master device.
>User should release the slave when it's not longer needed.
>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@xxxxxxxxxxxx>
>---
> include/linux/netdevice.h | 3 +++
> include/net/lag.h | 32 ++++++++++++++++++++++++++++++++
> 2 files changed, 35 insertions(+)
>
>diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>index 130a668049ab..e8852f3ad0b6 100644
>--- a/include/linux/netdevice.h
>+++ b/include/linux/netdevice.h
>@@ -1389,6 +1389,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_get_slave)(struct net_device *master_dev,
>+ struct sk_buff *skb,
>+ u16 flags);
Please adjust the name to:
ndo_get_lag_xmit_slave
> 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..c43b035989c4 100644
>--- a/include/net/lag.h
>+++ b/include/net/lag.h
>@@ -6,6 +6,38 @@
> #include <linux/if_team.h>
> #include <net/bonding.h>
>
>+enum lag_get_slaves_flags {
>+ LAG_FLAGS_HASH_ALL_SLAVES = 1<<0
Enum name and the values should be in sync. Also sync with the ndo name.
Why exactly do you need these flags? Do you anticipate more of them?
A simple bool arg to the ndo would do I believe. Can be changed later if
needed.
>+};
>+
>+/**
>+ * master_xmit_slave_get - Get the xmit slave of master device
>+ * @skb: The packet
>+ * @flags: lag_get_slaves_flags
>+ *
>+ * This can be called from any context and does its own locking.
>+ * The returned handle has the usage count incremented and the caller must
>+ * use dev_put() to release it when it is no longer needed.
>+ * %NULL is returned if no slave is found.
>+ */
>+
>+static inline
>+struct net_device *master_xmit_get_slave(struct net_device *master_dev,
Please honor the namespace:
net_lag_get_xmit_slave
Also, just "struct net_device *dev" would be enough.
>+ struct sk_buff *skb,
>+ u16 flags)
>+{
>+ const struct net_device_ops *ops = master_dev->netdev_ops;
>+ struct net_device *slave = NULL;
"slave_dev" please.
Just check the ndo here and return NULL if it is not defined. That way
you avoid unnecessary NULL initialization and rcu lock.
>+
>+ rcu_read_lock();
>+ if (ops->ndo_xmit_get_slave)
>+ slave = ops->ndo_xmit_get_slave(master_dev, skb, flags);
>+ if (slave)
>+ dev_hold(slave);
>+ 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))
>--
>2.17.2
>
