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: <5903f62b-5ab5-0600-019d-f129e6216c2d@denx.de>
Date:   Fri, 7 Dec 2018 20:56:03 +0100
From:   Marek Vasut <marex@...x.de>
To:     Tristram.Ha@...rochip.com
Cc:     f.fainelli@...il.com, vivien.didelot@...oirfairelinux.com,
        andrew@...n.ch, Woojung.Huh@...rochip.com,
        UNGLinuxDriver@...rochip.com, davem@...emloft.net,
        netdev@...r.kernel.org
Subject: Re: [PATCH] net: dsa: ksz: Fix port membership

On 12/07/2018 08:37 PM, Tristram.Ha@...rochip.com wrote:
>> 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.

Do you have a git tree with all the KSZ patches based on -next
somewhere, so I don't have to look for them in random MLs ?

-- 
Best regards,
Marek Vasut