[PATCH net-next v4 00/12] bridge: mrp: Add support for interconnect ring

[PATCH net-next v4 00/12] bridge: mrp: Add support for interconnect ring

[Horatiu Vultur]

This patch series extends existing MRP to add support for interconnect ring.  An
interconnect ring is a ring that connects 2 rings. In this way is possible to
connect multiple rings. Each interconnect ring is form of 4 nodes, in which 3
have the role MIC(Media Redundancy Interconnect Client) and one has the role
MIM(Media Redundancy Interconnect Manager). All these nodes need to have the
same ID and the ID needs to be unique between multiple interconnect rings. And 2
nodes needs to be part of one ring and the other 2 nodes needs to be part of the
other ring that is connected.

                 +---------+
                 |         |
      +----------|   MRM   |---------------+
      |          |         |               |
      |          +---------+               |
      |                                    |
      |                                    |
      |                                    |
+--------------+                  +-----------------+
|              |                  |                 |
|  MRC/MIC     |------------------|    MRC/MIM      |
|              |                  |                 |
+--------------+                  +-----------------+
      |                                     |
      |Interconnect port                    |Interconnect port
      |                                     |
      |                                     |
+--------------+                  +-----------------+
|              |                  |                 |
|  MRC/MIC     |----------------- |   MRC/MIC       |
|              |                  |                 |
+--------------+                  +-----------------+
      |                                     |
      |                                     |
      |          +---------+                |
      |          |         |                |
      +----------|  MRM    |----------------+
                 |         |
                 +---------+

Each node in a ring needs to have one of the following ring roles, MRM or MRC.
And it can also have an interconnect role like MIM or MIC if it is part of an
interconnect ring. In the figure above the MRM doesn't have any interconnect
role but the MRC from the top ring have the interconnect roles MIC respectively
MIM. Therefore it is not possible for a node to have only an interconnect role.

There are 2 ways for interconnect ring to detect when is open or closed:
1. To use CCM frames on the interconnect port to detect when the interconnect
   link goes down/up. This mode is called LC-mode.
2. To send InTest frames on all 3 ports(2 ring ports and 1 interconnect port)
   and detect when these frames are received back. This mode is called RC-mode.

This patch series adds support only for RC-mode. Where MIM sends InTest frames
on all 3 ports and detects when it receives back the InTest. When it receives
the InTest it means that the ring is closed so it would set the interconnect
port in blocking state. If it stops receiving the InTest frames then it would
set the port in forwarding state and it would send InTopo frames. These InTopo
frames will be received by MRM nodes and process them. And then the MRM will
send Topo frames in the rings so each client will clear its FDB.

v4:
  - always cancel delay work if the MRP instance is deleted or interconnect role
    is disabled but allow only to start to send InTest frames only if the role
    is MIM.

v3:
  - update 'br_mrp_set_in_role' to stop sending test if the role is disabled
    and don't allow to set a different interconnect port if there is already
    one.

v2:
  - rearrange structures not to contain holes
  - stop sending MRP_InTest frames when the MRP instance is deleted

Horatiu Vultur (12):
  switchdev: mrp: Extend switchdev API for MRP Interconnect
  bridge: uapi: mrp: Extend MRP attributes for MRP interconnect
  bridge: mrp: Extend bridge interface
  bridge: mrp: Extend br_mrp for MRP interconnect
  bridge: mrp: Rename br_mrp_port_open to br_mrp_ring_port_open
  bridge: mrp: Add br_mrp_in_port_open function
  bridge: switchdev: mrp: Extend MRP API for switchdev for MRP
    Interconnect
  bridge: mrp: Implement the MRP Interconnect API
  bridge: mrp: Extend MRP netlink interface for configuring MRP
    interconnect
  bridge: uapi: mrp: Extend MRP_INFO attributes for interconnect status
  bridge: mrp: Extend br_mrp_fill_info
  net: bridge: Add port attribute IFLA_BRPORT_MRP_IN_OPEN

 include/linux/if_bridge.h          |   1 +
 include/net/switchdev.h            |  38 ++
 include/uapi/linux/if_bridge.h     |  58 +++
 include/uapi/linux/if_link.h       |   1 +
 include/uapi/linux/mrp_bridge.h    |  38 ++
 net/bridge/br_mrp.c                | 576 +++++++++++++++++++++++++++--
 net/bridge/br_mrp_netlink.c        | 182 ++++++++-
 net/bridge/br_mrp_switchdev.c      |  62 ++++
 net/bridge/br_netlink.c            |   3 +
 net/bridge/br_private_mrp.h        |  27 +-
 tools/include/uapi/linux/if_link.h |   1 +
 11 files changed, 951 insertions(+), 36 deletions(-)

-- 
2.27.0

[PATCH net-next v4 01/12] switchdev: mrp: Extend switchdev API for MRP Interconnect

[Horatiu Vultur]

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

SWITCHDEV_OBJ_ID_IN_ROLE_MRP: This is used when the interconnect role
  of the node changes. The supported roles are MIM and MIC.

SWITCHDEV_OBJ_ID_IN_STATE_MRP: This is used when the interconnect ring
  changes it states to open or closed.

SWITCHDEV_OBJ_ID_IN_TEST_MRP: This is used to start/stop sending
  MRP_InTest frames on all MRP ports. This is called only on nodes that
  have the interconnect role MIM.

Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
---
 include/net/switchdev.h | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index b8c059b4e06d9..ff22469143013 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -76,6 +76,10 @@ enum switchdev_obj_id {
 	SWITCHDEV_OBJ_ID_RING_TEST_MRP,
 	SWITCHDEV_OBJ_ID_RING_ROLE_MRP,
 	SWITCHDEV_OBJ_ID_RING_STATE_MRP,
+	SWITCHDEV_OBJ_ID_IN_TEST_MRP,
+	SWITCHDEV_OBJ_ID_IN_ROLE_MRP,
+	SWITCHDEV_OBJ_ID_IN_STATE_MRP,
+
 #endif
 };
 
@@ -155,6 +159,40 @@ struct switchdev_obj_ring_state_mrp {
 #define SWITCHDEV_OBJ_RING_STATE_MRP(OBJ) \
 	container_of((OBJ), struct switchdev_obj_ring_state_mrp, obj)
 
+/* SWITCHDEV_OBJ_ID_IN_TEST_MRP */
+struct switchdev_obj_in_test_mrp {
+	struct switchdev_obj obj;
+	/* The value is in us and a value of 0 represents to stop */
+	u32 interval;
+	u32 in_id;
+	u32 period;
+	u8 max_miss;
+};
+
+#define SWITCHDEV_OBJ_IN_TEST_MRP(OBJ) \
+	container_of((OBJ), struct switchdev_obj_in_test_mrp, obj)
+
+/* SWICHDEV_OBJ_ID_IN_ROLE_MRP */
+struct switchdev_obj_in_role_mrp {
+	struct switchdev_obj obj;
+	struct net_device *i_port;
+	u32 ring_id;
+	u16 in_id;
+	u8 in_role;
+};
+
+#define SWITCHDEV_OBJ_IN_ROLE_MRP(OBJ) \
+	container_of((OBJ), struct switchdev_obj_in_role_mrp, obj)
+
+struct switchdev_obj_in_state_mrp {
+	struct switchdev_obj obj;
+	u32 in_id;
+	u8 in_state;
+};
+
+#define SWITCHDEV_OBJ_IN_STATE_MRP(OBJ) \
+	container_of((OBJ), struct switchdev_obj_in_state_mrp, obj)
+
 #endif
 
 typedef int switchdev_obj_dump_cb_t(struct switchdev_obj *obj);
-- 
2.27.0

Re: [PATCH net-next v4 01/12] switchdev: mrp: Extend switchdev API for MRP Interconnect

[Nikolay Aleksandrov]

On 14/07/2020 10:34, Horatiu Vultur wrote:
> Extend switchdev API to add support for MRP interconnect. The HW is
> notified in the following cases:
> 
> SWITCHDEV_OBJ_ID_IN_ROLE_MRP: This is used when the interconnect role
>   of the node changes. The supported roles are MIM and MIC.
> 
> SWITCHDEV_OBJ_ID_IN_STATE_MRP: This is used when the interconnect ring
>   changes it states to open or closed.
> 
> SWITCHDEV_OBJ_ID_IN_TEST_MRP: This is used to start/stop sending
>   MRP_InTest frames on all MRP ports. This is called only on nodes that
>   have the interconnect role MIM.
> 
> Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> ---
>  include/net/switchdev.h | 38 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
> 

Reviewed-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>

> diff --git a/include/net/switchdev.h b/include/net/switchdev.h
> index b8c059b4e06d9..ff22469143013 100644
> --- a/include/net/switchdev.h
> +++ b/include/net/switchdev.h
> @@ -76,6 +76,10 @@ enum switchdev_obj_id {
>  	SWITCHDEV_OBJ_ID_RING_TEST_MRP,
>  	SWITCHDEV_OBJ_ID_RING_ROLE_MRP,
>  	SWITCHDEV_OBJ_ID_RING_STATE_MRP,
> +	SWITCHDEV_OBJ_ID_IN_TEST_MRP,
> +	SWITCHDEV_OBJ_ID_IN_ROLE_MRP,
> +	SWITCHDEV_OBJ_ID_IN_STATE_MRP,
> +
>  #endif
>  };
>  
> @@ -155,6 +159,40 @@ struct switchdev_obj_ring_state_mrp {
>  #define SWITCHDEV_OBJ_RING_STATE_MRP(OBJ) \
>  	container_of((OBJ), struct switchdev_obj_ring_state_mrp, obj)
>  
> +/* SWITCHDEV_OBJ_ID_IN_TEST_MRP */
> +struct switchdev_obj_in_test_mrp {
> +	struct switchdev_obj obj;
> +	/* The value is in us and a value of 0 represents to stop */
> +	u32 interval;
> +	u32 in_id;
> +	u32 period;
> +	u8 max_miss;
> +};
> +
> +#define SWITCHDEV_OBJ_IN_TEST_MRP(OBJ) \
> +	container_of((OBJ), struct switchdev_obj_in_test_mrp, obj)
> +
> +/* SWICHDEV_OBJ_ID_IN_ROLE_MRP */
> +struct switchdev_obj_in_role_mrp {
> +	struct switchdev_obj obj;
> +	struct net_device *i_port;
> +	u32 ring_id;
> +	u16 in_id;
> +	u8 in_role;
> +};
> +
> +#define SWITCHDEV_OBJ_IN_ROLE_MRP(OBJ) \
> +	container_of((OBJ), struct switchdev_obj_in_role_mrp, obj)
> +
> +struct switchdev_obj_in_state_mrp {
> +	struct switchdev_obj obj;
> +	u32 in_id;
> +	u8 in_state;
> +};
> +
> +#define SWITCHDEV_OBJ_IN_STATE_MRP(OBJ) \
> +	container_of((OBJ), struct switchdev_obj_in_state_mrp, obj)
> +
>  #endif
>  
>  typedef int switchdev_obj_dump_cb_t(struct switchdev_obj *obj);
> 

[PATCH net-next v4 02/12] bridge: uapi: mrp: Extend MRP attributes for MRP interconnect

[Horatiu Vultur]

Extend the existing MRP netlink attributes to allow to configure MRP
Interconnect:

IFLA_BRIDGE_MRP_IN_ROLE - the parameter type is br_mrp_in_role which
  contains the interconnect id, the ring id, the interconnect role(MIM
  or MIC) and the port ifindex that represents the interconnect port.

IFLA_BRIDGE_MRP_IN_STATE - the parameter type is br_mrp_in_state which
  contains the interconnect id and the interconnect state.

IFLA_BRIDGE_MRP_IN_TEST - the parameter type is br_mrp_start_in_test
  which contains the interconnect id, the interval at which to send
  MRP_InTest frames, how many test frames can be missed before declaring
  the interconnect ring open and the period which represents for how long
  to send MRP_InTest frames.

Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
---
 include/uapi/linux/if_bridge.h  | 53 +++++++++++++++++++++++++++++++++
 include/uapi/linux/mrp_bridge.h | 38 +++++++++++++++++++++++
 2 files changed, 91 insertions(+)

diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
index c114c1c2bd533..d840a3e37a37c 100644
--- a/include/uapi/linux/if_bridge.h
+++ b/include/uapi/linux/if_bridge.h
@@ -167,6 +167,9 @@ enum {
 	IFLA_BRIDGE_MRP_RING_ROLE,
 	IFLA_BRIDGE_MRP_START_TEST,
 	IFLA_BRIDGE_MRP_INFO,
+	IFLA_BRIDGE_MRP_IN_ROLE,
+	IFLA_BRIDGE_MRP_IN_STATE,
+	IFLA_BRIDGE_MRP_START_IN_TEST,
 	__IFLA_BRIDGE_MRP_MAX,
 };
 
@@ -245,6 +248,37 @@ enum {
 
 #define IFLA_BRIDGE_MRP_INFO_MAX (__IFLA_BRIDGE_MRP_INFO_MAX - 1)
 
+enum {
+	IFLA_BRIDGE_MRP_IN_STATE_UNSPEC,
+	IFLA_BRIDGE_MRP_IN_STATE_IN_ID,
+	IFLA_BRIDGE_MRP_IN_STATE_STATE,
+	__IFLA_BRIDGE_MRP_IN_STATE_MAX,
+};
+
+#define IFLA_BRIDGE_MRP_IN_STATE_MAX (__IFLA_BRIDGE_MRP_IN_STATE_MAX - 1)
+
+enum {
+	IFLA_BRIDGE_MRP_IN_ROLE_UNSPEC,
+	IFLA_BRIDGE_MRP_IN_ROLE_RING_ID,
+	IFLA_BRIDGE_MRP_IN_ROLE_IN_ID,
+	IFLA_BRIDGE_MRP_IN_ROLE_ROLE,
+	IFLA_BRIDGE_MRP_IN_ROLE_I_IFINDEX,
+	__IFLA_BRIDGE_MRP_IN_ROLE_MAX,
+};
+
+#define IFLA_BRIDGE_MRP_IN_ROLE_MAX (__IFLA_BRIDGE_MRP_IN_ROLE_MAX - 1)
+
+enum {
+	IFLA_BRIDGE_MRP_START_IN_TEST_UNSPEC,
+	IFLA_BRIDGE_MRP_START_IN_TEST_IN_ID,
+	IFLA_BRIDGE_MRP_START_IN_TEST_INTERVAL,
+	IFLA_BRIDGE_MRP_START_IN_TEST_MAX_MISS,
+	IFLA_BRIDGE_MRP_START_IN_TEST_PERIOD,
+	__IFLA_BRIDGE_MRP_START_IN_TEST_MAX,
+};
+
+#define IFLA_BRIDGE_MRP_START_IN_TEST_MAX (__IFLA_BRIDGE_MRP_START_IN_TEST_MAX - 1)
+
 struct br_mrp_instance {
 	__u32 ring_id;
 	__u32 p_ifindex;
@@ -270,6 +304,25 @@ struct br_mrp_start_test {
 	__u32 monitor;
 };
 
+struct br_mrp_in_state {
+	__u32 in_state;
+	__u16 in_id;
+};
+
+struct br_mrp_in_role {
+	__u32 ring_id;
+	__u32 in_role;
+	__u32 i_ifindex;
+	__u16 in_id;
+};
+
+struct br_mrp_start_in_test {
+	__u32 interval;
+	__u32 max_miss;
+	__u32 period;
+	__u16 in_id;
+};
+
 struct bridge_stp_xstats {
 	__u64 transition_blk;
 	__u64 transition_fwd;
diff --git a/include/uapi/linux/mrp_bridge.h b/include/uapi/linux/mrp_bridge.h
index bee3665402129..6aeb13ef0b1ec 100644
--- a/include/uapi/linux/mrp_bridge.h
+++ b/include/uapi/linux/mrp_bridge.h
@@ -21,11 +21,22 @@ enum br_mrp_ring_role_type {
 	BR_MRP_RING_ROLE_MRA,
 };
 
+enum br_mrp_in_role_type {
+	BR_MRP_IN_ROLE_DISABLED,
+	BR_MRP_IN_ROLE_MIC,
+	BR_MRP_IN_ROLE_MIM,
+};
+
 enum br_mrp_ring_state_type {
 	BR_MRP_RING_STATE_OPEN,
 	BR_MRP_RING_STATE_CLOSED,
 };
 
+enum br_mrp_in_state_type {
+	BR_MRP_IN_STATE_OPEN,
+	BR_MRP_IN_STATE_CLOSED,
+};
+
 enum br_mrp_port_state_type {
 	BR_MRP_PORT_STATE_DISABLED,
 	BR_MRP_PORT_STATE_BLOCKED,
@@ -36,6 +47,7 @@ enum br_mrp_port_state_type {
 enum br_mrp_port_role_type {
 	BR_MRP_PORT_ROLE_PRIMARY,
 	BR_MRP_PORT_ROLE_SECONDARY,
+	BR_MRP_PORT_ROLE_INTER,
 };
 
 enum br_mrp_tlv_header_type {
@@ -45,6 +57,10 @@ enum br_mrp_tlv_header_type {
 	BR_MRP_TLV_HEADER_RING_TOPO = 0x3,
 	BR_MRP_TLV_HEADER_RING_LINK_DOWN = 0x4,
 	BR_MRP_TLV_HEADER_RING_LINK_UP = 0x5,
+	BR_MRP_TLV_HEADER_IN_TEST = 0x6,
+	BR_MRP_TLV_HEADER_IN_TOPO = 0x7,
+	BR_MRP_TLV_HEADER_IN_LINK_DOWN = 0x8,
+	BR_MRP_TLV_HEADER_IN_LINK_UP = 0x9,
 	BR_MRP_TLV_HEADER_OPTION = 0x7f,
 };
 
@@ -118,4 +134,26 @@ struct br_mrp_oui_hdr {
 	__u8 oui[MRP_OUI_LENGTH];
 };
 
+struct br_mrp_in_test_hdr {
+	__be16 id;
+	__u8 sa[ETH_ALEN];
+	__be16 port_role;
+	__be16 state;
+	__be16 transitions;
+	__be32 timestamp;
+};
+
+struct br_mrp_in_topo_hdr {
+	__u8 sa[ETH_ALEN];
+	__be16 id;
+	__be16 interval;
+};
+
+struct br_mrp_in_link_hdr {
+	__u8 sa[ETH_ALEN];
+	__be16 port_role;
+	__be16 id;
+	__be16 interval;
+};
+
 #endif
-- 
2.27.0

Re: [PATCH net-next v4 02/12] bridge: uapi: mrp: Extend MRP attributes for MRP interconnect

[Nikolay Aleksandrov]

On 14/07/2020 10:34, Horatiu Vultur wrote:
> Extend the existing MRP netlink attributes to allow to configure MRP
> Interconnect:
> 
> IFLA_BRIDGE_MRP_IN_ROLE - the parameter type is br_mrp_in_role which
>   contains the interconnect id, the ring id, the interconnect role(MIM
>   or MIC) and the port ifindex that represents the interconnect port.
> 
> IFLA_BRIDGE_MRP_IN_STATE - the parameter type is br_mrp_in_state which
>   contains the interconnect id and the interconnect state.
> 
> IFLA_BRIDGE_MRP_IN_TEST - the parameter type is br_mrp_start_in_test
>   which contains the interconnect id, the interval at which to send
>   MRP_InTest frames, how many test frames can be missed before declaring
>   the interconnect ring open and the period which represents for how long
>   to send MRP_InTest frames.
> 
> Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> ---
>  include/uapi/linux/if_bridge.h  | 53 +++++++++++++++++++++++++++++++++
>  include/uapi/linux/mrp_bridge.h | 38 +++++++++++++++++++++++
>  2 files changed, 91 insertions(+)
> 

Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>

> diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
> index c114c1c2bd533..d840a3e37a37c 100644
> --- a/include/uapi/linux/if_bridge.h
> +++ b/include/uapi/linux/if_bridge.h
> @@ -167,6 +167,9 @@ enum {
>  	IFLA_BRIDGE_MRP_RING_ROLE,
>  	IFLA_BRIDGE_MRP_START_TEST,
>  	IFLA_BRIDGE_MRP_INFO,
> +	IFLA_BRIDGE_MRP_IN_ROLE,
> +	IFLA_BRIDGE_MRP_IN_STATE,
> +	IFLA_BRIDGE_MRP_START_IN_TEST,
>  	__IFLA_BRIDGE_MRP_MAX,
>  };
>  
> @@ -245,6 +248,37 @@ enum {
>  
>  #define IFLA_BRIDGE_MRP_INFO_MAX (__IFLA_BRIDGE_MRP_INFO_MAX - 1)
>  
> +enum {
> +	IFLA_BRIDGE_MRP_IN_STATE_UNSPEC,
> +	IFLA_BRIDGE_MRP_IN_STATE_IN_ID,
> +	IFLA_BRIDGE_MRP_IN_STATE_STATE,
> +	__IFLA_BRIDGE_MRP_IN_STATE_MAX,
> +};
> +
> +#define IFLA_BRIDGE_MRP_IN_STATE_MAX (__IFLA_BRIDGE_MRP_IN_STATE_MAX - 1)
> +
> +enum {
> +	IFLA_BRIDGE_MRP_IN_ROLE_UNSPEC,
> +	IFLA_BRIDGE_MRP_IN_ROLE_RING_ID,
> +	IFLA_BRIDGE_MRP_IN_ROLE_IN_ID,
> +	IFLA_BRIDGE_MRP_IN_ROLE_ROLE,
> +	IFLA_BRIDGE_MRP_IN_ROLE_I_IFINDEX,
> +	__IFLA_BRIDGE_MRP_IN_ROLE_MAX,
> +};
> +
> +#define IFLA_BRIDGE_MRP_IN_ROLE_MAX (__IFLA_BRIDGE_MRP_IN_ROLE_MAX - 1)
> +
> +enum {
> +	IFLA_BRIDGE_MRP_START_IN_TEST_UNSPEC,
> +	IFLA_BRIDGE_MRP_START_IN_TEST_IN_ID,
> +	IFLA_BRIDGE_MRP_START_IN_TEST_INTERVAL,
> +	IFLA_BRIDGE_MRP_START_IN_TEST_MAX_MISS,
> +	IFLA_BRIDGE_MRP_START_IN_TEST_PERIOD,
> +	__IFLA_BRIDGE_MRP_START_IN_TEST_MAX,
> +};
> +
> +#define IFLA_BRIDGE_MRP_START_IN_TEST_MAX (__IFLA_BRIDGE_MRP_START_IN_TEST_MAX - 1)
> +
>  struct br_mrp_instance {
>  	__u32 ring_id;
>  	__u32 p_ifindex;
> @@ -270,6 +304,25 @@ struct br_mrp_start_test {
>  	__u32 monitor;
>  };
>  
> +struct br_mrp_in_state {
> +	__u32 in_state;
> +	__u16 in_id;
> +};
> +
> +struct br_mrp_in_role {
> +	__u32 ring_id;
> +	__u32 in_role;
> +	__u32 i_ifindex;
> +	__u16 in_id;
> +};
> +
> +struct br_mrp_start_in_test {
> +	__u32 interval;
> +	__u32 max_miss;
> +	__u32 period;
> +	__u16 in_id;
> +};
> +
>  struct bridge_stp_xstats {
>  	__u64 transition_blk;
>  	__u64 transition_fwd;
> diff --git a/include/uapi/linux/mrp_bridge.h b/include/uapi/linux/mrp_bridge.h
> index bee3665402129..6aeb13ef0b1ec 100644
> --- a/include/uapi/linux/mrp_bridge.h
> +++ b/include/uapi/linux/mrp_bridge.h
> @@ -21,11 +21,22 @@ enum br_mrp_ring_role_type {
>  	BR_MRP_RING_ROLE_MRA,
>  };
>  
> +enum br_mrp_in_role_type {
> +	BR_MRP_IN_ROLE_DISABLED,
> +	BR_MRP_IN_ROLE_MIC,
> +	BR_MRP_IN_ROLE_MIM,
> +};
> +
>  enum br_mrp_ring_state_type {
>  	BR_MRP_RING_STATE_OPEN,
>  	BR_MRP_RING_STATE_CLOSED,
>  };
>  
> +enum br_mrp_in_state_type {
> +	BR_MRP_IN_STATE_OPEN,
> +	BR_MRP_IN_STATE_CLOSED,
> +};
> +
>  enum br_mrp_port_state_type {
>  	BR_MRP_PORT_STATE_DISABLED,
>  	BR_MRP_PORT_STATE_BLOCKED,
> @@ -36,6 +47,7 @@ enum br_mrp_port_state_type {
>  enum br_mrp_port_role_type {
>  	BR_MRP_PORT_ROLE_PRIMARY,
>  	BR_MRP_PORT_ROLE_SECONDARY,
> +	BR_MRP_PORT_ROLE_INTER,
>  };
>  
>  enum br_mrp_tlv_header_type {
> @@ -45,6 +57,10 @@ enum br_mrp_tlv_header_type {
>  	BR_MRP_TLV_HEADER_RING_TOPO = 0x3,
>  	BR_MRP_TLV_HEADER_RING_LINK_DOWN = 0x4,
>  	BR_MRP_TLV_HEADER_RING_LINK_UP = 0x5,
> +	BR_MRP_TLV_HEADER_IN_TEST = 0x6,
> +	BR_MRP_TLV_HEADER_IN_TOPO = 0x7,
> +	BR_MRP_TLV_HEADER_IN_LINK_DOWN = 0x8,
> +	BR_MRP_TLV_HEADER_IN_LINK_UP = 0x9,
>  	BR_MRP_TLV_HEADER_OPTION = 0x7f,
>  };
>  
> @@ -118,4 +134,26 @@ struct br_mrp_oui_hdr {
>  	__u8 oui[MRP_OUI_LENGTH];
>  };
>  
> +struct br_mrp_in_test_hdr {
> +	__be16 id;
> +	__u8 sa[ETH_ALEN];
> +	__be16 port_role;
> +	__be16 state;
> +	__be16 transitions;
> +	__be32 timestamp;
> +};
> +
> +struct br_mrp_in_topo_hdr {
> +	__u8 sa[ETH_ALEN];
> +	__be16 id;
> +	__be16 interval;
> +};
> +
> +struct br_mrp_in_link_hdr {
> +	__u8 sa[ETH_ALEN];
> +	__be16 port_role;
> +	__be16 id;
> +	__be16 interval;
> +};
> +
>  #endif
> 

[PATCH net-next v4 03/12] bridge: mrp: Extend bridge interface

[Horatiu Vultur]

This patch adds a new flag(BR_MRP_LOST_IN_CONT) to the net bridge
ports. This bit will be set when the port lost the continuity of
MRP_InTest frames.

Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
---
 include/linux/if_bridge.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
index b3a8d3054af0f..6479a38e52fa9 100644
--- a/include/linux/if_bridge.h
+++ b/include/linux/if_bridge.h
@@ -49,6 +49,7 @@ struct br_ip_list {
 #define BR_ISOLATED		BIT(16)
 #define BR_MRP_AWARE		BIT(17)
 #define BR_MRP_LOST_CONT	BIT(18)
+#define BR_MRP_LOST_IN_CONT	BIT(19)
 
 #define BR_DEFAULT_AGEING_TIME	(300 * HZ)
 
-- 
2.27.0

Re: [PATCH net-next v4 03/12] bridge: mrp: Extend bridge interface

[Nikolay Aleksandrov]

On 14/07/2020 10:34, Horatiu Vultur wrote:
> This patch adds a new flag(BR_MRP_LOST_IN_CONT) to the net bridge
> ports. This bit will be set when the port lost the continuity of
> MRP_InTest frames.
> 
> Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> ---
>  include/linux/if_bridge.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
> index b3a8d3054af0f..6479a38e52fa9 100644
> --- a/include/linux/if_bridge.h
> +++ b/include/linux/if_bridge.h
> @@ -49,6 +49,7 @@ struct br_ip_list {
>  #define BR_ISOLATED		BIT(16)
>  #define BR_MRP_AWARE		BIT(17)
>  #define BR_MRP_LOST_CONT	BIT(18)
> +#define BR_MRP_LOST_IN_CONT	BIT(19)
>  
>  #define BR_DEFAULT_AGEING_TIME	(300 * HZ)
>  
> 

Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>

[PATCH net-next v4 04/12] bridge: mrp: Extend br_mrp for MRP interconnect

[Horatiu Vultur]

This patch extends the 'struct br_mrp' to contain information regarding
the MRP interconnect. It contains the following:
- the interconnect port 'i_port', which is NULL if the node doesn't have
  a interconnect role
- the interconnect id, which is similar with the ring id, but this field
  is also part of the MRP_InTest frames.
- the interconnect role, which can be MIM or MIC.
- the interconnect state, which can be open or closed.
- the interconnect delayed_work for sending MRP_InTest frames and check
  for lost of continuity.

Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
---
 net/bridge/br_private_mrp.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/net/bridge/br_private_mrp.h b/net/bridge/br_private_mrp.h
index 315eb37d89f0f..8841ba847fb29 100644
--- a/net/bridge/br_private_mrp.h
+++ b/net/bridge/br_private_mrp.h
@@ -12,8 +12,10 @@ struct br_mrp {
 
 	struct net_bridge_port __rcu	*p_port;
 	struct net_bridge_port __rcu	*s_port;
+	struct net_bridge_port __rcu	*i_port;
 
 	u32				ring_id;
+	u16				in_id;
 	u16				prio;
 
 	enum br_mrp_ring_role_type	ring_role;
@@ -21,6 +23,11 @@ struct br_mrp {
 	enum br_mrp_ring_state_type	ring_state;
 	u32				ring_transitions;
 
+	enum br_mrp_in_role_type	in_role;
+	u8				in_role_offloaded;
+	enum br_mrp_in_state_type	in_state;
+	u32				in_transitions;
+
 	struct delayed_work		test_work;
 	u32				test_interval;
 	unsigned long			test_end;
@@ -28,6 +35,12 @@ struct br_mrp {
 	u32				test_max_miss;
 	bool				test_monitor;
 
+	struct delayed_work		in_test_work;
+	u32				in_test_interval;
+	unsigned long			in_test_end;
+	u32				in_test_count_miss;
+	u32				in_test_max_miss;
+
 	u32				seq_id;
 
 	struct rcu_head			rcu;
-- 
2.27.0

Re: [PATCH net-next v4 04/12] bridge: mrp: Extend br_mrp for MRP interconnect

[Nikolay Aleksandrov]

On 14/07/2020 10:34, Horatiu Vultur wrote:
> This patch extends the 'struct br_mrp' to contain information regarding
> the MRP interconnect. It contains the following:
> - the interconnect port 'i_port', which is NULL if the node doesn't have
>   a interconnect role
> - the interconnect id, which is similar with the ring id, but this field
>   is also part of the MRP_InTest frames.
> - the interconnect role, which can be MIM or MIC.
> - the interconnect state, which can be open or closed.
> - the interconnect delayed_work for sending MRP_InTest frames and check
>   for lost of continuity.
> 
> Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> ---
>  net/bridge/br_private_mrp.h | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 

Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>

> diff --git a/net/bridge/br_private_mrp.h b/net/bridge/br_private_mrp.h
> index 315eb37d89f0f..8841ba847fb29 100644
> --- a/net/bridge/br_private_mrp.h
> +++ b/net/bridge/br_private_mrp.h
> @@ -12,8 +12,10 @@ struct br_mrp {
>  
>  	struct net_bridge_port __rcu	*p_port;
>  	struct net_bridge_port __rcu	*s_port;
> +	struct net_bridge_port __rcu	*i_port;
>  
>  	u32				ring_id;
> +	u16				in_id;
>  	u16				prio;
>  
>  	enum br_mrp_ring_role_type	ring_role;
> @@ -21,6 +23,11 @@ struct br_mrp {
>  	enum br_mrp_ring_state_type	ring_state;
>  	u32				ring_transitions;
>  
> +	enum br_mrp_in_role_type	in_role;
> +	u8				in_role_offloaded;
> +	enum br_mrp_in_state_type	in_state;
> +	u32				in_transitions;
> +
>  	struct delayed_work		test_work;
>  	u32				test_interval;
>  	unsigned long			test_end;
> @@ -28,6 +35,12 @@ struct br_mrp {
>  	u32				test_max_miss;
>  	bool				test_monitor;
>  
> +	struct delayed_work		in_test_work;
> +	u32				in_test_interval;
> +	unsigned long			in_test_end;
> +	u32				in_test_count_miss;
> +	u32				in_test_max_miss;
> +
>  	u32				seq_id;
>  
>  	struct rcu_head			rcu;
> 

[PATCH net-next v4 05/12] bridge: mrp: Rename br_mrp_port_open to br_mrp_ring_port_open

[Horatiu Vultur]

This patch renames the function br_mrp_port_open to
br_mrp_ring_port_open. In this way is more clear that a ring port lost
the continuity because there will be also a br_mrp_in_port_open.

Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
---
 net/bridge/br_mrp.c         | 6 +++---
 net/bridge/br_mrp_netlink.c | 2 +-
 net/bridge/br_private_mrp.h | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/net/bridge/br_mrp.c b/net/bridge/br_mrp.c
index 90592af9db619..fe7cf1446b58a 100644
--- a/net/bridge/br_mrp.c
+++ b/net/bridge/br_mrp.c
@@ -213,7 +213,7 @@ static void br_mrp_test_work_expired(struct work_struct *work)
 		}
 
 		if (notify_open && !mrp->ring_role_offloaded)
-			br_mrp_port_open(p->dev, true);
+			br_mrp_ring_port_open(p->dev, true);
 	}
 
 	p = rcu_dereference(mrp->s_port);
@@ -229,7 +229,7 @@ static void br_mrp_test_work_expired(struct work_struct *work)
 		}
 
 		if (notify_open && !mrp->ring_role_offloaded)
-			br_mrp_port_open(p->dev, true);
+			br_mrp_ring_port_open(p->dev, true);
 	}
 
 out:
@@ -537,7 +537,7 @@ static void br_mrp_mrm_process(struct br_mrp *mrp, struct net_bridge_port *port,
 	 * not closed
 	 */
 	if (mrp->ring_state != BR_MRP_RING_STATE_CLOSED)
-		br_mrp_port_open(port->dev, false);
+		br_mrp_ring_port_open(port->dev, false);
 }
 
 /* Determin if the test hdr has a better priority than the node */
diff --git a/net/bridge/br_mrp_netlink.c b/net/bridge/br_mrp_netlink.c
index c4f5c356811f3..acce300c0cc29 100644
--- a/net/bridge/br_mrp_netlink.c
+++ b/net/bridge/br_mrp_netlink.c
@@ -368,7 +368,7 @@ int br_mrp_fill_info(struct sk_buff *skb, struct net_bridge *br)
 	return -EMSGSIZE;
 }
 
-int br_mrp_port_open(struct net_device *dev, u8 loc)
+int br_mrp_ring_port_open(struct net_device *dev, u8 loc)
 {
 	struct net_bridge_port *p;
 	int err = 0;
diff --git a/net/bridge/br_private_mrp.h b/net/bridge/br_private_mrp.h
index 8841ba847fb29..e93c8f9d4df58 100644
--- a/net/bridge/br_private_mrp.h
+++ b/net/bridge/br_private_mrp.h
@@ -74,6 +74,6 @@ int br_mrp_port_switchdev_set_role(struct net_bridge_port *p,
 				   enum br_mrp_port_role_type role);
 
 /* br_mrp_netlink.c  */
-int br_mrp_port_open(struct net_device *dev, u8 loc);
+int br_mrp_ring_port_open(struct net_device *dev, u8 loc);
 
 #endif /* _BR_PRIVATE_MRP_H */
-- 
2.27.0

Re: [PATCH net-next v4 05/12] bridge: mrp: Rename br_mrp_port_open to br_mrp_ring_port_open

[Nikolay Aleksandrov]

On 14/07/2020 10:34, Horatiu Vultur wrote:
> This patch renames the function br_mrp_port_open to
> br_mrp_ring_port_open. In this way is more clear that a ring port lost
> the continuity because there will be also a br_mrp_in_port_open.
> 
> Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> ---
>  net/bridge/br_mrp.c         | 6 +++---
>  net/bridge/br_mrp_netlink.c | 2 +-
>  net/bridge/br_private_mrp.h | 2 +-
>  3 files changed, 5 insertions(+), 5 deletions(-)
> 

Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>

> diff --git a/net/bridge/br_mrp.c b/net/bridge/br_mrp.c
> index 90592af9db619..fe7cf1446b58a 100644
> --- a/net/bridge/br_mrp.c
> +++ b/net/bridge/br_mrp.c
> @@ -213,7 +213,7 @@ static void br_mrp_test_work_expired(struct work_struct *work)
>  		}
>  
>  		if (notify_open && !mrp->ring_role_offloaded)
> -			br_mrp_port_open(p->dev, true);
> +			br_mrp_ring_port_open(p->dev, true);
>  	}
>  
>  	p = rcu_dereference(mrp->s_port);
> @@ -229,7 +229,7 @@ static void br_mrp_test_work_expired(struct work_struct *work)
>  		}
>  
>  		if (notify_open && !mrp->ring_role_offloaded)
> -			br_mrp_port_open(p->dev, true);
> +			br_mrp_ring_port_open(p->dev, true);
>  	}
>  
>  out:
> @@ -537,7 +537,7 @@ static void br_mrp_mrm_process(struct br_mrp *mrp, struct net_bridge_port *port,
>  	 * not closed
>  	 */
>  	if (mrp->ring_state != BR_MRP_RING_STATE_CLOSED)
> -		br_mrp_port_open(port->dev, false);
> +		br_mrp_ring_port_open(port->dev, false);
>  }
>  
>  /* Determin if the test hdr has a better priority than the node */
> diff --git a/net/bridge/br_mrp_netlink.c b/net/bridge/br_mrp_netlink.c
> index c4f5c356811f3..acce300c0cc29 100644
> --- a/net/bridge/br_mrp_netlink.c
> +++ b/net/bridge/br_mrp_netlink.c
> @@ -368,7 +368,7 @@ int br_mrp_fill_info(struct sk_buff *skb, struct net_bridge *br)
>  	return -EMSGSIZE;
>  }
>  
> -int br_mrp_port_open(struct net_device *dev, u8 loc)
> +int br_mrp_ring_port_open(struct net_device *dev, u8 loc)
>  {
>  	struct net_bridge_port *p;
>  	int err = 0;
> diff --git a/net/bridge/br_private_mrp.h b/net/bridge/br_private_mrp.h
> index 8841ba847fb29..e93c8f9d4df58 100644
> --- a/net/bridge/br_private_mrp.h
> +++ b/net/bridge/br_private_mrp.h
> @@ -74,6 +74,6 @@ int br_mrp_port_switchdev_set_role(struct net_bridge_port *p,
>  				   enum br_mrp_port_role_type role);
>  
>  /* br_mrp_netlink.c  */
> -int br_mrp_port_open(struct net_device *dev, u8 loc);
> +int br_mrp_ring_port_open(struct net_device *dev, u8 loc);
>  
>  #endif /* _BR_PRIVATE_MRP_H */
> 

[PATCH net-next v4 06/12] bridge: mrp: Add br_mrp_in_port_open function

[Horatiu Vultur]

This function notifies the userspace when the node lost the continuity
of MRP_InTest frames.

Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
---
 net/bridge/br_mrp_netlink.c | 22 ++++++++++++++++++++++
 net/bridge/br_private_mrp.h |  1 +
 2 files changed, 23 insertions(+)

diff --git a/net/bridge/br_mrp_netlink.c b/net/bridge/br_mrp_netlink.c
index acce300c0cc29..4bf7aaeb29152 100644
--- a/net/bridge/br_mrp_netlink.c
+++ b/net/bridge/br_mrp_netlink.c
@@ -389,3 +389,25 @@ int br_mrp_ring_port_open(struct net_device *dev, u8 loc)
 out:
 	return err;
 }
+
+int br_mrp_in_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;
+	}
+
+	if (loc)
+		p->flags |= BR_MRP_LOST_IN_CONT;
+	else
+		p->flags &= ~BR_MRP_LOST_IN_CONT;
+
+	br_ifinfo_notify(RTM_NEWLINK, NULL, p);
+
+out:
+	return err;
+}
diff --git a/net/bridge/br_private_mrp.h b/net/bridge/br_private_mrp.h
index e93c8f9d4df58..23da2f956ad0e 100644
--- a/net/bridge/br_private_mrp.h
+++ b/net/bridge/br_private_mrp.h
@@ -75,5 +75,6 @@ int br_mrp_port_switchdev_set_role(struct net_bridge_port *p,
 
 /* br_mrp_netlink.c  */
 int br_mrp_ring_port_open(struct net_device *dev, u8 loc);
+int br_mrp_in_port_open(struct net_device *dev, u8 loc);
 
 #endif /* _BR_PRIVATE_MRP_H */
-- 
2.27.0

Re: [PATCH net-next v4 06/12] bridge: mrp: Add br_mrp_in_port_open function

[Nikolay Aleksandrov]

On 14/07/2020 10:34, Horatiu Vultur wrote:
> This function notifies the userspace when the node lost the continuity
> of MRP_InTest frames.
> 
> Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> ---
>  net/bridge/br_mrp_netlink.c | 22 ++++++++++++++++++++++
>  net/bridge/br_private_mrp.h |  1 +
>  2 files changed, 23 insertions(+)
> 

Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>

> diff --git a/net/bridge/br_mrp_netlink.c b/net/bridge/br_mrp_netlink.c
> index acce300c0cc29..4bf7aaeb29152 100644
> --- a/net/bridge/br_mrp_netlink.c
> +++ b/net/bridge/br_mrp_netlink.c
> @@ -389,3 +389,25 @@ int br_mrp_ring_port_open(struct net_device *dev, u8 loc)
>  out:
>  	return err;
>  }
> +
> +int br_mrp_in_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;
> +	}
> +
> +	if (loc)
> +		p->flags |= BR_MRP_LOST_IN_CONT;
> +	else
> +		p->flags &= ~BR_MRP_LOST_IN_CONT;
> +
> +	br_ifinfo_notify(RTM_NEWLINK, NULL, p);
> +
> +out:
> +	return err;
> +}
> diff --git a/net/bridge/br_private_mrp.h b/net/bridge/br_private_mrp.h
> index e93c8f9d4df58..23da2f956ad0e 100644
> --- a/net/bridge/br_private_mrp.h
> +++ b/net/bridge/br_private_mrp.h
> @@ -75,5 +75,6 @@ int br_mrp_port_switchdev_set_role(struct net_bridge_port *p,
>  
>  /* br_mrp_netlink.c  */
>  int br_mrp_ring_port_open(struct net_device *dev, u8 loc);
> +int br_mrp_in_port_open(struct net_device *dev, u8 loc);
>  
>  #endif /* _BR_PRIVATE_MRP_H */
> 

[PATCH net-next v4 07/12] bridge: switchdev: mrp: Extend MRP API for switchdev for MRP Interconnect

[Horatiu Vultur]

Implement the MRP API for interconnect switchdev. Similar with the other
br_mrp_switchdev function, these function will just eventually call the
switchdev functions: switchdev_port_obj_add/del.

Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
---
 net/bridge/br_mrp_switchdev.c | 62 +++++++++++++++++++++++++++++++++++
 net/bridge/br_private_mrp.h   |  7 ++++
 2 files changed, 69 insertions(+)

diff --git a/net/bridge/br_mrp_switchdev.c b/net/bridge/br_mrp_switchdev.c
index 0da68a0da4b5a..ed547e03ace17 100644
--- a/net/bridge/br_mrp_switchdev.c
+++ b/net/bridge/br_mrp_switchdev.c
@@ -107,6 +107,68 @@ int br_mrp_switchdev_set_ring_state(struct net_bridge *br,
 	return 0;
 }
 
+int br_mrp_switchdev_set_in_role(struct net_bridge *br, struct br_mrp *mrp,
+				 u16 in_id, u32 ring_id,
+				 enum br_mrp_in_role_type role)
+{
+	struct switchdev_obj_in_role_mrp mrp_role = {
+		.obj.orig_dev = br->dev,
+		.obj.id = SWITCHDEV_OBJ_ID_IN_ROLE_MRP,
+		.in_role = role,
+		.in_id = mrp->in_id,
+		.ring_id = mrp->ring_id,
+		.i_port = rtnl_dereference(mrp->i_port)->dev,
+	};
+	int err;
+
+	if (role == BR_MRP_IN_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_set_in_state(struct net_bridge *br, struct br_mrp *mrp,
+				  enum br_mrp_in_state_type state)
+{
+	struct switchdev_obj_in_state_mrp mrp_state = {
+		.obj.orig_dev = br->dev,
+		.obj.id = SWITCHDEV_OBJ_ID_IN_STATE_MRP,
+		.in_state = state,
+		.in_id = mrp->in_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_switchdev_send_in_test(struct net_bridge *br, struct br_mrp *mrp,
+				  u32 interval, u8 max_miss, u32 period)
+{
+	struct switchdev_obj_in_test_mrp test = {
+		.obj.orig_dev = br->dev,
+		.obj.id = SWITCHDEV_OBJ_ID_IN_TEST_MRP,
+		.interval = interval,
+		.max_miss = max_miss,
+		.in_id = mrp->in_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_port_switchdev_set_state(struct net_bridge_port *p,
 				    enum br_mrp_port_state_type state)
 {
diff --git a/net/bridge/br_private_mrp.h b/net/bridge/br_private_mrp.h
index 23da2f956ad0e..0d554ef88db85 100644
--- a/net/bridge/br_private_mrp.h
+++ b/net/bridge/br_private_mrp.h
@@ -72,6 +72,13 @@ 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);
+int br_mrp_switchdev_set_in_role(struct net_bridge *br, struct br_mrp *mrp,
+				 u16 in_id, u32 ring_id,
+				 enum br_mrp_in_role_type role);
+int br_mrp_switchdev_set_in_state(struct net_bridge *br, struct br_mrp *mrp,
+				  enum br_mrp_in_state_type state);
+int br_mrp_switchdev_send_in_test(struct net_bridge *br, struct br_mrp *mrp,
+				  u32 interval, u8 max_miss, u32 period);
 
 /* br_mrp_netlink.c  */
 int br_mrp_ring_port_open(struct net_device *dev, u8 loc);
-- 
2.27.0

Re: [PATCH net-next v4 07/12] bridge: switchdev: mrp: Extend MRP API for switchdev for MRP Interconnect

[Nikolay Aleksandrov]

On 14/07/2020 10:34, Horatiu Vultur wrote:
> Implement the MRP API for interconnect switchdev. Similar with the other
> br_mrp_switchdev function, these function will just eventually call the
> switchdev functions: switchdev_port_obj_add/del.
> 
> Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> ---
>  net/bridge/br_mrp_switchdev.c | 62 +++++++++++++++++++++++++++++++++++
>  net/bridge/br_private_mrp.h   |  7 ++++
>  2 files changed, 69 insertions(+)
> 

Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>

> diff --git a/net/bridge/br_mrp_switchdev.c b/net/bridge/br_mrp_switchdev.c
> index 0da68a0da4b5a..ed547e03ace17 100644
> --- a/net/bridge/br_mrp_switchdev.c
> +++ b/net/bridge/br_mrp_switchdev.c
> @@ -107,6 +107,68 @@ int br_mrp_switchdev_set_ring_state(struct net_bridge *br,
>  	return 0;
>  }
>  
> +int br_mrp_switchdev_set_in_role(struct net_bridge *br, struct br_mrp *mrp,
> +				 u16 in_id, u32 ring_id,
> +				 enum br_mrp_in_role_type role)
> +{
> +	struct switchdev_obj_in_role_mrp mrp_role = {
> +		.obj.orig_dev = br->dev,
> +		.obj.id = SWITCHDEV_OBJ_ID_IN_ROLE_MRP,
> +		.in_role = role,
> +		.in_id = mrp->in_id,
> +		.ring_id = mrp->ring_id,
> +		.i_port = rtnl_dereference(mrp->i_port)->dev,
> +	};
> +	int err;
> +
> +	if (role == BR_MRP_IN_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_set_in_state(struct net_bridge *br, struct br_mrp *mrp,
> +				  enum br_mrp_in_state_type state)
> +{
> +	struct switchdev_obj_in_state_mrp mrp_state = {
> +		.obj.orig_dev = br->dev,
> +		.obj.id = SWITCHDEV_OBJ_ID_IN_STATE_MRP,
> +		.in_state = state,
> +		.in_id = mrp->in_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_switchdev_send_in_test(struct net_bridge *br, struct br_mrp *mrp,
> +				  u32 interval, u8 max_miss, u32 period)
> +{
> +	struct switchdev_obj_in_test_mrp test = {
> +		.obj.orig_dev = br->dev,
> +		.obj.id = SWITCHDEV_OBJ_ID_IN_TEST_MRP,
> +		.interval = interval,
> +		.max_miss = max_miss,
> +		.in_id = mrp->in_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_port_switchdev_set_state(struct net_bridge_port *p,
>  				    enum br_mrp_port_state_type state)
>  {
> diff --git a/net/bridge/br_private_mrp.h b/net/bridge/br_private_mrp.h
> index 23da2f956ad0e..0d554ef88db85 100644
> --- a/net/bridge/br_private_mrp.h
> +++ b/net/bridge/br_private_mrp.h
> @@ -72,6 +72,13 @@ 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);
> +int br_mrp_switchdev_set_in_role(struct net_bridge *br, struct br_mrp *mrp,
> +				 u16 in_id, u32 ring_id,
> +				 enum br_mrp_in_role_type role);
> +int br_mrp_switchdev_set_in_state(struct net_bridge *br, struct br_mrp *mrp,
> +				  enum br_mrp_in_state_type state);
> +int br_mrp_switchdev_send_in_test(struct net_bridge *br, struct br_mrp *mrp,
> +				  u32 interval, u8 max_miss, u32 period);
>  
>  /* br_mrp_netlink.c  */
>  int br_mrp_ring_port_open(struct net_device *dev, u8 loc);
> 

[PATCH net-next v4 08/12] bridge: mrp: Implement the MRP Interconnect API

[Horatiu Vultur]

Thie patch adds support for MRP Interconnect. Similar with the MRP ring,
if the HW can't generate MRP_InTest frames, then the SW will try to
generate them. And if also the SW fails to generate the frames then an
error is return to userspace.

The forwarding/termination of MRP_In frames is happening in the kernel
and is done by MRP instances.

Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
---
 net/bridge/br_mrp.c         | 570 ++++++++++++++++++++++++++++++++++--
 net/bridge/br_private_mrp.h |   4 +
 2 files changed, 543 insertions(+), 31 deletions(-)

diff --git a/net/bridge/br_mrp.c b/net/bridge/br_mrp.c
index fe7cf1446b58a..b36689e6e7cba 100644
--- a/net/bridge/br_mrp.c
+++ b/net/bridge/br_mrp.c
@@ -4,6 +4,27 @@
 #include "br_private_mrp.h"
 
 static const u8 mrp_test_dmac[ETH_ALEN] = { 0x1, 0x15, 0x4e, 0x0, 0x0, 0x1 };
+static const u8 mrp_in_test_dmac[ETH_ALEN] = { 0x1, 0x15, 0x4e, 0x0, 0x0, 0x3 };
+
+static bool br_mrp_is_ring_port(struct net_bridge_port *p_port,
+				struct net_bridge_port *s_port,
+				struct net_bridge_port *port)
+{
+	if (port == p_port ||
+	    port == s_port)
+		return true;
+
+	return false;
+}
+
+static bool br_mrp_is_in_port(struct net_bridge_port *i_port,
+			      struct net_bridge_port *port)
+{
+	if (port == i_port)
+		return true;
+
+	return false;
+}
 
 static struct net_bridge_port *br_mrp_get_port(struct net_bridge *br,
 					       u32 ifindex)
@@ -37,6 +58,22 @@ static struct br_mrp *br_mrp_find_id(struct net_bridge *br, u32 ring_id)
 	return res;
 }
 
+static struct br_mrp *br_mrp_find_in_id(struct net_bridge *br, u32 in_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->in_id == in_id) {
+			res = mrp;
+			break;
+		}
+	}
+
+	return res;
+}
+
 static bool br_mrp_unique_ifindex(struct net_bridge *br, u32 ifindex)
 {
 	struct br_mrp *mrp;
@@ -52,6 +89,10 @@ static bool br_mrp_unique_ifindex(struct net_bridge *br, u32 ifindex)
 		p = rtnl_dereference(mrp->s_port);
 		if (p && p->dev->ifindex == ifindex)
 			return false;
+
+		p = rtnl_dereference(mrp->i_port);
+		if (p && p->dev->ifindex == ifindex)
+			return false;
 	}
 
 	return true;
@@ -66,7 +107,8 @@ static struct br_mrp *br_mrp_find_port(struct net_bridge *br,
 	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) {
+		    rcu_access_pointer(mrp->s_port) == p ||
+		    rcu_access_pointer(mrp->i_port) == p) {
 			res = mrp;
 			break;
 		}
@@ -160,6 +202,36 @@ static struct sk_buff *br_mrp_alloc_test_skb(struct br_mrp *mrp,
 	return skb;
 }
 
+static struct sk_buff *br_mrp_alloc_in_test_skb(struct br_mrp *mrp,
+						struct net_bridge_port *p,
+						enum br_mrp_port_role_type port_role)
+{
+	struct br_mrp_in_test_hdr *hdr = NULL;
+	struct sk_buff *skb = NULL;
+
+	if (!p)
+		return NULL;
+
+	skb = br_mrp_skb_alloc(p, p->dev->dev_addr, mrp_in_test_dmac);
+	if (!skb)
+		return NULL;
+
+	br_mrp_skb_tlv(skb, BR_MRP_TLV_HEADER_IN_TEST, sizeof(*hdr));
+	hdr = skb_put(skb, sizeof(*hdr));
+
+	hdr->id = cpu_to_be16(mrp->in_id);
+	ether_addr_copy(hdr->sa, p->br->dev->dev_addr);
+	hdr->port_role = cpu_to_be16(port_role);
+	hdr->state = cpu_to_be16(mrp->in_state);
+	hdr->transitions = cpu_to_be16(mrp->in_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;
+}
+
 /* This function is continuously called in the following cases:
  * - when node role is MRM, in this case test_monitor is always set to false
  *   because it needs to notify the userspace that the ring is open and needs to
@@ -239,6 +311,83 @@ static void br_mrp_test_work_expired(struct work_struct *work)
 			   usecs_to_jiffies(mrp->test_interval));
 }
 
+/* This function is continuously called when the node has the interconnect role
+ * MIM. It would generate interconnect test frames and will send them on all 3
+ * ports. But will also check if it stop receiving interconnect test frames.
+ */
+static void br_mrp_in_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, in_test_work);
+	struct net_bridge_port *p;
+	bool notify_open = false;
+	struct sk_buff *skb;
+
+	if (time_before_eq(mrp->in_test_end, jiffies))
+		return;
+
+	if (mrp->in_test_count_miss < mrp->in_test_max_miss) {
+		mrp->in_test_count_miss++;
+	} else {
+		/* Notify that the interconnect ring is open only if the
+		 * interconnect ring state is closed, otherwise it would
+		 * continue to notify at every interval.
+		 */
+		if (mrp->in_state == BR_MRP_IN_STATE_CLOSED)
+			notify_open = true;
+	}
+
+	rcu_read_lock();
+
+	p = rcu_dereference(mrp->p_port);
+	if (p) {
+		skb = br_mrp_alloc_in_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->in_role_offloaded)
+			br_mrp_in_port_open(p->dev, true);
+	}
+
+	p = rcu_dereference(mrp->s_port);
+	if (p) {
+		skb = br_mrp_alloc_in_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->in_role_offloaded)
+			br_mrp_in_port_open(p->dev, true);
+	}
+
+	p = rcu_dereference(mrp->i_port);
+	if (p) {
+		skb = br_mrp_alloc_in_test_skb(mrp, p,
+					       BR_MRP_PORT_ROLE_INTER);
+		if (!skb)
+			goto out;
+
+		skb_reset_network_header(skb);
+		dev_queue_xmit(skb);
+
+		if (notify_open && !mrp->in_role_offloaded)
+			br_mrp_in_port_open(p->dev, true);
+	}
+
+out:
+	rcu_read_unlock();
+
+	queue_delayed_work(system_wq, &mrp->in_test_work,
+			   usecs_to_jiffies(mrp->in_test_interval));
+}
+
 /* Deletes the MRP instance.
  * note: called under rtnl_lock
  */
@@ -251,6 +400,10 @@ static void br_mrp_del_impl(struct net_bridge *br, struct br_mrp *mrp)
 	cancel_delayed_work_sync(&mrp->test_work);
 	br_mrp_switchdev_send_ring_test(br, mrp, 0, 0, 0, 0);
 
+	/* Stop sending MRP_InTest frames if has an interconnect role */
+	cancel_delayed_work_sync(&mrp->in_test_work);
+	br_mrp_switchdev_send_in_test(br, mrp, 0, 0, 0);
+
 	br_mrp_switchdev_del(br, mrp);
 
 	/* Reset the ports */
@@ -278,6 +431,18 @@ static void br_mrp_del_impl(struct net_bridge *br, struct br_mrp *mrp)
 		rcu_assign_pointer(mrp->s_port, NULL);
 	}
 
+	p = rtnl_dereference(mrp->i_port);
+	if (p) {
+		spin_lock_bh(&br->lock);
+		state = netif_running(br->dev) ?
+				BR_STATE_FORWARDING : BR_STATE_DISABLED;
+		p->state = state;
+		p->flags &= ~BR_MRP_AWARE;
+		spin_unlock_bh(&br->lock);
+		br_mrp_port_switchdev_set_state(p, state);
+		rcu_assign_pointer(mrp->i_port, NULL);
+	}
+
 	list_del_rcu(&mrp->list);
 	kfree_rcu(mrp, rcu);
 }
@@ -329,6 +494,7 @@ int br_mrp_add(struct net_bridge *br, struct br_mrp_instance *instance)
 	rcu_assign_pointer(mrp->s_port, p);
 
 	INIT_DELAYED_WORK(&mrp->test_work, br_mrp_test_work_expired);
+	INIT_DELAYED_WORK(&mrp->in_test_work, br_mrp_in_test_work_expired);
 	list_add_tail_rcu(&mrp->list, &br->mrp_list);
 
 	err = br_mrp_switchdev_add(br, mrp);
@@ -511,6 +677,180 @@ int br_mrp_start_test(struct net_bridge *br,
 	return 0;
 }
 
+/* Set in state, int state can be only Open or Closed
+ * note: already called with rtnl_lock
+ */
+int br_mrp_set_in_state(struct net_bridge *br, struct br_mrp_in_state *state)
+{
+	struct br_mrp *mrp = br_mrp_find_in_id(br, state->in_id);
+
+	if (!mrp)
+		return -EINVAL;
+
+	if (mrp->in_state == BR_MRP_IN_STATE_CLOSED &&
+	    state->in_state != BR_MRP_IN_STATE_CLOSED)
+		mrp->in_transitions++;
+
+	mrp->in_state = state->in_state;
+
+	br_mrp_switchdev_set_in_state(br, mrp, state->in_state);
+
+	return 0;
+}
+
+/* Set in role, in role can be only MIM(Media Interconnection Manager) or
+ * MIC(Media Interconnection Client).
+ * note: already called with rtnl_lock
+ */
+int br_mrp_set_in_role(struct net_bridge *br, struct br_mrp_in_role *role)
+{
+	struct br_mrp *mrp = br_mrp_find_id(br, role->ring_id);
+	struct net_bridge_port *p;
+	int err;
+
+	if (!mrp)
+		return -EINVAL;
+
+	if (!br_mrp_get_port(br, role->i_ifindex))
+		return -EINVAL;
+
+	if (role->in_role == BR_MRP_IN_ROLE_DISABLED) {
+		u8 state;
+
+		/* It is not allowed to disable a port that doesn't exist */
+		p = rtnl_dereference(mrp->i_port);
+		if (!p)
+			return -EINVAL;
+
+		/* Stop the generating MRP_InTest frames */
+		cancel_delayed_work_sync(&mrp->in_test_work);
+		br_mrp_switchdev_send_in_test(br, mrp, 0, 0, 0);
+
+		/* Remove the port */
+		spin_lock_bh(&br->lock);
+		state = netif_running(br->dev) ?
+				BR_STATE_FORWARDING : BR_STATE_DISABLED;
+		p->state = state;
+		p->flags &= ~BR_MRP_AWARE;
+		spin_unlock_bh(&br->lock);
+		br_mrp_port_switchdev_set_state(p, state);
+		rcu_assign_pointer(mrp->i_port, NULL);
+
+		mrp->in_role = role->in_role;
+		mrp->in_id = 0;
+
+		return 0;
+	}
+
+	/* It is not possible to have the same port part of multiple rings */
+	if (!br_mrp_unique_ifindex(br, role->i_ifindex))
+		return -EINVAL;
+
+	/* It is not allowed to set a different interconnect port if the mrp
+	 * instance has already one. First it needs to be disabled and after
+	 * that set the new port
+	 */
+	if (rcu_access_pointer(mrp->i_port))
+		return -EINVAL;
+
+	p = br_mrp_get_port(br, role->i_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->i_port, p);
+
+	mrp->in_role = role->in_role;
+	mrp->in_id = role->in_id;
+
+	/* If there is an error just bailed out */
+	err = br_mrp_switchdev_set_in_role(br, mrp, role->in_id,
+					   role->ring_id, role->in_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 is MIM then the HW will notify the
+	 * SW when interconnect ring is open, but if the is not pushed to the HW
+	 * the SW will need to detect when the interconnect ring is open.
+	 */
+	mrp->in_role_offloaded = err == -EOPNOTSUPP ? 0 : 1;
+
+	return 0;
+}
+
+/* Start to generate MRP_InTest frames, the frames are generated by
+ * HW and if it fails, they are generated by the SW.
+ * note: already called with rtnl_lock
+ */
+int br_mrp_start_in_test(struct net_bridge *br,
+			 struct br_mrp_start_in_test *in_test)
+{
+	struct br_mrp *mrp = br_mrp_find_in_id(br, in_test->in_id);
+
+	if (!mrp)
+		return -EINVAL;
+
+	if (mrp->in_role != BR_MRP_IN_ROLE_MIM)
+		return -EINVAL;
+
+	/* Try to push it to the HW and if it fails then continue with SW
+	 * implementation and if that also fails then return error.
+	 */
+	if (!br_mrp_switchdev_send_in_test(br, mrp, in_test->interval,
+					   in_test->max_miss, in_test->period))
+		return 0;
+
+	mrp->in_test_interval = in_test->interval;
+	mrp->in_test_end = jiffies + usecs_to_jiffies(in_test->period);
+	mrp->in_test_max_miss = in_test->max_miss;
+	mrp->in_test_count_miss = 0;
+	queue_delayed_work(system_wq, &mrp->in_test_work,
+			   usecs_to_jiffies(in_test->interval));
+
+	return 0;
+}
+
+/* Determin if the frame type is a ring frame */
+static bool br_mrp_ring_frame(struct sk_buff *skb)
+{
+	const struct br_mrp_tlv_hdr *hdr;
+	struct br_mrp_tlv_hdr _hdr;
+
+	hdr = skb_header_pointer(skb, sizeof(uint16_t), sizeof(_hdr), &_hdr);
+	if (!hdr)
+		return false;
+
+	if (hdr->type == BR_MRP_TLV_HEADER_RING_TEST ||
+	    hdr->type == BR_MRP_TLV_HEADER_RING_TOPO ||
+	    hdr->type == BR_MRP_TLV_HEADER_RING_LINK_DOWN ||
+	    hdr->type == BR_MRP_TLV_HEADER_RING_LINK_UP ||
+	    hdr->type == BR_MRP_TLV_HEADER_OPTION)
+		return true;
+
+	return false;
+}
+
+/* Determin if the frame type is an interconnect frame */
+static bool br_mrp_in_frame(struct sk_buff *skb)
+{
+	const struct br_mrp_tlv_hdr *hdr;
+	struct br_mrp_tlv_hdr _hdr;
+
+	hdr = skb_header_pointer(skb, sizeof(uint16_t), sizeof(_hdr), &_hdr);
+	if (!hdr)
+		return false;
+
+	if (hdr->type == BR_MRP_TLV_HEADER_IN_TEST ||
+	    hdr->type == BR_MRP_TLV_HEADER_IN_TOPO ||
+	    hdr->type == BR_MRP_TLV_HEADER_IN_LINK_DOWN ||
+	    hdr->type == BR_MRP_TLV_HEADER_IN_LINK_UP)
+		return true;
+
+	return false;
+}
+
 /* Process only MRP Test frame. All the other MRP frames are processed by
  * userspace application
  * note: already called with rcu_read_lock
@@ -591,17 +931,92 @@ static void br_mrp_mra_process(struct br_mrp *mrp, struct net_bridge *br,
 		mrp->test_count_miss = 0;
 }
 
-/* This will just forward the frame to the other mrp ring port(MRC role) or will
- * not do anything.
+/* Process only MRP InTest frame. All the other MRP frames are processed by
+ * userspace application
+ * note: already called with rcu_read_lock
+ */
+static bool br_mrp_mim_process(struct br_mrp *mrp, struct net_bridge_port *port,
+			       struct sk_buff *skb)
+{
+	const struct br_mrp_in_test_hdr *in_hdr;
+	struct br_mrp_in_test_hdr _in_hdr;
+	const struct br_mrp_tlv_hdr *hdr;
+	struct br_mrp_tlv_hdr _hdr;
+
+	/* Each MRP header starts with a version field which is 16 bits.
+	 * Therefore skip the version and get directly the TLV header.
+	 */
+	hdr = skb_header_pointer(skb, sizeof(uint16_t), sizeof(_hdr), &_hdr);
+	if (!hdr)
+		return false;
+
+	/* The check for InTest frame type was already done */
+	in_hdr = skb_header_pointer(skb, sizeof(uint16_t) + sizeof(_hdr),
+				    sizeof(_in_hdr), &_in_hdr);
+	if (!in_hdr)
+		return false;
+
+	/* It needs to process only it's own InTest frames. */
+	if (mrp->in_id != ntohs(in_hdr->id))
+		return false;
+
+	mrp->in_test_count_miss = 0;
+
+	/* Notify the userspace that the ring is closed only when the ring is
+	 * not closed
+	 */
+	if (mrp->in_state != BR_MRP_IN_STATE_CLOSED)
+		br_mrp_in_port_open(port->dev, false);
+
+	return true;
+}
+
+/* Get the MRP frame type
+ * note: already called with rcu_read_lock
+ */
+static u8 br_mrp_get_frame_type(struct sk_buff *skb)
+{
+	const struct br_mrp_tlv_hdr *hdr;
+	struct br_mrp_tlv_hdr _hdr;
+
+	/* Each MRP header starts with a version field which is 16 bits.
+	 * Therefore skip the version and get directly the TLV header.
+	 */
+	hdr = skb_header_pointer(skb, sizeof(uint16_t), sizeof(_hdr), &_hdr);
+	if (!hdr)
+		return 0xff;
+
+	return hdr->type;
+}
+
+static bool br_mrp_mrm_behaviour(struct br_mrp *mrp)
+{
+	if (mrp->ring_role == BR_MRP_RING_ROLE_MRM ||
+	    (mrp->ring_role == BR_MRP_RING_ROLE_MRA && !mrp->test_monitor))
+		return true;
+
+	return false;
+}
+
+static bool br_mrp_mrc_behaviour(struct br_mrp *mrp)
+{
+	if (mrp->ring_role == BR_MRP_RING_ROLE_MRC ||
+	    (mrp->ring_role == BR_MRP_RING_ROLE_MRA && mrp->test_monitor))
+		return true;
+
+	return false;
+}
+
+/* This will just forward the frame to the other mrp ring ports, depending on
+ * the frame type, ring role and interconnect role
  * 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_port *p_port, *s_port, *i_port = NULL;
+	struct net_bridge_port *p_dst, *s_dst, *i_dst = NULL;
 	struct net_bridge *br;
-	struct sk_buff *nskb;
 	struct br_mrp *mrp;
 
 	/* If port is disabled don't accept any frames */
@@ -616,46 +1031,139 @@ static int br_mrp_rcv(struct net_bridge_port *p,
 	p_port = rcu_dereference(mrp->p_port);
 	if (!p_port)
 		return 0;
+	p_dst = p_port;
 
 	s_port = rcu_dereference(mrp->s_port);
 	if (!s_port)
 		return 0;
+	s_dst = s_port;
 
-	/* 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;
-	}
-
-	/* If the role is MRA then don't forward the frames if it behaves as
-	 * MRM node
+	/* If the frame is a ring frame then it is not required to check the
+	 * interconnect role and ports to process or forward the frame
 	 */
-	if (mrp->ring_role == BR_MRP_RING_ROLE_MRA) {
-		if (!mrp->test_monitor) {
+	if (br_mrp_ring_frame(skb)) {
+		/* 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;
+			goto no_forward;
 		}
 
-		br_mrp_mra_process(mrp, br, p, skb);
+		/* If the role is MRA then don't forward the frames if it
+		 * behaves as MRM node
+		 */
+		if (mrp->ring_role == BR_MRP_RING_ROLE_MRA) {
+			if (!mrp->test_monitor) {
+				br_mrp_mrm_process(mrp, p, skb);
+				goto no_forward;
+			}
+
+			br_mrp_mra_process(mrp, br, p, skb);
+		}
+
+		goto forward;
 	}
 
-	/* Clone the frame and forward it on the other MRP port */
-	nskb = skb_clone(skb, GFP_ATOMIC);
-	if (!nskb)
-		return 0;
+	if (br_mrp_in_frame(skb)) {
+		u8 in_type = br_mrp_get_frame_type(skb);
 
-	p_dev = p_port->dev;
-	s_dev = s_port->dev;
+		i_port = rcu_dereference(mrp->i_port);
+		i_dst = i_port;
 
-	if (p_dev == dev)
-		d_dev = s_dev;
-	else
-		d_dev = p_dev;
+		/* If the ring port is in block state it should not forward
+		 * In_Test frames
+		 */
+		if (br_mrp_is_ring_port(p_port, s_port, p) &&
+		    p->state == BR_STATE_BLOCKING &&
+		    in_type == BR_MRP_TLV_HEADER_IN_TEST)
+			goto no_forward;
+
+		/* Nodes that behaves as MRM needs to stop forwarding the
+		 * frames in case the ring is closed, otherwise will be a loop.
+		 * In this case the frame is no forward between the ring ports.
+		 */
+		if (br_mrp_mrm_behaviour(mrp) &&
+		    br_mrp_is_ring_port(p_port, s_port, p) &&
+		    (s_port->state != BR_STATE_FORWARDING ||
+		     p_port->state != BR_STATE_FORWARDING)) {
+			p_dst = NULL;
+			s_dst = NULL;
+		}
+
+		/* A node that behaves as MRC and doesn't have a interconnect
+		 * role then it should forward all frames between the ring ports
+		 * because it doesn't have an interconnect port
+		 */
+		if (br_mrp_mrc_behaviour(mrp) &&
+		    mrp->in_role == BR_MRP_IN_ROLE_DISABLED)
+			goto forward;
+
+		if (mrp->in_role == BR_MRP_IN_ROLE_MIM) {
+			if (in_type == BR_MRP_TLV_HEADER_IN_TEST) {
+				/* MIM should not forward it's own InTest
+				 * frames
+				 */
+				if (br_mrp_mim_process(mrp, p, skb)) {
+					goto no_forward;
+				} else {
+					if (br_mrp_is_ring_port(p_port, s_port,
+								p))
+						i_dst = NULL;
+
+					if (br_mrp_is_in_port(i_port, p))
+						goto no_forward;
+				}
+			} else {
+				/* MIM should forward IntLinkChange and
+				 * IntTopoChange between ring ports but MIM
+				 * should not forward IntLinkChange and
+				 * IntTopoChange if the frame was received at
+				 * the interconnect port
+				 */
+				if (br_mrp_is_ring_port(p_port, s_port, p))
+					i_dst = NULL;
+
+				if (br_mrp_is_in_port(i_port, p))
+					goto no_forward;
+			}
+		}
+
+		if (mrp->in_role == BR_MRP_IN_ROLE_MIC) {
+			/* MIC should forward InTest frames on all ports
+			 * regardless of the received port
+			 */
+			if (in_type == BR_MRP_TLV_HEADER_IN_TEST)
+				goto forward;
+
+			/* MIC should forward IntLinkChange frames only if they
+			 * are received on ring ports to all the ports
+			 */
+			if (br_mrp_is_ring_port(p_port, s_port, p) &&
+			    (in_type == BR_MRP_TLV_HEADER_IN_LINK_UP ||
+			     in_type == BR_MRP_TLV_HEADER_IN_LINK_DOWN))
+				goto forward;
+
+			/* Should forward the InTopo frames only between the
+			 * ring ports
+			 */
+			if (in_type == BR_MRP_TLV_HEADER_IN_TOPO) {
+				i_dst = NULL;
+				goto forward;
+			}
+
+			/* In all the other cases don't forward the frames */
+			goto no_forward;
+		}
+	}
 
-	nskb->dev = d_dev;
-	skb_push(nskb, ETH_HLEN);
-	dev_queue_xmit(nskb);
+forward:
+	if (p_dst)
+		br_forward(p_dst, skb, true, false);
+	if (s_dst)
+		br_forward(s_dst, skb, true, false);
+	if (i_dst)
+		br_forward(i_dst, skb, true, false);
 
+no_forward:
 	return 1;
 }
 
diff --git a/net/bridge/br_private_mrp.h b/net/bridge/br_private_mrp.h
index 0d554ef88db85..af0e9eff65493 100644
--- a/net/bridge/br_private_mrp.h
+++ b/net/bridge/br_private_mrp.h
@@ -57,6 +57,10 @@ 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);
+int br_mrp_set_in_state(struct net_bridge *br, struct br_mrp_in_state *state);
+int br_mrp_set_in_role(struct net_bridge *br, struct br_mrp_in_role *role);
+int br_mrp_start_in_test(struct net_bridge *br,
+			 struct br_mrp_start_in_test *test);
 
 /* br_mrp_switchdev.c */
 int br_mrp_switchdev_add(struct net_bridge *br, struct br_mrp *mrp);
-- 
2.27.0

Re: [PATCH net-next v4 08/12] bridge: mrp: Implement the MRP Interconnect API

[Nikolay Aleksandrov]

On 14/07/2020 10:34, Horatiu Vultur wrote:
> Thie patch adds support for MRP Interconnect. Similar with the MRP ring,
> if the HW can't generate MRP_InTest frames, then the SW will try to
> generate them. And if also the SW fails to generate the frames then an
> error is return to userspace.
> 
> The forwarding/termination of MRP_In frames is happening in the kernel
> and is done by MRP instances.
> 
> Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> ---
>  net/bridge/br_mrp.c         | 570 ++++++++++++++++++++++++++++++++++--
>  net/bridge/br_private_mrp.h |   4 +
>  2 files changed, 543 insertions(+), 31 deletions(-)
> 

Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>

> diff --git a/net/bridge/br_mrp.c b/net/bridge/br_mrp.c
> index fe7cf1446b58a..b36689e6e7cba 100644
> --- a/net/bridge/br_mrp.c
> +++ b/net/bridge/br_mrp.c
> @@ -4,6 +4,27 @@
>  #include "br_private_mrp.h"
>  
>  static const u8 mrp_test_dmac[ETH_ALEN] = { 0x1, 0x15, 0x4e, 0x0, 0x0, 0x1 };
> +static const u8 mrp_in_test_dmac[ETH_ALEN] = { 0x1, 0x15, 0x4e, 0x0, 0x0, 0x3 };
> +
> +static bool br_mrp_is_ring_port(struct net_bridge_port *p_port,
> +				struct net_bridge_port *s_port,
> +				struct net_bridge_port *port)
> +{
> +	if (port == p_port ||
> +	    port == s_port)
> +		return true;
> +
> +	return false;
> +}
> +
> +static bool br_mrp_is_in_port(struct net_bridge_port *i_port,
> +			      struct net_bridge_port *port)
> +{
> +	if (port == i_port)
> +		return true;
> +
> +	return false;
> +}
>  
>  static struct net_bridge_port *br_mrp_get_port(struct net_bridge *br,
>  					       u32 ifindex)
> @@ -37,6 +58,22 @@ static struct br_mrp *br_mrp_find_id(struct net_bridge *br, u32 ring_id)
>  	return res;
>  }
>  
> +static struct br_mrp *br_mrp_find_in_id(struct net_bridge *br, u32 in_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->in_id == in_id) {
> +			res = mrp;
> +			break;
> +		}
> +	}
> +
> +	return res;
> +}
> +
>  static bool br_mrp_unique_ifindex(struct net_bridge *br, u32 ifindex)
>  {
>  	struct br_mrp *mrp;
> @@ -52,6 +89,10 @@ static bool br_mrp_unique_ifindex(struct net_bridge *br, u32 ifindex)
>  		p = rtnl_dereference(mrp->s_port);
>  		if (p && p->dev->ifindex == ifindex)
>  			return false;
> +
> +		p = rtnl_dereference(mrp->i_port);
> +		if (p && p->dev->ifindex == ifindex)
> +			return false;
>  	}
>  
>  	return true;
> @@ -66,7 +107,8 @@ static struct br_mrp *br_mrp_find_port(struct net_bridge *br,
>  	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) {
> +		    rcu_access_pointer(mrp->s_port) == p ||
> +		    rcu_access_pointer(mrp->i_port) == p) {
>  			res = mrp;
>  			break;
>  		}
> @@ -160,6 +202,36 @@ static struct sk_buff *br_mrp_alloc_test_skb(struct br_mrp *mrp,
>  	return skb;
>  }
>  
> +static struct sk_buff *br_mrp_alloc_in_test_skb(struct br_mrp *mrp,
> +						struct net_bridge_port *p,
> +						enum br_mrp_port_role_type port_role)
> +{
> +	struct br_mrp_in_test_hdr *hdr = NULL;
> +	struct sk_buff *skb = NULL;
> +
> +	if (!p)
> +		return NULL;
> +
> +	skb = br_mrp_skb_alloc(p, p->dev->dev_addr, mrp_in_test_dmac);
> +	if (!skb)
> +		return NULL;
> +
> +	br_mrp_skb_tlv(skb, BR_MRP_TLV_HEADER_IN_TEST, sizeof(*hdr));
> +	hdr = skb_put(skb, sizeof(*hdr));
> +
> +	hdr->id = cpu_to_be16(mrp->in_id);
> +	ether_addr_copy(hdr->sa, p->br->dev->dev_addr);
> +	hdr->port_role = cpu_to_be16(port_role);
> +	hdr->state = cpu_to_be16(mrp->in_state);
> +	hdr->transitions = cpu_to_be16(mrp->in_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;
> +}
> +
>  /* This function is continuously called in the following cases:
>   * - when node role is MRM, in this case test_monitor is always set to false
>   *   because it needs to notify the userspace that the ring is open and needs to
> @@ -239,6 +311,83 @@ static void br_mrp_test_work_expired(struct work_struct *work)
>  			   usecs_to_jiffies(mrp->test_interval));
>  }
>  
> +/* This function is continuously called when the node has the interconnect role
> + * MIM. It would generate interconnect test frames and will send them on all 3
> + * ports. But will also check if it stop receiving interconnect test frames.
> + */
> +static void br_mrp_in_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, in_test_work);
> +	struct net_bridge_port *p;
> +	bool notify_open = false;
> +	struct sk_buff *skb;
> +
> +	if (time_before_eq(mrp->in_test_end, jiffies))
> +		return;
> +
> +	if (mrp->in_test_count_miss < mrp->in_test_max_miss) {
> +		mrp->in_test_count_miss++;
> +	} else {
> +		/* Notify that the interconnect ring is open only if the
> +		 * interconnect ring state is closed, otherwise it would
> +		 * continue to notify at every interval.
> +		 */
> +		if (mrp->in_state == BR_MRP_IN_STATE_CLOSED)
> +			notify_open = true;
> +	}
> +
> +	rcu_read_lock();
> +
> +	p = rcu_dereference(mrp->p_port);
> +	if (p) {
> +		skb = br_mrp_alloc_in_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->in_role_offloaded)
> +			br_mrp_in_port_open(p->dev, true);
> +	}
> +
> +	p = rcu_dereference(mrp->s_port);
> +	if (p) {
> +		skb = br_mrp_alloc_in_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->in_role_offloaded)
> +			br_mrp_in_port_open(p->dev, true);
> +	}
> +
> +	p = rcu_dereference(mrp->i_port);
> +	if (p) {
> +		skb = br_mrp_alloc_in_test_skb(mrp, p,
> +					       BR_MRP_PORT_ROLE_INTER);
> +		if (!skb)
> +			goto out;
> +
> +		skb_reset_network_header(skb);
> +		dev_queue_xmit(skb);
> +
> +		if (notify_open && !mrp->in_role_offloaded)
> +			br_mrp_in_port_open(p->dev, true);
> +	}
> +
> +out:
> +	rcu_read_unlock();
> +
> +	queue_delayed_work(system_wq, &mrp->in_test_work,
> +			   usecs_to_jiffies(mrp->in_test_interval));
> +}
> +
>  /* Deletes the MRP instance.
>   * note: called under rtnl_lock
>   */
> @@ -251,6 +400,10 @@ static void br_mrp_del_impl(struct net_bridge *br, struct br_mrp *mrp)
>  	cancel_delayed_work_sync(&mrp->test_work);
>  	br_mrp_switchdev_send_ring_test(br, mrp, 0, 0, 0, 0);
>  
> +	/* Stop sending MRP_InTest frames if has an interconnect role */
> +	cancel_delayed_work_sync(&mrp->in_test_work);
> +	br_mrp_switchdev_send_in_test(br, mrp, 0, 0, 0);
> +
>  	br_mrp_switchdev_del(br, mrp);
>  
>  	/* Reset the ports */
> @@ -278,6 +431,18 @@ static void br_mrp_del_impl(struct net_bridge *br, struct br_mrp *mrp)
>  		rcu_assign_pointer(mrp->s_port, NULL);
>  	}
>  
> +	p = rtnl_dereference(mrp->i_port);
> +	if (p) {
> +		spin_lock_bh(&br->lock);
> +		state = netif_running(br->dev) ?
> +				BR_STATE_FORWARDING : BR_STATE_DISABLED;
> +		p->state = state;
> +		p->flags &= ~BR_MRP_AWARE;
> +		spin_unlock_bh(&br->lock);
> +		br_mrp_port_switchdev_set_state(p, state);
> +		rcu_assign_pointer(mrp->i_port, NULL);
> +	}
> +
>  	list_del_rcu(&mrp->list);
>  	kfree_rcu(mrp, rcu);
>  }
> @@ -329,6 +494,7 @@ int br_mrp_add(struct net_bridge *br, struct br_mrp_instance *instance)
>  	rcu_assign_pointer(mrp->s_port, p);
>  
>  	INIT_DELAYED_WORK(&mrp->test_work, br_mrp_test_work_expired);
> +	INIT_DELAYED_WORK(&mrp->in_test_work, br_mrp_in_test_work_expired);
>  	list_add_tail_rcu(&mrp->list, &br->mrp_list);
>  
>  	err = br_mrp_switchdev_add(br, mrp);
> @@ -511,6 +677,180 @@ int br_mrp_start_test(struct net_bridge *br,
>  	return 0;
>  }
>  
> +/* Set in state, int state can be only Open or Closed
> + * note: already called with rtnl_lock
> + */
> +int br_mrp_set_in_state(struct net_bridge *br, struct br_mrp_in_state *state)
> +{
> +	struct br_mrp *mrp = br_mrp_find_in_id(br, state->in_id);
> +
> +	if (!mrp)
> +		return -EINVAL;
> +
> +	if (mrp->in_state == BR_MRP_IN_STATE_CLOSED &&
> +	    state->in_state != BR_MRP_IN_STATE_CLOSED)
> +		mrp->in_transitions++;
> +
> +	mrp->in_state = state->in_state;
> +
> +	br_mrp_switchdev_set_in_state(br, mrp, state->in_state);
> +
> +	return 0;
> +}
> +
> +/* Set in role, in role can be only MIM(Media Interconnection Manager) or
> + * MIC(Media Interconnection Client).
> + * note: already called with rtnl_lock
> + */
> +int br_mrp_set_in_role(struct net_bridge *br, struct br_mrp_in_role *role)
> +{
> +	struct br_mrp *mrp = br_mrp_find_id(br, role->ring_id);
> +	struct net_bridge_port *p;
> +	int err;
> +
> +	if (!mrp)
> +		return -EINVAL;
> +
> +	if (!br_mrp_get_port(br, role->i_ifindex))
> +		return -EINVAL;
> +
> +	if (role->in_role == BR_MRP_IN_ROLE_DISABLED) {
> +		u8 state;
> +
> +		/* It is not allowed to disable a port that doesn't exist */
> +		p = rtnl_dereference(mrp->i_port);
> +		if (!p)
> +			return -EINVAL;
> +
> +		/* Stop the generating MRP_InTest frames */
> +		cancel_delayed_work_sync(&mrp->in_test_work);
> +		br_mrp_switchdev_send_in_test(br, mrp, 0, 0, 0);
> +
> +		/* Remove the port */
> +		spin_lock_bh(&br->lock);
> +		state = netif_running(br->dev) ?
> +				BR_STATE_FORWARDING : BR_STATE_DISABLED;
> +		p->state = state;
> +		p->flags &= ~BR_MRP_AWARE;
> +		spin_unlock_bh(&br->lock);
> +		br_mrp_port_switchdev_set_state(p, state);
> +		rcu_assign_pointer(mrp->i_port, NULL);
> +
> +		mrp->in_role = role->in_role;
> +		mrp->in_id = 0;
> +
> +		return 0;
> +	}
> +
> +	/* It is not possible to have the same port part of multiple rings */
> +	if (!br_mrp_unique_ifindex(br, role->i_ifindex))
> +		return -EINVAL;
> +
> +	/* It is not allowed to set a different interconnect port if the mrp
> +	 * instance has already one. First it needs to be disabled and after
> +	 * that set the new port
> +	 */
> +	if (rcu_access_pointer(mrp->i_port))
> +		return -EINVAL;
> +
> +	p = br_mrp_get_port(br, role->i_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->i_port, p);
> +
> +	mrp->in_role = role->in_role;
> +	mrp->in_id = role->in_id;
> +
> +	/* If there is an error just bailed out */
> +	err = br_mrp_switchdev_set_in_role(br, mrp, role->in_id,
> +					   role->ring_id, role->in_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 is MIM then the HW will notify the
> +	 * SW when interconnect ring is open, but if the is not pushed to the HW
> +	 * the SW will need to detect when the interconnect ring is open.
> +	 */
> +	mrp->in_role_offloaded = err == -EOPNOTSUPP ? 0 : 1;
> +
> +	return 0;
> +}
> +
> +/* Start to generate MRP_InTest frames, the frames are generated by
> + * HW and if it fails, they are generated by the SW.
> + * note: already called with rtnl_lock
> + */
> +int br_mrp_start_in_test(struct net_bridge *br,
> +			 struct br_mrp_start_in_test *in_test)
> +{
> +	struct br_mrp *mrp = br_mrp_find_in_id(br, in_test->in_id);
> +
> +	if (!mrp)
> +		return -EINVAL;
> +
> +	if (mrp->in_role != BR_MRP_IN_ROLE_MIM)
> +		return -EINVAL;
> +
> +	/* Try to push it to the HW and if it fails then continue with SW
> +	 * implementation and if that also fails then return error.
> +	 */
> +	if (!br_mrp_switchdev_send_in_test(br, mrp, in_test->interval,
> +					   in_test->max_miss, in_test->period))
> +		return 0;
> +
> +	mrp->in_test_interval = in_test->interval;
> +	mrp->in_test_end = jiffies + usecs_to_jiffies(in_test->period);
> +	mrp->in_test_max_miss = in_test->max_miss;
> +	mrp->in_test_count_miss = 0;
> +	queue_delayed_work(system_wq, &mrp->in_test_work,
> +			   usecs_to_jiffies(in_test->interval));
> +
> +	return 0;
> +}
> +
> +/* Determin if the frame type is a ring frame */
> +static bool br_mrp_ring_frame(struct sk_buff *skb)
> +{
> +	const struct br_mrp_tlv_hdr *hdr;
> +	struct br_mrp_tlv_hdr _hdr;
> +
> +	hdr = skb_header_pointer(skb, sizeof(uint16_t), sizeof(_hdr), &_hdr);
> +	if (!hdr)
> +		return false;
> +
> +	if (hdr->type == BR_MRP_TLV_HEADER_RING_TEST ||
> +	    hdr->type == BR_MRP_TLV_HEADER_RING_TOPO ||
> +	    hdr->type == BR_MRP_TLV_HEADER_RING_LINK_DOWN ||
> +	    hdr->type == BR_MRP_TLV_HEADER_RING_LINK_UP ||
> +	    hdr->type == BR_MRP_TLV_HEADER_OPTION)
> +		return true;
> +
> +	return false;
> +}
> +
> +/* Determin if the frame type is an interconnect frame */
> +static bool br_mrp_in_frame(struct sk_buff *skb)
> +{
> +	const struct br_mrp_tlv_hdr *hdr;
> +	struct br_mrp_tlv_hdr _hdr;
> +
> +	hdr = skb_header_pointer(skb, sizeof(uint16_t), sizeof(_hdr), &_hdr);
> +	if (!hdr)
> +		return false;
> +
> +	if (hdr->type == BR_MRP_TLV_HEADER_IN_TEST ||
> +	    hdr->type == BR_MRP_TLV_HEADER_IN_TOPO ||
> +	    hdr->type == BR_MRP_TLV_HEADER_IN_LINK_DOWN ||
> +	    hdr->type == BR_MRP_TLV_HEADER_IN_LINK_UP)
> +		return true;
> +
> +	return false;
> +}
> +
>  /* Process only MRP Test frame. All the other MRP frames are processed by
>   * userspace application
>   * note: already called with rcu_read_lock
> @@ -591,17 +931,92 @@ static void br_mrp_mra_process(struct br_mrp *mrp, struct net_bridge *br,
>  		mrp->test_count_miss = 0;
>  }
>  
> -/* This will just forward the frame to the other mrp ring port(MRC role) or will
> - * not do anything.
> +/* Process only MRP InTest frame. All the other MRP frames are processed by
> + * userspace application
> + * note: already called with rcu_read_lock
> + */
> +static bool br_mrp_mim_process(struct br_mrp *mrp, struct net_bridge_port *port,
> +			       struct sk_buff *skb)
> +{
> +	const struct br_mrp_in_test_hdr *in_hdr;
> +	struct br_mrp_in_test_hdr _in_hdr;
> +	const struct br_mrp_tlv_hdr *hdr;
> +	struct br_mrp_tlv_hdr _hdr;
> +
> +	/* Each MRP header starts with a version field which is 16 bits.
> +	 * Therefore skip the version and get directly the TLV header.
> +	 */
> +	hdr = skb_header_pointer(skb, sizeof(uint16_t), sizeof(_hdr), &_hdr);
> +	if (!hdr)
> +		return false;
> +
> +	/* The check for InTest frame type was already done */
> +	in_hdr = skb_header_pointer(skb, sizeof(uint16_t) + sizeof(_hdr),
> +				    sizeof(_in_hdr), &_in_hdr);
> +	if (!in_hdr)
> +		return false;
> +
> +	/* It needs to process only it's own InTest frames. */
> +	if (mrp->in_id != ntohs(in_hdr->id))
> +		return false;
> +
> +	mrp->in_test_count_miss = 0;
> +
> +	/* Notify the userspace that the ring is closed only when the ring is
> +	 * not closed
> +	 */
> +	if (mrp->in_state != BR_MRP_IN_STATE_CLOSED)
> +		br_mrp_in_port_open(port->dev, false);
> +
> +	return true;
> +}
> +
> +/* Get the MRP frame type
> + * note: already called with rcu_read_lock
> + */
> +static u8 br_mrp_get_frame_type(struct sk_buff *skb)
> +{
> +	const struct br_mrp_tlv_hdr *hdr;
> +	struct br_mrp_tlv_hdr _hdr;
> +
> +	/* Each MRP header starts with a version field which is 16 bits.
> +	 * Therefore skip the version and get directly the TLV header.
> +	 */
> +	hdr = skb_header_pointer(skb, sizeof(uint16_t), sizeof(_hdr), &_hdr);
> +	if (!hdr)
> +		return 0xff;
> +
> +	return hdr->type;
> +}
> +
> +static bool br_mrp_mrm_behaviour(struct br_mrp *mrp)
> +{
> +	if (mrp->ring_role == BR_MRP_RING_ROLE_MRM ||
> +	    (mrp->ring_role == BR_MRP_RING_ROLE_MRA && !mrp->test_monitor))
> +		return true;
> +
> +	return false;
> +}
> +
> +static bool br_mrp_mrc_behaviour(struct br_mrp *mrp)
> +{
> +	if (mrp->ring_role == BR_MRP_RING_ROLE_MRC ||
> +	    (mrp->ring_role == BR_MRP_RING_ROLE_MRA && mrp->test_monitor))
> +		return true;
> +
> +	return false;
> +}
> +
> +/* This will just forward the frame to the other mrp ring ports, depending on
> + * the frame type, ring role and interconnect role
>   * 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_port *p_port, *s_port, *i_port = NULL;
> +	struct net_bridge_port *p_dst, *s_dst, *i_dst = NULL;
>  	struct net_bridge *br;
> -	struct sk_buff *nskb;
>  	struct br_mrp *mrp;
>  
>  	/* If port is disabled don't accept any frames */
> @@ -616,46 +1031,139 @@ static int br_mrp_rcv(struct net_bridge_port *p,
>  	p_port = rcu_dereference(mrp->p_port);
>  	if (!p_port)
>  		return 0;
> +	p_dst = p_port;
>  
>  	s_port = rcu_dereference(mrp->s_port);
>  	if (!s_port)
>  		return 0;
> +	s_dst = s_port;
>  
> -	/* 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;
> -	}
> -
> -	/* If the role is MRA then don't forward the frames if it behaves as
> -	 * MRM node
> +	/* If the frame is a ring frame then it is not required to check the
> +	 * interconnect role and ports to process or forward the frame
>  	 */
> -	if (mrp->ring_role == BR_MRP_RING_ROLE_MRA) {
> -		if (!mrp->test_monitor) {
> +	if (br_mrp_ring_frame(skb)) {
> +		/* 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;
> +			goto no_forward;
>  		}
>  
> -		br_mrp_mra_process(mrp, br, p, skb);
> +		/* If the role is MRA then don't forward the frames if it
> +		 * behaves as MRM node
> +		 */
> +		if (mrp->ring_role == BR_MRP_RING_ROLE_MRA) {
> +			if (!mrp->test_monitor) {
> +				br_mrp_mrm_process(mrp, p, skb);
> +				goto no_forward;
> +			}
> +
> +			br_mrp_mra_process(mrp, br, p, skb);
> +		}
> +
> +		goto forward;
>  	}
>  
> -	/* Clone the frame and forward it on the other MRP port */
> -	nskb = skb_clone(skb, GFP_ATOMIC);
> -	if (!nskb)
> -		return 0;
> +	if (br_mrp_in_frame(skb)) {
> +		u8 in_type = br_mrp_get_frame_type(skb);
>  
> -	p_dev = p_port->dev;
> -	s_dev = s_port->dev;
> +		i_port = rcu_dereference(mrp->i_port);
> +		i_dst = i_port;
>  
> -	if (p_dev == dev)
> -		d_dev = s_dev;
> -	else
> -		d_dev = p_dev;
> +		/* If the ring port is in block state it should not forward
> +		 * In_Test frames
> +		 */
> +		if (br_mrp_is_ring_port(p_port, s_port, p) &&
> +		    p->state == BR_STATE_BLOCKING &&
> +		    in_type == BR_MRP_TLV_HEADER_IN_TEST)
> +			goto no_forward;
> +
> +		/* Nodes that behaves as MRM needs to stop forwarding the
> +		 * frames in case the ring is closed, otherwise will be a loop.
> +		 * In this case the frame is no forward between the ring ports.
> +		 */
> +		if (br_mrp_mrm_behaviour(mrp) &&
> +		    br_mrp_is_ring_port(p_port, s_port, p) &&
> +		    (s_port->state != BR_STATE_FORWARDING ||
> +		     p_port->state != BR_STATE_FORWARDING)) {
> +			p_dst = NULL;
> +			s_dst = NULL;
> +		}
> +
> +		/* A node that behaves as MRC and doesn't have a interconnect
> +		 * role then it should forward all frames between the ring ports
> +		 * because it doesn't have an interconnect port
> +		 */
> +		if (br_mrp_mrc_behaviour(mrp) &&
> +		    mrp->in_role == BR_MRP_IN_ROLE_DISABLED)
> +			goto forward;
> +
> +		if (mrp->in_role == BR_MRP_IN_ROLE_MIM) {
> +			if (in_type == BR_MRP_TLV_HEADER_IN_TEST) {
> +				/* MIM should not forward it's own InTest
> +				 * frames
> +				 */
> +				if (br_mrp_mim_process(mrp, p, skb)) {
> +					goto no_forward;
> +				} else {
> +					if (br_mrp_is_ring_port(p_port, s_port,
> +								p))
> +						i_dst = NULL;
> +
> +					if (br_mrp_is_in_port(i_port, p))
> +						goto no_forward;
> +				}
> +			} else {
> +				/* MIM should forward IntLinkChange and
> +				 * IntTopoChange between ring ports but MIM
> +				 * should not forward IntLinkChange and
> +				 * IntTopoChange if the frame was received at
> +				 * the interconnect port
> +				 */
> +				if (br_mrp_is_ring_port(p_port, s_port, p))
> +					i_dst = NULL;
> +
> +				if (br_mrp_is_in_port(i_port, p))
> +					goto no_forward;
> +			}
> +		}
> +
> +		if (mrp->in_role == BR_MRP_IN_ROLE_MIC) {
> +			/* MIC should forward InTest frames on all ports
> +			 * regardless of the received port
> +			 */
> +			if (in_type == BR_MRP_TLV_HEADER_IN_TEST)
> +				goto forward;
> +
> +			/* MIC should forward IntLinkChange frames only if they
> +			 * are received on ring ports to all the ports
> +			 */
> +			if (br_mrp_is_ring_port(p_port, s_port, p) &&
> +			    (in_type == BR_MRP_TLV_HEADER_IN_LINK_UP ||
> +			     in_type == BR_MRP_TLV_HEADER_IN_LINK_DOWN))
> +				goto forward;
> +
> +			/* Should forward the InTopo frames only between the
> +			 * ring ports
> +			 */
> +			if (in_type == BR_MRP_TLV_HEADER_IN_TOPO) {
> +				i_dst = NULL;
> +				goto forward;
> +			}
> +
> +			/* In all the other cases don't forward the frames */
> +			goto no_forward;
> +		}
> +	}
>  
> -	nskb->dev = d_dev;
> -	skb_push(nskb, ETH_HLEN);
> -	dev_queue_xmit(nskb);
> +forward:
> +	if (p_dst)
> +		br_forward(p_dst, skb, true, false);
> +	if (s_dst)
> +		br_forward(s_dst, skb, true, false);
> +	if (i_dst)
> +		br_forward(i_dst, skb, true, false);
>  
> +no_forward:
>  	return 1;
>  }
>  
> diff --git a/net/bridge/br_private_mrp.h b/net/bridge/br_private_mrp.h
> index 0d554ef88db85..af0e9eff65493 100644
> --- a/net/bridge/br_private_mrp.h
> +++ b/net/bridge/br_private_mrp.h
> @@ -57,6 +57,10 @@ 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);
> +int br_mrp_set_in_state(struct net_bridge *br, struct br_mrp_in_state *state);
> +int br_mrp_set_in_role(struct net_bridge *br, struct br_mrp_in_role *role);
> +int br_mrp_start_in_test(struct net_bridge *br,
> +			 struct br_mrp_start_in_test *test);
>  
>  /* br_mrp_switchdev.c */
>  int br_mrp_switchdev_add(struct net_bridge *br, struct br_mrp *mrp);
> 

[PATCH net-next v4 09/12] bridge: mrp: Extend MRP netlink interface for configuring MRP interconnect

[Horatiu Vultur]

This patch extends the existing MRP netlink interface with the following
attributes: IFLA_BRIDGE_MRP_IN_ROLE, IFLA_BRIDGE_MRP_IN_STATE and
IFLA_BRIDGE_MRP_START_IN_TEST. These attributes are similar with their
ring attributes but they apply to the interconnect port.

Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
---
 net/bridge/br_mrp_netlink.c | 140 ++++++++++++++++++++++++++++++++++++
 1 file changed, 140 insertions(+)

diff --git a/net/bridge/br_mrp_netlink.c b/net/bridge/br_mrp_netlink.c
index 4bf7aaeb29152..a006e0771e8d3 100644
--- a/net/bridge/br_mrp_netlink.c
+++ b/net/bridge/br_mrp_netlink.c
@@ -14,6 +14,9 @@ static const struct nla_policy br_mrp_policy[IFLA_BRIDGE_MRP_MAX + 1] = {
 	[IFLA_BRIDGE_MRP_RING_STATE]	= { .type = NLA_NESTED },
 	[IFLA_BRIDGE_MRP_RING_ROLE]	= { .type = NLA_NESTED },
 	[IFLA_BRIDGE_MRP_START_TEST]	= { .type = NLA_NESTED },
+	[IFLA_BRIDGE_MRP_IN_ROLE]	= { .type = NLA_NESTED },
+	[IFLA_BRIDGE_MRP_IN_STATE]	= { .type = NLA_NESTED },
+	[IFLA_BRIDGE_MRP_START_IN_TEST]	= { .type = NLA_NESTED },
 };
 
 static const struct nla_policy
@@ -235,6 +238,121 @@ static int br_mrp_start_test_parse(struct net_bridge *br, struct nlattr *attr,
 	return br_mrp_start_test(br, &test);
 }
 
+static const struct nla_policy
+br_mrp_in_state_policy[IFLA_BRIDGE_MRP_IN_STATE_MAX + 1] = {
+	[IFLA_BRIDGE_MRP_IN_STATE_UNSPEC]	= { .type = NLA_REJECT },
+	[IFLA_BRIDGE_MRP_IN_STATE_IN_ID]	= { .type = NLA_U32 },
+	[IFLA_BRIDGE_MRP_IN_STATE_STATE]	= { .type = NLA_U32 },
+};
+
+static int br_mrp_in_state_parse(struct net_bridge *br, struct nlattr *attr,
+				 struct netlink_ext_ack *extack)
+{
+	struct nlattr *tb[IFLA_BRIDGE_MRP_IN_STATE_MAX + 1];
+	struct br_mrp_in_state state;
+	int err;
+
+	err = nla_parse_nested(tb, IFLA_BRIDGE_MRP_IN_STATE_MAX, attr,
+			       br_mrp_in_state_policy, extack);
+	if (err)
+		return err;
+
+	if (!tb[IFLA_BRIDGE_MRP_IN_STATE_IN_ID] ||
+	    !tb[IFLA_BRIDGE_MRP_IN_STATE_STATE]) {
+		NL_SET_ERR_MSG_MOD(extack,
+				   "Missing attribute: IN_ID or STATE");
+		return -EINVAL;
+	}
+
+	memset(&state, 0x0, sizeof(state));
+
+	state.in_id = nla_get_u32(tb[IFLA_BRIDGE_MRP_IN_STATE_IN_ID]);
+	state.in_state = nla_get_u32(tb[IFLA_BRIDGE_MRP_IN_STATE_STATE]);
+
+	return br_mrp_set_in_state(br, &state);
+}
+
+static const struct nla_policy
+br_mrp_in_role_policy[IFLA_BRIDGE_MRP_IN_ROLE_MAX + 1] = {
+	[IFLA_BRIDGE_MRP_IN_ROLE_UNSPEC]	= { .type = NLA_REJECT },
+	[IFLA_BRIDGE_MRP_IN_ROLE_RING_ID]	= { .type = NLA_U32 },
+	[IFLA_BRIDGE_MRP_IN_ROLE_IN_ID]		= { .type = NLA_U16 },
+	[IFLA_BRIDGE_MRP_IN_ROLE_ROLE]		= { .type = NLA_U32 },
+	[IFLA_BRIDGE_MRP_IN_ROLE_I_IFINDEX]	= { .type = NLA_U32 },
+};
+
+static int br_mrp_in_role_parse(struct net_bridge *br, struct nlattr *attr,
+				struct netlink_ext_ack *extack)
+{
+	struct nlattr *tb[IFLA_BRIDGE_MRP_IN_ROLE_MAX + 1];
+	struct br_mrp_in_role role;
+	int err;
+
+	err = nla_parse_nested(tb, IFLA_BRIDGE_MRP_IN_ROLE_MAX, attr,
+			       br_mrp_in_role_policy, extack);
+	if (err)
+		return err;
+
+	if (!tb[IFLA_BRIDGE_MRP_IN_ROLE_RING_ID] ||
+	    !tb[IFLA_BRIDGE_MRP_IN_ROLE_IN_ID] ||
+	    !tb[IFLA_BRIDGE_MRP_IN_ROLE_I_IFINDEX] ||
+	    !tb[IFLA_BRIDGE_MRP_IN_ROLE_ROLE]) {
+		NL_SET_ERR_MSG_MOD(extack,
+				   "Missing attribute: RING_ID or ROLE or IN_ID or I_IFINDEX");
+		return -EINVAL;
+	}
+
+	memset(&role, 0x0, sizeof(role));
+
+	role.ring_id = nla_get_u32(tb[IFLA_BRIDGE_MRP_IN_ROLE_RING_ID]);
+	role.in_id = nla_get_u16(tb[IFLA_BRIDGE_MRP_IN_ROLE_IN_ID]);
+	role.i_ifindex = nla_get_u32(tb[IFLA_BRIDGE_MRP_IN_ROLE_I_IFINDEX]);
+	role.in_role = nla_get_u32(tb[IFLA_BRIDGE_MRP_IN_ROLE_ROLE]);
+
+	return br_mrp_set_in_role(br, &role);
+}
+
+static const struct nla_policy
+br_mrp_start_in_test_policy[IFLA_BRIDGE_MRP_START_IN_TEST_MAX + 1] = {
+	[IFLA_BRIDGE_MRP_START_IN_TEST_UNSPEC]	= { .type = NLA_REJECT },
+	[IFLA_BRIDGE_MRP_START_IN_TEST_IN_ID]	= { .type = NLA_U32 },
+	[IFLA_BRIDGE_MRP_START_IN_TEST_INTERVAL]	= { .type = NLA_U32 },
+	[IFLA_BRIDGE_MRP_START_IN_TEST_MAX_MISS]	= { .type = NLA_U32 },
+	[IFLA_BRIDGE_MRP_START_IN_TEST_PERIOD]	= { .type = NLA_U32 },
+};
+
+static int br_mrp_start_in_test_parse(struct net_bridge *br,
+				      struct nlattr *attr,
+				      struct netlink_ext_ack *extack)
+{
+	struct nlattr *tb[IFLA_BRIDGE_MRP_START_IN_TEST_MAX + 1];
+	struct br_mrp_start_in_test test;
+	int err;
+
+	err = nla_parse_nested(tb, IFLA_BRIDGE_MRP_START_IN_TEST_MAX, attr,
+			       br_mrp_start_in_test_policy, extack);
+	if (err)
+		return err;
+
+	if (!tb[IFLA_BRIDGE_MRP_START_IN_TEST_IN_ID] ||
+	    !tb[IFLA_BRIDGE_MRP_START_IN_TEST_INTERVAL] ||
+	    !tb[IFLA_BRIDGE_MRP_START_IN_TEST_MAX_MISS] ||
+	    !tb[IFLA_BRIDGE_MRP_START_IN_TEST_PERIOD]) {
+		NL_SET_ERR_MSG_MOD(extack,
+				   "Missing attribute: RING_ID or INTERVAL or MAX_MISS or PERIOD");
+		return -EINVAL;
+	}
+
+	memset(&test, 0x0, sizeof(test));
+
+	test.in_id = nla_get_u32(tb[IFLA_BRIDGE_MRP_START_IN_TEST_IN_ID]);
+	test.interval = nla_get_u32(tb[IFLA_BRIDGE_MRP_START_IN_TEST_INTERVAL]);
+	test.max_miss = nla_get_u32(tb[IFLA_BRIDGE_MRP_START_IN_TEST_MAX_MISS]);
+	test.period = nla_get_u32(tb[IFLA_BRIDGE_MRP_START_IN_TEST_PERIOD]);
+
+	return br_mrp_start_in_test(br, &test);
+}
+
 int br_mrp_parse(struct net_bridge *br, struct net_bridge_port *p,
 		 struct nlattr *attr, int cmd, struct netlink_ext_ack *extack)
 {
@@ -301,6 +419,28 @@ int br_mrp_parse(struct net_bridge *br, struct net_bridge_port *p,
 			return err;
 	}
 
+	if (tb[IFLA_BRIDGE_MRP_IN_STATE]) {
+		err = br_mrp_in_state_parse(br, tb[IFLA_BRIDGE_MRP_IN_STATE],
+					    extack);
+		if (err)
+			return err;
+	}
+
+	if (tb[IFLA_BRIDGE_MRP_IN_ROLE]) {
+		err = br_mrp_in_role_parse(br, tb[IFLA_BRIDGE_MRP_IN_ROLE],
+					   extack);
+		if (err)
+			return err;
+	}
+
+	if (tb[IFLA_BRIDGE_MRP_START_IN_TEST]) {
+		err = br_mrp_start_in_test_parse(br,
+						 tb[IFLA_BRIDGE_MRP_START_IN_TEST],
+						 extack);
+		if (err)
+			return err;
+	}
+
 	return 0;
 }
 
-- 
2.27.0

Re: [PATCH net-next v4 09/12] bridge: mrp: Extend MRP netlink interface for configuring MRP interconnect

[Nikolay Aleksandrov]

On 14/07/2020 10:34, Horatiu Vultur wrote:
> This patch extends the existing MRP netlink interface with the following
> attributes: IFLA_BRIDGE_MRP_IN_ROLE, IFLA_BRIDGE_MRP_IN_STATE and
> IFLA_BRIDGE_MRP_START_IN_TEST. These attributes are similar with their
> ring attributes but they apply to the interconnect port.
> 
> Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> ---
>  net/bridge/br_mrp_netlink.c | 140 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 140 insertions(+)
> 

Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>

> diff --git a/net/bridge/br_mrp_netlink.c b/net/bridge/br_mrp_netlink.c
> index 4bf7aaeb29152..a006e0771e8d3 100644
> --- a/net/bridge/br_mrp_netlink.c
> +++ b/net/bridge/br_mrp_netlink.c
> @@ -14,6 +14,9 @@ static const struct nla_policy br_mrp_policy[IFLA_BRIDGE_MRP_MAX + 1] = {
>  	[IFLA_BRIDGE_MRP_RING_STATE]	= { .type = NLA_NESTED },
>  	[IFLA_BRIDGE_MRP_RING_ROLE]	= { .type = NLA_NESTED },
>  	[IFLA_BRIDGE_MRP_START_TEST]	= { .type = NLA_NESTED },
> +	[IFLA_BRIDGE_MRP_IN_ROLE]	= { .type = NLA_NESTED },
> +	[IFLA_BRIDGE_MRP_IN_STATE]	= { .type = NLA_NESTED },
> +	[IFLA_BRIDGE_MRP_START_IN_TEST]	= { .type = NLA_NESTED },
>  };
>  
>  static const struct nla_policy
> @@ -235,6 +238,121 @@ static int br_mrp_start_test_parse(struct net_bridge *br, struct nlattr *attr,
>  	return br_mrp_start_test(br, &test);
>  }
>  
> +static const struct nla_policy
> +br_mrp_in_state_policy[IFLA_BRIDGE_MRP_IN_STATE_MAX + 1] = {
> +	[IFLA_BRIDGE_MRP_IN_STATE_UNSPEC]	= { .type = NLA_REJECT },
> +	[IFLA_BRIDGE_MRP_IN_STATE_IN_ID]	= { .type = NLA_U32 },
> +	[IFLA_BRIDGE_MRP_IN_STATE_STATE]	= { .type = NLA_U32 },
> +};
> +
> +static int br_mrp_in_state_parse(struct net_bridge *br, struct nlattr *attr,
> +				 struct netlink_ext_ack *extack)
> +{
> +	struct nlattr *tb[IFLA_BRIDGE_MRP_IN_STATE_MAX + 1];
> +	struct br_mrp_in_state state;
> +	int err;
> +
> +	err = nla_parse_nested(tb, IFLA_BRIDGE_MRP_IN_STATE_MAX, attr,
> +			       br_mrp_in_state_policy, extack);
> +	if (err)
> +		return err;
> +
> +	if (!tb[IFLA_BRIDGE_MRP_IN_STATE_IN_ID] ||
> +	    !tb[IFLA_BRIDGE_MRP_IN_STATE_STATE]) {
> +		NL_SET_ERR_MSG_MOD(extack,
> +				   "Missing attribute: IN_ID or STATE");
> +		return -EINVAL;
> +	}
> +
> +	memset(&state, 0x0, sizeof(state));
> +
> +	state.in_id = nla_get_u32(tb[IFLA_BRIDGE_MRP_IN_STATE_IN_ID]);
> +	state.in_state = nla_get_u32(tb[IFLA_BRIDGE_MRP_IN_STATE_STATE]);
> +
> +	return br_mrp_set_in_state(br, &state);
> +}
> +
> +static const struct nla_policy
> +br_mrp_in_role_policy[IFLA_BRIDGE_MRP_IN_ROLE_MAX + 1] = {
> +	[IFLA_BRIDGE_MRP_IN_ROLE_UNSPEC]	= { .type = NLA_REJECT },
> +	[IFLA_BRIDGE_MRP_IN_ROLE_RING_ID]	= { .type = NLA_U32 },
> +	[IFLA_BRIDGE_MRP_IN_ROLE_IN_ID]		= { .type = NLA_U16 },
> +	[IFLA_BRIDGE_MRP_IN_ROLE_ROLE]		= { .type = NLA_U32 },
> +	[IFLA_BRIDGE_MRP_IN_ROLE_I_IFINDEX]	= { .type = NLA_U32 },
> +};
> +
> +static int br_mrp_in_role_parse(struct net_bridge *br, struct nlattr *attr,
> +				struct netlink_ext_ack *extack)
> +{
> +	struct nlattr *tb[IFLA_BRIDGE_MRP_IN_ROLE_MAX + 1];
> +	struct br_mrp_in_role role;
> +	int err;
> +
> +	err = nla_parse_nested(tb, IFLA_BRIDGE_MRP_IN_ROLE_MAX, attr,
> +			       br_mrp_in_role_policy, extack);
> +	if (err)
> +		return err;
> +
> +	if (!tb[IFLA_BRIDGE_MRP_IN_ROLE_RING_ID] ||
> +	    !tb[IFLA_BRIDGE_MRP_IN_ROLE_IN_ID] ||
> +	    !tb[IFLA_BRIDGE_MRP_IN_ROLE_I_IFINDEX] ||
> +	    !tb[IFLA_BRIDGE_MRP_IN_ROLE_ROLE]) {
> +		NL_SET_ERR_MSG_MOD(extack,
> +				   "Missing attribute: RING_ID or ROLE or IN_ID or I_IFINDEX");
> +		return -EINVAL;
> +	}
> +
> +	memset(&role, 0x0, sizeof(role));
> +
> +	role.ring_id = nla_get_u32(tb[IFLA_BRIDGE_MRP_IN_ROLE_RING_ID]);
> +	role.in_id = nla_get_u16(tb[IFLA_BRIDGE_MRP_IN_ROLE_IN_ID]);
> +	role.i_ifindex = nla_get_u32(tb[IFLA_BRIDGE_MRP_IN_ROLE_I_IFINDEX]);
> +	role.in_role = nla_get_u32(tb[IFLA_BRIDGE_MRP_IN_ROLE_ROLE]);
> +
> +	return br_mrp_set_in_role(br, &role);
> +}
> +
> +static const struct nla_policy
> +br_mrp_start_in_test_policy[IFLA_BRIDGE_MRP_START_IN_TEST_MAX + 1] = {
> +	[IFLA_BRIDGE_MRP_START_IN_TEST_UNSPEC]	= { .type = NLA_REJECT },
> +	[IFLA_BRIDGE_MRP_START_IN_TEST_IN_ID]	= { .type = NLA_U32 },
> +	[IFLA_BRIDGE_MRP_START_IN_TEST_INTERVAL]	= { .type = NLA_U32 },
> +	[IFLA_BRIDGE_MRP_START_IN_TEST_MAX_MISS]	= { .type = NLA_U32 },
> +	[IFLA_BRIDGE_MRP_START_IN_TEST_PERIOD]	= { .type = NLA_U32 },
> +};
> +
> +static int br_mrp_start_in_test_parse(struct net_bridge *br,
> +				      struct nlattr *attr,
> +				      struct netlink_ext_ack *extack)
> +{
> +	struct nlattr *tb[IFLA_BRIDGE_MRP_START_IN_TEST_MAX + 1];
> +	struct br_mrp_start_in_test test;
> +	int err;
> +
> +	err = nla_parse_nested(tb, IFLA_BRIDGE_MRP_START_IN_TEST_MAX, attr,
> +			       br_mrp_start_in_test_policy, extack);
> +	if (err)
> +		return err;
> +
> +	if (!tb[IFLA_BRIDGE_MRP_START_IN_TEST_IN_ID] ||
> +	    !tb[IFLA_BRIDGE_MRP_START_IN_TEST_INTERVAL] ||
> +	    !tb[IFLA_BRIDGE_MRP_START_IN_TEST_MAX_MISS] ||
> +	    !tb[IFLA_BRIDGE_MRP_START_IN_TEST_PERIOD]) {
> +		NL_SET_ERR_MSG_MOD(extack,
> +				   "Missing attribute: RING_ID or INTERVAL or MAX_MISS or PERIOD");
> +		return -EINVAL;
> +	}
> +
> +	memset(&test, 0x0, sizeof(test));
> +
> +	test.in_id = nla_get_u32(tb[IFLA_BRIDGE_MRP_START_IN_TEST_IN_ID]);
> +	test.interval = nla_get_u32(tb[IFLA_BRIDGE_MRP_START_IN_TEST_INTERVAL]);
> +	test.max_miss = nla_get_u32(tb[IFLA_BRIDGE_MRP_START_IN_TEST_MAX_MISS]);
> +	test.period = nla_get_u32(tb[IFLA_BRIDGE_MRP_START_IN_TEST_PERIOD]);
> +
> +	return br_mrp_start_in_test(br, &test);
> +}
> +
>  int br_mrp_parse(struct net_bridge *br, struct net_bridge_port *p,
>  		 struct nlattr *attr, int cmd, struct netlink_ext_ack *extack)
>  {
> @@ -301,6 +419,28 @@ int br_mrp_parse(struct net_bridge *br, struct net_bridge_port *p,
>  			return err;
>  	}
>  
> +	if (tb[IFLA_BRIDGE_MRP_IN_STATE]) {
> +		err = br_mrp_in_state_parse(br, tb[IFLA_BRIDGE_MRP_IN_STATE],
> +					    extack);
> +		if (err)
> +			return err;
> +	}
> +
> +	if (tb[IFLA_BRIDGE_MRP_IN_ROLE]) {
> +		err = br_mrp_in_role_parse(br, tb[IFLA_BRIDGE_MRP_IN_ROLE],
> +					   extack);
> +		if (err)
> +			return err;
> +	}
> +
> +	if (tb[IFLA_BRIDGE_MRP_START_IN_TEST]) {
> +		err = br_mrp_start_in_test_parse(br,
> +						 tb[IFLA_BRIDGE_MRP_START_IN_TEST],
> +						 extack);
> +		if (err)
> +			return err;
> +	}
> +
>  	return 0;
>  }
>  
> 

[PATCH net-next v4 10/12] bridge: uapi: mrp: Extend MRP_INFO attributes for interconnect status

[Horatiu Vultur]

Extend the existing MRP_INFO to return status of MRP interconnect. In
case there is no MRP interconnect on the node then the role will be
disabled so the other attributes can be ignored.

Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
---
 include/uapi/linux/if_bridge.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
index d840a3e37a37c..c1227aecd38fd 100644
--- a/include/uapi/linux/if_bridge.h
+++ b/include/uapi/linux/if_bridge.h
@@ -243,6 +243,11 @@ enum {
 	IFLA_BRIDGE_MRP_INFO_TEST_INTERVAL,
 	IFLA_BRIDGE_MRP_INFO_TEST_MAX_MISS,
 	IFLA_BRIDGE_MRP_INFO_TEST_MONITOR,
+	IFLA_BRIDGE_MRP_INFO_I_IFINDEX,
+	IFLA_BRIDGE_MRP_INFO_IN_STATE,
+	IFLA_BRIDGE_MRP_INFO_IN_ROLE,
+	IFLA_BRIDGE_MRP_INFO_IN_TEST_INTERVAL,
+	IFLA_BRIDGE_MRP_INFO_IN_TEST_MAX_MISS,
 	__IFLA_BRIDGE_MRP_INFO_MAX,
 };
 
-- 
2.27.0

Re: [PATCH net-next v4 10/12] bridge: uapi: mrp: Extend MRP_INFO attributes for interconnect status

[Nikolay Aleksandrov]

On 14/07/2020 10:34, Horatiu Vultur wrote:
> Extend the existing MRP_INFO to return status of MRP interconnect. In
> case there is no MRP interconnect on the node then the role will be
> disabled so the other attributes can be ignored.
> 
> Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> ---
>  include/uapi/linux/if_bridge.h | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
> index d840a3e37a37c..c1227aecd38fd 100644
> --- a/include/uapi/linux/if_bridge.h
> +++ b/include/uapi/linux/if_bridge.h
> @@ -243,6 +243,11 @@ enum {
>  	IFLA_BRIDGE_MRP_INFO_TEST_INTERVAL,
>  	IFLA_BRIDGE_MRP_INFO_TEST_MAX_MISS,
>  	IFLA_BRIDGE_MRP_INFO_TEST_MONITOR,
> +	IFLA_BRIDGE_MRP_INFO_I_IFINDEX,
> +	IFLA_BRIDGE_MRP_INFO_IN_STATE,
> +	IFLA_BRIDGE_MRP_INFO_IN_ROLE,
> +	IFLA_BRIDGE_MRP_INFO_IN_TEST_INTERVAL,
> +	IFLA_BRIDGE_MRP_INFO_IN_TEST_MAX_MISS,
>  	__IFLA_BRIDGE_MRP_INFO_MAX,
>  };
>  
> 

Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>

[PATCH net-next v4 11/12] bridge: mrp: Extend br_mrp_fill_info

[Horatiu Vultur]

This patch extends the function br_mrp_fill_info to return also the
status for the interconnect ring.

Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
---
 net/bridge/br_mrp_netlink.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/net/bridge/br_mrp_netlink.c b/net/bridge/br_mrp_netlink.c
index a006e0771e8d3..2a2fdf3500c5b 100644
--- a/net/bridge/br_mrp_netlink.c
+++ b/net/bridge/br_mrp_netlink.c
@@ -474,6 +474,11 @@ int br_mrp_fill_info(struct sk_buff *skb, struct net_bridge *br)
 				     p->dev->ifindex))
 			goto nla_put_failure;
 
+		p = rcu_dereference(mrp->i_port);
+		if (p && nla_put_u32(skb, IFLA_BRIDGE_MRP_INFO_I_IFINDEX,
+				     p->dev->ifindex))
+			goto nla_put_failure;
+
 		if (nla_put_u16(skb, IFLA_BRIDGE_MRP_INFO_PRIO,
 				mrp->prio))
 			goto nla_put_failure;
@@ -493,6 +498,19 @@ int br_mrp_fill_info(struct sk_buff *skb, struct net_bridge *br)
 				mrp->test_monitor))
 			goto nla_put_failure;
 
+		if (nla_put_u32(skb, IFLA_BRIDGE_MRP_INFO_IN_STATE,
+				mrp->in_state))
+			goto nla_put_failure;
+		if (nla_put_u32(skb, IFLA_BRIDGE_MRP_INFO_IN_ROLE,
+				mrp->in_role))
+			goto nla_put_failure;
+		if (nla_put_u32(skb, IFLA_BRIDGE_MRP_INFO_IN_TEST_INTERVAL,
+				mrp->in_test_interval))
+			goto nla_put_failure;
+		if (nla_put_u32(skb, IFLA_BRIDGE_MRP_INFO_IN_TEST_MAX_MISS,
+				mrp->in_test_max_miss))
+			goto nla_put_failure;
+
 		nla_nest_end(skb, tb);
 	}
 	nla_nest_end(skb, mrp_tb);
-- 
2.27.0

Re: [PATCH net-next v4 11/12] bridge: mrp: Extend br_mrp_fill_info

[Nikolay Aleksandrov]

On 14/07/2020 10:34, Horatiu Vultur wrote:
> This patch extends the function br_mrp_fill_info to return also the
> status for the interconnect ring.
> 
> Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> ---
>  net/bridge/br_mrp_netlink.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/net/bridge/br_mrp_netlink.c b/net/bridge/br_mrp_netlink.c
> index a006e0771e8d3..2a2fdf3500c5b 100644
> --- a/net/bridge/br_mrp_netlink.c
> +++ b/net/bridge/br_mrp_netlink.c
> @@ -474,6 +474,11 @@ int br_mrp_fill_info(struct sk_buff *skb, struct net_bridge *br)
>  				     p->dev->ifindex))
>  			goto nla_put_failure;
>  
> +		p = rcu_dereference(mrp->i_port);
> +		if (p && nla_put_u32(skb, IFLA_BRIDGE_MRP_INFO_I_IFINDEX,
> +				     p->dev->ifindex))
> +			goto nla_put_failure;
> +
>  		if (nla_put_u16(skb, IFLA_BRIDGE_MRP_INFO_PRIO,
>  				mrp->prio))
>  			goto nla_put_failure;
> @@ -493,6 +498,19 @@ int br_mrp_fill_info(struct sk_buff *skb, struct net_bridge *br)
>  				mrp->test_monitor))
>  			goto nla_put_failure;
>  
> +		if (nla_put_u32(skb, IFLA_BRIDGE_MRP_INFO_IN_STATE,
> +				mrp->in_state))
> +			goto nla_put_failure;
> +		if (nla_put_u32(skb, IFLA_BRIDGE_MRP_INFO_IN_ROLE,
> +				mrp->in_role))
> +			goto nla_put_failure;
> +		if (nla_put_u32(skb, IFLA_BRIDGE_MRP_INFO_IN_TEST_INTERVAL,
> +				mrp->in_test_interval))
> +			goto nla_put_failure;
> +		if (nla_put_u32(skb, IFLA_BRIDGE_MRP_INFO_IN_TEST_MAX_MISS,
> +				mrp->in_test_max_miss))
> +			goto nla_put_failure;
> +
>  		nla_nest_end(skb, tb);
>  	}
>  	nla_nest_end(skb, mrp_tb);
> 

Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>

[PATCH net-next v4 12/12] net: bridge: Add port attribute IFLA_BRPORT_MRP_IN_OPEN

[Horatiu Vultur]

This patch adds a new port attribute, IFLA_BRPORT_MRP_IN_OPEN, which
allows to notify the userspace when the node lost the contiuity of
MRP_InTest frames.

Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
---
 include/uapi/linux/if_link.h       | 1 +
 net/bridge/br_netlink.c            | 3 +++
 tools/include/uapi/linux/if_link.h | 1 +
 3 files changed, 5 insertions(+)

diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index cc185a007ade8..26842ffd0501d 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -344,6 +344,7 @@ enum {
 	IFLA_BRPORT_ISOLATED,
 	IFLA_BRPORT_BACKUP_PORT,
 	IFLA_BRPORT_MRP_RING_OPEN,
+	IFLA_BRPORT_MRP_IN_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 c532fa65c9834..147d52596e174 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -152,6 +152,7 @@ static inline size_t br_port_info_size(void)
 #endif
 		+ nla_total_size(sizeof(u16))	/* IFLA_BRPORT_GROUP_FWD_MASK */
 		+ nla_total_size(sizeof(u8))	/* IFLA_BRPORT_MRP_RING_OPEN */
+		+ nla_total_size(sizeof(u8))	/* IFLA_BRPORT_MRP_IN_OPEN */
 		+ 0;
 }
 
@@ -216,6 +217,8 @@ static int br_port_fill_attrs(struct sk_buff *skb,
 		       !!(p->flags & BR_NEIGH_SUPPRESS)) ||
 	    nla_put_u8(skb, IFLA_BRPORT_MRP_RING_OPEN, !!(p->flags &
 							  BR_MRP_LOST_CONT)) ||
+	    nla_put_u8(skb, IFLA_BRPORT_MRP_IN_OPEN,
+		       !!(p->flags & BR_MRP_LOST_IN_CONT)) ||
 	    nla_put_u8(skb, IFLA_BRPORT_ISOLATED, !!(p->flags & BR_ISOLATED)))
 		return -EMSGSIZE;
 
diff --git a/tools/include/uapi/linux/if_link.h b/tools/include/uapi/linux/if_link.h
index cafedbbfefbe9..781e482dc499f 100644
--- a/tools/include/uapi/linux/if_link.h
+++ b/tools/include/uapi/linux/if_link.h
@@ -344,6 +344,7 @@ enum {
 	IFLA_BRPORT_ISOLATED,
 	IFLA_BRPORT_BACKUP_PORT,
 	IFLA_BRPORT_MRP_RING_OPEN,
+	IFLA_BRPORT_MRP_IN_OPEN,
 	__IFLA_BRPORT_MAX
 };
 #define IFLA_BRPORT_MAX (__IFLA_BRPORT_MAX - 1)
-- 
2.27.0

Re: [PATCH net-next v4 12/12] net: bridge: Add port attribute IFLA_BRPORT_MRP_IN_OPEN

[Nikolay Aleksandrov]

On 14/07/2020 10:34, Horatiu Vultur wrote:
> This patch adds a new port attribute, IFLA_BRPORT_MRP_IN_OPEN, which
> allows to notify the userspace when the node lost the contiuity of
> MRP_InTest frames.
> 
> Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> ---
>  include/uapi/linux/if_link.h       | 1 +
>  net/bridge/br_netlink.c            | 3 +++
>  tools/include/uapi/linux/if_link.h | 1 +
>  3 files changed, 5 insertions(+)
> 

It's kind of late by now, but I'd wish these were contained in a nested MRP attribute. :)
Horatiu, do you expect to have many more MRP attributes outside of MRP netlink code?

Perhaps we should at least dump them only for MRP-aware ports, that should be easy.
They make no sense outside of MRP anyway, but increase the size of the dump for all
right now.

Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>

> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
> index cc185a007ade8..26842ffd0501d 100644
> --- a/include/uapi/linux/if_link.h
> +++ b/include/uapi/linux/if_link.h
> @@ -344,6 +344,7 @@ enum {
>  	IFLA_BRPORT_ISOLATED,
>  	IFLA_BRPORT_BACKUP_PORT,
>  	IFLA_BRPORT_MRP_RING_OPEN,
> +	IFLA_BRPORT_MRP_IN_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 c532fa65c9834..147d52596e174 100644
> --- a/net/bridge/br_netlink.c
> +++ b/net/bridge/br_netlink.c
> @@ -152,6 +152,7 @@ static inline size_t br_port_info_size(void)
>  #endif
>  		+ nla_total_size(sizeof(u16))	/* IFLA_BRPORT_GROUP_FWD_MASK */
>  		+ nla_total_size(sizeof(u8))	/* IFLA_BRPORT_MRP_RING_OPEN */
> +		+ nla_total_size(sizeof(u8))	/* IFLA_BRPORT_MRP_IN_OPEN */
>  		+ 0;
>  }
>  
> @@ -216,6 +217,8 @@ static int br_port_fill_attrs(struct sk_buff *skb,
>  		       !!(p->flags & BR_NEIGH_SUPPRESS)) ||
>  	    nla_put_u8(skb, IFLA_BRPORT_MRP_RING_OPEN, !!(p->flags &
>  							  BR_MRP_LOST_CONT)) ||
> +	    nla_put_u8(skb, IFLA_BRPORT_MRP_IN_OPEN,
> +		       !!(p->flags & BR_MRP_LOST_IN_CONT)) ||
>  	    nla_put_u8(skb, IFLA_BRPORT_ISOLATED, !!(p->flags & BR_ISOLATED)))
>  		return -EMSGSIZE;
>  
> diff --git a/tools/include/uapi/linux/if_link.h b/tools/include/uapi/linux/if_link.h
> index cafedbbfefbe9..781e482dc499f 100644
> --- a/tools/include/uapi/linux/if_link.h
> +++ b/tools/include/uapi/linux/if_link.h
> @@ -344,6 +344,7 @@ enum {
>  	IFLA_BRPORT_ISOLATED,
>  	IFLA_BRPORT_BACKUP_PORT,
>  	IFLA_BRPORT_MRP_RING_OPEN,
> +	IFLA_BRPORT_MRP_IN_OPEN,
>  	__IFLA_BRPORT_MAX
>  };
>  #define IFLA_BRPORT_MAX (__IFLA_BRPORT_MAX - 1)
> 

Re: [PATCH net-next v4 12/12] net: bridge: Add port attribute IFLA_BRPORT_MRP_IN_OPEN

[Horatiu Vultur]

The 07/14/2020 16:29, Nikolay Aleksandrov wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 14/07/2020 10:34, Horatiu Vultur wrote:
> > This patch adds a new port attribute, IFLA_BRPORT_MRP_IN_OPEN, which
> > allows to notify the userspace when the node lost the contiuity of
> > MRP_InTest frames.
> >
> > Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> > ---
> >  include/uapi/linux/if_link.h       | 1 +
> >  net/bridge/br_netlink.c            | 3 +++
> >  tools/include/uapi/linux/if_link.h | 1 +
> >  3 files changed, 5 insertions(+)
> >

Hi Nik,

> 
> It's kind of late by now, but I'd wish these were contained in a nested MRP attribute. :)
> Horatiu, do you expect to have many more MRP attributes outside of MRP netlink code?

I don't expect to add any other MRP attributes outside of MRP netlink
code.

> 
> Perhaps we should at least dump them only for MRP-aware ports, that should be easy.
> They make no sense outside of MRP anyway, but increase the size of the dump for all
> right now.

You are right. Then should I first send a fix on the net for this and
after that I will fix these patches or just fix this in the next patch
series?

> 
> Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
> 
> > diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
> > index cc185a007ade8..26842ffd0501d 100644
> > --- a/include/uapi/linux/if_link.h
> > +++ b/include/uapi/linux/if_link.h
> > @@ -344,6 +344,7 @@ enum {
> >       IFLA_BRPORT_ISOLATED,
> >       IFLA_BRPORT_BACKUP_PORT,
> >       IFLA_BRPORT_MRP_RING_OPEN,
> > +     IFLA_BRPORT_MRP_IN_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 c532fa65c9834..147d52596e174 100644
> > --- a/net/bridge/br_netlink.c
> > +++ b/net/bridge/br_netlink.c
> > @@ -152,6 +152,7 @@ static inline size_t br_port_info_size(void)
> >  #endif
> >               + nla_total_size(sizeof(u16))   /* IFLA_BRPORT_GROUP_FWD_MASK */
> >               + nla_total_size(sizeof(u8))    /* IFLA_BRPORT_MRP_RING_OPEN */
> > +             + nla_total_size(sizeof(u8))    /* IFLA_BRPORT_MRP_IN_OPEN */
> >               + 0;
> >  }
> >
> > @@ -216,6 +217,8 @@ static int br_port_fill_attrs(struct sk_buff *skb,
> >                      !!(p->flags & BR_NEIGH_SUPPRESS)) ||
> >           nla_put_u8(skb, IFLA_BRPORT_MRP_RING_OPEN, !!(p->flags &
> >                                                         BR_MRP_LOST_CONT)) ||
> > +         nla_put_u8(skb, IFLA_BRPORT_MRP_IN_OPEN,
> > +                    !!(p->flags & BR_MRP_LOST_IN_CONT)) ||
> >           nla_put_u8(skb, IFLA_BRPORT_ISOLATED, !!(p->flags & BR_ISOLATED)))
> >               return -EMSGSIZE;
> >
> > diff --git a/tools/include/uapi/linux/if_link.h b/tools/include/uapi/linux/if_link.h
> > index cafedbbfefbe9..781e482dc499f 100644
> > --- a/tools/include/uapi/linux/if_link.h
> > +++ b/tools/include/uapi/linux/if_link.h
> > @@ -344,6 +344,7 @@ enum {
> >       IFLA_BRPORT_ISOLATED,
> >       IFLA_BRPORT_BACKUP_PORT,
> >       IFLA_BRPORT_MRP_RING_OPEN,
> > +     IFLA_BRPORT_MRP_IN_OPEN,
> >       __IFLA_BRPORT_MAX
> >  };
> >  #define IFLA_BRPORT_MAX (__IFLA_BRPORT_MAX - 1)
> >
> 

-- 
/Horatiu

Re: [PATCH net-next v4 12/12] net: bridge: Add port attribute IFLA_BRPORT_MRP_IN_OPEN

[Nikolay Aleksandrov]

On 14/07/2020 18:07, Horatiu Vultur wrote:
> The 07/14/2020 16:29, Nikolay Aleksandrov wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> On 14/07/2020 10:34, Horatiu Vultur wrote:
>>> This patch adds a new port attribute, IFLA_BRPORT_MRP_IN_OPEN, which
>>> allows to notify the userspace when the node lost the contiuity of
>>> MRP_InTest frames.
>>>
>>> Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
>>> ---
>>>  include/uapi/linux/if_link.h       | 1 +
>>>  net/bridge/br_netlink.c            | 3 +++
>>>  tools/include/uapi/linux/if_link.h | 1 +
>>>  3 files changed, 5 insertions(+)
>>>
> 
> Hi Nik,
> 
>>
>> It's kind of late by now, but I'd wish these were contained in a nested MRP attribute. :)
>> Horatiu, do you expect to have many more MRP attributes outside of MRP netlink code?
> 
> I don't expect to add any other MRP attributes outside of MRP netlink
> code.
> 
>>
>> Perhaps we should at least dump them only for MRP-aware ports, that should be easy.
>> They make no sense outside of MRP anyway, but increase the size of the dump for all
>> right now.
> 
> You are right. Then should I first send a fix on the net for this and
> after that I will fix these patches or just fix this in the next patch
> series?
> 

IMO it's more of an improvement rather than a bug, but since you don't expect to have more
attributes outside of MRP's netlink I guess we can drop it for now. Up to you.

It definitely shouldn't block this patch-set.

>>
>> Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
>>
>>> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
>>> index cc185a007ade8..26842ffd0501d 100644
>>> --- a/include/uapi/linux/if_link.h
>>> +++ b/include/uapi/linux/if_link.h
>>> @@ -344,6 +344,7 @@ enum {
>>>       IFLA_BRPORT_ISOLATED,
>>>       IFLA_BRPORT_BACKUP_PORT,
>>>       IFLA_BRPORT_MRP_RING_OPEN,
>>> +     IFLA_BRPORT_MRP_IN_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 c532fa65c9834..147d52596e174 100644
>>> --- a/net/bridge/br_netlink.c
>>> +++ b/net/bridge/br_netlink.c
>>> @@ -152,6 +152,7 @@ static inline size_t br_port_info_size(void)
>>>  #endif
>>>               + nla_total_size(sizeof(u16))   /* IFLA_BRPORT_GROUP_FWD_MASK */
>>>               + nla_total_size(sizeof(u8))    /* IFLA_BRPORT_MRP_RING_OPEN */
>>> +             + nla_total_size(sizeof(u8))    /* IFLA_BRPORT_MRP_IN_OPEN */
>>>               + 0;
>>>  }
>>>
>>> @@ -216,6 +217,8 @@ static int br_port_fill_attrs(struct sk_buff *skb,
>>>                      !!(p->flags & BR_NEIGH_SUPPRESS)) ||
>>>           nla_put_u8(skb, IFLA_BRPORT_MRP_RING_OPEN, !!(p->flags &
>>>                                                         BR_MRP_LOST_CONT)) ||
>>> +         nla_put_u8(skb, IFLA_BRPORT_MRP_IN_OPEN,
>>> +                    !!(p->flags & BR_MRP_LOST_IN_CONT)) ||
>>>           nla_put_u8(skb, IFLA_BRPORT_ISOLATED, !!(p->flags & BR_ISOLATED)))
>>>               return -EMSGSIZE;
>>>
>>> diff --git a/tools/include/uapi/linux/if_link.h b/tools/include/uapi/linux/if_link.h
>>> index cafedbbfefbe9..781e482dc499f 100644
>>> --- a/tools/include/uapi/linux/if_link.h
>>> +++ b/tools/include/uapi/linux/if_link.h
>>> @@ -344,6 +344,7 @@ enum {
>>>       IFLA_BRPORT_ISOLATED,
>>>       IFLA_BRPORT_BACKUP_PORT,
>>>       IFLA_BRPORT_MRP_RING_OPEN,
>>> +     IFLA_BRPORT_MRP_IN_OPEN,
>>>       __IFLA_BRPORT_MAX
>>>  };
>>>  #define IFLA_BRPORT_MAX (__IFLA_BRPORT_MAX - 1)
>>>
>>
> 

Re: [PATCH net-next v4 12/12] net: bridge: Add port attribute IFLA_BRPORT_MRP_IN_OPEN

[Horatiu Vultur]

The 07/14/2020 18:33, Nikolay Aleksandrov wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 14/07/2020 18:07, Horatiu Vultur wrote:
> > The 07/14/2020 16:29, Nikolay Aleksandrov wrote:
> >> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >>
> >> On 14/07/2020 10:34, Horatiu Vultur wrote:
> >>> This patch adds a new port attribute, IFLA_BRPORT_MRP_IN_OPEN, which
> >>> allows to notify the userspace when the node lost the contiuity of
> >>> MRP_InTest frames.
> >>>
> >>> Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> >>> ---
> >>>  include/uapi/linux/if_link.h       | 1 +
> >>>  net/bridge/br_netlink.c            | 3 +++
> >>>  tools/include/uapi/linux/if_link.h | 1 +
> >>>  3 files changed, 5 insertions(+)
> >>>
> >
> > Hi Nik,
> >
> >>
> >> It's kind of late by now, but I'd wish these were contained in a nested MRP attribute. :)
> >> Horatiu, do you expect to have many more MRP attributes outside of MRP netlink code?
> >
> > I don't expect to add any other MRP attributes outside of MRP netlink
> > code.
> >
> >>
> >> Perhaps we should at least dump them only for MRP-aware ports, that should be easy.
> >> They make no sense outside of MRP anyway, but increase the size of the dump for all
> >> right now.
> >
> > You are right. Then should I first send a fix on the net for this and
> > after that I will fix these patches or just fix this in the next patch
> > series?
> >
> 
> IMO it's more of an improvement rather than a bug, but since you don't expect to have more
> attributes outside of MRP's netlink I guess we can drop it for now. Up to you.

OK, lets just drop it for now.

> 
> It definitely shouldn't block this patch-set.
> 
> >>
> >> Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
> >>
> >>> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
> >>> index cc185a007ade8..26842ffd0501d 100644
> >>> --- a/include/uapi/linux/if_link.h
> >>> +++ b/include/uapi/linux/if_link.h
> >>> @@ -344,6 +344,7 @@ enum {
> >>>       IFLA_BRPORT_ISOLATED,
> >>>       IFLA_BRPORT_BACKUP_PORT,
> >>>       IFLA_BRPORT_MRP_RING_OPEN,
> >>> +     IFLA_BRPORT_MRP_IN_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 c532fa65c9834..147d52596e174 100644
> >>> --- a/net/bridge/br_netlink.c
> >>> +++ b/net/bridge/br_netlink.c
> >>> @@ -152,6 +152,7 @@ static inline size_t br_port_info_size(void)
> >>>  #endif
> >>>               + nla_total_size(sizeof(u16))   /* IFLA_BRPORT_GROUP_FWD_MASK */
> >>>               + nla_total_size(sizeof(u8))    /* IFLA_BRPORT_MRP_RING_OPEN */
> >>> +             + nla_total_size(sizeof(u8))    /* IFLA_BRPORT_MRP_IN_OPEN */
> >>>               + 0;
> >>>  }
> >>>
> >>> @@ -216,6 +217,8 @@ static int br_port_fill_attrs(struct sk_buff *skb,
> >>>                      !!(p->flags & BR_NEIGH_SUPPRESS)) ||
> >>>           nla_put_u8(skb, IFLA_BRPORT_MRP_RING_OPEN, !!(p->flags &
> >>>                                                         BR_MRP_LOST_CONT)) ||
> >>> +         nla_put_u8(skb, IFLA_BRPORT_MRP_IN_OPEN,
> >>> +                    !!(p->flags & BR_MRP_LOST_IN_CONT)) ||
> >>>           nla_put_u8(skb, IFLA_BRPORT_ISOLATED, !!(p->flags & BR_ISOLATED)))
> >>>               return -EMSGSIZE;
> >>>
> >>> diff --git a/tools/include/uapi/linux/if_link.h b/tools/include/uapi/linux/if_link.h
> >>> index cafedbbfefbe9..781e482dc499f 100644
> >>> --- a/tools/include/uapi/linux/if_link.h
> >>> +++ b/tools/include/uapi/linux/if_link.h
> >>> @@ -344,6 +344,7 @@ enum {
> >>>       IFLA_BRPORT_ISOLATED,
> >>>       IFLA_BRPORT_BACKUP_PORT,
> >>>       IFLA_BRPORT_MRP_RING_OPEN,
> >>> +     IFLA_BRPORT_MRP_IN_OPEN,
> >>>       __IFLA_BRPORT_MAX
> >>>  };
> >>>  #define IFLA_BRPORT_MAX (__IFLA_BRPORT_MAX - 1)
> >>>
> >>
> >
> 

-- 
/Horatiu

Re: [PATCH net-next v4 00/12] bridge: mrp: Add support for interconnect ring

[David Miller]

From: Horatiu Vultur <horatiu.vultur@microchip.com>
Date: Tue, 14 Jul 2020 09:34:46 +0200

> This patch series extends existing MRP to add support for interconnect ring.
 ...

Series applied, thank you.