The 11/23/2020 14:13, Nikolay Aleksandrov wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> 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.
Hi Nik,
Thanks for taking time to review the patch.
>
> > 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?
Yes, you are right, the struct is not used any. But I put it there as I
put the other frame types for MRP.
>
> 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
> > */
> >
>
--
/Horatiu
