The source address parameter in ovs_router_lookup is both an input and
an output. However, the interface was complex. The caller could
inadvertently set a search over v4 or v6 rules based on if the source
address was initialized to in6addr_any or in6addr_v4mapped_any. The
lookup function even used these two values interchangeably.
This patch uses dst address to determine if the lookup is v4 or v6, and
considers both v6_any and v4mapped_any to be the null value equally. Now
if the caller just wants src as output, they can initialize it to v6_any
and lookup will still work correctly.
Fixes: dc14e92bcc25 ("route-table: Introduce multi-table route lookup.")
Signed-off-by: Mike Pattrick <[email protected]>
---
v2:
- Split src parameter into in and out versions in lookup function.
- Big refactor of lookup function.
- Added comment to explain their use.
- Changed unit test formatting.
- Added address family to router classifier.
---
lib/flow.c | 2 +-
lib/netdev-vport.c | 2 +-
lib/ovs-router.c | 115 +++++++++++++++++++----------------
lib/ovs-router.h | 2 +-
ofproto/ofproto-dpif-sflow.c | 2 +-
ofproto/ofproto-dpif-xlate.c | 30 +++++----
tests/ovs-router.at | 22 +++++++
7 files changed, 108 insertions(+), 67 deletions(-)
diff --git a/lib/flow.c b/lib/flow.c
index a59a25c46..d14603fe3 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -3731,7 +3731,7 @@ flow_get_tunnel_netdev(struct flow_tnl *tunnel)
return NULL;
}
- if (!ovs_router_lookup(0, &ip6, iface, NULL, &gw)) {
+ if (!ovs_router_lookup(0, &ip6, NULL, iface, NULL, &gw)) {
return NULL;
}
diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
index d11269d00..aa5223128 100644
--- a/lib/netdev-vport.c
+++ b/lib/netdev-vport.c
@@ -300,7 +300,7 @@ tunnel_check_status_change__(struct netdev_vport *netdev)
iface[0] = '\0';
route = &tnl_cfg->ipv6_dst;
mark = tnl_cfg->egress_pkt_mark;
- if (ovs_router_lookup(mark, route, iface, NULL, &gw)) {
+ if (ovs_router_lookup(mark, route, NULL, iface, NULL, &gw)) {
struct netdev *egress_netdev;
if (!netdev_open(iface, NULL, &egress_netdev)) {
diff --git a/lib/ovs-router.c b/lib/ovs-router.c
index c11a52764..30113592f 100644
--- a/lib/ovs-router.c
+++ b/lib/ovs-router.c
@@ -169,90 +169,94 @@ ovs_router_lookup_fallback(const struct in6_addr *ip6_dst,
return true;
}
+/* If the caller does not want to match on source prefix, src_in must be NULL.
+ * If src_in is not NULL, src_out will be set based on it instead of the route
+ * entry. */
bool
ovs_router_lookup(uint32_t mark, const struct in6_addr *ip6_dst,
+ const struct in6_addr *src_in,
char output_netdev[],
- struct in6_addr *src, struct in6_addr *gw)
+ struct in6_addr *src_out, struct in6_addr *gw)
{
- struct flow flow = {.ipv6_dst = *ip6_dst, .pkt_mark = mark};
- const struct in6_addr *from_src = src;
- const struct cls_rule *cr = NULL;
+ bool is_ipv4 = IN6_IS_ADDR_V4MAPPED(ip6_dst);
+ ovs_be16 dl_type = is_ipv4 ? htons(ETH_TYPE_IP) : htons(ETH_TYPE_IPV6);
+ const struct cls_rule *cr;
struct router_rule *rule;
+ struct classifier *cls;
+ struct flow flow;
- if (src && ipv6_addr_is_set(src)) {
- struct flow flow_src = {.ipv6_dst = *src, .pkt_mark = mark};
- struct classifier *cls_local = cls_find(CLS_LOCAL);
- const struct cls_rule *cr_src;
+ if (src_in) {
+ flow = (struct flow) {.ipv6_dst = *src_in, .pkt_mark = mark,
+ .dl_type = dl_type};
+ ovs_assert(is_ipv4 == IN6_IS_ADDR_V4MAPPED(src_in));
- if (!cls_local) {
+ cls = cls_find(CLS_LOCAL);
+
+ if (!cls) {
return false;
}
- cr_src = classifier_lookup(cls_local, OVS_VERSION_MAX, &flow_src,
+ cr = classifier_lookup(cls, OVS_VERSION_MAX, &flow,
NULL, NULL);
- if (!cr_src) {
+ if (!cr) {
return false;
}
- }
-
- if (!from_src) {
- if (IN6_IS_ADDR_V4MAPPED(ip6_dst)) {
- from_src = &in6addr_v4mapped_any;
+ if (src_out) {
+ *src_out = *src_in;
+ src_out = NULL;
+ }
+ } else {
+ if (is_ipv4) {
+ src_in = &in6addr_v4mapped_any;
} else {
- from_src = &in6addr_any;
+ src_in = &in6addr_any;
}
}
+ flow = (struct flow){.ipv6_dst = *ip6_dst, .pkt_mark = mark,
+ .dl_type = dl_type};
+
PVECTOR_FOR_EACH (rule, &rules) {
uint8_t plen = rule->ipv4 ? rule->src_prefix + 96 : rule->src_prefix;
bool matched;
- if ((IN6_IS_ADDR_V4MAPPED(from_src) && !rule->ipv4) ||
- (!IN6_IS_ADDR_V4MAPPED(from_src) && rule->ipv4)) {
+ if (is_ipv4 != rule->ipv4) {
continue;
}
matched = (!rule->src_prefix ||
- ipv6_addr_equals_masked(&rule->from_addr, from_src, plen));
+ ipv6_addr_equals_masked(&rule->from_addr, src_in, plen));
if (rule->invert) {
matched = !matched;
}
- if (matched) {
- struct classifier *cls = cls_find(rule->lookup_table);
+ if (!matched) {
+ continue;
+ }
+ cls = cls_find(rule->lookup_table);
- if (!cls) {
- /* A rule can be added before the table is created. */
- continue;
- }
- cr = classifier_lookup(cls, OVS_VERSION_MAX, &flow, NULL,
- NULL);
- if (cr) {
- struct ovs_router_entry *p = ovs_router_entry_cast(cr);
- /* Avoid matching mapped IPv4 of a packet against default IPv6
- * route entry. Either packet dst is IPv6 or both packet and
- * route entry dst are mapped IPv4.
- */
- if (!IN6_IS_ADDR_V4MAPPED(ip6_dst) ||
- IN6_IS_ADDR_V4MAPPED(&p->nw_addr)) {
- break;
- }
- }
+ if (!cls) {
+ /* A rule can be added before the table is created. */
+ continue;
+ }
+ cr = classifier_lookup(cls, OVS_VERSION_MAX, &flow, NULL,
+ NULL);
+ if (!cr) {
+ continue;
}
- }
- if (cr) {
struct ovs_router_entry *p = ovs_router_entry_cast(cr);
ovs_strlcpy(output_netdev, p->output_netdev, IFNAMSIZ);
*gw = p->gw;
- if (src && !ipv6_addr_is_set(src)) {
- *src = p->src_addr;
+ if (src_out) {
+ *src_out = p->src_addr;
}
return true;
}
- return ovs_router_lookup_fallback(ip6_dst, output_netdev, src, gw);
+
+ return ovs_router_lookup_fallback(ip6_dst, output_netdev, src_out, gw);
}
static void
@@ -268,14 +272,23 @@ static void rt_init_match(struct match *match, uint32_t
mark,
{
struct in6_addr dst;
struct in6_addr mask;
+ ovs_be16 dl_type;
+
+ if (IN6_IS_ADDR_V4MAPPED(ip6_dst)) {
+ dl_type = htons(ETH_TYPE_IP);
+ } else {
+ dl_type = htons(ETH_TYPE_IPV6);
+ }
mask = ipv6_create_mask(plen);
dst = ipv6_addr_bitand(ip6_dst, &mask);
memset(match, 0, sizeof *match);
match->flow.ipv6_dst = dst;
+ match->flow.dl_type = dl_type;
match->wc.masks.ipv6_dst = mask;
match->wc.masks.pkt_mark = UINT32_MAX;
+ match->wc.masks.dl_type = htons(UINT16_MAX);
match->flow.pkt_mark = mark;
}
@@ -1126,7 +1139,8 @@ static void
ovs_router_lookup_cmd(struct unixctl_conn *conn, int argc,
const char *argv[], void *aux OVS_UNUSED)
{
- struct in6_addr gw, src6 = in6addr_any;
+ struct in6_addr gw, src_out, src6;
+ struct in6_addr *src_in = NULL;
char src6_s[IPV6_SCAN_LEN + 1];
char iface[IFNAMSIZ];
struct in6_addr ip6;
@@ -1156,10 +1170,13 @@ ovs_router_lookup_cmd(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)) {
+ src_in = &src6;
continue;
}
} else {
if (ovs_scan(argv[i], "src="IP_SCAN_FMT, IP_SCAN_ARGS(&src))) {
+ in6_addr_set_mapped_ipv4(&src6, src);
+ src_in = &src6;
continue;
}
}
@@ -1168,15 +1185,11 @@ ovs_router_lookup_cmd(struct unixctl_conn *conn, int
argc,
return;
}
- if (src) {
- in6_addr_set_mapped_ipv4(&src6, src);
- }
-
- if (ovs_router_lookup(mark, &ip6, iface, &src6, &gw)) {
+ if (ovs_router_lookup(mark, &ip6, src_in, iface, &src_out, &gw)) {
struct ds ds = DS_EMPTY_INITIALIZER;
ds_put_format(&ds, "src ");
- ipv6_format_mapped(&src6, &ds);
+ ipv6_format_mapped(&src_out, &ds);
ds_put_format(&ds, "\ngateway ");
ipv6_format_mapped(&gw, &ds);
ds_put_format(&ds, "\ndev %s\n", iface);
diff --git a/lib/ovs-router.h b/lib/ovs-router.h
index bd1ab7a9a..ff465a7c7 100644
--- a/lib/ovs-router.h
+++ b/lib/ovs-router.h
@@ -34,7 +34,7 @@ enum {
};
bool ovs_router_lookup(uint32_t mark, const struct in6_addr *ip_dst,
- char output_netdev[],
+ const struct in6_addr *src_in, char output_netdev[],
struct in6_addr *src, struct in6_addr *gw);
void ovs_router_init(void);
bool ovs_router_is_referenced(uint32_t table);
diff --git a/ofproto/ofproto-dpif-sflow.c b/ofproto/ofproto-dpif-sflow.c
index e043d7cbc..27e5955a6 100644
--- a/ofproto/ofproto-dpif-sflow.c
+++ b/ofproto/ofproto-dpif-sflow.c
@@ -482,7 +482,7 @@ sflow_choose_agent_address(const char *agent_device,
struct in6_addr gw, src = in6addr_any;
char name[IFNAMSIZ];
- if (ovs_router_lookup(0, &target_ip, name, &src, &gw)) {
+ if (ovs_router_lookup(0, &target_ip, NULL, name, &src, &gw)) {
ip = src;
goto success;
}
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index f2db4723b..cfeca24a3 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3679,16 +3679,28 @@ process_special(struct xlate_ctx *ctx, const struct
xport *xport)
static int
tnl_route_lookup_flow(const struct xlate_ctx *ctx,
const struct flow *oflow,
- struct in6_addr *ip, struct in6_addr *src,
+ struct in6_addr *ip, struct in6_addr *out_src,
struct xport **out_port)
{
- char out_dev[IFNAMSIZ];
struct xbridge *xbridge;
- struct in6_addr gw;
+ char out_dev[IFNAMSIZ];
+ struct in6_addr in_src;
+ struct in6_addr *_in_src = &in_src;
struct in6_addr dst;
+ struct in6_addr gw;
+
+ /* If tunnel src address is set, it overrides route lookup. */
+ if (oflow->tunnel.ip_src) {
+ in6_addr_set_mapped_ipv4(&in_src, oflow->tunnel.ip_src);
+ } else if (ipv6_addr_is_set(&oflow->tunnel.ipv6_src)) {
+ in_src = oflow->tunnel.ipv6_src;
+ } else {
+ _in_src = NULL;
+ }
dst = flow_tnl_dst(&oflow->tunnel);
- if (!ovs_router_lookup(oflow->pkt_mark, &dst, out_dev, src, &gw)) {
+ if (!ovs_router_lookup(oflow->pkt_mark, &dst, _in_src, out_dev, out_src,
+ &gw)) {
return -ENOENT;
}
@@ -3878,8 +3890,8 @@ native_tunnel_output(struct xlate_ctx *ctx, const struct
xport *xport,
struct ovs_action_push_tnl tnl_push_data;
struct xport *out_dev = NULL;
ovs_be32 s_ip = 0, d_ip = 0;
- struct in6_addr s_ip6 = in6addr_any;
- struct in6_addr d_ip6 = in6addr_any;
+ struct in6_addr s_ip6;
+ struct in6_addr d_ip6;
struct eth_addr smac;
struct eth_addr dmac;
int err;
@@ -3897,12 +3909,6 @@ native_tunnel_output(struct xlate_ctx *ctx, const struct
xport *xport,
memcpy(&old_base_flow, &ctx->base_flow, sizeof old_base_flow);
memcpy(&old_flow, &ctx->xin->flow, sizeof old_flow);
- if (flow->tunnel.ip_src) {
- in6_addr_set_mapped_ipv4(&s_ip6, flow->tunnel.ip_src);
- } else if (ipv6_addr_is_set(&flow->tunnel.ipv6_src)) {
- s_ip6 = flow->tunnel.ipv6_src;
- }
-
err = tnl_route_lookup_flow(ctx, flow, &d_ip6, &s_ip6, &out_dev);
if (err) {
put_cloned_drop_action(ctx->xbridge->ofproto, ctx->odp_actions,
diff --git a/tests/ovs-router.at b/tests/ovs-router.at
index b5282fd19..5f37e6c26 100644
--- a/tests/ovs-router.at
+++ b/tests/ovs-router.at
@@ -315,6 +315,28 @@ OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1
type=dummy])
AT_CHECK([ovs-appctl netdev-dummy/ip4addr br0 192.0.2.1/24], [0], [OK
])
+AT_CHECK([ovs-appctl ovs/route/rule/add from=all table=15], [0], [OK
+])
+AT_CHECK([ovs-appctl ovs/route/add 2.2.2.3/32 br0 192.0.2.1 table=15], [0], [OK
+])
+
+AT_CHECK([ovs-appctl ovs/route/show table=all | sort], [0], [dnl
+Cached: 192.0.2.0/24 dev br0 SRC 192.0.2.1
+Cached: 192.0.2.1/32 dev br0 SRC 192.0.2.1 local
+User: 2.2.2.3/32 dev br0 GW 192.0.2.1 SRC 192.0.2.1 table 15
+])
+
+AT_CHECK([ovs-appctl ovs/route/lookup 2.2.2.3], [0], [dnl
+src 192.0.2.1
+gateway 192.0.2.1
+dev br0
+])
+
+AT_CHECK([ovs-appctl ovs/route/del 2.2.2.3/32 table=15], [0], [OK
+])
+AT_CHECK([ovs-appctl ovs/route/rule/del from=all table=15], [0], [OK
+])
+
AT_CHECK([ovs-appctl ovs/route/add 10.1.1.0/24 br0 192.0.2.2 table=11], [0],
[OK
])
AT_CHECK([ovs-appctl ovs/route/add 10.2.2.0/24 br0 192.0.2.2 table=12], [0],
[OK
--
2.53.0
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
