On Wed, Dec 16, 2020 at 05:00:54PM +0100, Tobias Waldekranz wrote:
> diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
> index 183003e45762..deee4c0ecb31 100644
> --- a/net/dsa/dsa2.c
> +++ b/net/dsa/dsa2.c
> @@ -21,6 +21,46 @@
>  static DEFINE_MUTEX(dsa2_mutex);
>  LIST_HEAD(dsa_tree_list);
>
> +void dsa_lag_map(struct dsa_switch_tree *dst, struct net_device *lag)

Maybe a small comment here and in dsa_lag_unmap, describing what they're
for? They look a bit bland. Just a few words about the linear array will
suffice.

> +{
> +     unsigned int id;
> +
> +     if (dsa_lag_id(dst, lag) >= 0)
> +             /* Already mapped */
> +             return;
> +
> +     for (id = 0; id < dst->lags_len; id++) {
> +             if (!dsa_lag_dev(dst, id)) {
> +                     dst->lags[id] = lag;
> +                     return;
> +             }
> +     }
> +
> +     /* No IDs left, which is OK. Some drivers do not need it. The
> +      * ones that do, e.g. mv88e6xxx, will discover that
> +      * dsa_tree_lag_id returns an error for this device when
> +      * joining the LAG. The driver can then return -EOPNOTSUPP
> +      * back to DSA, which will fall back to a software LAG.
> +      */
> +}
> +
> +void dsa_lag_unmap(struct dsa_switch_tree *dst, struct net_device *lag)
> +{
> +     struct dsa_port *dp;
> +     unsigned int id;
> +
> +     dsa_lag_foreach_port(dp, dst, lag)
> +             /* There are remaining users of this mapping */
> +             return;
> +
> +     dsa_lags_foreach_id(id, dst) {
> +             if (dsa_lag_dev(dst, id) == lag) {
> +                     dst->lags[id] = NULL;
> +                     break;
> +             }
> +     }
> +}
> diff --git a/net/dsa/port.c b/net/dsa/port.c
> index 73569c9af3cc..121e5044dbe7 100644
> --- a/net/dsa/port.c
> +++ b/net/dsa/port.c
> @@ -193,6 +193,85 @@ void dsa_port_bridge_leave(struct dsa_port *dp, struct 
> net_device *br)
>       dsa_port_set_state_now(dp, BR_STATE_FORWARDING);
>  }
>
> +int dsa_port_lag_change(struct dsa_port *dp,
> +                     struct netdev_lag_lower_state_info *linfo)
> +{
> +     struct dsa_notifier_lag_info info = {
> +             .sw_index = dp->ds->index,
> +             .port = dp->index,
> +     };
> +     bool tx_enabled;
> +
> +     if (!dp->lag_dev)
> +             return 0;
> +
> +     /* On statically configured aggregates (e.g. loadbalance
> +      * without LACP) ports will always be tx_enabled, even if the
> +      * link is down. Thus we require both link_up and tx_enabled
> +      * in order to include it in the tx set.
> +      */
> +     tx_enabled = linfo->link_up && linfo->tx_enabled;
> +
> +     if (tx_enabled == dp->lag_tx_enabled)
> +             return 0;

Why would we get a NETDEV_CHANGELOWERSTATE notification if tx_enabled ==
dp->lag_tx_enabled? What is it that changed?

> +
> +     dp->lag_tx_enabled = tx_enabled;
> +
> +     return dsa_port_notify(dp, DSA_NOTIFIER_LAG_CHANGE, &info);
> +}

I am very happy with how simple this turned out. Thanks for the patience.
You can add these tags when you resend once net-next opens.

Reviewed-by: Vladimir Oltean <olte...@gmail.com>
Tested-by: Vladimir Oltean <olte...@gmail.com>

Reply via email to