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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2556a385-425c-0f25-2be5-efcfdc77aeaa@linux.ibm.com>
Date:   Fri, 10 Dec 2021 17:53:17 +0100
From:   Alexandra Winter <wintera@...ux.ibm.com>
To:     David Lamparter <equinox@...c24.net>, netdev@...r.kernel.org
Cc:     Nikolay Aleksandrov <nikolay@...dia.com>
Subject: Re: [PATCH] bridge: extend BR_ISOLATE to full split-horizon



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@...c24.net>
> Cc: Nikolay Aleksandrov <nikolay@...dia.com>
> Cc: Alexandra Winter <wintera@...ux.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---