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

[Hongbo Wang <hongbo.wang@nxp.com>]

> 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