net: dsa: mxl862xx: add support for bridge offloading
Daniel Golle
maintainercover net: dsa: mxl862xx: add support for bridge offloading
As a next step to complete the mxl862xx DSA driver, add support for offloading forwarding between bridged ports to the switch hardware.
This works pretty much without any big surprises, apart from two subtleties:
* per-port control over flooding behavior has to be implemented by (ab)using a 0-rate QoS meters as stopper in lack of any better option.
* STP state transition unconditionally enables learning on a port even if it was previously explicitely disabled (a firmware bug)
Note that as the driver is still lacking all VLAN features (which are going to be added next), at this point some of the bridge_vlan_aware.sh tests are failing after applying this series.
This is expected and cannot be avoided without implementing port_vlan_filtering + port_vlan_add/del. And adding both bridge and VLAN offloading at the same time would be too much for anyone to review.
All other relevant selftests (including bridge_vlan_unaware.sh) are still passing.
Daniel Golle (2):
dsa: tag_mxl862xx: set dsa_default_offload_fwd_mark()
net: dsa: mxl862xx: implement bridge offloading
drivers/net/dsa/mxl862xx/mxl862xx-api.h | 171 +++++++ drivers/net/dsa/mxl862xx/mxl862xx-cmd.h | 19 +- drivers/net/dsa/mxl862xx/mxl862xx.c | 576 ++++++++++++++++++++++-- drivers/net/dsa/mxl862xx/mxl862xx.h | 21 + net/dsa/tag_mxl862xx.c | 2 + 5 files changed, 757 insertions(+), 32 deletions(-)
Signature · 1 line
2.53.0
Daniel Golle
maintainerPATCH 1/2 dsa: tag_mxl862xx: set dsa_default_offload_fwd_mark()
The MxL862xx offloads forwarding between bridged ports to the hardware, so set dsa_default_offload_fwd_mark() to avoid duplicate forwarding of packets of (eg. flooded) frames arriving at the CPU port.
--- net/dsa/tag_mxl862xx.c | 2 ++ 1 file changed, 2 insertions(+)
net/dsa/tag_mxl862xx.c +2 -0
@@ -92,6 +92,8 @@ static struct sk_buff *mxl862_tag_rcv(struct sk_buff *skb,skb_pull_rcsum (skb,MXL862_HEADER_LEN );dsa_strip_etype_header (skb,MXL862_HEADER_LEN ); +dsa_default_offload_fwd_mark (skb); +return skb; }
Signature · 1 line
2.53.0
Re: [PATCH net-next 1/2] dsa: tag_mxl862xx: set dsa_default_offload_fwd_mark()
Hi,
Quoted 15 lines · Daniel Golle
The MxL862xx offloads forwarding between bridged ports to the hardware, so set
dsa_default_offload_fwd_mark()to avoid duplicate forwarding of packets of (eg. flooded) frames arriving at the CPU port.Signed-off-by Daniel Golle <[email protected]>--- net/dsa/tag_mxl862xx.c | 2 ++ 1 file changed, 2 insertions(+)net/dsa/tag_mxl862xx.c +0
-0
net/dsa/tag_mxl862xx.c static struct sk_buff *mxl862_tag_rcv(struct sk_buff *skb, · line 95
Quoted 4 lines
net/dsa/tag_mxl862xx.c +1
-0@@ -92,6 +92,8 @@ static struct sk_buff *mxl862_tag_rcv(struct sk_buff *skb,skb_pull_rcsum (skb,MXL862_HEADER_LEN );dsa_strip_etype_header (skb,MXL862_HEADER_LEN ); +dsa_default_offload_fwd_mark (skb);
Does the switch (by default) also flood link local traffic (e.g. STP, LACP, etc)? If not, you should not mark these as fwd offloaded.
Is there is a bit in the header that says whether a packet was flooded or trapped that you can check?
Best regards,
Jonas
Daniel Golle
maintainerRe: [PATCH net-next 1/2] dsa: tag_mxl862xx: set dsa_default_offload_fwd_mark()
Quoted 21 lines · Jonas Gorski
Hi,
Quoted 18 lines · Daniel Golle
The MxL862xx offloads forwarding between bridged ports to the hardware, so set
dsa_default_offload_fwd_mark()to avoid duplicate forwarding of packets of (eg. flooded) frames arriving at the CPU port.Signed-off-by Daniel Golle <[email protected]>--- net/dsa/tag_mxl862xx.c | 2 ++ 1 file changed, 2 insertions(+)net/dsa/tag_mxl862xx.c +0
-0@@ -92,6 +92,8 @@ static struct sk_buff *mxl862_tag_rcv(struct sk_buff *skb,skb_pull_rcsum (skb,MXL862_HEADER_LEN );dsa_strip_etype_header (skb,MXL862_HEADER_LEN );
Quoted 4 lines
net/dsa/tag_mxl862xx.c static struct sk_buff *mxl862_tag_rcv(struct sk_buff *skb, · line 95
Quoted 1 line
net/dsa/tag_mxl862xx.c +1
-0+dsa_default_offload_fwd_mark (skb);Does the switch (by default) also flood link local traffic (e.g. STP, LACP, etc)? If not, you should not mark these as fwd offloaded.
No, it doesn't. Testing revealed that apparently it silently consumes them. I assume we'll need something like
if (likely(!is_link_local_ether_addr(eth_hdr(skb)->h_dest)))
dsa_default_offload_fwd_mark(skb);
similar to tag_brcm.c, right?
Quoted 2 lines
Is there is a bit in the header that says whether a packet was flooded or trapped that you can check?
Earlier testing showed that frames arriving at the microcontroller under some conditions are relayed to the DSA CPU port -- and are destinguishable as such. However, there is not bit directly indicating whether the packet was (also) flooded already.
Re: [PATCH net-next 1/2] dsa: tag_mxl862xx: set dsa_default_offload_fwd_mark()
Quoted 28 lines · Daniel Golle
Quoted 21 lines · Jonas Gorski
Hi,
Quoted 18 lines · Daniel Golle
The MxL862xx offloads forwarding between bridged ports to the hardware, so set
dsa_default_offload_fwd_mark()to avoid duplicate forwarding of packets of (eg. flooded) frames arriving at the CPU port.Signed-off-by Daniel Golle <[email protected]>--- net/dsa/tag_mxl862xx.c | 2 ++ 1 file changed, 2 insertions(+)net/dsa/tag_mxl862xx.c +0
-0@@ -92,6 +92,8 @@ static struct sk_buff *mxl862_tag_rcv(struct sk_buff *skb,skb_pull_rcsum (skb,MXL862_HEADER_LEN );dsa_strip_etype_header (skb,MXL862_HEADER_LEN );Quoted 4 lines
net/dsa/tag_mxl862xx.c static struct sk_buff *mxl862_tag_rcv(struct sk_buff *skb, · line 95
Quoted 1 line
net/dsa/tag_mxl862xx.c +1
-0+dsa_default_offload_fwd_mark (skb);Does the switch (by default) also flood link local traffic (e.g. STP, LACP, etc)? If not, you should not mark these as fwd offloaded.
Quoted 7 lines
No, it doesn't. Testing revealed that apparently it silently consumes them. I assume we'll need something like
if (likely(!is_link_local_ether_addr(eth_hdr(skb)->h_dest)))
dsa_default_offload_fwd_mark(skb);similar to
tag_brcm.c, right?
Yepp, that would have been my suggestion :-)
Quoted 7 lines
Quoted 2 lines
Is there is a bit in the header that says whether a packet was flooded or trapped that you can check?
Earlier testing showed that frames arriving at the microcontroller under some conditions are relayed to the DSA CPU port -- and are destinguishable as such. However, there is not bit directly indicating whether the packet was (also) flooded already.
So you know if they were copied to the cpu port due to some rules, but not if they were snooped or trapped?
Best regards,
Jonas
Daniel Golle
maintainerRe: [PATCH net-next 1/2] dsa: tag_mxl862xx: set dsa_default_offload_fwd_mark()
Quoted 32 lines · Jonas Gorski
Quoted 31 lines · Daniel Golle
Quoted 21 lines · Jonas Gorski
Hi,
Quoted 18 lines · Daniel Golle
The MxL862xx offloads forwarding between bridged ports to the hardware, so set
dsa_default_offload_fwd_mark()to avoid duplicate forwarding of packets of (eg. flooded) frames arriving at the CPU port.Signed-off-by Daniel Golle <[email protected]>--- net/dsa/tag_mxl862xx.c | 2 ++ 1 file changed, 2 insertions(+)net/dsa/tag_mxl862xx.c +0
-0@@ -92,6 +92,8 @@ static struct sk_buff *mxl862_tag_rcv(struct sk_buff *skb,skb_pull_rcsum (skb,MXL862_HEADER_LEN );dsa_strip_etype_header (skb,MXL862_HEADER_LEN );Quoted 4 lines
net/dsa/tag_mxl862xx.c static struct sk_buff *mxl862_tag_rcv(struct sk_buff *skb, · line 95
Quoted 1 line
net/dsa/tag_mxl862xx.c +1
-0+dsa_default_offload_fwd_mark (skb);Does the switch (by default) also flood link local traffic (e.g. STP, LACP, etc)? If not, you should not mark these as fwd offloaded.
No, it doesn't. Testing revealed that apparently it silently consumes them. I assume we'll need something like
Quoted 6 lines
Quoted 4 lines
if (likely(!is_link_local_ether_addr(eth_hdr(skb)->h_dest)))
dsa_default_offload_fwd_mark(skb);similar to
tag_brcm.c, right?Yepp, that would have been my suggestion :-)
Ok, I'll do that then.
Quoted 11 lines
Quoted 8 lines
Quoted 3 lines
Is there is a bit in the header that says whether a packet was flooded or trapped that you can check?
Earlier testing showed that frames arriving at the microcontroller under some conditions are relayed to the DSA CPU port -- and are destinguishable as such. However, there is not bit directly indicating whether the packet was (also) flooded already.
So you know if they were copied to the cpu port due to some rules, but not if they were snooped or trapped?
I haven't found any details about that documented, and all I can say is that in some cases (some of the IEEE1588v2 frames generated by the local_termination.sh selftest, for example) the microcontroller seems to send an additional copy of the frame to the DSA CPU port using fields of the 8-byte tag which usually aren't used -- and that's why I even noticed them. The initial driver now merged upstream disabled the microcontroller SDMA and FMDA, effectively disconnecting it from the switch datapath, so it would shut up, and this effect can now longer be observed.
Now, when testing if link-local frames would be forwarded I noticed that there is a set of default rules instatiated after reset which trap link-local frames to the microcontroller port. I suppose it could also be setup to snoop instead of trap by manually erasing these PCE rules and setting new ones instead. In this case, however, there would be no way to tell from the frame received on the DSA CPU port if the frame had also been sent to the microcontroller.
Re: [PATCH net-next 1/2] dsa: tag_mxl862xx: set dsa_default_offload_fwd_mark()
Quoted 65 lines · Daniel Golle
Quoted 32 lines · Jonas Gorski
Quoted 31 lines · Daniel Golle
Quoted 21 lines · Jonas Gorski
Hi,
Quoted 18 lines · Daniel Golle
The MxL862xx offloads forwarding between bridged ports to the hardware, so set
dsa_default_offload_fwd_mark()to avoid duplicate forwarding of packets of (eg. flooded) frames arriving at the CPU port.Signed-off-by Daniel Golle <[email protected]>--- net/dsa/tag_mxl862xx.c | 2 ++ 1 file changed, 2 insertions(+)net/dsa/tag_mxl862xx.c +0
-0@@ -92,6 +92,8 @@ static struct sk_buff *mxl862_tag_rcv(struct sk_buff *skb,skb_pull_rcsum (skb,MXL862_HEADER_LEN );dsa_strip_etype_header (skb,MXL862_HEADER_LEN );Quoted 4 lines
net/dsa/tag_mxl862xx.c static struct sk_buff *mxl862_tag_rcv(struct sk_buff *skb, · line 95
Quoted 1 line
net/dsa/tag_mxl862xx.c +1
-0+dsa_default_offload_fwd_mark (skb);Does the switch (by default) also flood link local traffic (e.g. STP, LACP, etc)? If not, you should not mark these as fwd offloaded.
No, it doesn't. Testing revealed that apparently it silently consumes them. I assume we'll need something like
Quoted 6 lines
Quoted 4 lines
if (likely(!is_link_local_ether_addr(eth_hdr(skb)->h_dest)))
dsa_default_offload_fwd_mark(skb);similar to
tag_brcm.c, right?Yepp, that would have been my suggestion :-)
Ok, I'll do that then.
Quoted 11 lines
Quoted 8 lines
Quoted 3 lines
Is there is a bit in the header that says whether a packet was flooded or trapped that you can check?
Earlier testing showed that frames arriving at the microcontroller under some conditions are relayed to the DSA CPU port -- and are destinguishable as such. However, there is not bit directly indicating whether the packet was (also) flooded already.
So you know if they were copied to the cpu port due to some rules, but not if they were snooped or trapped?
I haven't found any details about that documented, and all I can say is that in some cases (some of the IEEE1588v2 frames generated by the
local_termination.shselftest, for example) the microcontroller seems to send an additional copy of the frame to the DSA CPU port using fields of the 8-byte tag which usually aren't used -- and that's why I even noticed them. The initial driver now merged upstream disabled the microcontroller SDMA and FMDA, effectively disconnecting it from the switch datapath, so it would shut up, and this effect can now longer be observed.
Quoted 7 lines
Now, when testing if link-local frames would be forwarded I noticed that there is a set of default rules instatiated after reset which trap link-local frames to the microcontroller port. I suppose it could also be setup to snoop instead of trap by manually erasing these PCE rules and setting new ones instead. In this case, however, there would be no way to tell from the frame received on the DSA CPU port if the frame had also been sent to the microcontroller.
For dynamic configuration you would probably need to then check the destination address, check if snooping or trapping is configured for it, and then set (not) mark offloaded accordingly. While linux allows you do dynamically configure some of the groups between flooding/trapping, I'm not sure if this is propagated via dsa or even switchdev.
I suggest sticking to the default of trapping and not marking as offloaded. It's much easier to flood in software if needed than the other way round.
Best regards,
Jonas
Daniel Golle
maintainerRe: [PATCH net-next 1/2] dsa: tag_mxl862xx: set dsa_default_offload_fwd_mark()
Quoted 81 lines · Jonas Gorski
Quoted 65 lines · Daniel Golle
Quoted 32 lines · Jonas Gorski
Quoted 31 lines · Daniel Golle
Quoted 21 lines · Jonas Gorski
Hi,
Quoted 18 lines · Daniel Golle
The MxL862xx offloads forwarding between bridged ports to the hardware, so set
dsa_default_offload_fwd_mark()to avoid duplicate forwarding of packets of (eg. flooded) frames arriving at the CPU port.Signed-off-by Daniel Golle <[email protected]>--- net/dsa/tag_mxl862xx.c | 2 ++ 1 file changed, 2 insertions(+)net/dsa/tag_mxl862xx.c +0
-0@@ -92,6 +92,8 @@ static struct sk_buff *mxl862_tag_rcv(struct sk_buff *skb,skb_pull_rcsum (skb,MXL862_HEADER_LEN );dsa_strip_etype_header (skb,MXL862_HEADER_LEN );Quoted 4 lines
net/dsa/tag_mxl862xx.c static struct sk_buff *mxl862_tag_rcv(struct sk_buff *skb, · line 95
Quoted 1 line
net/dsa/tag_mxl862xx.c +1
-0+dsa_default_offload_fwd_mark (skb);Does the switch (by default) also flood link local traffic (e.g. STP, LACP, etc)? If not, you should not mark these as fwd offloaded.
No, it doesn't. Testing revealed that apparently it silently consumes them. I assume we'll need something like
Quoted 6 lines
Quoted 4 lines
if (likely(!is_link_local_ether_addr(eth_hdr(skb)->h_dest)))
dsa_default_offload_fwd_mark(skb);similar to
tag_brcm.c, right?Yepp, that would have been my suggestion :-)
Ok, I'll do that then.
Quoted 11 lines
Quoted 8 lines
Quoted 3 lines
Is there is a bit in the header that says whether a packet was flooded or trapped that you can check?
Earlier testing showed that frames arriving at the microcontroller under some conditions are relayed to the DSA CPU port -- and are destinguishable as such. However, there is not bit directly indicating whether the packet was (also) flooded already.
So you know if they were copied to the cpu port due to some rules, but not if they were snooped or trapped?
I haven't found any details about that documented, and all I can say is that in some cases (some of the IEEE1588v2 frames generated by the
local_termination.shselftest, for example) the microcontroller seems to send an additional copy of the frame to the DSA CPU port using fields of the 8-byte tag which usually aren't used -- and that's why I even noticed them. The initial driver now merged upstream disabled the microcontroller SDMA and FMDA, effectively disconnecting it from the switch datapath, so it would shut up, and this effect can now longer be observed.Quoted 7 lines
Now, when testing if link-local frames would be forwarded I noticed that there is a set of default rules instatiated after reset which trap link-local frames to the microcontroller port. I suppose it could also be setup to snoop instead of trap by manually erasing these PCE rules and setting new ones instead. In this case, however, there would be no way to tell from the frame received on the DSA CPU port if the frame had also been sent to the microcontroller.
For dynamic configuration you would probably need to then check the destination address, check if snooping or trapping is configured for it, and then set (not) mark offloaded accordingly. While linux allows you do dynamically configure some of the groups between flooding/trapping, I'm not sure if this is propagated via dsa or even switchdev.
Quoted 3 lines
I suggest sticking to the default of trapping and not marking as offloaded. It's much easier to flood in software if needed than the other way round.
Yeah, I'll do that and right now I'm trying to come up with a set of PCE rules which allow for link-local frames to even arrive at the CPU port at all (which would allow software flooding or even doing anything with them on the CPU)
Vladimir Oltean
maintainerRe: [PATCH net-next 1/2] dsa: tag_mxl862xx: set dsa_default_offload_fwd_mark()
In reply to Jonas GorskiQuoted 7 lines · Jonas Gorski
For dynamic configuration you would probably need to then check the destination address, check if snooping or trapping is configured for it, and then set (not) mark offloaded accordingly. While linux allows you do dynamically configure some of the groups between flooding/trapping, I'm not sure if this is propagated via dsa or even switchdev.
Quoted 6 lines
I suggest sticking to the default of trapping and not marking as offloaded. It's much easier to flood in software if needed than the other way round.
Best regards,
Jonas
+1, good comments, thanks.
The group_fwd_mask isn't offloaded to switchdev.
Daniel Golle
maintainerPATCH 2/2 net: dsa: mxl862xx: implement bridge offloading
In reply to Daniel GolleAdd support for bridge offloading on the mxl862xx DSA driver.
Implement joining and leaving bridges as well as add, delete and dump operations on isolated FDBs and setting a port's STP state.
The switch supports a maximum of 63 bridges, however, up to 12 may be used as "single-port bridges" to isolate standalone ports. Allowing up to 48 bridges to be offloaded seems more than enough on that hardware, hence that is set as max_num_bridges.
A total of 128 bridge ports are supported in the bridge portmap, and virtual bridge ports have to be used eg. for link-aggregation, hence potentially exceeding the number of hardware ports.
As there are now more users of the BRIDGEPORT_CONFIG_SET API and the state of each port is cached locally, introduce a helper function mxl862xx_set_bridge_port(struct dsa_switch *ds, int port) which is then also be used to replace the direct calls to the API in mxl862xx_setup_cpu_bridge() and mxl862xx_add_single_port_bridge(), while removing the accidentally added VLAN-aware-learning settings.
Note that there is no convenient way to control flooding on per-port level, so the driver is using a QoS meter setup as a stopper in lack of any better option. This works, but allows a single 64-byte packet to pass once after reset. While this limitation doesn't seem to be a problem in practice, it has the effect that the bridge_vlan_unaware.sh selftest only passes the FDB test the 2nd time the test is run after boot (and any subsequent time after that, of course).
--- drivers/net/dsa/mxl862xx/mxl862xx-api.h | 171 +++++++ drivers/net/dsa/mxl862xx/mxl862xx-cmd.h | 19 +- drivers/net/dsa/mxl862xx/mxl862xx.c | 576 ++++++++++++++++++++++-- drivers/net/dsa/mxl862xx/mxl862xx.h | 21 + 4 files changed, 755 insertions(+), 32 deletions(-)
drivers/net/dsa/mxl862xx/mxl862xx-api.h +171 -0
@@ -34,6 +34,117 @@ struct mxl862xx_register_mod {__le16 mask; } __packed; +/** +* enum mxl862xx_mac_table_filter - Source/Destination MAC address filtering +* +* @MXL862XX_MAC_FILTER_NONE: no filter +* @MXL862XX_MAC_FILTER_SRC: source address filter +* @MXL862XX_MAC_FILTER_DEST: destination address filter +* @MXL862XX_MAC_FILTER_BOTH: both source and destination filter +*/ +enum mxl862xx_mac_table_filter { +MXL862XX_MAC_FILTER_NONE =0 , +MXL862XX_MAC_FILTER_SRC =BIT (0 ), +MXL862XX_MAC_FILTER_DEST =BIT (1 ), +MXL862XX_MAC_FILTER_BOTH =BIT (0 ) |BIT (1 ), +}; + +/** +* struct mxl862xx_mac_table_add - MAC Table Entry to be added +* @fid: Filtering Identifier (FID) (not supported by all switches) +* @port_id: Ethernet Port number +* @port_map: Bridge Port Map +* @sub_if_id: Sub-Interface Identifier Destination +* @age_timer: Aging Time in seconds +* @vlan_id: STAG VLAN Id +* @static_entry: Static Entry (value will be aged out if not set to static) +* @traffic_class: Egress queue traffic class +* @mac: MAC Address to add to the table +* @filter_flag: See &enum mxl862xx_mac_table_filter +* @igmp_controlled: Packet is marked as IGMP controlled if destination MAC +* address matche MAC in this entry +* @associated_mac: Associated Mac address +* @tci: TCI for B-Step +* Bit [0:11] - VLAN ID +* Bit [12] - VLAN CFI/DEI +* Bit [13:15] - VLAN PRI +*/ +struct mxl862xx_mac_table_add { +__le16 fid; +__le32 port_id; +__le16 port_map[8 ]; +__le16 sub_if_id; +int age_timer; +__le16 vlan_id; +u8 static_entry; +u8 traffic_class; +u8 mac[ETH_ALEN ]; +u8 filter_flag; +u8 igmp_controlled; +u8 associated_mac[ETH_ALEN ]; +__le16 tci; +} __packed; + +/** +* struct mxl862xx_mac_table_remove - MAC Table Entry to be removed +* @fid: Filtering Identifier (FID) +* @mac: MAC Address to be removed from the table. +* @filter_flag: See &enum mxl862xx_mac_table_filter +* @tci: TCI for B-Step +* Bit [0:11] - VLAN ID +* Bit [12] - VLAN CFI/DEI +* Bit [13:15] - VLAN PRI +*/ +struct mxl862xx_mac_table_remove { +__le16 fid; +u8 mac[ETH_ALEN ]; +u8 filter_flag; +__le16 tci; +} __packed; + +/** +* struct mxl862xx_mac_table_read - MAC Table Entry to be read +* @initial: Restart the get operation from the beginning of the table +* @last: Indicates that the read operation returned last entry +* @fid: Get the MAC table entry belonging to the given Filtering Identifier +* @port_id: The Bridge Port ID +* @port_map: Bridge Port Map +* @age_timer: Aging Time +* @vlan_id: STAG VLAN Id +* @static_entry: Indicates if this is a Static Entry +* @sub_if_id: Sub-Interface Identifier Destination +* @mac: MAC Address. Filled out by the switch API implementation. +* @filter_flag: See &enum mxl862xx_mac_table_filter +* @igmp_controlled: Packet is marked as IGMP controlled if destination MAC +* address matches the MAC in this entry +* @entry_changed: Indicate if the Entry has Changed +* @associated_mac: Associated MAC address +* @hit_status: MAC Table Hit Status Update +* @tci: TCI for B-Step +* Bit [0:11] - VLAN ID +* Bit [12] - VLAN CFI/DEI +* Bit [13:15] - VLAN PRI +*/ +struct mxl862xx_mac_table_read { +u8 initial; +u8 last; +__le16 fid; +__le32 port_id; +__le16 port_map[8 ]; +int age_timer; +__le16 vlan_id; +u8 static_entry; +__le16 sub_if_id; +u8 mac[ETH_ALEN ]; +u8 filter_flag; +u8 igmp_controlled; +u8 entry_changed; +u8 associated_mac[ETH_ALEN ]; +u8 hit_status; +__le16 tci; +__le16 first_bridge_port_id; +} __packed; +/** * enum mxl862xx_mac_clear_type - MAC table clear type * @MXL862XX_MAC_CLEAR_PHY_PORT: clear dynamic entries based on port_id
@@ -138,6 +249,40 @@ enum mxl862xx_bridge_port_egress_meter { MXL862XX_BRIDGE_PORT_EGRESS_METER_MAX, }; +/** +* struct mxl862xx_qos_meter_cfg - Rate meter configuration +* @enable: Enable/disable meter +* @meter_id: Meter ID (assigned by firmware on alloc) +* @meter_name: Meter name string +* @meter_type: Meter algorithm type (srTCM = 0, trTCM = 1) +* @cbs: Committed Burst Size (in bytes) +* @res1: Reserved +* @ebs: Excess Burst Size (in bytes) +* @res2: Reserved +* @rate: Committed Information Rate (in kbit/s) +* @pi_rate: Peak Information Rate (in kbit/s) +* @colour_blind_mode: Colour-blind mode enable +* @pkt_mode: Packet mode enable +* @local_overhd: Local overhead accounting enable +* @local_overhd_val: Local overhead accounting value +*/ +struct mxl862xx_qos_meter_cfg { +u8 enable; +__le16 meter_id; +char meter_name[32 ]; +__le32 meter_type; +__le32 cbs; +__le32 res1; +__le32 ebs; +__le32 res2; +__le32 rate; +__le32 pi_rate; +u8 colour_blind_mode; +u8 pkt_mode; +u8 local_overhd; +__le16 local_overhd_val; +} __packed; +/** * enum mxl862xx_bridge_forward_mode - Bridge forwarding type of packet * @MXL862XX_BRIDGE_FORWARD_FLOOD: Packet is flooded to port members of
@@ -658,6 +803,32 @@ struct mxl862xx_ctp_port_assignment {__le16 bridge_port_id; } __packed; +/** +* enum mxl862xx_stp_port_state +* @MXL862XX_STP_PORT_STATE_FORWARD: Forwarding state +* @MXL862XX_STP_PORT_STATE_DISABLE: Disabled/Discarding state +* @MXL862XX_STP_PORT_STATE_LEARNING: Learning state +* @MXL862XX_STP_PORT_STATE_BLOCKING: Blocking/Listening +*/ +enum mxl862xx_stp_port_state { +MXL862XX_STP_PORT_STATE_FORWARD =0 , +MXL862XX_STP_PORT_STATE_DISABLE , +MXL862XX_STP_PORT_STATE_LEARNING , +MXL862XX_STP_PORT_STATE_BLOCKING , +}; + +/** +* struct mxl862xx_stp_port_cfg - Configures the Spanning Tree Protocol state of an Ethernet port +* @port_id: Port number +* @fid: Filtering Identifier (FID) +* @port_state: See &enum mxl862xx_stp_port_state +*/ +struct mxl862xx_stp_port_cfg { +__le16 port_id; +__le16 fid; +__le32 port_state;/* enum mxl862xx_stp_port_state */ +} __packed; +/** * struct mxl862xx_sys_fw_image_version - Firmware version information * @iv_major: firmware major version
drivers/net/dsa/mxl862xx/mxl862xx-cmd.h +15 -4
@@ -15,12 +15,15 @@#define MXL862XX_BRDG_MAGIC 0x300#define MXL862XX_BRDGPORT_MAGIC 0x400#define MXL862XX_CTP_MAGIC 0x500 +#define MXL862XX_QOS_MAGIC 0x600#define MXL862XX_SWMAC_MAGIC 0xa00 +#define MXL862XX_STP_MAGIC 0xf00#define MXL862XX_SS_MAGIC 0x1600#define GPY_GPY2XX_MAGIC 0x1800#define SYS_MISC_MAGIC 0x1900#define MXL862XX_COMMON_CFGGET (MXL862XX_COMMON_MAGIC + 0x9) +#define MXL862XX_COMMON_CFGSET (MXL862XX_COMMON_MAGIC + 0xa)#define MXL862XX_COMMON_REGISTERMOD (MXL862XX_COMMON_MAGIC + 0x11)#define MXL862XX_BRIDGE_ALLOC (MXL862XX_BRDG_MAGIC + 0x1)
@@ -35,14 +38,22 @@#define MXL862XX_CTP_PORTASSIGNMENTSET (MXL862XX_CTP_MAGIC + 0x3) +#define MXL862XX_QOS_METERCFGSET (MXL862XX_QOS_MAGIC + 0x2) +#define MXL862XX_QOS_METERALLOC (MXL862XX_QOS_MAGIC + 0x2a) + +#define MXL862XX_MAC_TABLEENTRYADD (MXL862XX_SWMAC_MAGIC + 0x2) +#define MXL862XX_MAC_TABLEENTRYREAD (MXL862XX_SWMAC_MAGIC + 0x3) +#define MXL862XX_MAC_TABLEENTRYREMOVE (MXL862XX_SWMAC_MAGIC + 0x5)#define MXL862XX_MAC_TABLECLEARCOND (MXL862XX_SWMAC_MAGIC + 0x8) -#define MXL862XX_SS_SPTAG_SET (MXL862XX_SS_MAGIC + 0x02) +#define MXL862XX_SS_SPTAG_SET (MXL862XX_SS_MAGIC + 0x2) + +#define MXL862XX_STP_PORTCFGSET (MXL862XX_STP_MAGIC + 0x2) -#define INT_GPHY_READ (GPY_GPY2XX_MAGIC + 0x01) -#define INT_GPHY_WRITE (GPY_GPY2XX_MAGIC + 0x02) +#define INT_GPHY_READ (GPY_GPY2XX_MAGIC + 0x1) +#define INT_GPHY_WRITE (GPY_GPY2XX_MAGIC + 0x2) -#define SYS_MISC_FW_VERSION (SYS_MISC_MAGIC + 0x02) +#define SYS_MISC_FW_VERSION (SYS_MISC_MAGIC + 0x2)#define MMD_API_MAXIMUM_ID 0x7fff
drivers/net/dsa/mxl862xx/mxl862xx.c +548 -28
660 lines not on this page · raw message
drivers/net/dsa/mxl862xx/mxl862xx.h +21 -0
@@ -7,10 +7,31 @@#include <net/dsa.h> #define MXL862XX_MAX_PORTS 17 +#define MXL862XX_DEFAULT_BRIDGE 0 +#define MXL862XX_MAX_BRIDGES 48 +#define MXL862XX_MAX_BRIDGE_PORTS 128 + +struct mxl862xx_bridge { +unsigned int dsa_bridge_num; +u16 bridge_id; + DECLARE_BITMAP(portmap, MXL862XX_MAX_BRIDGE_PORTS); +struct list_head list; +}; + +struct mxl862xx_port { +u16 fid;/* single-port bridge ID (permanent) */ +struct mxl862xx_bridge *bridge; + DECLARE_BITMAP(portmap, MXL862XX_MAX_BRIDGE_PORTS); +unsigned long flood_block;/* bitmask of meter indices with metering on */ +bool learning; +};struct mxl862xx_priv {struct dsa_switch *ds;struct mdio_device *mdiodev; +u16 drop_meter;/* single zero-rate meter shared by all ports */ +struct mxl862xx_port ports[MXL862XX_MAX_PORTS ]; +struct list_head bridges; };#endif /* __MXL862XX_H */
Signature · 1 line
2.53.0
Jakub Kicinski
maintainerRe: [PATCH net-next 2/2] net: dsa: mxl862xx: implement bridge offloading
Quoted 21 lines · On Sat, 7 Mar 2026 03:31:17 +0000 Daniel Golle wrote:
Add support for bridge offloading on the mxl862xx DSA driver.
Implement joining and leaving bridges as well as add, delete and dump operations on isolated FDBs and setting a port's STP state.
The switch supports a maximum of 63 bridges, however, up to 12 may be used as "single-port bridges" to isolate standalone ports. Allowing up to 48 bridges to be offloaded seems more than enough on that hardware, hence that is set as
max_num_bridges.A total of 128 bridge ports are supported in the bridge portmap, and virtual bridge ports have to be used eg. for link-aggregation, hence potentially exceeding the number of hardware ports.
As there are now more users of the
BRIDGEPORT_CONFIG_SETAPI and the state of each port is cached locally, introduce a helper functionmxl862xx_set_bridge_port(struct dsa_switch *ds, int port)which is then also be used to replace the direct calls to the API inmxl862xx_setup_cpu_bridge()andmxl862xx_add_single_port_bridge(), while removing the accidentally added VLAN-aware-learning settings.
Quoted 7 lines
Note that there is no convenient way to control flooding on per-port level, so the driver is using a QoS meter setup as a stopper in lack of any better option. This works, but allows a single 64-byte packet to pass once after reset. While this limitation doesn't seem to be a problem in practice, it has the effect that the
bridge_vlan_unaware.shselftest only passes the FDB test the 2nd time the test is run after boot (and any subsequent time after that, of course).
Warning: drivers/net/dsa/mxl862xx/mxl862xx-api.h:145 struct member 'first_bridge_port_id' not described in 'mxl862xx_mac_table_read'
Warning: drivers/net/dsa/mxl862xx/mxl862xx-api.h:806 missing initial short description on line:
* enum mxl862xx_stp_port_state
Also AI has a bunch of endianness complaints:
https://netdev-ai.bots.linux.dev/ai-review.html?id=b924beb1-9dec-4afb-ae21-5585ec8e7667
Signature · 1 line
pw-bot: cr
Vladimir Oltean
maintainerRe: [PATCH net-next 2/2] net: dsa: mxl862xx: implement bridge offloading
In reply to Daniel Golledrivers/net/dsa/mxl862xx/mxl862xx.c static int mxl862xx_configure_sp_tag_proto(struct dsa_switch *ds, int port, · line 489
Quoted 3 lines · Daniel Golle
drivers/net/dsa/mxl862xx/mxl862xx.c +3
-0+static int mxl862xx_update_bridge (struct dsa_switch *ds, +struct mxl862xx_bridge *mxlbridge, +int port,bool join)
There's a lot of false sharing here. If you move the "if (join)" and "if (!join)" blocks to their respective callers, you end up with a much smaller truly common function.
Quoted 15 lines
drivers/net/dsa/mxl862xx/mxl862xx.c +15
-0+{ +struct mxl862xx_priv *priv = ds->priv; +struct dsa_port *dp; +int member, ret; + +if (join) { +__set_bit (port, mxlbridge->portmap); + priv->ports[port].bridge = mxlbridge; + }else { +__clear_bit (port, mxlbridge->portmap); + priv->ports[port].bridge =NULL ; + } + +/* Update all current bridge members' portmaps */ + for_each_set_bit(member, mxlbridge->portmap,
drivers/net/dsa/mxl862xx/mxl862xx.c static int mxl862xx_configure_sp_tag_proto(struct dsa_switch *ds, int port, · line 514
Quoted 10 lines
drivers/net/dsa/mxl862xx/mxl862xx.c +10
-0+ MXL862XX_MAX_BRIDGE_PORTS) { + dp =dsa_to_port (ds, member); + +/* Build portmap: CPU port + all bridge members except self */ +bitmap_copy (priv->ports[member].portmap, mxlbridge->portmap, +MXL862XX_MAX_BRIDGE_PORTS ); +__clear_bit (member, priv->ports[member].portmap); +__set_bit (dp->cpu_dp->index, priv->ports[member].portmap); + + priv->ports[member].learning = true;
Actually, dsa_port_inherit_brport_flags() / dsa_port_clear_brport_flags() ensures the BR_LEARNING flag passed through mxl862xx_port_bridge_flags() will keep the hardware in sync with the bridge. You don't need to set this to true or to false at runtime. Just at initial probe time, before DSA has made any call.
drivers/net/dsa/mxl862xx/mxl862xx.c static int mxl862xx_configure_sp_tag_proto(struct dsa_switch *ds, int port, · line 517
Quoted 3 lines
drivers/net/dsa/mxl862xx/mxl862xx.c +3
-0+ ret =mxl862xx_set_bridge_port (ds, member); +if (ret) +return ret;
What about going through all ports anyway, and return error at the end if any bridge member portmap update failed? I know this patch set is written assuming FW I/O failure is catastrophic, but here you may be failing for a different port than the one which is leaving the bridge. You can at least make an attempt to disable forwarding to/from that port.
Quoted 38 lines
drivers/net/dsa/mxl862xx/mxl862xx.c +36
-1+ } + +/* Revert leaving port to its single-port bridge */ +if (!join) { + dp =dsa_to_port (ds, port); + +bitmap_zero (priv->ports[port].portmap,MXL862XX_MAX_BRIDGE_PORTS ); +__set_bit (dp->cpu_dp->index, priv->ports[port].portmap); + priv->ports[port].flood_block =0 ; + priv->ports[port].learning = false; + ret =mxl862xx_set_bridge_port (ds, port); +if (ret) +return ret; + +mxl862xx_port_fast_age (ds, port); + } + +return 0 ; +} + +static int mxl862xx_port_bridge_join (struct dsa_switch *ds,int port, +struct dsa_bridge bridge, +bool *tx_fwd_offload, +struct netlink_ext_ack *extack) +{ +struct mxl862xx_bridge *mxlbridge; -return MXL862XX_API_WRITE (ds->priv,MXL862XX_BRIDGEPORT_CONFIGSET , br_port_cfg); + mxlbridge =mxl862xx_find_bridge (ds, bridge); +if (!mxlbridge) { + mxlbridge =mxl862xx_allocate_bridge (ds, bridge.num); +if (IS_ERR (mxlbridge)) +return PTR_ERR (mxlbridge); + } + +return mxl862xx_update_bridge (ds, mxlbridge, port, true); +} +
drivers/net/dsa/mxl862xx/mxl862xx.c static int mxl862xx_configure_sp_tag_proto(struct dsa_switch *ds, int port, · line 564
Quoted 10 lines
drivers/net/dsa/mxl862xx/mxl862xx.c +10
-0+static void mxl862xx_port_bridge_leave (struct dsa_switch *ds,int port, +struct dsa_bridge bridge) +{ +struct mxl862xx_bridge *mxlbridge; + + mxlbridge =mxl862xx_find_bridge (ds, bridge); +if (!mxlbridge) +return ; + +mxl862xx_update_bridge (ds, mxlbridge, port, false);
Don't let this fail silently.
Quoted 30 lines
drivers/net/dsa/mxl862xx/mxl862xx.c +23
-0+ +if (bitmap_empty (mxlbridge->portmap,MXL862XX_MAX_BRIDGE_PORTS )) +mxl862xx_free_bridge (ds, mxlbridge); }static int mxl862xx_port_setup (struct dsa_switch *ds,int port)@@ -366,6 +614,262 @@ static void mxl862xx_phylink_get_caps(struct dsa_switch *ds, int port, config->supported_interfaces); } +static int mxl862xx_port_fdb_add (struct dsa_switch *ds,int port, +const unsigned char *addr,u16 vid,struct dsa_db db) +{ +struct mxl862xx_mac_table_add param = { }; +struct mxl862xx_priv *priv = ds->priv; +struct mxl862xx_bridge *mxlbridge; +u16 fid; +int ret; + +switch (db.type) { +case DSA_DB_PORT : + fid = priv->ports[db.dp->index].fid; +break ; + +case DSA_DB_BRIDGE : + mxlbridge =mxl862xx_find_bridge (ds, db.bridge); +if (!mxlbridge) +return -ENOENT ; + fid = mxlbridge->bridge_id; +break ;
drivers/net/dsa/mxl862xx/mxl862xx.c static void mxl862xx_phylink_get_caps(struct dsa_switch *ds, int port, · line 646
Quoted 10 lines
drivers/net/dsa/mxl862xx/mxl862xx.c +10
-0+ +default : +return -EOPNOTSUPP ; + } + + param.port_id =cpu_to_le32 (port); + param.fid =cpu_to_le16 (fid); + param.static_entry = true; + param.tci =cpu_to_le16 (vid &0xFFF ); +memcpy (param.mac, addr,ETH_ALEN );
ether_addr_copy() - similar comment for port_fdb_del()
Quoted 23 lines
drivers/net/dsa/mxl862xx/mxl862xx.c +23
-0+ + ret =MXL862XX_API_WRITE (priv,MXL862XX_MAC_TABLEENTRYADD , param); +if (ret) +dev_err (ds->dev,"failed to add FDB entry on port %d\n" , port); + +return ret; +} + +static int mxl862xx_port_fdb_del (struct dsa_switch *ds,int port, +const unsigned char *addr,u16 vid,struct dsa_db db) +{ +struct mxl862xx_mac_table_remove param = { }; +struct mxl862xx_priv *priv = ds->priv; +struct mxl862xx_bridge *mxlbridge; +u16 fid; +int ret; + +switch (db.type) { +case DSA_DB_PORT : + fid = priv->ports[db.dp->index].fid; +break ; + +case DSA_DB_BRIDGE :
drivers/net/dsa/mxl862xx/mxl862xx.c static void mxl862xx_phylink_get_caps(struct dsa_switch *ds, int port, · line 679
Quoted 10 lines
drivers/net/dsa/mxl862xx/mxl862xx.c +10
-0+/* Use multi-port bridge FID */ + mxlbridge =mxl862xx_find_bridge (ds, db.bridge); +if (!mxlbridge) +return -ENOENT ; + fid = mxlbridge->bridge_id; +break ; + +default : +return -EOPNOTSUPP ; + }
Please group this together with the FID selection from port_fdb_add() into a single helper.
Quoted 23 lines
drivers/net/dsa/mxl862xx/mxl862xx.c +23
-0+ + param.fid =cpu_to_le16 (fid); + param.tci =cpu_to_le16 (vid &0xFFF ); +memcpy (param.mac, addr,ETH_ALEN ); + + ret =MXL862XX_API_WRITE (priv,MXL862XX_MAC_TABLEENTRYREMOVE , param); +if (ret) +dev_err (ds->dev,"failed to remove FDB entry on port %d\n" , port); + +return ret; +} + +static int mxl862xx_port_fdb_dump (struct dsa_switch *ds,int port, +dsa_fdb_dump_cb_t *cb,void *data) +{ +struct mxl862xx_mac_table_read param = { }; +struct mxl862xx_priv *priv = ds->priv; +u32 entry_port_id; +int ret; + +while (true) { + ret =MXL862XX_API_READ (priv,MXL862XX_MAC_TABLEENTRYREAD , param); +if (ret)
drivers/net/dsa/mxl862xx/mxl862xx.c static void mxl862xx_phylink_get_caps(struct dsa_switch *ds, int port, · line 712
Quoted 10 lines
drivers/net/dsa/mxl862xx/mxl862xx.c +10
-0+return ret; + +if (param.last) +break ; + + entry_port_id =le32_to_cpu (param.port_id); + +if (entry_port_id == port) +cb (param.mac, param.tci &0x0FFF , + param.static_entry, data);
Catch and propagate the error from the FDB dump callback. See commit 21b52fed928e ("net: dsa: sja1105: fix broken backpressure in .port_fdb_dump").
Quoted 73 lines
drivers/net/dsa/mxl862xx/mxl862xx.c +73
-0+ +memset (¶m,0 ,sizeof (param)); + } + +return 0 ; +} + +static int mxl862xx_port_mdb_add (struct dsa_switch *ds,int port, +const struct switchdev_obj_port_mdb *mdb, +struct dsa_db db) +{ +return mxl862xx_port_fdb_add (ds, port, mdb->addr, mdb->vid, db); +} + +static int mxl862xx_port_mdb_del (struct dsa_switch *ds,int port, +const struct switchdev_obj_port_mdb *mdb, +struct dsa_db db) +{ +return mxl862xx_port_fdb_del (ds, port, mdb->addr, mdb->vid, db); +} + +static int mxl862xx_set_ageing_time (struct dsa_switch *ds,unsigned int msecs) +{ +struct mxl862xx_cfg param; +int ret; + + ret =MXL862XX_API_READ (ds->priv,MXL862XX_COMMON_CFGGET , param); +if (ret) { +dev_err (ds->dev,"failed to read switch config\n" ); +return ret; + } + + param.mac_table_age_timer =cpu_to_le32 (MXL862XX_AGETIMER_CUSTOM ); + param.age_timer =cpu_to_le32 (msecs /1000 ); + ret =MXL862XX_API_WRITE (ds->priv,MXL862XX_COMMON_CFGSET , param); +if (ret) +dev_err (ds->dev,"failed to set ageing\n" ); + +return ret; +} + +static void mxl862xx_port_stp_state_set (struct dsa_switch *ds,int port, +u8 state) +{ +struct mxl862xx_stp_port_cfg param = { + .port_id =cpu_to_le16 (port), + }; +struct mxl862xx_priv *priv = ds->priv; +int ret; + +switch (state) { +case BR_STATE_DISABLED : + param.port_state =MXL862XX_STP_PORT_STATE_DISABLE ; +break ; +case BR_STATE_BLOCKING : +case BR_STATE_LISTENING : + param.port_state =MXL862XX_STP_PORT_STATE_BLOCKING ; +break ; +case BR_STATE_LEARNING : + param.port_state =MXL862XX_STP_PORT_STATE_LEARNING ; +break ; +case BR_STATE_FORWARDING : + param.port_state =MXL862XX_STP_PORT_STATE_FORWARD ; +break ; +default : +dev_err (ds->dev,"invalid STP state: %d\n" , state); +return ; + } + + ret =MXL862XX_API_WRITE (priv,MXL862XX_STP_PORTCFGSET , param); +if (ret) { +dev_err (ds->dev,"failed to set STP state on port %d\n" , port); +return ;
drivers/net/dsa/mxl862xx/mxl862xx.c static void mxl862xx_phylink_get_caps(struct dsa_switch *ds, int port, · line 795
Quoted 10 lines
drivers/net/dsa/mxl862xx/mxl862xx.c +10
-0+ } + +/* The firmware may re-enable MAC learning as a side-effect of +* entering LEARNING or FORWARDING state (per 802.1D defaults). +* Re-apply the driver's intended learning and metering config +* so that standalone ports keep learning disabled. +*/ + ret =mxl862xx_set_bridge_port (ds, port); +if (ret) +dev_err (ds->dev,"failed to reapply brport flags on port %d\n" , port);
Do you need to also manually fast-age the port if p->learning == false? Packets may be coming in as soon as you put the port in MXL862XX_STP_PORT_STATE_LEARNING already, and they will end up in the FDB even if from Linux' perspective, it had learning off all along. It is a race condition.
Quoted 1 line
(quoted diff) +1
-0+}
Daniel Golle
maintainerQuoted 15 lines · Daniel Golle
As a next step to complete the mxl862xx DSA driver, add support for offloading forwarding between bridged ports to the switch hardware.
This works pretty much without any big surprises, apart from two subtleties:
* per-port control over flooding behavior has to be implemented by (ab)using a 0-rate QoS meters as stopper in lack of any better option.
* STP state transition unconditionally enables learning on a port even if it was previously explicitely disabled (a firmware bug)Note that as the driver is still lacking all VLAN features (which are going to be added next), at this point some of the
bridge_vlan_aware.shtests are failing after applying this series.
Quoted 7 lines
This is expected and cannot be avoided without implementing
port_vlan_filtering+ port_vlan_add/del. And adding both bridge and VLAN offloading at the same time would be too much for anyone to review.All other relevant selftests (including
bridge_vlan_unaware.sh) are still passing.
I forgot to add that (as expected) also test
VLAN-aware bridge with different PVIDs: Checking which packets were received
now fails in no_forwarding.sh, and will be fixed by implementing VLAN operations in the next series.
Quoted 14 lines
Daniel Golle (2):
dsa:tag_mxl862xx: setdsa_default_offload_fwd_mark()
net: dsa: mxl862xx: implement bridge offloadingdrivers/net/dsa/mxl862xx/mxl862xx-api.h | 171 +++++++ drivers/net/dsa/mxl862xx/mxl862xx-cmd.h | 19 +- drivers/net/dsa/mxl862xx/mxl862xx.c | 576 ++++++++++++++++++++++-- drivers/net/dsa/mxl862xx/mxl862xx.h | 21 + net/dsa/tag_mxl862xx.c | 2 + 5 files changed, 757 insertions(+), 32 deletions(-)Signature · 1 line
2.53.0