netdev - Re: [PATCH v3 RFC 4/4] net: dsa: hsr: Provide generic HSR ksz_hsr

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: <20230905140423.weisoaygc2tjvezb@skbuf>
Date: Tue, 5 Sep 2023 17:04:23 +0300
From: Vladimir Oltean <olteanv@...il.com>
To: Lukasz Majewski <lukma@...x.de>
Cc: Eric Dumazet <edumazet@...gle.com>, Andrew Lunn <andrew@...n.ch>,
	davem@...emloft.net, Paolo Abeni <pabeni@...hat.com>,
	Woojung Huh <woojung.huh@...rochip.com>, Tristram.Ha@...rochip.com,
	Florian Fainelli <f.fainelli@...il.com>,
	Jakub Kicinski <kuba@...nel.org>, UNGLinuxDriver@...rochip.com,
	George McCollister <george.mccollister@...il.com>,
	Oleksij Rempel <o.rempel@...gutronix.de>, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 RFC 4/4] net: dsa: hsr: Provide generic HSR
 ksz_hsr_{join|leave} functions

On Mon, Sep 04, 2023 at 02:02:09PM +0200, Lukasz Majewski wrote:
> +static int ksz_hsr_leave(struct dsa_switch *ds, int port,
> +			 struct net_device *hsr)
> +{
> +	struct dsa_port *partner = NULL, *dp;
> +	struct ksz_device *dev = ds->priv;
> +
> +	dsa_hsr_foreach_port(dp, ds, hsr) {
> +		if (dp->index != port) {
> +			partner = dp;
> +			break;
> +		}
> +	}
> +
> +	if (!partner)
> +		return 0;
> +
> +	switch (dev->chip_id) {
> +	case KSZ9477_CHIP_ID:
> +		return ksz9477_hsr_leave(ds, port, hsr, partner);
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +
> +	return 0;
> +}

It's hard for me to put this in the proper perspective in this email,
since ksz9477_hsr_leave() is implemented in a different patch, so I'm
just going to reproduce it here:

int ksz9477_hsr_leave(struct dsa_switch *ds, int port, struct net_device *hsr,
		      struct dsa_port *partner)
{
	struct ksz_device *dev = ds->priv;

	/* Clear ports HSR support */
	ksz_write32(dev, REG_HSR_PORT_MAP__4, 0);

	/* Disable forwarding frames between HSR ports */
	ksz_prmw32(dev, port, REG_PORT_VLAN_MEMBERSHIP__4, dev->hsr_ports, 0);
	ksz_prmw32(dev, partner->index, REG_PORT_VLAN_MEMBERSHIP__4,
		   dev->hsr_ports, 0);

	/* Disable per port self-address filtering */
	ksz_port_cfg(dev, port, REG_PORT_LUE_CTRL, PORT_SRC_ADDR_FILTER, false);
	ksz_port_cfg(dev, partner->index, REG_PORT_LUE_CTRL,
		     PORT_SRC_ADDR_FILTER, false);

	return 0;
}

The code pattern from ksz_hsr_leave() is to disable HSR offload in both
member ports, after *both* member ports have left the HSR device, correct?

So it means that after this set of commands:

ip link add name hsr0 type hsr slave1 lan1 slave2 lan2 supervision 45 version 1
ip link set dev lan1 up
ip link set dev lan2 up
ip link set lan1 nomaster

lan1 will still have HSR offload enabled, and forwarding enabled towards
lan2, correct? That's not good, because lan1 is now a standalone port
and should operate as such.