diff options
| author | Thibaut Collet <thibaut.collet@6wind.com> | 2018-05-29 10:34:38 +0200 |
|---|---|---|
| committer | Thibaut Collet <thibaut.collet@6wind.com> | 2018-08-30 14:37:59 +0200 |
| commit | ee2f2c23ca2778feafa398e773d24da07d6f174e (patch) | |
| tree | 8df2720aa15d22fab78106e98c7090c8b96935fc /zebra/if_netlink.c | |
| parent | 20c87e98d8399c322f3f8da9d34ca19fd4ca1865 (diff) | |
zebra: fix crash when interface vrf changes
This crash occurs only with netns implementation.
vrf meaning is different regarging its implementation (netns or
vrf-lite)
- With vrf-lite implementation vrf is a property of the interface that
can be changed as the speed or the state (iproute2 command: "ip link
set dev IF_NAME master VRF_NAME"). All interfaces of the system are in
the same netns and so interface name is unique.
- With netns implementation vrf is a characteristic of the interface
that CANNOT be changed: it is the id of the netns where the interface
is located. To change the vrf of an interface (iproute2 command to
move an interface "ip netns exec VRF_NAME1 ip link set dev IF_NAME
netns VRF_NAME2") the interface is deleted from the old vrf and
created in the new vrf.
Interface name is not unique, the same name can be present in the
different netns (typically the lo interface) and search of interface
must be done by the tuple (interface name, netns id).
Current tests on the vrf implementation (vrf-lite or netns) are not
sufficient. In some cases (for example when an interface is moved from
a vrf X to the default vrf and then move back to VRF X) we can have a
corruption message and then a crash of zebra.
To avoid this corruption test on the vrf implementation, needed when an
interface changes, has been rewritten:
- For all interface changes except deletion the if_get_by_name function,
that checks if an interface exists and creates or updates it if
needed, is changed:
* The vrf-lite implementation is unchanged: search of the interface
is based only on the name and update the vrf-id if needed.
* The netns implementation search of the interface is based on the
(name, vrf-id) tuple and interface is created if not found, the
vrf-id is never updated.
- deletion of an interface (reception of a RTM_DELLINK netlink message):
* The vrf-lite implementation is unchanged: the interface
information are cleared and the interface is moved to the default
vrf if it does not belong to (to allow vrf deletion)
* The netns implementation is changed: only the interface
information are cleared and the interface stays in its vrf to
avoid conflict with interface with the same name in the default
vrf.
This implementation reverts (partially or totally):
commit 393ec5424e35 ("zebra: fix missing node attribute set in ifp")
commit e9e9b1150f0c ("lib: create interface even if name is the same")
commit 9373219c67e1 ("zebra: improve logs when replacing interface to an
other netns")
Fixes: b53686c52a59 ("zebra: delete interface that disappeared")
Signed-off-by: Thibaut Collet <thibaut.collet@6wind.com>
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Diffstat (limited to 'zebra/if_netlink.c')
| -rw-r--r-- | zebra/if_netlink.c | 67 |
1 files changed, 0 insertions, 67 deletions
diff --git a/zebra/if_netlink.c b/zebra/if_netlink.c index a15d914243..1fc3e61a3b 100644 --- a/zebra/if_netlink.c +++ b/zebra/if_netlink.c @@ -1042,67 +1042,6 @@ int netlink_interface_addr(struct nlmsghdr *h, ns_id_t ns_id, int startup) return 0; } -/* helper function called by if_netlink_change - * to delete interfaces in case the interface moved - * to an other netns - */ -static void if_netlink_check_ifp_instance_consistency(uint16_t cmd, - struct interface *ifp, - ns_id_t ns_id) -{ - struct interface *other_ifp; - - /* - * look if interface name is also found on other netns - * - only if vrf backend is netns - * - do not concern lo interface - * - then remove previous one - * - for new link case, check found interface is not active - */ - if (!vrf_is_backend_netns() || - !strcmp(ifp->name, "lo")) - return; - other_ifp = if_lookup_by_name_not_ns(ns_id, ifp->name); - if (!other_ifp) - return; - /* because previous interface may be inactive, - * interface is moved back to default vrf - * then one may find the same pointer; ignore - */ - if (other_ifp == ifp) - return; - if ((cmd == RTM_NEWLINK) - && (CHECK_FLAG(other_ifp->status, ZEBRA_INTERFACE_ACTIVE))) - return; - if (IS_ZEBRA_DEBUG_KERNEL && cmd == RTM_NEWLINK) { - zlog_debug("RTM_NEWLINK %s(%u, VRF %u) replaces %s(%u, VRF %u)\n", - ifp->name, - ifp->ifindex, - ifp->vrf_id, - other_ifp->name, - other_ifp->ifindex, - other_ifp->vrf_id); - } else if (IS_ZEBRA_DEBUG_KERNEL && cmd == RTM_DELLINK) { - zlog_debug("RTM_DELLINK %s(%u, VRF %u) is replaced by %s(%u, VRF %u)\n", - ifp->name, - ifp->ifindex, - ifp->vrf_id, - other_ifp->name, - other_ifp->ifindex, - other_ifp->vrf_id); - } - /* the found interface replaces the current one - * remove it - */ - if (cmd == RTM_DELLINK) - if_delete(ifp); - else - if_delete(other_ifp); - /* the found interface is replaced by the current one - * suppress it - */ -} - int netlink_link_change(struct nlmsghdr *h, ns_id_t ns_id, int startup) { int len; @@ -1276,8 +1215,6 @@ int netlink_link_change(struct nlmsghdr *h, ns_id_t ns_id, int startup) if (IS_ZEBRA_IF_BRIDGE_SLAVE(ifp)) zebra_l2if_update_bridge_slave(ifp, bridge_ifindex); - if_netlink_check_ifp_instance_consistency(RTM_NEWLINK, - ifp, ns_id); } else if (ifp->vrf_id != vrf_id) { /* VRF change for an interface. */ if (IS_ZEBRA_DEBUG_KERNEL) @@ -1351,8 +1288,6 @@ int netlink_link_change(struct nlmsghdr *h, ns_id_t ns_id, int startup) if (IS_ZEBRA_IF_BRIDGE_SLAVE(ifp) || was_bridge_slave) zebra_l2if_update_bridge_slave(ifp, bridge_ifindex); - if_netlink_check_ifp_instance_consistency(RTM_NEWLINK, - ifp, ns_id); } } else { /* Delete interface notification from kernel */ @@ -1376,8 +1311,6 @@ int netlink_link_change(struct nlmsghdr *h, ns_id_t ns_id, int startup) if (!IS_ZEBRA_IF_VRF(ifp)) if_delete_update(ifp); - if_netlink_check_ifp_instance_consistency(RTM_DELLINK, - ifp, ns_id); } return 0; |
