Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <252cdb0b-3630-9e29-45a6-ea0474f0d983@bootlin.com>
Date:   Fri, 11 Aug 2023 16:42:18 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH net-next v5 2/3] net: dsa: rzn1-a5psw: add support for
 .port_bridge_flags
To:     Vladimir Oltean <olteanv@gmail.com>
Cc:     =?UTF-8?Q?Cl=C3=A9ment_Leger?= <clement@clement-leger.fr>,
        Andrew Lunn <andrew@lunn.ch>,
        Florian Fainelli <f.fainelli@gmail.com>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        linux-renesas-soc@vger.kernel.org, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        Miquel Raynal <miquel.raynal@bootlin.com>,
        Milan Stevanovic <milan.stevanovic@se.com>,
        Jimmy Lalande <jimmy.lalande@se.com>,
        Pascal Eberhard <pascal.eberhard@se.com>,
        Thomas Petazzoni <thomas.petazzoni@bootlin.com>
References: <20230810093651.102509-1-alexis.lothore@bootlin.com>
 <20230810093651.102509-3-alexis.lothore@bootlin.com>
 <20230811100307.ocqkijjj5f6hi3q2@skbuf>
Content-Language: en-US
From:   =?UTF-8?Q?Alexis_Lothor=C3=A9?= <alexis.lothore@bootlin.com>
In-Reply-To: <20230811100307.ocqkijjj5f6hi3q2@skbuf>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk

Hello Vladimir,
On 8/11/23 12:03, Vladimir Oltean wrote:
> Hi Alexis,
> 
> On Thu, Aug 10, 2023 at 11:36:50AM +0200, alexis.lothore@bootlin.com wrote:
>> +	if (flags.mask & BR_FLOOD) {
>> +		val = flags.val & BR_FLOOD ? BIT(port) : 0;
>> +		a5psw_reg_rmw(a5psw, A5PSW_UCAST_DEF_MASK, BIT(port), val);
>> +	}
>> +
>> +	if (flags.mask & BR_MCAST_FLOOD) {
>> +		val = flags.val & BR_MCAST_FLOOD ? BIT(port) : 0;
>> +		a5psw_reg_rmw(a5psw, A5PSW_MCAST_DEF_MASK, BIT(port), val);
>> +	}
>> +
>> +	if (flags.mask & BR_BCAST_FLOOD) {
>> +		val = flags.val & BR_BCAST_FLOOD ? BIT(port) : 0;
>> +		a5psw_reg_rmw(a5psw, A5PSW_BCAST_DEF_MASK, BIT(port), val);
>> +	}
> 
> These 3 port masks will only do what you expect while the bridge has
> vlan_filtering=0, correct? When vlan_filtering=1, packets classified to
> a VLAN which don't hit any FDB entry will be always flooded to all ports
> in that VLAN, correct?

After thoroughly reading the A5PSW doc again, I feel that this sentence is not
exactly true. If I refer to section 4.5.3.9, paragraph 3.c:

The VLAN table is used for both, VLAN domain verification [...] as well as VLAN
resolution. Once the frame has passed any VLAN domain verification (i.e. will
not be discarded by the verification function already), the forwarding
resolution applies.
[...]
- If the destination MAC address (Unicast or Multicast) is not found in the MAC
address table, or if the destination address is the Broadcast address, the frame
is forwarded according to the following rules:
  - The destination port mask is loaded from the respective register
U/M/BCAST_DEFAULT_MASK depending on unicast, multicast or broadcast. Then the
following filtering on this mask applies.
    - If the frame carries a VLAN tag, the VLAN resolution table is searched for
a matching VLAN ID and the frame is sent only to ports that are associated with
the VLAN ID.
    - If the frame carries a VLAN tag and the VLAN ID does not match any entry
in the VLAN Resolution Table, or the frame does not carry a VLAN tag, the frame
is forwarded to all ports that are enabled by the default mask.
    - If it cannot be associated with any VLAN group and if the default group
has been set to all zero, the frame is discarded.
[...]

I understand from the second bullet that even when vlan filtering is enabled
(which occurs as first step), the first flooding filter (used in second step,
resolution) remains the flooding masks from unicast/multicast/broadcast default
mask registers. The vlan resolution is then applied over it as a second filter,
and only make the flooding more "restrictive", it does not bypass it (so if a
port is in the vlan which VID is in an incoming packet but the port is not also
defined in the U/M/B default mask, incoming packet won't be flooded to it).
> 
> Maybe you could restrict transitions to flooding disabled on ports with
> vlan_filtering 1, and restrict transitions to vlan_filtering 1 on ports
> with flooding disabled. Or at least add some comments about the
> limitations. I wouldn't want subtle incompatibilities between the
> hardware design and Linux' expectations to go under the radar like this.
> 

-- 
Alexis Lothoré, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com