Re: [PATCH] bridge: extend BR_ISOLATE to full split-horizon

[Alexandra Winter <wintera@linux.ibm.com>]

On 09.12.21 13:14, David Lamparter wrote:
> Split-horizon essentially just means being able to create multiple
> groups of isolated ports that are isolated within the group, but not
> with respect to each other.
> 
> The intent is very different, while isolation is a policy feature,
> split-horizon is intended to provide functional "multiple member ports
> are treated as one for loop avoidance."  But it boils down to the same
> thing in the end.
> 
> Signed-off-by: David Lamparter <equinox@diac24.net>
> Cc: Nikolay Aleksandrov <nikolay@nvidia.com>
> Cc: Alexandra Winter <wintera@linux.ibm.com>
> ---
> 
> Alexandra, could you double check my change to the qeth_l2 driver?  I
> can't really test it...
> 
> Cheers,
> 
> David
> ---
Reviewed and tested for s390/qeth. Looks good to me, see 2 comments below.
Kind regards 
Alexandra


>  drivers/s390/net/qeth_l2_main.c | 10 ++++++----
>  include/linux/if_bridge.h       |  9 ++++++++-
>  include/uapi/linux/if_link.h    |  1 +
>  net/bridge/br_if.c              | 12 ++++++++++++
>  net/bridge/br_input.c           |  2 +-
>  net/bridge/br_netlink.c         | 33 +++++++++++++++++++++++++++++++--
>  net/bridge/br_private.h         | 13 ++++++++++---
>  net/bridge/br_sysfs_if.c        | 33 ++++++++++++++++++++++++++++++++-
>  8 files changed, 101 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/s390/net/qeth_l2_main.c b/drivers/s390/net/qeth_l2_main.c
> index 303461d70af3..405d36757c22 100644
> --- a/drivers/s390/net/qeth_l2_main.c
> +++ b/drivers/s390/net/qeth_l2_main.c
> @@ -729,8 +729,8 @@ static bool qeth_l2_must_learn(struct net_device *netdev,
>  	priv = netdev_priv(netdev);
>  	return (netdev != dstdev &&
>  		(priv->brport_features & BR_LEARNING_SYNC) &&
> -		!(br_port_flag_is_set(netdev, BR_ISOLATED) &&
> -		  br_port_flag_is_set(dstdev, BR_ISOLATED)) &&
> +		!(br_port_horizon_group(netdev) != 0 &&
> +		  br_port_horizon_group(netdev) == br_port_horizon_group(dstdev)) &&
WARNING: line length of 84 exceeds 80 columns

>  		(netdev->netdev_ops == &qeth_l2_iqd_netdev_ops ||
>  		 netdev->netdev_ops == &qeth_l2_osa_netdev_ops));
>  }
> @@ -757,6 +757,7 @@ static void qeth_l2_br2dev_worker(struct work_struct *work)
>  	struct net_device *lowerdev;
>  	struct list_head *iter;
>  	int err = 0;
> +	u32 horizon_group;
reverse Christmas tree!
>  
>  	kfree(br2dev_event_work);
>  	QETH_CARD_TEXT_(card, 4, "b2dw%04lx", event);
> @@ -770,12 +771,13 @@ static void qeth_l2_br2dev_worker(struct work_struct *work)
>  	if (!qeth_l2_must_learn(lsyncdev, dstdev))
>  		goto unlock;
>  
> -	if (br_port_flag_is_set(lsyncdev, BR_ISOLATED)) {
> +	horizon_group = br_port_horizon_group(lsyncdev);
> +	if (horizon_group) {
>  		/* Update lsyncdev and its isolated sibling(s): */
>  		iter = &brdev->adj_list.lower;
>  		lowerdev = netdev_next_lower_dev_rcu(brdev, &iter);
>  		while (lowerdev) {
> -			if (br_port_flag_is_set(lowerdev, BR_ISOLATED)) {
> +			if (br_port_horizon_group(lowerdev) == horizon_group) {
>  				switch (event) {
>  				case SWITCHDEV_FDB_ADD_TO_DEVICE:
>  					err = dev_uc_add(lowerdev, addr);

---snip---