On Thu, Dec 11, 2025 at 12:32 PM Dumitru Ceara <[email protected]> wrote:
> On 12/10/25 11:18 AM, Ales Musil wrote: > > The advertised routes would always be specified as blackhole > > without any nexthop. Allow the route to be configured with > > specific nexthop even from different IP family to allow > > routing of IPv4 over IPv6 nexthop. > > > > At the same time also extend the netlink testing with > > route-sync so we can test this out independently of > > the rest of route advertisement. > > > > Signed-off-by: Ales Musil <[email protected]> > > --- > > Hi Ales, > > Thanks for the patch! > > > controller/route-exchange-netlink.c | 126 ++++++++++++------ > > controller/route-exchange-netlink.h | 9 +- > > controller/route.c | 16 ++- > > controller/route.h | 5 +- > > tests/automake.mk | 2 + > > tests/system-ovn-netlink.at | 200 ++++++++++++++++++++++++++++ > > tests/test-ovn-netlink.c | 68 ++++++++++ > > tests/test-utils.c | 18 +++ > > tests/test-utils.h | 4 + > > 9 files changed, 397 insertions(+), 51 deletions(-) > > > > diff --git a/controller/route-exchange-netlink.c > b/controller/route-exchange-netlink.c > > index 60bae4781..55670729f 100644 > > --- a/controller/route-exchange-netlink.c > > +++ b/controller/route-exchange-netlink.c > > @@ -38,6 +38,9 @@ VLOG_DEFINE_THIS_MODULE(route_exchange_netlink); > > > > #define NETNL_REQ_BUFFER_SIZE 128 > > > > +static void re_nl_encode_nexthop(struct ofpbuf *, bool dst_is_ipv4, > > + const struct in6_addr *); > > + > > int > > re_nl_create_vrf(const char *ifname, uint32_t table_id) > > { > > @@ -95,13 +98,32 @@ re_nl_delete_vrf(const char *ifname) > > return err; > > } > > > > +void > > +re_route_format(struct ds *ds, uint32_t table_id, const struct in6_addr > *dst, > > + unsigned int plen, const struct in6_addr *nexthop, int > err) > > +{ > > + ds_put_format(ds, "table_id=%"PRIu32" dst=", table_id); > > + ipv6_format_mapped(dst, ds); > > + ds_put_format(ds, " plen=%u nexthop=", plen); > > + if (ipv6_is_zero(nexthop)) { > > + ds_put_cstr(ds, "(blackhole)"); > > + } else { > > + ipv6_format_mapped(nexthop, ds); > > + } > > + > > + if (err) { > > + ds_put_format(ds, " failed: %s", ovs_strerror(err)); > > + } > > +} > > + > > static int > > modify_route(uint32_t type, uint32_t flags_arg, uint32_t table_id, > > const struct in6_addr *dst, unsigned int plen, > > - unsigned int priority) > > + const struct in6_addr *nexhhop, unsigned int priority) > > Typo: "nexhhop" > > > { > > uint32_t flags = NLM_F_REQUEST | NLM_F_ACK; > > bool is_ipv4 = IN6_IS_ADDR_V4MAPPED(dst); > > + bool nexthop_unspec = ipv6_is_zero(nexhhop); > > struct rtmsg *rt; > > int err; > > > > @@ -117,7 +139,7 @@ modify_route(uint32_t type, uint32_t flags_arg, > uint32_t table_id, > > rt->rtm_table = RT_TABLE_UNSPEC; /* RTA_TABLE attribute allows id > > 256 */ > > /* Manage only OVN routes */ > > rt->rtm_protocol = RTPROT_OVN; > > - rt->rtm_type = RTN_BLACKHOLE; > > + rt->rtm_type = nexthop_unspec ? RTN_BLACKHOLE : RTN_UNICAST; > > rt->rtm_scope = RT_SCOPE_UNIVERSE; > > rt->rtm_dst_len = plen; > > > > @@ -130,25 +152,16 @@ modify_route(uint32_t type, uint32_t flags_arg, > uint32_t table_id, > > nl_msg_put_in6_addr(&request, RTA_DST, dst); > > } > > > > + if (!nexthop_unspec) { > > + re_nl_encode_nexthop(&request, is_ipv4, nexhhop); > > + } > > + > > if (VLOG_IS_DBG_ENABLED()) { > > struct ds msg = DS_EMPTY_INITIALIZER; > > > > - if (type == RTM_DELROUTE) { > > - ds_put_cstr(&msg, "Removing blackhole route from "); > > - } else { > > - ds_put_cstr(&msg, "Adding blackhole route to "); > > - } > > - > > - ds_put_format(&msg, "table %"PRIu32 " for prefix ", table_id); > > - if (IN6_IS_ADDR_V4MAPPED(dst)) { > > - ds_put_format(&msg, IP_FMT, > > - IP_ARGS(in6_addr_get_mapped_ipv4(dst))); > > - } else { > > - ipv6_format_addr(dst, &msg); > > - } > > - ds_put_format(&msg, "/%u", plen); > > - > > - VLOG_DBG("%s", ds_cstr(&msg)); > > + re_route_format(&msg, table_id, dst, plen, nexhhop, 0); > > + VLOG_DBG("%s route %s", type == RTM_DELROUTE ? "Removing" : > "Adding", > > + ds_cstr(&msg)); > > ds_destroy(&msg); > > } > > > > @@ -160,7 +173,8 @@ modify_route(uint32_t type, uint32_t flags_arg, > uint32_t table_id, > > > > int > > re_nl_add_route(uint32_t table_id, const struct in6_addr *dst, > > - unsigned int plen, unsigned int priority) > > + unsigned int plen, const struct in6_addr *nexthop, > > + unsigned int priority) > > { > > if (!TABLE_ID_VALID(table_id)) { > > static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20); > > @@ -171,12 +185,13 @@ re_nl_add_route(uint32_t table_id, const struct > in6_addr *dst, > > } > > > > return modify_route(RTM_NEWROUTE, NLM_F_CREATE | NLM_F_EXCL, > table_id, > > - dst, plen, priority); > > + dst, plen, nexthop, priority); > > } > > > > int > > re_nl_delete_route(uint32_t table_id, const struct in6_addr *dst, > > - unsigned int plen, unsigned int priority) > > + unsigned int plen, const struct in6_addr *nexthop, > > + unsigned int priority) > > { > > if (!TABLE_ID_VALID(table_id)) { > > static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20); > > @@ -186,7 +201,8 @@ re_nl_delete_route(uint32_t table_id, const struct > in6_addr *dst, > > return EINVAL; > > } > > > > - return modify_route(RTM_DELROUTE, 0, table_id, dst, plen, priority); > > + return modify_route(RTM_DELROUTE, 0, table_id, dst, plen, > > + nexthop, priority); > > } > > > > struct route_msg_handle_data { > > @@ -249,10 +265,14 @@ handle_route_msg(const struct route_table_msg > *msg, void *data) > > } > > > > if (handle_data->routes_to_advertise) { > > - uint32_t hash = advertise_route_hash(&rd->rta_dst, > rd->rtm_dst_len); > > + uint32_t hash = advertise_route_hash(&rd->rta_dst, > > + > &rd->primary_next_hop__.addr, > > + rd->rtm_dst_len); > > HMAP_FOR_EACH_WITH_HASH (ar, node, hash, handle_data->routes) { > > if (ipv6_addr_equals(&ar->addr, &rd->rta_dst) > > && ar->plen == rd->rtm_dst_len > > + && ipv6_addr_equals(&ar->nexthop, > > + &rd->primary_next_hop__.addr) > > I wonder if the reason behind OVS naming this field with trailing > underscores is because it should be "private". Looking at the > definition in 'struct route_data': > > * For the common case of one next hop, the nexthops list will contain a > * single entry pointing to the struct route_data primary_next_hop__ > * element. > > I think it would make sense to use: > > CONTAINER_OF(ovs_list_front(&rd->nexthops), > const struct route_data_nexthop, > nexthop_node); > > instead. > > Applies to the two occurences below too. > > > && ar->priority == rd->rta_priority) { > > hmapx_find_and_delete(handle_data->routes_to_advertise, > ar); > > return; > > @@ -268,30 +288,52 @@ handle_route_msg(const struct route_table_msg > *msg, void *data) > > static int > > re_nl_delete_stale_routes(const struct vector *stale_routes) > > { > > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20); > > + struct ds ds = DS_EMPTY_INITIALIZER; > > int ret = 0; > > > > const struct route_data *rd; > > VECTOR_FOR_EACH_PTR (stale_routes, rd) { > > int err = re_nl_delete_route(rd->rta_table_id, &rd->rta_dst, > > - rd->rtm_dst_len, rd->rta_priority); > > + rd->rtm_dst_len, > > + &rd->primary_next_hop__.addr, > > + rd->rta_priority); > > if (err) { > > - char addr_s[INET6_ADDRSTRLEN + 1]; > > - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, > 20); > > - VLOG_WARN_RL(&rl, "Delete route table_id=%"PRIu32" dst=%s > plen=%d " > > - "failed: %s", rd->rta_table_id, > > - ipv6_string_mapped( > > - addr_s, &rd->rta_dst) ? addr_s : > "(invalid)", > > - rd->rtm_dst_len, > > - ovs_strerror(err)); > > + re_route_format(&ds, rd->rta_table_id, &rd->rta_dst, > > + rd->rtm_dst_len, > &rd->primary_next_hop__.addr, > > + err); > > + VLOG_WARN_RL(&rl, "Delete route %s", ds_cstr(&ds)); > > + ds_clear(&ds); > > if (!ret) { > > ret = err; > > } > > } > > } > > > > + ds_destroy(&ds); > > return ret; > > } > > > > +static void > > +re_nl_encode_nexthop(struct ofpbuf *request, bool dst_is_ipv4, > > + const struct in6_addr *nexthop) > > +{ > > + bool nh_is_ipv4 = IN6_IS_ADDR_V4MAPPED(nexthop); > > + size_t len = nh_is_ipv4 ? sizeof(ovs_be32) : sizeof(struct > in6_addr); > > + > > + ovs_be32 nexthop4 = in6_addr_get_mapped_ipv4(nexthop); > > + void *nl_attr_dst = nh_is_ipv4 ? (void *) &nexthop4 : (void *) > nexthop; > > + > > + if (dst_is_ipv4 != nh_is_ipv4) { > > + struct rtvia *via = nl_msg_put_unspec_uninit(request, RTA_VIA, > > + sizeof *via + len); > > + via->rtvia_family = nh_is_ipv4 ? AF_INET : AF_INET6; > > + memcpy(via->rtvia_addr, nl_attr_dst, len); > > + } else { > > + nl_msg_put_unspec(request, RTA_GATEWAY, nl_attr_dst, len); > > + } > > +} > > + > > int > > re_nl_sync_routes(uint32_t table_id, const struct hmap *routes, > > struct vector *learned_routes, > > @@ -320,22 +362,21 @@ re_nl_sync_routes(uint32_t table_id, const struct > hmap *routes, > > > > int ret = re_nl_delete_stale_routes(&stale_routes); > > > > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20); > > + struct ds ds = DS_EMPTY_INITIALIZER; > > + > > /* Add any remaining routes in the routes_to_advertise hmapx to the > > * system routing table. */ > > struct hmapx_node *hn; > > HMAPX_FOR_EACH (hn, &routes_to_advertise) { > > ar = hn->data; > > - int err = re_nl_add_route(table_id, &ar->addr, ar->plen, > ar->priority); > > + int err = re_nl_add_route(table_id, &ar->addr, ar->plen, > > + &ar->nexthop, ar->priority); > > if (err) { > > - char addr_s[INET6_ADDRSTRLEN + 1]; > > - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, > 20); > > - VLOG_WARN_RL(&rl, "Add route table_id=%"PRIu32" dst=%s " > > - "plen=%d: %s", > > - table_id, > > - ipv6_string_mapped( > > - addr_s, &ar->addr) ? addr_s : "(invalid)", > > - ar->plen, > > - ovs_strerror(err)); > > + re_route_format(&ds, table_id, &ar->addr, ar->plen, > > + &ar->nexthop, err); > > + VLOG_WARN_RL(&rl, "Add route %s", ds_cstr(&ds)); > > + ds_clear(&ds); > > if (!ret) { > > /* Report the first error value to the caller. */ > > ret = err; > > @@ -345,6 +386,7 @@ re_nl_sync_routes(uint32_t table_id, const struct > hmap *routes, > > > > hmapx_destroy(&routes_to_advertise); > > vector_destroy(&stale_routes); > > + ds_destroy(&ds); > > > > return ret; > > } > > diff --git a/controller/route-exchange-netlink.h > b/controller/route-exchange-netlink.h > > index 1741f761d..4ee513d40 100644 > > --- a/controller/route-exchange-netlink.h > > +++ b/controller/route-exchange-netlink.h > > @@ -52,10 +52,15 @@ struct re_nl_received_route_node { > > int re_nl_create_vrf(const char *ifname, uint32_t table_id); > > int re_nl_delete_vrf(const char *ifname); > > > > +void re_route_format(struct ds *, uint32_t table_id, > > + const struct in6_addr *dst, unsigned int plen, > > + const struct in6_addr *nexthop, int err); > > int re_nl_add_route(uint32_t table_id, const struct in6_addr *dst, > > - unsigned int plen, unsigned int priority); > > + unsigned int plen, const struct in6_addr *nexthop, > > + unsigned int priority); > > int re_nl_delete_route(uint32_t table_id, const struct in6_addr *dst, > > - unsigned int plen, unsigned int priority); > > + unsigned int plen, const struct in6_addr > *nexthop, > > + unsigned int priority); > > > > int re_nl_sync_routes(uint32_t table_id, const struct hmap *routes, > > struct vector *learned_routes, > > diff --git a/controller/route.c b/controller/route.c > > index 093306c5b..082a4630d 100644 > > --- a/controller/route.c > > +++ b/controller/route.c > > @@ -43,9 +43,11 @@ route_exchange_relevant_port(const struct > sbrec_port_binding *pb) > > } > > > > uint32_t > > -advertise_route_hash(const struct in6_addr *dst, unsigned int plen) > > +advertise_route_hash(const struct in6_addr *dst, > > + const struct in6_addr *nexthop, unsigned int plen) > > { > > - uint32_t hash = hash_bytes(dst->s6_addr, 16, 0); > > + uint32_t hash = hash_add_in6_addr(0, dst); > > + hash = hash_add_in6_addr(hash, nexthop); > > return hash_int(plen, hash); > > } > > > > @@ -318,11 +320,13 @@ route_run(struct route_ctx_in *r_ctx_in, > > } > > > > struct advertise_route_entry *ar = xmalloc(sizeof(*ar)); > > - ar->addr = prefix; > > - ar->plen = plen; > > - ar->priority = priority; > > + *ar = (struct advertise_route_entry) { > > + .addr = prefix, > > + .plen = plen, > > + .priority = priority, > > + }; > > hmap_insert(&ad->routes, &ar->node, > > - advertise_route_hash(&prefix, plen)); > > + advertise_route_hash(&ar->addr, &ar->nexthop, > plen)); > > } > > > > smap_destroy(&port_mapping); > > diff --git a/controller/route.h b/controller/route.h > > index 5c800f4ab..cf84a3079 100644 > > --- a/controller/route.h > > +++ b/controller/route.h > > @@ -79,6 +79,7 @@ struct advertise_datapath_entry { > > struct advertise_route_entry { > > struct hmap_node node; > > struct in6_addr addr; > > + struct in6_addr nexthop; > > unsigned int plen; > > unsigned int priority; > > }; > > @@ -88,7 +89,9 @@ const struct sbrec_port_binding > *route_exchange_find_port( > > const struct sbrec_chassis *chassis, > > const struct sbrec_port_binding *pb, > > const char **dynamic_routing_port_name); > > -uint32_t advertise_route_hash(const struct in6_addr *dst, unsigned int > plen); > > +uint32_t advertise_route_hash(const struct in6_addr *dst, > > + const struct in6_addr *nexthop, > > + unsigned int plen); > > void route_run(struct route_ctx_in *, struct route_ctx_out *); > > void route_cleanup(struct hmap *announce_routes); > > uint32_t route_get_table_id(const struct sbrec_datapath_binding *); > > diff --git a/tests/automake.mk b/tests/automake.mk > > index 18472d9cc..b037a3634 100644 > > --- a/tests/automake.mk > > +++ b/tests/automake.mk > > @@ -306,6 +306,8 @@ tests_ovstest_SOURCES += \ > > controller/neighbor-table-notify.h \ > > controller/neighbor.c \ > > controller/neighbor.h \ > > + controller/route-exchange-netlink.c \ > > + controller/route-exchange-netlink.h \ > > tests/test-ovn-netlink.c > > endif > > > > diff --git a/tests/system-ovn-netlink.at b/tests/system-ovn-netlink.at > > index 392973144..79988534f 100644 > > --- a/tests/system-ovn-netlink.at > > +++ b/tests/system-ovn-netlink.at > > @@ -299,3 +299,203 @@ AT_CHECK([ovstest test-ovn-netlink host-if-monitor > lo-test \ > > 0 > > ]) > > AT_CLEANUP > > + > > +AT_SETUP([sync netlink routes - Blackhole]) > > +AT_KEYWORDS([netlink-routes]) > > + > > +table_id=100 > > + > > +check ip link add vrf-$table_id type vrf table $table_id > > +on_exit 'ip link del vrf-$table_id' > > +check ip link set dev vrf-$table_id up > > + > > +check ip link add lo-test type dummy > > +on_exit 'ip link del lo-test' > > +check ip link set lo-test master vrf-$table_id > > +check ip link set lo-test address 00:00:00:00:00:10 > > +check ip addr add 20.0.0.10/24 dev lo-test > > +check ip addr add fd20::10/64 dev lo-test > > +check ip link set up lo-test > > + > > +OVS_WAIT_FOR_OUTPUT_UNQUOTED([ovstest test-ovn-netlink route-sync \ > > + $table_id | sort], [0], [dnl > > +]) > > + > > +check ip route add 10.10.10.1 via 20.0.0.1 vrf vrf-$table_id proto zebra > > +check ip route add fd20:100::10 via fd20::1 vrf vrf-$table_id proto > zebra > > +check ip route add 10.10.20.0/24 via 20.0.0.1 vrf vrf-$table_id > > + > > +AS_BOX([No advertisement]) > > +OVS_WAIT_FOR_OUTPUT_UNQUOTED([ovstest test-ovn-netlink route-sync \ > > + $table_id | sort], [0], [dnl > > +Route table_id=$table_id dst=10.10.10.1 plen=32 nexthop=20.0.0.1 > > +Route table_id=$table_id dst=fd20:100::10 plen=128 nexthop=fd20::1 > > +]) > > + > > +OVN_ROUTE_EQUAL([vrf-$table_id], [dnl > > +10.10.10.1 via 20.0.0.1 dev lo-test proto zebra > > +10.10.20.0/24 via 20.0.0.1 dev lo-test > > +20.0.0.0/24 dev lo-test proto kernel scope link src 20.0.0.10]) > > + > > +OVN_ROUTE_V6_EQUAL([vrf-$table_id], [dnl > > +fd20::/64 dev lo-test proto kernel metric 256 pref medium > > +fd20:100::10 via fd20::1 dev lo-test proto zebra metric 1024 pref medium > > +fe80::/64 dev lo-test proto kernel metric 256 pref medium > > +multicast ff00::/8 dev lo-test proto kernel metric 256 pref medium]) > > + > > +AS_BOX([Advertise just IPv4]) > > +OVS_WAIT_FOR_OUTPUT_UNQUOTED([ovstest test-ovn-netlink route-sync \ > > + $table_id 192.168.100.0/24 | sort], [0], [dnl > > +Route table_id=$table_id dst=10.10.10.1 plen=32 nexthop=20.0.0.1 > > +Route table_id=$table_id dst=fd20:100::10 plen=128 nexthop=fd20::1 > > +]) > > + > > +OVN_ROUTE_EQUAL([vrf-$table_id], [dnl > > +10.10.10.1 via 20.0.0.1 dev lo-test proto zebra > > +10.10.20.0/24 via 20.0.0.1 dev lo-test > > +20.0.0.0/24 dev lo-test proto kernel scope link src 20.0.0.10 > > +blackhole 192.168.100.0/24 proto ovn]) > > + > > +OVN_ROUTE_V6_EQUAL([vrf-$table_id], [dnl > > +fd20::/64 dev lo-test proto kernel metric 256 pref medium > > +fd20:100::10 via fd20::1 dev lo-test proto zebra metric 1024 pref medium > > +fe80::/64 dev lo-test proto kernel metric 256 pref medium > > +multicast ff00::/8 dev lo-test proto kernel metric 256 pref medium]) > > + > > +AS_BOX([Advertise both IPv4 and IPv6]) > > +OVS_WAIT_FOR_OUTPUT_UNQUOTED([ovstest test-ovn-netlink route-sync \ > > + $table_id 192.168.100.0/24 fd20:100::/64 | sort], [0], [dnl > > +Route table_id=$table_id dst=10.10.10.1 plen=32 nexthop=20.0.0.1 > > +Route table_id=$table_id dst=fd20:100::10 plen=128 nexthop=fd20::1 > > +]) > > + > > +OVN_ROUTE_EQUAL([vrf-$table_id], [dnl > > +10.10.10.1 via 20.0.0.1 dev lo-test proto zebra > > +10.10.20.0/24 via 20.0.0.1 dev lo-test > > +20.0.0.0/24 dev lo-test proto kernel scope link src 20.0.0.10 > > +blackhole 192.168.100.0/24 proto ovn]) > > + > > +OVN_ROUTE_V6_EQUAL([vrf-$table_id], [dnl > > +fd20::/64 dev lo-test proto kernel metric 256 pref medium > > +fd20:100::10 via fd20::1 dev lo-test proto zebra metric 1024 pref medium > > +blackhole fd20:100::/64 dev lo proto ovn metric 1024 pref medium > > +fe80::/64 dev lo-test proto kernel metric 256 pref medium > > +multicast ff00::/8 dev lo-test proto kernel metric 256 pref medium]) > > + > > +AS_BOX([Advertise just IPv4, should remove the IPv6]) > > +OVS_WAIT_FOR_OUTPUT_UNQUOTED([ovstest test-ovn-netlink route-sync \ > > + $table_id 192.168.100.0/24 | sort], [0], [dnl > > +Route table_id=$table_id dst=10.10.10.1 plen=32 nexthop=20.0.0.1 > > +Route table_id=$table_id dst=fd20:100::10 plen=128 nexthop=fd20::1 > > +]) > > + > > +OVN_ROUTE_EQUAL([vrf-$table_id], [dnl > > +10.10.10.1 via 20.0.0.1 dev lo-test proto zebra > > +10.10.20.0/24 via 20.0.0.1 dev lo-test > > +20.0.0.0/24 dev lo-test proto kernel scope link src 20.0.0.10 > > +blackhole 192.168.100.0/24 proto ovn]) > > + > > +OVN_ROUTE_V6_EQUAL([vrf-$table_id], [dnl > > +fd20::/64 dev lo-test proto kernel metric 256 pref medium > > +fd20:100::10 via fd20::1 dev lo-test proto zebra metric 1024 pref medium > > +fe80::/64 dev lo-test proto kernel metric 256 pref medium > > +multicast ff00::/8 dev lo-test proto kernel metric 256 pref medium]) > > + > > +AS_BOX([No advertisement should remove IPv4]) > > +OVS_WAIT_FOR_OUTPUT_UNQUOTED([ovstest test-ovn-netlink route-sync \ > > + $table_id | sort], [0], [dnl > > +Route table_id=$table_id dst=10.10.10.1 plen=32 nexthop=20.0.0.1 > > +Route table_id=$table_id dst=fd20:100::10 plen=128 nexthop=fd20::1 > > +]) > > + > > +OVN_ROUTE_EQUAL([vrf-$table_id], [dnl > > +10.10.10.1 via 20.0.0.1 dev lo-test proto zebra > > +10.10.20.0/24 via 20.0.0.1 dev lo-test > > +20.0.0.0/24 dev lo-test proto kernel scope link src 20.0.0.10]) > > + > > +OVN_ROUTE_V6_EQUAL([vrf-$table_id], [dnl > > +fd20::/64 dev lo-test proto kernel metric 256 pref medium > > +fd20:100::10 via fd20::1 dev lo-test proto zebra metric 1024 pref medium > > +fe80::/64 dev lo-test proto kernel metric 256 pref medium > > +multicast ff00::/8 dev lo-test proto kernel metric 256 pref medium]) > > + > > +AT_CLEANUP > > + > > +AT_SETUP([sync netlink routes - Nexthop]) > > +AT_KEYWORDS([netlink-routes]) > > + > > +table_id=100 > > + > > +check ip link add vrf-$table_id type vrf table $table_id > > +on_exit 'ip link del vrf-$table_id' > > +check ip link set dev vrf-$table_id up > > + > > +check ip link add lo-test type dummy > > +on_exit 'ip link del lo-test' > > +check ip link set lo-test master vrf-$table_id > > +check ip link set lo-test address 00:00:00:00:00:10 > > +check ip addr add 20.0.0.10/24 dev lo-test > > +check ip addr add fd20::10/64 dev lo-test > > +check ip link set up lo-test > > + > > +check ovstest test-ovn-netlink route-sync $table_id > > + > > +AS_BOX([Advertise IPv4 via IPv4]) > > +check ovstest test-ovn-netlink route-sync $table_id \ > > + 192.168.100.0/24 via 20.0.0.1 > > + > > +OVN_ROUTE_EQUAL([vrf-$table_id], [dnl > > +20.0.0.0/24 dev lo-test proto kernel scope link src 20.0.0.10 > > +192.168.100.0/24 via 20.0.0.1 dev lo-test proto ovn]) > > + > > +OVN_ROUTE_V6_EQUAL([vrf-$table_id], [dnl > > +fd20::/64 dev lo-test proto kernel metric 256 pref medium > > +fe80::/64 dev lo-test proto kernel metric 256 pref medium > > +multicast ff00::/8 dev lo-test proto kernel metric 256 pref medium]) > > + > > +AS_BOX([Advertise IPv4 via IPv4 and IPv6 via IPv6]) > > +check ovstest test-ovn-netlink route-sync $table_id \ > > + 192.168.100.0/24 via 20.0.0.1 \ > > + fd20:100::/64 via fd20::1 > > + > > +OVN_ROUTE_EQUAL([vrf-$table_id], [dnl > > +20.0.0.0/24 dev lo-test proto kernel scope link src 20.0.0.10 > > +192.168.100.0/24 via 20.0.0.1 dev lo-test proto ovn]) > > + > > +OVN_ROUTE_V6_EQUAL([vrf-$table_id], [dnl > > +fd20::/64 dev lo-test proto kernel metric 256 pref medium > > +fd20:100::/64 via fd20::1 dev lo-test proto ovn metric 1024 pref medium > > +fe80::/64 dev lo-test proto kernel metric 256 pref medium > > +multicast ff00::/8 dev lo-test proto kernel metric 256 pref medium]) > > + > > +AS_BOX([Advertise IPv4 via IPv6 and IPv6 via IPv6]) > > +check ovstest test-ovn-netlink route-sync $table_id \ > > + 192.168.100.0/24 via fd20::1 \ > > + fd20:100::/64 via fd20::1 > > + > > +OVN_ROUTE_EQUAL([vrf-$table_id], [dnl > > +20.0.0.0/24 dev lo-test proto kernel scope link src 20.0.0.10 > > +192.168.100.0/24 via inet6 fd20::1 dev lo-test proto ovn]) > > + > > +OVN_ROUTE_V6_EQUAL([vrf-$table_id], [dnl > > +fd20::/64 dev lo-test proto kernel metric 256 pref medium > > +fd20:100::/64 via fd20::1 dev lo-test proto ovn metric 1024 pref medium > > +fe80::/64 dev lo-test proto kernel metric 256 pref medium > > +multicast ff00::/8 dev lo-test proto kernel metric 256 pref medium]) > > + > > +AS_BOX([Replace both with blackhole]) > > +check ovstest test-ovn-netlink route-sync $table_id \ > > + 192.168.100.0/24 \ > > + fd20:100::/64 > > + > > +OVN_ROUTE_EQUAL([vrf-$table_id], [dnl > > +20.0.0.0/24 dev lo-test proto kernel scope link src 20.0.0.10 > > +blackhole 192.168.100.0/24 proto ovn]) > > + > > +OVN_ROUTE_V6_EQUAL([vrf-$table_id], [dnl > > +fd20::/64 dev lo-test proto kernel metric 256 pref medium > > +blackhole fd20:100::/64 dev lo proto ovn metric 1024 pref medium > > +fe80::/64 dev lo-test proto kernel metric 256 pref medium > > +multicast ff00::/8 dev lo-test proto kernel metric 256 pref medium]) > > + > > +AT_CLEANUP > > diff --git a/tests/test-ovn-netlink.c b/tests/test-ovn-netlink.c > > index 84e24d0dd..701692fa6 100644 > > --- a/tests/test-ovn-netlink.c > > +++ b/tests/test-ovn-netlink.c > > @@ -16,6 +16,7 @@ > > #include <config.h> > > > > #include "openvswitch/hmap.h" > > +#include "route-table.h" > > #include "packets.h" > > #include "tests/ovstest.h" > > #include "tests/test-utils.h" > > @@ -24,6 +25,8 @@ > > #include "controller/neighbor-exchange-netlink.h" > > #include "controller/neighbor-table-notify.h" > > #include "controller/neighbor.h" > > +#include "controller/route.h" > > +#include "controller/route-exchange-netlink.h" > > > > static void > > test_neighbor_sync(struct ovs_cmdl_context *ctx) > > @@ -177,6 +180,70 @@ test_host_if_monitor(struct ovs_cmdl_context *ctx) > > sset_destroy(&if_names); > > } > > > > +static void > > +test_route_sync(struct ovs_cmdl_context *ctx) > > +{ > > + struct advertise_route_entry *e; > > + unsigned int shift = 1; > > + > > + unsigned int table_id; > > + if (!test_read_uint_value(ctx, shift++, "table id", &table_id)) { > > + return; > > + } > > + > > + struct hmap routes_to_advertise = > HMAP_INITIALIZER(&routes_to_advertise); > > + struct vector received_routes = > > + VECTOR_EMPTY_INITIALIZER(struct re_nl_received_route_node); > > + > > + while (shift < ctx->argc) { > > + struct advertise_route_entry *ar = xzalloc(sizeof *ar); > > + if (!test_read_ipv6_cidr_mapped_value(ctx, shift++, "IP > address", > > + &ar->addr, &ar->plen)) { > > + free(ar); > > + goto done; > > + } > > + > > + /* Check if we are adding only blackhole route. */ > > + if (shift + 1 < ctx->argc) { > > + const char *via = test_read_value(ctx, shift++, "via"); > > + if (strcmp(via, "via")) { > > + shift--; > > + continue; > > + } > > + > > + if (!test_read_ipv6_mapped_value(ctx, shift++, "IP address", > > + &ar->nexthop)) { > > + free(ar); > > + goto done; > > + } > > + } > > + hmap_insert(&routes_to_advertise, &ar->node, > > + advertise_route_hash(&ar->addr, &ar->nexthop, > ar->plen)); > > + } > > + > > + ovs_assert(re_nl_sync_routes(table_id, &routes_to_advertise, > > + &received_routes, NULL) == 0); > > + > > + > > Nit: too many empty lines. One is probably enough. > > > + struct ds msg = DS_EMPTY_INITIALIZER; > > + > > + struct re_nl_received_route_node *rr; > > + VECTOR_FOR_EACH_PTR (&received_routes, rr) { > > + re_route_format(&msg, table_id, &rr->prefix, > > + rr->plen, &rr->nexthop, 0); > > + printf("Route %s\n", ds_cstr(&msg)); > > + ds_clear(&msg); > > + } > > + > > +done: > > + HMAP_FOR_EACH_POP (e, node, &routes_to_advertise) { > > + free(e); > > + } > > + hmap_destroy(&routes_to_advertise); > > + vector_destroy(&received_routes); > > + ds_destroy(&msg); > > +} > > + > > static void > > test_ovn_netlink(int argc, char *argv[]) > > { > > @@ -186,6 +253,7 @@ test_ovn_netlink(int argc, char *argv[]) > > {"neighbor-table-notify", NULL, 3, 4, > > test_neighbor_table_notify, OVS_RO}, > > {"host-if-monitor", NULL, 2, 3, test_host_if_monitor, OVS_RO}, > > + {"route-sync", NULL, 1, INT_MAX, test_route_sync, OVS_RO}, > > {NULL, NULL, 0, 0, NULL, OVS_RO}, > > }; > > struct ovs_cmdl_context ctx; > > diff --git a/tests/test-utils.c b/tests/test-utils.c > > index e55557066..8edd521d0 100644 > > --- a/tests/test-utils.c > > +++ b/tests/test-utils.c > > @@ -114,3 +114,21 @@ test_read_ipv6_mapped_value(struct ovs_cmdl_context > *ctx, unsigned int index, > > } > > return true; > > } > > + > > +bool > > +test_read_ipv6_cidr_mapped_value(struct ovs_cmdl_context *ctx, > > + unsigned int index, const char *descr, > > + struct in6_addr *result, unsigned int > *plen) > > +{ > > + if (index >= ctx->argc) { > > + fprintf(stderr, "Missing %s argument\n", descr); > > + return false; > > + } > > + > > + const char *arg = ctx->argv[index]; > > + if (!ip46_parse_cidr(arg, result, plen)) { > > + fprintf(stderr, "Invalid %s: %s\n", descr, arg); > > + return false; > > + } > > + return true; > > +} > > diff --git a/tests/test-utils.h b/tests/test-utils.h > > index fef67e799..e8176ce44 100644 > > --- a/tests/test-utils.h > > +++ b/tests/test-utils.h > > @@ -35,4 +35,8 @@ bool test_read_eth_addr_value(struct ovs_cmdl_context > *ctx, unsigned int index, > > bool test_read_ipv6_mapped_value(struct ovs_cmdl_context *ctx, > > unsigned int index, const char *descr, > > struct in6_addr *result); > > +bool test_read_ipv6_cidr_mapped_value(struct ovs_cmdl_context *ctx, > > + unsigned int index, const char > *descr, > > + struct in6_addr *result, > > + unsigned int *plen); > > #endif /* tests/test-utils.h */ > > With the small issues I mentioned above addressed the patch looks good > to me. Feel free to add my ack if you're just changing what I listed > above: > > Acked-by: Dumitru Ceara <[email protected]> > > Regards, > Dumitru > > Thank you Dumitru, I have discovered an issue in the process of making those changes, I will post v3 with the fix. Regards, Ales _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
