From mboxrd@z Thu Jan 1 00:00:00 1970 From: Florian Fainelli Subject: Re: [PATCH RFT] net: dsa: Allow configuring CPU port VLANs Date: Tue, 28 Aug 2018 09:58:05 -0700 Message-ID: References: <20180624153339.13572-1-f.fainelli@gmail.com> <20180625091713.GA13442@apalos> <9ce291a4-b40d-81d8-1c1a-c4311e5cc113@gmail.com> <20180828083257.GA10872@apalos> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: Petr Machata , netdev@vger.kernel.org, jiri@mellanox.com, Andrew Lunn , Vivien Didelot , "David S. Miller" , open list To: Ilias Apalodimas Return-path: In-Reply-To: <20180828083257.GA10872@apalos> Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org List-Id: netdev.vger.kernel.org On 08/28/2018 01:32 AM, Ilias Apalodimas wrote: > On Fri, Aug 10, 2018 at 04:58:10PM -0700, Florian Fainelli wrote: >> On 06/25/2018 02:17 AM, Ilias Apalodimas wrote: >>> On Mon, Jun 25, 2018 at 12:13:10PM +0300, Petr Machata wrote: >>>> Florian Fainelli writes: >>>> >>>>> if (netif_is_bridge_master(vlan->obj.orig_dev)) >>>>> - return -EOPNOTSUPP; >>>>> + info.port = dp->cpu_dp->index; >>>> >>>> The condition above will trigger also when a VLAN is added on a member >>>> port, and there's no other port with that VLAN. In that case the VLAN >>>> comes without the BRIDGE_VLAN_INFO_BRENTRY flag. In mlxsw we have this >>>> to get the bridge VLANs: >>>> >>>> if (netif_is_bridge_master(orig_dev)) { >>>> [...] >>>> if ((vlan->flags & BRIDGE_VLAN_INFO_BRENTRY) && >>>> [...] >>>> >>>> This doesn't appear to be done in DSA unless I'm missing something. >>> Petr's right. This will trigger for VLANs added on 'not cpu ports' if the VLAN >>> is not already a member. >>> >>> This command has BRIDGE_VLAN_INFO_BRENTRY set: >>> bridge vlan add dev br0 vid 100 pvid untagged self >>> I had the same issue on my CPSW RFC and solved it >>> exactly the same was as Petr suggested. >> >> Humm, there must be something obvious I am missing, but the following >> don't exactly result in what I would expect after adding a check for >> vlan->flags & BRIDGE_VLAN_INFO_BRENTRY: >> >> brctl addbr br0 >> echo 1 > /sys/class/net/br0/bridge/vlan_filtering >> brctl addif br0 lan1 >> >> #1 results in lan1 being programmed with VID 1, PVID, untagged, but not >> the CPU port. I would have sort of expected that the bridge layer would >> also push the configuration to br0/CPU port since this is the default VLAN: >> >> bridge vlan show dev br0 >> port vlan ids >> br0 1 PVID Egress Untagged >> >> But it does not. >> >> bridge vlan add vid 2 dev lan1 >> >> #2 same thing, results in only lan1 being programmed with VID 2, tagged >> but that is expected because we are creating the VLAN only for the >> user-facing port. >> >> bridge vlan add vid 3 dev br0 self >> >> #3 results in the CPU port being programmed with VID 3, tagged, again, >> this is expected because we are only programming the bridge master/CPU >> port here. >> >> Does #1 also happen for cpsw and mlxsw or do you actually get events >> about the bridge's default VLAN configuration? Or does the switch driver >> actually need to obtain that at the time the port is enslaved somehow? > As long as ports are attached you get the events (one event per attached port > iirc) > if the event is checked against BRIDGE_VLAN_INFO_BRENTRY, the only way to add a > VLAN to the cpu port is via 'bridge vlan add vid 3 dev br0 self' Do we have a guarantee that upon port enslavement, whatever default_pvid is configured on the bridge master device also happens to be the port's default_pvid settings as well? -- Florian