On 7 Feb 2023, at 7:48, Nobuhiro MIKI wrote:
> We can use the "ip route add ... src ..." command to set the preferred
> source address for each entry in the kernel FIB. OVS has a mechanism to
> cache the FIB, but the preferred source address is ignored and
> calculated with its own logic. This patch resolves the difference
> between kernel FIB and OVS route table cache by retrieving the
> RTA_PREFSRC attribute of Netlink messages.
Thanks for the patch, one small nit inline, the rest looks good.
Cheers,
Eelco
> Signed-off-by: Nobuhiro MIKI <nm...@yahoo-corp.jp>
> ---
> lib/ovs-router.c | 6 +++---
> lib/ovs-router.h | 3 ++-
> lib/route-table.c | 16 +++++++++++++++-
> tests/system-route.at | 39 +++++++++++++++++++++++++++++++++++++++
> 4 files changed, 59 insertions(+), 5 deletions(-)
>
> diff --git a/lib/ovs-router.c b/lib/ovs-router.c
> index fc95a91012f9..160f6fb085bd 100644
> --- a/lib/ovs-router.c
> +++ b/lib/ovs-router.c
> @@ -276,13 +276,13 @@ ovs_router_insert__(uint32_t mark, uint8_t priority,
> bool local,
>
> void
> ovs_router_insert(uint32_t mark, const struct in6_addr *ip_dst, uint8_t plen,
> - bool local, const char output_bridge[],
> - const struct in6_addr *gw)
> + bool local, const char output_bridge[],
> + const struct in6_addr *gw, const struct in6_addr *prefsrc)
> {
> if (use_system_routing_table) {
> uint8_t priority = local ? plen + 64 : plen;
> ovs_router_insert__(mark, priority, local, ip_dst, plen,
> - output_bridge, gw, &in6addr_any);
> + output_bridge, gw, prefsrc);
> }
> }
>
> diff --git a/lib/ovs-router.h b/lib/ovs-router.h
> index d8ce3c00ded5..eb4ff85d9e63 100644
> --- a/lib/ovs-router.h
> +++ b/lib/ovs-router.h
> @@ -32,7 +32,8 @@ bool ovs_router_lookup(uint32_t mark, const struct in6_addr
> *ip_dst,
> void ovs_router_init(void);
> void ovs_router_insert(uint32_t mark, const struct in6_addr *ip_dst,
> uint8_t plen, bool local,
> - const char output_bridge[], const struct in6_addr
> *gw);
> + const char output_bridge[], const struct in6_addr *gw,
> + const struct in6_addr *prefsrc);
> void ovs_router_flush(void);
>
> void ovs_router_disable_system_routing_table(void);
> diff --git a/lib/route-table.c b/lib/route-table.c
> index ac82cf262f86..bb030c8ef66c 100644
> --- a/lib/route-table.c
> +++ b/lib/route-table.c
> @@ -51,6 +51,7 @@ struct route_data {
>
> /* Extracted from Netlink attributes. */
> struct in6_addr rta_dst; /* 0 if missing. */
> + struct in6_addr rta_prefsrc; /* 0 if missing. */
> struct in6_addr rta_gw;
> char ifname[IFNAMSIZ]; /* Interface name. */
> uint32_t mark;
> @@ -201,6 +202,7 @@ route_table_parse(struct ofpbuf *buf, struct
> route_table_msg *change)
> [RTA_OIF] = { .type = NL_A_U32, .optional = true },
> [RTA_GATEWAY] = { .type = NL_A_U32, .optional = true },
> [RTA_MARK] = { .type = NL_A_U32, .optional = true },
> + [RTA_PREFSRC] = { .type = NL_A_U32, .optional = true },
You have a double space after true and before }.
> };
>
> static const struct nl_policy policy6[] = {
> @@ -208,6 +210,7 @@ route_table_parse(struct ofpbuf *buf, struct
> route_table_msg *change)
> [RTA_OIF] = { .type = NL_A_U32, .optional = true },
> [RTA_MARK] = { .type = NL_A_U32, .optional = true },
> [RTA_GATEWAY] = { .type = NL_A_IPV6, .optional = true },
> + [RTA_PREFSRC] = { .type = NL_A_IPV6, .optional = true },
> };
>
> struct nlattr *attrs[ARRAY_SIZE(policy)];
> @@ -274,6 +277,16 @@ route_table_parse(struct ofpbuf *buf, struct
> route_table_msg *change)
> } else if (ipv4) {
> in6_addr_set_mapped_ipv4(&change->rd.rta_dst, 0);
> }
> + if (attrs[RTA_PREFSRC]) {
> + if (ipv4) {
> + ovs_be32 prefsrc;
> + prefsrc = nl_attr_get_be32(attrs[RTA_PREFSRC]);
> + in6_addr_set_mapped_ipv4(&change->rd.rta_prefsrc, prefsrc);
> + } else {
> + change->rd.rta_prefsrc =
> + nl_attr_get_in6_addr(attrs[RTA_PREFSRC]);
> + }
> + }
> if (attrs[RTA_GATEWAY]) {
> if (ipv4) {
> ovs_be32 gw;
> @@ -309,7 +322,8 @@ route_table_handle_msg(const struct route_table_msg
> *change)
> const struct route_data *rd = &change->rd;
>
> ovs_router_insert(rd->mark, &rd->rta_dst, rd->rtm_dst_len,
> - rd->local, rd->ifname, &rd->rta_gw);
> + rd->local, rd->ifname, &rd->rta_gw,
> + &rd->rta_prefsrc);
> }
> }
>
> diff --git a/tests/system-route.at b/tests/system-route.at
> index 270956d13f6f..114aaebc77f3 100644
> --- a/tests/system-route.at
> +++ b/tests/system-route.at
> @@ -25,3 +25,42 @@ OVS_WAIT_UNTIL([test `ovs-appctl ovs/route/show | grep -c
> 'p1-route'` -eq 0 ])
>
> OVS_TRAFFIC_VSWITCHD_STOP
> AT_CLEANUP
> +
> +AT_SETUP([ovs-route - add system route with src - ipv4])
> +AT_KEYWORDS([route])
> +OVS_TRAFFIC_VSWITCHD_START()
> +AT_CHECK([ip link set br0 up])
> +
> +AT_CHECK([ip addr add 192.168.9.2/24 dev br0], [0], [stdout])
> +AT_CHECK([ip addr add 192.168.9.3/24 dev br0], [0], [stdout])
> +
> +AT_CHECK([ip route add 192.168.10.12/32 dev br0 via 192.168.9.1 src
> 192.168.9.2], [0], [stdout])
> +AT_CHECK([ip route add 192.168.10.13/32 dev br0 via 192.168.9.1 src
> 192.168.9.3], [0], [stdout])
> +
> +OVS_WAIT_UNTIL_EQUAL([ovs-appctl ovs/route/show | grep -E
> '192.168.10.1[[23]]/32' | sort], [dnl
> +Cached: 192.168.10.12/32 dev br0 GW 192.168.9.1 SRC 192.168.9.2
> +Cached: 192.168.10.13/32 dev br0 GW 192.168.9.1 SRC 192.168.9.3])
> +
> +OVS_TRAFFIC_VSWITCHD_STOP
> +AT_CLEANUP
> +
> +AT_SETUP([ovs-route - add system route with src - ipv6])
> +AT_KEYWORDS([route])
> +OVS_TRAFFIC_VSWITCHD_START()
> +AT_CHECK([ip link set br0 up])
> +
> +AT_CHECK([ip -6 addr add fc00:db8:cafe::2/64 dev br0], [0], [stdout])
> +AT_CHECK([ip -6 addr add fc00:db8:cafe::3/64 dev br0], [0], [stdout])
> +
> +dnl If we try to add a route immediately after assigning ipv6 addresses,
> +dnl iproute2 would give us "Invalid source address" error,
> +dnl so wait a while to succeed.
> +OVS_WAIT_UNTIL([ip -6 route add fc00:db8:beef::12/128 via fc00:db8:cafe::1
> dev br0 src fc00:db8:cafe::3])
> +OVS_WAIT_UNTIL([ip -6 route add fc00:db8:beef::13/128 via fc00:db8:cafe::1
> dev br0 src fc00:db8:cafe::2])
> +
> +OVS_WAIT_UNTIL_EQUAL([ovs-appctl ovs/route/show | grep -E
> 'fc00:db8:beef::1[[23]]/128' | sort], [dnl
> +Cached: fc00:db8:beef::12/128 dev br0 GW fc00:db8:cafe::1 SRC
> fc00:db8:cafe::3
> +Cached: fc00:db8:beef::13/128 dev br0 GW fc00:db8:cafe::1 SRC
> fc00:db8:cafe::2])
> +
> +OVS_TRAFFIC_VSWITCHD_STOP
> +AT_CLEANUP
> --
> 2.31.1
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
