On Thu, Oct 12, 2017 at 10:19 AM, David Ahern <dsah...@gmail.com> wrote: > On 10/12/17 7:51 AM, Roman Mashak wrote: >> v2: >> Return err immediately if nbp_vlan_delete() fails (pointed by David Ahern) >> >> Signed-off-by: Roman Mashak <m...@mojatatu.com> >> --- >> net/bridge/br_netlink.c | 8 +++++--- >> 1 file changed, 5 insertions(+), 3 deletions(-) >> >> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c >> index f0e8268..1efdd48 100644 >> --- a/net/bridge/br_netlink.c >> +++ b/net/bridge/br_netlink.c >> @@ -527,11 +527,13 @@ static int br_vlan_info(struct net_bridge *br, struct >> net_bridge_port *p, >> >> case RTM_DELLINK: >> if (p) { >> - nbp_vlan_delete(p, vinfo->vid); >> + err = nbp_vlan_delete(p, vinfo->vid); >> + if (err) >> + break; > > I'm not sure a break is the right thing to do. Seems like you leave it > in a half configured state. > >> if (vinfo->flags & BRIDGE_VLAN_INFO_MASTER) >> - br_vlan_delete(p->br, vinfo->vid); >> + err = br_vlan_delete(p->br, vinfo->vid); >> } else { >> - br_vlan_delete(br, vinfo->vid); >> + err = br_vlan_delete(br, vinfo->vid); >> } >> break; >> } >> > > Why do you want to return the error code here? Walking the code paths > seems like ENOENT or err from switchdev_port_obj_del are the 2 error > possibilities.
For example, if you attempt to delete a non-existing vlan on a port, the current code succeeds and also sends event : rtnetlink_rcv_msg rtnl_bridge_dellink br_dellink br_afspec br_vlan_info int br_dellink(..) { ... err = br_afspec() if (err == 0) br_ifinfo_notify(RTM_NEWLINK, p); } This is misleading, so a proper errcode has to be produced.