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
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
