On 22 Feb 2023, at 11:29, Nobuhiro MIKI wrote:
> When adding a route with ovs/route/add command, the source address > in "ovs_router_entry" structure is always the FIRST address that the > interface has. See "ovs_router_get_netdev_source_address" > function for more information. > > If an interface has multiple ipv4 and/or ipv6 addresses, there are use > cases where the user wants to control the source address. This patch > therefore addresses this issue by adding a src parameter. > > Note that same constraints also exist when caching routes from > Kernel FIB with Netlink, but are not dealt with in this patch. > > Signed-off-by: Nobuhiro MIKI <nm...@yahoo-corp.jp> Changes look good, two small nits below. Acked-by: Eelco Chaudron <echau...@redhat.com> > --- > NEWS | 3 ++ > lib/ovs-router.c | 83 +++++++++++++++++++++++++++++---- > ofproto/ofproto-tnl-unixctl.man | 5 +- > tests/ovs-router.at | 78 +++++++++++++++++++++++++++++++ > 4 files changed, 158 insertions(+), 11 deletions(-) > > diff --git a/NEWS b/NEWS > index 85b34962145e..d062ca4fac1a 100644 > --- a/NEWS > +++ b/NEWS > @@ -10,6 +10,9 @@ Post-v3.1.0 > in order to create OVSDB sockets with access mode of 0770. > - QoS: > * Added new configuration option 'jitter' for a linux-netem QoS type. > + - ovs-appctl: > + * Add support for selecting the source address with the > + “ovs-appctl ovs/route/add" command. > > > v3.1.0 - 16 Feb 2023 > diff --git a/lib/ovs-router.c b/lib/ovs-router.c > index 6c5faf46ea15..dca1a406cb44 100644 > --- a/lib/ovs-router.c > +++ b/lib/ovs-router.c > @@ -164,6 +164,46 @@ static void rt_init_match(struct match *match, uint32_t > mark, > match->flow.pkt_mark = mark; > } > > +static int > +verify_prefsrc(const struct in6_addr *ip6_dst, > + const char output_bridge[], > + struct in6_addr *prefsrc) > +{ > + struct in6_addr *mask, *addr6; > + struct netdev *dev; > + int err, n_in6, i; > + > + err = netdev_open(output_bridge, NULL, &dev); > + if (err) { > + return err; > + } > + > + err = netdev_get_addr_list(dev, &addr6, &mask, &n_in6); > + if (err) { > + goto out; > + } > + > + for (i = 0; i < n_in6; i++) { > + struct in6_addr a1, a2; > + a1 = ipv6_addr_bitand(ip6_dst, &mask[i]); > + a2 = ipv6_addr_bitand(prefsrc, &mask[i]); > + > + /* Check that the intarface has "prefsrc" and > + * it is same broadcast domain with "ip6_dst". */ > + if (IN6_ARE_ADDR_EQUAL(prefsrc, &addr6[i]) && > + IN6_ARE_ADDR_EQUAL(&a1, &a2)) { > + goto out; > + } > + } > + err = ENOENT; > + > +out: > + free(addr6); > + free(mask); > + netdev_close(dev); > + return err; > +} > + > int > ovs_router_get_netdev_source_address(const struct in6_addr *ip6_dst, > const char output_bridge[], > @@ -217,8 +257,12 @@ static int > ovs_router_insert__(uint32_t mark, uint8_t priority, bool local, > const struct in6_addr *ip6_dst, > uint8_t plen, const char output_bridge[], > - const struct in6_addr *gw) > + const struct in6_addr *gw, > + const struct in6_addr *ip6_src) > { > + int (*f)(const struct in6_addr *ip6_dst, Give it a more meaningful name than f, i.e. get_src_addr()? > + const char output_bridge[], > + struct in6_addr *prefsrc); > const struct cls_rule *cr; > struct ovs_router_entry *p; > struct match match; > @@ -236,11 +280,17 @@ ovs_router_insert__(uint32_t mark, uint8_t priority, > bool local, > p->plen = plen; > p->local = local; > p->priority = priority; > - err = ovs_router_get_netdev_source_address(ip6_dst, output_bridge, > - &p->src_addr); > + > + if (ipv6_addr_is_set(ip6_src)) { > + p->src_addr = *ip6_src; > + f = verify_prefsrc; > + } else { > + f = ovs_router_get_netdev_source_address; > + } > + > + err = f(ip6_dst, output_bridge, &p->src_addr); > if (err && ipv6_addr_is_set(gw)) { > - err = ovs_router_get_netdev_source_address(gw, output_bridge, > - &p->src_addr); > + err = f(gw, output_bridge, &p->src_addr); > } > if (err) { > struct ds ds = DS_EMPTY_INITIALIZER; > @@ -274,7 +324,8 @@ ovs_router_insert(uint32_t mark, const struct in6_addr > *ip_dst, uint8_t plen, > { > if (use_system_routing_table) { > uint8_t priority = local ? plen + 64 : plen; > - ovs_router_insert__(mark, priority, local, ip_dst, plen, > output_bridge, gw); > + ovs_router_insert__(mark, priority, local, ip_dst, plen, > + output_bridge, gw, &in6addr_any); > } > } > > @@ -341,10 +392,13 @@ static void > ovs_router_add(struct unixctl_conn *conn, int argc, > const char *argv[], void *aux OVS_UNUSED) > { > + struct in6_addr src6 = in6addr_any; > struct in6_addr gw6 = in6addr_any; > + char src6_s[IPV6_SCAN_LEN + 1]; > struct in6_addr ip6; > uint32_t mark = 0; > unsigned int plen; > + ovs_be32 src = 0; > ovs_be32 gw = 0; > bool is_ipv6; > ovs_be32 ip; > @@ -369,10 +423,17 @@ ovs_router_add(struct unixctl_conn *conn, int argc, > } > > if (is_ipv6) { > + if (ovs_scan(argv[i], "src="IPV6_SCAN_FMT, src6_s) && > + ipv6_parse(src6_s, &src6)) { > + continue; > + } > if (ipv6_parse(argv[i], &gw6)) { > continue; > } > } else { > + if (ovs_scan(argv[i], "src="IP_SCAN_FMT, IP_SCAN_ARGS(&src))) { > + continue; > + } See comment in other patch about updating error message, now it should probably also include src :) > if (ip_parse(argv[i], &gw)) { > continue; > } > @@ -385,8 +446,12 @@ ovs_router_add(struct unixctl_conn *conn, int argc, > if (gw) { > in6_addr_set_mapped_ipv4(&gw6, gw); > } > + if (src) { > + in6_addr_set_mapped_ipv4(&src6, src); > + } > > - err = ovs_router_insert__(mark, plen + 32, false, &ip6, plen, argv[2], > &gw6); > + err = ovs_router_insert__(mark, plen + 32, false, &ip6, plen, argv[2], > + &gw6, &src6); > if (err) { > unixctl_command_reply_error(conn, "Error while inserting route."); > } else { > @@ -537,8 +602,8 @@ ovs_router_init(void) > classifier_init(&cls, NULL); > unixctl_command_register("ovs/route/add", > "ip_addr/prefix_len out_br_name [gw] " > - "[pkt_mark=mark]", > - 2, 4, ovs_router_add, NULL); > + "[pkt_mark=mark] [src=src_ip_addr]", > + 2, 5, ovs_router_add, NULL); > unixctl_command_register("ovs/route/show", "", 0, 0, > ovs_router_show, NULL); > unixctl_command_register("ovs/route/del", "ip_addr/prefix_len " > diff --git a/ofproto/ofproto-tnl-unixctl.man b/ofproto/ofproto-tnl-unixctl.man > index 0c0a392820fb..86f5e759a2d4 100644 > --- a/ofproto/ofproto-tnl-unixctl.man > +++ b/ofproto/ofproto-tnl-unixctl.man > @@ -1,8 +1,9 @@ > .SS "OPENVSWITCH TUNNELING COMMANDS" > These commands query and modify OVS tunnel components. > . > -.IP "\fBovs/route/add ip/plen output_bridge [GW]\fR" > -Adds ip/plen route to vswitchd routing table. output_bridge > +.IP "\fBovs/route/add \fIip\fB/\fIplen\fB \fIoutput_bridge\fB \ > +[\fIgateway\fB] [pkt_mark=\fImark\fB] [src=\fIsrc_ip\fB]\fR" > +Adds \fIip\fR/\fIplen\fR route to vswitchd routing table. \fIoutput_bridge\fR > needs to be OVS bridge name. This command is useful if OVS cached > routes does not look right. > . > diff --git a/tests/ovs-router.at b/tests/ovs-router.at > index 96fb6e188eef..a3083a67fb8d 100644 > --- a/tests/ovs-router.at > +++ b/tests/ovs-router.at > @@ -31,6 +31,84 @@ User: 2.2.2.3/32 MARK 1 dev br0 SRC 2.2.2.2 > OVS_VSWITCHD_STOP > AT_CLEANUP > > +AT_SETUP([appctl - route/add with src - ipv4]) > +AT_KEYWORDS([ovs_router]) > +OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=dummy]) > +AT_CHECK([ovs-appctl netdev-dummy/ip4addr br0 192.168.9.2/24], [0], [OK > +]) > +AT_CHECK([ovs-appctl netdev-dummy/ip4addr br0 192.168.9.3/24], [0], [OK > +]) > +AT_CHECK([ovs-appctl ovs/route/add 192.168.9.11/32 br0 src=192.168.9.3], > [0], [OK > +]) > +AT_CHECK([ovs-appctl ovs/route/add 192.168.10.12/32 br0 192.168.9.1 > src=192.168.9.3], [0], [OK > +]) > +AT_CHECK([ovs-appctl ovs/route/add 192.168.10.13/32 br0 192.168.9.1 > pkt_mark=13 src=192.168.9.3], [0], [OK > +]) > +AT_CHECK([ovs-appctl ovs/route/add 192.168.10.14/32 br0 192.168.9.1 > pkt_mark=14 src=192.168.9.2], [0], [OK > +]) > +AT_CHECK([ovs-appctl ovs/route/add 192.168.10.15/32 br0 192.168.9.1 > src=foo.bar.9.200], [2], [], [dnl > +Invalid parameters > +ovs-appctl: ovs-vswitchd: server returned an error > +]) > +AT_CHECK([ovs-appctl ovs/route/add 192.168.10.16/32 br0 192.168.9.1 > src=192.168.9.200], [2], [], [dnl > +Error while inserting route. > +ovs-appctl: ovs-vswitchd: server returned an error > +]) > +AT_CHECK([ovs-appctl ovs/route/add 192.168.10.17/32 br0 192.168.11.1 > src=192.168.9.3], [2], [], [dnl > +Error while inserting route. > +ovs-appctl: ovs-vswitchd: server returned an error > +]) > +AT_CHECK([ovs-appctl ovs/route/add 192.168.10.18/32 br0 src=192.168.9.3], > [2], [], [dnl > +Error while inserting route. > +ovs-appctl: ovs-vswitchd: server returned an error > +]) > +AT_CHECK([ovs-appctl ovs/route/show | grep User | grep 192.168.10 | sort], > [0], [dnl > +User: 192.168.10.12/32 dev br0 GW 192.168.9.1 SRC 192.168.9.3 > +User: 192.168.10.13/32 MARK 13 dev br0 GW 192.168.9.1 SRC 192.168.9.3 > +User: 192.168.10.14/32 MARK 14 dev br0 GW 192.168.9.1 SRC 192.168.9.2 > +]) > +OVS_VSWITCHD_STOP > +AT_CLEANUP > + > +AT_SETUP([appctl - route/add with src - ipv6]) > +AT_KEYWORDS([ovs_router]) > +OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=dummy]) > +AT_CHECK([ovs-appctl netdev-dummy/ip6addr br0 2001:db8:cafe::2/64], [0], [OK > +]) > +AT_CHECK([ovs-appctl netdev-dummy/ip6addr br0 2001:db8:cafe::3/64], [0], [OK > +]) > +AT_CHECK([ovs-appctl ovs/route/add 2001:db8:cafe::11/128 br0 > src=2001:db8:cafe::3], [0], [OK > +]) > +AT_CHECK([ovs-appctl ovs/route/add 2001:db8:beef::12/128 br0 > 2001:db8:cafe::1 src=2001:db8:cafe::3], [0], [OK > +]) > +AT_CHECK([ovs-appctl ovs/route/add 2001:db8:beef::13/128 br0 > 2001:db8:cafe::1 pkt_mark=13 src=2001:db8:cafe::3], [0], [OK > +]) > +AT_CHECK([ovs-appctl ovs/route/add 2001:db8:beef::14/128 br0 > 2001:db8:cafe::1 pkt_mark=14 src=2001:db8:cafe::2], [0], [OK > +]) > +AT_CHECK([ovs-appctl ovs/route/add 2001:db8:beef::15/128 br0 > 2001:db8:cafe::1 src=foo:bar:2001:db8:cafe], [2], [], [dnl > +Invalid parameters > +ovs-appctl: ovs-vswitchd: server returned an error > +]) > +AT_CHECK([ovs-appctl ovs/route/add 2001:db8:beef::16/128 br0 > 2001:db8:cafe::1 src=2001:db8:cafe::200], [2], [], [dnl > +Error while inserting route. > +ovs-appctl: ovs-vswitchd: server returned an error > +]) > +AT_CHECK([ovs-appctl ovs/route/add 2001:db8:beef::17/128 br0 > 2001:db8:face::1 src=2001:db8:cafe::3], [2], [], [dnl > +Error while inserting route. > +ovs-appctl: ovs-vswitchd: server returned an error > +]) > +AT_CHECK([ovs-appctl ovs/route/add 2001:db8:beef::18/128 br0 > src=2001:db8:cafe::3], [2], [], [dnl > +Error while inserting route. > +ovs-appctl: ovs-vswitchd: server returned an error > +]) > +AT_CHECK([ovs-appctl ovs/route/show | grep User | grep 2001:db8:beef | > sort], [0], [dnl > +User: 2001:db8:beef::12/128 dev br0 GW 2001:db8:cafe::1 SRC 2001:db8:cafe::3 > +User: 2001:db8:beef::13/128 MARK 13 dev br0 GW 2001:db8:cafe::1 SRC > 2001:db8:cafe::3 > +User: 2001:db8:beef::14/128 MARK 14 dev br0 GW 2001:db8:cafe::1 SRC > 2001:db8:cafe::2 > +]) > +OVS_VSWITCHD_STOP > +AT_CLEANUP > + > AT_SETUP([appctl - route/lookup]) > AT_KEYWORDS([ovs_router]) > OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=dummy]) > -- > 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