On 10/9/26 7:34 PM, Mike Pattrick via dev wrote:
> 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.
This is not fully accurate since the changes in one of the previous
versions, right?
>
> 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.
> v3:
> - Split ipv4 addresses out of ipv6_dst field
> - Updated comments
> v4:
> - Removed assert, replaced with conditional
> v5:
> - Convered rt_init_match to match_* functions
> - Renamed variables to in/out suffix
> - Updated prototypes and comments
> v5:
> - Add a sanity check to scan_ipv6_route
> - Correct ovs_router_lookup comment
> ---
> lib/flow.c | 2 +-
> lib/netdev-vport.c | 2 +-
> lib/ovs-router.c | 152 ++++++++++++++++++++---------------
> lib/ovs-router.h | 6 +-
> ofproto/ofproto-dpif-sflow.c | 2 +-
> ofproto/ofproto-dpif-xlate.c | 30 ++++---
> tests/ovs-router.at | 22 +++++
> 7 files changed, 135 insertions(+), 81 deletions(-)
[...]
> @@ -270,17 +283,20 @@ static void rt_init_match(struct match *match, uint32_t
> mark,
> const struct in6_addr *ip6_dst,
> uint8_t plen)
> {
> - struct in6_addr dst;
> - struct in6_addr mask;
>
> - mask = ipv6_create_mask(plen);
> + match_init_catchall(match);
> + match_set_pkt_mark(match, mark);
>
> - dst = ipv6_addr_bitand(ip6_dst, &mask);
> - memset(match, 0, sizeof *match);
> - match->flow.ipv6_dst = dst;
> - match->wc.masks.ipv6_dst = mask;
> - match->wc.masks.pkt_mark = UINT32_MAX;
> - match->flow.pkt_mark = mark;
> + if (IN6_IS_ADDR_V4MAPPED(ip6_dst)) {
> + match_set_dl_type(match, htons(ETH_TYPE_IP));
> + match_set_nw_dst_masked(match, in6_addr_get_mapped_ipv4(ip6_dst),
> + be32_prefix_mask(plen - 96));
Sorry for coming back to this, but it seems like the ovs/route commands
are not the only ones that can pass a v4mapped address with a mask smaller
than 96 here. The netdev-dummy/ip6addr seems to be able to do so. And it
appears to be possible to get an ipv6 route for an ipv4 mapped address
installed into and dumped from the kernel.
We should probably not allow those as well. Maybe something like this:
diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c
index 5e8b04cc0..d07b5f4de 100644
--- a/lib/netdev-dummy.c
+++ b/lib/netdev-dummy.c
@@ -2186,7 +2186,7 @@ netdev_dummy_ip6addr(struct unixctl_conn *conn, int argc
OVS_UNUSED,
uint32_t plen;
error = ipv6_parse_cidr(argv[2], &ip6, &plen);
- if (!error) {
+ if (!error && !IN6_IS_ADDR_V4MAPPED(&ip6)) {
struct in6_addr mask;
mask = ipv6_create_mask(plen);
@@ -2200,9 +2200,11 @@ netdev_dummy_ip6addr(struct unixctl_conn *conn, int argc
OVS_UNUSED,
&in6addr_any, &ip6);
unixctl_command_reply(conn, "OK");
- } else {
+ } else if (error) {
unixctl_command_reply_error(conn, error);
free(error);
+ } else {
+ unixctl_command_reply_error(conn, "v4 mapped address");
}
} else {
unixctl_command_reply_error(conn, "Unknown Dummy Interface");
diff --git a/lib/route-table.c b/lib/route-table.c
index 2a13a5cc7..3eaef3751 100644
--- a/lib/route-table.c
+++ b/lib/route-table.c
@@ -560,6 +560,12 @@ route_table_parse__(struct ofpbuf *buf, size_t ofs,
} else if (ipv4) {
in6_addr_set_mapped_ipv4(&change->rd.rta_dst, 0);
}
+
+ if (!ipv4 && IN6_IS_ADDR_V4MAPPED(&change->rd.rta_dst)) {
+ VLOG_DBG_RL(&rl, "unsupported IPv4 mapped RTA_DST in IPv6 route");
+ goto error_out;
+ }
+
if (attrs[RTA_PREFSRC]) {
if (ipv4) {
ovs_be32 prefsrc;
---
What do you think? Might be a separate patch though, at least the
route-table part.
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
