On Sun, Oct 01, 2017 at 12:46:35PM -0700, Florian Fainelli wrote:
> Add the necessary logic to support network device events targetting LAG 
> events,
> this is loosely inspired from mlxsw/spectrum.c.
> 
> In the process we change dsa_slave_changeupper() to be more generic and be 
> called
> from both LAG events as well as normal bridge enslaving events paths.
> 
> The DSA layer takes care of managing the LAG group identifiers, how many LAGs
> may be supported by a switch, and how many members per LAG are supported by a
> switch device. When a LAG group is identified, the port is then configured to
> be a part of that group. When a LAG group no longer has any users, we remove 
> it
> and we tell the drivers whether it is safe to disable trunking altogether.
> 
> Signed-off-by: Florian Fainelli <f.faine...@gmail.com>
> ---
>  include/net/dsa.h  |  25 +++++++++
>  net/dsa/dsa2.c     |  12 ++++
>  net/dsa/dsa_priv.h |   7 +++
>  net/dsa/port.c     |  92 +++++++++++++++++++++++++++++++
>  net/dsa/slave.c    | 157 
> +++++++++++++++++++++++++++++++++++++++++++++++++----
>  net/dsa/switch.c   |  30 ++++++++++
>  6 files changed, 312 insertions(+), 11 deletions(-)
> 
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index 10dceccd9ce8..247ea58add68 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -182,12 +182,20 @@ struct dsa_port {
>       u8                      stp_state;
>       struct net_device       *bridge_dev;
>       struct devlink_port     devlink_port;
> +     u8                      lag_id;
> +     bool                    lagged;
>       /*
>        * Original copy of the master netdev ethtool_ops
>        */
>       const struct ethtool_ops *orig_ethtool_ops;
>  };
>  
> +struct dsa_lag_group {
> +     /* Used to know when we can disable lag on the switch */
> +     unsigned int            ref_count;

Hi Florian

In what contexts is ref_count manipulated. Normally you use would
refcounf_t and the operations in linux/refcount.h. But if you know
there is some other protection, e.g. rtnl, an unsigned int is O.K.
Maybe scatter some assert_RTNL() in the code?

> +static bool dsa_slave_lag_check(struct net_device *dev, struct net_device 
> *lag_dev,
> +                             struct netdev_lag_upper_info *lag_upper_info)
> +{
> +     struct dsa_slave_priv *p = netdev_priv(dev);
> +     u8 lag_id;
> +
> +     /* No more lag identifiers available or already in use */
> +     if (dsa_switch_lag_get_index(p->dp->ds, lag_dev, &lag_id) != 0)
> +             return false;
> +
> +     if (lag_upper_info->tx_type != NETDEV_LAG_TX_TYPE_HASH)
> +             return false;

I wounder if the driver needs to decide this? Can different hardware
support different tx_types?

        Andrew

Reply via email to