[PATCH] net: dsa: mv88e6xxx: egress all frames

[PATCH] net: dsa: mv88e6xxx: egress all frames

[Stefan Eichenberger]

Egress multicast and egress unicast is only enabled for CPU/DSA ports
but for switching operation it seems it should be enabled for all ports.
Do I miss something here?

I did the following test:
brctl addbr br0
brctl addif br0 lan0
brctl addif br0 lan1

In this scenario the unicast and multicast packets were not forwarded,
therefore ARP requests were not resolved, and no connection could be
established.

If no bridge is configured we do not forward unicast and multicast
packets because the VLAN mapping is active.

Signed-off-by: Stefan Eichenberger <stefan.eichenberger@netmodule.com>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 883fd98..fe76372 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -2506,15 +2506,14 @@ static int mv88e6xxx_setup_port(struct mv88e6xxx_chip *chip, int port)
 	    mv88e6xxx_6185_family(chip) || mv88e6xxx_6320_family(chip))
 		reg = PORT_CONTROL_IGMP_MLD_SNOOP |
 		PORT_CONTROL_USE_TAG | PORT_CONTROL_USE_IP |
-		PORT_CONTROL_STATE_FORWARDING;
+		PORT_CONTROL_STATE_FORWARDING |
+		PORT_CONTROL_FORWARD_UNKNOWN_MC | PORT_CONTROL_FORWARD_UNKNOWN;
 	if (dsa_is_cpu_port(ds, port)) {
 		if (mv88e6xxx_has(chip, MV88E6XXX_FLAG_EDSA))
-			reg |= PORT_CONTROL_FRAME_ETHER_TYPE_DSA |
-				PORT_CONTROL_FORWARD_UNKNOWN_MC;
+			reg |= PORT_CONTROL_FRAME_ETHER_TYPE_DSA;
 		else
 			reg |= PORT_CONTROL_DSA_TAG;
-		reg |= PORT_CONTROL_EGRESS_ADD_TAG |
-			PORT_CONTROL_FORWARD_UNKNOWN;
+		reg |= PORT_CONTROL_EGRESS_ADD_TAG;
 	}
 	if (dsa_is_dsa_port(ds, port)) {
 		if (mv88e6xxx_6095_family(chip) ||
-- 
2.9.3

Re: [PATCH] net: dsa: mv88e6xxx: egress all frames

[Andrew Lunn]

On Tue, Nov 22, 2016 at 11:39:44AM +0100, Stefan Eichenberger wrote:
> Egress multicast and egress unicast is only enabled for CPU/DSA ports
> but for switching operation it seems it should be enabled for all ports.
> Do I miss something here?
> 
> I did the following test:
> brctl addbr br0
> brctl addif br0 lan0
> brctl addif br0 lan1
> 
> In this scenario the unicast and multicast packets were not forwarded,
> therefore ARP requests were not resolved, and no connection could be
> established.

Hi Stefan

This is probably specific to the 6097 family. It works fine without
this on other devices. Creating a bridge like above and pinging across
it is one of my standard tests. But i only test modern devices like
the 6165, 6352, 6351, 6390 families.

In fact, you might need to review all the code and look where
mv88e6xxx_6095_family(chip) is used and consider if you need to add
mv88e6xxx_6097_family(chip). e.g.

        if (mv88e6xxx_6095_family(chip) || mv88e6xxx_6185_family(chip)) {
                /* Set the upstream port this port should use */
                reg |= dsa_upstream_port(ds);
                /* enable forwarding of unknown multicast addresses to
                 * the upstream port
                 */
                if (port == dsa_upstream_port(ds))
                        reg |= PORT_CONTROL_2_FORWARD_UNKNOWN;
        }

Maybe this is your problem?

	Andrew

Re: [PATCH] net: dsa: mv88e6xxx: egress all frames

[Stefan Eichenberger]

Hi Andrew

On Tue, Nov 22, 2016 at 04:03:30PM +0100, Andrew Lunn wrote:
> On Tue, Nov 22, 2016 at 11:39:44AM +0100, Stefan Eichenberger wrote:
> > Egress multicast and egress unicast is only enabled for CPU/DSA ports
> > but for switching operation it seems it should be enabled for all ports.
> > Do I miss something here?
> > 
> > I did the following test:
> > brctl addbr br0
> > brctl addif br0 lan0
> > brctl addif br0 lan1
> > 
> > In this scenario the unicast and multicast packets were not forwarded,
> > therefore ARP requests were not resolved, and no connection could be
> > established.
> 
> Hi Stefan
> 
> This is probably specific to the 6097 family. It works fine without
> this on other devices. Creating a bridge like above and pinging across
> it is one of my standard tests. But i only test modern devices like
> the 6165, 6352, 6351, 6390 families.

Okay perfect, I wasn't 100% sure if I would have to configure something
additionally.

> 
> In fact, you might need to review all the code and look where
> mv88e6xxx_6095_family(chip) is used and consider if you need to add
> mv88e6xxx_6097_family(chip). e.g.
> 
>         if (mv88e6xxx_6095_family(chip) || mv88e6xxx_6185_family(chip)) {
>                 /* Set the upstream port this port should use */
>                 reg |= dsa_upstream_port(ds);
>                 /* enable forwarding of unknown multicast addresses to
>                  * the upstream port
>                  */
>                 if (port == dsa_upstream_port(ds))
>                         reg |= PORT_CONTROL_2_FORWARD_UNKNOWN;
>         }
> 
> Maybe this is your problem?

I think I still don't understand exactly how the driver works.

My problem is that the multicast and broadcast frames are filtered and
the following counter is increasing in ethtool:
sw_in_filtered: 596

This makes sense because "Egress Floods" in the Port Control Register is
set to 0. What kind of mechanism should make sure that for example ARP
packets are sent trough all ports anyway?

Unfortunately I don't have any devices available with more modern
devices, so I can't double check the registers.

Regards,
Stefan

Re: [PATCH] net: dsa: mv88e6xxx: egress all frames

[Andrew Lunn]

On Tue, Nov 22, 2016 at 07:37:33PM +0100, Stefan Eichenberger wrote:
> Hi Andrew
> 
> On Tue, Nov 22, 2016 at 04:03:30PM +0100, Andrew Lunn wrote:
> > On Tue, Nov 22, 2016 at 11:39:44AM +0100, Stefan Eichenberger wrote:
> > > Egress multicast and egress unicast is only enabled for CPU/DSA ports
> > > but for switching operation it seems it should be enabled for all ports.
> > > Do I miss something here?
> > > 
> > > I did the following test:
> > > brctl addbr br0
> > > brctl addif br0 lan0
> > > brctl addif br0 lan1
> > > 
> > > In this scenario the unicast and multicast packets were not forwarded,
> > > therefore ARP requests were not resolved, and no connection could be
> > > established.
> > 
> > Hi Stefan
> > 
> > This is probably specific to the 6097 family. It works fine without
> > this on other devices. Creating a bridge like above and pinging across
> > it is one of my standard tests. But i only test modern devices like
> > the 6165, 6352, 6351, 6390 families.
> 
> Okay perfect, I wasn't 100% sure if I would have to configure something
> additionally.

No. The idea is you treat the interfaces as normal interfaces. You
should not need to do anything additional to what you would do with a
normal interface, when adding it to a bridge.
 
> > In fact, you might need to review all the code and look where
> > mv88e6xxx_6095_family(chip) is used and consider if you need to add
> > mv88e6xxx_6097_family(chip). e.g.
> > 
> >         if (mv88e6xxx_6095_family(chip) || mv88e6xxx_6185_family(chip)) {
> >                 /* Set the upstream port this port should use */
> >                 reg |= dsa_upstream_port(ds);
> >                 /* enable forwarding of unknown multicast addresses to
> >                  * the upstream port
> >                  */
> >                 if (port == dsa_upstream_port(ds))
> >                         reg |= PORT_CONTROL_2_FORWARD_UNKNOWN;
> >         }
> > 
> > Maybe this is your problem?
> 
> I think I still don't understand exactly how the driver works.
> 
> My problem is that the multicast and broadcast frames are filtered and
> the following counter is increasing in ethtool:
> sw_in_filtered: 596

This is not what is supposed to happen. Broadcast and multicast frames
should go to all ports in the bridge. There are two different ways
this can happen:

1) The mv88e6xxx driver started out with the host doing all bridge
operations. The switch forwards all frames to the software bridge, and
the software bridge then sends them out another port if needed.

2) We later added support for hardware bridging. That is, the switch
itself bridges frames between ports. It will only pass frames to the
software bridge if it does not know what to do with a frame itself.

Now, the different families are not 100% compatible with each
other. We never had access to a 6097, so it has not been tested
recently, and we have probably broken it... My guess would be,
anywhere mv88e6xxx_6095_family(chip) is used, there also needs to be
an mv88e6xxx_6097_family(chip). But i could be wrong.

What you might find useful is

https://github.com/vivien/linux.git 161b96bd7d16d21b0f046c935b70c3b2d277ccc2

although it might need some changes for recent commits.

With that, you can see deeper into the switches registers.

     Andrew

Re: [PATCH] net: dsa: mv88e6xxx: egress all frames

[Vivien Didelot]

Hi Andrew, Stefan,

Andrew Lunn <andrew@lunn.ch> writes:

> What you might find useful is
>
> https://github.com/vivien/linux.git 161b96bd7d16d21b0f046c935b70c3b2d277ccc2
>
> although it might need some changes for recent commits.
>
> With that, you can see deeper into the switches registers.

FYI, I have rebased it on top of the latest net-next (f9aa9dc7d2d0):

    https://github.com/vivien/linux.git dsa/dev

Thanks,

        Vivien

Re: [PATCH] net: dsa: mv88e6xxx: egress all frames

[Stefan Eichenberger]

Hi Vivien

On Tue, Nov 22, 2016 at 05:15:25PM -0500, Vivien Didelot wrote:
> Hi Andrew, Stefan,
> 
> Andrew Lunn <andrew@lunn.ch> writes:
> 
> > What you might find useful is
> >
> > https://github.com/vivien/linux.git 161b96bd7d16d21b0f046c935b70c3b2d277ccc2
> >
> > although it might need some changes for recent commits.
> >
> > With that, you can see deeper into the switches registers.
> 
> FYI, I have rebased it on top of the latest net-next (f9aa9dc7d2d0):
> 
>     https://github.com/vivien/linux.git dsa/dev
> 

Perfect that is really helpful, thanks a lot!
Stefan

Re: [PATCH] net: dsa: mv88e6xxx: egress all frames

[Stefan Eichenberger]

On Tue, Nov 22, 2016 at 08:02:06PM +0100, Andrew Lunn wrote:
> On Tue, Nov 22, 2016 at 07:37:33PM +0100, Stefan Eichenberger wrote:
> > Hi Andrew
> > 
> > On Tue, Nov 22, 2016 at 04:03:30PM +0100, Andrew Lunn wrote:
> > > On Tue, Nov 22, 2016 at 11:39:44AM +0100, Stefan Eichenberger wrote:
> > > > Egress multicast and egress unicast is only enabled for CPU/DSA ports
> > > > but for switching operation it seems it should be enabled for all ports.
> > > > Do I miss something here?
> > > > 
> > > > I did the following test:
> > > > brctl addbr br0
> > > > brctl addif br0 lan0
> > > > brctl addif br0 lan1
> > > > 
> > > > In this scenario the unicast and multicast packets were not forwarded,
> > > > therefore ARP requests were not resolved, and no connection could be
> > > > established.
> > > 
> > > Hi Stefan
> > > 
> > > This is probably specific to the 6097 family. It works fine without
> > > this on other devices. Creating a bridge like above and pinging across
> > > it is one of my standard tests. But i only test modern devices like
> > > the 6165, 6352, 6351, 6390 families.
> > 
> > Okay perfect, I wasn't 100% sure if I would have to configure something
> > additionally.
> 
> No. The idea is you treat the interfaces as normal interfaces. You
> should not need to do anything additional to what you would do with a
> normal interface, when adding it to a bridge.
>  
> > > In fact, you might need to review all the code and look where
> > > mv88e6xxx_6095_family(chip) is used and consider if you need to add
> > > mv88e6xxx_6097_family(chip). e.g.
> > > 
> > >         if (mv88e6xxx_6095_family(chip) || mv88e6xxx_6185_family(chip)) {
> > >                 /* Set the upstream port this port should use */
> > >                 reg |= dsa_upstream_port(ds);
> > >                 /* enable forwarding of unknown multicast addresses to
> > >                  * the upstream port
> > >                  */
> > >                 if (port == dsa_upstream_port(ds))
> > >                         reg |= PORT_CONTROL_2_FORWARD_UNKNOWN;
> > >         }
> > > 
> > > Maybe this is your problem?
> > 
> > I think I still don't understand exactly how the driver works.
> > 
> > My problem is that the multicast and broadcast frames are filtered and
> > the following counter is increasing in ethtool:
> > sw_in_filtered: 596
> 
> This is not what is supposed to happen. Broadcast and multicast frames
> should go to all ports in the bridge. There are two different ways
> this can happen:
> 
> 1) The mv88e6xxx driver started out with the host doing all bridge
> operations. The switch forwards all frames to the software bridge, and
> the software bridge then sends them out another port if needed.
> 
> 2) We later added support for hardware bridging. That is, the switch
> itself bridges frames between ports. It will only pass frames to the
> software bridge if it does not know what to do with a frame itself.

Thanks for this explanation it helped a lot.

> 
> Now, the different families are not 100% compatible with each
> other. We never had access to a 6097, so it has not been tested
> recently, and we have probably broken it... My guess would be,
> anywhere mv88e6xxx_6095_family(chip) is used, there also needs to be
> an mv88e6xxx_6097_family(chip). But i could be wrong.

I think I probably found the problem. For EDSA type switches the bit
PORT_CONTROL_FORWARD_UNKNOWN_MC is set on the cpu port but not for DSA 
type switches. Broadcast addresses are threaded as multicast addresses, 
so unknown frames will never leave the switch.

Do you know if there is a reason why this bit isn't set for DSA type
switches too? The patch would be extremely simple and it seems to work
perfectly with this bit set on the CPU port.

Thanks
Stefan

Re: [PATCH] net: dsa: mv88e6xxx: egress all frames

[Vivien Didelot]

Hi Stefan,

Stefan Eichenberger <stefan.eichenberger@netmodule.com> writes:

>> Now, the different families are not 100% compatible with each
>> other. We never had access to a 6097, so it has not been tested
>> recently, and we have probably broken it... My guess would be,
>> anywhere mv88e6xxx_6095_family(chip) is used, there also needs to be
>> an mv88e6xxx_6097_family(chip). But i could be wrong.
>
> I think I probably found the problem. For EDSA type switches the bit
> PORT_CONTROL_FORWARD_UNKNOWN_MC is set on the cpu port but not for DSA 
> type switches. Broadcast addresses are threaded as multicast addresses, 
> so unknown frames will never leave the switch.

The Port Control Register (0x04) is one of these registers which changes
almost completely among chip models.

Are you able to give us the layout of the port register 0x04 on your
88E6097? I don't have access to its datasheet.

For instance on 88E6185 bit 3 is reserved, on 88E6352 and 88E6390 bit
3:2 are "Egress Floods" and 0x2 means "Do not egress any frame with an
unknown unicast DA".

> Do you know if there is a reason why this bit isn't set for DSA type
> switches too? The patch would be extremely simple and it seems to work
> perfectly with this bit set on the CPU port.

All these family checks for bit masking are quite messy and ideally need
proper abstraction...

Can you give us the chunk of patch you are refering to?

Thanks,

        Vivien

Re: [PATCH] net: dsa: mv88e6xxx: egress all frames

[Stefan Eichenberger]

Hi Vivien

On Wed, Nov 23, 2016 at 10:59:13AM -0500, Vivien Didelot wrote:
> Hi Stefan,
> 
> Stefan Eichenberger <stefan.eichenberger@netmodule.com> writes:
> 
> >> Now, the different families are not 100% compatible with each
> >> other. We never had access to a 6097, so it has not been tested
> >> recently, and we have probably broken it... My guess would be,
> >> anywhere mv88e6xxx_6095_family(chip) is used, there also needs to be
> >> an mv88e6xxx_6097_family(chip). But i could be wrong.
> >
> > I think I probably found the problem. For EDSA type switches the bit
> > PORT_CONTROL_FORWARD_UNKNOWN_MC is set on the cpu port but not for DSA 
> > type switches. Broadcast addresses are threaded as multicast addresses, 
> > so unknown frames will never leave the switch.
> 
> The Port Control Register (0x04) is one of these registers which changes
> almost completely among chip models.
> 
> Are you able to give us the layout of the port register 0x04 on your
> 88E6097? I don't have access to its datasheet.

Yes sure, the layout of the Port Control Register for the 88E6097 is the same
as for the 88E6352:

15:14: SA Filtering: 00 -> SA filtering disabled
                     01 -> Drop on lock
                     10 -> Drop on Unlock
                     11 -> Drop to CPU
13:12: Egress Mode:  00 -> default unmodified mode
                     01 -> default to transmit all frames untagged
                     10 -> default to transmit all frames tagged
                     11 -> reserved for future use
11:    Header:       Ingress&Egress header mode (PORT_CONTROL_HEADER)
10:    IGMP Snoop:   IGMP/MLD Snooping (PORT_CONTROL_IGMP_MLD_SNOOP)
9:8    Frame Mode:   00 -> Normal Network
                     01 -> DSA (FRAME_MODE_DSA)
                     10 -> Provider (FRAME_MODE_PROVIDER)
                     11 -> Ether Type DSA (FRAME_ETHER_TYPE_DSA)
7:     VLAN Tunnel:  VLAN Tunnel (VLAN_TUNNEL)
6:     TagIfBoth:    Use tag info for QPri
5:4:   InitialPri:   00 -> Use Port defaults for FPri and QPri
                     01 -> Use Tag Priority
                     10 -> Use IP Priority
                     11 -> Use Tag & IP Priority
3:2:   Egress Floods:00 -> Do not egress any frame with unknown DA
                     01 -> Do not egress any frame with an unknown mc DA
                     10 -> Do not egress any frame with an unknown DA
                     11 -> Egress all frames with an unknown DA
                     Broadcasts are threaded as multicast if FloodBC in
                     global2 register is not set.
1:0:   PortState:    00 -> Disabled
                     01 -> Blocking/Listening
                     10 -> Learning
                     11 -> Forwarding

I hope this helps, feel free to ask for more infos.

> 
> For instance on 88E6185 bit 3 is reserved, on 88E6352 and 88E6390 bit
> 3:2 are "Egress Floods" and 0x2 means "Do not egress any frame with an
> unknown unicast DA".
> 
> > Do you know if there is a reason why this bit isn't set for DSA type
> > switches too? The patch would be extremely simple and it seems to work
> > perfectly with this bit set on the CPU port.
> 
> All these family checks for bit masking are quite messy and ideally need
> proper abstraction...
> 
> Can you give us the chunk of patch you are refering to?

I will send the patch in a few minutes.

Regards,
Stefan

[PATCH v2] net: dsa: mv88e6xxx: forward unknown mc packets on mv88e6097

[Stefan Eichenberger]

Packets with unknown destination addresses are not forwarded to the cpu
port on mv88e6097 based switches (e.g. MV88E6097) at the moment. This
commit enables PORT_CONTROL_FORWARD_UNKNOWN_MC for this family.

Signed-off-by: Stefan Eichenberger <stefan.eichenberger@netmodule.com>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index b14b3d5..4d21086 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -2487,6 +2487,10 @@ static int mv88e6xxx_setup_port(struct mv88e6xxx_chip *chip, int port)
 				PORT_CONTROL_FORWARD_UNKNOWN_MC;
 		else
 			reg |= PORT_CONTROL_DSA_TAG;
+
+		if (mv88e6xxx_6097_family(chip))
+			reg |= PORT_CONTROL_FORWARD_UNKNOWN_MC;
+
 		reg |= PORT_CONTROL_EGRESS_ADD_TAG |
 			PORT_CONTROL_FORWARD_UNKNOWN;
 	}
-- 
2.9.3

Re: [PATCH] net: dsa: mv88e6xxx: egress all frames

[Andrew Lunn]

> 9:8    Frame Mode:   00 -> Normal Network
>                      01 -> DSA (FRAME_MODE_DSA)
>                      10 -> Provider (FRAME_MODE_PROVIDER)
>                      11 -> Ether Type DSA (FRAME_ETHER_TYPE_DSA)

Ah, there is one issue. This device supports EDSA. However,
MV88E6XXX_FLAGS_FAMILY_6097 does not list MV88E6XXX_FLAG_EDSA.

Without that flag set, the code assumes the device can only do DSA,
using the older definition of this register.

    Andrew

Re: [PATCH v2] net: dsa: mv88e6xxx: forward unknown mc packets on mv88e6097

[Andrew Lunn]

On Wed, Nov 23, 2016 at 05:54:40PM +0100, Stefan Eichenberger wrote:
> Packets with unknown destination addresses are not forwarded to the cpu
> port on mv88e6097 based switches (e.g. MV88E6097) at the moment. This
> commit enables PORT_CONTROL_FORWARD_UNKNOWN_MC for this family.

Please try adding MV88E6XXX_FLAG_EDSA to
MV88E6XXX_FLAGS_FAMILY_6097. That is the better fix if it works.

     Andrew

[PATCH v3] net: dsa: mv88e6xxx: enable EDSA on mv88e6097

[Stefan Eichenberger]

EDSA is currently disabled on mv88e6097 devices, this commit enables it.

Signed-off-by: Stefan Eichenberger <stefan.eichenberger@netmodule.com>
---
 drivers/net/dsa/mv88e6xxx/mv88e6xxx.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
index ab52c37..a2ff1fc 100644
--- a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
@@ -543,7 +543,8 @@ enum mv88e6xxx_cap {
 	 MV88E6XXX_FLAGS_MULTI_CHIP)
 
 #define MV88E6XXX_FLAGS_FAMILY_6097	\
-	(MV88E6XXX_FLAG_G1_ATU_FID |	\
+	(MV88E6XXX_FLAG_EDSA |		\
+	 MV88E6XXX_FLAG_G1_ATU_FID |	\
 	 MV88E6XXX_FLAG_G1_VTU_FID |	\
 	 MV88E6XXX_FLAG_GLOBAL2 |	\
 	 MV88E6XXX_FLAG_G2_MGMT_EN_2X |	\
-- 
2.9.3

Re: [PATCH v3] net: dsa: mv88e6xxx: enable EDSA on mv88e6097

[Andrew Lunn]

On Wed, Nov 23, 2016 at 06:11:35PM +0100, Stefan Eichenberger wrote:
> EDSA is currently disabled on mv88e6097 devices, this commit enables it.

And was that sufficient to fix all your issues?

    Andrew

Re: [PATCH v2] net: dsa: mv88e6xxx: forward unknown mc packets on mv88e6097

[Stefan Eichenberger]

On Wed, Nov 23, 2016 at 05:59:49PM +0100, Andrew Lunn wrote:
> On Wed, Nov 23, 2016 at 05:54:40PM +0100, Stefan Eichenberger wrote:
> > Packets with unknown destination addresses are not forwarded to the cpu
> > port on mv88e6097 based switches (e.g. MV88E6097) at the moment. This
> > commit enables PORT_CONTROL_FORWARD_UNKNOWN_MC for this family.
> 
> Please try adding MV88E6XXX_FLAG_EDSA to
> MV88E6XXX_FLAGS_FAMILY_6097. That is the better fix if it works.

I was even wondering what EDSA means:) Thanks this solved the problem!

Regards
Stefan

Re: [PATCH v2] net: dsa: mv88e6xxx: forward unknown mc packets on mv88e6097

[Andrew Lunn]

On Wed, Nov 23, 2016 at 06:14:41PM +0100, Stefan Eichenberger wrote:
> On Wed, Nov 23, 2016 at 05:59:49PM +0100, Andrew Lunn wrote:
> > On Wed, Nov 23, 2016 at 05:54:40PM +0100, Stefan Eichenberger wrote:
> > > Packets with unknown destination addresses are not forwarded to the cpu
> > > port on mv88e6097 based switches (e.g. MV88E6097) at the moment. This
> > > commit enables PORT_CONTROL_FORWARD_UNKNOWN_MC for this family.
> > 
> > Please try adding MV88E6XXX_FLAG_EDSA to
> > MV88E6XXX_FLAGS_FAMILY_6097. That is the better fix if it works.
> 
> I was even wondering what EDSA means:) Thanks this solved the problem!

Great.

We should fix up a few minor issues and resubmit.

What is the status of the first patch, which added 6097 to the driver?
I don't think David accepted it yet. So lets make one patchset
containing the two patches.

The subject line of the patches need to have net-next in it. e.g.

[PATCH net-next 0/2] Add support for the MV88e6097

Include a cover node, saying what the patchset as a whole does.
This gets used as the merge commit message.

Then the two patches.

When posting the patchset, please start a new thread. A new version of
a patchset or patch should be a new thread.

Thanks
	Andrew

Re: [PATCH v2] net: dsa: mv88e6xxx: forward unknown mc packets on mv88e6097

[Andrew Lunn]

On Wed, Nov 23, 2016 at 06:14:41PM +0100, Stefan Eichenberger wrote:
> On Wed, Nov 23, 2016 at 05:59:49PM +0100, Andrew Lunn wrote:
> > On Wed, Nov 23, 2016 at 05:54:40PM +0100, Stefan Eichenberger wrote:
> > > Packets with unknown destination addresses are not forwarded to the cpu
> > > port on mv88e6097 based switches (e.g. MV88E6097) at the moment. This
> > > commit enables PORT_CONTROL_FORWARD_UNKNOWN_MC for this family.
> > 
> > Please try adding MV88E6XXX_FLAG_EDSA to
> > MV88E6XXX_FLAGS_FAMILY_6097. That is the better fix if it works.
> 
> I was even wondering what EDSA means:) Thanks this solved the problem!

Plain DSA puts four bytes of header between the MAC source address and
the EtherType/Length.

EDSA puts in an 8 byte header, and includes an Ethertype value of
0xdada. Having that ethertype value makes it more obvious what is
going on. And if you have a recent version of tcpdump, it will decode
the header.

    Andrew

Re: [PATCH v2] net: dsa: mv88e6xxx: forward unknown mc packets on mv88e6097

[Stefan Eichenberger]

On Wed, Nov 23, 2016 at 06:32:30PM +0100, Andrew Lunn wrote:
> On Wed, Nov 23, 2016 at 06:14:41PM +0100, Stefan Eichenberger wrote:
> > On Wed, Nov 23, 2016 at 05:59:49PM +0100, Andrew Lunn wrote:
> > > On Wed, Nov 23, 2016 at 05:54:40PM +0100, Stefan Eichenberger wrote:
> > > > Packets with unknown destination addresses are not forwarded to the cpu
> > > > port on mv88e6097 based switches (e.g. MV88E6097) at the moment. This
> > > > commit enables PORT_CONTROL_FORWARD_UNKNOWN_MC for this family.
> > > 
> > > Please try adding MV88E6XXX_FLAG_EDSA to
> > > MV88E6XXX_FLAGS_FAMILY_6097. That is the better fix if it works.
> > 
> > I was even wondering what EDSA means:) Thanks this solved the problem!
> 
> Great.
> 
> We should fix up a few minor issues and resubmit.
> 
> What is the status of the first patch, which added 6097 to the driver?
> I don't think David accepted it yet. So lets make one patchset
> containing the two patches.
> 
> The subject line of the patches need to have net-next in it. e.g.
> 
> [PATCH net-next 0/2] Add support for the MV88e6097
> 
> Include a cover node, saying what the patchset as a whole does.
> This gets used as the merge commit message.
> 
> Then the two patches.

Perfect, thanks a lot for the help! The patchset will follow.

Thanks
Stefan

Re: [PATCH v2] net: dsa: mv88e6xxx: forward unknown mc packets on mv88e6097

[Vivien Didelot]

Hi Andrew,

Andrew Lunn <andrew@lunn.ch> writes:

> And if you have a recent version of tcpdump, it will decode
> the header.

Since d729eb4, thanks to you Andrew ;-)

I move up the cleanup of ports setup in my priority list. The code is
quite cluttered at the moment and it's hard to read through it. We need
proper helpers for egress floods, (E)DSA setup, etc. like what is being
done for the other devices.

Thanks,

        Vivien

Re: [PATCH v2] net: dsa: mv88e6xxx: forward unknown mc packets on mv88e6097

[Andrew Lunn]

On Wed, Nov 23, 2016 at 12:52:52PM -0500, Vivien Didelot wrote:
> Hi Andrew,
> 
> Andrew Lunn <andrew@lunn.ch> writes:
> 
> > And if you have a recent version of tcpdump, it will decode
> > the header.
> 
> Since d729eb4, thanks to you Andrew ;-)
> 
> I move up the cleanup of ports setup in my priority list.

Hi Vivien

Please take a look at my mv88e6390 branch. I already refactored this
code, because the mv88e6390 does something slightly different...

I hope to post another batch of mv88e6390 patches soon, and they will
include this cleanup. Since they will clash with these patches, i will
post them first as RFC.

      Andrew

Re: [PATCH v2] net: dsa: mv88e6xxx: forward unknown mc packets on mv88e6097

[Vivien Didelot]

Hi Andrew,

Andrew Lunn <andrew@lunn.ch> writes:

> On Wed, Nov 23, 2016 at 12:52:52PM -0500, Vivien Didelot wrote:
>> Hi Andrew,
>> 
>> Andrew Lunn <andrew@lunn.ch> writes:
>> 
>> > And if you have a recent version of tcpdump, it will decode
>> > the header.
>> 
>> Since d729eb4, thanks to you Andrew ;-)
>> 
>> I move up the cleanup of ports setup in my priority list.
>
> Hi Vivien
>
> Please take a look at my mv88e6390 branch. I already refactored this
> code, because the mv88e6390 does something slightly different...
>
> I hope to post another batch of mv88e6390 patches soon, and they will
> include this cleanup. Since they will clash with these patches, i will
> post them first as RFC.

Perfect. Please split an RFC only including this cleanup if
possible. Fewer patches will be easier to review, since the first port
registers differs a lot.

Thanks,

        Vivien