This patch series provides support for HSR HW offloading in KSZ9477
switch IC.
To test this feature:
ip link add name hsr0 type hsr slave1 lan1 slave2 lan2 supervision 45 version 1
ip link set dev lan1 up
ip link set dev lan2 up
ip a add 192.168.0.1/24 dev hsr0
ip link set dev hsr0 up
To remove HSR network device:
ip link del hsr0
To test if one can adjust MAC address:
ip link set lan2 address 00:01:02:AA:BB:CC
It is also possible to create another HSR interface, but it will
only support HSR is software - e.g.
ip link add name hsr1 type hsr slave1 lan3 slave2 lan4 supervision 45 version 1
Test HW:
Two KSZ9477-EVB boards with HSR ports set to "Port1" and "Port2".
Performance SW used:
nuttcp -S --nofork
nuttcp -vv -T 60 -r 192.168.0.2
nuttcp -vv -T 60 -t 192.168.0.2
Code: v6.6.0-rc1+ Linux repository
Tested HSR v0 and v1
Results:
With KSZ9477 offloading support added: RX: 100 Mbps TX: 98 Mbps
With no offloading RX: 63 Mbps TX: 63 Mbps
Lukasz Majewski (2):
net: dsa: tag_ksz: Extend ksz9477_xmit() for HSR frame duplication
net: dsa: microchip: Enable HSR offloading for KSZ9477
Vladimir Oltean (3):
net: dsa: propagate extack to ds->ops->port_hsr_join()
net: dsa: notify drivers of MAC address changes on user ports
net: dsa: microchip: move REG_SW_MAC_ADDR to dev->info->regs[]
drivers/net/dsa/microchip/ksz8795_reg.h | 7 --
drivers/net/dsa/microchip/ksz9477.c | 70 +++++++++++
drivers/net/dsa/microchip/ksz9477.h | 2 +
drivers/net/dsa/microchip/ksz9477_reg.h | 7 --
drivers/net/dsa/microchip/ksz_common.c | 149 ++++++++++++++++++++++++
drivers/net/dsa/microchip/ksz_common.h | 10 ++
drivers/net/dsa/xrs700x/xrs700x.c | 18 ++-
include/net/dsa.h | 13 ++-
net/dsa/port.c | 5 +-
net/dsa/port.h | 3 +-
net/dsa/slave.c | 9 +-
net/dsa/tag_ksz.c | 8 ++
12 files changed, 276 insertions(+), 25 deletions(-)
--
2.20.1
From: Vladimir Oltean <[email protected]>
Defining macros which have the same name but different values is bad
practice, because it makes it hard to avoid code duplication. The same
code does different things, depending on the file it's placed in.
Case in point, we want to access REG_SW_MAC_ADDR from ksz_common.c, but
currently we can't, because we don't know which kszXXXX_reg.h to include
from the common code.
Remove the REG_SW_MAC_ADDR_{0..5} macros from ksz8795_reg.h and
ksz9477_reg.h, and re-add this register offset to the dev->info->regs[]
array.
Signed-off-by: Vladimir Oltean <[email protected]>
Signed-off-by: Lukasz Majewski <[email protected]>
---
Changes for v5:
- New patch
---
drivers/net/dsa/microchip/ksz8795_reg.h | 7 -------
drivers/net/dsa/microchip/ksz9477_reg.h | 7 -------
drivers/net/dsa/microchip/ksz_common.c | 2 ++
drivers/net/dsa/microchip/ksz_common.h | 1 +
4 files changed, 3 insertions(+), 14 deletions(-)
diff --git a/drivers/net/dsa/microchip/ksz8795_reg.h b/drivers/net/dsa/microchip/ksz8795_reg.h
index 7a57c6088f80..ee1b673d5f30 100644
--- a/drivers/net/dsa/microchip/ksz8795_reg.h
+++ b/drivers/net/dsa/microchip/ksz8795_reg.h
@@ -323,13 +323,6 @@
((addr) + REG_PORT_1_CTRL_0 + (port) * \
(REG_PORT_2_CTRL_0 - REG_PORT_1_CTRL_0))
-#define REG_SW_MAC_ADDR_0 0x68
-#define REG_SW_MAC_ADDR_1 0x69
-#define REG_SW_MAC_ADDR_2 0x6A
-#define REG_SW_MAC_ADDR_3 0x6B
-#define REG_SW_MAC_ADDR_4 0x6C
-#define REG_SW_MAC_ADDR_5 0x6D
-
#define TABLE_EXT_SELECT_S 5
#define TABLE_EEE_V 1
#define TABLE_ACL_V 2
diff --git a/drivers/net/dsa/microchip/ksz9477_reg.h b/drivers/net/dsa/microchip/ksz9477_reg.h
index cba3dba58bc3..c8866c180fe5 100644
--- a/drivers/net/dsa/microchip/ksz9477_reg.h
+++ b/drivers/net/dsa/microchip/ksz9477_reg.h
@@ -166,13 +166,6 @@
#define SW_DOUBLE_TAG BIT(7)
#define SW_RESET BIT(1)
-#define REG_SW_MAC_ADDR_0 0x0302
-#define REG_SW_MAC_ADDR_1 0x0303
-#define REG_SW_MAC_ADDR_2 0x0304
-#define REG_SW_MAC_ADDR_3 0x0305
-#define REG_SW_MAC_ADDR_4 0x0306
-#define REG_SW_MAC_ADDR_5 0x0307
-
#define REG_SW_MTU__2 0x0308
#define REG_SW_MTU_MASK GENMASK(13, 0)
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 42db7679c360..c67ad0f6e1fa 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -298,6 +298,7 @@ static const struct ksz_dev_ops lan937x_dev_ops = {
};
static const u16 ksz8795_regs[] = {
+ [REG_SW_MAC_ADDR] = 0x68,
[REG_IND_CTRL_0] = 0x6E,
[REG_IND_DATA_8] = 0x70,
[REG_IND_DATA_CHECK] = 0x72,
@@ -426,6 +427,7 @@ static u8 ksz8863_shifts[] = {
};
static const u16 ksz9477_regs[] = {
+ [REG_SW_MAC_ADDR] = 0x0302,
[P_STP_CTRL] = 0x0B04,
[S_START_CTRL] = 0x0300,
[S_BROADCAST_CTRL] = 0x0332,
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index a4de58847dea..1f3fb6c23f36 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -211,6 +211,7 @@ enum ksz_chip_id {
};
enum ksz_regs {
+ REG_SW_MAC_ADDR,
REG_IND_CTRL_0,
REG_IND_DATA_8,
REG_IND_DATA_CHECK,
--
2.20.1
This patch adds functions for providing in KSZ9477 switch HSR
(High-availability Seamless Redundancy) hardware offloading.
According to AN3474 application note following features are provided:
- TX packet duplication from host to switch (NETIF_F_HW_HSR_DUP)
- RX packet duplication discarding
- Prevention of packet loop
For last two ones - there is a probability that some packets will not
be filtered in HW (in some special cases - described in AN3474).
Hence, the HSR core code shall be used to discard those not caught frames.
Moreover, some switch registers adjustments are required - like setting
MAC address of HSR network interface.
Additionally, the KSZ9477 switch has been configured to forward frames
between HSR ports (e.g. 1,2) members to provide support for
NETIF_F_HW_HSR_FWD flag.
Join and leave functions are written in a way, that are executed with
single port - i.e. configuration is NOT done only when second HSR port
is configured.
Co-developed-by: Vladimir Oltean <[email protected]>
Signed-off-by: Vladimir Oltean <[email protected]>
Signed-off-by: Lukasz Majewski <[email protected]>
---
Changes for v2:
- Use struct ksz_device to store hsr ports information (not struct dsa)
Changes for v3:
- Enable in-switch forwarding of frames between HSR ports (i.e. enable
bridging of those two ports)
- The NETIF_F_HW_HSR_FWD flag has been marked as supported by the HSR
network device
- Remove ETH MAC address validity check as it is done earlier in the net
driver
- Add comment regarding adding support for NETIF_F_HW_HSR_FWD flag
Changes for v4:
- Merge patches for ksz_common.c and ksz9477.c
- Remove code to set global self-address filtering as this bit has
already been set at ksz9477_reset_switch() function. Update also
comment.
- Use already available ksz9477_cfg_port_member() instead of ksz_prmw32()
- Add description about chip limitations
- Allow having only one offloaded hsr interface (e.g. hsr0). Other ones
(like hsr1) will have only SW HSR support
- Add check if MAC address of HSR device is equal to one from DSA master
- Rewrite the code to support per port join (i.e. not making init only
when second HSR port is join)
- Add check to allow only one HSR port HW offloading
- Add hsr_ports to ksz_device struct to allow clean removal of network
interfaces composing hsr device
Changes for v5:
- Add implementation of .port_set_mac_address callback for KSZ switch
to prevent MAC address change when HSR HW offloading is used
- Use NL_SET_ERR_MSG_MOD() to propagate error messages to user
- Implement functions to use reference counter to allow in-HW MAC
address change only when no other functionality requires it.
---
drivers/net/dsa/microchip/ksz9477.c | 70 ++++++++++++
drivers/net/dsa/microchip/ksz9477.h | 2 +
drivers/net/dsa/microchip/ksz_common.c | 147 +++++++++++++++++++++++++
drivers/net/dsa/microchip/ksz_common.h | 9 ++
4 files changed, 228 insertions(+)
diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index 83b7f2d5c1ea..c4da8dc7638e 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -1141,6 +1141,76 @@ int ksz9477_tc_cbs_set_cinc(struct ksz_device *dev, int port, u32 val)
return ksz_pwrite16(dev, port, REG_PORT_MTI_CREDIT_INCREMENT, val);
}
+/* The KSZ9477 provides following HW features to accelerate
+ * HSR frames handling:
+ *
+ * 1. TX PACKET DUPLICATION FROM HOST TO SWITCH
+ * 2. RX PACKET DUPLICATION DISCARDING
+ * 3. PREVENTING PACKET LOOP IN THE RING BY SELF-ADDRESS FILTERING
+ *
+ * Only one from point 1. has the NETIF_F* flag available.
+ *
+ * Ones from point 2 and 3 are "best effort" - i.e. those will
+ * work correctly most of the time, but it may happen that some
+ * frames will not be caught - to be more specific; there is a race
+ * condition in hardware such that, when duplicate packets are received
+ * on member ports very close in time to each other, the hardware fails
+ * to detect that they are duplicates.
+ *
+ * Hence, the SW needs to handle those special cases. However, the speed
+ * up gain is considerable when above features are used.
+ *
+ * Moreover, the NETIF_F_HW_HSR_FWD feature is also enabled, as HSR frames
+ * can be forwarded in the switch fabric between HSR ports.
+ */
+#define KSZ9477_SUPPORTED_HSR_FEATURES (NETIF_F_HW_HSR_DUP | NETIF_F_HW_HSR_FWD)
+
+void ksz9477_hsr_join(struct dsa_switch *ds, int port, struct net_device *hsr)
+{
+ struct ksz_device *dev = ds->priv;
+ struct net_device *slave;
+ u8 data;
+
+ /* Program which port(s) shall support HSR */
+ ksz_rmw32(dev, REG_HSR_PORT_MAP__4, BIT(port), BIT(port));
+
+ /* Forward frames between HSR ports (i.e. bridge together HSR ports) */
+ ksz9477_cfg_port_member(dev, port,
+ BIT(dsa_upstream_port(ds, port)) | BIT(port));
+
+ if (!dev->hsr_ports) {
+ /* Enable discarding of received HSR frames */
+ ksz_read8(dev, REG_HSR_ALU_CTRL_0__1, &data);
+ data |= HSR_DUPLICATE_DISCARD;
+ data &= ~HSR_NODE_UNICAST;
+ ksz_write8(dev, REG_HSR_ALU_CTRL_0__1, data);
+ }
+
+ /* Enable per port self-address filtering.
+ * The global self-address filtering has already been enabled in the
+ * ksz9477_reset_switch() function.
+ */
+ ksz_port_cfg(dev, port, REG_PORT_LUE_CTRL, PORT_SRC_ADDR_FILTER, true);
+
+ /* Setup HW supported features for lan HSR ports */
+ slave = dsa_to_port(ds, port)->slave;
+ slave->features |= KSZ9477_SUPPORTED_HSR_FEATURES;
+}
+
+void ksz9477_hsr_leave(struct dsa_switch *ds, int port, struct net_device *hsr)
+{
+ struct ksz_device *dev = ds->priv;
+
+ /* Clear port HSR support */
+ ksz_rmw32(dev, REG_HSR_PORT_MAP__4, BIT(port), 0);
+
+ /* Disable forwarding frames between HSR ports */
+ ksz9477_cfg_port_member(dev, port, BIT(dsa_upstream_port(ds, port)));
+
+ /* Disable per port self-address filtering */
+ ksz_port_cfg(dev, port, REG_PORT_LUE_CTRL, PORT_SRC_ADDR_FILTER, false);
+}
+
int ksz9477_switch_init(struct ksz_device *dev)
{
u8 data8;
diff --git a/drivers/net/dsa/microchip/ksz9477.h b/drivers/net/dsa/microchip/ksz9477.h
index a6f425866a29..8625bf474764 100644
--- a/drivers/net/dsa/microchip/ksz9477.h
+++ b/drivers/net/dsa/microchip/ksz9477.h
@@ -56,5 +56,7 @@ int ksz9477_reset_switch(struct ksz_device *dev);
int ksz9477_switch_init(struct ksz_device *dev);
void ksz9477_switch_exit(struct ksz_device *dev);
void ksz9477_port_queue_split(struct ksz_device *dev, int port);
+void ksz9477_hsr_join(struct dsa_switch *ds, int port, struct net_device *hsr);
+void ksz9477_hsr_leave(struct dsa_switch *ds, int port, struct net_device *hsr);
#endif
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index c67ad0f6e1fa..0a8d99ae1132 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -16,6 +16,7 @@
#include <linux/etherdevice.h>
#include <linux/if_bridge.h>
#include <linux/if_vlan.h>
+#include <linux/if_hsr.h>
#include <linux/irq.h>
#include <linux/irqdomain.h>
#include <linux/of.h>
@@ -3421,6 +3422,149 @@ static int ksz_setup_tc(struct dsa_switch *ds, int port,
}
}
+static int ksz_port_set_mac_address(struct dsa_switch *ds, int port,
+ const unsigned char *addr)
+{
+ struct dsa_port *dp = dsa_to_port(ds, port);
+
+ if (dp->hsr_dev) {
+ dev_err(ds->dev,
+ "Cannot change MAC address on port %d with active HSR offload\n",
+ port);
+ return -EBUSY;
+ }
+
+ return 0;
+}
+
+/* Program the switch's MAC address register with the MAC address of the
+ * requesting user port. This single address is used by the switch for multiple
+ * features, like HSR self-address filtering and WoL. Other user ports are
+ * allowed to share ownership of this address as long as their MAC address is
+ * the same. The user ports' MAC addresses must not change while they have
+ * ownership of the switch MAC address.
+ */
+static int ksz_switch_macaddr_get(struct dsa_switch *ds, int port,
+ struct netlink_ext_ack *extack)
+{
+ struct net_device *slave = dsa_to_port(ds, port)->slave;
+ const unsigned char *addr = slave->dev_addr;
+ struct ksz_switch_macaddr *switch_macaddr;
+ struct ksz_device *dev = ds->priv;
+ const u16 *regs = dev->info->regs;
+ int i;
+
+ /* Make sure concurrent MAC address changes are blocked */
+ ASSERT_RTNL();
+
+ switch_macaddr = dev->switch_macaddr;
+ if (switch_macaddr) {
+ if (!ether_addr_equal(switch_macaddr->addr, addr)) {
+ NL_SET_ERR_MSG_FMT_MOD(extack,
+ "Switch already configured for MAC address %pM",
+ switch_macaddr->addr);
+ return -EBUSY;
+ }
+
+ refcount_inc(&switch_macaddr->refcount);
+ return 0;
+ }
+
+ switch_macaddr = kzalloc(sizeof(*switch_macaddr), GFP_KERNEL);
+ if (!switch_macaddr)
+ return -ENOMEM;
+
+ ether_addr_copy(switch_macaddr->addr, addr);
+ refcount_set(&switch_macaddr->refcount, 1);
+ dev->switch_macaddr = switch_macaddr;
+
+ /* Program the switch MAC address to hardware */
+ for (i = 0; i < ETH_ALEN; i++)
+ ksz_write8(dev, regs[REG_SW_MAC_ADDR] + i, addr[i]);
+
+ return 0;
+}
+
+static void ksz_switch_macaddr_put(struct dsa_switch *ds)
+{
+ struct ksz_switch_macaddr *switch_macaddr;
+ struct ksz_device *dev = ds->priv;
+ const u16 *regs = dev->info->regs;
+ int i;
+
+ /* Make sure concurrent MAC address changes are blocked */
+ ASSERT_RTNL();
+
+ switch_macaddr = dev->switch_macaddr;
+ if (!refcount_dec_and_test(&switch_macaddr->refcount))
+ return;
+
+ for (i = 0; i < ETH_ALEN; i++)
+ ksz_write8(dev, regs[REG_SW_MAC_ADDR] + i, 0);
+
+ dev->switch_macaddr = NULL;
+ kfree(switch_macaddr);
+}
+
+static int ksz_hsr_join(struct dsa_switch *ds, int port, struct net_device *hsr,
+ struct netlink_ext_ack *extack)
+{
+ struct ksz_device *dev = ds->priv;
+ enum hsr_version ver;
+ int ret;
+
+ ret = hsr_get_version(hsr, &ver);
+ if (ret)
+ return ret;
+
+ if (dev->chip_id != KSZ9477_CHIP_ID) {
+ NL_SET_ERR_MSG_MOD(extack, "Chip does not support HSR offload");
+ return -EOPNOTSUPP;
+ }
+
+ /* KSZ9477 can support HW offloading of only 1 HSR device */
+ if (dev->hsr_dev && hsr != dev->hsr_dev) {
+ NL_SET_ERR_MSG_MOD(extack, "Offload supported for a single HSR");
+ return -EOPNOTSUPP;
+ }
+
+ /* KSZ9477 only supports HSR v0 and v1 */
+ if (!(ver == HSR_V0 || ver == HSR_V1)) {
+ NL_SET_ERR_MSG_MOD(extack, "Only HSR v0 and v1 supported");
+ return -EOPNOTSUPP;
+ }
+
+ /* Self MAC address filtering, to avoid frames traversing
+ * the HSR ring more than once.
+ */
+ ret = ksz_switch_macaddr_get(ds, port, extack);
+ if (ret)
+ return ret;
+
+ ksz9477_hsr_join(ds, port, hsr);
+ dev->hsr_dev = hsr;
+ dev->hsr_ports |= BIT(port);
+
+ return 0;
+}
+
+static int ksz_hsr_leave(struct dsa_switch *ds, int port,
+ struct net_device *hsr)
+{
+ struct ksz_device *dev = ds->priv;
+
+ WARN_ON(dev->chip_id != KSZ9477_CHIP_ID);
+
+ ksz9477_hsr_leave(ds, port, hsr);
+ dev->hsr_ports &= ~BIT(port);
+ if (!dev->hsr_ports)
+ dev->hsr_dev = NULL;
+
+ ksz_switch_macaddr_put(ds);
+
+ return 0;
+}
+
static const struct dsa_switch_ops ksz_switch_ops = {
.get_tag_protocol = ksz_get_tag_protocol,
.connect_tag_protocol = ksz_connect_tag_protocol,
@@ -3440,6 +3584,9 @@ static const struct dsa_switch_ops ksz_switch_ops = {
.get_sset_count = ksz_sset_count,
.port_bridge_join = ksz_port_bridge_join,
.port_bridge_leave = ksz_port_bridge_leave,
+ .port_hsr_join = ksz_hsr_join,
+ .port_hsr_leave = ksz_hsr_leave,
+ .port_set_mac_address = ksz_port_set_mac_address,
.port_stp_state_set = ksz_port_stp_state_set,
.port_pre_bridge_flags = ksz_port_pre_bridge_flags,
.port_bridge_flags = ksz_port_bridge_flags,
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index 1f3fb6c23f36..1f447a34f555 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -101,6 +101,11 @@ struct ksz_ptp_irq {
int num;
};
+struct ksz_switch_macaddr {
+ unsigned char addr[ETH_ALEN];
+ refcount_t refcount;
+};
+
struct ksz_port {
bool remove_tag; /* Remove Tag flag set, for ksz8795 only */
bool learning;
@@ -169,6 +174,10 @@ struct ksz_device {
struct mutex lock_irq; /* IRQ Access */
struct ksz_irq girq;
struct ksz_ptp_data ptp_data;
+
+ struct ksz_switch_macaddr *switch_macaddr;
+ struct net_device *hsr_dev; /* HSR */
+ u8 hsr_ports;
};
/* List of supported models */
--
2.20.1
The KSZ9477 has support for HSR (High-Availability Seamless Redundancy).
One of its offloading (i.e. performed in the switch IC hardware) features
is to duplicate received frame to both HSR aware switch ports.
To achieve this goal - the tail TAG needs to be modified. To be more
specific, both ports must be marked as destination (egress) ones.
The NETIF_F_HW_HSR_DUP flag indicates that the device supports HSR and
assures (in HSR core code) that frame is sent only once from HOST to
switch with tail tag indicating both ports.
Signed-off-by: Lukasz Majewski <[email protected]>
---
Changes for v2:
- Use ksz_hsr_get_ports() to obtain the bits values corresponding to
HSR aware ports
Changes for v3:
- None
Changes for v4:
- Iterate over switch ports to find ones supporting HSR. Comparing to v3,
where caching of egress tag bits were used, no noticeable performance
regression has been observed.
Changes for v5:
- None
---
net/dsa/tag_ksz.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/net/dsa/tag_ksz.c b/net/dsa/tag_ksz.c
index ea100bd25939..3632e47dea9e 100644
--- a/net/dsa/tag_ksz.c
+++ b/net/dsa/tag_ksz.c
@@ -293,6 +293,14 @@ static struct sk_buff *ksz9477_xmit(struct sk_buff *skb,
if (is_link_local_ether_addr(hdr->h_dest))
val |= KSZ9477_TAIL_TAG_OVERRIDE;
+ if (dev->features & NETIF_F_HW_HSR_DUP) {
+ struct net_device *hsr_dev = dp->hsr_dev;
+ struct dsa_port *other_dp;
+
+ dsa_hsr_foreach_port(other_dp, dp->ds, hsr_dev)
+ val |= BIT(other_dp->index);
+ }
+
*tag = cpu_to_be16(val);
return ksz_defer_xmit(dp, skb);
--
2.20.1
On Wed, Sep 20, 2023 at 01:43:43PM +0200, Lukasz Majewski wrote:
> +void ksz9477_hsr_join(struct dsa_switch *ds, int port, struct net_device *hsr)
> +{
> + struct ksz_device *dev = ds->priv;
> + struct net_device *slave;
> + u8 data;
> +
> + /* Program which port(s) shall support HSR */
> + ksz_rmw32(dev, REG_HSR_PORT_MAP__4, BIT(port), BIT(port));
> +
> + /* Forward frames between HSR ports (i.e. bridge together HSR ports) */
> + ksz9477_cfg_port_member(dev, port,
> + BIT(dsa_upstream_port(ds, port)) | BIT(port));
Isn't this supposed to be
ksz9477_cfg_port_member(dev, port,
BIT(dsa_upstream_port(ds, port)) | BIT(pair));
where "pair" is not even passed as an argument to ksz9477_hsr_join(),
but represents the *other* port in the HSR ring, not this one?
> +
> + if (!dev->hsr_ports) {
> + /* Enable discarding of received HSR frames */
> + ksz_read8(dev, REG_HSR_ALU_CTRL_0__1, &data);
> + data |= HSR_DUPLICATE_DISCARD;
> + data &= ~HSR_NODE_UNICAST;
> + ksz_write8(dev, REG_HSR_ALU_CTRL_0__1, data);
> + }
> +
> + /* Enable per port self-address filtering.
> + * The global self-address filtering has already been enabled in the
> + * ksz9477_reset_switch() function.
> + */
> + ksz_port_cfg(dev, port, REG_PORT_LUE_CTRL, PORT_SRC_ADDR_FILTER, true);
> +
> + /* Setup HW supported features for lan HSR ports */
> + slave = dsa_to_port(ds, port)->slave;
> + slave->features |= KSZ9477_SUPPORTED_HSR_FEATURES;
> +}
> diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
> index 1f3fb6c23f36..1f447a34f555 100644
> --- a/drivers/net/dsa/microchip/ksz_common.h
> +++ b/drivers/net/dsa/microchip/ksz_common.h
> @@ -101,6 +101,11 @@ struct ksz_ptp_irq {
> int num;
> };
>
> +struct ksz_switch_macaddr {
> + unsigned char addr[ETH_ALEN];
> + refcount_t refcount;
> +};
> +
> struct ksz_port {
> bool remove_tag; /* Remove Tag flag set, for ksz8795 only */
> bool learning;
> @@ -169,6 +174,10 @@ struct ksz_device {
> struct mutex lock_irq; /* IRQ Access */
> struct ksz_irq girq;
> struct ksz_ptp_data ptp_data;
> +
> + struct ksz_switch_macaddr *switch_macaddr;
> + struct net_device *hsr_dev; /* HSR */
Please be consistent with the lines above, and use tabs to align the "/* HSR */"
comment.
> + u8 hsr_ports;
> };
>
> /* List of supported models */
> --
> 2.20.1
>
On 9/20/23 04:43, Lukasz Majewski wrote:
> The KSZ9477 has support for HSR (High-Availability Seamless Redundancy).
> One of its offloading (i.e. performed in the switch IC hardware) features
> is to duplicate received frame to both HSR aware switch ports.
>
> To achieve this goal - the tail TAG needs to be modified. To be more
> specific, both ports must be marked as destination (egress) ones.
>
> The NETIF_F_HW_HSR_DUP flag indicates that the device supports HSR and
> assures (in HSR core code) that frame is sent only once from HOST to
> switch with tail tag indicating both ports.
>
> Signed-off-by: Lukasz Majewski <[email protected]>
Reviewed-by: Florian Fainelli <[email protected]>
--
Florian
On Wed, Sep 20, 2023 at 01:43:41PM +0200, Lukasz Majewski wrote:
> The KSZ9477 has support for HSR (High-Availability Seamless Redundancy).
> One of its offloading (i.e. performed in the switch IC hardware) features
> is to duplicate received frame to both HSR aware switch ports.
>
> To achieve this goal - the tail TAG needs to be modified. To be more
> specific, both ports must be marked as destination (egress) ones.
>
> The NETIF_F_HW_HSR_DUP flag indicates that the device supports HSR and
> assures (in HSR core code) that frame is sent only once from HOST to
> switch with tail tag indicating both ports.
>
> Signed-off-by: Lukasz Majewski <[email protected]>
>
> ---
I'm not sure what in your process causes this, but there is an atypical
extra new line between the last Signed-off-by: line and ---. Anyway.
Reviewed-by: Vladimir Oltean <[email protected]>
On 9/20/23 04:43, Lukasz Majewski wrote:
> From: Vladimir Oltean <[email protected]>
>
> Defining macros which have the same name but different values is bad
> practice, because it makes it hard to avoid code duplication. The same
> code does different things, depending on the file it's placed in.
> Case in point, we want to access REG_SW_MAC_ADDR from ksz_common.c, but
> currently we can't, because we don't know which kszXXXX_reg.h to include
> from the common code.
>
> Remove the REG_SW_MAC_ADDR_{0..5} macros from ksz8795_reg.h and
> ksz9477_reg.h, and re-add this register offset to the dev->info->regs[]
> array.
>
> Signed-off-by: Vladimir Oltean <[email protected]>
> Signed-off-by: Lukasz Majewski <[email protected]>
Reviewed-by: Florian Fainelli <[email protected]>
--
Florian
Hi Lukasz,
On Wed, Sep 20, 2023 at 01:43:38PM +0200, Lukasz Majewski wrote:
> Code: v6.6.0-rc1+ Linux repository
Your patches conflict with Oleksij's ACL patches, merged on the 14th of September.
https://patchwork.kernel.org/project/netdevbpf/patch/[email protected]/
Please always submit patches formatted on the most recent tip of the
"main" branch of https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git
at the time of submission, not "v6.6.0-rc1+ Linux repository". There is
nothing that will be done with patches formatted on older trees.
https://patchwork.kernel.org/project/netdevbpf/patch/[email protected]/
netdev/apply fail Patch does not apply to net-next
On 9/20/23 04:43, Lukasz Majewski wrote:
> This patch adds functions for providing in KSZ9477 switch HSR
> (High-availability Seamless Redundancy) hardware offloading.
>
> According to AN3474 application note following features are provided:
> - TX packet duplication from host to switch (NETIF_F_HW_HSR_DUP)
> - RX packet duplication discarding
> - Prevention of packet loop
>
> For last two ones - there is a probability that some packets will not
> be filtered in HW (in some special cases - described in AN3474).
> Hence, the HSR core code shall be used to discard those not caught frames.
>
> Moreover, some switch registers adjustments are required - like setting
> MAC address of HSR network interface.
>
> Additionally, the KSZ9477 switch has been configured to forward frames
> between HSR ports (e.g. 1,2) members to provide support for
> NETIF_F_HW_HSR_FWD flag.
>
> Join and leave functions are written in a way, that are executed with
> single port - i.e. configuration is NOT done only when second HSR port
> is configured.
>
> Co-developed-by: Vladimir Oltean <[email protected]>
> Signed-off-by: Vladimir Oltean <[email protected]>
> Signed-off-by: Lukasz Majewski <[email protected]>
Reviewed-by: Florian Fainelli <[email protected]>
--
Florian
Hi Vladimir,
> On Wed, Sep 20, 2023 at 01:43:43PM +0200, Lukasz Majewski wrote:
> > +void ksz9477_hsr_join(struct dsa_switch *ds, int port, struct
> > net_device *hsr) +{
> > + struct ksz_device *dev = ds->priv;
> > + struct net_device *slave;
> > + u8 data;
> > +
> > + /* Program which port(s) shall support HSR */
> > + ksz_rmw32(dev, REG_HSR_PORT_MAP__4, BIT(port), BIT(port));
> > +
> > + /* Forward frames between HSR ports (i.e. bridge together
> > HSR ports) */
> > + ksz9477_cfg_port_member(dev, port,
> > + BIT(dsa_upstream_port(ds, port)) |
> > BIT(port));
>
> Isn't this supposed to be
>
> ksz9477_cfg_port_member(dev, port,
> BIT(dsa_upstream_port(ds, port)) |
> BIT(pair));
>
> where "pair" is not even passed as an argument to ksz9477_hsr_join(),
> but represents the *other* port in the HSR ring, not this one?
Unfortunately, yes...
The code as it is now -> would set for port lan1 0x21 and lan2 0x22.
However the setup shall be 0x23 for both ports.
More info here:
https://github.com/Microchip-Ethernet/EVB-KSZ9477/issues/98#issuecomment-1701557449
I will setup this register from dev->hsr_ports when both HSR ports are
known.
>
> > +
> > + if (!dev->hsr_ports) {
> > + /* Enable discarding of received HSR frames */
> > + ksz_read8(dev, REG_HSR_ALU_CTRL_0__1, &data);
> > + data |= HSR_DUPLICATE_DISCARD;
> > + data &= ~HSR_NODE_UNICAST;
> > + ksz_write8(dev, REG_HSR_ALU_CTRL_0__1, data);
> > + }
> > +
> > + /* Enable per port self-address filtering.
> > + * The global self-address filtering has already been
> > enabled in the
> > + * ksz9477_reset_switch() function.
> > + */
> > + ksz_port_cfg(dev, port, REG_PORT_LUE_CTRL,
> > PORT_SRC_ADDR_FILTER, true); +
> > + /* Setup HW supported features for lan HSR ports */
> > + slave = dsa_to_port(ds, port)->slave;
> > + slave->features |= KSZ9477_SUPPORTED_HSR_FEATURES;
> > +}
> > diff --git a/drivers/net/dsa/microchip/ksz_common.h
> > b/drivers/net/dsa/microchip/ksz_common.h index
> > 1f3fb6c23f36..1f447a34f555 100644 ---
> > a/drivers/net/dsa/microchip/ksz_common.h +++
> > b/drivers/net/dsa/microchip/ksz_common.h @@ -101,6 +101,11 @@
> > struct ksz_ptp_irq { int num;
> > };
> >
> > +struct ksz_switch_macaddr {
> > + unsigned char addr[ETH_ALEN];
> > + refcount_t refcount;
> > +};
> > +
> > struct ksz_port {
> > bool remove_tag; /* Remove Tag flag set,
> > for ksz8795 only */ bool learning;
> > @@ -169,6 +174,10 @@ struct ksz_device {
> > struct mutex lock_irq; /* IRQ Access */
> > struct ksz_irq girq;
> > struct ksz_ptp_data ptp_data;
> > +
> > + struct ksz_switch_macaddr *switch_macaddr;
> > + struct net_device *hsr_dev; /* HSR */
>
> Please be consistent with the lines above, and use tabs to align the
> "/* HSR */" comment.
>
> > + u8 hsr_ports;
> > };
> >
> > /* List of supported models */
> > --
> > 2.20.1
> >
>
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: [email protected]
Hi Vladimir,
> On Fri, Sep 22, 2023 at 01:18:38PM +0200, Lukasz Majewski wrote:
> > By mistake my net-next repo was pointing to:
> > git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net
> >
> > Please correct me if I'm wrong but it looks like the net repo for
> > current mainline fixes...
>
> Yes, net.git is for fixes to the current mainline branch, and net-next
> is for new features to be included in mainline during the next merge
> window. They are the same at the beginning of the development cycle
> and then they start to diverge.
>
> > However, after fetching net-next - I can apply v5 without issues on
> > top of it.
> >
> > SHA1: 5a1b322cb0b7d0d33a2d13462294dc0f46911172
> > "Merge branch 'mlxsw-multicast'"
> >
> > https://source.denx.de/linux/linux-ksz9477/-/commits/net-next-ksz-HSR-devel-v5?ref_type=heads
> > Linux version from `uname -a`: 6.6.0-rc2+
> >
> > However, it looks like I would need to prepare v6 anyway...
>
> I don't know. "git rebase" is a bit smarter than "git am" and can
> automatically resolve some conflicts, on which "git am" will simply
> bail out if even the context is not identical. Either way, both
> patchwork and me failed to apply your v5 series on net-next, and the
> patches won't be accepted without build testing.
Ok. I will test them with git am -3
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: [email protected]
On Fri, Sep 22, 2023 at 01:29:04PM +0200, Lukasz Majewski wrote:
> Unfortunately, yes...
>
> The code as it is now -> would set for port lan1 0x21 and lan2 0x22.
>
> However the setup shall be 0x23 for both ports.
>
> More info here:
> https://github.com/Microchip-Ethernet/EVB-KSZ9477/issues/98#issuecomment-1701557449
>
> I will setup this register from dev->hsr_ports when both HSR ports are
> known.
Testing after making changes is key.
On Fri, Sep 22, 2023 at 01:18:38PM +0200, Lukasz Majewski wrote:
> By mistake my net-next repo was pointing to:
> git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net
>
> Please correct me if I'm wrong but it looks like the net repo for
> current mainline fixes...
Yes, net.git is for fixes to the current mainline branch, and net-next
is for new features to be included in mainline during the next merge window.
They are the same at the beginning of the development cycle and then
they start to diverge.
> However, after fetching net-next - I can apply v5 without issues on top
> of it.
>
> SHA1: 5a1b322cb0b7d0d33a2d13462294dc0f46911172
> "Merge branch 'mlxsw-multicast'"
>
> https://source.denx.de/linux/linux-ksz9477/-/commits/net-next-ksz-HSR-devel-v5?ref_type=heads
> Linux version from `uname -a`: 6.6.0-rc2+
>
> However, it looks like I would need to prepare v6 anyway...
I don't know. "git rebase" is a bit smarter than "git am" and can
automatically resolve some conflicts, on which "git am" will simply bail
out if even the context is not identical. Either way, both patchwork and
me failed to apply your v5 series on net-next, and the patches won't be
accepted without build testing.
Hi Vladimir,
> Hi Lukasz,
>
> On Wed, Sep 20, 2023 at 01:43:38PM +0200, Lukasz Majewski wrote:
> > Code: v6.6.0-rc1+ Linux repository
>
> Your patches conflict with Oleksij's ACL patches, merged on the 14th
> of September.
> https://patchwork.kernel.org/project/netdevbpf/patch/[email protected]/
>
> Please always submit patches formatted on the most recent tip of the
> "main" branch of
> https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git
By mistake my net-next repo was pointing to:
git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net
Please correct me if I'm wrong but it looks like the net repo for
current mainline fixes...
> at the time of submission, not "v6.6.0-rc1+ Linux repository". There
> is nothing that will be done with patches formatted on older trees.
>
> https://patchwork.kernel.org/project/netdevbpf/patch/[email protected]/
> netdev/apply fail Patch does not apply to net-next
However, after fetching net-next - I can apply v5 without issues on top
of it.
SHA1: 5a1b322cb0b7d0d33a2d13462294dc0f46911172
"Merge branch 'mlxsw-multicast'"
https://source.denx.de/linux/linux-ksz9477/-/commits/net-next-ksz-HSR-devel-v5?ref_type=heads
Linux version from `uname -a`: 6.6.0-rc2+
However, it looks like I would need to prepare v6 anyway...
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: [email protected]