2020-04-20 15:20:30

by Horatiu Vultur

[permalink] [raw]
Subject: [PATCH net-next 00/13] net: bridge: mrp: Add support for Media Redundancy Protocol(MRP)

Media Redundancy Protocol is a data network protocol standardized by
International Electrotechnical Commission as IEC 62439-2. It allows rings of
Ethernet switches to overcome any single failure with recovery time faster than
STP. It is primarily used in Industrial Ethernet applications.

Based on the previous RFC[1][2][3][4][5], the MRP state machine and all the
timers were moved to userspace, except for the timers used to generate MRP Test
frames. In this way the userspace doesn't know and should not know if the HW or
the kernel will generate the MRP Test frames. The following changes were added
to the bridge to support the MRP:
- the existing netlink interface was extended with MRP support,
- allow to detect when a MRP frame was received on a MRP ring port
- allow MRP instance to forward/terminate MRP frames
- generate MRP Test frames in case the HW doesn't have support for this

To be able to offload MRP support to HW, the switchdev API was extend.

With these changes the userspace doesn't do the following because already the
kernel/HW will do:
- doesn't need to forward/terminate MRP frames
- doesn't need to generate MRP Test frames
- doesn't need to detect when the ring is open/closed.

The userspace application that is using the new netlink can be found here[6].

The current implementation both in kernel and userspace supports only 2 roles:
MRM - this one is responsible to send MRP_Test and MRP_Topo frames on both
ring ports. It needs to process MRP_Test to know if the ring is open or
closed. This operation is desired to be offloaded to the HW because it
requires to generate and process up to 4000 frames per second. Whenever it
detects that the ring is open it sends MRP_Topo frames to notify all MRC about
changes in the topology. MRM needs also to process MRP_LinkChange frames,
these frames are generated by the MRC. When the ring is open then the state
of both ports is to forward frames and when the ring is closed then the
secondary port is blocked.

MRC - this one is responsible to forward MRP frames between the ring ports.
In case one of the ring ports gets a link down or up, then MRC will generate
a MRP_LinkChange frames. This node should also process MRP_Topo frames and to
clear its FDB when it receives this frame.

Userspace
Deamon +----------+ Client
+
|
+--------------|-----------------------------------------+
Kernel |
+ Netlink

| + Interrupt
| |
+--------------|------------------------------|----------+
HW | Switchdev |
+ |

The user interacts using the client (called 'mrp'), the client talks to the
deamon (called 'mrp_server'), which talks with the kernel using netlink. The
kernel will try to offload the requests to the HW via switchdev API.

If this will be accepted then in the future the netlink interface can be
expended with multiple attributes which are required by different roles of the
MRP. Like Media Redundancy Automanager(MRA), Media Interconnect Manager(MIM) and
Media Interconnect Client(MIC).

[1] https://www.spinics.net/lists/netdev/msg623647.html
[2] https://www.spinics.net/lists/netdev/msg624378.html
[3] https://www.spinics.net/lists/netdev/msg627500.html
[4] https://www.spinics.net/lists/netdev/msg641005.html
[5] https://www.spinics.net/lists/netdev/msg643991.html
[6] https://github.com/microchip-ung/mrp/tree/patch-v6

-v1:
- fix bisectability issues
- in case of errors use extack

-RFC v5:
- use nla_parse_nested
- rework the usage of the rcu in br_mrp
- reorder patches
- few other small issues raised by Nikolay

-RFC v4:
- extend existing netlink interface to add mrp support
- use rcu locks

-RFC v3:
- move MRP state machine in userspace
- create generic netlink interface for configuring the HW using switchdev API

-RFC v2:
- extend switchdev API to offload to HW

Horatiu Vultur (13):
bridge: uapi: mrp: Add mrp attributes.
net: bridge: Add port attribute IFLA_BRPORT_MRP_RING_OPEN
bridge: mrp: Update Kconfig
bridge: mrp: Expose function br_mrp_port_open
bridge: mrp: Add MRP interface.
bridge: mrp: Extend bridge interface
switchdev: mrp: Extend switchdev API to offload MRP
bridge: switchdev: mrp: Implement MRP API for switchdev
bridge: mrp: Connect MRP API with the switchev API
bridge: mrp: Implement netlink interface to configure MRP
bridge: mrp: Update Makefile
bridge: mrp: Integrate MRP into the bridge
net: bridge: Add checks for enabling the STP.

include/linux/if_bridge.h | 1 +
include/linux/mrp_bridge.h | 27 ++
include/net/switchdev.h | 62 ++++
include/uapi/linux/if_bridge.h | 42 +++
include/uapi/linux/if_ether.h | 1 +
include/uapi/linux/if_link.h | 1 +
include/uapi/linux/mrp_bridge.h | 84 +++++
net/bridge/Kconfig | 12 +
net/bridge/Makefile | 2 +
net/bridge/br_device.c | 3 +
net/bridge/br_if.c | 2 +
net/bridge/br_input.c | 3 +
net/bridge/br_ioctl.c | 3 +-
net/bridge/br_mrp.c | 551 +++++++++++++++++++++++++++++
net/bridge/br_mrp_netlink.c | 117 ++++++
net/bridge/br_mrp_switchdev.c | 141 ++++++++
net/bridge/br_netlink.c | 15 +-
net/bridge/br_private.h | 42 ++-
net/bridge/br_private_mrp.h | 60 ++++
net/bridge/br_stp.c | 6 +
net/bridge/br_stp_if.c | 11 +-
net/bridge/br_sysfs_br.c | 4 +-
tools/include/uapi/linux/if_link.h | 1 +
23 files changed, 1183 insertions(+), 8 deletions(-)
create mode 100644 include/linux/mrp_bridge.h
create mode 100644 include/uapi/linux/mrp_bridge.h
create mode 100644 net/bridge/br_mrp.c
create mode 100644 net/bridge/br_mrp_netlink.c
create mode 100644 net/bridge/br_mrp_switchdev.c
create mode 100644 net/bridge/br_private_mrp.h

--
2.17.1


2020-04-20 15:21:01

by Horatiu Vultur

[permalink] [raw]
Subject: [PATCH net-next 10/13] bridge: mrp: Implement netlink interface to configure MRP

Implement netlink interface to configure MRP. The implementation
will do sanity checks over the attributes and then eventually call the MRP
interface.

Signed-off-by: Horatiu Vultur <[email protected]>
---
net/bridge/br_mrp_netlink.c | 117 ++++++++++++++++++++++++++++++++++++
1 file changed, 117 insertions(+)
create mode 100644 net/bridge/br_mrp_netlink.c

diff --git a/net/bridge/br_mrp_netlink.c b/net/bridge/br_mrp_netlink.c
new file mode 100644
index 000000000000..0ff42e7c7f57
--- /dev/null
+++ b/net/bridge/br_mrp_netlink.c
@@ -0,0 +1,117 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include <net/genetlink.h>
+
+#include <uapi/linux/mrp_bridge.h>
+#include "br_private.h"
+#include "br_private_mrp.h"
+
+static const struct nla_policy br_mrp_policy[IFLA_BRIDGE_MRP_MAX + 1] = {
+ [IFLA_BRIDGE_MRP_UNSPEC] = { .type = NLA_REJECT },
+ [IFLA_BRIDGE_MRP_INSTANCE] = { .type = NLA_EXACT_LEN,
+ .len = sizeof(struct br_mrp_instance)},
+ [IFLA_BRIDGE_MRP_PORT_STATE] = { .type = NLA_U32 },
+ [IFLA_BRIDGE_MRP_PORT_ROLE] = { .type = NLA_EXACT_LEN,
+ .len = sizeof(struct br_mrp_port_role)},
+ [IFLA_BRIDGE_MRP_RING_STATE] = { .type = NLA_EXACT_LEN,
+ .len = sizeof(struct br_mrp_ring_state)},
+ [IFLA_BRIDGE_MRP_RING_ROLE] = { .type = NLA_EXACT_LEN,
+ .len = sizeof(struct br_mrp_ring_role)},
+ [IFLA_BRIDGE_MRP_START_TEST] = { .type = NLA_EXACT_LEN,
+ .len = sizeof(struct br_mrp_start_test)},
+};
+
+int br_mrp_parse(struct net_bridge *br, struct net_bridge_port *p,
+ struct nlattr *attr, int cmd, struct netlink_ext_ack *extack)
+{
+ struct nlattr *tb[IFLA_BRIDGE_MRP_MAX + 1];
+ int err;
+
+ if (br->stp_enabled != BR_NO_STP) {
+ NL_SET_ERR_MSG_MOD(extack, "MRP can't be enabled if STP is already enabled\n");
+ return -EINVAL;
+ }
+
+ err = nla_parse_nested(tb, IFLA_BRIDGE_MRP_MAX, attr,
+ NULL, extack);
+ if (err)
+ return err;
+
+ if (tb[IFLA_BRIDGE_MRP_INSTANCE]) {
+ struct br_mrp_instance *instance =
+ nla_data(tb[IFLA_BRIDGE_MRP_INSTANCE]);
+
+ if (cmd == RTM_SETLINK)
+ err = br_mrp_add(br, instance);
+ else
+ err = br_mrp_del(br, instance);
+ if (err)
+ return err;
+ }
+
+ if (tb[IFLA_BRIDGE_MRP_PORT_STATE]) {
+ enum br_mrp_port_state_type state =
+ nla_get_u32(tb[IFLA_BRIDGE_MRP_PORT_STATE]);
+
+ err = br_mrp_set_port_state(p, state);
+ if (err)
+ return err;
+ }
+
+ if (tb[IFLA_BRIDGE_MRP_PORT_ROLE]) {
+ struct br_mrp_port_role *role =
+ nla_data(tb[IFLA_BRIDGE_MRP_PORT_ROLE]);
+
+ err = br_mrp_set_port_role(p, role);
+ if (err)
+ return err;
+ }
+
+ if (tb[IFLA_BRIDGE_MRP_RING_STATE]) {
+ struct br_mrp_ring_state *state =
+ nla_data(tb[IFLA_BRIDGE_MRP_RING_STATE]);
+
+ err = br_mrp_set_ring_state(br, state);
+ if (err)
+ return err;
+ }
+
+ if (tb[IFLA_BRIDGE_MRP_RING_ROLE]) {
+ struct br_mrp_ring_role *role =
+ nla_data(tb[IFLA_BRIDGE_MRP_RING_ROLE]);
+
+ err = br_mrp_set_ring_role(br, role);
+ if (err)
+ return err;
+ }
+
+ if (tb[IFLA_BRIDGE_MRP_START_TEST]) {
+ struct br_mrp_start_test *test =
+ nla_data(tb[IFLA_BRIDGE_MRP_START_TEST]);
+
+ err = br_mrp_start_test(br, test);
+ if (err)
+ return err;
+ }
+
+ return 0;
+}
+
+int br_mrp_port_open(struct net_device *dev, u8 loc)
+{
+ struct net_bridge_port *p;
+ int err = 0;
+
+ p = br_port_get_rcu(dev);
+ if (!p) {
+ err = -EINVAL;
+ goto out;
+ }
+
+ p->loc = loc;
+ br_ifinfo_notify(RTM_NEWLINK, NULL, p);
+
+out:
+ return err;
+}
+EXPORT_SYMBOL(br_mrp_port_open);
--
2.17.1

2020-04-20 15:21:08

by Horatiu Vultur

[permalink] [raw]
Subject: [PATCH net-next 11/13] bridge: mrp: Update Makefile

Add all new mrp files to be compiled.

Signed-off-by: Horatiu Vultur <[email protected]>
---
net/bridge/Makefile | 2 ++
1 file changed, 2 insertions(+)

diff --git a/net/bridge/Makefile b/net/bridge/Makefile
index 49da7ae6f077..9bf3e1be3328 100644
--- a/net/bridge/Makefile
+++ b/net/bridge/Makefile
@@ -25,3 +25,5 @@ bridge-$(CONFIG_BRIDGE_VLAN_FILTERING) += br_vlan.o br_vlan_tunnel.o br_vlan_opt
bridge-$(CONFIG_NET_SWITCHDEV) += br_switchdev.o

obj-$(CONFIG_NETFILTER) += netfilter/
+
+bridge-$(CONFIG_BRIDGE_MRP) += br_mrp.o br_mrp_netlink.o br_mrp_switchdev.o
--
2.17.1

2020-04-20 15:21:10

by Horatiu Vultur

[permalink] [raw]
Subject: [PATCH net-next 03/13] bridge: mrp: Update Kconfig

Add the option BRIDGE_MRP to allow to build in or not MRP support.
The default value is N.

Signed-off-by: Horatiu Vultur <[email protected]>
---
net/bridge/Kconfig | 12 ++++++++++++
1 file changed, 12 insertions(+)

diff --git a/net/bridge/Kconfig b/net/bridge/Kconfig
index e4fb050e2078..51a6414145d2 100644
--- a/net/bridge/Kconfig
+++ b/net/bridge/Kconfig
@@ -61,3 +61,15 @@ config BRIDGE_VLAN_FILTERING
Say N to exclude this support and reduce the binary size.

If unsure, say Y.
+
+config BRIDGE_MRP
+ bool "MRP protocol"
+ depends on BRIDGE
+ default n
+ help
+ If you say Y here, then the Ethernet bridge will be able to run MRP
+ protocol to detect loops
+
+ Say N to exclude this support and reduce the binary size.
+
+ If unsure, say N.
--
2.17.1

2020-04-20 15:21:57

by Horatiu Vultur

[permalink] [raw]
Subject: [PATCH net-next 01/13] bridge: uapi: mrp: Add mrp attributes.

Add new nested netlink attribute to configure the MRP. These attributes are used
by the userspace to add/delete/configure MRP instances and by the kernel to
notify the userspace when the MRP ring gets open/closed. MRP nested attribute
has the following attributes:

IFLA_BRIDGE_MRP_INSTANCE - the parameter type is br_mrp_instance which contains
the instance id, and the ifindex of the two ports. The ports can't be part of
multiple instances. This is used to create/delete MRP instances.

IFLA_BRIDGE_MRP_PORT_STATE - the parameter type is u32. Which can be forwarding,
blocking or disabled.

IFLA_BRIDGE_MRP_PORT_ROLE - the parameter type is br_mrp_port_role which
contains the instance id and the role. The role can be primary or secondary.

IFLA_BRIDGE_MRP_RING_STATE - the parameter type is br_mrp_ring_state which
contains the instance id and the state. The state can be open or closed.

IFLA_BRIDGE_MRP_RING_ROLE - the parameter type is br_mrp_ring_role which
contains the instance id and the ring role. The role can be MRM or MRC.

IFLA_BRIDGE_MRP_START_TEST - the parameter type is br_mrp_start_test which
contains the instance id, the interval at which to send the MRP_Test frames,
how many test frames can be missed before declaring the ring open and the
period which represent for how long to send the test frames.

Also add the file include/uapi/linux/mrp_bridge.h which defines all the types
used by MRP that are also needed by the userpace.

Signed-off-by: Horatiu Vultur <[email protected]>
---
include/uapi/linux/if_bridge.h | 42 +++++++++++++++++
include/uapi/linux/if_ether.h | 1 +
include/uapi/linux/mrp_bridge.h | 84 +++++++++++++++++++++++++++++++++
3 files changed, 127 insertions(+)
create mode 100644 include/uapi/linux/mrp_bridge.h

diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
index bfe621ea51b3..bd8c95488f16 100644
--- a/include/uapi/linux/if_bridge.h
+++ b/include/uapi/linux/if_bridge.h
@@ -120,6 +120,7 @@ enum {
IFLA_BRIDGE_MODE,
IFLA_BRIDGE_VLAN_INFO,
IFLA_BRIDGE_VLAN_TUNNEL_INFO,
+ IFLA_BRIDGE_MRP,
__IFLA_BRIDGE_MAX,
};
#define IFLA_BRIDGE_MAX (__IFLA_BRIDGE_MAX - 1)
@@ -157,6 +158,47 @@ struct bridge_vlan_xstats {
__u32 pad2;
};

+enum {
+ IFLA_BRIDGE_MRP_UNSPEC,
+ IFLA_BRIDGE_MRP_INSTANCE,
+ IFLA_BRIDGE_MRP_PORT_STATE,
+ IFLA_BRIDGE_MRP_PORT_ROLE,
+ IFLA_BRIDGE_MRP_RING_STATE,
+ IFLA_BRIDGE_MRP_RING_ROLE,
+ IFLA_BRIDGE_MRP_START_TEST,
+ __IFLA_BRIDGE_MRP_MAX,
+};
+
+struct br_mrp_instance {
+ __u32 ring_id;
+ __u32 p_ifindex;
+ __u32 s_ifindex;
+};
+
+struct br_mrp_port_role {
+ __u32 ring_id;
+ __u32 role;
+};
+
+struct br_mrp_ring_state {
+ __u32 ring_id;
+ __u32 ring_state;
+};
+
+struct br_mrp_ring_role {
+ __u32 ring_id;
+ __u32 ring_role;
+};
+
+struct br_mrp_start_test {
+ __u32 ring_id;
+ __u32 interval;
+ __u32 max_miss;
+ __u32 period;
+};
+
+#define IFLA_BRIDGE_MRP_MAX (__IFLA_BRIDGE_MRP_MAX - 1)
+
struct bridge_stp_xstats {
__u64 transition_blk;
__u64 transition_fwd;
diff --git a/include/uapi/linux/if_ether.h b/include/uapi/linux/if_ether.h
index f6ceb2e63d1e..d6de2b167448 100644
--- a/include/uapi/linux/if_ether.h
+++ b/include/uapi/linux/if_ether.h
@@ -92,6 +92,7 @@
#define ETH_P_PREAUTH 0x88C7 /* 802.11 Preauthentication */
#define ETH_P_TIPC 0x88CA /* TIPC */
#define ETH_P_LLDP 0x88CC /* Link Layer Discovery Protocol */
+#define ETH_P_MRP 0x88E3 /* Media Redundancy Protocol */
#define ETH_P_MACSEC 0x88E5 /* 802.1ae MACsec */
#define ETH_P_8021AH 0x88E7 /* 802.1ah Backbone Service Tag */
#define ETH_P_MVRP 0x88F5 /* 802.1Q MVRP */
diff --git a/include/uapi/linux/mrp_bridge.h b/include/uapi/linux/mrp_bridge.h
new file mode 100644
index 000000000000..2600cdf5a284
--- /dev/null
+++ b/include/uapi/linux/mrp_bridge.h
@@ -0,0 +1,84 @@
+/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
+
+#ifndef _UAPI_LINUX_MRP_BRIDGE_H_
+#define _UAPI_LINUX_MRP_BRIDGE_H_
+
+#include <linux/types.h>
+#include <linux/if_ether.h>
+
+#define MRP_MAX_FRAME_LENGTH 200
+#define MRP_DEFAULT_PRIO 0x8000
+#define MRP_DOMAIN_UUID_LENGTH 16
+#define MRP_VERSION 1
+#define MRP_FRAME_PRIO 7
+
+enum br_mrp_ring_role_type {
+ BR_MRP_RING_ROLE_DISABLED,
+ BR_MRP_RING_ROLE_MRC,
+ BR_MRP_RING_ROLE_MRM,
+};
+
+enum br_mrp_ring_state_type {
+ BR_MRP_RING_STATE_OPEN,
+ BR_MRP_RING_STATE_CLOSED,
+};
+
+enum br_mrp_port_state_type {
+ BR_MRP_PORT_STATE_DISABLED,
+ BR_MRP_PORT_STATE_BLOCKED,
+ BR_MRP_PORT_STATE_FORWARDING,
+ BR_MRP_PORT_STATE_NOT_CONNECTED,
+};
+
+enum br_mrp_port_role_type {
+ BR_MRP_PORT_ROLE_PRIMARY,
+ BR_MRP_PORT_ROLE_SECONDARY,
+ BR_MRP_PORT_ROLE_NONE,
+};
+
+enum br_mrp_tlv_header_type {
+ BR_MRP_TLV_HEADER_END = 0x0,
+ BR_MRP_TLV_HEADER_COMMON = 0x1,
+ BR_MRP_TLV_HEADER_RING_TEST = 0x2,
+ BR_MRP_TLV_HEADER_RING_TOPO = 0x3,
+ BR_MRP_TLV_HEADER_RING_LINK_DOWN = 0x4,
+ BR_MRP_TLV_HEADER_RING_LINK_UP = 0x5,
+};
+
+struct br_mrp_tlv_hdr {
+ __u8 type;
+ __u8 length;
+};
+
+struct br_mrp_end_hdr {
+ struct br_mrp_tlv_hdr hdr;
+};
+
+struct br_mrp_common_hdr {
+ __u16 seq_id;
+ __u8 domain[MRP_DOMAIN_UUID_LENGTH];
+};
+
+struct br_mrp_ring_test_hdr {
+ __u16 prio;
+ __u8 sa[ETH_ALEN];
+ __u16 port_role;
+ __u16 state;
+ __u16 transitions;
+ __u32 timestamp;
+};
+
+struct br_mrp_ring_topo_hdr {
+ __u16 prio;
+ __u8 sa[ETH_ALEN];
+ __u16 interval;
+};
+
+struct br_mrp_ring_link_hdr {
+ __u8 sa[ETH_ALEN];
+ __u16 port_role;
+ __u16 interval;
+ __u16 blocked;
+};
+
+#endif
--
2.17.1

2020-04-20 15:21:59

by Horatiu Vultur

[permalink] [raw]
Subject: [PATCH net-next 02/13] net: bridge: Add port attribute IFLA_BRPORT_MRP_RING_OPEN

This patch adds a new port attribute, IFLA_BRPORT_MRP_RING_OPEN, which allows
to notify the userspace when the port lost the continuite of MRP frames.

This attribute is set by kernel whenever the SW or HW detects that the ring is
being open or closed.

Signed-off-by: Horatiu Vultur <[email protected]>
---
include/uapi/linux/if_link.h | 1 +
net/bridge/br_netlink.c | 6 ++++++
net/bridge/br_private.h | 4 ++++
tools/include/uapi/linux/if_link.h | 1 +
4 files changed, 12 insertions(+)

diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index 61e0801c82df..4a295deb933b 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -343,6 +343,7 @@ enum {
IFLA_BRPORT_NEIGH_SUPPRESS,
IFLA_BRPORT_ISOLATED,
IFLA_BRPORT_BACKUP_PORT,
+ IFLA_BRPORT_MRP_RING_OPEN,
__IFLA_BRPORT_MAX
};
#define IFLA_BRPORT_MAX (__IFLA_BRPORT_MAX - 1)
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 43dab4066f91..2f3a9e50c168 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -151,6 +151,9 @@ static inline size_t br_port_info_size(void)
+ nla_total_size(sizeof(u8)) /* IFLA_BRPORT_MULTICAST_ROUTER */
#endif
+ nla_total_size(sizeof(u16)) /* IFLA_BRPORT_GROUP_FWD_MASK */
+#if IS_ENABLED(CONFIG_BRIDGE_MRP)
+ + nla_total_size(sizeof(u8)) /* IFLA_BRPORT_MRP_RING_OPEN */
+#endif
+ 0;
}

@@ -213,6 +216,9 @@ static int br_port_fill_attrs(struct sk_buff *skb,
nla_put_u16(skb, IFLA_BRPORT_GROUP_FWD_MASK, p->group_fwd_mask) ||
nla_put_u8(skb, IFLA_BRPORT_NEIGH_SUPPRESS,
!!(p->flags & BR_NEIGH_SUPPRESS)) ||
+#if IS_ENABLED(CONFIG_BRIDGE_MRP)
+ nla_put_u8(skb, IFLA_BRPORT_MRP_RING_OPEN, p->loc) ||
+#endif
nla_put_u8(skb, IFLA_BRPORT_ISOLATED, !!(p->flags & BR_ISOLATED)))
return -EMSGSIZE;

diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 1f97703a52ff..735ec6ff86cc 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -289,6 +289,10 @@ struct net_bridge_port {
u16 backup_redirected_cnt;

struct bridge_stp_xstats stp_xstats;
+
+#if IS_ENABLED(CONFIG_BRIDGE_MRP)
+ u8 loc;
+#endif
};

#define kobj_to_brport(obj) container_of(obj, struct net_bridge_port, kobj)
diff --git a/tools/include/uapi/linux/if_link.h b/tools/include/uapi/linux/if_link.h
index 024af2d1d0af..70dae9ba16f4 100644
--- a/tools/include/uapi/linux/if_link.h
+++ b/tools/include/uapi/linux/if_link.h
@@ -343,6 +343,7 @@ enum {
IFLA_BRPORT_NEIGH_SUPPRESS,
IFLA_BRPORT_ISOLATED,
IFLA_BRPORT_BACKUP_PORT,
+ IFLA_BRPORT_MRP_RING_OPEN,
__IFLA_BRPORT_MAX
};
#define IFLA_BRPORT_MAX (__IFLA_BRPORT_MAX - 1)
--
2.17.1

2020-04-20 15:22:02

by Horatiu Vultur

[permalink] [raw]
Subject: [PATCH net-next 09/13] bridge: mrp: Connect MRP API with the switchev API

Implement the MRP API.

In case the HW can't generate MRP Test frames then the SW will try to generate
the frames. In case that also the SW will fail in generating the frames then a
error is return to the userspace. The userspace is responsible to generate all
the other MRP frames regardless if the test frames are generated by HW or SW.

The forwarding/termination of MRP frames is happening in the kernel and is done
by the MRP instance. The userspace application doesn't do the forwarding.

Signed-off-by: Horatiu Vultur <[email protected]>
---
net/bridge/br_mrp.c | 551 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 551 insertions(+)
create mode 100644 net/bridge/br_mrp.c

diff --git a/net/bridge/br_mrp.c b/net/bridge/br_mrp.c
new file mode 100644
index 000000000000..a52f02f174c3
--- /dev/null
+++ b/net/bridge/br_mrp.c
@@ -0,0 +1,551 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include <linux/mrp_bridge.h>
+#include "br_private_mrp.h"
+
+static const u8 mrp_test_dmac[ETH_ALEN] = { 0x1, 0x15, 0x4e, 0x0, 0x0, 0x1 };
+
+static struct net_bridge_port *br_mrp_get_port(struct net_bridge *br,
+ u32 ifindex)
+{
+ struct net_bridge_port *res = NULL;
+ struct net_bridge_port *port;
+
+ list_for_each_entry(port, &br->port_list, list) {
+ if (port->dev->ifindex == ifindex) {
+ res = port;
+ break;
+ }
+ }
+
+ return res;
+}
+
+static struct br_mrp *br_mrp_find_id(struct net_bridge *br, u32 ring_id)
+{
+ struct br_mrp *res = NULL;
+ struct br_mrp *mrp;
+
+ list_for_each_entry_rcu(mrp, &br->mrp_list, list,
+ lockdep_rtnl_is_held()) {
+ if (mrp->ring_id == ring_id) {
+ res = mrp;
+ break;
+ }
+ }
+
+ return res;
+}
+
+static struct br_mrp *br_mrp_find_port(struct net_bridge *br,
+ struct net_bridge_port *p)
+{
+ struct br_mrp *res = NULL;
+ struct br_mrp *mrp;
+
+ list_for_each_entry_rcu(mrp, &br->mrp_list, list,
+ lockdep_rtnl_is_held()) {
+ if (rcu_access_pointer(mrp->p_port) == p ||
+ rcu_access_pointer(mrp->s_port) == p) {
+ res = mrp;
+ break;
+ }
+ }
+
+ return res;
+}
+
+static int br_mrp_next_seq(struct br_mrp *mrp)
+{
+ mrp->seq_id++;
+ return mrp->seq_id;
+}
+
+static struct sk_buff *br_mrp_skb_alloc(struct net_bridge_port *p,
+ const u8 *src, const u8 *dst)
+{
+ struct ethhdr *eth_hdr;
+ struct sk_buff *skb;
+ u16 *version;
+
+ skb = dev_alloc_skb(MRP_MAX_FRAME_LENGTH);
+ if (!skb)
+ return NULL;
+
+ skb->dev = p->dev;
+ skb->protocol = htons(ETH_P_MRP);
+ skb->priority = MRP_FRAME_PRIO;
+ skb_reserve(skb, sizeof(*eth_hdr));
+
+ eth_hdr = skb_push(skb, sizeof(*eth_hdr));
+ ether_addr_copy(eth_hdr->h_dest, dst);
+ ether_addr_copy(eth_hdr->h_source, src);
+ eth_hdr->h_proto = htons(ETH_P_MRP);
+
+ version = skb_put(skb, sizeof(*version));
+ *version = cpu_to_be16(MRP_VERSION);
+
+ return skb;
+}
+
+static void br_mrp_skb_tlv(struct sk_buff *skb,
+ enum br_mrp_tlv_header_type type,
+ u8 length)
+{
+ struct br_mrp_tlv_hdr *hdr;
+
+ hdr = skb_put(skb, sizeof(*hdr));
+ hdr->type = type;
+ hdr->length = length;
+}
+
+static void br_mrp_skb_common(struct sk_buff *skb, struct br_mrp *mrp)
+{
+ struct br_mrp_common_hdr *hdr;
+
+ br_mrp_skb_tlv(skb, BR_MRP_TLV_HEADER_COMMON, sizeof(*hdr));
+
+ hdr = skb_put(skb, sizeof(*hdr));
+ hdr->seq_id = cpu_to_be16(br_mrp_next_seq(mrp));
+ memset(hdr->domain, 0xff, MRP_DOMAIN_UUID_LENGTH);
+}
+
+static struct sk_buff *br_mrp_alloc_test_skb(struct br_mrp *mrp,
+ struct net_bridge_port *p,
+ enum br_mrp_port_role_type port_role)
+{
+ struct br_mrp_ring_test_hdr *hdr = NULL;
+ struct net_bridge *br = p->br;
+ struct sk_buff *skb = NULL;
+
+ if (!p)
+ return NULL;
+
+ br = p->br;
+
+ skb = br_mrp_skb_alloc(p, p->dev->dev_addr, mrp_test_dmac);
+ if (!skb)
+ return NULL;
+
+ br_mrp_skb_tlv(skb, BR_MRP_TLV_HEADER_RING_TEST, sizeof(*hdr));
+ hdr = skb_put(skb, sizeof(*hdr));
+
+ hdr->prio = cpu_to_be16(MRP_DEFAULT_PRIO);
+ ether_addr_copy(hdr->sa, p->br->dev->dev_addr);
+ hdr->port_role = cpu_to_be16(port_role);
+ hdr->state = cpu_to_be16(mrp->ring_state);
+ hdr->transitions = cpu_to_be16(mrp->ring_transitions);
+ hdr->timestamp = cpu_to_be32(jiffies_to_msecs(jiffies));
+
+ br_mrp_skb_common(skb, mrp);
+ br_mrp_skb_tlv(skb, BR_MRP_TLV_HEADER_END, 0x0);
+
+ return skb;
+}
+
+static void br_mrp_test_work_expired(struct work_struct *work)
+{
+ struct delayed_work *del_work = to_delayed_work(work);
+ struct br_mrp *mrp = container_of(del_work, struct br_mrp, test_work);
+ struct net_bridge_port *p;
+ bool notify_open = false;
+ struct sk_buff *skb;
+
+ if (time_before_eq(mrp->test_end, jiffies))
+ return;
+
+ if (mrp->test_count_miss < mrp->test_max_miss) {
+ mrp->test_count_miss++;
+ } else {
+ /* Notify that the ring is open only if the ring state is
+ * closed, otherwise it would continue to notify at every
+ * interval.
+ */
+ if (mrp->ring_state == BR_MRP_RING_STATE_CLOSED)
+ notify_open = true;
+ }
+
+ rcu_read_lock();
+
+ p = rcu_dereference(mrp->p_port);
+ if (p) {
+ skb = br_mrp_alloc_test_skb(mrp, p, BR_MRP_PORT_ROLE_PRIMARY);
+ if (!skb)
+ goto out;
+
+ skb_reset_network_header(skb);
+ dev_queue_xmit(skb);
+
+ if (notify_open && !mrp->ring_role_offloaded)
+ br_mrp_port_open(p->dev, true);
+ }
+
+ p = rcu_dereference(mrp->s_port);
+ if (p) {
+ skb = br_mrp_alloc_test_skb(mrp, p, BR_MRP_PORT_ROLE_SECONDARY);
+ if (!skb)
+ goto out;
+
+ skb_reset_network_header(skb);
+ dev_queue_xmit(skb);
+
+ if (notify_open && !mrp->ring_role_offloaded)
+ br_mrp_port_open(p->dev, true);
+ }
+
+out:
+ rcu_read_unlock();
+
+ queue_delayed_work(system_wq, &mrp->test_work,
+ usecs_to_jiffies(mrp->test_interval));
+}
+
+/* Deletes the MRP instance.
+ * note: called under rtnl_lock
+ */
+static void br_mrp_del_impl(struct net_bridge *br, struct br_mrp *mrp)
+{
+ struct net_bridge_port *p;
+
+ /* Stop sending MRP_Test frames */
+ cancel_delayed_work_sync(&mrp->test_work);
+ br_mrp_switchdev_send_ring_test(br, mrp, 0, 0, 0);
+
+ br_mrp_switchdev_del(br, mrp);
+
+ /* Reset the ports */
+ p = rtnl_dereference(mrp->p_port);
+ if (p) {
+ spin_lock_bh(&br->lock);
+ p->state = BR_STATE_FORWARDING;
+ p->flags &= ~BR_MRP_AWARE;
+ spin_unlock_bh(&br->lock);
+ br_mrp_port_switchdev_set_state(p, BR_STATE_FORWARDING);
+ rcu_assign_pointer(mrp->p_port, NULL);
+ }
+
+ p = rtnl_dereference(mrp->s_port);
+ if (p) {
+ spin_lock_bh(&br->lock);
+ p->state = BR_STATE_FORWARDING;
+ p->flags &= ~BR_MRP_AWARE;
+ spin_unlock_bh(&br->lock);
+ br_mrp_port_switchdev_set_state(p, BR_STATE_FORWARDING);
+ rcu_assign_pointer(mrp->s_port, NULL);
+ }
+
+ list_del_rcu(&mrp->list);
+ kfree_rcu(mrp, rcu);
+}
+
+/* Adds a new MRP instance.
+ * note: called under rtnl_lock
+ */
+int br_mrp_add(struct net_bridge *br, struct br_mrp_instance *instance)
+{
+ struct net_bridge_port *p;
+ struct br_mrp *mrp;
+ int err;
+
+ /* If the ring exists, it is not possible to create another one with the
+ * same ring_id
+ */
+ mrp = br_mrp_find_id(br, instance->ring_id);
+ if (mrp)
+ return -EINVAL;
+
+ if (!br_mrp_get_port(br, instance->p_ifindex) ||
+ !br_mrp_get_port(br, instance->s_ifindex))
+ return -EINVAL;
+
+ mrp = kzalloc(sizeof(*mrp), GFP_KERNEL);
+ if (!mrp)
+ return -ENOMEM;
+
+ mrp->ring_id = instance->ring_id;
+
+ p = br_mrp_get_port(br, instance->p_ifindex);
+ spin_lock_bh(&br->lock);
+ p->state = BR_STATE_FORWARDING;
+ p->flags |= BR_MRP_AWARE;
+ spin_unlock_bh(&br->lock);
+ rcu_assign_pointer(mrp->p_port, p);
+
+ p = br_mrp_get_port(br, instance->s_ifindex);
+ spin_lock_bh(&br->lock);
+ p->state = BR_STATE_FORWARDING;
+ p->flags |= BR_MRP_AWARE;
+ spin_unlock_bh(&br->lock);
+ rcu_assign_pointer(mrp->s_port, p);
+
+ INIT_DELAYED_WORK(&mrp->test_work, br_mrp_test_work_expired);
+ list_add_tail_rcu(&mrp->list, &br->mrp_list);
+
+ err = br_mrp_switchdev_add(br, mrp);
+ if (err)
+ goto delete_mrp;
+
+ return 0;
+
+delete_mrp:
+ br_mrp_del_impl(br, mrp);
+
+ return err;
+}
+
+/* Deletes the MRP instance from which the port is part of
+ * note: called under rtnl_lock
+ */
+void br_mrp_port_del(struct net_bridge *br, struct net_bridge_port *p)
+{
+ struct br_mrp *mrp = br_mrp_find_port(br, p);
+
+ /* If the port is not part of a MRP instance just bail out */
+ if (!mrp)
+ return;
+
+ br_mrp_del_impl(br, mrp);
+}
+
+/* Deletes existing MRP instance based on ring_id
+ * note: called under rtnl_lock
+ */
+int br_mrp_del(struct net_bridge *br, struct br_mrp_instance *instance)
+{
+ struct br_mrp *mrp = br_mrp_find_id(br, instance->ring_id);
+
+ if (!mrp)
+ return -EINVAL;
+
+ br_mrp_del_impl(br, mrp);
+
+ return 0;
+}
+
+/* Set port state, port state can be forwarding, blocked or disabled
+ * note: already called with rcu_read_lock
+ */
+int br_mrp_set_port_state(struct net_bridge_port *p,
+ enum br_mrp_port_state_type state)
+{
+ spin_lock_bh(&p->br->lock);
+
+ if (state == BR_MRP_PORT_STATE_FORWARDING)
+ p->state = BR_STATE_FORWARDING;
+ else
+ p->state = BR_STATE_BLOCKING;
+
+ spin_unlock_bh(&p->br->lock);
+
+ br_mrp_port_switchdev_set_state(p, state);
+
+ return 0;
+}
+
+/* Set port role, port role can be primary or secondary
+ * note: already called with rcu_read_lock
+ */
+int br_mrp_set_port_role(struct net_bridge_port *p,
+ struct br_mrp_port_role *role)
+{
+ struct br_mrp *mrp = br_mrp_find_id(p->br, role->ring_id);
+
+ if (!mrp)
+ return -EINVAL;
+
+ if (role->role == BR_MRP_PORT_ROLE_PRIMARY)
+ rcu_assign_pointer(mrp->p_port, p);
+ else
+ rcu_assign_pointer(mrp->s_port, p);
+
+ br_mrp_port_switchdev_set_role(p, role->role);
+
+ return 0;
+}
+
+/* Set ring state, ring state can be only Open or Closed
+ * note: already called with rcu_read_lock
+ */
+int br_mrp_set_ring_state(struct net_bridge *br,
+ struct br_mrp_ring_state *state)
+{
+ struct br_mrp *mrp = br_mrp_find_id(br, state->ring_id);
+
+ if (!mrp)
+ return -EINVAL;
+
+ if (mrp->ring_state == BR_MRP_RING_STATE_CLOSED &&
+ state->ring_state != BR_MRP_RING_STATE_CLOSED)
+ mrp->ring_transitions++;
+
+ mrp->ring_state = state->ring_state;
+
+ br_mrp_switchdev_set_ring_state(br, mrp, state->ring_state);
+
+ return 0;
+}
+
+/* Set ring role, ring role can be only MRM(Media Redundancy Manager) or
+ * MRC(Media Redundancy Client).
+ * note: already called with rcu_read_lock
+ */
+int br_mrp_set_ring_role(struct net_bridge *br,
+ struct br_mrp_ring_role *role)
+{
+ struct br_mrp *mrp = br_mrp_find_id(br, role->ring_id);
+ int err;
+
+ if (!mrp)
+ return -EINVAL;
+
+ mrp->ring_role = role->ring_role;
+
+ /* If there is an error just bailed out */
+ err = br_mrp_switchdev_set_ring_role(br, mrp, role->ring_role);
+ if (err && err != -EOPNOTSUPP)
+ return err;
+
+ /* Now detect if the HW actually applied the role or not. If the HW
+ * applied the role it means that the SW will not to do those operations
+ * anymore. For example if the role ir MRM then the HW will notify the
+ * SW when ring is open, but if the is not pushed to the HW the SW will
+ * need to detect when the ring is open
+ */
+ mrp->ring_role_offloaded = err == -EOPNOTSUPP ? 0 : 1;
+
+ return 0;
+}
+
+/* Start to generate MRP test frames, the frames are generated by HW and if it
+ * fails, they are generated by the SW.
+ * note: already called with rcu_read_lock
+ */
+int br_mrp_start_test(struct net_bridge *br,
+ struct br_mrp_start_test *test)
+{
+ struct br_mrp *mrp = br_mrp_find_id(br, test->ring_id);
+
+ if (!mrp)
+ return -EINVAL;
+
+ /* Try to push is to the HW and if it fails then continue to generate in
+ * SW and if that also fails then return error
+ */
+ if (!br_mrp_switchdev_send_ring_test(br, mrp, test->interval,
+ test->max_miss, test->period))
+ return 0;
+
+ mrp->test_interval = test->interval;
+ mrp->test_end = jiffies + usecs_to_jiffies(test->period);
+ mrp->test_max_miss = test->max_miss;
+ mrp->test_count_miss = 0;
+ queue_delayed_work(system_wq, &mrp->test_work,
+ usecs_to_jiffies(test->interval));
+
+ return 0;
+}
+
+/* Process only MRP Test frame. All the other MRP frames are processed by
+ * userspace application
+ * note: already called with rcu_read_lock
+ */
+static void br_mrp_mrm_process(struct br_mrp *mrp, struct net_bridge_port *port,
+ struct sk_buff *skb)
+{
+ struct br_mrp_tlv_hdr *hdr;
+
+ hdr = (struct br_mrp_tlv_hdr *)(skb->data + sizeof(uint16_t));
+
+ if (!hdr)
+ return;
+
+ if (hdr->type != BR_MRP_TLV_HEADER_RING_TEST)
+ return;
+
+ mrp->test_count_miss = 0;
+
+ /* Notify the userspace that the ring is closed only when the ring is
+ * not closed
+ */
+ if (mrp->ring_state != BR_MRP_RING_STATE_CLOSED)
+ br_mrp_port_open(port->dev, false);
+}
+
+/* This will just forward the frame to the other mrp ring port(MRC role) or will
+ * not do anything.
+ * note: already called with rcu_read_lock
+ */
+static int br_mrp_rcv(struct net_bridge_port *p,
+ struct sk_buff *skb, struct net_device *dev)
+{
+ struct net_device *s_dev, *p_dev, *d_dev;
+ struct net_bridge_port *p_port, *s_port;
+ struct net_bridge *br;
+ struct sk_buff *nskb;
+ struct br_mrp *mrp;
+
+ /* If port is disabled don't accept any frames */
+ if (p->state == BR_STATE_DISABLED)
+ return 0;
+
+ br = p->br;
+ mrp = br_mrp_find_port(br, p);
+ if (unlikely(!mrp))
+ return 0;
+
+ p_port = rcu_dereference(mrp->p_port);
+ if (!p_port)
+ return 0;
+
+ s_port = rcu_dereference(mrp->s_port);
+ if (!s_port)
+ return 0;
+
+ /* If the role is MRM then don't forward the frames */
+ if (mrp->ring_role == BR_MRP_RING_ROLE_MRM) {
+ br_mrp_mrm_process(mrp, p, skb);
+ return 1;
+ }
+
+ /* Clone the frame and forward it on the other MRP port */
+ nskb = skb_clone(skb, GFP_ATOMIC);
+ if (!nskb)
+ return 0;
+
+ p_dev = p_port->dev;
+ s_dev = s_port->dev;
+
+ if (p_dev == dev)
+ d_dev = s_dev;
+ else
+ d_dev = p_dev;
+
+ nskb->dev = d_dev;
+ skb_push(nskb, ETH_HLEN);
+ dev_queue_xmit(nskb);
+
+ return 1;
+}
+
+/* Check if the frame was received on a port that is part of MRP ring
+ * and if the frame has MRP eth. In that case process the frame otherwise do
+ * normal forwarding.
+ * note: already called with rcu_read_lock
+ */
+int br_mrp_process(struct net_bridge_port *p, struct sk_buff *skb)
+{
+ /* If there is no MRP instance do normal forwarding */
+ if (likely(!(p->flags & BR_MRP_AWARE)))
+ goto out;
+
+ if (unlikely(skb->protocol == htons(ETH_P_MRP)))
+ return br_mrp_rcv(p, skb, p->dev);
+
+out:
+ return 0;
+}
+
+bool br_mrp_enabled(struct net_bridge *br)
+{
+ return !list_empty(&br->mrp_list);
+}
--
2.17.1

2020-04-20 15:22:24

by Horatiu Vultur

[permalink] [raw]
Subject: [PATCH net-next 06/13] bridge: mrp: Extend bridge interface

To integrate MRP into the bridge, first the bridge needs to be aware of ports
that are part of an MRP ring and which rings are on the bridge.
Therefore extend bridge interface with the following:
- add new flag(BR_MPP_AWARE) to the net bridge ports, this bit will be
set when the port is added to an MRP instance. In this way it knows if
the frame was received on MRP ring port
- add a list of MRP instances

Signed-off-by: Horatiu Vultur <[email protected]>
---
include/linux/if_bridge.h | 1 +
net/bridge/br_private.h | 4 ++++
2 files changed, 5 insertions(+)

diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
index 9e57c4411734..10baa9efdae8 100644
--- a/include/linux/if_bridge.h
+++ b/include/linux/if_bridge.h
@@ -47,6 +47,7 @@ struct br_ip_list {
#define BR_BCAST_FLOOD BIT(14)
#define BR_NEIGH_SUPPRESS BIT(15)
#define BR_ISOLATED BIT(16)
+#define BR_MRP_AWARE BIT(17)

#define BR_DEFAULT_AGEING_TIME (300 * HZ)

diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 735ec6ff86cc..b3c176a0a136 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -432,6 +432,10 @@ struct net_bridge {
int offload_fwd_mark;
#endif
struct hlist_head fdb_list;
+
+#if IS_ENABLED(CONFIG_BRIDGE_MRP)
+ struct list_head __rcu mrp_list;
+#endif
};

struct br_input_skb_cb {
--
2.17.1

2020-04-20 15:23:01

by Horatiu Vultur

[permalink] [raw]
Subject: [PATCH net-next 07/13] switchdev: mrp: Extend switchdev API to offload MRP

Extend switchdev API to add support for MRP. The HW is notified in
following cases:

SWITCHDEV_OBJ_ID_MRP: This is used when a MRP instance is added/removed
from the MRP ring.

SWITCHDEV_OBJ_ID_RING_ROLE_MRP: This is used when the role of the node
changes. The current supported roles are MRM and MRC.

SWITCHDEV_OBJ_ID_RING_TEST_MRP: This is used when to start/stop sending
MRP_Test frames on the mrp ring ports. This is called only on nodes that have
the role MRM. In case this fails then the SW will generate the frames.

SWITCHDEV_OBJ_ID_RING_STATE_STATE: This is used when the ring changes it states
to open or closed. This is required to notify HW because the MRP_Test frame
contains the field MRP_InState which contains this information.

SWITCHDEV_ATTR_ID_MRP_PORT_STATE: This is used when the port's state is
changed. It can be in blocking/forwarding mode.

SWITCHDEV_ATTR_ID_MRP_PORT_ROLE: This is used when port's role changes. The
roles of the port can be primary/secondary. This is required to notify HW
because the MRP_Test frame contains the field MRP_PortRole that contains this
information.

Signed-off-by: Horatiu Vultur <[email protected]>
---
include/net/switchdev.h | 62 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 62 insertions(+)

diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index aee86a189432..ae7aeb0d1f9c 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -40,6 +40,10 @@ enum switchdev_attr_id {
SWITCHDEV_ATTR_ID_BRIDGE_VLAN_FILTERING,
SWITCHDEV_ATTR_ID_BRIDGE_MC_DISABLED,
SWITCHDEV_ATTR_ID_BRIDGE_MROUTER,
+#if IS_ENABLED(CONFIG_BRIDGE_MRP)
+ SWITCHDEV_ATTR_ID_MRP_PORT_STATE,
+ SWITCHDEV_ATTR_ID_MRP_PORT_ROLE,
+#endif
};

struct switchdev_attr {
@@ -55,6 +59,11 @@ struct switchdev_attr {
clock_t ageing_time; /* BRIDGE_AGEING_TIME */
bool vlan_filtering; /* BRIDGE_VLAN_FILTERING */
bool mc_disabled; /* MC_DISABLED */
+#if IS_ENABLED(CONFIG_BRIDGE_MRP)
+ u8 mrp_port_state; /* MRP_PORT_STATE */
+ u8 mrp_port_role; /* MRP_PORT_ROLE */
+ u8 mrp_ring_state; /* MRP_RING_STATE */
+#endif
} u;
};

@@ -63,6 +72,12 @@ enum switchdev_obj_id {
SWITCHDEV_OBJ_ID_PORT_VLAN,
SWITCHDEV_OBJ_ID_PORT_MDB,
SWITCHDEV_OBJ_ID_HOST_MDB,
+#if IS_ENABLED(CONFIG_BRIDGE_MRP)
+ SWITCHDEV_OBJ_ID_MRP,
+ SWITCHDEV_OBJ_ID_RING_TEST_MRP,
+ SWITCHDEV_OBJ_ID_RING_ROLE_MRP,
+ SWITCHDEV_OBJ_ID_RING_STATE_MRP,
+#endif
};

struct switchdev_obj {
@@ -94,6 +109,53 @@ struct switchdev_obj_port_mdb {
#define SWITCHDEV_OBJ_PORT_MDB(OBJ) \
container_of((OBJ), struct switchdev_obj_port_mdb, obj)

+
+#if IS_ENABLED(CONFIG_BRIDGE_MRP)
+/* SWITCHDEV_OBJ_ID_MRP */
+struct switchdev_obj_mrp {
+ struct switchdev_obj obj;
+ struct net_device *p_port;
+ struct net_device *s_port;
+ u32 ring_id;
+};
+
+#define SWITCHDEV_OBJ_MRP(OBJ) \
+ container_of((OBJ), struct switchdev_obj_mrp, obj)
+
+/* SWITCHDEV_OBJ_ID_RING_TEST_MRP */
+struct switchdev_obj_ring_test_mrp {
+ struct switchdev_obj obj;
+ /* The value is in us and a value of 0 represents to stop */
+ u32 interval;
+ u8 max_miss;
+ u32 ring_id;
+ u32 period;
+};
+
+#define SWITCHDEV_OBJ_RING_TEST_MRP(OBJ) \
+ container_of((OBJ), struct switchdev_obj_ring_test_mrp, obj)
+
+/* SWICHDEV_OBJ_ID_RING_ROLE_MRP */
+struct switchdev_obj_ring_role_mrp {
+ struct switchdev_obj obj;
+ u8 ring_role;
+ u32 ring_id;
+};
+
+#define SWITCHDEV_OBJ_RING_ROLE_MRP(OBJ) \
+ container_of((OBJ), struct switchdev_obj_ring_role_mrp, obj)
+
+struct switchdev_obj_ring_state_mrp {
+ struct switchdev_obj obj;
+ u8 ring_state;
+ u32 ring_id;
+};
+
+#define SWITCHDEV_OBJ_RING_STATE_MRP(OBJ) \
+ container_of((OBJ), struct switchdev_obj_ring_state_mrp, obj)
+
+#endif
+
typedef int switchdev_obj_dump_cb_t(struct switchdev_obj *obj);

enum switchdev_notifier_type {
--
2.17.1

2020-04-20 15:23:43

by Horatiu Vultur

[permalink] [raw]
Subject: [PATCH net-next 05/13] bridge: mrp: Add MRP interface.

Define the MRP interface.
This interface is used by the netlink to update the MRP instances and by the MRP
to make the calls to switchdev to offload it to HW.

It defines an MRP instance 'struct br_mrp' which is a list of MRP instances.
Which will be part of the 'struct net_bridge'. Each instance has 2 ring ports,
a bridge and an ID.

In case the HW can't generate MRP Test frames then the SW will generate those.

br_mrp_add - adds a new MRP instance.

br_mrp_del - deletes an existing MRP instance. Each instance has an ID(ring_id).

br_mrp_set_port_state - changes the port state. The port can be in forwarding
state, which means that the frames can pass through or in blocked state which
means that the frames can't pass through except MRP frames. This will
eventually call the switchdev API to notify the HW. This information is used
also by the SW bridge to know how to forward frames in case the HW doesn't
have this capability.

br_mrp_set_port_role - a port role can be primary or secondary. This
information is required to be pushed to HW in case the HW can generate
MRP_Test frames. Because the MRP_Test frames contains a file with this
information. Otherwise the HW will not be able to generate the frames
correctly.

br_mrp_set_ring_state - a ring can be in state open or closed. State open means
that the mrp port stopped receiving MRP_Test frames, while closed means that
the mrp port received MRP_Test frames. Similar with br_mrp_port_role, this
information is pushed in HW because the MRP_Test frames contain this
information.

br_mrp_set_ring_role - a ring can have the following roles MRM or MRC. For the
role MRM it is expected that the HW can terminate the MRP frames, notify the
SW that it stopped receiving MRP_Test frames and trapp all the other MRP
frames. While for MRC mode it is expected that the HW can forward the MRP
frames only between the MRP ports and copy MRP_Topology frames to CPU. In
case the HW doesn't support a role it needs to return an error code different
than -EOPNOTSUPP.

br_mrp_start_test - this starts/stops the generation of MRP_Test frames. To stop
the generation of frames the interval needs to have a value of 0. In this case
the userspace needs to know if the HW supports this or not. Not to have
duplicate frames(generated by HW and SW). Because if the HW supports this then
the SW will not generate anymore frames and will expect that the HW will
notify when it stopped receiving MRP frames using the function
br_mrp_port_open.

br_mrp_port_open - this function is used by drivers to notify the userspace via
a netlink callback that one of the ports stopped receiving MRP_Test frames.
This function is called only when the node has the role MRM. It is not
supposed to be called from userspace.

br_mrp_port_switchdev_add - this corresponds to the function br_mrp_add,
and will notify the HW that a MRP instance is added. The function gets
as parameter the MRP instance.

br_mrp_port_switchdev_del - this corresponds to the function br_mrp_del,
and will notify the HW that a MRP instance is removed. The function
gets as parameter the ID of the MRP instance that is removed.

br_mrp_port_switchdev_set_state - this corresponds to the function
br_mrp_set_port_state. It would notify the HW if it should block or not
non-MRP frames.

br_mrp_port_switchdev_set_port - this corresponds to the function
br_mrp_set_port_role. It would set the port role, primary or secondary.

br_mrp_switchdev_set_role - this corresponds to the function
br_mrp_set_ring_role and would set one of the role MRM or MRC.

br_mrp_switchdev_set_ring_state - this corresponds to the function
br_mrp_set_ring_state and would set the ring to be open or closed.

br_mrp_switchdev_send_ring_test - this corresponds to the function
br_mrp_start_test. This will notify the HW to start or stop generating
MRP_Test frames. Value 0 for the interval parameter means to stop generating
the frames.

Signed-off-by: Horatiu Vultur <[email protected]>
---
net/bridge/br_private_mrp.h | 60 +++++++++++++++++++++++++++++++++++++
1 file changed, 60 insertions(+)
create mode 100644 net/bridge/br_private_mrp.h

diff --git a/net/bridge/br_private_mrp.h b/net/bridge/br_private_mrp.h
new file mode 100644
index 000000000000..5af8d13cce2b
--- /dev/null
+++ b/net/bridge/br_private_mrp.h
@@ -0,0 +1,60 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#ifndef _BR_PRIVATE_MRP_H_
+#define _BR_PRIVATE_MRP_H_
+
+#include "br_private.h"
+#include <uapi/linux/mrp_bridge.h>
+
+struct br_mrp {
+ /* list of mrp instances */
+ struct list_head __rcu list;
+
+ struct net_bridge_port __rcu *p_port;
+ struct net_bridge_port __rcu *s_port;
+
+ u32 ring_id;
+
+ enum br_mrp_ring_role_type ring_role;
+ u8 ring_role_offloaded;
+ enum br_mrp_ring_state_type ring_state;
+ u32 ring_transitions;
+
+ struct delayed_work test_work;
+ u32 test_interval;
+ unsigned long test_end;
+ u32 test_count_miss;
+ u32 test_max_miss;
+
+ u32 seq_id;
+
+ struct rcu_head rcu;
+};
+
+/* br_mrp.c */
+int br_mrp_add(struct net_bridge *br, struct br_mrp_instance *instance);
+int br_mrp_del(struct net_bridge *br, struct br_mrp_instance *instance);
+int br_mrp_set_port_state(struct net_bridge_port *p,
+ enum br_mrp_port_state_type state);
+int br_mrp_set_port_role(struct net_bridge_port *p,
+ struct br_mrp_port_role *role);
+int br_mrp_set_ring_state(struct net_bridge *br,
+ struct br_mrp_ring_state *state);
+int br_mrp_set_ring_role(struct net_bridge *br, struct br_mrp_ring_role *role);
+int br_mrp_start_test(struct net_bridge *br, struct br_mrp_start_test *test);
+
+/* br_mrp_switchdev.c */
+int br_mrp_switchdev_add(struct net_bridge *br, struct br_mrp *mrp);
+int br_mrp_switchdev_del(struct net_bridge *br, struct br_mrp *mrp);
+int br_mrp_switchdev_set_ring_role(struct net_bridge *br, struct br_mrp *mrp,
+ enum br_mrp_ring_role_type role);
+int br_mrp_switchdev_set_ring_state(struct net_bridge *br, struct br_mrp *mrp,
+ enum br_mrp_ring_state_type state);
+int br_mrp_switchdev_send_ring_test(struct net_bridge *br, struct br_mrp *mrp,
+ u32 interval, u8 max_miss, u32 period);
+int br_mrp_port_switchdev_set_state(struct net_bridge_port *p,
+ enum br_mrp_port_state_type state);
+int br_mrp_port_switchdev_set_role(struct net_bridge_port *p,
+ enum br_mrp_port_role_type role);
+
+#endif /* _BR_PRIVATE_MRP_H */
--
2.17.1

2020-04-20 16:41:16

by Horatiu Vultur

[permalink] [raw]
Subject: [PATCH net-next 08/13] bridge: switchdev: mrp: Implement MRP API for switchdev

Implement the MRP api for switchdev.
These functions will just eventually call the switchdev functions:
switchdev_port_obj_add/del and switchdev_port_attr_set.

Signed-off-by: Horatiu Vultur <[email protected]>
---
net/bridge/br_mrp_switchdev.c | 141 ++++++++++++++++++++++++++++++++++
1 file changed, 141 insertions(+)
create mode 100644 net/bridge/br_mrp_switchdev.c

diff --git a/net/bridge/br_mrp_switchdev.c b/net/bridge/br_mrp_switchdev.c
new file mode 100644
index 000000000000..6ece7817c757
--- /dev/null
+++ b/net/bridge/br_mrp_switchdev.c
@@ -0,0 +1,141 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include <net/switchdev.h>
+
+#include "br_private_mrp.h"
+
+int br_mrp_switchdev_add(struct net_bridge *br, struct br_mrp *mrp)
+{
+ struct switchdev_obj_mrp mrp_obj = {
+ .obj.orig_dev = br->dev,
+ .obj.id = SWITCHDEV_OBJ_ID_MRP,
+ .p_port = rtnl_dereference(mrp->p_port)->dev,
+ .s_port = rtnl_dereference(mrp->s_port)->dev,
+ .ring_id = mrp->ring_id,
+ };
+ int err;
+
+ err = switchdev_port_obj_add(br->dev, &mrp_obj.obj, NULL);
+
+ if (err && err != -EOPNOTSUPP)
+ return err;
+
+ return 0;
+}
+
+int br_mrp_switchdev_del(struct net_bridge *br, struct br_mrp *mrp)
+{
+ struct switchdev_obj_mrp mrp_obj = {
+ .obj.orig_dev = br->dev,
+ .obj.id = SWITCHDEV_OBJ_ID_MRP,
+ .p_port = NULL,
+ .s_port = NULL,
+ .ring_id = mrp->ring_id,
+ };
+ int err;
+
+ err = switchdev_port_obj_del(br->dev, &mrp_obj.obj);
+
+ if (err && err != -EOPNOTSUPP)
+ return err;
+
+ return 0;
+}
+
+int br_mrp_switchdev_set_ring_role(struct net_bridge *br,
+ struct br_mrp *mrp,
+ enum br_mrp_ring_role_type role)
+{
+ struct switchdev_obj_ring_role_mrp mrp_role = {
+ .obj.orig_dev = br->dev,
+ .obj.id = SWITCHDEV_OBJ_ID_RING_ROLE_MRP,
+ .ring_role = role,
+ .ring_id = mrp->ring_id,
+ };
+ int err;
+
+ if (role == BR_MRP_RING_ROLE_DISABLED)
+ err = switchdev_port_obj_del(br->dev, &mrp_role.obj);
+ else
+ err = switchdev_port_obj_add(br->dev, &mrp_role.obj, NULL);
+
+ return err;
+}
+
+int br_mrp_switchdev_send_ring_test(struct net_bridge *br,
+ struct br_mrp *mrp, u32 interval,
+ u8 max_miss, u32 period)
+{
+ struct switchdev_obj_ring_test_mrp test = {
+ .obj.orig_dev = br->dev,
+ .obj.id = SWITCHDEV_OBJ_ID_RING_TEST_MRP,
+ .interval = interval,
+ .max_miss = max_miss,
+ .ring_id = mrp->ring_id,
+ .period = period,
+ };
+ int err;
+
+ if (interval == 0)
+ err = switchdev_port_obj_del(br->dev, &test.obj);
+ else
+ err = switchdev_port_obj_add(br->dev, &test.obj, NULL);
+
+ return err;
+}
+
+int br_mrp_switchdev_set_ring_state(struct net_bridge *br,
+ struct br_mrp *mrp,
+ enum br_mrp_ring_state_type state)
+{
+ struct switchdev_obj_ring_state_mrp mrp_state = {
+ .obj.orig_dev = br->dev,
+ .obj.id = SWITCHDEV_OBJ_ID_RING_STATE_MRP,
+ .ring_state = state,
+ .ring_id = mrp->ring_id,
+ };
+ int err;
+
+ err = switchdev_port_obj_add(br->dev, &mrp_state.obj, NULL);
+
+ if (err && err != -EOPNOTSUPP)
+ return err;
+
+ return 0;
+}
+
+int br_mrp_port_switchdev_set_state(struct net_bridge_port *p,
+ enum br_mrp_port_state_type state)
+{
+ struct switchdev_attr attr = {
+ .orig_dev = p->dev,
+ .id = SWITCHDEV_ATTR_ID_MRP_PORT_STATE,
+ .u.mrp_port_state = state,
+ };
+ int err;
+
+ err = switchdev_port_attr_set(p->dev, &attr);
+ if (err && err != -EOPNOTSUPP)
+ br_warn(p->br, "error setting offload MRP state on port %u(%s)\n",
+ (unsigned int)p->port_no, p->dev->name);
+
+ return err;
+}
+
+int br_mrp_port_switchdev_set_role(struct net_bridge_port *p,
+ enum br_mrp_port_role_type role)
+{
+ struct switchdev_attr attr = {
+ .orig_dev = p->dev,
+ .id = SWITCHDEV_ATTR_ID_MRP_PORT_ROLE,
+ .u.mrp_port_role = role,
+ };
+ int err;
+
+ err = switchdev_port_attr_set(p->dev, &attr);
+ if (err && err != -EOPNOTSUPP)
+ return err;
+
+ return 0;
+}
+
--
2.17.1

2020-04-20 16:41:19

by Horatiu Vultur

[permalink] [raw]
Subject: [PATCH net-next 13/13] net: bridge: Add checks for enabling the STP.

It is not possible to have the MRP and STP running at the same time on the
bridge, therefore add check when enabling the STP to check if MRP is already
enabled. In that case return error.

Signed-off-by: Horatiu Vultur <[email protected]>
---
net/bridge/br_ioctl.c | 3 +--
net/bridge/br_netlink.c | 4 +++-
net/bridge/br_private.h | 3 ++-
net/bridge/br_stp.c | 6 ++++++
net/bridge/br_stp_if.c | 11 ++++++++++-
net/bridge/br_sysfs_br.c | 4 +---
6 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/net/bridge/br_ioctl.c b/net/bridge/br_ioctl.c
index ae22d784b88a..5e71fc8b826f 100644
--- a/net/bridge/br_ioctl.c
+++ b/net/bridge/br_ioctl.c
@@ -242,8 +242,7 @@ static int old_dev_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
if (!ns_capable(dev_net(dev)->user_ns, CAP_NET_ADMIN))
return -EPERM;

- br_stp_set_enabled(br, args[1]);
- ret = 0;
+ ret = br_stp_set_enabled(br, args[1], NULL);
break;

case BRCTL_SET_BRIDGE_PRIORITY:
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 52f7bbd3f382..dfdf076616c6 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -1112,7 +1112,9 @@ static int br_changelink(struct net_device *brdev, struct nlattr *tb[],
if (data[IFLA_BR_STP_STATE]) {
u32 stp_enabled = nla_get_u32(data[IFLA_BR_STP_STATE]);

- br_stp_set_enabled(br, stp_enabled);
+ err = br_stp_set_enabled(br, stp_enabled, extack);
+ if (err)
+ return err;
}

if (data[IFLA_BR_PRIORITY]) {
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index bbc496fcb181..5c24ff92352c 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -1287,7 +1287,8 @@ int br_set_ageing_time(struct net_bridge *br, clock_t ageing_time);
/* br_stp_if.c */
void br_stp_enable_bridge(struct net_bridge *br);
void br_stp_disable_bridge(struct net_bridge *br);
-void br_stp_set_enabled(struct net_bridge *br, unsigned long val);
+int br_stp_set_enabled(struct net_bridge *br, unsigned long val,
+ struct netlink_ext_ack *extack);
void br_stp_enable_port(struct net_bridge_port *p);
void br_stp_disable_port(struct net_bridge_port *p);
bool br_stp_recalculate_bridge_id(struct net_bridge *br);
diff --git a/net/bridge/br_stp.c b/net/bridge/br_stp.c
index 1f14b8455345..3e88be7aa269 100644
--- a/net/bridge/br_stp.c
+++ b/net/bridge/br_stp.c
@@ -36,6 +36,12 @@ void br_set_state(struct net_bridge_port *p, unsigned int state)
};
int err;

+ /* Don't change the state of the ports if they are driven by a different
+ * protocol.
+ */
+ if (p->flags & BR_MRP_AWARE)
+ return;
+
p->state = state;
err = switchdev_port_attr_set(p->dev, &attr);
if (err && err != -EOPNOTSUPP)
diff --git a/net/bridge/br_stp_if.c b/net/bridge/br_stp_if.c
index d174d3a566aa..a42850b7eb9a 100644
--- a/net/bridge/br_stp_if.c
+++ b/net/bridge/br_stp_if.c
@@ -196,10 +196,17 @@ static void br_stp_stop(struct net_bridge *br)
br->stp_enabled = BR_NO_STP;
}

-void br_stp_set_enabled(struct net_bridge *br, unsigned long val)
+int br_stp_set_enabled(struct net_bridge *br, unsigned long val,
+ struct netlink_ext_ack *extack)
{
ASSERT_RTNL();

+ if (br_mrp_enabled(br)) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "STP can't be enabled if MRP is already enabled\n");
+ return -EINVAL;
+ }
+
if (val) {
if (br->stp_enabled == BR_NO_STP)
br_stp_start(br);
@@ -207,6 +214,8 @@ void br_stp_set_enabled(struct net_bridge *br, unsigned long val)
if (br->stp_enabled != BR_NO_STP)
br_stp_stop(br);
}
+
+ return 0;
}

/* called under bridge lock */
diff --git a/net/bridge/br_sysfs_br.c b/net/bridge/br_sysfs_br.c
index 9ab0f00b1081..7db06e3f642a 100644
--- a/net/bridge/br_sysfs_br.c
+++ b/net/bridge/br_sysfs_br.c
@@ -126,9 +126,7 @@ static ssize_t stp_state_show(struct device *d,

static int set_stp_state(struct net_bridge *br, unsigned long val)
{
- br_stp_set_enabled(br, val);
-
- return 0;
+ return br_stp_set_enabled(br, val, NULL);
}

static ssize_t stp_state_store(struct device *d,
--
2.17.1

2020-04-20 17:19:41

by Nikolay Aleksandrov

[permalink] [raw]
Subject: Re: [PATCH net-next 09/13] bridge: mrp: Connect MRP API with the switchev API

On 20/04/2020 18:09, Horatiu Vultur wrote:
> Implement the MRP API.
>
> In case the HW can't generate MRP Test frames then the SW will try to generate
> the frames. In case that also the SW will fail in generating the frames then a
> error is return to the userspace. The userspace is responsible to generate all
> the other MRP frames regardless if the test frames are generated by HW or SW.
>
> The forwarding/termination of MRP frames is happening in the kernel and is done
> by the MRP instance. The userspace application doesn't do the forwarding.
>
> Signed-off-by: Horatiu Vultur <[email protected]>
> ---
> net/bridge/br_mrp.c | 551 ++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 551 insertions(+)
> create mode 100644 net/bridge/br_mrp.c
>
> diff --git a/net/bridge/br_mrp.c b/net/bridge/br_mrp.c
> new file mode 100644
> index 000000000000..a52f02f174c3
> --- /dev/null
> +++ b/net/bridge/br_mrp.c
> @@ -0,0 +1,551 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +#include <linux/mrp_bridge.h>
> +#include "br_private_mrp.h"
> +
> +static const u8 mrp_test_dmac[ETH_ALEN] = { 0x1, 0x15, 0x4e, 0x0, 0x0, 0x1 };
> +
> +static struct net_bridge_port *br_mrp_get_port(struct net_bridge *br,
> + u32 ifindex)
> +{
> + struct net_bridge_port *res = NULL;
> + struct net_bridge_port *port;
> +
> + list_for_each_entry(port, &br->port_list, list) {
> + if (port->dev->ifindex == ifindex) {
> + res = port;
> + break;
> + }
> + }
> +
> + return res;
> +}
> +
> +static struct br_mrp *br_mrp_find_id(struct net_bridge *br, u32 ring_id)
> +{
> + struct br_mrp *res = NULL;
> + struct br_mrp *mrp;
> +
> + list_for_each_entry_rcu(mrp, &br->mrp_list, list,
> + lockdep_rtnl_is_held()) {
> + if (mrp->ring_id == ring_id) {
> + res = mrp;
> + break;
> + }
> + }
> +
> + return res;
> +}
> +
> +static struct br_mrp *br_mrp_find_port(struct net_bridge *br,
> + struct net_bridge_port *p)
> +{
> + struct br_mrp *res = NULL;
> + struct br_mrp *mrp;
> +
> + list_for_each_entry_rcu(mrp, &br->mrp_list, list,
> + lockdep_rtnl_is_held()) {
> + if (rcu_access_pointer(mrp->p_port) == p ||
> + rcu_access_pointer(mrp->s_port) == p) {
> + res = mrp;
> + break;
> + }
> + }
> +
> + return res;
> +}
> +
> +static int br_mrp_next_seq(struct br_mrp *mrp)
> +{
> + mrp->seq_id++;
> + return mrp->seq_id;
> +}
> +
> +static struct sk_buff *br_mrp_skb_alloc(struct net_bridge_port *p,
> + const u8 *src, const u8 *dst)
> +{
> + struct ethhdr *eth_hdr;
> + struct sk_buff *skb;
> + u16 *version;
> +
> + skb = dev_alloc_skb(MRP_MAX_FRAME_LENGTH);
> + if (!skb)
> + return NULL;
> +
> + skb->dev = p->dev;
> + skb->protocol = htons(ETH_P_MRP);
> + skb->priority = MRP_FRAME_PRIO;
> + skb_reserve(skb, sizeof(*eth_hdr));
> +
> + eth_hdr = skb_push(skb, sizeof(*eth_hdr));
> + ether_addr_copy(eth_hdr->h_dest, dst);
> + ether_addr_copy(eth_hdr->h_source, src);
> + eth_hdr->h_proto = htons(ETH_P_MRP);
> +
> + version = skb_put(skb, sizeof(*version));
> + *version = cpu_to_be16(MRP_VERSION);
> +
> + return skb;
> +}
> +
> +static void br_mrp_skb_tlv(struct sk_buff *skb,
> + enum br_mrp_tlv_header_type type,
> + u8 length)
> +{
> + struct br_mrp_tlv_hdr *hdr;
> +
> + hdr = skb_put(skb, sizeof(*hdr));
> + hdr->type = type;
> + hdr->length = length;
> +}
> +
> +static void br_mrp_skb_common(struct sk_buff *skb, struct br_mrp *mrp)
> +{
> + struct br_mrp_common_hdr *hdr;
> +
> + br_mrp_skb_tlv(skb, BR_MRP_TLV_HEADER_COMMON, sizeof(*hdr));
> +
> + hdr = skb_put(skb, sizeof(*hdr));
> + hdr->seq_id = cpu_to_be16(br_mrp_next_seq(mrp));
> + memset(hdr->domain, 0xff, MRP_DOMAIN_UUID_LENGTH);
> +}
> +
> +static struct sk_buff *br_mrp_alloc_test_skb(struct br_mrp *mrp,
> + struct net_bridge_port *p,
> + enum br_mrp_port_role_type port_role)
> +{
> + struct br_mrp_ring_test_hdr *hdr = NULL;
> + struct net_bridge *br = p->br;
> + struct sk_buff *skb = NULL;
> +
> + if (!p)
> + return NULL;
> +
> + br = p->br;
> +
> + skb = br_mrp_skb_alloc(p, p->dev->dev_addr, mrp_test_dmac);
> + if (!skb)
> + return NULL;
> +
> + br_mrp_skb_tlv(skb, BR_MRP_TLV_HEADER_RING_TEST, sizeof(*hdr));
> + hdr = skb_put(skb, sizeof(*hdr));
> +
> + hdr->prio = cpu_to_be16(MRP_DEFAULT_PRIO);
> + ether_addr_copy(hdr->sa, p->br->dev->dev_addr);
> + hdr->port_role = cpu_to_be16(port_role);
> + hdr->state = cpu_to_be16(mrp->ring_state);
> + hdr->transitions = cpu_to_be16(mrp->ring_transitions);
> + hdr->timestamp = cpu_to_be32(jiffies_to_msecs(jiffies));
> +
> + br_mrp_skb_common(skb, mrp);
> + br_mrp_skb_tlv(skb, BR_MRP_TLV_HEADER_END, 0x0);
> +
> + return skb;
> +}
> +
> +static void br_mrp_test_work_expired(struct work_struct *work)
> +{
> + struct delayed_work *del_work = to_delayed_work(work);
> + struct br_mrp *mrp = container_of(del_work, struct br_mrp, test_work);
> + struct net_bridge_port *p;
> + bool notify_open = false;
> + struct sk_buff *skb;
> +
> + if (time_before_eq(mrp->test_end, jiffies))
> + return;
> +
> + if (mrp->test_count_miss < mrp->test_max_miss) {
> + mrp->test_count_miss++;
> + } else {
> + /* Notify that the ring is open only if the ring state is
> + * closed, otherwise it would continue to notify at every
> + * interval.
> + */
> + if (mrp->ring_state == BR_MRP_RING_STATE_CLOSED)
> + notify_open = true;
> + }
> +
> + rcu_read_lock();
> +
> + p = rcu_dereference(mrp->p_port);
> + if (p) {
> + skb = br_mrp_alloc_test_skb(mrp, p, BR_MRP_PORT_ROLE_PRIMARY);
> + if (!skb)
> + goto out;
> +
> + skb_reset_network_header(skb);
> + dev_queue_xmit(skb);
> +
> + if (notify_open && !mrp->ring_role_offloaded)
> + br_mrp_port_open(p->dev, true);
> + }
> +
> + p = rcu_dereference(mrp->s_port);
> + if (p) {
> + skb = br_mrp_alloc_test_skb(mrp, p, BR_MRP_PORT_ROLE_SECONDARY);
> + if (!skb)
> + goto out;
> +
> + skb_reset_network_header(skb);
> + dev_queue_xmit(skb);
> +
> + if (notify_open && !mrp->ring_role_offloaded)
> + br_mrp_port_open(p->dev, true);
> + }
> +
> +out:
> + rcu_read_unlock();
> +
> + queue_delayed_work(system_wq, &mrp->test_work,
> + usecs_to_jiffies(mrp->test_interval));
> +}
> +
> +/* Deletes the MRP instance.
> + * note: called under rtnl_lock
> + */
> +static void br_mrp_del_impl(struct net_bridge *br, struct br_mrp *mrp)
> +{
> + struct net_bridge_port *p;
> +
> + /* Stop sending MRP_Test frames */
> + cancel_delayed_work_sync(&mrp->test_work);
> + br_mrp_switchdev_send_ring_test(br, mrp, 0, 0, 0);
> +
> + br_mrp_switchdev_del(br, mrp);
> +
> + /* Reset the ports */
> + p = rtnl_dereference(mrp->p_port);
> + if (p) {
> + spin_lock_bh(&br->lock);
> + p->state = BR_STATE_FORWARDING;
> + p->flags &= ~BR_MRP_AWARE;
> + spin_unlock_bh(&br->lock);
> + br_mrp_port_switchdev_set_state(p, BR_STATE_FORWARDING);
> + rcu_assign_pointer(mrp->p_port, NULL);
> + }
> +
> + p = rtnl_dereference(mrp->s_port);
> + if (p) {
> + spin_lock_bh(&br->lock);
> + p->state = BR_STATE_FORWARDING;
> + p->flags &= ~BR_MRP_AWARE;
> + spin_unlock_bh(&br->lock);
> + br_mrp_port_switchdev_set_state(p, BR_STATE_FORWARDING);
> + rcu_assign_pointer(mrp->s_port, NULL);
> + }
> +
> + list_del_rcu(&mrp->list);
> + kfree_rcu(mrp, rcu);
> +}
> +
> +/* Adds a new MRP instance.
> + * note: called under rtnl_lock
> + */
> +int br_mrp_add(struct net_bridge *br, struct br_mrp_instance *instance)
> +{
> + struct net_bridge_port *p;
> + struct br_mrp *mrp;
> + int err;
> +
> + /* If the ring exists, it is not possible to create another one with the
> + * same ring_id
> + */
> + mrp = br_mrp_find_id(br, instance->ring_id);
> + if (mrp)
> + return -EINVAL;
> +
> + if (!br_mrp_get_port(br, instance->p_ifindex) ||
> + !br_mrp_get_port(br, instance->s_ifindex))
> + return -EINVAL;
> +
> + mrp = kzalloc(sizeof(*mrp), GFP_KERNEL);
> + if (!mrp)
> + return -ENOMEM;
> +
> + mrp->ring_id = instance->ring_id;
> +
> + p = br_mrp_get_port(br, instance->p_ifindex);
> + spin_lock_bh(&br->lock);
> + p->state = BR_STATE_FORWARDING;
> + p->flags |= BR_MRP_AWARE;
> + spin_unlock_bh(&br->lock);
> + rcu_assign_pointer(mrp->p_port, p);
> +
> + p = br_mrp_get_port(br, instance->s_ifindex);
> + spin_lock_bh(&br->lock);
> + p->state = BR_STATE_FORWARDING;
> + p->flags |= BR_MRP_AWARE;
> + spin_unlock_bh(&br->lock);
> + rcu_assign_pointer(mrp->s_port, p);
> +
> + INIT_DELAYED_WORK(&mrp->test_work, br_mrp_test_work_expired);
> + list_add_tail_rcu(&mrp->list, &br->mrp_list);
> +
> + err = br_mrp_switchdev_add(br, mrp);
> + if (err)
> + goto delete_mrp;
> +
> + return 0;
> +
> +delete_mrp:
> + br_mrp_del_impl(br, mrp);
> +
> + return err;
> +}
> +
> +/* Deletes the MRP instance from which the port is part of
> + * note: called under rtnl_lock
> + */
> +void br_mrp_port_del(struct net_bridge *br, struct net_bridge_port *p)
> +{
> + struct br_mrp *mrp = br_mrp_find_port(br, p);
> +
> + /* If the port is not part of a MRP instance just bail out */
> + if (!mrp)
> + return;
> +
> + br_mrp_del_impl(br, mrp);
> +}
> +
> +/* Deletes existing MRP instance based on ring_id
> + * note: called under rtnl_lock
> + */
> +int br_mrp_del(struct net_bridge *br, struct br_mrp_instance *instance)
> +{
> + struct br_mrp *mrp = br_mrp_find_id(br, instance->ring_id);
> +
> + if (!mrp)
> + return -EINVAL;
> +
> + br_mrp_del_impl(br, mrp);
> +
> + return 0;
> +}
> +
> +/* Set port state, port state can be forwarding, blocked or disabled
> + * note: already called with rcu_read_lock
> + */
> +int br_mrp_set_port_state(struct net_bridge_port *p,
> + enum br_mrp_port_state_type state)
> +{
> + spin_lock_bh(&p->br->lock);
> +
> + if (state == BR_MRP_PORT_STATE_FORWARDING)
> + p->state = BR_STATE_FORWARDING;
> + else
> + p->state = BR_STATE_BLOCKING;
> +
> + spin_unlock_bh(&p->br->lock);
> +
> + br_mrp_port_switchdev_set_state(p, state);
> +
> + return 0;
> +}
> +
> +/* Set port role, port role can be primary or secondary
> + * note: already called with rcu_read_lock
> + */
> +int br_mrp_set_port_role(struct net_bridge_port *p,
> + struct br_mrp_port_role *role)
> +{
> + struct br_mrp *mrp = br_mrp_find_id(p->br, role->ring_id);
> +
> + if (!mrp)
> + return -EINVAL;
> +
> + if (role->role == BR_MRP_PORT_ROLE_PRIMARY)
> + rcu_assign_pointer(mrp->p_port, p);
> + else
> + rcu_assign_pointer(mrp->s_port, p);
> +
> + br_mrp_port_switchdev_set_role(p, role->role);
> +
> + return 0;
> +}
> +
> +/* Set ring state, ring state can be only Open or Closed
> + * note: already called with rcu_read_lock
> + */
> +int br_mrp_set_ring_state(struct net_bridge *br,
> + struct br_mrp_ring_state *state)
> +{
> + struct br_mrp *mrp = br_mrp_find_id(br, state->ring_id);
> +
> + if (!mrp)
> + return -EINVAL;
> +
> + if (mrp->ring_state == BR_MRP_RING_STATE_CLOSED &&
> + state->ring_state != BR_MRP_RING_STATE_CLOSED)
> + mrp->ring_transitions++;
> +
> + mrp->ring_state = state->ring_state;
> +
> + br_mrp_switchdev_set_ring_state(br, mrp, state->ring_state);
> +
> + return 0;
> +}
> +
> +/* Set ring role, ring role can be only MRM(Media Redundancy Manager) or
> + * MRC(Media Redundancy Client).
> + * note: already called with rcu_read_lock
> + */
> +int br_mrp_set_ring_role(struct net_bridge *br,
> + struct br_mrp_ring_role *role)
> +{
> + struct br_mrp *mrp = br_mrp_find_id(br, role->ring_id);
> + int err;
> +
> + if (!mrp)
> + return -EINVAL;
> +
> + mrp->ring_role = role->ring_role;
> +
> + /* If there is an error just bailed out */
> + err = br_mrp_switchdev_set_ring_role(br, mrp, role->ring_role);
> + if (err && err != -EOPNOTSUPP)
> + return err;
> +
> + /* Now detect if the HW actually applied the role or not. If the HW
> + * applied the role it means that the SW will not to do those operations
> + * anymore. For example if the role ir MRM then the HW will notify the
> + * SW when ring is open, but if the is not pushed to the HW the SW will
> + * need to detect when the ring is open
> + */
> + mrp->ring_role_offloaded = err == -EOPNOTSUPP ? 0 : 1;
> +
> + return 0;
> +}
> +
> +/* Start to generate MRP test frames, the frames are generated by HW and if it
> + * fails, they are generated by the SW.
> + * note: already called with rcu_read_lock
> + */
> +int br_mrp_start_test(struct net_bridge *br,
> + struct br_mrp_start_test *test)
> +{
> + struct br_mrp *mrp = br_mrp_find_id(br, test->ring_id);
> +
> + if (!mrp)
> + return -EINVAL;
> +
> + /* Try to push is to the HW and if it fails then continue to generate in
> + * SW and if that also fails then return error
> + */
> + if (!br_mrp_switchdev_send_ring_test(br, mrp, test->interval,
> + test->max_miss, test->period))
> + return 0;
> +
> + mrp->test_interval = test->interval;
> + mrp->test_end = jiffies + usecs_to_jiffies(test->period);
> + mrp->test_max_miss = test->max_miss;
> + mrp->test_count_miss = 0;
> + queue_delayed_work(system_wq, &mrp->test_work,
> + usecs_to_jiffies(test->interval));
> +
> + return 0;
> +}
> +
> +/* Process only MRP Test frame. All the other MRP frames are processed by
> + * userspace application
> + * note: already called with rcu_read_lock
> + */
> +static void br_mrp_mrm_process(struct br_mrp *mrp, struct net_bridge_port *port,
> + struct sk_buff *skb)
> +{
> + struct br_mrp_tlv_hdr *hdr;
> +
> + hdr = (struct br_mrp_tlv_hdr *)(skb->data + sizeof(uint16_t));
> +
> + if (!hdr)
> + return;
> +
> + if (hdr->type != BR_MRP_TLV_HEADER_RING_TEST)
> + return;
> +
> + mrp->test_count_miss = 0;
> +
> + /* Notify the userspace that the ring is closed only when the ring is
> + * not closed
> + */
> + if (mrp->ring_state != BR_MRP_RING_STATE_CLOSED)
> + br_mrp_port_open(port->dev, false);

This function is defined in the next patch. The only reason this compiles is because
you update the Makefile in patch 11 which hides such errors. Please update the Makefile
in the respective patches where these new files are added so they can be compiled, and
such things can be caught automatically.

> +}
> +
> +/* This will just forward the frame to the other mrp ring port(MRC role) or will
> + * not do anything.
> + * note: already called with rcu_read_lock
> + */
> +static int br_mrp_rcv(struct net_bridge_port *p,
> + struct sk_buff *skb, struct net_device *dev)
> +{
> + struct net_device *s_dev, *p_dev, *d_dev;
> + struct net_bridge_port *p_port, *s_port;
> + struct net_bridge *br;
> + struct sk_buff *nskb;
> + struct br_mrp *mrp;
> +
> + /* If port is disabled don't accept any frames */
> + if (p->state == BR_STATE_DISABLED)
> + return 0;
> +
> + br = p->br;
> + mrp = br_mrp_find_port(br, p);
> + if (unlikely(!mrp))
> + return 0;
> +
> + p_port = rcu_dereference(mrp->p_port);
> + if (!p_port)
> + return 0;
> +
> + s_port = rcu_dereference(mrp->s_port);
> + if (!s_port)
> + return 0;
> +
> + /* If the role is MRM then don't forward the frames */
> + if (mrp->ring_role == BR_MRP_RING_ROLE_MRM) {
> + br_mrp_mrm_process(mrp, p, skb);
> + return 1;
> + }
> +
> + /* Clone the frame and forward it on the other MRP port */
> + nskb = skb_clone(skb, GFP_ATOMIC);
> + if (!nskb)
> + return 0;
> +
> + p_dev = p_port->dev;
> + s_dev = s_port->dev;
> +
> + if (p_dev == dev)
> + d_dev = s_dev;
> + else
> + d_dev = p_dev;
> +
> + nskb->dev = d_dev;
> + skb_push(nskb, ETH_HLEN);
> + dev_queue_xmit(nskb);
> +
> + return 1;
> +}
> +
> +/* Check if the frame was received on a port that is part of MRP ring
> + * and if the frame has MRP eth. In that case process the frame otherwise do
> + * normal forwarding.
> + * note: already called with rcu_read_lock
> + */
> +int br_mrp_process(struct net_bridge_port *p, struct sk_buff *skb)
> +{
> + /* If there is no MRP instance do normal forwarding */
> + if (likely(!(p->flags & BR_MRP_AWARE)))
> + goto out;
> +
> + if (unlikely(skb->protocol == htons(ETH_P_MRP)))
> + return br_mrp_rcv(p, skb, p->dev);
> +
> +out:
> + return 0;
> +}
> +
> +bool br_mrp_enabled(struct net_bridge *br)
> +{
> + return !list_empty(&br->mrp_list);
> +}
>

2020-04-20 17:20:12

by Nikolay Aleksandrov

[permalink] [raw]
Subject: Re: [PATCH net-next 10/13] bridge: mrp: Implement netlink interface to configure MRP

On 20/04/2020 18:09, Horatiu Vultur wrote:
> Implement netlink interface to configure MRP. The implementation
> will do sanity checks over the attributes and then eventually call the MRP
> interface.
>
> Signed-off-by: Horatiu Vultur <[email protected]>
> ---
> net/bridge/br_mrp_netlink.c | 117 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 117 insertions(+)
> create mode 100644 net/bridge/br_mrp_netlink.c
>
> diff --git a/net/bridge/br_mrp_netlink.c b/net/bridge/br_mrp_netlink.c
> new file mode 100644
> index 000000000000..0ff42e7c7f57
> --- /dev/null
> +++ b/net/bridge/br_mrp_netlink.c
> @@ -0,0 +1,117 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +#include <net/genetlink.h>
> +
> +#include <uapi/linux/mrp_bridge.h>
> +#include "br_private.h"
> +#include "br_private_mrp.h"
> +
> +static const struct nla_policy br_mrp_policy[IFLA_BRIDGE_MRP_MAX + 1] = {
> + [IFLA_BRIDGE_MRP_UNSPEC] = { .type = NLA_REJECT },
> + [IFLA_BRIDGE_MRP_INSTANCE] = { .type = NLA_EXACT_LEN,
> + .len = sizeof(struct br_mrp_instance)},
> + [IFLA_BRIDGE_MRP_PORT_STATE] = { .type = NLA_U32 },
> + [IFLA_BRIDGE_MRP_PORT_ROLE] = { .type = NLA_EXACT_LEN,
> + .len = sizeof(struct br_mrp_port_role)},
> + [IFLA_BRIDGE_MRP_RING_STATE] = { .type = NLA_EXACT_LEN,
> + .len = sizeof(struct br_mrp_ring_state)},
> + [IFLA_BRIDGE_MRP_RING_ROLE] = { .type = NLA_EXACT_LEN,
> + .len = sizeof(struct br_mrp_ring_role)},
> + [IFLA_BRIDGE_MRP_START_TEST] = { .type = NLA_EXACT_LEN,
> + .len = sizeof(struct br_mrp_start_test)},
> +};
> +
> +int br_mrp_parse(struct net_bridge *br, struct net_bridge_port *p,
> + struct nlattr *attr, int cmd, struct netlink_ext_ack *extack)
> +{
> + struct nlattr *tb[IFLA_BRIDGE_MRP_MAX + 1];
> + int err;
> +
> + if (br->stp_enabled != BR_NO_STP) {
> + NL_SET_ERR_MSG_MOD(extack, "MRP can't be enabled if STP is already enabled\n");
> + return -EINVAL;
> + }
> +
> + err = nla_parse_nested(tb, IFLA_BRIDGE_MRP_MAX, attr,
> + NULL, extack);
> + if (err)
> + return err;
> +
> + if (tb[IFLA_BRIDGE_MRP_INSTANCE]) {
> + struct br_mrp_instance *instance =
> + nla_data(tb[IFLA_BRIDGE_MRP_INSTANCE]);
> +
> + if (cmd == RTM_SETLINK)
> + err = br_mrp_add(br, instance);
> + else
> + err = br_mrp_del(br, instance);
> + if (err)
> + return err;
> + }
> +
> + if (tb[IFLA_BRIDGE_MRP_PORT_STATE]) {
> + enum br_mrp_port_state_type state =
> + nla_get_u32(tb[IFLA_BRIDGE_MRP_PORT_STATE]);
> +
> + err = br_mrp_set_port_state(p, state);
> + if (err)
> + return err;
> + }
> +
> + if (tb[IFLA_BRIDGE_MRP_PORT_ROLE]) {
> + struct br_mrp_port_role *role =
> + nla_data(tb[IFLA_BRIDGE_MRP_PORT_ROLE]);
> +
> + err = br_mrp_set_port_role(p, role);
> + if (err)
> + return err;
> + }
> +
> + if (tb[IFLA_BRIDGE_MRP_RING_STATE]) {
> + struct br_mrp_ring_state *state =
> + nla_data(tb[IFLA_BRIDGE_MRP_RING_STATE]);
> +
> + err = br_mrp_set_ring_state(br, state);
> + if (err)
> + return err;
> + }
> +
> + if (tb[IFLA_BRIDGE_MRP_RING_ROLE]) {
> + struct br_mrp_ring_role *role =
> + nla_data(tb[IFLA_BRIDGE_MRP_RING_ROLE]);
> +
> + err = br_mrp_set_ring_role(br, role);
> + if (err)
> + return err;
> + }
> +
> + if (tb[IFLA_BRIDGE_MRP_START_TEST]) {
> + struct br_mrp_start_test *test =
> + nla_data(tb[IFLA_BRIDGE_MRP_START_TEST]);
> +
> + err = br_mrp_start_test(br, test);
> + if (err)
> + return err;
> + }
> +
> + return 0;
> +}
> +
> +int br_mrp_port_open(struct net_device *dev, u8 loc)
> +{
> + struct net_bridge_port *p;
> + int err = 0;
> +
> + p = br_port_get_rcu(dev);
> + if (!p) {
> + err = -EINVAL;
> + goto out;
> + }
> +
> + p->loc = loc;
> + br_ifinfo_notify(RTM_NEWLINK, NULL, p);
> +
> +out:
> + return err;
> +}
> +EXPORT_SYMBOL(br_mrp_port_open);
>

I just noticed the EXPORT_SYMBOL() here, why do you need it?

2020-04-20 18:55:08

by Horatiu Vultur

[permalink] [raw]
Subject: [PATCH net-next 04/13] bridge: mrp: Expose function br_mrp_port_open

In case the HW is capable to detect when the MRP ring is open or closed. It is
expected that the network driver will notify the SW that the ring is open or
closed.

The function br_mrp_port_open is used to notify the kernel that one of the ports
stopped receiving MRP_Test frames. The argument 'loc' has a value of '1' when
the port stopped receiving MRP_Test and '0' when it started to receive MRP_Test.

Signed-off-by: Horatiu Vultur <[email protected]>
---
include/linux/mrp_bridge.h | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
create mode 100644 include/linux/mrp_bridge.h

diff --git a/include/linux/mrp_bridge.h b/include/linux/mrp_bridge.h
new file mode 100644
index 000000000000..61c1f0d395c7
--- /dev/null
+++ b/include/linux/mrp_bridge.h
@@ -0,0 +1,27 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#ifndef _LINUX_MRP_BRIDGE_H
+#define _LINUX_MRO_BRIDGE_H
+
+#include <linux/netdevice.h>
+
+/* The drivers are responsible to call this function when it detects that the
+ * MRP port stopped receiving MRP_Test frames or it started to receive MRP_Test.
+ * The argument dev represents the port and loc(Lost of Continuity) has a value
+ * of 1 when it stopped receiving MRP_Test frames and a value of 0 when it
+ * started to receive frames.
+ * Needs to be called with rcu_read_lock().
+ *
+ * This eventually notify the userspace which is required to react on these
+ * changes.
+ */
+
+#if IS_ENABLED(CONFIG_BRIDGE_MRP)
+int br_mrp_port_open(struct net_device *dev, u8 loc);
+#else
+static inline int br_mrp_port_open(struct net_device *dev, u8 loc)
+{
+}
+#endif
+
+#endif
--
2.17.1

2020-04-20 19:16:29

by Horatiu Vultur

[permalink] [raw]
Subject: Re: [PATCH net-next 09/13] bridge: mrp: Connect MRP API with the switchev API

The 04/20/2020 20:17, Nikolay Aleksandrov wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> On 20/04/2020 18:09, Horatiu Vultur wrote:
> > Implement the MRP API.
> >
> > In case the HW can't generate MRP Test frames then the SW will try to generate
> > the frames. In case that also the SW will fail in generating the frames then a
> > error is return to the userspace. The userspace is responsible to generate all
> > the other MRP frames regardless if the test frames are generated by HW or SW.
> >
> > The forwarding/termination of MRP frames is happening in the kernel and is done
> > by the MRP instance. The userspace application doesn't do the forwarding.
> >
> > Signed-off-by: Horatiu Vultur <[email protected]>
> > ---
> > net/bridge/br_mrp.c | 551 ++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 551 insertions(+)
> > create mode 100644 net/bridge/br_mrp.c
> >
> > diff --git a/net/bridge/br_mrp.c b/net/bridge/br_mrp.c
> > new file mode 100644
> > index 000000000000..a52f02f174c3
> > --- /dev/null
> > +++ b/net/bridge/br_mrp.c
> > @@ -0,0 +1,551 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +
> > +#include <linux/mrp_bridge.h>
> > +#include "br_private_mrp.h"
> > +
> > +static const u8 mrp_test_dmac[ETH_ALEN] = { 0x1, 0x15, 0x4e, 0x0, 0x0, 0x1 };
> > +
> > +static struct net_bridge_port *br_mrp_get_port(struct net_bridge *br,
> > + u32 ifindex)
> > +{
> > + struct net_bridge_port *res = NULL;
> > + struct net_bridge_port *port;
> > +
> > + list_for_each_entry(port, &br->port_list, list) {
> > + if (port->dev->ifindex == ifindex) {
> > + res = port;
> > + break;
> > + }
> > + }
> > +
> > + return res;
> > +}
> > +
> > +static struct br_mrp *br_mrp_find_id(struct net_bridge *br, u32 ring_id)
> > +{
> > + struct br_mrp *res = NULL;
> > + struct br_mrp *mrp;
> > +
> > + list_for_each_entry_rcu(mrp, &br->mrp_list, list,
> > + lockdep_rtnl_is_held()) {
> > + if (mrp->ring_id == ring_id) {
> > + res = mrp;
> > + break;
> > + }
> > + }
> > +
> > + return res;
> > +}
> > +
> > +static struct br_mrp *br_mrp_find_port(struct net_bridge *br,
> > + struct net_bridge_port *p)
> > +{
> > + struct br_mrp *res = NULL;
> > + struct br_mrp *mrp;
> > +
> > + list_for_each_entry_rcu(mrp, &br->mrp_list, list,
> > + lockdep_rtnl_is_held()) {
> > + if (rcu_access_pointer(mrp->p_port) == p ||
> > + rcu_access_pointer(mrp->s_port) == p) {
> > + res = mrp;
> > + break;
> > + }
> > + }
> > +
> > + return res;
> > +}
> > +
> > +static int br_mrp_next_seq(struct br_mrp *mrp)
> > +{
> > + mrp->seq_id++;
> > + return mrp->seq_id;
> > +}
> > +
> > +static struct sk_buff *br_mrp_skb_alloc(struct net_bridge_port *p,
> > + const u8 *src, const u8 *dst)
> > +{
> > + struct ethhdr *eth_hdr;
> > + struct sk_buff *skb;
> > + u16 *version;
> > +
> > + skb = dev_alloc_skb(MRP_MAX_FRAME_LENGTH);
> > + if (!skb)
> > + return NULL;
> > +
> > + skb->dev = p->dev;
> > + skb->protocol = htons(ETH_P_MRP);
> > + skb->priority = MRP_FRAME_PRIO;
> > + skb_reserve(skb, sizeof(*eth_hdr));
> > +
> > + eth_hdr = skb_push(skb, sizeof(*eth_hdr));
> > + ether_addr_copy(eth_hdr->h_dest, dst);
> > + ether_addr_copy(eth_hdr->h_source, src);
> > + eth_hdr->h_proto = htons(ETH_P_MRP);
> > +
> > + version = skb_put(skb, sizeof(*version));
> > + *version = cpu_to_be16(MRP_VERSION);
> > +
> > + return skb;
> > +}
> > +
> > +static void br_mrp_skb_tlv(struct sk_buff *skb,
> > + enum br_mrp_tlv_header_type type,
> > + u8 length)
> > +{
> > + struct br_mrp_tlv_hdr *hdr;
> > +
> > + hdr = skb_put(skb, sizeof(*hdr));
> > + hdr->type = type;
> > + hdr->length = length;
> > +}
> > +
> > +static void br_mrp_skb_common(struct sk_buff *skb, struct br_mrp *mrp)
> > +{
> > + struct br_mrp_common_hdr *hdr;
> > +
> > + br_mrp_skb_tlv(skb, BR_MRP_TLV_HEADER_COMMON, sizeof(*hdr));
> > +
> > + hdr = skb_put(skb, sizeof(*hdr));
> > + hdr->seq_id = cpu_to_be16(br_mrp_next_seq(mrp));
> > + memset(hdr->domain, 0xff, MRP_DOMAIN_UUID_LENGTH);
> > +}
> > +
> > +static struct sk_buff *br_mrp_alloc_test_skb(struct br_mrp *mrp,
> > + struct net_bridge_port *p,
> > + enum br_mrp_port_role_type port_role)
> > +{
> > + struct br_mrp_ring_test_hdr *hdr = NULL;
> > + struct net_bridge *br = p->br;
> > + struct sk_buff *skb = NULL;
> > +
> > + if (!p)
> > + return NULL;
> > +
> > + br = p->br;
> > +
> > + skb = br_mrp_skb_alloc(p, p->dev->dev_addr, mrp_test_dmac);
> > + if (!skb)
> > + return NULL;
> > +
> > + br_mrp_skb_tlv(skb, BR_MRP_TLV_HEADER_RING_TEST, sizeof(*hdr));
> > + hdr = skb_put(skb, sizeof(*hdr));
> > +
> > + hdr->prio = cpu_to_be16(MRP_DEFAULT_PRIO);
> > + ether_addr_copy(hdr->sa, p->br->dev->dev_addr);
> > + hdr->port_role = cpu_to_be16(port_role);
> > + hdr->state = cpu_to_be16(mrp->ring_state);
> > + hdr->transitions = cpu_to_be16(mrp->ring_transitions);
> > + hdr->timestamp = cpu_to_be32(jiffies_to_msecs(jiffies));
> > +
> > + br_mrp_skb_common(skb, mrp);
> > + br_mrp_skb_tlv(skb, BR_MRP_TLV_HEADER_END, 0x0);
> > +
> > + return skb;
> > +}
> > +
> > +static void br_mrp_test_work_expired(struct work_struct *work)
> > +{
> > + struct delayed_work *del_work = to_delayed_work(work);
> > + struct br_mrp *mrp = container_of(del_work, struct br_mrp, test_work);
> > + struct net_bridge_port *p;
> > + bool notify_open = false;
> > + struct sk_buff *skb;
> > +
> > + if (time_before_eq(mrp->test_end, jiffies))
> > + return;
> > +
> > + if (mrp->test_count_miss < mrp->test_max_miss) {
> > + mrp->test_count_miss++;
> > + } else {
> > + /* Notify that the ring is open only if the ring state is
> > + * closed, otherwise it would continue to notify at every
> > + * interval.
> > + */
> > + if (mrp->ring_state == BR_MRP_RING_STATE_CLOSED)
> > + notify_open = true;
> > + }
> > +
> > + rcu_read_lock();
> > +
> > + p = rcu_dereference(mrp->p_port);
> > + if (p) {
> > + skb = br_mrp_alloc_test_skb(mrp, p, BR_MRP_PORT_ROLE_PRIMARY);
> > + if (!skb)
> > + goto out;
> > +
> > + skb_reset_network_header(skb);
> > + dev_queue_xmit(skb);
> > +
> > + if (notify_open && !mrp->ring_role_offloaded)
> > + br_mrp_port_open(p->dev, true);
> > + }
> > +
> > + p = rcu_dereference(mrp->s_port);
> > + if (p) {
> > + skb = br_mrp_alloc_test_skb(mrp, p, BR_MRP_PORT_ROLE_SECONDARY);
> > + if (!skb)
> > + goto out;
> > +
> > + skb_reset_network_header(skb);
> > + dev_queue_xmit(skb);
> > +
> > + if (notify_open && !mrp->ring_role_offloaded)
> > + br_mrp_port_open(p->dev, true);
> > + }
> > +
> > +out:
> > + rcu_read_unlock();
> > +
> > + queue_delayed_work(system_wq, &mrp->test_work,
> > + usecs_to_jiffies(mrp->test_interval));
> > +}
> > +
> > +/* Deletes the MRP instance.
> > + * note: called under rtnl_lock
> > + */
> > +static void br_mrp_del_impl(struct net_bridge *br, struct br_mrp *mrp)
> > +{
> > + struct net_bridge_port *p;
> > +
> > + /* Stop sending MRP_Test frames */
> > + cancel_delayed_work_sync(&mrp->test_work);
> > + br_mrp_switchdev_send_ring_test(br, mrp, 0, 0, 0);
> > +
> > + br_mrp_switchdev_del(br, mrp);
> > +
> > + /* Reset the ports */
> > + p = rtnl_dereference(mrp->p_port);
> > + if (p) {
> > + spin_lock_bh(&br->lock);
> > + p->state = BR_STATE_FORWARDING;
> > + p->flags &= ~BR_MRP_AWARE;
> > + spin_unlock_bh(&br->lock);
> > + br_mrp_port_switchdev_set_state(p, BR_STATE_FORWARDING);
> > + rcu_assign_pointer(mrp->p_port, NULL);
> > + }
> > +
> > + p = rtnl_dereference(mrp->s_port);
> > + if (p) {
> > + spin_lock_bh(&br->lock);
> > + p->state = BR_STATE_FORWARDING;
> > + p->flags &= ~BR_MRP_AWARE;
> > + spin_unlock_bh(&br->lock);
> > + br_mrp_port_switchdev_set_state(p, BR_STATE_FORWARDING);
> > + rcu_assign_pointer(mrp->s_port, NULL);
> > + }
> > +
> > + list_del_rcu(&mrp->list);
> > + kfree_rcu(mrp, rcu);
> > +}
> > +
> > +/* Adds a new MRP instance.
> > + * note: called under rtnl_lock
> > + */
> > +int br_mrp_add(struct net_bridge *br, struct br_mrp_instance *instance)
> > +{
> > + struct net_bridge_port *p;
> > + struct br_mrp *mrp;
> > + int err;
> > +
> > + /* If the ring exists, it is not possible to create another one with the
> > + * same ring_id
> > + */
> > + mrp = br_mrp_find_id(br, instance->ring_id);
> > + if (mrp)
> > + return -EINVAL;
> > +
> > + if (!br_mrp_get_port(br, instance->p_ifindex) ||
> > + !br_mrp_get_port(br, instance->s_ifindex))
> > + return -EINVAL;
> > +
> > + mrp = kzalloc(sizeof(*mrp), GFP_KERNEL);
> > + if (!mrp)
> > + return -ENOMEM;
> > +
> > + mrp->ring_id = instance->ring_id;
> > +
> > + p = br_mrp_get_port(br, instance->p_ifindex);
> > + spin_lock_bh(&br->lock);
> > + p->state = BR_STATE_FORWARDING;
> > + p->flags |= BR_MRP_AWARE;
> > + spin_unlock_bh(&br->lock);
> > + rcu_assign_pointer(mrp->p_port, p);
> > +
> > + p = br_mrp_get_port(br, instance->s_ifindex);
> > + spin_lock_bh(&br->lock);
> > + p->state = BR_STATE_FORWARDING;
> > + p->flags |= BR_MRP_AWARE;
> > + spin_unlock_bh(&br->lock);
> > + rcu_assign_pointer(mrp->s_port, p);
> > +
> > + INIT_DELAYED_WORK(&mrp->test_work, br_mrp_test_work_expired);
> > + list_add_tail_rcu(&mrp->list, &br->mrp_list);
> > +
> > + err = br_mrp_switchdev_add(br, mrp);
> > + if (err)
> > + goto delete_mrp;
> > +
> > + return 0;
> > +
> > +delete_mrp:
> > + br_mrp_del_impl(br, mrp);
> > +
> > + return err;
> > +}
> > +
> > +/* Deletes the MRP instance from which the port is part of
> > + * note: called under rtnl_lock
> > + */
> > +void br_mrp_port_del(struct net_bridge *br, struct net_bridge_port *p)
> > +{
> > + struct br_mrp *mrp = br_mrp_find_port(br, p);
> > +
> > + /* If the port is not part of a MRP instance just bail out */
> > + if (!mrp)
> > + return;
> > +
> > + br_mrp_del_impl(br, mrp);
> > +}
> > +
> > +/* Deletes existing MRP instance based on ring_id
> > + * note: called under rtnl_lock
> > + */
> > +int br_mrp_del(struct net_bridge *br, struct br_mrp_instance *instance)
> > +{
> > + struct br_mrp *mrp = br_mrp_find_id(br, instance->ring_id);
> > +
> > + if (!mrp)
> > + return -EINVAL;
> > +
> > + br_mrp_del_impl(br, mrp);
> > +
> > + return 0;
> > +}
> > +
> > +/* Set port state, port state can be forwarding, blocked or disabled
> > + * note: already called with rcu_read_lock
> > + */
> > +int br_mrp_set_port_state(struct net_bridge_port *p,
> > + enum br_mrp_port_state_type state)
> > +{
> > + spin_lock_bh(&p->br->lock);
> > +
> > + if (state == BR_MRP_PORT_STATE_FORWARDING)
> > + p->state = BR_STATE_FORWARDING;
> > + else
> > + p->state = BR_STATE_BLOCKING;
> > +
> > + spin_unlock_bh(&p->br->lock);
> > +
> > + br_mrp_port_switchdev_set_state(p, state);
> > +
> > + return 0;
> > +}
> > +
> > +/* Set port role, port role can be primary or secondary
> > + * note: already called with rcu_read_lock
> > + */
> > +int br_mrp_set_port_role(struct net_bridge_port *p,
> > + struct br_mrp_port_role *role)
> > +{
> > + struct br_mrp *mrp = br_mrp_find_id(p->br, role->ring_id);
> > +
> > + if (!mrp)
> > + return -EINVAL;
> > +
> > + if (role->role == BR_MRP_PORT_ROLE_PRIMARY)
> > + rcu_assign_pointer(mrp->p_port, p);
> > + else
> > + rcu_assign_pointer(mrp->s_port, p);
> > +
> > + br_mrp_port_switchdev_set_role(p, role->role);
> > +
> > + return 0;
> > +}
> > +
> > +/* Set ring state, ring state can be only Open or Closed
> > + * note: already called with rcu_read_lock
> > + */
> > +int br_mrp_set_ring_state(struct net_bridge *br,
> > + struct br_mrp_ring_state *state)
> > +{
> > + struct br_mrp *mrp = br_mrp_find_id(br, state->ring_id);
> > +
> > + if (!mrp)
> > + return -EINVAL;
> > +
> > + if (mrp->ring_state == BR_MRP_RING_STATE_CLOSED &&
> > + state->ring_state != BR_MRP_RING_STATE_CLOSED)
> > + mrp->ring_transitions++;
> > +
> > + mrp->ring_state = state->ring_state;
> > +
> > + br_mrp_switchdev_set_ring_state(br, mrp, state->ring_state);
> > +
> > + return 0;
> > +}
> > +
> > +/* Set ring role, ring role can be only MRM(Media Redundancy Manager) or
> > + * MRC(Media Redundancy Client).
> > + * note: already called with rcu_read_lock
> > + */
> > +int br_mrp_set_ring_role(struct net_bridge *br,
> > + struct br_mrp_ring_role *role)
> > +{
> > + struct br_mrp *mrp = br_mrp_find_id(br, role->ring_id);
> > + int err;
> > +
> > + if (!mrp)
> > + return -EINVAL;
> > +
> > + mrp->ring_role = role->ring_role;
> > +
> > + /* If there is an error just bailed out */
> > + err = br_mrp_switchdev_set_ring_role(br, mrp, role->ring_role);
> > + if (err && err != -EOPNOTSUPP)
> > + return err;
> > +
> > + /* Now detect if the HW actually applied the role or not. If the HW
> > + * applied the role it means that the SW will not to do those operations
> > + * anymore. For example if the role ir MRM then the HW will notify the
> > + * SW when ring is open, but if the is not pushed to the HW the SW will
> > + * need to detect when the ring is open
> > + */
> > + mrp->ring_role_offloaded = err == -EOPNOTSUPP ? 0 : 1;
> > +
> > + return 0;
> > +}
> > +
> > +/* Start to generate MRP test frames, the frames are generated by HW and if it
> > + * fails, they are generated by the SW.
> > + * note: already called with rcu_read_lock
> > + */
> > +int br_mrp_start_test(struct net_bridge *br,
> > + struct br_mrp_start_test *test)
> > +{
> > + struct br_mrp *mrp = br_mrp_find_id(br, test->ring_id);
> > +
> > + if (!mrp)
> > + return -EINVAL;
> > +
> > + /* Try to push is to the HW and if it fails then continue to generate in
> > + * SW and if that also fails then return error
> > + */
> > + if (!br_mrp_switchdev_send_ring_test(br, mrp, test->interval,
> > + test->max_miss, test->period))
> > + return 0;
> > +
> > + mrp->test_interval = test->interval;
> > + mrp->test_end = jiffies + usecs_to_jiffies(test->period);
> > + mrp->test_max_miss = test->max_miss;
> > + mrp->test_count_miss = 0;
> > + queue_delayed_work(system_wq, &mrp->test_work,
> > + usecs_to_jiffies(test->interval));
> > +
> > + return 0;
> > +}
> > +
> > +/* Process only MRP Test frame. All the other MRP frames are processed by
> > + * userspace application
> > + * note: already called with rcu_read_lock
> > + */
> > +static void br_mrp_mrm_process(struct br_mrp *mrp, struct net_bridge_port *port,
> > + struct sk_buff *skb)
> > +{
> > + struct br_mrp_tlv_hdr *hdr;
> > +
> > + hdr = (struct br_mrp_tlv_hdr *)(skb->data + sizeof(uint16_t));
> > +
> > + if (!hdr)
> > + return;
> > +
> > + if (hdr->type != BR_MRP_TLV_HEADER_RING_TEST)
> > + return;
> > +
> > + mrp->test_count_miss = 0;
> > +
> > + /* Notify the userspace that the ring is closed only when the ring is
> > + * not closed
> > + */
> > + if (mrp->ring_state != BR_MRP_RING_STATE_CLOSED)
> > + br_mrp_port_open(port->dev, false);
>
> This function is defined in the next patch. The only reason this compiles is because
> you update the Makefile in patch 11 which hides such errors. Please update the Makefile
> in the respective patches where these new files are added so they can be compiled, and
> such things can be caught automatically.

Yes, I will update this in next patch series.

>
> > +}
> > +
> > +/* This will just forward the frame to the other mrp ring port(MRC role) or will
> > + * not do anything.
> > + * note: already called with rcu_read_lock
> > + */
> > +static int br_mrp_rcv(struct net_bridge_port *p,
> > + struct sk_buff *skb, struct net_device *dev)
> > +{
> > + struct net_device *s_dev, *p_dev, *d_dev;
> > + struct net_bridge_port *p_port, *s_port;
> > + struct net_bridge *br;
> > + struct sk_buff *nskb;
> > + struct br_mrp *mrp;
> > +
> > + /* If port is disabled don't accept any frames */
> > + if (p->state == BR_STATE_DISABLED)
> > + return 0;
> > +
> > + br = p->br;
> > + mrp = br_mrp_find_port(br, p);
> > + if (unlikely(!mrp))
> > + return 0;
> > +
> > + p_port = rcu_dereference(mrp->p_port);
> > + if (!p_port)
> > + return 0;
> > +
> > + s_port = rcu_dereference(mrp->s_port);
> > + if (!s_port)
> > + return 0;
> > +
> > + /* If the role is MRM then don't forward the frames */
> > + if (mrp->ring_role == BR_MRP_RING_ROLE_MRM) {
> > + br_mrp_mrm_process(mrp, p, skb);
> > + return 1;
> > + }
> > +
> > + /* Clone the frame and forward it on the other MRP port */
> > + nskb = skb_clone(skb, GFP_ATOMIC);
> > + if (!nskb)
> > + return 0;
> > +
> > + p_dev = p_port->dev;
> > + s_dev = s_port->dev;
> > +
> > + if (p_dev == dev)
> > + d_dev = s_dev;
> > + else
> > + d_dev = p_dev;
> > +
> > + nskb->dev = d_dev;
> > + skb_push(nskb, ETH_HLEN);
> > + dev_queue_xmit(nskb);
> > +
> > + return 1;
> > +}
> > +
> > +/* Check if the frame was received on a port that is part of MRP ring
> > + * and if the frame has MRP eth. In that case process the frame otherwise do
> > + * normal forwarding.
> > + * note: already called with rcu_read_lock
> > + */
> > +int br_mrp_process(struct net_bridge_port *p, struct sk_buff *skb)
> > +{
> > + /* If there is no MRP instance do normal forwarding */
> > + if (likely(!(p->flags & BR_MRP_AWARE)))
> > + goto out;
> > +
> > + if (unlikely(skb->protocol == htons(ETH_P_MRP)))
> > + return br_mrp_rcv(p, skb, p->dev);
> > +
> > +out:
> > + return 0;
> > +}
> > +
> > +bool br_mrp_enabled(struct net_bridge *br)
> > +{
> > + return !list_empty(&br->mrp_list);
> > +}
> >
>

--
/Horatiu

2020-04-20 19:20:06

by Horatiu Vultur

[permalink] [raw]
Subject: Re: [PATCH net-next 10/13] bridge: mrp: Implement netlink interface to configure MRP

The 04/20/2020 20:18, Nikolay Aleksandrov wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> On 20/04/2020 18:09, Horatiu Vultur wrote:
> > Implement netlink interface to configure MRP. The implementation
> > will do sanity checks over the attributes and then eventually call the MRP
> > interface.
> >
> > Signed-off-by: Horatiu Vultur <[email protected]>
> > ---
> > net/bridge/br_mrp_netlink.c | 117 ++++++++++++++++++++++++++++++++++++
> > 1 file changed, 117 insertions(+)
> > create mode 100644 net/bridge/br_mrp_netlink.c
> >
> > diff --git a/net/bridge/br_mrp_netlink.c b/net/bridge/br_mrp_netlink.c
> > new file mode 100644
> > index 000000000000..0ff42e7c7f57
> > --- /dev/null
> > +++ b/net/bridge/br_mrp_netlink.c
> > @@ -0,0 +1,117 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +
> > +#include <net/genetlink.h>
> > +
> > +#include <uapi/linux/mrp_bridge.h>
> > +#include "br_private.h"
> > +#include "br_private_mrp.h"
> > +
> > +static const struct nla_policy br_mrp_policy[IFLA_BRIDGE_MRP_MAX + 1] = {
> > + [IFLA_BRIDGE_MRP_UNSPEC] = { .type = NLA_REJECT },
> > + [IFLA_BRIDGE_MRP_INSTANCE] = { .type = NLA_EXACT_LEN,
> > + .len = sizeof(struct br_mrp_instance)},
> > + [IFLA_BRIDGE_MRP_PORT_STATE] = { .type = NLA_U32 },
> > + [IFLA_BRIDGE_MRP_PORT_ROLE] = { .type = NLA_EXACT_LEN,
> > + .len = sizeof(struct br_mrp_port_role)},
> > + [IFLA_BRIDGE_MRP_RING_STATE] = { .type = NLA_EXACT_LEN,
> > + .len = sizeof(struct br_mrp_ring_state)},
> > + [IFLA_BRIDGE_MRP_RING_ROLE] = { .type = NLA_EXACT_LEN,
> > + .len = sizeof(struct br_mrp_ring_role)},
> > + [IFLA_BRIDGE_MRP_START_TEST] = { .type = NLA_EXACT_LEN,
> > + .len = sizeof(struct br_mrp_start_test)},
> > +};
> > +
> > +int br_mrp_parse(struct net_bridge *br, struct net_bridge_port *p,
> > + struct nlattr *attr, int cmd, struct netlink_ext_ack *extack)
> > +{
> > + struct nlattr *tb[IFLA_BRIDGE_MRP_MAX + 1];
> > + int err;
> > +
> > + if (br->stp_enabled != BR_NO_STP) {
> > + NL_SET_ERR_MSG_MOD(extack, "MRP can't be enabled if STP is already enabled\n");
> > + return -EINVAL;
> > + }
> > +
> > + err = nla_parse_nested(tb, IFLA_BRIDGE_MRP_MAX, attr,
> > + NULL, extack);
> > + if (err)
> > + return err;
> > +
> > + if (tb[IFLA_BRIDGE_MRP_INSTANCE]) {
> > + struct br_mrp_instance *instance =
> > + nla_data(tb[IFLA_BRIDGE_MRP_INSTANCE]);
> > +
> > + if (cmd == RTM_SETLINK)
> > + err = br_mrp_add(br, instance);
> > + else
> > + err = br_mrp_del(br, instance);
> > + if (err)
> > + return err;
> > + }
> > +
> > + if (tb[IFLA_BRIDGE_MRP_PORT_STATE]) {
> > + enum br_mrp_port_state_type state =
> > + nla_get_u32(tb[IFLA_BRIDGE_MRP_PORT_STATE]);
> > +
> > + err = br_mrp_set_port_state(p, state);
> > + if (err)
> > + return err;
> > + }
> > +
> > + if (tb[IFLA_BRIDGE_MRP_PORT_ROLE]) {
> > + struct br_mrp_port_role *role =
> > + nla_data(tb[IFLA_BRIDGE_MRP_PORT_ROLE]);
> > +
> > + err = br_mrp_set_port_role(p, role);
> > + if (err)
> > + return err;
> > + }
> > +
> > + if (tb[IFLA_BRIDGE_MRP_RING_STATE]) {
> > + struct br_mrp_ring_state *state =
> > + nla_data(tb[IFLA_BRIDGE_MRP_RING_STATE]);
> > +
> > + err = br_mrp_set_ring_state(br, state);
> > + if (err)
> > + return err;
> > + }
> > +
> > + if (tb[IFLA_BRIDGE_MRP_RING_ROLE]) {
> > + struct br_mrp_ring_role *role =
> > + nla_data(tb[IFLA_BRIDGE_MRP_RING_ROLE]);
> > +
> > + err = br_mrp_set_ring_role(br, role);
> > + if (err)
> > + return err;
> > + }
> > +
> > + if (tb[IFLA_BRIDGE_MRP_START_TEST]) {
> > + struct br_mrp_start_test *test =
> > + nla_data(tb[IFLA_BRIDGE_MRP_START_TEST]);
> > +
> > + err = br_mrp_start_test(br, test);
> > + if (err)
> > + return err;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +int br_mrp_port_open(struct net_device *dev, u8 loc)
> > +{
> > + struct net_bridge_port *p;
> > + int err = 0;
> > +
> > + p = br_port_get_rcu(dev);
> > + if (!p) {
> > + err = -EINVAL;
> > + goto out;
> > + }
> > +
> > + p->loc = loc;
> > + br_ifinfo_notify(RTM_NEWLINK, NULL, p);
> > +
> > +out:
> > + return err;
> > +}
> > +EXPORT_SYMBOL(br_mrp_port_open);
> >
>
> I just noticed the EXPORT_SYMBOL() here, why do you need it?

Actually is not needed. But I am thinking to drop patch 4. The reason
for patch nr 4, was that the drivers should notify when the ports lost
the continuity. But currently there is no driver using the MRP then
there is no point to have. I will add it back once the drivers start to
use the MRP.

>

--
/Horatiu

2020-04-21 08:47:15

by Nikolay Aleksandrov

[permalink] [raw]
Subject: Re: [PATCH net-next 09/13] bridge: mrp: Connect MRP API with the switchev API

On 20/04/2020 18:09, Horatiu Vultur wrote:
> Implement the MRP API.
>
> In case the HW can't generate MRP Test frames then the SW will try to generate
> the frames. In case that also the SW will fail in generating the frames then a
> error is return to the userspace. The userspace is responsible to generate all
> the other MRP frames regardless if the test frames are generated by HW or SW.
>
> The forwarding/termination of MRP frames is happening in the kernel and is done
> by the MRP instance. The userspace application doesn't do the forwarding.
>
> Signed-off-by: Horatiu Vultur <[email protected]>
> ---
> net/bridge/br_mrp.c | 551 ++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 551 insertions(+)
> create mode 100644 net/bridge/br_mrp.c
>
> diff --git a/net/bridge/br_mrp.c b/net/bridge/br_mrp.c
> new file mode 100644
> index 000000000000..a52f02f174c3
> --- /dev/null
> +++ b/net/bridge/br_mrp.c
> @@ -0,0 +1,551 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +#include <linux/mrp_bridge.h>
> +#include "br_private_mrp.h"
> +
> +static const u8 mrp_test_dmac[ETH_ALEN] = { 0x1, 0x15, 0x4e, 0x0, 0x0, 0x1 };
> +
> +static struct net_bridge_port *br_mrp_get_port(struct net_bridge *br,
> + u32 ifindex)
> +{
> + struct net_bridge_port *res = NULL;
> + struct net_bridge_port *port;
> +
> + list_for_each_entry(port, &br->port_list, list) {
> + if (port->dev->ifindex == ifindex) {
> + res = port;
> + break;
> + }
> + }
> +
> + return res;
> +}
> +
> +static struct br_mrp *br_mrp_find_id(struct net_bridge *br, u32 ring_id)
> +{
> + struct br_mrp *res = NULL;
> + struct br_mrp *mrp;
> +
> + list_for_each_entry_rcu(mrp, &br->mrp_list, list,
> + lockdep_rtnl_is_held()) {
> + if (mrp->ring_id == ring_id) {
> + res = mrp;
> + break;
> + }
> + }
> +
> + return res;
> +}
> +
> +static struct br_mrp *br_mrp_find_port(struct net_bridge *br,
> + struct net_bridge_port *p)
> +{
> + struct br_mrp *res = NULL;
> + struct br_mrp *mrp;
> +
> + list_for_each_entry_rcu(mrp, &br->mrp_list, list,
> + lockdep_rtnl_is_held()) {
> + if (rcu_access_pointer(mrp->p_port) == p ||
> + rcu_access_pointer(mrp->s_port) == p) {
> + res = mrp;
> + break;
> + }
> + }
> +
> + return res;
> +}
> +
> +static int br_mrp_next_seq(struct br_mrp *mrp)
> +{
> + mrp->seq_id++;
> + return mrp->seq_id;
> +}
> +
> +static struct sk_buff *br_mrp_skb_alloc(struct net_bridge_port *p,
> + const u8 *src, const u8 *dst)
> +{
> + struct ethhdr *eth_hdr;
> + struct sk_buff *skb;
> + u16 *version;
> +
> + skb = dev_alloc_skb(MRP_MAX_FRAME_LENGTH);
> + if (!skb)
> + return NULL;
> +
> + skb->dev = p->dev;
> + skb->protocol = htons(ETH_P_MRP);
> + skb->priority = MRP_FRAME_PRIO;
> + skb_reserve(skb, sizeof(*eth_hdr));
> +
> + eth_hdr = skb_push(skb, sizeof(*eth_hdr));
> + ether_addr_copy(eth_hdr->h_dest, dst);
> + ether_addr_copy(eth_hdr->h_source, src);
> + eth_hdr->h_proto = htons(ETH_P_MRP);
> +
> + version = skb_put(skb, sizeof(*version));
> + *version = cpu_to_be16(MRP_VERSION);
> +
> + return skb;
> +}
> +
> +static void br_mrp_skb_tlv(struct sk_buff *skb,
> + enum br_mrp_tlv_header_type type,
> + u8 length)
> +{
> + struct br_mrp_tlv_hdr *hdr;
> +
> + hdr = skb_put(skb, sizeof(*hdr));
> + hdr->type = type;
> + hdr->length = length;
> +}
> +
> +static void br_mrp_skb_common(struct sk_buff *skb, struct br_mrp *mrp)
> +{
> + struct br_mrp_common_hdr *hdr;
> +
> + br_mrp_skb_tlv(skb, BR_MRP_TLV_HEADER_COMMON, sizeof(*hdr));
> +
> + hdr = skb_put(skb, sizeof(*hdr));
> + hdr->seq_id = cpu_to_be16(br_mrp_next_seq(mrp));
> + memset(hdr->domain, 0xff, MRP_DOMAIN_UUID_LENGTH);
> +}
> +
> +static struct sk_buff *br_mrp_alloc_test_skb(struct br_mrp *mrp,
> + struct net_bridge_port *p,
> + enum br_mrp_port_role_type port_role)
> +{
> + struct br_mrp_ring_test_hdr *hdr = NULL;
> + struct net_bridge *br = p->br;
> + struct sk_buff *skb = NULL;
> +
> + if (!p)
> + return NULL;
> +
> + br = p->br;
> +
> + skb = br_mrp_skb_alloc(p, p->dev->dev_addr, mrp_test_dmac);
> + if (!skb)
> + return NULL;
> +
> + br_mrp_skb_tlv(skb, BR_MRP_TLV_HEADER_RING_TEST, sizeof(*hdr));
> + hdr = skb_put(skb, sizeof(*hdr));
> +
> + hdr->prio = cpu_to_be16(MRP_DEFAULT_PRIO);
> + ether_addr_copy(hdr->sa, p->br->dev->dev_addr);
> + hdr->port_role = cpu_to_be16(port_role);
> + hdr->state = cpu_to_be16(mrp->ring_state);
> + hdr->transitions = cpu_to_be16(mrp->ring_transitions);
> + hdr->timestamp = cpu_to_be32(jiffies_to_msecs(jiffies));
> +
> + br_mrp_skb_common(skb, mrp);
> + br_mrp_skb_tlv(skb, BR_MRP_TLV_HEADER_END, 0x0);
> +
> + return skb;
> +}
> +
> +static void br_mrp_test_work_expired(struct work_struct *work)
> +{
> + struct delayed_work *del_work = to_delayed_work(work);
> + struct br_mrp *mrp = container_of(del_work, struct br_mrp, test_work);
> + struct net_bridge_port *p;
> + bool notify_open = false;
> + struct sk_buff *skb;
> +
> + if (time_before_eq(mrp->test_end, jiffies))
> + return;
> +
> + if (mrp->test_count_miss < mrp->test_max_miss) {
> + mrp->test_count_miss++;
> + } else {
> + /* Notify that the ring is open only if the ring state is
> + * closed, otherwise it would continue to notify at every
> + * interval.
> + */
> + if (mrp->ring_state == BR_MRP_RING_STATE_CLOSED)
> + notify_open = true;
> + }
> +
> + rcu_read_lock();
> +
> + p = rcu_dereference(mrp->p_port);
> + if (p) {
> + skb = br_mrp_alloc_test_skb(mrp, p, BR_MRP_PORT_ROLE_PRIMARY);
> + if (!skb)
> + goto out;
> +
> + skb_reset_network_header(skb);
> + dev_queue_xmit(skb);
> +
> + if (notify_open && !mrp->ring_role_offloaded)
> + br_mrp_port_open(p->dev, true);
> + }
> +
> + p = rcu_dereference(mrp->s_port);
> + if (p) {
> + skb = br_mrp_alloc_test_skb(mrp, p, BR_MRP_PORT_ROLE_SECONDARY);
> + if (!skb)
> + goto out;
> +
> + skb_reset_network_header(skb);
> + dev_queue_xmit(skb);
> +
> + if (notify_open && !mrp->ring_role_offloaded)
> + br_mrp_port_open(p->dev, true);
> + }
> +
> +out:
> + rcu_read_unlock();
> +
> + queue_delayed_work(system_wq, &mrp->test_work,
> + usecs_to_jiffies(mrp->test_interval));
> +}
> +
> +/* Deletes the MRP instance.
> + * note: called under rtnl_lock
> + */
> +static void br_mrp_del_impl(struct net_bridge *br, struct br_mrp *mrp)
> +{
> + struct net_bridge_port *p;
> +
> + /* Stop sending MRP_Test frames */
> + cancel_delayed_work_sync(&mrp->test_work);
> + br_mrp_switchdev_send_ring_test(br, mrp, 0, 0, 0);
> +
> + br_mrp_switchdev_del(br, mrp);
> +
> + /* Reset the ports */
> + p = rtnl_dereference(mrp->p_port);
> + if (p) {
> + spin_lock_bh(&br->lock);
> + p->state = BR_STATE_FORWARDING;
> + p->flags &= ~BR_MRP_AWARE;
> + spin_unlock_bh(&br->lock);
> + br_mrp_port_switchdev_set_state(p, BR_STATE_FORWARDING);
> + rcu_assign_pointer(mrp->p_port, NULL);
> + }
> +
> + p = rtnl_dereference(mrp->s_port);
> + if (p) {
> + spin_lock_bh(&br->lock);
> + p->state = BR_STATE_FORWARDING;
> + p->flags &= ~BR_MRP_AWARE;
> + spin_unlock_bh(&br->lock);
> + br_mrp_port_switchdev_set_state(p, BR_STATE_FORWARDING);
> + rcu_assign_pointer(mrp->s_port, NULL);
> + }
> +
> + list_del_rcu(&mrp->list);
> + kfree_rcu(mrp, rcu);
> +}
> +
> +/* Adds a new MRP instance.
> + * note: called under rtnl_lock
> + */
> +int br_mrp_add(struct net_bridge *br, struct br_mrp_instance *instance)
> +{
> + struct net_bridge_port *p;
> + struct br_mrp *mrp;
> + int err;
> +
> + /* If the ring exists, it is not possible to create another one with the
> + * same ring_id
> + */
> + mrp = br_mrp_find_id(br, instance->ring_id);
> + if (mrp)
> + return -EINVAL;
> +
> + if (!br_mrp_get_port(br, instance->p_ifindex) ||
> + !br_mrp_get_port(br, instance->s_ifindex))
> + return -EINVAL;
> +
> + mrp = kzalloc(sizeof(*mrp), GFP_KERNEL);
> + if (!mrp)
> + return -ENOMEM;
> +
> + mrp->ring_id = instance->ring_id;
> +
> + p = br_mrp_get_port(br, instance->p_ifindex);
> + spin_lock_bh(&br->lock);
> + p->state = BR_STATE_FORWARDING;
> + p->flags |= BR_MRP_AWARE;
> + spin_unlock_bh(&br->lock);
> + rcu_assign_pointer(mrp->p_port, p);
> +
> + p = br_mrp_get_port(br, instance->s_ifindex);
> + spin_lock_bh(&br->lock);
> + p->state = BR_STATE_FORWARDING;
> + p->flags |= BR_MRP_AWARE;
> + spin_unlock_bh(&br->lock);
> + rcu_assign_pointer(mrp->s_port, p);
> +
> + INIT_DELAYED_WORK(&mrp->test_work, br_mrp_test_work_expired);
> + list_add_tail_rcu(&mrp->list, &br->mrp_list);
> +
> + err = br_mrp_switchdev_add(br, mrp);
> + if (err)
> + goto delete_mrp;
> +
> + return 0;
> +
> +delete_mrp:
> + br_mrp_del_impl(br, mrp);
> +
> + return err;
> +}
> +
> +/* Deletes the MRP instance from which the port is part of
> + * note: called under rtnl_lock
> + */
> +void br_mrp_port_del(struct net_bridge *br, struct net_bridge_port *p)
> +{
> + struct br_mrp *mrp = br_mrp_find_port(br, p);
> +
> + /* If the port is not part of a MRP instance just bail out */
> + if (!mrp)
> + return;
> +
> + br_mrp_del_impl(br, mrp);
> +}
> +
> +/* Deletes existing MRP instance based on ring_id
> + * note: called under rtnl_lock
> + */
> +int br_mrp_del(struct net_bridge *br, struct br_mrp_instance *instance)
> +{
> + struct br_mrp *mrp = br_mrp_find_id(br, instance->ring_id);
> +
> + if (!mrp)
> + return -EINVAL;
> +
> + br_mrp_del_impl(br, mrp);
> +
> + return 0;
> +}
> +
> +/* Set port state, port state can be forwarding, blocked or disabled
> + * note: already called with rcu_read_lock
> + */
> +int br_mrp_set_port_state(struct net_bridge_port *p,
> + enum br_mrp_port_state_type state)
> +{
> + spin_lock_bh(&p->br->lock);
> +
> + if (state == BR_MRP_PORT_STATE_FORWARDING)
> + p->state = BR_STATE_FORWARDING;
> + else
> + p->state = BR_STATE_BLOCKING;
> +
> + spin_unlock_bh(&p->br->lock);
> +
> + br_mrp_port_switchdev_set_state(p, state);
> +
> + return 0;
> +}
> +
> +/* Set port role, port role can be primary or secondary
> + * note: already called with rcu_read_lock
> + */
> +int br_mrp_set_port_role(struct net_bridge_port *p,
> + struct br_mrp_port_role *role)
> +{

In the previous review I was talking about this function which is called directly from
the MRP netlink code, one could invoke it with any port - it doesn't need to be MRP_AWARE
and then upon MRP delete you'll remove the flag from the wrong port. In addition the flag
on the original port will stay set.

> + struct br_mrp *mrp = br_mrp_find_id(p->br, role->ring_id);
> +
> + if (!mrp)
> + return -EINVAL;
> +
> + if (role->role == BR_MRP_PORT_ROLE_PRIMARY)
> + rcu_assign_pointer(mrp->p_port, p);
> + else
> + rcu_assign_pointer(mrp->s_port, p);
> +
> + br_mrp_port_switchdev_set_role(p, role->role);
> +
> + return 0;
> +}
> +
> +/* Set ring state, ring state can be only Open or Closed
> + * note: already called with rcu_read_lock
> + */
> +int br_mrp_set_ring_state(struct net_bridge *br,
> + struct br_mrp_ring_state *state)
> +{
> + struct br_mrp *mrp = br_mrp_find_id(br, state->ring_id);
> +
> + if (!mrp)
> + return -EINVAL;
> +
> + if (mrp->ring_state == BR_MRP_RING_STATE_CLOSED &&
> + state->ring_state != BR_MRP_RING_STATE_CLOSED)
> + mrp->ring_transitions++;
> +
> + mrp->ring_state = state->ring_state;
> +
> + br_mrp_switchdev_set_ring_state(br, mrp, state->ring_state);
> +
> + return 0;
> +}
> +
> +/* Set ring role, ring role can be only MRM(Media Redundancy Manager) or
> + * MRC(Media Redundancy Client).
> + * note: already called with rcu_read_lock
> + */
> +int br_mrp_set_ring_role(struct net_bridge *br,
> + struct br_mrp_ring_role *role)
> +{
> + struct br_mrp *mrp = br_mrp_find_id(br, role->ring_id);
> + int err;
> +
> + if (!mrp)
> + return -EINVAL;
> +
> + mrp->ring_role = role->ring_role;
> +
> + /* If there is an error just bailed out */
> + err = br_mrp_switchdev_set_ring_role(br, mrp, role->ring_role);
> + if (err && err != -EOPNOTSUPP)
> + return err;
> +
> + /* Now detect if the HW actually applied the role or not. If the HW
> + * applied the role it means that the SW will not to do those operations
> + * anymore. For example if the role ir MRM then the HW will notify the
> + * SW when ring is open, but if the is not pushed to the HW the SW will
> + * need to detect when the ring is open
> + */
> + mrp->ring_role_offloaded = err == -EOPNOTSUPP ? 0 : 1;
> +
> + return 0;
> +}
> +
> +/* Start to generate MRP test frames, the frames are generated by HW and if it
> + * fails, they are generated by the SW.
> + * note: already called with rcu_read_lock
> + */
> +int br_mrp_start_test(struct net_bridge *br,
> + struct br_mrp_start_test *test)
> +{
> + struct br_mrp *mrp = br_mrp_find_id(br, test->ring_id);
> +
> + if (!mrp)
> + return -EINVAL;
> +
> + /* Try to push is to the HW and if it fails then continue to generate in
> + * SW and if that also fails then return error
> + */
> + if (!br_mrp_switchdev_send_ring_test(br, mrp, test->interval,
> + test->max_miss, test->period))
> + return 0;
> +
> + mrp->test_interval = test->interval;
> + mrp->test_end = jiffies + usecs_to_jiffies(test->period);
> + mrp->test_max_miss = test->max_miss;
> + mrp->test_count_miss = 0;
> + queue_delayed_work(system_wq, &mrp->test_work,
> + usecs_to_jiffies(test->interval));
> +
> + return 0;
> +}
> +
> +/* Process only MRP Test frame. All the other MRP frames are processed by
> + * userspace application
> + * note: already called with rcu_read_lock
> + */
> +static void br_mrp_mrm_process(struct br_mrp *mrp, struct net_bridge_port *port,
> + struct sk_buff *skb)
> +{
> + struct br_mrp_tlv_hdr *hdr;
> +
> + hdr = (struct br_mrp_tlv_hdr *)(skb->data + sizeof(uint16_t));
> +
> + if (!hdr)
> + return;
> +
> + if (hdr->type != BR_MRP_TLV_HEADER_RING_TEST)
> + return;
> +
> + mrp->test_count_miss = 0;
> +
> + /* Notify the userspace that the ring is closed only when the ring is
> + * not closed
> + */
> + if (mrp->ring_state != BR_MRP_RING_STATE_CLOSED)
> + br_mrp_port_open(port->dev, false);
> +}
> +
> +/* This will just forward the frame to the other mrp ring port(MRC role) or will
> + * not do anything.
> + * note: already called with rcu_read_lock
> + */
> +static int br_mrp_rcv(struct net_bridge_port *p,
> + struct sk_buff *skb, struct net_device *dev)
> +{
> + struct net_device *s_dev, *p_dev, *d_dev;
> + struct net_bridge_port *p_port, *s_port;
> + struct net_bridge *br;
> + struct sk_buff *nskb;
> + struct br_mrp *mrp;
> +
> + /* If port is disabled don't accept any frames */
> + if (p->state == BR_STATE_DISABLED)
> + return 0;
> +
> + br = p->br;
> + mrp = br_mrp_find_port(br, p);
> + if (unlikely(!mrp))
> + return 0;
> +
> + p_port = rcu_dereference(mrp->p_port);
> + if (!p_port)
> + return 0;
> +
> + s_port = rcu_dereference(mrp->s_port);
> + if (!s_port)
> + return 0;
> +
> + /* If the role is MRM then don't forward the frames */
> + if (mrp->ring_role == BR_MRP_RING_ROLE_MRM) {
> + br_mrp_mrm_process(mrp, p, skb);
> + return 1;
> + }
> +
> + /* Clone the frame and forward it on the other MRP port */
> + nskb = skb_clone(skb, GFP_ATOMIC);
> + if (!nskb)
> + return 0;
> +
> + p_dev = p_port->dev;
> + s_dev = s_port->dev;
> +
> + if (p_dev == dev)
> + d_dev = s_dev;
> + else
> + d_dev = p_dev;
> +
> + nskb->dev = d_dev;
> + skb_push(nskb, ETH_HLEN);
> + dev_queue_xmit(nskb);
> +
> + return 1;
> +}
> +
> +/* Check if the frame was received on a port that is part of MRP ring
> + * and if the frame has MRP eth. In that case process the frame otherwise do
> + * normal forwarding.
> + * note: already called with rcu_read_lock
> + */
> +int br_mrp_process(struct net_bridge_port *p, struct sk_buff *skb)
> +{
> + /* If there is no MRP instance do normal forwarding */
> + if (likely(!(p->flags & BR_MRP_AWARE)))
> + goto out;
> +
> + if (unlikely(skb->protocol == htons(ETH_P_MRP)))
> + return br_mrp_rcv(p, skb, p->dev);
> +
> +out:
> + return 0;
> +}
> +
> +bool br_mrp_enabled(struct net_bridge *br)
> +{
> + return !list_empty(&br->mrp_list);
> +}
>

2020-04-21 08:49:10

by Nikolay Aleksandrov

[permalink] [raw]
Subject: Re: [PATCH net-next 10/13] bridge: mrp: Implement netlink interface to configure MRP

On 20/04/2020 18:09, Horatiu Vultur wrote:
> Implement netlink interface to configure MRP. The implementation
> will do sanity checks over the attributes and then eventually call the MRP
> interface.
>
> Signed-off-by: Horatiu Vultur <[email protected]>
> ---
> net/bridge/br_mrp_netlink.c | 117 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 117 insertions(+)
> create mode 100644 net/bridge/br_mrp_netlink.c
>
> diff --git a/net/bridge/br_mrp_netlink.c b/net/bridge/br_mrp_netlink.c
> new file mode 100644
> index 000000000000..0ff42e7c7f57
> --- /dev/null
> +++ b/net/bridge/br_mrp_netlink.c
> @@ -0,0 +1,117 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +#include <net/genetlink.h>
> +
> +#include <uapi/linux/mrp_bridge.h>
> +#include "br_private.h"
> +#include "br_private_mrp.h"
> +
> +static const struct nla_policy br_mrp_policy[IFLA_BRIDGE_MRP_MAX + 1] = {
> + [IFLA_BRIDGE_MRP_UNSPEC] = { .type = NLA_REJECT },
> + [IFLA_BRIDGE_MRP_INSTANCE] = { .type = NLA_EXACT_LEN,
> + .len = sizeof(struct br_mrp_instance)},
> + [IFLA_BRIDGE_MRP_PORT_STATE] = { .type = NLA_U32 },
> + [IFLA_BRIDGE_MRP_PORT_ROLE] = { .type = NLA_EXACT_LEN,
> + .len = sizeof(struct br_mrp_port_role)},
> + [IFLA_BRIDGE_MRP_RING_STATE] = { .type = NLA_EXACT_LEN,
> + .len = sizeof(struct br_mrp_ring_state)},
> + [IFLA_BRIDGE_MRP_RING_ROLE] = { .type = NLA_EXACT_LEN,
> + .len = sizeof(struct br_mrp_ring_role)},
> + [IFLA_BRIDGE_MRP_START_TEST] = { .type = NLA_EXACT_LEN,
> + .len = sizeof(struct br_mrp_start_test)},
> +};
> +
> +int br_mrp_parse(struct net_bridge *br, struct net_bridge_port *p,
> + struct nlattr *attr, int cmd, struct netlink_ext_ack *extack)
> +{

Note that "p" can be NULL here if br_afspec() was called for the bridge device.
Some of the functions below dereference it without any checks and will deref a
null ptr.

> + struct nlattr *tb[IFLA_BRIDGE_MRP_MAX + 1];
> + int err;
> +
> + if (br->stp_enabled != BR_NO_STP) {
> + NL_SET_ERR_MSG_MOD(extack, "MRP can't be enabled if STP is already enabled\n");
> + return -EINVAL;
> + }
> +
> + err = nla_parse_nested(tb, IFLA_BRIDGE_MRP_MAX, attr,
> + NULL, extack);
> + if (err)
> + return err;
> +
> + if (tb[IFLA_BRIDGE_MRP_INSTANCE]) {
> + struct br_mrp_instance *instance =
> + nla_data(tb[IFLA_BRIDGE_MRP_INSTANCE]);
> +
> + if (cmd == RTM_SETLINK)
> + err = br_mrp_add(br, instance);
> + else
> + err = br_mrp_del(br, instance);
> + if (err)
> + return err;
> + }
> +
> + if (tb[IFLA_BRIDGE_MRP_PORT_STATE]) {
> + enum br_mrp_port_state_type state =
> + nla_get_u32(tb[IFLA_BRIDGE_MRP_PORT_STATE]);
> +
> + err = br_mrp_set_port_state(p, state);
> + if (err)
> + return err;
> + }
> +
> + if (tb[IFLA_BRIDGE_MRP_PORT_ROLE]) {
> + struct br_mrp_port_role *role =
> + nla_data(tb[IFLA_BRIDGE_MRP_PORT_ROLE]);
> +
> + err = br_mrp_set_port_role(p, role);
> + if (err)
> + return err;
> + }
> +
> + if (tb[IFLA_BRIDGE_MRP_RING_STATE]) {
> + struct br_mrp_ring_state *state =
> + nla_data(tb[IFLA_BRIDGE_MRP_RING_STATE]);
> +
> + err = br_mrp_set_ring_state(br, state);
> + if (err)
> + return err;
> + }
> +
> + if (tb[IFLA_BRIDGE_MRP_RING_ROLE]) {
> + struct br_mrp_ring_role *role =
> + nla_data(tb[IFLA_BRIDGE_MRP_RING_ROLE]);
> +
> + err = br_mrp_set_ring_role(br, role);
> + if (err)
> + return err;
> + }
> +
> + if (tb[IFLA_BRIDGE_MRP_START_TEST]) {
> + struct br_mrp_start_test *test =
> + nla_data(tb[IFLA_BRIDGE_MRP_START_TEST]);
> +
> + err = br_mrp_start_test(br, test);
> + if (err)
> + return err;
> + }
> +
> + return 0;
> +}
> +
> +int br_mrp_port_open(struct net_device *dev, u8 loc)
> +{
> + struct net_bridge_port *p;
> + int err = 0;
> +
> + p = br_port_get_rcu(dev);
> + if (!p) {
> + err = -EINVAL;
> + goto out;
> + }
> +
> + p->loc = loc;
> + br_ifinfo_notify(RTM_NEWLINK, NULL, p);
> +
> +out:
> + return err;
> +}
> +EXPORT_SYMBOL(br_mrp_port_open);
>

2020-04-21 09:21:23

by Horatiu Vultur

[permalink] [raw]
Subject: Re: [PATCH net-next 10/13] bridge: mrp: Implement netlink interface to configure MRP

The 04/21/2020 11:47, Nikolay Aleksandrov wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> On 20/04/2020 18:09, Horatiu Vultur wrote:
> > Implement netlink interface to configure MRP. The implementation
> > will do sanity checks over the attributes and then eventually call the MRP
> > interface.
> >
> > Signed-off-by: Horatiu Vultur <[email protected]>
> > ---
> > net/bridge/br_mrp_netlink.c | 117 ++++++++++++++++++++++++++++++++++++
> > 1 file changed, 117 insertions(+)
> > create mode 100644 net/bridge/br_mrp_netlink.c
> >
> > diff --git a/net/bridge/br_mrp_netlink.c b/net/bridge/br_mrp_netlink.c
> > new file mode 100644
> > index 000000000000..0ff42e7c7f57
> > --- /dev/null
> > +++ b/net/bridge/br_mrp_netlink.c
> > @@ -0,0 +1,117 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +
> > +#include <net/genetlink.h>
> > +
> > +#include <uapi/linux/mrp_bridge.h>
> > +#include "br_private.h"
> > +#include "br_private_mrp.h"
> > +
> > +static const struct nla_policy br_mrp_policy[IFLA_BRIDGE_MRP_MAX + 1] = {
> > + [IFLA_BRIDGE_MRP_UNSPEC] = { .type = NLA_REJECT },
> > + [IFLA_BRIDGE_MRP_INSTANCE] = { .type = NLA_EXACT_LEN,
> > + .len = sizeof(struct br_mrp_instance)},
> > + [IFLA_BRIDGE_MRP_PORT_STATE] = { .type = NLA_U32 },
> > + [IFLA_BRIDGE_MRP_PORT_ROLE] = { .type = NLA_EXACT_LEN,
> > + .len = sizeof(struct br_mrp_port_role)},
> > + [IFLA_BRIDGE_MRP_RING_STATE] = { .type = NLA_EXACT_LEN,
> > + .len = sizeof(struct br_mrp_ring_state)},
> > + [IFLA_BRIDGE_MRP_RING_ROLE] = { .type = NLA_EXACT_LEN,
> > + .len = sizeof(struct br_mrp_ring_role)},
> > + [IFLA_BRIDGE_MRP_START_TEST] = { .type = NLA_EXACT_LEN,
> > + .len = sizeof(struct br_mrp_start_test)},
> > +};
> > +
> > +int br_mrp_parse(struct net_bridge *br, struct net_bridge_port *p,
> > + struct nlattr *attr, int cmd, struct netlink_ext_ack *extack)
> > +{
>
> Note that "p" can be NULL here if br_afspec() was called for the bridge device.
> Some of the functions below dereference it without any checks and will deref a
> null ptr.

Good check, I will update the functions br_mrp_set_port_role and
br_mrp_set_port_state to check for the null ptr.

>
> > + struct nlattr *tb[IFLA_BRIDGE_MRP_MAX + 1];
> > + int err;
> > +
> > + if (br->stp_enabled != BR_NO_STP) {
> > + NL_SET_ERR_MSG_MOD(extack, "MRP can't be enabled if STP is already enabled\n");
> > + return -EINVAL;
> > + }
> > +
> > + err = nla_parse_nested(tb, IFLA_BRIDGE_MRP_MAX, attr,
> > + NULL, extack);
> > + if (err)
> > + return err;
> > +
> > + if (tb[IFLA_BRIDGE_MRP_INSTANCE]) {
> > + struct br_mrp_instance *instance =
> > + nla_data(tb[IFLA_BRIDGE_MRP_INSTANCE]);
> > +
> > + if (cmd == RTM_SETLINK)
> > + err = br_mrp_add(br, instance);
> > + else
> > + err = br_mrp_del(br, instance);
> > + if (err)
> > + return err;
> > + }
> > +
> > + if (tb[IFLA_BRIDGE_MRP_PORT_STATE]) {
> > + enum br_mrp_port_state_type state =
> > + nla_get_u32(tb[IFLA_BRIDGE_MRP_PORT_STATE]);
> > +
> > + err = br_mrp_set_port_state(p, state);
> > + if (err)
> > + return err;
> > + }
> > +
> > + if (tb[IFLA_BRIDGE_MRP_PORT_ROLE]) {
> > + struct br_mrp_port_role *role =
> > + nla_data(tb[IFLA_BRIDGE_MRP_PORT_ROLE]);
> > +
> > + err = br_mrp_set_port_role(p, role);
> > + if (err)
> > + return err;
> > + }
> > +
> > + if (tb[IFLA_BRIDGE_MRP_RING_STATE]) {
> > + struct br_mrp_ring_state *state =
> > + nla_data(tb[IFLA_BRIDGE_MRP_RING_STATE]);
> > +
> > + err = br_mrp_set_ring_state(br, state);
> > + if (err)
> > + return err;
> > + }
> > +
> > + if (tb[IFLA_BRIDGE_MRP_RING_ROLE]) {
> > + struct br_mrp_ring_role *role =
> > + nla_data(tb[IFLA_BRIDGE_MRP_RING_ROLE]);
> > +
> > + err = br_mrp_set_ring_role(br, role);
> > + if (err)
> > + return err;
> > + }
> > +
> > + if (tb[IFLA_BRIDGE_MRP_START_TEST]) {
> > + struct br_mrp_start_test *test =
> > + nla_data(tb[IFLA_BRIDGE_MRP_START_TEST]);
> > +
> > + err = br_mrp_start_test(br, test);
> > + if (err)
> > + return err;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +int br_mrp_port_open(struct net_device *dev, u8 loc)
> > +{
> > + struct net_bridge_port *p;
> > + int err = 0;
> > +
> > + p = br_port_get_rcu(dev);
> > + if (!p) {
> > + err = -EINVAL;
> > + goto out;
> > + }
> > +
> > + p->loc = loc;
> > + br_ifinfo_notify(RTM_NEWLINK, NULL, p);
> > +
> > +out:
> > + return err;
> > +}
> > +EXPORT_SYMBOL(br_mrp_port_open);
> >
>

--
/Horatiu

2020-04-21 09:33:23

by Horatiu Vultur

[permalink] [raw]
Subject: Re: [PATCH net-next 09/13] bridge: mrp: Connect MRP API with the switchev API

The 04/21/2020 11:44, Nikolay Aleksandrov wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> On 20/04/2020 18:09, Horatiu Vultur wrote:
> > Implement the MRP API.
> >
> > In case the HW can't generate MRP Test frames then the SW will try to generate
> > the frames. In case that also the SW will fail in generating the frames then a
> > error is return to the userspace. The userspace is responsible to generate all
> > the other MRP frames regardless if the test frames are generated by HW or SW.
> >
> > The forwarding/termination of MRP frames is happening in the kernel and is done
> > by the MRP instance. The userspace application doesn't do the forwarding.
> >
> > Signed-off-by: Horatiu Vultur <[email protected]>
> > ---
> > net/bridge/br_mrp.c | 551 ++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 551 insertions(+)
> > create mode 100644 net/bridge/br_mrp.c
> >
> > diff --git a/net/bridge/br_mrp.c b/net/bridge/br_mrp.c
> > new file mode 100644
> > index 000000000000..a52f02f174c3
> > --- /dev/null
> > +++ b/net/bridge/br_mrp.c
> > @@ -0,0 +1,551 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +
> > +#include <linux/mrp_bridge.h>
> > +#include "br_private_mrp.h"
> > +
> > +static const u8 mrp_test_dmac[ETH_ALEN] = { 0x1, 0x15, 0x4e, 0x0, 0x0, 0x1 };
> > +
> > +static struct net_bridge_port *br_mrp_get_port(struct net_bridge *br,
> > + u32 ifindex)
> > +{
> > + struct net_bridge_port *res = NULL;
> > + struct net_bridge_port *port;
> > +
> > + list_for_each_entry(port, &br->port_list, list) {
> > + if (port->dev->ifindex == ifindex) {
> > + res = port;
> > + break;
> > + }
> > + }
> > +
> > + return res;
> > +}
> > +
> > +static struct br_mrp *br_mrp_find_id(struct net_bridge *br, u32 ring_id)
> > +{
> > + struct br_mrp *res = NULL;
> > + struct br_mrp *mrp;
> > +
> > + list_for_each_entry_rcu(mrp, &br->mrp_list, list,
> > + lockdep_rtnl_is_held()) {
> > + if (mrp->ring_id == ring_id) {
> > + res = mrp;
> > + break;
> > + }
> > + }
> > +
> > + return res;
> > +}
> > +
> > +static struct br_mrp *br_mrp_find_port(struct net_bridge *br,
> > + struct net_bridge_port *p)
> > +{
> > + struct br_mrp *res = NULL;
> > + struct br_mrp *mrp;
> > +
> > + list_for_each_entry_rcu(mrp, &br->mrp_list, list,
> > + lockdep_rtnl_is_held()) {
> > + if (rcu_access_pointer(mrp->p_port) == p ||
> > + rcu_access_pointer(mrp->s_port) == p) {
> > + res = mrp;
> > + break;
> > + }
> > + }
> > +
> > + return res;
> > +}
> > +
> > +static int br_mrp_next_seq(struct br_mrp *mrp)
> > +{
> > + mrp->seq_id++;
> > + return mrp->seq_id;
> > +}
> > +
> > +static struct sk_buff *br_mrp_skb_alloc(struct net_bridge_port *p,
> > + const u8 *src, const u8 *dst)
> > +{
> > + struct ethhdr *eth_hdr;
> > + struct sk_buff *skb;
> > + u16 *version;
> > +
> > + skb = dev_alloc_skb(MRP_MAX_FRAME_LENGTH);
> > + if (!skb)
> > + return NULL;
> > +
> > + skb->dev = p->dev;
> > + skb->protocol = htons(ETH_P_MRP);
> > + skb->priority = MRP_FRAME_PRIO;
> > + skb_reserve(skb, sizeof(*eth_hdr));
> > +
> > + eth_hdr = skb_push(skb, sizeof(*eth_hdr));
> > + ether_addr_copy(eth_hdr->h_dest, dst);
> > + ether_addr_copy(eth_hdr->h_source, src);
> > + eth_hdr->h_proto = htons(ETH_P_MRP);
> > +
> > + version = skb_put(skb, sizeof(*version));
> > + *version = cpu_to_be16(MRP_VERSION);
> > +
> > + return skb;
> > +}
> > +
> > +static void br_mrp_skb_tlv(struct sk_buff *skb,
> > + enum br_mrp_tlv_header_type type,
> > + u8 length)
> > +{
> > + struct br_mrp_tlv_hdr *hdr;
> > +
> > + hdr = skb_put(skb, sizeof(*hdr));
> > + hdr->type = type;
> > + hdr->length = length;
> > +}
> > +
> > +static void br_mrp_skb_common(struct sk_buff *skb, struct br_mrp *mrp)
> > +{
> > + struct br_mrp_common_hdr *hdr;
> > +
> > + br_mrp_skb_tlv(skb, BR_MRP_TLV_HEADER_COMMON, sizeof(*hdr));
> > +
> > + hdr = skb_put(skb, sizeof(*hdr));
> > + hdr->seq_id = cpu_to_be16(br_mrp_next_seq(mrp));
> > + memset(hdr->domain, 0xff, MRP_DOMAIN_UUID_LENGTH);
> > +}
> > +
> > +static struct sk_buff *br_mrp_alloc_test_skb(struct br_mrp *mrp,
> > + struct net_bridge_port *p,
> > + enum br_mrp_port_role_type port_role)
> > +{
> > + struct br_mrp_ring_test_hdr *hdr = NULL;
> > + struct net_bridge *br = p->br;
> > + struct sk_buff *skb = NULL;
> > +
> > + if (!p)
> > + return NULL;
> > +
> > + br = p->br;
> > +
> > + skb = br_mrp_skb_alloc(p, p->dev->dev_addr, mrp_test_dmac);
> > + if (!skb)
> > + return NULL;
> > +
> > + br_mrp_skb_tlv(skb, BR_MRP_TLV_HEADER_RING_TEST, sizeof(*hdr));
> > + hdr = skb_put(skb, sizeof(*hdr));
> > +
> > + hdr->prio = cpu_to_be16(MRP_DEFAULT_PRIO);
> > + ether_addr_copy(hdr->sa, p->br->dev->dev_addr);
> > + hdr->port_role = cpu_to_be16(port_role);
> > + hdr->state = cpu_to_be16(mrp->ring_state);
> > + hdr->transitions = cpu_to_be16(mrp->ring_transitions);
> > + hdr->timestamp = cpu_to_be32(jiffies_to_msecs(jiffies));
> > +
> > + br_mrp_skb_common(skb, mrp);
> > + br_mrp_skb_tlv(skb, BR_MRP_TLV_HEADER_END, 0x0);
> > +
> > + return skb;
> > +}
> > +
> > +static void br_mrp_test_work_expired(struct work_struct *work)
> > +{
> > + struct delayed_work *del_work = to_delayed_work(work);
> > + struct br_mrp *mrp = container_of(del_work, struct br_mrp, test_work);
> > + struct net_bridge_port *p;
> > + bool notify_open = false;
> > + struct sk_buff *skb;
> > +
> > + if (time_before_eq(mrp->test_end, jiffies))
> > + return;
> > +
> > + if (mrp->test_count_miss < mrp->test_max_miss) {
> > + mrp->test_count_miss++;
> > + } else {
> > + /* Notify that the ring is open only if the ring state is
> > + * closed, otherwise it would continue to notify at every
> > + * interval.
> > + */
> > + if (mrp->ring_state == BR_MRP_RING_STATE_CLOSED)
> > + notify_open = true;
> > + }
> > +
> > + rcu_read_lock();
> > +
> > + p = rcu_dereference(mrp->p_port);
> > + if (p) {
> > + skb = br_mrp_alloc_test_skb(mrp, p, BR_MRP_PORT_ROLE_PRIMARY);
> > + if (!skb)
> > + goto out;
> > +
> > + skb_reset_network_header(skb);
> > + dev_queue_xmit(skb);
> > +
> > + if (notify_open && !mrp->ring_role_offloaded)
> > + br_mrp_port_open(p->dev, true);
> > + }
> > +
> > + p = rcu_dereference(mrp->s_port);
> > + if (p) {
> > + skb = br_mrp_alloc_test_skb(mrp, p, BR_MRP_PORT_ROLE_SECONDARY);
> > + if (!skb)
> > + goto out;
> > +
> > + skb_reset_network_header(skb);
> > + dev_queue_xmit(skb);
> > +
> > + if (notify_open && !mrp->ring_role_offloaded)
> > + br_mrp_port_open(p->dev, true);
> > + }
> > +
> > +out:
> > + rcu_read_unlock();
> > +
> > + queue_delayed_work(system_wq, &mrp->test_work,
> > + usecs_to_jiffies(mrp->test_interval));
> > +}
> > +
> > +/* Deletes the MRP instance.
> > + * note: called under rtnl_lock
> > + */
> > +static void br_mrp_del_impl(struct net_bridge *br, struct br_mrp *mrp)
> > +{
> > + struct net_bridge_port *p;
> > +
> > + /* Stop sending MRP_Test frames */
> > + cancel_delayed_work_sync(&mrp->test_work);
> > + br_mrp_switchdev_send_ring_test(br, mrp, 0, 0, 0);
> > +
> > + br_mrp_switchdev_del(br, mrp);
> > +
> > + /* Reset the ports */
> > + p = rtnl_dereference(mrp->p_port);
> > + if (p) {
> > + spin_lock_bh(&br->lock);
> > + p->state = BR_STATE_FORWARDING;
> > + p->flags &= ~BR_MRP_AWARE;
> > + spin_unlock_bh(&br->lock);
> > + br_mrp_port_switchdev_set_state(p, BR_STATE_FORWARDING);
> > + rcu_assign_pointer(mrp->p_port, NULL);
> > + }
> > +
> > + p = rtnl_dereference(mrp->s_port);
> > + if (p) {
> > + spin_lock_bh(&br->lock);
> > + p->state = BR_STATE_FORWARDING;
> > + p->flags &= ~BR_MRP_AWARE;
> > + spin_unlock_bh(&br->lock);
> > + br_mrp_port_switchdev_set_state(p, BR_STATE_FORWARDING);
> > + rcu_assign_pointer(mrp->s_port, NULL);
> > + }
> > +
> > + list_del_rcu(&mrp->list);
> > + kfree_rcu(mrp, rcu);
> > +}
> > +
> > +/* Adds a new MRP instance.
> > + * note: called under rtnl_lock
> > + */
> > +int br_mrp_add(struct net_bridge *br, struct br_mrp_instance *instance)
> > +{
> > + struct net_bridge_port *p;
> > + struct br_mrp *mrp;
> > + int err;
> > +
> > + /* If the ring exists, it is not possible to create another one with the
> > + * same ring_id
> > + */
> > + mrp = br_mrp_find_id(br, instance->ring_id);
> > + if (mrp)
> > + return -EINVAL;
> > +
> > + if (!br_mrp_get_port(br, instance->p_ifindex) ||
> > + !br_mrp_get_port(br, instance->s_ifindex))
> > + return -EINVAL;
> > +
> > + mrp = kzalloc(sizeof(*mrp), GFP_KERNEL);
> > + if (!mrp)
> > + return -ENOMEM;
> > +
> > + mrp->ring_id = instance->ring_id;
> > +
> > + p = br_mrp_get_port(br, instance->p_ifindex);
> > + spin_lock_bh(&br->lock);
> > + p->state = BR_STATE_FORWARDING;
> > + p->flags |= BR_MRP_AWARE;
> > + spin_unlock_bh(&br->lock);
> > + rcu_assign_pointer(mrp->p_port, p);
> > +
> > + p = br_mrp_get_port(br, instance->s_ifindex);
> > + spin_lock_bh(&br->lock);
> > + p->state = BR_STATE_FORWARDING;
> > + p->flags |= BR_MRP_AWARE;
> > + spin_unlock_bh(&br->lock);
> > + rcu_assign_pointer(mrp->s_port, p);
> > +
> > + INIT_DELAYED_WORK(&mrp->test_work, br_mrp_test_work_expired);
> > + list_add_tail_rcu(&mrp->list, &br->mrp_list);
> > +
> > + err = br_mrp_switchdev_add(br, mrp);
> > + if (err)
> > + goto delete_mrp;
> > +
> > + return 0;
> > +
> > +delete_mrp:
> > + br_mrp_del_impl(br, mrp);
> > +
> > + return err;
> > +}
> > +
> > +/* Deletes the MRP instance from which the port is part of
> > + * note: called under rtnl_lock
> > + */
> > +void br_mrp_port_del(struct net_bridge *br, struct net_bridge_port *p)
> > +{
> > + struct br_mrp *mrp = br_mrp_find_port(br, p);
> > +
> > + /* If the port is not part of a MRP instance just bail out */
> > + if (!mrp)
> > + return;
> > +
> > + br_mrp_del_impl(br, mrp);
> > +}
> > +
> > +/* Deletes existing MRP instance based on ring_id
> > + * note: called under rtnl_lock
> > + */
> > +int br_mrp_del(struct net_bridge *br, struct br_mrp_instance *instance)
> > +{
> > + struct br_mrp *mrp = br_mrp_find_id(br, instance->ring_id);
> > +
> > + if (!mrp)
> > + return -EINVAL;
> > +
> > + br_mrp_del_impl(br, mrp);
> > +
> > + return 0;
> > +}
> > +
> > +/* Set port state, port state can be forwarding, blocked or disabled
> > + * note: already called with rcu_read_lock
> > + */
> > +int br_mrp_set_port_state(struct net_bridge_port *p,
> > + enum br_mrp_port_state_type state)
> > +{
> > + spin_lock_bh(&p->br->lock);
> > +
> > + if (state == BR_MRP_PORT_STATE_FORWARDING)
> > + p->state = BR_STATE_FORWARDING;
> > + else
> > + p->state = BR_STATE_BLOCKING;
> > +
> > + spin_unlock_bh(&p->br->lock);
> > +
> > + br_mrp_port_switchdev_set_state(p, state);
> > +
> > + return 0;
> > +}
> > +
> > +/* Set port role, port role can be primary or secondary
> > + * note: already called with rcu_read_lock
> > + */
> > +int br_mrp_set_port_role(struct net_bridge_port *p,
> > + struct br_mrp_port_role *role)
> > +{
>
> In the previous review I was talking about this function which is called directly from
> the MRP netlink code, one could invoke it with any port - it doesn't need to be MRP_AWARE
> and then upon MRP delete you'll remove the flag from the wrong port. In addition the flag
> on the original port will stay set.

I think now I understand it. So if a bridge has 3 ports, and a MRP
instance has ports eth0 and eth1. It is possible to call this function
for eth2 and nothing will prevent from doing this.
Therefore I will add a check if the port has the MRP_AWARE flag and if
it doesn't return -EINVAL.

>
> > + struct br_mrp *mrp = br_mrp_find_id(p->br, role->ring_id);
> > +
> > + if (!mrp)
> > + return -EINVAL;
> > +
> > + if (role->role == BR_MRP_PORT_ROLE_PRIMARY)
> > + rcu_assign_pointer(mrp->p_port, p);
> > + else
> > + rcu_assign_pointer(mrp->s_port, p);
> > +
> > + br_mrp_port_switchdev_set_role(p, role->role);
> > +
> > + return 0;
> > +}
> > +
> > +/* Set ring state, ring state can be only Open or Closed
> > + * note: already called with rcu_read_lock
> > + */
> > +int br_mrp_set_ring_state(struct net_bridge *br,
> > + struct br_mrp_ring_state *state)
> > +{
> > + struct br_mrp *mrp = br_mrp_find_id(br, state->ring_id);
> > +
> > + if (!mrp)
> > + return -EINVAL;
> > +
> > + if (mrp->ring_state == BR_MRP_RING_STATE_CLOSED &&
> > + state->ring_state != BR_MRP_RING_STATE_CLOSED)
> > + mrp->ring_transitions++;
> > +
> > + mrp->ring_state = state->ring_state;
> > +
> > + br_mrp_switchdev_set_ring_state(br, mrp, state->ring_state);
> > +
> > + return 0;
> > +}
> > +
> > +/* Set ring role, ring role can be only MRM(Media Redundancy Manager) or
> > + * MRC(Media Redundancy Client).
> > + * note: already called with rcu_read_lock
> > + */
> > +int br_mrp_set_ring_role(struct net_bridge *br,
> > + struct br_mrp_ring_role *role)
> > +{
> > + struct br_mrp *mrp = br_mrp_find_id(br, role->ring_id);
> > + int err;
> > +
> > + if (!mrp)
> > + return -EINVAL;
> > +
> > + mrp->ring_role = role->ring_role;
> > +
> > + /* If there is an error just bailed out */
> > + err = br_mrp_switchdev_set_ring_role(br, mrp, role->ring_role);
> > + if (err && err != -EOPNOTSUPP)
> > + return err;
> > +
> > + /* Now detect if the HW actually applied the role or not. If the HW
> > + * applied the role it means that the SW will not to do those operations
> > + * anymore. For example if the role ir MRM then the HW will notify the
> > + * SW when ring is open, but if the is not pushed to the HW the SW will
> > + * need to detect when the ring is open
> > + */
> > + mrp->ring_role_offloaded = err == -EOPNOTSUPP ? 0 : 1;
> > +
> > + return 0;
> > +}
> > +
> > +/* Start to generate MRP test frames, the frames are generated by HW and if it
> > + * fails, they are generated by the SW.
> > + * note: already called with rcu_read_lock
> > + */
> > +int br_mrp_start_test(struct net_bridge *br,
> > + struct br_mrp_start_test *test)
> > +{
> > + struct br_mrp *mrp = br_mrp_find_id(br, test->ring_id);
> > +
> > + if (!mrp)
> > + return -EINVAL;
> > +
> > + /* Try to push is to the HW and if it fails then continue to generate in
> > + * SW and if that also fails then return error
> > + */
> > + if (!br_mrp_switchdev_send_ring_test(br, mrp, test->interval,
> > + test->max_miss, test->period))
> > + return 0;
> > +
> > + mrp->test_interval = test->interval;
> > + mrp->test_end = jiffies + usecs_to_jiffies(test->period);
> > + mrp->test_max_miss = test->max_miss;
> > + mrp->test_count_miss = 0;
> > + queue_delayed_work(system_wq, &mrp->test_work,
> > + usecs_to_jiffies(test->interval));
> > +
> > + return 0;
> > +}
> > +
> > +/* Process only MRP Test frame. All the other MRP frames are processed by
> > + * userspace application
> > + * note: already called with rcu_read_lock
> > + */
> > +static void br_mrp_mrm_process(struct br_mrp *mrp, struct net_bridge_port *port,
> > + struct sk_buff *skb)
> > +{
> > + struct br_mrp_tlv_hdr *hdr;
> > +
> > + hdr = (struct br_mrp_tlv_hdr *)(skb->data + sizeof(uint16_t));
> > +
> > + if (!hdr)
> > + return;
> > +
> > + if (hdr->type != BR_MRP_TLV_HEADER_RING_TEST)
> > + return;
> > +
> > + mrp->test_count_miss = 0;
> > +
> > + /* Notify the userspace that the ring is closed only when the ring is
> > + * not closed
> > + */
> > + if (mrp->ring_state != BR_MRP_RING_STATE_CLOSED)
> > + br_mrp_port_open(port->dev, false);
> > +}
> > +
> > +/* This will just forward the frame to the other mrp ring port(MRC role) or will
> > + * not do anything.
> > + * note: already called with rcu_read_lock
> > + */
> > +static int br_mrp_rcv(struct net_bridge_port *p,
> > + struct sk_buff *skb, struct net_device *dev)
> > +{
> > + struct net_device *s_dev, *p_dev, *d_dev;
> > + struct net_bridge_port *p_port, *s_port;
> > + struct net_bridge *br;
> > + struct sk_buff *nskb;
> > + struct br_mrp *mrp;
> > +
> > + /* If port is disabled don't accept any frames */
> > + if (p->state == BR_STATE_DISABLED)
> > + return 0;
> > +
> > + br = p->br;
> > + mrp = br_mrp_find_port(br, p);
> > + if (unlikely(!mrp))
> > + return 0;
> > +
> > + p_port = rcu_dereference(mrp->p_port);
> > + if (!p_port)
> > + return 0;
> > +
> > + s_port = rcu_dereference(mrp->s_port);
> > + if (!s_port)
> > + return 0;
> > +
> > + /* If the role is MRM then don't forward the frames */
> > + if (mrp->ring_role == BR_MRP_RING_ROLE_MRM) {
> > + br_mrp_mrm_process(mrp, p, skb);
> > + return 1;
> > + }
> > +
> > + /* Clone the frame and forward it on the other MRP port */
> > + nskb = skb_clone(skb, GFP_ATOMIC);
> > + if (!nskb)
> > + return 0;
> > +
> > + p_dev = p_port->dev;
> > + s_dev = s_port->dev;
> > +
> > + if (p_dev == dev)
> > + d_dev = s_dev;
> > + else
> > + d_dev = p_dev;
> > +
> > + nskb->dev = d_dev;
> > + skb_push(nskb, ETH_HLEN);
> > + dev_queue_xmit(nskb);
> > +
> > + return 1;
> > +}
> > +
> > +/* Check if the frame was received on a port that is part of MRP ring
> > + * and if the frame has MRP eth. In that case process the frame otherwise do
> > + * normal forwarding.
> > + * note: already called with rcu_read_lock
> > + */
> > +int br_mrp_process(struct net_bridge_port *p, struct sk_buff *skb)
> > +{
> > + /* If there is no MRP instance do normal forwarding */
> > + if (likely(!(p->flags & BR_MRP_AWARE)))
> > + goto out;
> > +
> > + if (unlikely(skb->protocol == htons(ETH_P_MRP)))
> > + return br_mrp_rcv(p, skb, p->dev);
> > +
> > +out:
> > + return 0;
> > +}
> > +
> > +bool br_mrp_enabled(struct net_bridge *br)
> > +{
> > + return !list_empty(&br->mrp_list);
> > +}
> >
>

--
/Horatiu