netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH 0/2] enable broadcom tagging for bcm531x5 switches
@ 2019-06-18 17:57 Benedikt Spranger
  2019-06-18 17:57 ` [RFC PATCH 1/2] net: dsa: b53: Turn on managed mode and set IMP port Benedikt Spranger
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Benedikt Spranger @ 2019-06-18 17:57 UTC (permalink / raw)
  To: netdev; +Cc: Benedikt Spranger

Hi,

while puting a Banana Pi R1 board into operation I faced network hickups
and get into serious trouble from my coworkers:

Banana Pi network setup:
PC (eth1) <--> BPi R1 (wan) / BPi R1 (lan4) <--> DUT (eth0)
10.0.32.1      10.0.32.2      172.16.0.1         172.16.0.2
---8<---
#! /bin/bash

# create VLANs
ip link add link eth0 name eth0.1 type vlan id 1
ip link add link eth0 name eth0.100 type vlan id 100

# create bridges
ip link add br0 type bridge
ip link set dev br0 type bridge vlan_filtering 1

ip link add br1 type bridge
ip link set dev br1 type bridge vlan_filtering 1

# add interfaces to bridges
ip link set lan1 master br0
ip link set lan2 master br0
ip link set lan3 master br0
ip link set lan4 master br0
ip link set eth0.100 master br0

ip link set wan master br1

# adjust tagging
bridge vlan add dev lan1 vid 100 pvid untagged
bridge vlan add dev lan2 vid 100 pvid untagged
bridge vlan add dev lan3 vid 100 pvid untagged
bridge vlan add dev lan4 vid 100 pvid untagged

bridge vlan del dev lan1 vid 1
bridge vlan del dev lan2 vid 1
bridge vlan del dev lan3 vid 1
bridge vlan del dev lan4 vid 1

# set IPs
ip addr add 10.0.32.2/24 dev eth0.1
ip addr add 172.16.0.1/24 dev br0

# up and run
ip link set eth0 up
ip link set eth0.1 up
ip link set eth0.100 up

ip link set br0 up
ip link set br1 up

ip link set wan up

ip link set lan1 up
ip link set lan2 up
ip link set lan3 up
ip link set lan4 up
---8<---

While trying to ping a non existing host 10.0.32.111 result in
doublication of ARP broadcasts:

On the BPi R1:
# tshark -i br0
    1 8.157471671 DavicomS_43:18:fc → Broadcast    ARP 60 Who has 10.0.32.111?
 Tell 10.0.32.1
    2 9.167563213 DavicomS_43:18:fc → Broadcast    ARP 60 Who has 10.0.32.111?
 Tell 10.0.32.1
    3 10.191703047 DavicomS_43:18:fc → Broadcast    ARP 60 Who has 10.0.32.111
? Tell 10.0.32.1
    4 11.215905797 DavicomS_43:18:fc → Broadcast    ARP 60 Who has 10.0.32.111
? Tell 10.0.32.1
    5 12.243932964 DavicomS_43:18:fc → Broadcast    ARP 60 Who has 10.0.32.111
? Tell 10.0.32.1
    6 13.264033298 DavicomS_43:18:fc → Broadcast    ARP 60 Who has 10.0.32.111
? Tell 10.0.32.1

# tshark -i wan0
    1 8.157421295 DavicomS_43:18:fc → Broadcast    ARP 60 Who has 10.0.32.111?
 Tell 10.0.32.1
    2 9.167510087 DavicomS_43:18:fc → Broadcast    ARP 60 Who has 10.0.32.111?
 Tell 10.0.32.1
    3 10.191649213 DavicomS_43:18:fc → Broadcast    ARP 60 Who has 10.0.32.111
? Tell 10.0.32.1
    4 11.215856838 DavicomS_43:18:fc → Broadcast    ARP 60 Who has 10.0.32.111
? Tell 10.0.32.1
    5 12.243881922 DavicomS_43:18:fc → Broadcast    ARP 60 Who has 10.0.32.111
? Tell 10.0.32.1
    6 13.263981506 DavicomS_43:18:fc → Broadcast    ARP 60 Who has 10.0.32.111
? Tell 10.0.32.1

On the PC:
# tshark -i eth1
  116 272.660717059 DavicomS_43:18:fc → Broadcast    ARP 42 Who has 10.0.32.111? Tell 10.0.32.1
  117 272.661062292 DavicomS_43:18:fc → Broadcast    ARP 60 Who has 10.0.32.111? Tell 10.0.32.1
  118 273.670690338 DavicomS_43:18:fc → Broadcast    ARP 42 Who has 10.0.32.111? Tell 10.0.32.1
  119 273.671058605 DavicomS_43:18:fc → Broadcast    ARP 60 Who has 10.0.32.111? Tell 10.0.32.1
  120 274.694720511 DavicomS_43:18:fc → Broadcast    ARP 42 Who has 10.0.32.111? Tell 10.0.32.1
  121 274.695250723 DavicomS_43:18:fc → Broadcast    ARP 60 Who has 10.0.32.111? Tell 10.0.32.1
  122 275.718813538 DavicomS_43:18:fc → Broadcast    ARP 42 Who has 10.0.32.111? Tell 10.0.32.1
  123 275.719285852 DavicomS_43:18:fc → Broadcast    ARP 60 Who has 10.0.32.111? Tell 10.0.32.1
  124 276.746729795 DavicomS_43:18:fc → Broadcast    ARP 42 Who has 10.0.32.111? Tell 10.0.32.1
  125 276.747236341 DavicomS_43:18:fc → Broadcast    ARP 60 Who has 10.0.32.111? Tell 10.0.32.1
  126 277.766722429 DavicomS_43:18:fc → Broadcast    ARP 42 Who has 10.0.32.111? Tell 10.0.32.1
  127 277.767233098 DavicomS_43:18:fc → Broadcast    ARP 60 Who has 10.0.32.111? Tell 10.0.32.1

Choosing between network disturbance by mirrored broadcast packages (and angry
coworkers) or lack of multicast, I choose the later. 

Regards
    Bene Spranger

Florian Fainelli (1):
  net: dsa: b53: Turn on managed mode and set IMP port

Sebastian Andrzej Siewior (1):
  net: dsa: b53: enbale broadcom tags on bcm531x5

 drivers/net/dsa/b53/b53_common.c | 21 ++++++++++++++++-----
 drivers/net/dsa/bcm_sf2.c        |  3 ---
 2 files changed, 16 insertions(+), 8 deletions(-)

-- 
2.19.0


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [RFC PATCH 1/2] net: dsa: b53: Turn on managed mode and set IMP port
  2019-06-18 17:57 [RFC PATCH 0/2] enable broadcom tagging for bcm531x5 switches Benedikt Spranger
@ 2019-06-18 17:57 ` Benedikt Spranger
  2019-06-18 18:10   ` Florian Fainelli
  2019-06-18 17:57 ` [RFC PATCH 2/2] net: dsa: b53: enbale broadcom tags on bcm531x5 Benedikt Spranger
  2019-06-18 18:16 ` [RFC PATCH 0/2] enable broadcom tagging for bcm531x5 switches Florian Fainelli
  2 siblings, 1 reply; 16+ messages in thread
From: Benedikt Spranger @ 2019-06-18 17:57 UTC (permalink / raw)
  To: netdev; +Cc: Florian Fainelli, Benedikt Spranger

From: Florian Fainelli <f.fainelli@gmail.com>

When enabling Broadcom tags on earlier devices such as BCM53125, we need
to also enable Managed mode, as well as configure which port is going to
be the IMP port. If we did not do that, the switch would just pass
through the Broadcom tags on the wire and not act on them. We need to
have bcm_sf2 stop overwriting the SWMODE register and let b53 deal with
that now.

Fixes: 7edc58d614d4 ("net: dsa: b53: Turn on Broadcom tags")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Benedikt Spranger <b.spranger@linutronix.de>
---
 drivers/net/dsa/b53/b53_common.c | 19 +++++++++++++++----
 drivers/net/dsa/bcm_sf2.c        |  3 ---
 2 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index c8040ecf4425..06b9a7a81ae0 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -331,9 +331,9 @@ static void b53_set_forwarding(struct b53_device *dev, int enable)
 	b53_read8(dev, B53_CTRL_PAGE, B53_SWITCH_MODE, &mgmt);
 
 	if (enable)
-		mgmt |= SM_SW_FWD_EN;
+		mgmt |= SM_SW_FWD_EN | SM_SW_FWD_MODE;
 	else
-		mgmt &= ~SM_SW_FWD_EN;
+		mgmt &= ~(SM_SW_FWD_EN | SM_SW_FWD_MODE);
 
 	b53_write8(dev, B53_CTRL_PAGE, B53_SWITCH_MODE, mgmt);
 
@@ -364,8 +364,6 @@ static void b53_enable_vlan(struct b53_device *dev, bool enable,
 		b53_read8(dev, B53_VLAN_PAGE, B53_VLAN_CTRL5, &vc5);
 	}
 
-	mgmt &= ~SM_SW_FWD_MODE;
-
 	if (enable) {
 		vc0 |= VC0_VLAN_EN | VC0_VID_CHK_EN | VC0_VID_HASH_VID;
 		vc1 |= VC1_RX_MCST_UNTAG_EN | VC1_RX_MCST_FWD_EN;
@@ -589,6 +587,19 @@ void b53_brcm_hdr_setup(struct dsa_switch *ds, int port)
 		hdr_ctl &= ~val;
 	b53_write8(dev, B53_MGMT_PAGE, B53_BRCM_HDR, hdr_ctl);
 
+	/* Set IMP port number, necessary for Broadcom tags to be used
+	 * while in managed mode
+	 */
+	if (tag_en) {
+		b53_read8(dev, B53_MGMT_PAGE, B53_GLOBAL_CONFIG, &hdr_ctl);
+		hdr_ctl &= ~GC_FRM_MGMT_PORT_M;
+		val = 0;
+		if (port == 8)
+			val = GC_FRM_MGMT_PORT_MII;
+		hdr_ctl |= val;
+		b53_write8(dev, B53_MGMT_PAGE, B53_GLOBAL_CONFIG, hdr_ctl);
+	}
+
 	/* Registers below are only accessible on newer devices */
 	if (!is58xx(dev))
 		return;
diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index 3811fdbda13e..47f55dab14f6 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -53,9 +53,6 @@ static void bcm_sf2_imp_setup(struct dsa_switch *ds, int port)
 	reg &= ~(RX_DIS | TX_DIS);
 	core_writel(priv, reg, CORE_IMP_CTL);
 
-	/* Enable forwarding */
-	core_writel(priv, SW_FWDG_EN, CORE_SWMODE);
-
 	/* Enable IMP port in dumb mode */
 	reg = core_readl(priv, CORE_SWITCH_CTRL);
 	reg |= MII_DUMB_FWDG_EN;
-- 
2.19.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [RFC PATCH 2/2] net: dsa: b53: enbale broadcom tags on bcm531x5
  2019-06-18 17:57 [RFC PATCH 0/2] enable broadcom tagging for bcm531x5 switches Benedikt Spranger
  2019-06-18 17:57 ` [RFC PATCH 1/2] net: dsa: b53: Turn on managed mode and set IMP port Benedikt Spranger
@ 2019-06-18 17:57 ` Benedikt Spranger
  2019-06-18 18:16 ` [RFC PATCH 0/2] enable broadcom tagging for bcm531x5 switches Florian Fainelli
  2 siblings, 0 replies; 16+ messages in thread
From: Benedikt Spranger @ 2019-06-18 17:57 UTC (permalink / raw)
  To: netdev; +Cc: Sebastian Andrzej Siewior, Benedikt Spranger

From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

The broadcom tags are required to distinguish the traffic from external
ports and not use a bridge like setup (which is currently broken because
managed mode is now enabled). The tagged mode works for broadcast and
unicast traffic. The multicast missis are not handled.

Reviewed-by: Kurt Kanzenbach <kurt@linutronix.de>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Benedikt Spranger <b.spranger@linutronix.de>
---
 drivers/net/dsa/b53/b53_common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 06b9a7a81ae0..10f1ece64104 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -1793,7 +1793,7 @@ enum dsa_tag_protocol b53_get_tag_protocol(struct dsa_switch *ds, int port)
 	 * mode to be turned on which means we need to specifically manage ARL
 	 * misses on multicast addresses (TBD).
 	 */
-	if (is5325(dev) || is5365(dev) || is539x(dev) || is531x5(dev) ||
+	if (is5325(dev) || is5365(dev) || is539x(dev) ||
 	    !b53_can_enable_brcm_tags(ds, port))
 		return DSA_TAG_PROTO_NONE;
 
-- 
2.19.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [RFC PATCH 1/2] net: dsa: b53: Turn on managed mode and set IMP port
  2019-06-18 17:57 ` [RFC PATCH 1/2] net: dsa: b53: Turn on managed mode and set IMP port Benedikt Spranger
@ 2019-06-18 18:10   ` Florian Fainelli
  0 siblings, 0 replies; 16+ messages in thread
From: Florian Fainelli @ 2019-06-18 18:10 UTC (permalink / raw)
  To: Benedikt Spranger, netdev

On 6/18/19 10:57 AM, Benedikt Spranger wrote:
> From: Florian Fainelli <f.fainelli@gmail.com>
> 
> When enabling Broadcom tags on earlier devices such as BCM53125, we need
> to also enable Managed mode, as well as configure which port is going to
> be the IMP port. If we did not do that, the switch would just pass
> through the Broadcom tags on the wire and not act on them. We need to
> have bcm_sf2 stop overwriting the SWMODE register and let b53 deal with
> that now.
> 
> Fixes: 7edc58d614d4 ("net: dsa: b53: Turn on Broadcom tags")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> Signed-off-by: Benedikt Spranger <b.spranger@linutronix.de>

This patch was part of a bigger series that you don't submit, so you are
simply not just potentially breaking all users of b53, but you are also
breaking bcm_sf2 which you probably don't have access to for testing,
that's unfortunate. More on the cover letter.
-- 
Florian

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFC PATCH 0/2] enable broadcom tagging for bcm531x5 switches
  2019-06-18 17:57 [RFC PATCH 0/2] enable broadcom tagging for bcm531x5 switches Benedikt Spranger
  2019-06-18 17:57 ` [RFC PATCH 1/2] net: dsa: b53: Turn on managed mode and set IMP port Benedikt Spranger
  2019-06-18 17:57 ` [RFC PATCH 2/2] net: dsa: b53: enbale broadcom tags on bcm531x5 Benedikt Spranger
@ 2019-06-18 18:16 ` Florian Fainelli
  2019-06-19  9:18   ` Benedikt Spranger
  2 siblings, 1 reply; 16+ messages in thread
From: Florian Fainelli @ 2019-06-18 18:16 UTC (permalink / raw)
  To: Benedikt Spranger, netdev, Sebastian Andrzej Siewior, Kurt Kanzenbach

On 6/18/19 10:57 AM, Benedikt Spranger wrote:
> Hi,
> 
> while puting a Banana Pi R1 board into operation I faced network hickups
> and get into serious trouble from my coworkers:
> 
> Banana Pi network setup:
> PC (eth1) <--> BPi R1 (wan) / BPi R1 (lan4) <--> DUT (eth0)
> 10.0.32.1      10.0.32.2      172.16.0.1         172.16.0.2
> ---8<---
> #! /bin/bash
> 
> # create VLANs
> ip link add link eth0 name eth0.1 type vlan id 1
> ip link add link eth0 name eth0.100 type vlan id 100
> 
> # create bridges
> ip link add br0 type bridge
> ip link set dev br0 type bridge vlan_filtering 1
> 
> ip link add br1 type bridge
> ip link set dev br1 type bridge vlan_filtering 1
> 
> # add interfaces to bridges
> ip link set lan1 master br0
> ip link set lan2 master br0
> ip link set lan3 master br0
> ip link set lan4 master br0
> ip link set eth0.100 master br0
> 
> ip link set wan master br1
> 
> # adjust tagging
> bridge vlan add dev lan1 vid 100 pvid untagged
> bridge vlan add dev lan2 vid 100 pvid untagged
> bridge vlan add dev lan3 vid 100 pvid untagged
> bridge vlan add dev lan4 vid 100 pvid untagged
> 
> bridge vlan del dev lan1 vid 1
> bridge vlan del dev lan2 vid 1
> bridge vlan del dev lan3 vid 1
> bridge vlan del dev lan4 vid 1
> 
> # set IPs
> ip addr add 10.0.32.2/24 dev eth0.1
> ip addr add 172.16.0.1/24 dev br0
> 
> # up and run
> ip link set eth0 up
> ip link set eth0.1 up
> ip link set eth0.100 up
> 
> ip link set br0 up
> ip link set br1 up
> 
> ip link set wan up
> 
> ip link set lan1 up
> ip link set lan2 up
> ip link set lan3 up
> ip link set lan4 up
> ---8<---
> 
> While trying to ping a non existing host 10.0.32.111 result in
> doublication of ARP broadcasts:
> 
> On the BPi R1:
> # tshark -i br0
>     1 8.157471671 DavicomS_43:18:fc → Broadcast    ARP 60 Who has 10.0.32.111?
>  Tell 10.0.32.1
>     2 9.167563213 DavicomS_43:18:fc → Broadcast    ARP 60 Who has 10.0.32.111?
>  Tell 10.0.32.1
>     3 10.191703047 DavicomS_43:18:fc → Broadcast    ARP 60 Who has 10.0.32.111
> ? Tell 10.0.32.1
>     4 11.215905797 DavicomS_43:18:fc → Broadcast    ARP 60 Who has 10.0.32.111
> ? Tell 10.0.32.1
>     5 12.243932964 DavicomS_43:18:fc → Broadcast    ARP 60 Who has 10.0.32.111
> ? Tell 10.0.32.1
>     6 13.264033298 DavicomS_43:18:fc → Broadcast    ARP 60 Who has 10.0.32.111
> ? Tell 10.0.32.1
> 
> # tshark -i wan0
>     1 8.157421295 DavicomS_43:18:fc → Broadcast    ARP 60 Who has 10.0.32.111?
>  Tell 10.0.32.1
>     2 9.167510087 DavicomS_43:18:fc → Broadcast    ARP 60 Who has 10.0.32.111?
>  Tell 10.0.32.1
>     3 10.191649213 DavicomS_43:18:fc → Broadcast    ARP 60 Who has 10.0.32.111
> ? Tell 10.0.32.1
>     4 11.215856838 DavicomS_43:18:fc → Broadcast    ARP 60 Who has 10.0.32.111
> ? Tell 10.0.32.1
>     5 12.243881922 DavicomS_43:18:fc → Broadcast    ARP 60 Who has 10.0.32.111
> ? Tell 10.0.32.1
>     6 13.263981506 DavicomS_43:18:fc → Broadcast    ARP 60 Who has 10.0.32.111
> ? Tell 10.0.32.1
> 
> On the PC:
> # tshark -i eth1
>   116 272.660717059 DavicomS_43:18:fc → Broadcast    ARP 42 Who has 10.0.32.111? Tell 10.0.32.1
>   117 272.661062292 DavicomS_43:18:fc → Broadcast    ARP 60 Who has 10.0.32.111? Tell 10.0.32.1
>   118 273.670690338 DavicomS_43:18:fc → Broadcast    ARP 42 Who has 10.0.32.111? Tell 10.0.32.1
>   119 273.671058605 DavicomS_43:18:fc → Broadcast    ARP 60 Who has 10.0.32.111? Tell 10.0.32.1
>   120 274.694720511 DavicomS_43:18:fc → Broadcast    ARP 42 Who has 10.0.32.111? Tell 10.0.32.1
>   121 274.695250723 DavicomS_43:18:fc → Broadcast    ARP 60 Who has 10.0.32.111? Tell 10.0.32.1
>   122 275.718813538 DavicomS_43:18:fc → Broadcast    ARP 42 Who has 10.0.32.111? Tell 10.0.32.1
>   123 275.719285852 DavicomS_43:18:fc → Broadcast    ARP 60 Who has 10.0.32.111? Tell 10.0.32.1
>   124 276.746729795 DavicomS_43:18:fc → Broadcast    ARP 42 Who has 10.0.32.111? Tell 10.0.32.1
>   125 276.747236341 DavicomS_43:18:fc → Broadcast    ARP 60 Who has 10.0.32.111? Tell 10.0.32.1
>   126 277.766722429 DavicomS_43:18:fc → Broadcast    ARP 42 Who has 10.0.32.111? Tell 10.0.32.1
>   127 277.767233098 DavicomS_43:18:fc → Broadcast    ARP 60 Who has 10.0.32.111? Tell 10.0.32.1
> 
> Choosing between network disturbance by mirrored broadcast packages (and angry
> coworkers) or lack of multicast, I choose the later. 

How is that a problem for other machines? Does that lead to some kind of
broadcast storm because there are machines that keep trying to respond
to ARP solicitations?

The few aspects that bother me, not in any particular order, are that:

- you might be able to not change anything and just get away with the
one line patch below that sets skb->offload_fwd_mark to 1 to indicate to
the bridge, not to bother with sending a copy of the packet, since the
HW took care of that already

- the patch from me that you included was part of a larger series that
also addressed multicast while in management mode, such we would not
have to chose like you did, have you tested it, did it somehow not work?

- you have not copied the DSA maintainers on all of the patches, so you
get a C grade for your patch submission

diff --git a/net/dsa/tag_brcm.c b/net/dsa/tag_brcm.c
index 9c3114179690..9169b63a89e3 100644
--- a/net/dsa/tag_brcm.c
+++ b/net/dsa/tag_brcm.c
@@ -140,6 +140,8 @@ static struct sk_buff *brcm_tag_rcv_ll(struct
sk_buff *skb,
        /* Remove Broadcom tag and update checksum */
        skb_pull_rcsum(skb, BRCM_TAG_LEN);

+       skb->offload_fwd_mark = 1;
+
        return skb;
 }
 #endif


-- 
Florian

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [RFC PATCH 0/2] enable broadcom tagging for bcm531x5 switches
  2019-06-18 18:16 ` [RFC PATCH 0/2] enable broadcom tagging for bcm531x5 switches Florian Fainelli
@ 2019-06-19  9:18   ` Benedikt Spranger
  2019-06-23  2:24     ` Florian Fainelli
  0 siblings, 1 reply; 16+ messages in thread
From: Benedikt Spranger @ 2019-06-19  9:18 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: netdev, Sebastian Andrzej Siewior, Kurt Kanzenbach

On Tue, 18 Jun 2019 11:16:23 -0700
Florian Fainelli <f.fainelli@gmail.com> wrote:

> How is that a problem for other machines? Does that lead to some kind
> of broadcast storm because there are machines that keep trying to
> respond to ARP solicitations?
Mirroring broadcast packages on the interface they are coming in, is
IMHO a poor idea. As a result any switch connected to wan update the
MAC table and send packages on a port where they do not belong to.
Just imagine to send a DHCP request. The BPi R1 acts as nearly perfect
black hole in such a situation.

> The few aspects that bother me, not in any particular order, are that:
> 
> - you might be able to not change anything and just get away with the
> one line patch below that sets skb->offload_fwd_mark to 1 to indicate
> to the bridge, not to bother with sending a copy of the packet, since
> the HW took care of that already

I can test it, but i like to note that the changed function is not
executed in case of bcm53125.

See commit 54e98b5d663f ("net: dsa: b53: Turn off Broadcom tags for
more switches")

From drivers/net/dsa/b53/b53_common.c:
---8<---
/* Older models (5325, 5365) support a different tag format that we do
 * not support in net/dsa/tag_brcm.c yet. 539x and 531x5 require managed
 * mode to be turned on which means we need to specifically manage ARL
 * misses on multicast addresses (TBD).
 */
if (is5325(dev) || is5365(dev) || is539x(dev) || is531x5(dev) ||
    !b53_can_enable_brcm_tags(ds, port))
	return DSA_TAG_PROTO_NONE;
---8<---

> - the patch from me that you included was part of a larger series that
> also addressed multicast while in management mode, such we would not
> have to chose like you did, have you tested it, did it somehow not
> work?
Since the patch series was not available any more, I had to gather the
snippets. Trying these patches was the last effort to get the switch
running without causing network problems.
(As far as I see are other parts of the original patch series in
mainline)

> - you have not copied the DSA maintainers on all of the patches, so
> you get a C grade for your patch submission
OK. As a RFC patch I am more interrested in the attention of the
original author of the patch, which I received :)

Regards
    Bene Spranger

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFC PATCH 0/2] enable broadcom tagging for bcm531x5 switches
  2019-06-19  9:18   ` Benedikt Spranger
@ 2019-06-23  2:24     ` Florian Fainelli
  2019-06-25 11:20       ` Benedikt Spranger
  0 siblings, 1 reply; 16+ messages in thread
From: Florian Fainelli @ 2019-06-23  2:24 UTC (permalink / raw)
  To: Benedikt Spranger; +Cc: netdev, Sebastian Andrzej Siewior, Kurt Kanzenbach

[-- Attachment #1: Type: text/plain, Size: 1836 bytes --]



On 6/19/2019 2:18 AM, Benedikt Spranger wrote:
> On Tue, 18 Jun 2019 11:16:23 -0700
> Florian Fainelli <f.fainelli@gmail.com> wrote:
> 
>> How is that a problem for other machines? Does that lead to some kind
>> of broadcast storm because there are machines that keep trying to
>> respond to ARP solicitations?
> Mirroring broadcast packages on the interface they are coming in, is
> IMHO a poor idea. As a result any switch connected to wan update the
> MAC table and send packages on a port where they do not belong to.
> Just imagine to send a DHCP request. The BPi R1 acts as nearly perfect
> black hole in such a situation.

Fair enough.

> 
>> The few aspects that bother me, not in any particular order, are that:
>>
>> - you might be able to not change anything and just get away with the
>> one line patch below that sets skb->offload_fwd_mark to 1 to indicate
>> to the bridge, not to bother with sending a copy of the packet, since
>> the HW took care of that already
> 
> I can test it, but i like to note that the changed function is not
> executed in case of bcm53125.

Indeed, that won't work, how about implementing port_egress_flood() for
b53? That would make sure that multicast is flooded (as well as unicast)
before the switch port is enslaved into the bridge and this should take
care of both your problem and the lack of multicast flooding once
Broadcom tags are turned on. You might also need to set
skb->fwd_offload_mark accordingly in case you still see duplicate
packets, though that should not happen anymore AFAICT.

Something like this should take care of that (untested). You might have
to explicitly set the IMP port (port 8) in B53_UC_FWD_EN and
B53_MC_FWD_EN, though since you turn on management mode, this may not be
required. I might have some time tomorrow to test that on a Lamobo R1.
-- 
Florian

[-- Attachment #2: b53-egress-floods.patch --]
[-- Type: text/plain, Size: 2684 bytes --]

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index c8040ecf4425..a47f5bc667bd 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -342,6 +342,13 @@ static void b53_set_forwarding(struct b53_device *dev, int enable)
 	b53_read8(dev, B53_CTRL_PAGE, B53_SWITCH_CTRL, &mgmt);
 	mgmt |= B53_MII_DUMB_FWDG_EN;
 	b53_write8(dev, B53_CTRL_PAGE, B53_SWITCH_CTRL, mgmt);
+
+	/* Look at B53_UC_FWD_EN and B53_MC_FWD_EN to decide whether
+	 * frames should be flooed or not.
+	 */
+	b53_read8(dev, B53_CTRL_PAGE, B53_IP_MULTICAST_CTRL, &mgmt);
+	mgmt |= B53_UC_FWD_EN | B53_MC_FWD_EN;
+	b53_write8(dev, B53_CTRL_PAGE, B53_IP_MULTICAST_CTRL, mgmt);
 }
 
 static void b53_enable_vlan(struct b53_device *dev, bool enable,
@@ -1748,6 +1755,31 @@ void b53_br_fast_age(struct dsa_switch *ds, int port)
 }
 EXPORT_SYMBOL(b53_br_fast_age);
 
+int b53_br_egress_floods(struct dsa_switch *ds, int port,
+			 bool unicast, bool multicast)
+{
+	struct b53_device *dev = ds->priv;
+	u16 uc, mc;
+
+	b53_read16(dev, B53_CTRL_PAGE, B53_UC_FWD_EN, &uc);
+	if (unicast)
+		uc |= BIT(port);
+	else
+		uc &= ~BIT(port);
+	b53_write16(dev, B53_CTRL_PAGE, B53_UC_FWD_EN, uc);
+
+	b53_read16(dev, B53_CTRL_PAGE, B53_MC_FWD_EN, &mc);
+	if (multicast)
+		mc |= BIT(port);
+	else
+		mc &= ~BIT(port);
+	b53_write16(dev, B53_CTRL_PAGE, B53_MC_FWD_EN, mc);
+
+	return 0;
+
+}
+EXPORT_SYMBOL(b53_br_egress_floods);
+
 static bool b53_possible_cpu_port(struct dsa_switch *ds, int port)
 {
 	/* Broadcom switches will accept enabling Broadcom tags on the
@@ -1948,6 +1980,7 @@ static const struct dsa_switch_ops b53_switch_ops = {
 	.port_bridge_leave	= b53_br_leave,
 	.port_stp_state_set	= b53_br_set_stp_state,
 	.port_fast_age		= b53_br_fast_age,
+	.port_egress_floods	= b53_br_egress_floods,
 	.port_vlan_filtering	= b53_vlan_filtering,
 	.port_vlan_prepare	= b53_vlan_prepare,
 	.port_vlan_add		= b53_vlan_add,
diff --git a/drivers/net/dsa/b53/b53_priv.h b/drivers/net/dsa/b53/b53_priv.h
index f25bc80c4ffc..a7dd8acc281b 100644
--- a/drivers/net/dsa/b53/b53_priv.h
+++ b/drivers/net/dsa/b53/b53_priv.h
@@ -319,6 +319,8 @@ int b53_br_join(struct dsa_switch *ds, int port, struct net_device *bridge);
 void b53_br_leave(struct dsa_switch *ds, int port, struct net_device *bridge);
 void b53_br_set_stp_state(struct dsa_switch *ds, int port, u8 state);
 void b53_br_fast_age(struct dsa_switch *ds, int port);
+int b53_br_egress_floods(struct dsa_switch *ds, int port,
+			 bool unicast, bool multicast);
 void b53_port_event(struct dsa_switch *ds, int port);
 void b53_phylink_validate(struct dsa_switch *ds, int port,
 			  unsigned long *supported,

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [RFC PATCH 0/2] enable broadcom tagging for bcm531x5 switches
  2019-06-23  2:24     ` Florian Fainelli
@ 2019-06-25 11:20       ` Benedikt Spranger
  2019-06-25 18:17         ` Florian Fainelli
  0 siblings, 1 reply; 16+ messages in thread
From: Benedikt Spranger @ 2019-06-25 11:20 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: netdev, Sebastian Andrzej Siewior, Kurt Kanzenbach

On Sat, 22 Jun 2019 19:24:10 -0700
Florian Fainelli <f.fainelli@gmail.com> wrote:

> Something like this should take care of that (untested). You might
> have to explicitly set the IMP port (port 8) in B53_UC_FWD_EN and
> B53_MC_FWD_EN, though since you turn on management mode, this may not
> be required. I might have some time tomorrow to test that on a Lamobo
> R1.

The patch work fine in a manual setup, but not with automagicaly setup
of $DISTRO. But that is a very different story. Trying to debug whats
going wrong there.

Thanks for the patch.

The whole setup of the switch seem to be desperate fragile. Any chance
to get that more robust?

Regards
    Bene Spranger

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFC PATCH 0/2] enable broadcom tagging for bcm531x5 switches
  2019-06-25 11:20       ` Benedikt Spranger
@ 2019-06-25 18:17         ` Florian Fainelli
  2019-06-27 10:15           ` [RFC PATCH 0/1] Document the configuration of b53 Benedikt Spranger
  0 siblings, 1 reply; 16+ messages in thread
From: Florian Fainelli @ 2019-06-25 18:17 UTC (permalink / raw)
  To: Benedikt Spranger; +Cc: netdev, Sebastian Andrzej Siewior, Kurt Kanzenbach

On 6/25/19 4:20 AM, Benedikt Spranger wrote:
> On Sat, 22 Jun 2019 19:24:10 -0700
> Florian Fainelli <f.fainelli@gmail.com> wrote:
> 
>> Something like this should take care of that (untested). You might
>> have to explicitly set the IMP port (port 8) in B53_UC_FWD_EN and
>> B53_MC_FWD_EN, though since you turn on management mode, this may not
>> be required. I might have some time tomorrow to test that on a Lamobo
>> R1.
> 
> The patch work fine in a manual setup, but not with automagicaly setup
> of $DISTRO. But that is a very different story. Trying to debug whats
> going wrong there.
> 
> Thanks for the patch.
> 
> The whole setup of the switch seem to be desperate fragile. Any chance
> to get that more robust?
I don't have as much as time as I used to have (both life and work
factors) to contribute to b53, and comments like those really make me to
continue digging my cave and move away from any sort of upstream
development.

This is open source, so if you have the datasheet and feel like you do
better, please go ahead, I certainly won't object and will happily
review patches.
-- 
Florian

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [RFC PATCH 0/1] Document the configuration of b53
  2019-06-25 18:17         ` Florian Fainelli
@ 2019-06-27 10:15           ` Benedikt Spranger
  2019-06-27 10:15             ` [RFC PATCH 1/1] Documentation: net: dsa: b53: Describe b53 configuration Benedikt Spranger
  0 siblings, 1 reply; 16+ messages in thread
From: Benedikt Spranger @ 2019-06-27 10:15 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: netdev, Sebastian Andrzej Siewior, Kurt Kanzenbach, Andrew Lunn,
	Vivien Didelot

Hi,

my comment about the configuration got misunderstood.
I apologize for that.

I try to update the Debian ifupdown util to handle DSA capable
configurations. To avoid bafflement and frayed nerves I would like to get
some conclusion about the configuration of the b53. I would like to know
if this configuration can be expected to be expected to remain unchanged,
or if some parts need to be handled with more or special care.

Please consider this patch as starting/discussion point to get a quotable
reference in the kernel documentation.

This reference would ease the development and justification to upstream
changes to tools like ifupdown.

Regards
    Bene Spranger

Benedikt Spranger (1):
  Documentation: net: dsa: b53: Describe b53 configuration

 Documentation/networking/dsa/b53.rst | 300 +++++++++++++++++++++++++++
 1 file changed, 300 insertions(+)
 create mode 100644 Documentation/networking/dsa/b53.rst

-- 
2.20.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [RFC PATCH 1/1] Documentation: net: dsa: b53: Describe b53 configuration
  2019-06-27 10:15           ` [RFC PATCH 0/1] Document the configuration of b53 Benedikt Spranger
@ 2019-06-27 10:15             ` Benedikt Spranger
  2019-06-27 13:49               ` Andrew Lunn
  2019-06-27 16:38               ` Florian Fainelli
  0 siblings, 2 replies; 16+ messages in thread
From: Benedikt Spranger @ 2019-06-27 10:15 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: netdev, Sebastian Andrzej Siewior, Kurt Kanzenbach, Andrew Lunn,
	Vivien Didelot

Document the different needs of documentation for the b53 driver.

Signed-off-by: Benedikt Spranger <b.spranger@linutronix.de>
---
 Documentation/networking/dsa/b53.rst | 300 +++++++++++++++++++++++++++
 1 file changed, 300 insertions(+)
 create mode 100644 Documentation/networking/dsa/b53.rst

diff --git a/Documentation/networking/dsa/b53.rst b/Documentation/networking/dsa/b53.rst
new file mode 100644
index 000000000000..5838cf6230da
--- /dev/null
+++ b/Documentation/networking/dsa/b53.rst
@@ -0,0 +1,300 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+==========================================
+Broadcom RoboSwitch Ethernet switch driver
+==========================================
+
+The Broadcom RoboSwitch Ethernet switch family is used in quite a range of
+xDSL router, cable modems and other multimedia devices.
+
+The actual implementation supports the devices BCM5325E, BCM5365, BCM539x,
+BCM53115 and BCM53125 as well as BCM63XX.
+
+Implementation details
+======================
+
+The driver is located in ``drivers/net/dsa/bcm_sf2.c`` and is implemented as a
+DSA driver; see ``Documentation/networking/dsa/dsa.rst`` for details on the
+subsystemand what it provides.
+
+The switch is, if possible, configured to enable a Broadcom specific 4-bytes
+switch tag which gets inserted by the switch for every packet forwarded to the
+CPU interface, conversely, the CPU network interface should insert a similar
+tag for packets entering the CPU port. The tag format is described in
+``net/dsa/tag_brcm.c``.
+
+The configuration of the device depends on whether or not tagging is
+supported.
+
+Configuration with tagging support
+----------------------------------
+
+The tagging based configuration is desired.
+
+To use the b53 DSA driver some configuration need to be performed. As
+example configuration the following scenarios are used:
+
+*single port*
+  Every switch port acts as a different configurable ethernet port
+
+*bridge*
+  Every switch port is part of one configurable ethernet bridge
+
+*gateway*
+  Every switch port except one upstream port is part of a configurable
+  ethernet bridge.
+  The upstream port acts as different configurable ethernet port.
+
+All configurations are performed with tools from iproute2, wich is available at
+https://www.kernel.org/pub/linux/utils/net/iproute2/
+
+In this documentation the following ethernet ports are used:
+
+*eth0*
+  CPU port
+
+*LAN1*
+  a switch port
+
+*LAN2*
+  another switch port
+
+*WAN*
+  A switch port dedicated as upstream port
+
+Further ethernet ports can be configured similar.
+The configured IPs and networks are:
+
+*single port*
+  *  wan: 192.0.2.1/30 (192.0.2.0 - 192.0.2.3)
+  * lan1: 192.0.2.5/30 (192.0.2.4 - 192.0.2.7)
+  * lan2: 192.0.2.9/30 (192.0.2.8 - 192.0.2.11)
+
+*bridge*
+  * br0: 192.0.2.129/25 (192.0.2.128 - 192.0.2.255)
+
+*gateway*
+  * br0: 192.0.2.129/25 (192.0.2.128 - 192.0.2.255)
+  * wan: 192.0.2.1/30 (192.0.2.0 - 192.0.2.3)
+
+single port
+~~~~~~~~~~~
+
+.. code-block:: sh
+
+  # configure each interface
+  ip addr add 192.0.2.1/30 dev wan
+  ip addr add 192.0.2.5/30 dev lan1
+  ip addr add 192.0.2.9/30 dev lan2
+
+  # The master interface needs to be brought up before the slave ports.
+  ip link set eth0 up
+
+  # bring up the slave interfaces
+  ip link set wan up
+  ip link set lan1 up
+  ip link set lan2 up
+
+bridge
+~~~~~~
+
+.. code-block:: sh
+
+  # create bridge
+  ip link add name br0 type bridge
+
+  # add ports to bridge
+  ip link set dev wan master br0
+  ip link set dev lan1 master br0
+  ip link set dev lan2 master br0
+
+  # configure the bridge
+  ip addr add 192.0.2.129/25 dev br0
+
+  # The master interface needs to be brought up before the slave ports.
+  ip link set eth0 up
+
+  # bring up the slave interfaces
+  ip link set wan up
+  ip link set lan1 up
+  ip link set lan2 up
+
+  # bring up the bridge
+  ip link set dev br0 up
+
+gateway
+~~~~~~~
+
+.. code-block:: sh
+
+  # create bridge
+  ip link add name br0 type bridge
+
+  # add ports to bridge
+  ip link set dev lan1 master br0
+  ip link set dev lan2 master br0
+
+  # configure the bridge
+  ip addr add 192.0.2.129/25 dev br0
+
+  # configure the upstream port
+  ip addr add 192.0.2.1/30 dev wan
+
+  # The master interface needs to be brought up before the slave ports.
+  ip link set eth0 up
+
+  # bring up the slave interfaces
+  ip link set wan up
+  ip link set lan1 up
+  ip link set lan2 up
+
+  # bring up the bridge
+  ip link set dev br0 up
+
+Configuration without tagging support
+-------------------------------------
+
+Older models (5325, 5365) support a different tag format that is not supported
+yet. 539x and 531x5 require managed mode and some special handling, which is
+also not yet supported. The tagging support is disabled in these cases and the
+switch need a different configuration.
+
+single port
+~~~~~~~~~~~
+The configuration can only be set up via VLAN tagging and bridge setup.
+By default packages are tagged with vid 1:
+
+.. code-block:: sh
+
+  # tag traffic on CPU port
+  ip link add link eth0 name eth0.1 type vlan id 1
+  ip link add link eth0 name eth0.2 type vlan id 2
+  ip link add link eth0 name eth0.3 type vlan id 3
+
+  # create bridges
+  ip link add name br0 type bridge
+  ip link add name br1 type bridge
+  ip link add name br2 type bridge
+
+  # activate VLAN filtering
+  ip link set dev br0 type bridge vlan_filtering 1
+  ip link set dev br1 type bridge vlan_filtering 1
+  ip link set dev br2 type bridge vlan_filtering 1
+
+  # add ports to bridges
+  ip link set dev wan master br0
+  ip link set eth0.1 master br0
+  ip link set dev lan1 master br1
+  ip link set eth0.2 master br1
+  ip link set dev lan2 master br2
+  ip link set eth0.3 master br2
+
+  # tag traffic on ports
+  bridge vlan add dev lan1 vid 2 pvid untagged
+  bridge vlan del dev lan1 vid 1
+  bridge vlan add dev lan2 vid 3 pvid untagged
+  bridge vlan del dev lan2 vid 1
+
+  # configure the bridges
+  ip addr add 192.0.2.1/30 dev br0
+  ip addr add 192.0.2.5/30 dev br1
+  ip addr add 192.0.2.9/30 dev br2
+
+  # The master interface needs to be brought up before the slave ports.
+  ip link set eth0 up
+  ip link set eth0.1 up
+  ip link set eth0.2 up
+  ip link set eth0.3 up
+
+  # bring up the slave interfaces
+  ip link set wan up
+  ip link set lan1 up
+  ip link set lan2 up
+
+  # bring up the bridge devices
+  ip link set br0 up
+  ip link set br1 up
+  ip link set br2 up
+
+bridge
+~~~~~~
+
+.. code-block:: sh
+
+  # tag traffic on CPU port
+  ip link add link eth0 name eth0.1 type vlan id 1
+
+  # create bridge
+  ip link add name br0 type bridge
+
+  # activate VLAN filtering
+  ip link set dev br0 type bridge vlan_filtering 1
+
+  # add ports to bridge
+  ip link set dev wan master br0
+  ip link set dev lan1 master br0
+  ip link set dev lan2 master br0
+  ip link set eth0.1 master br0
+
+  # configure the bridge
+  ip addr add 192.0.2.129/25 dev br0
+
+  # The master interface needs to be brought up before the slave ports.
+  ip link set eth0 up
+  ip link set eth0.1 up
+
+  # bring up the slave interfaces
+  ip link set wan up
+  ip link set lan1 up
+  ip link set lan2 up
+
+  # bring up the bridge
+  ip link set dev br0 up
+
+gateway
+~~~~~~~
+
+.. code-block:: sh
+
+  # tag traffic on CPU port
+  ip link add link eth0 name eth0.1 type vlan id 1
+  ip link add link eth0 name eth0.2 type vlan id 2
+
+  # create bridges
+  ip link add name br0 type bridge
+  ip link add name br1 type bridge
+
+  # activate VLAN filtering
+  ip link set dev br0 type bridge vlan_filtering 1
+  ip link set dev br1 type bridge vlan_filtering 1
+
+  # add ports to bridges
+  ip link set dev wan master br0
+  ip link set eth0.1 master br0
+  ip link set dev lan1 master br1
+  ip link set dev lan2 master br1
+  ip link set eth0.2 master br1
+
+  # tag traffic on ports
+  bridge vlan add dev lan1 vid 2 pvid untagged
+  bridge vlan add dev lan2 vid 2 pvid untagged
+  bridge vlan del dev lan1 vid 1
+  bridge vlan del dev lan2 vid 1
+
+  # configure the bridges
+  ip addr add 192.0.2.1/30 dev br0
+  ip addr add 192.0.2.129/25 dev br1
+
+  # The master interface needs to be brought up before the slave ports.
+  ip link set eth0 up
+  ip link set eth0.1 up
+  ip link set eth0.2 up
+
+  # bring up the slave interfaces
+  ip link set wan up
+  ip link set lan1 up
+  ip link set lan2 up
+
+  # bring up the bridge devices
+  ip link set br0 up
+  ip link set br1 up
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [RFC PATCH 1/1] Documentation: net: dsa: b53: Describe b53 configuration
  2019-06-27 10:15             ` [RFC PATCH 1/1] Documentation: net: dsa: b53: Describe b53 configuration Benedikt Spranger
@ 2019-06-27 13:49               ` Andrew Lunn
  2019-06-27 14:43                 ` Benedikt Spranger
  2019-06-27 16:38               ` Florian Fainelli
  1 sibling, 1 reply; 16+ messages in thread
From: Andrew Lunn @ 2019-06-27 13:49 UTC (permalink / raw)
  To: Benedikt Spranger
  Cc: Florian Fainelli, netdev, Sebastian Andrzej Siewior,
	Kurt Kanzenbach, Vivien Didelot

On Thu, Jun 27, 2019 at 12:15:06PM +0200, Benedikt Spranger wrote:

Hi Benedikt

> +Configuration with tagging support
> +----------------------------------
> +
> +The tagging based configuration is desired.
> +
> +To use the b53 DSA driver some configuration need to be performed. As
> +example configuration the following scenarios are used:
> +
> +*single port*
> +  Every switch port acts as a different configurable ethernet port
> +
> +*bridge*
> +  Every switch port is part of one configurable ethernet bridge
> +
> +*gateway*
> +  Every switch port except one upstream port is part of a configurable
> +  ethernet bridge.
> +  The upstream port acts as different configurable ethernet port.
> +
> +All configurations are performed with tools from iproute2, wich is available at
> +https://www.kernel.org/pub/linux/utils/net/iproute2/
> +
> +In this documentation the following ethernet ports are used:
> +
> +*eth0*
> +  CPU port

In DSA terminology, this is the master interface. The switch port
which the master is connected to is called the CPU port. So you are
causing confusion with DSA terms here.

> +
> +*LAN1*
> +  a switch port
> +
> +*LAN2*
> +  another switch port
> +
> +*WAN*
> +  A switch port dedicated as upstream port

These are all slave interfaces, when using DSA terms.

> +Further ethernet ports can be configured similar.
> +The configured IPs and networks are:
> +
> +*single port*
> +  *  wan: 192.0.2.1/30 (192.0.2.0 - 192.0.2.3)
> +  * lan1: 192.0.2.5/30 (192.0.2.4 - 192.0.2.7)
> +  * lan2: 192.0.2.9/30 (192.0.2.8 - 192.0.2.11)
> +
> +*bridge*
> +  * br0: 192.0.2.129/25 (192.0.2.128 - 192.0.2.255)
> +
> +*gateway*
> +  * br0: 192.0.2.129/25 (192.0.2.128 - 192.0.2.255)
> +  * wan: 192.0.2.1/30 (192.0.2.0 - 192.0.2.3)
> +
> +single port
> +~~~~~~~~~~~
> +
> +.. code-block:: sh
> +
> +  # configure each interface
> +  ip addr add 192.0.2.1/30 dev wan
> +  ip addr add 192.0.2.5/30 dev lan1
> +  ip addr add 192.0.2.9/30 dev lan2
> +
> +  # The master interface needs to be brought up before the slave ports.
> +  ip link set eth0 up
> +
> +  # bring up the slave interfaces
> +  ip link set wan up
> +  ip link set lan1 up
> +  ip link set lan2 up
> +
> +bridge
> +~~~~~~
> +
> +.. code-block:: sh
> +
> +  # create bridge
> +  ip link add name br0 type bridge
> +
> +  # add ports to bridge
> +  ip link set dev wan master br0
> +  ip link set dev lan1 master br0
> +  ip link set dev lan2 master br0
> +
> +  # configure the bridge
> +  ip addr add 192.0.2.129/25 dev br0
> +
> +  # The master interface needs to be brought up before the slave ports.
> +  ip link set eth0 up
> +
> +  # bring up the slave interfaces
> +  ip link set wan up
> +  ip link set lan1 up
> +  ip link set lan2 up

I would probably do this in a different order. Bring the master up
first, then the slaves. Then enslave the slaves to bridge, and lastly
configure the bridge.

> +
> +  # bring up the bridge
> +  ip link set dev br0 up
> +
> +gateway
> +~~~~~~~
> +
> +.. code-block:: sh
> +
> +  # create bridge
> +  ip link add name br0 type bridge
> +
> +  # add ports to bridge
> +  ip link set dev lan1 master br0
> +  ip link set dev lan2 master br0
> +
> +  # configure the bridge
> +  ip addr add 192.0.2.129/25 dev br0
> +
> +  # configure the upstream port
> +  ip addr add 192.0.2.1/30 dev wan
> +
> +  # The master interface needs to be brought up before the slave ports.
> +  ip link set eth0 up
> +
> +  # bring up the slave interfaces
> +  ip link set wan up
> +  ip link set lan1 up
> +  ip link set lan2 up
> +
> +  # bring up the bridge
> +  ip link set dev br0 up

It would be good to add a note that there is nothing specific to the
B53 here. This same process will work for all DSA drivers which
support tagging, which is actually the majority.

I also tell people that once you configure the master interface up,
they should just use the slave interfaces a normal linux
interfaces. The fact they are on a switch does not matter, and should
not matter. Just use them as normal.

	Andrew

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFC PATCH 1/1] Documentation: net: dsa: b53: Describe b53 configuration
  2019-06-27 13:49               ` Andrew Lunn
@ 2019-06-27 14:43                 ` Benedikt Spranger
  0 siblings, 0 replies; 16+ messages in thread
From: Benedikt Spranger @ 2019-06-27 14:43 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Florian Fainelli, netdev, Sebastian Andrzej Siewior,
	Kurt Kanzenbach, Vivien Didelot

Am Thu, 27 Jun 2019 15:49:29 +0200
schrieb Andrew Lunn <andrew@lunn.ch>:

> On Thu, Jun 27, 2019 at 12:15:06PM +0200, Benedikt Spranger wrote:
> 
> Hi Benedikt
> 
> > +Configuration with tagging support
> > +----------------------------------
> > +
> > +The tagging based configuration is desired.
> > +
> > +To use the b53 DSA driver some configuration need to be performed. As
> > +example configuration the following scenarios are used:
> > +
> > +*single port*
> > +  Every switch port acts as a different configurable ethernet port
> > +
> > +*bridge*
> > +  Every switch port is part of one configurable ethernet bridge
> > +
> > +*gateway*
> > +  Every switch port except one upstream port is part of a configurable
> > +  ethernet bridge.
> > +  The upstream port acts as different configurable ethernet port.
> > +
> > +All configurations are performed with tools from iproute2, wich is available at
> > +https://www.kernel.org/pub/linux/utils/net/iproute2/
> > +
> > +In this documentation the following ethernet ports are used:
> > +
> > +*eth0*
> > +  CPU port  
> 
> In DSA terminology, this is the master interface. The switch port
> which the master is connected to is called the CPU port. So you are
> causing confusion with DSA terms here.

Changed the whole section to:

Through DSA every port of a switch is handled like a normal linux ethernet
interface. The CPU port is the switch port connected to an ethernet MAC chip.
The corresponding linux ethernet interface is called the master interface.
All other corresponding linux interfaces are called slave interfaces.

The slave interfaces depend on the master interface. They can only brought up,
when the master interface is up.

In this documentation the following ethernet interfaces are used:

*eth0*
  the master interface

*LAN1*
  a slave interface

*LAN2*
  another slave interface

*WAN*
  A slave interface dedicated for upstream traffic

> > +bridge
> > +~~~~~~
> > +
> > +.. code-block:: sh
> > +
> > +  # create bridge
> > +  ip link add name br0 type bridge
> > +
> > +  # add ports to bridge
> > +  ip link set dev wan master br0
> > +  ip link set dev lan1 master br0
> > +  ip link set dev lan2 master br0
> > +
> > +  # configure the bridge
> > +  ip addr add 192.0.2.129/25 dev br0
> > +
> > +  # The master interface needs to be brought up before the slave ports.
> > +  ip link set eth0 up
> > +
> > +  # bring up the slave interfaces
> > +  ip link set wan up
> > +  ip link set lan1 up
> > +  ip link set lan2 up  
> 
> I would probably do this in a different order. Bring the master up
> first, then the slaves. Then enslave the slaves to bridge, and lastly
> configure the bridge.

No objection. Will change the order.

> > +
> > +  # bring up the bridge
> > +  ip link set dev br0 up
> > +
> > +gateway
> > +~~~~~~~
> > +
> > +.. code-block:: sh
> > +
> > +  # create bridge
> > +  ip link add name br0 type bridge
> > +
> > +  # add ports to bridge
> > +  ip link set dev lan1 master br0
> > +  ip link set dev lan2 master br0
> > +
> > +  # configure the bridge
> > +  ip addr add 192.0.2.129/25 dev br0
> > +
> > +  # configure the upstream port
> > +  ip addr add 192.0.2.1/30 dev wan
> > +
> > +  # The master interface needs to be brought up before the slave ports.
> > +  ip link set eth0 up
> > +
> > +  # bring up the slave interfaces
> > +  ip link set wan up
> > +  ip link set lan1 up
> > +  ip link set lan2 up
> > +
> > +  # bring up the bridge
> > +  ip link set dev br0 up  
> 
> It would be good to add a note that there is nothing specific to the
> B53 here. This same process will work for all DSA drivers which
> support tagging, which is actually the majority.
Will state that.

> I also tell people that once you configure the master interface up,
> they should just use the slave interfaces a normal linux
> interfaces. The fact they are on a switch does not matter, and should
> not matter. Just use them as normal.
OK.

Regards
    Bene Spranger

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFC PATCH 1/1] Documentation: net: dsa: b53: Describe b53 configuration
  2019-06-27 10:15             ` [RFC PATCH 1/1] Documentation: net: dsa: b53: Describe b53 configuration Benedikt Spranger
  2019-06-27 13:49               ` Andrew Lunn
@ 2019-06-27 16:38               ` Florian Fainelli
  2019-06-28 11:44                 ` Kurt Kanzenbach
  2019-06-28 16:57                 ` Benedikt Spranger
  1 sibling, 2 replies; 16+ messages in thread
From: Florian Fainelli @ 2019-06-27 16:38 UTC (permalink / raw)
  To: Benedikt Spranger
  Cc: netdev, Sebastian Andrzej Siewior, Kurt Kanzenbach, Andrew Lunn,
	Vivien Didelot

On 6/27/19 3:15 AM, Benedikt Spranger wrote:
> Document the different needs of documentation for the b53 driver.
> 
> Signed-off-by: Benedikt Spranger <b.spranger@linutronix.de>
> ---
>  Documentation/networking/dsa/b53.rst | 300 +++++++++++++++++++++++++++
>  1 file changed, 300 insertions(+)
>  create mode 100644 Documentation/networking/dsa/b53.rst
> 
> diff --git a/Documentation/networking/dsa/b53.rst b/Documentation/networking/dsa/b53.rst
> new file mode 100644
> index 000000000000..5838cf6230da
> --- /dev/null
> +++ b/Documentation/networking/dsa/b53.rst
> @@ -0,0 +1,300 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +==========================================
> +Broadcom RoboSwitch Ethernet switch driver
> +==========================================
> +
> +The Broadcom RoboSwitch Ethernet switch family is used in quite a range of
> +xDSL router, cable modems and other multimedia devices.
> +
> +The actual implementation supports the devices BCM5325E, BCM5365, BCM539x,
> +BCM53115 and BCM53125 as well as BCM63XX.
> +
> +Implementation details
> +======================
> +
> +The driver is located in ``drivers/net/dsa/bcm_sf2.c`` and is implemented as a
> +DSA driver; see ``Documentation/networking/dsa/dsa.rst`` for details on the
> +subsystemand what it provides.

The driver is under drivers/net/dsa/b53/

s/ethernet/Ethernet/ for your global submission.

What you are describing is not entirely specific to the B53 driver
(maybe with the exception of having a VLAN tag on the DSA master network
device, since B53 puts the CPU port tagged in all VLANs by default), and
therefore the entire document should be written with the general DSA
devices in mind, and eventually pointing out where B53 differs in a
separate document.

There are largely two kinds of behavior:

- switches that are configured with DSA_TAG_PROTO_NONE, which behave in
a certain way and require a bridge with VLAN filtering even when ports
are intended to be used as standalone devices.

- switches that are configured with a tagging protocol other than
DSA_TAG_PROTO_NONE, which behave more like traditional network devices
people are used to use.

> +
> +The switch is, if possible, configured to enable a Broadcom specific 4-bytes
> +switch tag which gets inserted by the switch for every packet forwarded to the
> +CPU interface, conversely, the CPU network interface should insert a similar
> +tag for packets entering the CPU port. The tag format is described in
> +``net/dsa/tag_brcm.c``.
> +
> +The configuration of the device depends on whether or not tagging is
> +supported.
> +
> +Configuration with tagging support
> +----------------------------------
> +
> +The tagging based configuration is desired.
> +
> +To use the b53 DSA driver some configuration need to be performed. As
> +example configuration the following scenarios are used:
> +
> +*single port*
> +  Every switch port acts as a different configurable ethernet port
> +
> +*bridge*
> +  Every switch port is part of one configurable ethernet bridge
> +
> +*gateway*
> +  Every switch port except one upstream port is part of a configurable
> +  ethernet bridge.
> +  The upstream port acts as different configurable ethernet port.
> +
> +All configurations are performed with tools from iproute2, wich is available at
> +https://www.kernel.org/pub/linux/utils/net/iproute2/
> +
> +In this documentation the following ethernet ports are used:
> +
> +*eth0*
> +  CPU port
> +
> +*LAN1*
> +  a switch port
> +
> +*LAN2*
> +  another switch port
> +
> +*WAN*
> +  A switch port dedicated as upstream port
> +
> +Further ethernet ports can be configured similar.
> +The configured IPs and networks are:
> +
> +*single port*
> +  *  wan: 192.0.2.1/30 (192.0.2.0 - 192.0.2.3)
> +  * lan1: 192.0.2.5/30 (192.0.2.4 - 192.0.2.7)
> +  * lan2: 192.0.2.9/30 (192.0.2.8 - 192.0.2.11)
> +
> +*bridge*
> +  * br0: 192.0.2.129/25 (192.0.2.128 - 192.0.2.255)
> +
> +*gateway*
> +  * br0: 192.0.2.129/25 (192.0.2.128 - 192.0.2.255)
> +  * wan: 192.0.2.1/30 (192.0.2.0 - 192.0.2.3)
> +
> +single port
> +~~~~~~~~~~~
> +
> +.. code-block:: sh
> +
> +  # configure each interface
> +  ip addr add 192.0.2.1/30 dev wan
> +  ip addr add 192.0.2.5/30 dev lan1
> +  ip addr add 192.0.2.9/30 dev lan2
> +
> +  # The master interface needs to be brought up before the slave ports.
> +  ip link set eth0 up
> +
> +  # bring up the slave interfaces
> +  ip link set wan up
> +  ip link set lan1 up
> +  ip link set lan2 up
> +
> +bridge
> +~~~~~~

I would add something like:

All ports being part of a single bridge/broadcast domain or something
along those lines. Seeing the "wan" interface being added to a bridge is
a bit confusing.

> +
> +.. code-block:: sh
> +
> +  # create bridge
> +  ip link add name br0 type bridge
> +
> +  # add ports to bridge
> +  ip link set dev wan master br0
> +  ip link set dev lan1 master br0
> +  ip link set dev lan2 master br0
> +
> +  # configure the bridge
> +  ip addr add 192.0.2.129/25 dev br0
> +
> +  # The master interface needs to be brought up before the slave ports.
> +  ip link set eth0 up
> +
> +  # bring up the slave interfaces
> +  ip link set wan up
> +  ip link set lan1 up
> +  ip link set lan2 up
> +
> +  # bring up the bridge
> +  ip link set dev br0 up
> +
> +gateway
> +~~~~~~~
> +
> +.. code-block:: sh
> +
> +  # create bridge
> +  ip link add name br0 type bridge
> +
> +  # add ports to bridge
> +  ip link set dev lan1 master br0
> +  ip link set dev lan2 master br0
> +
> +  # configure the bridge
> +  ip addr add 192.0.2.129/25 dev br0
> +
> +  # configure the upstream port
> +  ip addr add 192.0.2.1/30 dev wan
> +
> +  # The master interface needs to be brought up before the slave ports.
> +  ip link set eth0 up
> +
> +  # bring up the slave interfaces
> +  ip link set wan up
> +  ip link set lan1 up
> +  ip link set lan2 up
> +
> +  # bring up the bridge
> +  ip link set dev br0 up
> +
> +Configuration without tagging support
> +-------------------------------------
> +
> +Older models (5325, 5365) support a different tag format that is not supported
> +yet. 539x and 531x5 require managed mode and some special handling, which is
> +also not yet supported. The tagging support is disabled in these cases and the
> +switch need a different configuration.

On that topic, did the patch I sent you ended up working the way you
wanted it with ifupdown-scripts or are you still chasing some other
issues with it?

> +
> +single port
> +~~~~~~~~~~~
> +The configuration can only be set up via VLAN tagging and bridge setup.
> +By default packages are tagged with vid 1:
> +
> +.. code-block:: sh
> +
> +  # tag traffic on CPU port
> +  ip link add link eth0 name eth0.1 type vlan id 1
> +  ip link add link eth0 name eth0.2 type vlan id 2
> +  ip link add link eth0 name eth0.3 type vlan id 3

That part is indeed a B53 implementation specific detail because B53
tags the CPU port in all VLANs, since otherwise any PVID untagged VLAN
programming would basically change the CPU port's default PVID and make
it untagged, undesirable.

> +
> +  # create bridges
> +  ip link add name br0 type bridge
> +  ip link add name br1 type bridge
> +  ip link add name br2 type bridge
> +
> +  # activate VLAN filtering
> +  ip link set dev br0 type bridge vlan_filtering 1
> +  ip link set dev br1 type bridge vlan_filtering 1
> +  ip link set dev br2 type bridge vlan_filtering 1
> +
> +  # add ports to bridges
> +  ip link set dev wan master br0
> +  ip link set eth0.1 master br0
> +  ip link set dev lan1 master br1
> +  ip link set eth0.2 master br1
> +  ip link set dev lan2 master br2
> +  ip link set eth0.3 master br2

I don't think you need one bridge for each port you want to isolate with
DSA_PROTO_TAG_NONE, you can just have a single bridge and assign the
ports a different VLAN with the commands below:

> +
> +  # tag traffic on ports
> +  bridge vlan add dev lan1 vid 2 pvid untagged
> +  bridge vlan del dev lan1 vid 1
> +  bridge vlan add dev lan2 vid 3 pvid untagged
> +  bridge vlan del dev lan2 vid 1

And also permit the different VLANs that you created on the bridge
master device itself:

bridge vlan add vid 2 dev br0 self
bridve vlan add vid 3 dev br0 self

Maybe that last part above ^^ was missing and that's why people tend to
create multiple bridge devices?

> +
> +  # configure the bridges
> +  ip addr add 192.0.2.1/30 dev br0
> +  ip addr add 192.0.2.5/30 dev br1
> +  ip addr add 192.0.2.9/30 dev br2
> +
> +  # The master interface needs to be brought up before the slave ports.
> +  ip link set eth0 up
> +  ip link set eth0.1 up
> +  ip link set eth0.2 up
> +  ip link set eth0.3 up
> +
> +  # bring up the slave interfaces
> +  ip link set wan up
> +  ip link set lan1 up
> +  ip link set lan2 up
> +
> +  # bring up the bridge devices
> +  ip link set br0 up
> +  ip link set br1 up
> +  ip link set br2 up
> +
> +bridge
> +~~~~~~
> +
> +.. code-block:: sh
> +
> +  # tag traffic on CPU port
> +  ip link add link eth0 name eth0.1 type vlan id 1
> +
> +  # create bridge
> +  ip link add name br0 type bridge
> +
> +  # activate VLAN filtering
> +  ip link set dev br0 type bridge vlan_filtering 1
> +
> +  # add ports to bridge
> +  ip link set dev wan master br0
> +  ip link set dev lan1 master br0
> +  ip link set dev lan2 master br0
> +  ip link set eth0.1 master br0
> +
> +  # configure the bridge
> +  ip addr add 192.0.2.129/25 dev br0
> +
> +  # The master interface needs to be brought up before the slave ports.
> +  ip link set eth0 up
> +  ip link set eth0.1 up
> +
> +  # bring up the slave interfaces
> +  ip link set wan up
> +  ip link set lan1 up
> +  ip link set lan2 up
> +
> +  # bring up the bridge
> +  ip link set dev br0 up
> +
> +gateway
> +~~~~~~~
> +
> +.. code-block:: sh
> +
> +  # tag traffic on CPU port
> +  ip link add link eth0 name eth0.1 type vlan id 1
> +  ip link add link eth0 name eth0.2 type vlan id 2
> +
> +  # create bridges
> +  ip link add name br0 type bridge
> +  ip link add name br1 type bridge

Likewise, I have seen claims of people telling me they used two bridge
devices, but AFAICT this is only because the bridge master did not have
VID 2 programmed to it, so if you did the following:

bridge vlan add vid 2 dev br0 self

you should be able to get away with a single bridge master device, can
you try that?

Overall this is fills a lot of gaps and questions that were being asked
on the lamobo R1 threads on various forums, thanks a lot for doing that!
-- 
Florian

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFC PATCH 1/1] Documentation: net: dsa: b53: Describe b53 configuration
  2019-06-27 16:38               ` Florian Fainelli
@ 2019-06-28 11:44                 ` Kurt Kanzenbach
  2019-06-28 16:57                 ` Benedikt Spranger
  1 sibling, 0 replies; 16+ messages in thread
From: Kurt Kanzenbach @ 2019-06-28 11:44 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Benedikt Spranger, netdev, Sebastian Andrzej Siewior,
	Andrew Lunn, Vivien Didelot

[-- Attachment #1: Type: text/plain, Size: 2077 bytes --]

Hi,

On Thu, Jun 27, 2019 at 09:38:16AM -0700, Florian Fainelli wrote:
> On 6/27/19 3:15 AM, Benedikt Spranger wrote:
> > Document the different needs of documentation for the b53 driver.
> >
> > Signed-off-by: Benedikt Spranger <b.spranger@linutronix.de>
> > ---
> >  Documentation/networking/dsa/b53.rst | 300 +++++++++++++++++++++++++++
> >  1 file changed, 300 insertions(+)
> >  create mode 100644 Documentation/networking/dsa/b53.rst
> >
> > diff --git a/Documentation/networking/dsa/b53.rst b/Documentation/networking/dsa/b53.rst
> > new file mode 100644
> > index 000000000000..5838cf6230da
> > --- /dev/null
> > +++ b/Documentation/networking/dsa/b53.rst
> > @@ -0,0 +1,300 @@
> > +.. SPDX-License-Identifier: GPL-2.0
> > +
> > +==========================================
> > +Broadcom RoboSwitch Ethernet switch driver
> > +==========================================
> > +
> > +The Broadcom RoboSwitch Ethernet switch family is used in quite a range of
> > +xDSL router, cable modems and other multimedia devices.
> > +
> > +The actual implementation supports the devices BCM5325E, BCM5365, BCM539x,
> > +BCM53115 and BCM53125 as well as BCM63XX.
> > +
> > +Implementation details
> > +======================
> > +
> > +The driver is located in ``drivers/net/dsa/bcm_sf2.c`` and is implemented as a
> > +DSA driver; see ``Documentation/networking/dsa/dsa.rst`` for details on the
> > +subsystemand what it provides.
>
> The driver is under drivers/net/dsa/b53/
>
> s/ethernet/Ethernet/ for your global submission.
>
> What you are describing is not entirely specific to the B53 driver
> (maybe with the exception of having a VLAN tag on the DSA master network
> device, since B53 puts the CPU port tagged in all VLANs by default), and
> therefore the entire document should be written with the general DSA
> devices in mind, and eventually pointing out where B53 differs in a
> separate document.

That's true. It makes sense to split the documentation into a generic
and the b53 specific part.

Thanks a lot, Bene, for doing this. It's really helpful.

Thanks,
Kurt

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [RFC PATCH 1/1] Documentation: net: dsa: b53: Describe b53 configuration
  2019-06-27 16:38               ` Florian Fainelli
  2019-06-28 11:44                 ` Kurt Kanzenbach
@ 2019-06-28 16:57                 ` Benedikt Spranger
  1 sibling, 0 replies; 16+ messages in thread
From: Benedikt Spranger @ 2019-06-28 16:57 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: netdev, Sebastian Andrzej Siewior, Kurt Kanzenbach, Andrew Lunn,
	Vivien Didelot

Am Thu, 27 Jun 2019 09:38:16 -0700
schrieb Florian Fainelli <f.fainelli@gmail.com>:

> On 6/27/19 3:15 AM, Benedikt Spranger wrote:
> > Document the different needs of documentation for the b53 driver.
> > 
> > Signed-off-by: Benedikt Spranger <b.spranger@linutronix.de>
> > ---
> >  Documentation/networking/dsa/b53.rst | 300
> > +++++++++++++++++++++++++++ 1 file changed, 300 insertions(+)
> >  create mode 100644 Documentation/networking/dsa/b53.rst
> > 
> > diff --git a/Documentation/networking/dsa/b53.rst
> > b/Documentation/networking/dsa/b53.rst new file mode 100644
> > index 000000000000..5838cf6230da
> > --- /dev/null
> > +++ b/Documentation/networking/dsa/b53.rst
> > @@ -0,0 +1,300 @@
> > +.. SPDX-License-Identifier: GPL-2.0
> > +
> > +==========================================
> > +Broadcom RoboSwitch Ethernet switch driver
> > +==========================================
> > +
> > +The Broadcom RoboSwitch Ethernet switch family is used in quite a
> > range of +xDSL router, cable modems and other multimedia devices.
> > +
> > +The actual implementation supports the devices BCM5325E, BCM5365,
> > BCM539x, +BCM53115 and BCM53125 as well as BCM63XX.
> > +
> > +Implementation details
> > +======================
> > +
> > +The driver is located in ``drivers/net/dsa/bcm_sf2.c`` and is
> > implemented as a +DSA driver; see
> > ``Documentation/networking/dsa/dsa.rst`` for details on the
> > +subsystemand what it provides.  
> 
> The driver is under drivers/net/dsa/b53/
Fixed.

> s/ethernet/Ethernet/ for your global submission.
OK.
 
> What you are describing is not entirely specific to the B53 driver
> (maybe with the exception of having a VLAN tag on the DSA master
> network device, since B53 puts the CPU port tagged in all VLANs by
> default), and therefore the entire document should be written with
> the general DSA devices in mind, and eventually pointing out where
> B53 differs in a separate document.

I have split up the Documentation into 
Documentation/networking/dsa/configuration.rst
and
Documentation/networking/dsa/b53.rst

> There are largely two kinds of behavior:
> 
> - switches that are configured with DSA_TAG_PROTO_NONE, which behave
> in a certain way and require a bridge with VLAN filtering even when
> ports are intended to be used as standalone devices.
> 
> - switches that are configured with a tagging protocol other than
> DSA_TAG_PROTO_NONE, which behave more like traditional network devices
> people are used to use.
OK.

> > +bridge
> > +~~~~~~  
> 
> I would add something like:
> 
> All ports being part of a single bridge/broadcast domain or something
> along those lines. Seeing the "wan" interface being added to a bridge
> is a bit confusing.

 
> > +
> > +.. code-block:: sh
> > +
> > +  # create bridge
> > +  ip link add name br0 type bridge
> > +
> > +  # add ports to bridge
> > +  ip link set dev wan master br0
> > +  ip link set dev lan1 master br0
> > +  ip link set dev lan2 master br0
> > +
> > +  # configure the bridge
> > +  ip addr add 192.0.2.129/25 dev br0
> > +
> > +  # The master interface needs to be brought up before the slave
> > ports.
> > +  ip link set eth0 up
> > +
> > +  # bring up the slave interfaces
> > +  ip link set wan up
> > +  ip link set lan1 up
> > +  ip link set lan2 up
> > +
> > +  # bring up the bridge
> > +  ip link set dev br0 up
> > +
> > +gateway
> > +~~~~~~~
> > +
> > +.. code-block:: sh
> > +
> > +  # create bridge
> > +  ip link add name br0 type bridge
> > +
> > +  # add ports to bridge
> > +  ip link set dev lan1 master br0
> > +  ip link set dev lan2 master br0
> > +
> > +  # configure the bridge
> > +  ip addr add 192.0.2.129/25 dev br0
> > +
> > +  # configure the upstream port
> > +  ip addr add 192.0.2.1/30 dev wan
> > +
> > +  # The master interface needs to be brought up before the slave
> > ports.
> > +  ip link set eth0 up
> > +
> > +  # bring up the slave interfaces
> > +  ip link set wan up
> > +  ip link set lan1 up
> > +  ip link set lan2 up
> > +
> > +  # bring up the bridge
> > +  ip link set dev br0 up
> > +
> > +Configuration without tagging support
> > +-------------------------------------
> > +
> > +Older models (5325, 5365) support a different tag format that is
> > not supported +yet. 539x and 531x5 require managed mode and some
> > special handling, which is +also not yet supported. The tagging
> > support is disabled in these cases and the +switch need a different
> > configuration.  
> 
> On that topic, did the patch I sent you ended up working the way you
> wanted it with ifupdown-scripts or are you still chasing some other
> issues with it?
Your patch is needed and working. Stumbled over my own feed.

I have a hackary solution by now. Hackary in terms of not tracking the
master interface state (just *up* it unconditionally).

> > +
> > +single port
> > +~~~~~~~~~~~
> > +The configuration can only be set up via VLAN tagging and bridge
> > setup. +By default packages are tagged with vid 1:
> > +
> > +.. code-block:: sh
> > +
> > +  # tag traffic on CPU port
> > +  ip link add link eth0 name eth0.1 type vlan id 1
> > +  ip link add link eth0 name eth0.2 type vlan id 2
> > +  ip link add link eth0 name eth0.3 type vlan id 3  
> 
> That part is indeed a B53 implementation specific detail because B53
> tags the CPU port in all VLANs, since otherwise any PVID untagged VLAN
> programming would basically change the CPU port's default PVID and
> make it untagged, undesirable.
OK.

> > +
> > +  # create bridges
> > +  ip link add name br0 type bridge
> > +  ip link add name br1 type bridge
> > +  ip link add name br2 type bridge
> > +
> > +  # activate VLAN filtering
> > +  ip link set dev br0 type bridge vlan_filtering 1
> > +  ip link set dev br1 type bridge vlan_filtering 1
> > +  ip link set dev br2 type bridge vlan_filtering 1
> > +
> > +  # add ports to bridges
> > +  ip link set dev wan master br0
> > +  ip link set eth0.1 master br0
> > +  ip link set dev lan1 master br1
> > +  ip link set eth0.2 master br1
> > +  ip link set dev lan2 master br2
> > +  ip link set eth0.3 master br2  
> 
> I don't think you need one bridge for each port you want to isolate
> with DSA_PROTO_TAG_NONE, you can just have a single bridge and assign
> the ports a different VLAN with the commands below:

Tried that out:

  # tag traffic on CPU port
  ip link add link eth0 name eth0.1 type vlan id 1
  ip link add link eth0 name eth0.2 type vlan id 2
  ip link add link eth0 name eth0.3 type vlan id 3

  # The master interface needs to be brought up before the slave ports.
  ip link set eth0 up
  ip link set eth0.1 up
  ip link set eth0.2 up
  ip link set eth0.3 up

  # bring up the slave interfaces
  ip link set wan up
  ip link set lan1 up
  ip link set lan2 up

  # create bridge
  ip link add name br0 type bridge

  # activate VLAN filtering
  ip link set dev br0 type bridge vlan_filtering 1

  # add ports to bridges
  ip link set dev wan master br0
  ip link set dev lan1 master br0
  ip link set dev lan2 master br0

  # tag traffic on ports
  bridge vlan add dev lan1 vid 2 pvid untagged
  bridge vlan del dev lan1 vid 1
  bridge vlan add dev lan2 vid 3 pvid untagged
  bridge vlan del dev lan2 vid 1

  # configure the VLANs
  ip addr add 192.0.2.1/30 dev eth0.1
  ip addr add 192.0.2.5/30 dev eth0.2
  ip addr add 192.0.2.9/30 dev eth0.3

  # bring up the bridge devices
  ip link set br0 up

Works quite well :)

> > +
> > +  # tag traffic on ports
> > +  bridge vlan add dev lan1 vid 2 pvid untagged
> > +  bridge vlan del dev lan1 vid 1
> > +  bridge vlan add dev lan2 vid 3 pvid untagged
> > +  bridge vlan del dev lan2 vid 1  
> 
> And also permit the different VLANs that you created on the bridge
> master device itself:
> 
> bridge vlan add vid 2 dev br0 self
> bridve vlan add vid 3 dev br0 self
> 
> Maybe that last part above ^^ was missing and that's why people tend
> to create multiple bridge devices?

From my side there was some tree in the woods problem...

> > +
> > +  # configure the bridges
> > +  ip addr add 192.0.2.1/30 dev br0
> > +  ip addr add 192.0.2.5/30 dev br1
> > +  ip addr add 192.0.2.9/30 dev br2
> > +
> > +  # The master interface needs to be brought up before the slave
> > ports.
> > +  ip link set eth0 up
> > +  ip link set eth0.1 up
> > +  ip link set eth0.2 up
> > +  ip link set eth0.3 up
> > +
> > +  # bring up the slave interfaces
> > +  ip link set wan up
> > +  ip link set lan1 up
> > +  ip link set lan2 up
> > +
> > +  # bring up the bridge devices
> > +  ip link set br0 up
> > +  ip link set br1 up
> > +  ip link set br2 up
> > +
> > +bridge
> > +~~~~~~
> > +
> > +.. code-block:: sh
> > +
> > +  # tag traffic on CPU port
> > +  ip link add link eth0 name eth0.1 type vlan id 1
> > +
> > +  # create bridge
> > +  ip link add name br0 type bridge
> > +
> > +  # activate VLAN filtering
> > +  ip link set dev br0 type bridge vlan_filtering 1
> > +
> > +  # add ports to bridge
> > +  ip link set dev wan master br0
> > +  ip link set dev lan1 master br0
> > +  ip link set dev lan2 master br0
> > +  ip link set eth0.1 master br0
> > +
> > +  # configure the bridge
> > +  ip addr add 192.0.2.129/25 dev br0
> > +
> > +  # The master interface needs to be brought up before the slave
> > ports.
> > +  ip link set eth0 up
> > +  ip link set eth0.1 up
> > +
> > +  # bring up the slave interfaces
> > +  ip link set wan up
> > +  ip link set lan1 up
> > +  ip link set lan2 up
> > +
> > +  # bring up the bridge
> > +  ip link set dev br0 up
> > +
> > +gateway
> > +~~~~~~~
> > +
> > +.. code-block:: sh
> > +
> > +  # tag traffic on CPU port
> > +  ip link add link eth0 name eth0.1 type vlan id 1
> > +  ip link add link eth0 name eth0.2 type vlan id 2
> > +
> > +  # create bridges
> > +  ip link add name br0 type bridge
> > +  ip link add name br1 type bridge  
> 
> Likewise, I have seen claims of people telling me they used two bridge
> devices, but AFAICT this is only because the bridge master did not
> have VID 2 programmed to it, so if you did the following:
> 
> bridge vlan add vid 2 dev br0 self

I tried the following:
  # tag traffic on CPU port
  ip link add link eth0 name eth0.1 type vlan id 1
  ip link add link eth0 name eth0.2 type vlan id 2

  # The master interface needs to be brought up before the slave ports.
  ip link set eth0 up
  ip link set eth0.1 up
  ip link set eth0.2 up

  # bring up the slave interfaces
  ip link set wan up
  ip link set lan1 up
  ip link set lan2 up

  # create bridge
  ip link add name br0 type bridge

  # activate VLAN filtering
  ip link set dev br0 type bridge vlan_filtering 1

  # add ports to bridges
  ip link set dev wan master br0
  ip link set eth0.1 master br0
  ip link set dev lan1 master br0
  ip link set dev lan2 master br0

  # tag traffic on ports
  bridge vlan add dev wan vid 2 pvid untagged
  bridge vlan del dev wan vid 1

  # configure the VLANs
  ip addr add 192.0.2.1/30 dev eth0.2
  ip addr add 192.0.2.129/25 dev br0

  # bring up the bridge devices
  ip link set br0 up

Maybe I got it fundamently wrong:
I try to seperate the traffic. If i bound everything to the bridge the
separation will not work out. To split it up like above on the other
hand makes it very clear (at least for me).

I have separate interfaces (here br0 and eth0.2) for the different
networks.

But I am open and grateful for any help and suggestions in that area.

> you should be able to get away with a single bridge master device, can
> you try that?
Done. See above. Works!

> Overall this is fills a lot of gaps and questions that were being
> asked on the lamobo R1 threads on various forums, thanks a lot for
> doing that!
Thx.

Regards
    Bene Spranger

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2019-06-28 17:07 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-18 17:57 [RFC PATCH 0/2] enable broadcom tagging for bcm531x5 switches Benedikt Spranger
2019-06-18 17:57 ` [RFC PATCH 1/2] net: dsa: b53: Turn on managed mode and set IMP port Benedikt Spranger
2019-06-18 18:10   ` Florian Fainelli
2019-06-18 17:57 ` [RFC PATCH 2/2] net: dsa: b53: enbale broadcom tags on bcm531x5 Benedikt Spranger
2019-06-18 18:16 ` [RFC PATCH 0/2] enable broadcom tagging for bcm531x5 switches Florian Fainelli
2019-06-19  9:18   ` Benedikt Spranger
2019-06-23  2:24     ` Florian Fainelli
2019-06-25 11:20       ` Benedikt Spranger
2019-06-25 18:17         ` Florian Fainelli
2019-06-27 10:15           ` [RFC PATCH 0/1] Document the configuration of b53 Benedikt Spranger
2019-06-27 10:15             ` [RFC PATCH 1/1] Documentation: net: dsa: b53: Describe b53 configuration Benedikt Spranger
2019-06-27 13:49               ` Andrew Lunn
2019-06-27 14:43                 ` Benedikt Spranger
2019-06-27 16:38               ` Florian Fainelli
2019-06-28 11:44                 ` Kurt Kanzenbach
2019-06-28 16:57                 ` Benedikt Spranger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).