On 23/11/2020 13:14, Horatiu Vultur wrote:
> Extend MRP to support LC mode(link check) for the interconnect port.
> This applies only to the interconnect ring.
>
> Opposite to RC mode(ring check) the LC mode is using CFM frames to
> detect when the link goes up or down and based on that the userspace
> will need to react.
> One advantage of the LC mode over RC mode is that there will be fewer
> frames in the normal rings. Because RC mode generates InTest on all
> ports while LC mode sends CFM frame only on the interconnect port.
>
> All 4 nodes part of the interconnect ring needs to have the same mode.
> And it is not possible to have running LC and RC mode at the same time
> on a node.
>
> Whenever the MIM starts it needs to detect the status of the other 3
> nodes in the interconnect ring so it would send a frame called
> InLinkStatus, on which the clients needs to reply with their link
> status.
>
> This patch adds the frame header for the frame InLinkStatus and
> extends existing rules on how to forward this frame.
>
> Signed-off-by: Horatiu Vultur <horatiu.vultur@xxxxxxxxxxxxx>
> ---
> include/uapi/linux/mrp_bridge.h | 7 +++++++
> net/bridge/br_mrp.c | 18 +++++++++++++++---
> 2 files changed, 22 insertions(+), 3 deletions(-)
>
Hi Horatiu,
The patch looks good overall, just one question below.
> diff --git a/include/uapi/linux/mrp_bridge.h b/include/uapi/linux/mrp_bridge.h
> index 6aeb13ef0b1e..450f6941a5a1 100644
> --- a/include/uapi/linux/mrp_bridge.h
> +++ b/include/uapi/linux/mrp_bridge.h
> @@ -61,6 +61,7 @@ enum br_mrp_tlv_header_type {
> 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_IN_LINK_STATUS = 0xa,
> BR_MRP_TLV_HEADER_OPTION = 0x7f,
> };
>
> @@ -156,4 +157,10 @@ struct br_mrp_in_link_hdr {
> __be16 interval;
> };
>
> +struct br_mrp_in_link_status_hdr {
> + __u8 sa[ETH_ALEN];
> + __be16 port_role;
> + __be16 id;
> +};
> +
I didn't see this struct used anywhere, am I missing anything?
Cheers,
Nik
> #endif
> diff --git a/net/bridge/br_mrp.c b/net/bridge/br_mrp.c
> index bb12fbf9aaf2..cec2c4e4561d 100644
> --- a/net/bridge/br_mrp.c
> +++ b/net/bridge/br_mrp.c
> @@ -858,7 +858,8 @@ static bool br_mrp_in_frame(struct sk_buff *skb)
> 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)
> + hdr->type == BR_MRP_TLV_HEADER_IN_LINK_UP ||
> + hdr->type == BR_MRP_TLV_HEADER_IN_LINK_STATUS)
> return true;
>
> return false;
> @@ -1126,9 +1127,9 @@ static int br_mrp_rcv(struct net_bridge_port *p,
> goto no_forward;
> }
> } else {
> - /* MIM should forward IntLinkChange and
> + /* MIM should forward IntLinkChange/Status and
> * IntTopoChange between ring ports but MIM
> - * should not forward IntLinkChange and
> + * should not forward IntLinkChange/Status and
> * IntTopoChange if the frame was received at
> * the interconnect port
> */
> @@ -1155,6 +1156,17 @@ static int br_mrp_rcv(struct net_bridge_port *p,
> in_type == BR_MRP_TLV_HEADER_IN_LINK_DOWN))
> goto forward;
>
> + /* MIC should forward IntLinkStatus frames only to
> + * interconnect port if it was received on a ring port.
> + * If it is received on interconnect port then, it
> + * should be forward on both ring ports
> + */
> + if (br_mrp_is_ring_port(p_port, s_port, p) &&
> + in_type == BR_MRP_TLV_HEADER_IN_LINK_STATUS) {
> + p_dst = NULL;
> + s_dst = NULL;
> + }
> +
> /* Should forward the InTopo frames only between the
> * ring ports
> */
>
