Conversation
This comment was marked as resolved.
This comment was marked as resolved.
32ee9ca to
cd7d523
Compare
cd7d523 to
f555616
Compare
f555616 to
2d6041a
Compare
2d6041a to
1054788
Compare
bdd5d30 to
3259146
Compare
Comment on lines
443
to
452
| static void nh_cleanup_interface_cb(struct nexthop *nh, void *priv) { | ||
| if (nh->iface_id == (uintptr_t)priv) { | ||
| if (nh->type == GR_NH_T_L3) { | ||
| struct nexthop_info_l3 *l3 = nexthop_info_l3(nh); | ||
| if ((l3->flags & NH_LOCAL_ADDR_FLAGS) == NH_LOCAL_ADDR_FLAGS) | ||
| return; // addresses are cleaned per address family | ||
| } | ||
| nexthop_routes_cleanup(nh); | ||
| while (nh->ref_count) | ||
| nexthop_decref(nh); |
Collaborator
There was a problem hiding this comment.
Is it possible to rely on GR_EVENT_IFACE_PRE_REMOVE and GR_EVENT_IFACE_REMOVE events to guarantee the order between nexthop_iface_cleanup and address-family-specific
handlers ?
Collaborator
Author
There was a problem hiding this comment.
We could do that (maybe) but the issue is that once GR_EVENT_IFACE_REMOVE fires, the interface_id is already unreachable/stale. So all nexthops that reference it point to NULL. As long as everyone checks the return value of iface_from_id(nh->iface_id) that's OK. But I'm not not 100% sure.
grep -q '10.' can match any 3 characters that start with '10' (e.g. 100, 10a, 10:, etc.). So it can match parts of IPv6 link local addresses. Use full address and -F/--fixed-strings to avoid any special regexp characters. We want verbatim match. Fixes: 74228b7 ("cli: add address flush command") Signed-off-by: Robin Jarry <rjarry@redhat.com>
There is no "name" argument available when creating an interface. The name is the first argument. Fixes: 9d5152f ("smoke: add VRF configuration tests") Signed-off-by: Robin Jarry <rjarry@redhat.com>
When removing a port which is the xconnect peer of another one,
iface_from_id(iface->domain_id) will return NULL since the interface was
deleted.
Program terminated with signal SIGSEGV, Segmentation fault.
xconnect_process at modules/infra/datapath/xconnect.c:36
if (peer->type == GR_IFACE_TYPE_PORT) {
__rte_node_process at subprojects/dpdk/lib/graph/rte_graph_worker_common.h:216
rte_graph_walk_rtc at subprojects/dpdk/lib/graph/rte_graph_model_rtc.h:42
rte_graph_walk at subprojects/dpdk/lib/graph/rte_graph_worker.h:38
gr_datapath_loop at modules/infra/datapath/main_loop.c:252
Check the return value and drop the packet in that case.
Signed-off-by: Robin Jarry <rjarry@redhat.com>
When an interface leaves VRF mode (e.g. reconfigured as cross-connect), any IPv4 and IPv6 addresses previously configured on it become invalid. Likewise, when an interface moves to a different VRF, its addresses belong to the old VRF and need to be removed. Subscribe to GR_EVENT_IFACE_POST_RECONFIG in both IPv4 and IPv6 address modules. On reconfiguration, flush all addresses when the interface is no longer in VRF mode or has moved to a different VRF. For IPv6, also reinitialize link-local and well-known multicast addresses when entering VRF mode or changing VRFs. Extend the IPv6 add/del smoke test to exercise VRF reassignment and cross-connect mode transitions. Signed-off-by: Robin Jarry <rjarry@redhat.com>
When an interface is removed, GR_EVENT_IFACE_PRE_REMOVE is handled by both nexthop_iface_cleanup() in nexthop.c and address-family-specific handlers in ip/control/address.c and ip6/control/address.c (added in 6a1362c "ip,ip6: flush addresses on interface mode change"). The event handler execution order is not guaranteed. If nexthop_iface_cleanup() runs first, it destroys local address nexthops by decrementing their ref_count to zero. When the address-family handler runs next, it accesses already-freed nexthops via nexthop_info_l3(), leading to use-after-free. Skip local address nexthops (NH_LOCAL_ADDR_FLAGS) in nh_cleanup_interface_cb(), leaving their cleanup to addr4_delete() and addr6_delete() which properly remove them from the per-interface address vector and handle associated routes. Signed-off-by: Robin Jarry <rjarry@redhat.com>
When a bond is destroyed, its member ports are detached but remain without a VRF assignment. When a port is destroyed, its peer interfaces (other ports whose domain_id points to this port) lose their domain reference. In both cases, reassign the orphaned ports to the default VRF and fire GR_EVENT_IFACE_POST_RECONFIG so that address-family handlers can flush stale addresses and reinitialize as needed. Export vrf_default_get_or_create() so it can be used from bond and port teardown paths. Signed-off-by: Robin Jarry <rjarry@redhat.com>
3259146 to
e3a165d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Release Notes
Bug Fixes
Improvements