Message-ID: <9f771318-5a59-ac31-a333-e2ad9947679f@gmail.com>
Date:   Fri, 17 Mar 2023 09:54:59 -0700
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.7.1
Subject: Re: [PATCH] net: dsa: tag_brcm: legacy: fix daisy-chained switches
Content-Language: en-US
To:     Jonas Gorski <jonas.gorski@gmail.com>, Andrew Lunn <andrew@lunn.ch>
Cc:     =?UTF-8?Q?=c3=81lvaro_Fern=c3=a1ndez_Rojas?= <noltari@gmail.com>,
        olteanv@gmail.com, davem@davemloft.net, edumazet@google.com,
        kuba@kernel.org, pabeni@redhat.com, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <20230317120815.321871-1-noltari@gmail.com>
 <00783066-a99c-4bab-ae60-514f4bce687b@lunn.ch>
 <CAOiHx==TiSZKE4AP3PZ9Ah4zuAsrfpOTvRADWpT2kMS9UVRH9Q@mail.gmail.com>
From:   Florian Fainelli <f.fainelli@gmail.com>
In-Reply-To: <CAOiHx==TiSZKE4AP3PZ9Ah4zuAsrfpOTvRADWpT2kMS9UVRH9Q@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 3/17/23 09:49, Jonas Gorski wrote:
> On Fri, 17 Mar 2023 at 17:32, Andrew Lunn <andrew@lunn.ch> wrote:
>>
>> On Fri, Mar 17, 2023 at 01:08:15PM +0100, Álvaro Fernández Rojas wrote:
>>> When BCM63xx internal switches are connected to switches with a 4-byte
>>> Broadcom tag, it does not identify the packet as VLAN tagged, so it adds one
>>> based on its PVID (which is likely 0).
>>> Right now, the packet is received by the BCM63xx internal switch and the 6-byte
>>> tag is properly processed. The next step would to decode the corresponding
>>> 4-byte tag. However, the internal switch adds an invalid VLAN tag after the
>>> 6-byte tag and the 4-byte tag handling fails.
>>> In order to fix this we need to remove the invalid VLAN tag after the 6-byte
>>> tag before passing it to the 4-byte tag decoding.
>>
>> Is there an errata for this invalid VLAN tag? Or is the driver simply
>> missing some configuration for it to produce a valid VLAN tag?
>>
>> The description does not convince me you are fixing the correct
>> problem.
> 
> This isn't a bug per se, it's just the interaction of a packet going
> through two tagging CPU ports.
> 
> My understanding of the behaviour is:
> 
> 1. The external switch inserts a 4-byte Broadcom header before the
> VLAN tag, and sends it to the internal switch.
> 2. The internal switch looks at the EtherType, finds it is not a VLAN
> EtherType, so assumes it is untagged, and adds a VLAN tag based on the
> configured PVID (which 0 in the default case).
> 3. The internal switch inserts a legacy 6-byte Broadcom header before
> the VLAN tag when forwarding to its CPU port.
> 
> The internal switch does not know how to handle the (non-legacy)
> Broadcom tag, so it does not know that there is a VLAN tag after it.
> 
> The internal switch enforces VLAN tags on its CPU port when it is in
> VLAN enabled mode, regardless what the VLAN table's untag bit says.
> 
> The result is a bogus VID 0 and priority 0 tag between the two
> Broadcom Headers. The VID would likely change based on the PVID of the
> port of the external switch.

My understanding matches yours, at the very least, we should only strip 
off the VLAN tag == 0, in case we are stacked onto a 4-bytes Broadcom 
tag speaking switch, otherwise it seems to me we are stripping of VLAN 
tags a bait too greedily.
-- 
Florian