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.vul...@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 <niko...@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) >>> >> >