From af4fa965e909db69ecde9deb6b7f9a53553aeccb Mon Sep 17 00:00:00 2001 From: Pim van Pelt Date: Sun, 21 May 2023 08:35:26 +0200 Subject: linux-cp: Fix add vs update on routes Linux uses NLM_F_REPLACE in the netlink message to signal a FIB update The code invariably does a FIB update for IPv4 and a addition for IPv6. Without this fix, the following: ip route add 2001:db8::/48 via 2001:db8::1 ip route replace 2001:db8::/48 via 2001:db8::2 ends up as two separate FIB entries in VPP. With the fix, there will be one FIB entry (the second one with nexthop ::2). Type: fix Change-Id: I8f98d6ded52ae0c60bfddaa7fc39acbbaa19d34a Signed-off-by: Pim van Pelt --- src/plugins/linux-cp/lcp_nl.c | 5 +- src/plugins/linux-cp/lcp_nl.h | 20 +++++--- src/plugins/linux-cp/lcp_router.c | 102 +++++++++++++++++++------------------- 3 files changed, 69 insertions(+), 58 deletions(-) (limited to 'src') diff --git a/src/plugins/linux-cp/lcp_nl.c b/src/plugins/linux-cp/lcp_nl.c index 8f2bffd6e99..b4fef7e0b40 100644 --- a/src/plugins/linux-cp/lcp_nl.c +++ b/src/plugins/linux-cp/lcp_nl.c @@ -205,7 +205,10 @@ nl_route_del (struct rtnl_route *rr, void *arg) static void nl_route_add (struct rtnl_route *rr, void *arg) { - FOREACH_VFT (nvl_rt_route_add, rr); + nl_msg_info_t *msg_info = (nl_msg_info_t *) arg; + struct nlmsghdr *nlh = nlmsg_hdr (msg_info->msg); + + FOREACH_VFT_CTX (nvl_rt_route_add, rr, (nlh->nlmsg_flags & NLM_F_REPLACE)); } static void diff --git a/src/plugins/linux-cp/lcp_nl.h b/src/plugins/linux-cp/lcp_nl.h index 7b2fccc29cd..41757e9b983 100644 --- a/src/plugins/linux-cp/lcp_nl.h +++ b/src/plugins/linux-cp/lcp_nl.h @@ -26,7 +26,8 @@ typedef void (*nl_rt_addr_cb_t) (struct rtnl_addr *ra); typedef void (*nl_rt_addr_sync_cb_t) (void); typedef void (*nl_rt_neigh_cb_t) (struct rtnl_neigh *rr); typedef void (*nl_rt_neigh_sync_cb_t) (void); -typedef void (*nl_rt_route_cb_t) (struct rtnl_route *rn); +typedef void (*nl_rt_route_add_cb_t) (struct rtnl_route *rn, int is_replace); +typedef void (*nl_rt_route_del_cb_t) (struct rtnl_route *rn); typedef void (*nl_rt_route_sync_cb_t) (void); #define NL_RT_COMMON uword is_mp_safe @@ -73,12 +74,19 @@ typedef struct nl_rt_neigh_sync_t_ nl_rt_neigh_sync_cb_t cb; } nl_rt_neigh_sync_t; -typedef struct nl_rt_route_t_ +typedef struct nl_rt_route_add_t_ { NL_RT_COMMON; - nl_rt_route_cb_t cb; -} nl_rt_route_t; + nl_rt_route_add_cb_t cb; +} nl_rt_route_add_t; + +typedef struct nl_rt_route_del_t_ +{ + NL_RT_COMMON; + + nl_rt_route_del_cb_t cb; +} nl_rt_route_del_t; typedef struct nl_rt_route_sync_t_ { @@ -103,8 +111,8 @@ typedef struct nl_vft_t_ nl_rt_neigh_t nvl_rt_neigh_del; nl_rt_neigh_sync_t nvl_rt_neigh_sync_begin; nl_rt_neigh_sync_t nvl_rt_neigh_sync_end; - nl_rt_route_t nvl_rt_route_add; - nl_rt_route_t nvl_rt_route_del; + nl_rt_route_add_t nvl_rt_route_add; + nl_rt_route_del_t nvl_rt_route_del; nl_rt_route_sync_t nvl_rt_route_sync_begin; nl_rt_route_sync_t nvl_rt_route_sync_end; } nl_vft_t; diff --git a/src/plugins/linux-cp/lcp_router.c b/src/plugins/linux-cp/lcp_router.c index ad701c5ceda..88b7c537afc 100644 --- a/src/plugins/linux-cp/lcp_router.c +++ b/src/plugins/linux-cp/lcp_router.c @@ -1184,7 +1184,7 @@ lcp_router_route_del (struct rtnl_route *rr) } static void -lcp_router_route_add (struct rtnl_route *rr) +lcp_router_route_add (struct rtnl_route *rr, int is_replace) { fib_entry_flag_t entry_flags; uint32_t table_id; @@ -1213,71 +1213,71 @@ lcp_router_route_add (struct rtnl_route *rr) LCP_ROUTER_DBG ("route skip: %d:%U %U", rtnl_route_get_table (rr), format_fib_prefix, &pfx, format_fib_entry_flags, entry_flags); + return; } - else - { - LCP_ROUTER_DBG ("route add: %d:%U %U", rtnl_route_get_table (rr), - format_fib_prefix, &pfx, format_fib_entry_flags, - entry_flags); + LCP_ROUTER_DBG ("route %s: %d:%U %U", is_replace ? "replace" : "add", + rtnl_route_get_table (rr), format_fib_prefix, &pfx, + format_fib_entry_flags, entry_flags); - lcp_router_route_path_parse_t np = { - .route_proto = pfx.fp_proto, - .is_mcast = (rtype == RTN_MULTICAST), - .type_flags = lcp_router_route_type_frpflags[rtype], - .preference = (u8) rtnl_route_get_priority (rr), - }; + lcp_router_route_path_parse_t np = { + .route_proto = pfx.fp_proto, + .is_mcast = (rtype == RTN_MULTICAST), + .type_flags = lcp_router_route_type_frpflags[rtype], + .preference = (u8) rtnl_route_get_priority (rr), + }; - rtnl_route_foreach_nexthop (rr, lcp_router_route_path_parse, &np); - lcp_router_route_path_add_special (rr, &np); + rtnl_route_foreach_nexthop (rr, lcp_router_route_path_parse, &np); + lcp_router_route_path_add_special (rr, &np); - if (0 != vec_len (np.paths)) + if (0 != vec_len (np.paths)) + { + if (rtype == RTN_MULTICAST) { - if (rtype == RTN_MULTICAST) - { - /* it's not clear to me how linux expresses the RPF paramters - * so we'll allow from all interfaces and hope for the best */ - mfib_prefix_t mpfx = {}; + /* it's not clear to me how linux expresses the RPF paramters + * so we'll allow from all interfaces and hope for the best */ + mfib_prefix_t mpfx = {}; - lcp_router_route_mk_mprefix (rr, &mpfx); + lcp_router_route_mk_mprefix (rr, &mpfx); - mfib_table_entry_update ( - nlt->nlt_mfib_index, &mpfx, MFIB_SOURCE_PLUGIN_LOW, - MFIB_RPF_ID_NONE, MFIB_ENTRY_FLAG_ACCEPT_ALL_ITF); + mfib_table_entry_update (nlt->nlt_mfib_index, &mpfx, + MFIB_SOURCE_PLUGIN_LOW, MFIB_RPF_ID_NONE, + MFIB_ENTRY_FLAG_ACCEPT_ALL_ITF); - mfib_table_entry_paths_update (nlt->nlt_mfib_index, &mpfx, - MFIB_SOURCE_PLUGIN_LOW, - MFIB_ENTRY_FLAG_NONE, np.paths); - } - else - { - fib_source_t fib_src; - const fib_route_path_t *rpath; + mfib_table_entry_paths_update (nlt->nlt_mfib_index, &mpfx, + MFIB_SOURCE_PLUGIN_LOW, + MFIB_ENTRY_FLAG_NONE, np.paths); + } + else + { + fib_source_t fib_src; + const fib_route_path_t *rpath; - vec_foreach (rpath, np.paths) + vec_foreach (rpath, np.paths) + { + if (fib_route_path_is_attached (rpath)) { - if (fib_route_path_is_attached (rpath)) - { - entry_flags |= FIB_ENTRY_FLAG_ATTACHED; - break; - } + entry_flags |= FIB_ENTRY_FLAG_ATTACHED; + break; } + } - fib_src = lcp_router_proto_fib_source (rproto); + fib_src = lcp_router_proto_fib_source (rproto); - if (pfx.fp_proto == FIB_PROTOCOL_IP6) - fib_table_entry_path_add2 (nlt->nlt_fib_index, &pfx, fib_src, - entry_flags, np.paths); - else - fib_table_entry_update (nlt->nlt_fib_index, &pfx, fib_src, - entry_flags, np.paths); - } + if (is_replace) + fib_table_entry_update (nlt->nlt_fib_index, &pfx, fib_src, + entry_flags, np.paths); + else + fib_table_entry_path_add2 (nlt->nlt_fib_index, &pfx, fib_src, + entry_flags, np.paths); } - else - LCP_ROUTER_DBG ("no paths for route add: %d:%U %U", - rtnl_route_get_table (rr), format_fib_prefix, &pfx, - format_fib_entry_flags, entry_flags); - vec_free (np.paths); } + else + { + LCP_ROUTER_DBG ("no paths for route: %d:%U %U", + rtnl_route_get_table (rr), format_fib_prefix, &pfx, + format_fib_entry_flags, entry_flags); + } + vec_free (np.paths); } static void -- cgit 1.2.3-korg