netdev - RE: [PATCH] net: dsa: ksz: Fix port membership

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: <SN1PR11MB04464EEB30A4CD8854894BBFECAA0@SN1PR11MB0446.namprd11.prod.outlook.com>
Date:   Fri, 7 Dec 2018 19:37:35 +0000
From:   <Tristram.Ha@...rochip.com>
To:     <marex@...x.de>
CC:     <f.fainelli@...il.com>, <vivien.didelot@...oirfairelinux.com>,
        <andrew@...n.ch>, <Woojung.Huh@...rochip.com>,
        <UNGLinuxDriver@...rochip.com>, <Woojung.Huh@...rochip.com>,
        <davem@...emloft.net>, <netdev@...r.kernel.org>
Subject: RE: [PATCH] net: dsa: ksz: Fix port membership

> If two ports are in the same bridge and in forwarding state, the packets
> must be able to pass between them in both directions. The current code
> only configures this bridge membership for a port newly added to the
> bridge, but does not update all the other ports. Thus, ingress packets
> on the new port will be forwarded, but ingress packets on other ports
> destined for the new port (eg. a reply) will not be forwarded back to
> the new port, because they are not configured to do so. This patch fixes
> that by updating the membership registers of all ports.
> 
> Signed-off-by: Marek Vasut <marex@...x.de>
> Cc: Vivien Didelot <vivien.didelot@...oirfairelinux.com>
> Cc: Woojung Huh <woojung.huh@...rochip.com>
> Cc: David S. Miller <davem@...emloft.net>
> Cc: Tristram Ha <Tristram.Ha@...rochip.com>
> ---
>  drivers/net/dsa/microchip/ksz9477.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/dsa/microchip/ksz9477.c
> b/drivers/net/dsa/microchip/ksz9477.c
> index 0684657fbf9a9..e24dd14ccde77 100644
> --- a/drivers/net/dsa/microchip/ksz9477.c
> +++ b/drivers/net/dsa/microchip/ksz9477.c
> @@ -396,7 +396,7 @@ static void ksz9477_port_stp_state_set(struct
> dsa_switch *ds, int port,
>  	struct ksz_device *dev = ds->priv;
>  	struct ksz_port *p = &dev->ports[port];
>  	u8 data;
> -	int member = -1;
> +	int i, member = -1;
> 
>  	ksz_pread8(dev, port, P_STP_CTRL, &data);
>  	data &= ~(PORT_TX_ENABLE | PORT_RX_ENABLE |
> PORT_LEARN_DISABLE);
> @@ -454,8 +454,8 @@ static void ksz9477_port_stp_state_set(struct
> dsa_switch *ds, int port,
>  		dev->tx_ports &= ~(1 << port);
> 
>  	/* Port membership may share register with STP state. */
> -	if (member >= 0 && member != p->member)
> -		ksz9477_cfg_port_member(dev, port, (u8)member);
> +	for (i = 0; i < SWITCH_PORT_NUM; i++)
> +		ksz9477_cfg_port_member(dev, i, (u8)member);
> 
>  	/* Check if forwarding needs to be updated. */
>  	if (state != BR_STATE_FORWARDING) {

The original DSA model did not have a way to tell the bridge device not to
forward the frame, so the switch driver always setup the membership to
disable forwarding between ports.

When lan devices are setup they act like individual devices.  A bridge device
adding them under it will forward the frames.

The new switchdev model adds the offload_fwd_mark bit to tell the bridge not to
forward frame.

The ksz_update_port_member function in ksz_common.c is doing this membership
setup for all forwarding ports.  It was finally enabled in one of the RFC patches I
submitted recently (Add switch forward offloading support).

I think if you do this without setting offload_fwd_mark you will receive duplicate
frame.