> +static int dsa_tree_setup_lags(struct dsa_switch_tree *dst)
> +{
> + struct dsa_port *dp;
> + unsigned int num;
> +
> + list_for_each_entry(dp, &dst->ports, list)
> + num = dp->ds->num_lags;
> +
> + list_for_each_entry(dp, &dst->ports, list)
> + num = min(num, dp->ds->num_lags);
Do you really need to loop over the list twice? Cannot num be
initialised to UINT_MAX and then just do the second loop.
> +static inline bool dsa_port_can_offload(struct dsa_port *dp,
> + struct net_device *dev)
That name is a bit generic. We have a number of different offloads.
The mv88E6060 cannot offload anything!
> +{
> + /* Switchdev offloading can be configured on: */
> +
> + if (dev == dp->slave)
> + /* DSA ports directly connected to a bridge. */
> + return true;
> +
> + if (dp->lag && dev == rtnl_dereference(dp->lag->dev))
> + /* DSA ports connected to a bridge via a LAG */
> + return true;
> +
> + return false;
> +}
> +static void dsa_lag_put(struct dsa_switch_tree *dst, struct dsa_lag *lag)
> +{
> + if (!refcount_dec_and_test(&lag->refcount))
> + return;
> +
> + clear_bit(lag->id, dst->lags.busy);
> + WRITE_ONCE(lag->dev, NULL);
> + memset(lag, 0, sizeof(*lag));
> +}
I don't know what the locking is here, but wouldn't it be safer to
clear the bit last, after the memset and WRITE_ONCE.
Andrew
