[PATCH net-next 0/3] net: dsa: lan9303: Fix STP and flooding issues

[PATCH net-next 0/3] net: dsa: lan9303: Fix STP and flooding issues

[Egil Hjelmeland]

This patch set finishes the STP support, and fixes flooding issues.

Patch 1 fixes a flooding issue in the previous patch set.
Patch 2 finishes STP support by adding a ALR entry.
Patch 3 prevent duplicate flooding in HW and SW bridge.


Egil Hjelmeland (3):
  net: dsa: lan9303: Transmit using ALR when unicast
  net: dsa: lan9303: Add STP ALR entry on port 0
  net: dsa: lan9303: lan9303_rcv set skb->offload_fwd_mark

 drivers/net/dsa/lan9303-core.c | 2 ++
 include/linux/dsa/lan9303.h    | 2 ++
 net/dsa/tag_lan9303.c          | 7 ++++---
 3 files changed, 8 insertions(+), 3 deletions(-)

-- 
2.11.0

[PATCH net-next 1/3] net: dsa: lan9303: Transmit using ALR when unicast

[Egil Hjelmeland]

lan9303_xmit_use_arl() introduced in previous patch set is wrong.
The chip flood broadcast and unknown multicast frames. The effect is that
broadcasts and multicasts are duplicated on egress. It is not possible to
configure the chip to direct unknown multicasts to CPU port only.

This means that only unicast frames can be transmitted using ALR lookup.

Signed-off-by: Egil Hjelmeland <privat@egil-hjelmeland.no>
---
 net/dsa/tag_lan9303.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/dsa/tag_lan9303.c b/net/dsa/tag_lan9303.c
index 64092325aac3..537ca991fafe 100644
--- a/net/dsa/tag_lan9303.c
+++ b/net/dsa/tag_lan9303.c
@@ -46,7 +46,7 @@
 
 /* Decide whether to transmit using ALR lookup, or transmit directly to
  * port using tag. ALR learning is performed only when using ALR lookup.
- * If the two external ports are bridged and the packet is not STP BPDU,
+ * If the two external ports are bridged and the frame is unicast,
  * then use ALR lookup to allow ALR learning on CPU port.
  * Otherwise transmit directly to port with STP state override.
  * See also: lan9303_separate_ports() and lan9303.pdf 6.4.10.1
@@ -55,7 +55,7 @@ static int lan9303_xmit_use_arl(struct dsa_port *dp, u8 *dest_addr)
 {
 	struct lan9303 *chip = dp->ds->priv;
 
-	return chip->is_bridged && !ether_addr_equal(dest_addr, eth_stp_addr);
+	return chip->is_bridged && !is_multicast_ether_addr(dest_addr);
 }
 
 static struct sk_buff *lan9303_xmit(struct sk_buff *skb, struct net_device *dev)
-- 
2.11.0

[PATCH net-next 2/3] net: dsa: lan9303: Add STP ALR entry on port 0

[Egil Hjelmeland]

STP BPDUs arriving on user ports must sent to CPU port only,
for processing by the SW bridge.

Add an ALR entry with STP state override to fix that.

Signed-off-by: Egil Hjelmeland <privat@egil-hjelmeland.no>
---
 drivers/net/dsa/lan9303-core.c | 2 ++
 include/linux/dsa/lan9303.h    | 2 ++
 net/dsa/tag_lan9303.c          | 1 -
 3 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
index 4c412bd52319..c4afc8f1a66d 100644
--- a/drivers/net/dsa/lan9303-core.c
+++ b/drivers/net/dsa/lan9303-core.c
@@ -773,6 +773,7 @@ static int lan9303_separate_ports(struct lan9303 *chip)
 {
 	int ret;
 
+	lan9303_alr_del_port(chip, eth_stp_addr, 0);
 	ret = lan9303_write_switch_reg(chip, LAN9303_SWE_PORT_MIRROR,
 				LAN9303_SWE_PORT_MIRROR_SNIFFER_PORT0 |
 				LAN9303_SWE_PORT_MIRROR_MIRRORED_PORT1 |
@@ -797,6 +798,7 @@ static void lan9303_bridge_ports(struct lan9303 *chip)
 
 	lan9303_write_switch_reg(chip, LAN9303_SWE_PORT_STATE,
 				 chip->swe_port_state);
+	lan9303_alr_add_port(chip, eth_stp_addr, 0, true);
 }
 
 static int lan9303_handle_reset(struct lan9303 *chip)
diff --git a/include/linux/dsa/lan9303.h b/include/linux/dsa/lan9303.h
index 05d8d136baab..b2110e69630f 100644
--- a/include/linux/dsa/lan9303.h
+++ b/include/linux/dsa/lan9303.h
@@ -34,3 +34,5 @@ struct lan9303 {
 	 **/
 	struct lan9303_alr_cache_entry alr_cache[LAN9303_NUM_ALR_RECORDS];
 };
+
+#define eth_stp_addr eth_reserved_addr_base
diff --git a/net/dsa/tag_lan9303.c b/net/dsa/tag_lan9303.c
index 537ca991fafe..18f45cd9f625 100644
--- a/net/dsa/tag_lan9303.c
+++ b/net/dsa/tag_lan9303.c
@@ -42,7 +42,6 @@
 #define LAN9303_TAG_LEN 4
 # define LAN9303_TAG_TX_USE_ALR BIT(3)
 # define LAN9303_TAG_TX_STP_OVERRIDE BIT(4)
-#define eth_stp_addr eth_reserved_addr_base
 
 /* Decide whether to transmit using ALR lookup, or transmit directly to
  * port using tag. ALR learning is performed only when using ALR lookup.
-- 
2.11.0

[PATCH net-next 3/3] net: dsa: lan9303: lan9303_rcv set skb->offload_fwd_mark

[Egil Hjelmeland]

The chip flood broadcast and unknown multicast frames.
On receive set skb->offload_fwd_mark to prevent the SW from flooding to the
same ports.

One exception: Because the ALR is set up to forward STP BPDUs only to CPU,
the SW bridge should flood STP BPDUs if local STP is not enabled.
This is archived by not setting skb->offload_fwd_mark on STP BPDUs.

Signed-off-by: Egil Hjelmeland <privat@egil-hjelmeland.no>
---
 net/dsa/tag_lan9303.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/dsa/tag_lan9303.c b/net/dsa/tag_lan9303.c
index 18f45cd9f625..e526c8967b98 100644
--- a/net/dsa/tag_lan9303.c
+++ b/net/dsa/tag_lan9303.c
@@ -126,6 +126,8 @@ static struct sk_buff *lan9303_rcv(struct sk_buff *skb, struct net_device *dev,
 	skb_pull_rcsum(skb, 2 + 2);
 	memmove(skb->data - ETH_HLEN, skb->data - (ETH_HLEN + LAN9303_TAG_LEN),
 		2 * ETH_ALEN);
+	skb->offload_fwd_mark = !ether_addr_equal(skb->data - ETH_HLEN,
+						  eth_stp_addr);
 
 	return skb;
 }
-- 
2.11.0

Re: [PATCH net-next 3/3] net: dsa: lan9303: lan9303_rcv set skb->offload_fwd_mark

[Andrew Lunn]

> One exception: Because the ALR is set up to forward STP BPDUs only to CPU,
> the SW bridge should flood STP BPDUs if local STP is not enabled.
> This is archived by not setting skb->offload_fwd_mark on STP BPDUs.

Hi Egil

This bit is interesting. I had not thought about that.

I think we need a few more drivers to set offload_fwd_mark, and then
maybe try to consolidate it into the core. But it is too early now.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net-next 2/3] net: dsa: lan9303: Add STP ALR entry on port 0

[Andrew Lunn]

On Tue, Oct 31, 2017 at 03:48:01PM +0100, Egil Hjelmeland wrote:
> STP BPDUs arriving on user ports must sent to CPU port only,
> for processing by the SW bridge.
> 
> Add an ALR entry with STP state override to fix that.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net-next 1/3] net: dsa: lan9303: Transmit using ALR when unicast

[Andrew Lunn]

On Tue, Oct 31, 2017 at 03:48:00PM +0100, Egil Hjelmeland wrote:
> lan9303_xmit_use_arl() introduced in previous patch set is wrong.
> The chip flood broadcast and unknown multicast frames. The effect is that
> broadcasts and multicasts are duplicated on egress. It is not possible to
> configure the chip to direct unknown multicasts to CPU port only.
> 
> This means that only unicast frames can be transmitted using ALR lookup.
> 
> Signed-off-by: Egil Hjelmeland <privat@egil-hjelmeland.no>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net-next 0/3] net: dsa: lan9303: Fix STP and flooding issues

[David Miller]

From: Egil Hjelmeland <privat@egil-hjelmeland.no>
Date: Tue, 31 Oct 2017 15:47:59 +0100

> This patch set finishes the STP support, and fixes flooding issues.
> 
> Patch 1 fixes a flooding issue in the previous patch set.
> Patch 2 finishes STP support by adding a ALR entry.
> Patch 3 prevent duplicate flooding in HW and SW bridge.

Series applied, thank you.

Re: [PATCH net-next 0/3] net: dsa: lan9303: Fix STP and flooding issues

[Egil Hjelmeland]

On 01. nov. 2017 13:30, David Miller wrote:
> From: Egil Hjelmeland <privat@egil-hjelmeland.no>
> Date: Tue, 31 Oct 2017 15:47:59 +0100
> 
>> This patch set finishes the STP support, and fixes flooding issues.
>>
> 
> Series applied, thank you.
> 

Thanks! Based on happy testing on my desktop, I conclude that the 
net-next lan9303 driver has now reached a level where my employer 
Zenitel can use it out of the box. We have covered everything we need 
from our proprietary driver.

Our old proprietary driver can not separate the interfaces. Being able 
to do so may be interesting in some custom usages of our product. And we 
are very much looking forward to Andrew progressing his series for IGMP 
snooping for local traffic!

Egil