| Message ID | 20201007120700.2152699-1-henrik.bjoernlund@microchip.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [net] bridge: Netlink interface fix. | expand |
On Wed, 2020-10-07 at 12:07 +0000, Henrik Bjoernlund wrote: > This commit is correcting NETLINK br_fill_ifinfo() to be able to > handle 'filter_mask' with multiple flags asserted. > > Fixes: 36a8e8e265420 ("bridge: Extend br_fill_ifinfo to return MPR status") > > Signed-off-by: Henrik Bjoernlund <henrik.bjoernlund@microchip.com> > Reviewed-by: Horatiu Vultur <horatiu.vultur@microchip.com> > Suggested-by: Nikolay Aleksandrov <nikolay@nvidia.com> > Tested-by: Horatiu Vultur <horatiu.vultur@microchip.com> > --- > net/bridge/br_netlink.c | 26 +++++++++++--------------- > 1 file changed, 11 insertions(+), 15 deletions(-) > The patch looks good, please don't separate the Fixes tag from the others. Acked-by: Nikolay Aleksandrov <nikolay@nvidia.com>
On Wed, 2020-10-07 at 14:49 +0000, Nikolay Aleksandrov wrote: > On Wed, 2020-10-07 at 12:07 +0000, Henrik Bjoernlund wrote: > > This commit is correcting NETLINK br_fill_ifinfo() to be able to > > handle 'filter_mask' with multiple flags asserted. > > > > Fixes: 36a8e8e265420 ("bridge: Extend br_fill_ifinfo to return MPR status") > > > > Signed-off-by: Henrik Bjoernlund <henrik.bjoernlund@microchip.com> > > Reviewed-by: Horatiu Vultur <horatiu.vultur@microchip.com> > > Suggested-by: Nikolay Aleksandrov <nikolay@nvidia.com> > > Tested-by: Horatiu Vultur <horatiu.vultur@microchip.com> > > --- > > net/bridge/br_netlink.c | 26 +++++++++++--------------- > > 1 file changed, 11 insertions(+), 15 deletions(-) > > > > The patch looks good, please don't separate the Fixes tag from the others. > Acked-by: Nikolay Aleksandrov <nikolay@nvidia.com> > TBH, this does change a user facing api (the attribute nesting), but I think in this case it's acceptable due to the format being wrong and MRP being new, so I doubt anyone is yet dumping it mixed with vlan filter_mask and checking for two identical attributes, i.e. in the old/broken case parsing the attributes into a table would hide one of them and you'd have to walk over all attributes to catch that.
On Thu, 2020-10-08 at 10:09 -0700, Jakub Kicinski wrote: > On Thu, 8 Oct 2020 10:18:09 +0000 Nikolay Aleksandrov wrote: > > On Wed, 2020-10-07 at 14:49 +0000, Nikolay Aleksandrov wrote: > > > On Wed, 2020-10-07 at 12:07 +0000, Henrik Bjoernlund wrote: > > > > This commit is correcting NETLINK br_fill_ifinfo() to be able to > > > > handle 'filter_mask' with multiple flags asserted. > > > > > > > > Fixes: 36a8e8e265420 ("bridge: Extend br_fill_ifinfo to return MPR status") > > > > > > > > Signed-off-by: Henrik Bjoernlund <henrik.bjoernlund@microchip.com> > > > > Reviewed-by: Horatiu Vultur <horatiu.vultur@microchip.com> > > > > Suggested-by: Nikolay Aleksandrov <nikolay@nvidia.com> > > > > Tested-by: Horatiu Vultur <horatiu.vultur@microchip.com> > > > > --- > > > > net/bridge/br_netlink.c | 26 +++++++++++--------------- > > > > 1 file changed, 11 insertions(+), 15 deletions(-) > > > > > > > > > > The patch looks good, please don't separate the Fixes tag from the others. > > > Acked-by: Nikolay Aleksandrov <nikolay@nvidia.com> > > > > > > > TBH, this does change a user facing api (the attribute nesting), but I think > > in this case it's acceptable due to the format being wrong and MRP being new, so > > I doubt anyone is yet dumping it mixed with vlan filter_mask and checking for > > two identical attributes, i.e. in the old/broken case parsing the attributes > > into a table would hide one of them and you'd have to walk over all attributes > > to catch that. > > To be clear - this changes the uAPI as far as 5.9-rcs are concerned. > So if this change was to hit 5.9 final there would be no uAPI breakage, > right? Yes, correct.
Hello: This patch was applied to netdev/net.git (refs/heads/master): On Wed, 7 Oct 2020 12:07:00 +0000 you wrote: > This commit is correcting NETLINK br_fill_ifinfo() to be able to > handle 'filter_mask' with multiple flags asserted. > > Fixes: 36a8e8e265420 ("bridge: Extend br_fill_ifinfo to return MPR status") > > Signed-off-by: Henrik Bjoernlund <henrik.bjoernlund@microchip.com> > Reviewed-by: Horatiu Vultur <horatiu.vultur@microchip.com> > Suggested-by: Nikolay Aleksandrov <nikolay@nvidia.com> > Tested-by: Horatiu Vultur <horatiu.vultur@microchip.com> > > [...] Here is the summary with links: - [net] bridge: Netlink interface fix. https://git.kernel.org/netdev/net/c/b6c02ef54913 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c index 8a71c60fa357..92d64abffa87 100644 --- a/net/bridge/br_netlink.c +++ b/net/bridge/br_netlink.c @@ -380,6 +380,7 @@ static int br_fill_ifinfo(struct sk_buff *skb, u32 filter_mask, const struct net_device *dev) { u8 operstate = netif_running(dev) ? dev->operstate : IF_OPER_DOWN; + struct nlattr *af = NULL; struct net_bridge *br; struct ifinfomsg *hdr; struct nlmsghdr *nlh; @@ -423,11 +424,18 @@ static int br_fill_ifinfo(struct sk_buff *skb, nla_nest_end(skb, nest); } + if (filter_mask & (RTEXT_FILTER_BRVLAN | + RTEXT_FILTER_BRVLAN_COMPRESSED | + RTEXT_FILTER_MRP)) { + af = nla_nest_start_noflag(skb, IFLA_AF_SPEC); + if (!af) + goto nla_put_failure; + } + /* Check if the VID information is requested */ if ((filter_mask & RTEXT_FILTER_BRVLAN) || (filter_mask & RTEXT_FILTER_BRVLAN_COMPRESSED)) { struct net_bridge_vlan_group *vg; - struct nlattr *af; int err; /* RCU needed because of the VLAN locking rules (rcu || rtnl) */ @@ -441,11 +449,6 @@ static int br_fill_ifinfo(struct sk_buff *skb, rcu_read_unlock(); goto done; } - af = nla_nest_start_noflag(skb, IFLA_AF_SPEC); - if (!af) { - rcu_read_unlock(); - goto nla_put_failure; - } if (filter_mask & RTEXT_FILTER_BRVLAN_COMPRESSED) err = br_fill_ifvlaninfo_compressed(skb, vg); else @@ -456,32 +459,25 @@ static int br_fill_ifinfo(struct sk_buff *skb, rcu_read_unlock(); if (err) goto nla_put_failure; - - nla_nest_end(skb, af); } if (filter_mask & RTEXT_FILTER_MRP) { - struct nlattr *af; int err; if (!br_mrp_enabled(br) || port) goto done; - af = nla_nest_start_noflag(skb, IFLA_AF_SPEC); - if (!af) - goto nla_put_failure; - rcu_read_lock(); err = br_mrp_fill_info(skb, br); rcu_read_unlock(); if (err) goto nla_put_failure; - - nla_nest_end(skb, af); } done: + if (af) + nla_nest_end(skb, af); nlmsg_end(skb, nlh); return 0;
This commit is correcting NETLINK br_fill_ifinfo() to be able to handle 'filter_mask' with multiple flags asserted. Fixes: 36a8e8e265420 ("bridge: Extend br_fill_ifinfo to return MPR status") Signed-off-by: Henrik Bjoernlund <henrik.bjoernlund@microchip.com> Reviewed-by: Horatiu Vultur <horatiu.vultur@microchip.com> Suggested-by: Nikolay Aleksandrov <nikolay@nvidia.com> Tested-by: Horatiu Vultur <horatiu.vultur@microchip.com> --- net/bridge/br_netlink.c | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-)