[net-next,v2,0/2] net: dsa: b53: Configure VLANs while not filtering

Hi David, Jakub,

These two patches allow the b53 driver which always configures its CPU
port as egress tagged to behave correctly with VLANs being always
configured whenever a port is added to a bridge.

Vladimir provides a patch that aligns the bridge with vlan_filtering=0
receive path to behave the same as vlan_filtering=1. Per discussion with
Nikolay, this behavior is deemed to be too DSA specific to be done in
the bridge proper.

This is a preliminary series for Vladimir to make
configure_vlan_while_filtering the default behavior for all DSA drivers
in the future.

Thanks!

Changes in v2:

- moved the call to dsa_untag_bridge_pvid() into net/dsa/tag_brcm.c
  since we have a single user for now

Florian Fainelli (1):
  net: dsa: b53: Configure VLANs while not filtering

Vladimir Oltean (1):
  net: dsa: untag the bridge pvid from rx skbs

 drivers/net/dsa/b53/b53_common.c | 19 +--------
 drivers/net/dsa/b53/b53_priv.h   |  1 -
 net/dsa/dsa_priv.h               | 66 ++++++++++++++++++++++++++++++++
 net/dsa/tag_brcm.c               | 16 +++++++-
 4 files changed, 82 insertions(+), 20 deletions(-)

On 9/23/20 1:45 PM, Florian Fainelli wrote:
> Hi David, Jakub,
> 
> These two patches allow the b53 driver which always configures its CPU
> port as egress tagged to behave correctly with VLANs being always
> configured whenever a port is added to a bridge.
> 
> Vladimir provides a patch that aligns the bridge with vlan_filtering=0
> receive path to behave the same as vlan_filtering=1. Per discussion with
> Nikolay, this behavior is deemed to be too DSA specific to be done in
> the bridge proper.
> 
> This is a preliminary series for Vladimir to make
> configure_vlan_while_filtering the default behavior for all DSA drivers
> in the future.

David, Jakub, there is an unnecessary header inclusion in
net/dsa/tag_brcm.c in the second patch and the description at the end of
the commit was not updated, let me send a v3 right away.

On Wed, Sep 23, 2020 at 01:49:54PM -0700, Florian Fainelli wrote:
> On 9/23/20 1:45 PM, Florian Fainelli wrote:

> 

> David, Jakub, there is an unnecessary header inclusion in

> net/dsa/tag_brcm.c in the second patch and the description at the end of

> the commit was not updated, let me send a v3 right away.

> -- 

> Florian


Wait a few minutes, I don't think anybody has had a chance to look at it..

Thanks,
-Vladimir

On Wed, Sep 23, 2020 at 11:51:48PM +0300, Vladimir Oltean wrote:
> On Wed, Sep 23, 2020 at 01:49:54PM -0700, Florian Fainelli wrote:
> > On 9/23/20 1:45 PM, Florian Fainelli wrote:
> >
> > David, Jakub, there is an unnecessary header inclusion in
> > net/dsa/tag_brcm.c in the second patch and the description at the end of
> > the commit was not updated, let me send a v3 right away.
> > --
> > Florian
>
> Wait a few minutes, I don't think anybody has had a chance to look at it..

So I studied a little bit the situation with the MAC header pointer in
the second patch, especially since there are 2 call paths, but it's ok,
since at that point the skb->data points to its final position already,
then the MAC header is naturally 14 bytes behind.
I've been testing with the ocelot driver which resets the MAC header
manually, that explains why I missed that. Anyway, if there are no other
comments I think you can resend.

On 9/23/20 2:30 PM, Vladimir Oltean wrote:
> On Wed, Sep 23, 2020 at 11:51:48PM +0300, Vladimir Oltean wrote:

>> On Wed, Sep 23, 2020 at 01:49:54PM -0700, Florian Fainelli wrote:

>>> On 9/23/20 1:45 PM, Florian Fainelli wrote:

>>>

>>> David, Jakub, there is an unnecessary header inclusion in

>>> net/dsa/tag_brcm.c in the second patch and the description at the end of

>>> the commit was not updated, let me send a v3 right away.

>>> --

>>> Florian

>>

>> Wait a few minutes, I don't think anybody has had a chance to look at it..

> 

> So I studied a little bit the situation with the MAC header pointer in

> the second patch, especially since there are 2 call paths, but it's ok,

> since at that point the skb->data points to its final position already,

> then the MAC header is naturally 14 bytes behind.

> I've been testing with the ocelot driver which resets the MAC header

> manually, that explains why I missed that. Anyway, if there are no other

> comments I think you can resend.


Yes, I was looking at tag_ocelot.c as well for a reference. At some
point I was considering adding a version that let the tagging driver
specifying the struct vlan_ethhdr reference directly since the tagging
driver knows, but using skb_set_mac_header() works equally well in a
more portable way even.

Thanks, v3 on its way with your A-b tag.
-- 
Florian