RE: [EXT] Re: [PATCH v4 2/2] net: dsa: ocelot: Add support for QinQ Operation

From: Hongbo Wang
Date: Tue Aug 04 2020 - 22:33:01 EST


> Caution: EXT Email
>
> On 8/3/2020 11:36 PM, Hongbo Wang wrote:
> >>> + if (vlan->proto == ETH_P_8021AD) {
> >>> + ocelot->enable_qinq = true;
> >>> + ocelot_port->qinq_mode = true;
> >>> + }
> >> ...
> >>> + if (vlan->proto == ETH_P_8021AD) {
> >>> + ocelot->enable_qinq = false;
> >>> + ocelot_port->qinq_mode = false;
> >>> + }
> >>> +
> >>
> >> I don't understand how this can work just by using a boolean to track
> >> the state.
> >>
> >> This won't work properly if you are handling multiple QinQ VLAN entries.
> >>
> >> Also, I need Andrew and Florian to review and ACK the DSA layer
> >> changes that add the protocol value to the device notifier block.
> >
> > Hi David,
> > Thanks for reply.
> >
> > When setting bridge's VLAN protocol to 802.1AD by the command "ip link
> > set br0 type bridge vlan_protocol 802.1ad", it will call
> > dsa_slave_vlan_rx_add(dev, proto, vid) for every port in the bridge,
> > the parameter vid is port's pvid 1, if pvid's proto is 802.1AD, I will
> > enable switch's enable_qinq, and the related port's qinq_mode,
> >
> > When there are multiple QinQ VLAN entries, If one VLAN's proto is 802.1AD,
> I will enable switch and the related port into QinQ mode.
>
> The enabling appears fine, the problem is the disabling, the first 802.1AD VLAN
> entry that gets deleted will lead to the port and switch no longer being in QinQ
> mode, and this does not look intended.
> --
> Florian

Thanks, Florian

I will add reference counter.
When deleting VLAN entry, only if the counter is zero, I will set the switch to exit QinQ mode.

hongbo