* [PATCH v2 1/8] ndp: Remove redundant update to addr_seen
2024-11-14 3:33 [PATCH v2 0/8] ndp: Unsolicited RAs David Gibson
@ 2024-11-14 3:33 ` David Gibson
2024-11-14 3:33 ` [PATCH v2 2/8] ndp: Add ndp_send() helper David Gibson
` (7 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: David Gibson @ 2024-11-14 3:33 UTC (permalink / raw)
To: Stefano Brivio, passt-dev; +Cc: David Gibson
ndp() updates addr_seen or addr_ll_seen based on the source address of the
received packet. This is redundant since tap6_handler() has already
updated addr_seen for any type of packet, not just NDP.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
ndp.c | 9 ++-------
ndp.h | 4 ++--
2 files changed, 4 insertions(+), 9 deletions(-)
diff --git a/ndp.c b/ndp.c
index faae408..ab80898 100644
--- a/ndp.c
+++ b/ndp.c
@@ -179,8 +179,8 @@ struct ndp_ns {
*
* Return: 0 if not handled here, 1 if handled, -1 on failure
*/
-int ndp(struct ctx *c, const struct icmp6hdr *ih, const struct in6_addr *saddr,
- const struct pool *p)
+int ndp(const struct ctx *c, const struct icmp6hdr *ih,
+ const struct in6_addr *saddr, const struct pool *p)
{
struct ndp_na na = {
.ih = {
@@ -336,11 +336,6 @@ dns_done:
return 1;
}
- if (IN6_IS_ADDR_LINKLOCAL(saddr))
- c->ip6.addr_ll_seen = *saddr;
- else
- c->ip6.addr_seen = *saddr;
-
rsaddr = &c->ip6.our_tap_ll;
if (ih->icmp6_type == NS) {
diff --git a/ndp.h b/ndp.h
index a786441..abe6d02 100644
--- a/ndp.h
+++ b/ndp.h
@@ -6,7 +6,7 @@
#ifndef NDP_H
#define NDP_H
-int ndp(struct ctx *c, const struct icmp6hdr *ih, const struct in6_addr *saddr,
- const struct pool *p);
+int ndp(const struct ctx *c, const struct icmp6hdr *ih,
+ const struct in6_addr *saddr, const struct pool *p);
#endif /* NDP_H */
--
@@ -6,7 +6,7 @@
#ifndef NDP_H
#define NDP_H
-int ndp(struct ctx *c, const struct icmp6hdr *ih, const struct in6_addr *saddr,
- const struct pool *p);
+int ndp(const struct ctx *c, const struct icmp6hdr *ih,
+ const struct in6_addr *saddr, const struct pool *p);
#endif /* NDP_H */
--
2.47.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v2 2/8] ndp: Add ndp_send() helper
2024-11-14 3:33 [PATCH v2 0/8] ndp: Unsolicited RAs David Gibson
2024-11-14 3:33 ` [PATCH v2 1/8] ndp: Remove redundant update to addr_seen David Gibson
@ 2024-11-14 3:33 ` David Gibson
2024-11-14 3:33 ` [PATCH v2 3/8] ndp: Split out helpers for sending specific NDP message types David Gibson
` (6 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: David Gibson @ 2024-11-14 3:33 UTC (permalink / raw)
To: Stefano Brivio, passt-dev; +Cc: David Gibson
ndp() has a conditional on message type generating the reply message, then
a tiny amount of common code, then another conditional to send the reply
with slightly different parameters. We can make this a bit neater by
making a helper function for sending the reply, and call it from each of
the different message type paths.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
ndp.c | 32 ++++++++++++++++++--------------
1 file changed, 18 insertions(+), 14 deletions(-)
diff --git a/ndp.c b/ndp.c
index ab80898..fa1b67a 100644
--- a/ndp.c
+++ b/ndp.c
@@ -170,6 +170,21 @@ struct ndp_ns {
struct in6_addr target_addr;
} __attribute__((packed));
+/**
+ * ndp_send() - Send an NDP message
+ * @c: Execution context
+ * @dst: IPv6 address to send the message to
+ * @buf: ICMPv6 header + message payload
+ * @l4len: Length of message, including ICMPv6 header
+ */
+static void ndp_send(const struct ctx *c, const struct in6_addr *dst,
+ const void *buf, size_t l4len)
+{
+ const struct in6_addr *src = &c->ip6.our_tap_ll;
+
+ tap_icmp6_send(c, src, dst, buf, l4len);
+}
+
/**
* ndp() - Check for NDP solicitations, reply as needed
* @c: Execution context
@@ -223,9 +238,6 @@ int ndp(const struct ctx *c, const struct icmp6hdr *ih,
},
},
};
- const struct in6_addr *rsaddr; /* src addr for reply */
- unsigned char *ptr = NULL;
- size_t dlen;
if (ih->icmp6_type < RS || ih->icmp6_type > NA)
return 0;
@@ -249,7 +261,9 @@ int ndp(const struct ctx *c, const struct icmp6hdr *ih,
sizeof(na.target_addr));
memcpy(na.target_l2_addr.mac, c->our_tap_mac, ETH_ALEN);
+ ndp_send(c, saddr, &na, sizeof(struct ndp_na));
} else if (ih->icmp6_type == RS) {
+ unsigned char *ptr = NULL;
size_t dns_s_len = 0;
int i, n;
@@ -332,18 +346,8 @@ int ndp(const struct ctx *c, const struct icmp6hdr *ih,
dns_done:
memcpy(&ra.source_ll.mac, c->our_tap_mac, ETH_ALEN);
- } else {
- return 1;
- }
- rsaddr = &c->ip6.our_tap_ll;
-
- if (ih->icmp6_type == NS) {
- dlen = sizeof(struct ndp_na);
- tap_icmp6_send(c, rsaddr, saddr, &na, dlen);
- } else if (ih->icmp6_type == RS) {
- dlen = ptr - (unsigned char *)&ra;
- tap_icmp6_send(c, rsaddr, saddr, &ra, dlen);
+ ndp_send(c, saddr, &ra, ptr - (unsigned char *)&ra);
}
return 1;
--
@@ -170,6 +170,21 @@ struct ndp_ns {
struct in6_addr target_addr;
} __attribute__((packed));
+/**
+ * ndp_send() - Send an NDP message
+ * @c: Execution context
+ * @dst: IPv6 address to send the message to
+ * @buf: ICMPv6 header + message payload
+ * @l4len: Length of message, including ICMPv6 header
+ */
+static void ndp_send(const struct ctx *c, const struct in6_addr *dst,
+ const void *buf, size_t l4len)
+{
+ const struct in6_addr *src = &c->ip6.our_tap_ll;
+
+ tap_icmp6_send(c, src, dst, buf, l4len);
+}
+
/**
* ndp() - Check for NDP solicitations, reply as needed
* @c: Execution context
@@ -223,9 +238,6 @@ int ndp(const struct ctx *c, const struct icmp6hdr *ih,
},
},
};
- const struct in6_addr *rsaddr; /* src addr for reply */
- unsigned char *ptr = NULL;
- size_t dlen;
if (ih->icmp6_type < RS || ih->icmp6_type > NA)
return 0;
@@ -249,7 +261,9 @@ int ndp(const struct ctx *c, const struct icmp6hdr *ih,
sizeof(na.target_addr));
memcpy(na.target_l2_addr.mac, c->our_tap_mac, ETH_ALEN);
+ ndp_send(c, saddr, &na, sizeof(struct ndp_na));
} else if (ih->icmp6_type == RS) {
+ unsigned char *ptr = NULL;
size_t dns_s_len = 0;
int i, n;
@@ -332,18 +346,8 @@ int ndp(const struct ctx *c, const struct icmp6hdr *ih,
dns_done:
memcpy(&ra.source_ll.mac, c->our_tap_mac, ETH_ALEN);
- } else {
- return 1;
- }
- rsaddr = &c->ip6.our_tap_ll;
-
- if (ih->icmp6_type == NS) {
- dlen = sizeof(struct ndp_na);
- tap_icmp6_send(c, rsaddr, saddr, &na, dlen);
- } else if (ih->icmp6_type == RS) {
- dlen = ptr - (unsigned char *)&ra;
- tap_icmp6_send(c, rsaddr, saddr, &ra, dlen);
+ ndp_send(c, saddr, &ra, ptr - (unsigned char *)&ra);
}
return 1;
--
2.47.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v2 3/8] ndp: Split out helpers for sending specific NDP message types
2024-11-14 3:33 [PATCH v2 0/8] ndp: Unsolicited RAs David Gibson
2024-11-14 3:33 ` [PATCH v2 1/8] ndp: Remove redundant update to addr_seen David Gibson
2024-11-14 3:33 ` [PATCH v2 2/8] ndp: Add ndp_send() helper David Gibson
@ 2024-11-14 3:33 ` David Gibson
2024-11-14 3:33 ` [PATCH v2 4/8] ndp: Use struct assignment in preference to memcpy() for IPv6 addresses David Gibson
` (5 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: David Gibson @ 2024-11-14 3:33 UTC (permalink / raw)
To: Stefano Brivio, passt-dev; +Cc: David Gibson
Currently the large ndp() function responds to all NDP messages we handle,
both parsing the message as necessary and sending the response. Split out
the code to construct and send specific message types into ndp_na() (to
send NA messages) and ndp_ra() (to send RA messages).
As well as breaking up an excessively large function, this is a first step
to being able to send unsolicited NDP messages.
While we're there, remove a slighty ugly goto.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
ndp.c | 132 +++++++++++++++++++++++++++++++++-------------------------
1 file changed, 76 insertions(+), 56 deletions(-)
diff --git a/ndp.c b/ndp.c
index fa1b67a..8f52471 100644
--- a/ndp.c
+++ b/ndp.c
@@ -186,16 +186,13 @@ static void ndp_send(const struct ctx *c, const struct in6_addr *dst,
}
/**
- * ndp() - Check for NDP solicitations, reply as needed
+ * ndp_na() - Send an NDP Neighbour Advertisement (NA) message
* @c: Execution context
- * @ih: ICMPv6 header
- * @saddr: Source IPv6 address
- * @p: Packet pool
- *
- * Return: 0 if not handled here, 1 if handled, -1 on failure
+ * @dst: IPv6 address to send the NA to
+ * @addr: IPv6 address to advertise
*/
-int ndp(const struct ctx *c, const struct icmp6hdr *ih,
- const struct in6_addr *saddr, const struct pool *p)
+static void ndp_na(const struct ctx *c, const struct in6_addr *dst,
+ const void *addr)
{
struct ndp_na na = {
.ih = {
@@ -212,6 +209,20 @@ int ndp(const struct ctx *c, const struct icmp6hdr *ih,
},
}
};
+
+ memcpy(&na.target_addr, addr, sizeof(na.target_addr));
+ memcpy(na.target_l2_addr.mac, c->our_tap_mac, ETH_ALEN);
+
+ ndp_send(c, dst, &na, sizeof(na));
+}
+
+/**
+ * ndp_ra() - Send an NDP Router Advertisement (RA) message
+ * @c: Execution context
+ * @dst: IPv6 address to send the RA to
+ */
+static void ndp_ra(const struct ctx *c, const struct in6_addr *dst)
+{
struct ndp_ra ra = {
.ih = {
.icmp6_type = RA,
@@ -238,58 +249,28 @@ int ndp(const struct ctx *c, const struct icmp6hdr *ih,
},
},
};
+ unsigned char *ptr = NULL;
- if (ih->icmp6_type < RS || ih->icmp6_type > NA)
- return 0;
-
- if (c->no_ndp)
- return 1;
-
- if (ih->icmp6_type == NS) {
- const struct ndp_ns *ns =
- packet_get(p, 0, 0, sizeof(struct ndp_ns), NULL);
+ memcpy(&ra.prefix, &c->ip6.addr, sizeof(ra.prefix));
- if (!ns)
- return -1;
+ ptr = &ra.var[0];
- if (IN6_IS_ADDR_UNSPECIFIED(saddr))
- return 1;
-
- info("NDP: received NS, sending NA");
-
- memcpy(&na.target_addr, &ns->target_addr,
- sizeof(na.target_addr));
- memcpy(na.target_l2_addr.mac, c->our_tap_mac, ETH_ALEN);
+ if (c->mtu != -1) {
+ struct opt_mtu *mtu = (struct opt_mtu *)ptr;
+ *mtu = (struct opt_mtu) {
+ .header = {
+ .type = OPT_MTU,
+ .len = 1,
+ },
+ .value = htonl(c->mtu),
+ };
+ ptr += sizeof(struct opt_mtu);
+ }
- ndp_send(c, saddr, &na, sizeof(struct ndp_na));
- } else if (ih->icmp6_type == RS) {
- unsigned char *ptr = NULL;
+ if (!c->no_dhcp_dns) {
size_t dns_s_len = 0;
int i, n;
- if (c->no_ra)
- return 1;
-
- info("NDP: received RS, sending RA");
- memcpy(&ra.prefix, &c->ip6.addr, sizeof(ra.prefix));
-
- ptr = &ra.var[0];
-
- if (c->mtu != -1) {
- struct opt_mtu *mtu = (struct opt_mtu *)ptr;
- *mtu = (struct opt_mtu) {
- .header = {
- .type = OPT_MTU,
- .len = 1,
- },
- .value = htonl(c->mtu),
- };
- ptr += sizeof(struct opt_mtu);
- }
-
- if (c->no_dhcp_dns)
- goto dns_done;
-
for (n = 0; !IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns[n]); n++);
if (n) {
struct opt_rdnss *rdnss = (struct opt_rdnss *)ptr;
@@ -343,11 +324,50 @@ int ndp(const struct ctx *c, const struct icmp6hdr *ih,
memset(ptr, 0, 8 - dns_s_len % 8); /* padding */
ptr += 8 - dns_s_len % 8;
}
+ }
-dns_done:
- memcpy(&ra.source_ll.mac, c->our_tap_mac, ETH_ALEN);
+ memcpy(&ra.source_ll.mac, c->our_tap_mac, ETH_ALEN);
- ndp_send(c, saddr, &ra, ptr - (unsigned char *)&ra);
+ ndp_send(c, dst, &ra, ptr - (unsigned char *)&ra);
+}
+
+/**
+ * ndp() - Check for NDP solicitations, reply as needed
+ * @c: Execution context
+ * @ih: ICMPv6 header
+ * @saddr: Source IPv6 address
+ * @p: Packet pool
+ *
+ * Return: 0 if not handled here, 1 if handled, -1 on failure
+ */
+int ndp(const struct ctx *c, const struct icmp6hdr *ih,
+ const struct in6_addr *saddr, const struct pool *p)
+{
+ if (ih->icmp6_type < RS || ih->icmp6_type > NA)
+ return 0;
+
+ if (c->no_ndp)
+ return 1;
+
+ if (ih->icmp6_type == NS) {
+ const struct ndp_ns *ns;
+
+ ns = packet_get(p, 0, 0, sizeof(struct ndp_ns), NULL);
+ if (!ns)
+ return -1;
+
+ if (IN6_IS_ADDR_UNSPECIFIED(saddr))
+ return 1;
+
+ info("NDP: received NS, sending NA");
+
+ ndp_na(c, saddr, &ns->target_addr);
+ } else if (ih->icmp6_type == RS) {
+ if (c->no_ra)
+ return 1;
+
+ info("NDP: received RS, sending RA");
+ ndp_ra(c, saddr);
}
return 1;
--
@@ -186,16 +186,13 @@ static void ndp_send(const struct ctx *c, const struct in6_addr *dst,
}
/**
- * ndp() - Check for NDP solicitations, reply as needed
+ * ndp_na() - Send an NDP Neighbour Advertisement (NA) message
* @c: Execution context
- * @ih: ICMPv6 header
- * @saddr: Source IPv6 address
- * @p: Packet pool
- *
- * Return: 0 if not handled here, 1 if handled, -1 on failure
+ * @dst: IPv6 address to send the NA to
+ * @addr: IPv6 address to advertise
*/
-int ndp(const struct ctx *c, const struct icmp6hdr *ih,
- const struct in6_addr *saddr, const struct pool *p)
+static void ndp_na(const struct ctx *c, const struct in6_addr *dst,
+ const void *addr)
{
struct ndp_na na = {
.ih = {
@@ -212,6 +209,20 @@ int ndp(const struct ctx *c, const struct icmp6hdr *ih,
},
}
};
+
+ memcpy(&na.target_addr, addr, sizeof(na.target_addr));
+ memcpy(na.target_l2_addr.mac, c->our_tap_mac, ETH_ALEN);
+
+ ndp_send(c, dst, &na, sizeof(na));
+}
+
+/**
+ * ndp_ra() - Send an NDP Router Advertisement (RA) message
+ * @c: Execution context
+ * @dst: IPv6 address to send the RA to
+ */
+static void ndp_ra(const struct ctx *c, const struct in6_addr *dst)
+{
struct ndp_ra ra = {
.ih = {
.icmp6_type = RA,
@@ -238,58 +249,28 @@ int ndp(const struct ctx *c, const struct icmp6hdr *ih,
},
},
};
+ unsigned char *ptr = NULL;
- if (ih->icmp6_type < RS || ih->icmp6_type > NA)
- return 0;
-
- if (c->no_ndp)
- return 1;
-
- if (ih->icmp6_type == NS) {
- const struct ndp_ns *ns =
- packet_get(p, 0, 0, sizeof(struct ndp_ns), NULL);
+ memcpy(&ra.prefix, &c->ip6.addr, sizeof(ra.prefix));
- if (!ns)
- return -1;
+ ptr = &ra.var[0];
- if (IN6_IS_ADDR_UNSPECIFIED(saddr))
- return 1;
-
- info("NDP: received NS, sending NA");
-
- memcpy(&na.target_addr, &ns->target_addr,
- sizeof(na.target_addr));
- memcpy(na.target_l2_addr.mac, c->our_tap_mac, ETH_ALEN);
+ if (c->mtu != -1) {
+ struct opt_mtu *mtu = (struct opt_mtu *)ptr;
+ *mtu = (struct opt_mtu) {
+ .header = {
+ .type = OPT_MTU,
+ .len = 1,
+ },
+ .value = htonl(c->mtu),
+ };
+ ptr += sizeof(struct opt_mtu);
+ }
- ndp_send(c, saddr, &na, sizeof(struct ndp_na));
- } else if (ih->icmp6_type == RS) {
- unsigned char *ptr = NULL;
+ if (!c->no_dhcp_dns) {
size_t dns_s_len = 0;
int i, n;
- if (c->no_ra)
- return 1;
-
- info("NDP: received RS, sending RA");
- memcpy(&ra.prefix, &c->ip6.addr, sizeof(ra.prefix));
-
- ptr = &ra.var[0];
-
- if (c->mtu != -1) {
- struct opt_mtu *mtu = (struct opt_mtu *)ptr;
- *mtu = (struct opt_mtu) {
- .header = {
- .type = OPT_MTU,
- .len = 1,
- },
- .value = htonl(c->mtu),
- };
- ptr += sizeof(struct opt_mtu);
- }
-
- if (c->no_dhcp_dns)
- goto dns_done;
-
for (n = 0; !IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns[n]); n++);
if (n) {
struct opt_rdnss *rdnss = (struct opt_rdnss *)ptr;
@@ -343,11 +324,50 @@ int ndp(const struct ctx *c, const struct icmp6hdr *ih,
memset(ptr, 0, 8 - dns_s_len % 8); /* padding */
ptr += 8 - dns_s_len % 8;
}
+ }
-dns_done:
- memcpy(&ra.source_ll.mac, c->our_tap_mac, ETH_ALEN);
+ memcpy(&ra.source_ll.mac, c->our_tap_mac, ETH_ALEN);
- ndp_send(c, saddr, &ra, ptr - (unsigned char *)&ra);
+ ndp_send(c, dst, &ra, ptr - (unsigned char *)&ra);
+}
+
+/**
+ * ndp() - Check for NDP solicitations, reply as needed
+ * @c: Execution context
+ * @ih: ICMPv6 header
+ * @saddr: Source IPv6 address
+ * @p: Packet pool
+ *
+ * Return: 0 if not handled here, 1 if handled, -1 on failure
+ */
+int ndp(const struct ctx *c, const struct icmp6hdr *ih,
+ const struct in6_addr *saddr, const struct pool *p)
+{
+ if (ih->icmp6_type < RS || ih->icmp6_type > NA)
+ return 0;
+
+ if (c->no_ndp)
+ return 1;
+
+ if (ih->icmp6_type == NS) {
+ const struct ndp_ns *ns;
+
+ ns = packet_get(p, 0, 0, sizeof(struct ndp_ns), NULL);
+ if (!ns)
+ return -1;
+
+ if (IN6_IS_ADDR_UNSPECIFIED(saddr))
+ return 1;
+
+ info("NDP: received NS, sending NA");
+
+ ndp_na(c, saddr, &ns->target_addr);
+ } else if (ih->icmp6_type == RS) {
+ if (c->no_ra)
+ return 1;
+
+ info("NDP: received RS, sending RA");
+ ndp_ra(c, saddr);
}
return 1;
--
2.47.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v2 4/8] ndp: Use struct assignment in preference to memcpy() for IPv6 addresses
2024-11-14 3:33 [PATCH v2 0/8] ndp: Unsolicited RAs David Gibson
` (2 preceding siblings ...)
2024-11-14 3:33 ` [PATCH v2 3/8] ndp: Split out helpers for sending specific NDP message types David Gibson
@ 2024-11-14 3:33 ` David Gibson
2024-11-14 3:33 ` [PATCH v2 5/8] ndp: Make route lifetime a #define David Gibson
` (4 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: David Gibson @ 2024-11-14 3:33 UTC (permalink / raw)
To: Stefano Brivio, passt-dev; +Cc: David Gibson
There are a number of places we can simply assign IPv6 addresses about,
rather than the current mildly ugly memcpy().
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
ndp.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/ndp.c b/ndp.c
index 8f52471..fd512ae 100644
--- a/ndp.c
+++ b/ndp.c
@@ -158,7 +158,7 @@ struct ndp_ra {
unsigned char var[sizeof(struct opt_mtu) + sizeof(struct opt_rdnss) +
sizeof(struct opt_dnssl)];
-} __attribute__((packed));
+} __attribute__((packed, aligned(__alignof__(struct in6_addr))));
/**
* struct ndp_ns - NDP Neighbor Solicitation (NS) message
@@ -168,7 +168,7 @@ struct ndp_ra {
struct ndp_ns {
struct icmp6hdr ih;
struct in6_addr target_addr;
-} __attribute__((packed));
+} __attribute__((packed, aligned(__alignof__(struct in6_addr))));
/**
* ndp_send() - Send an NDP message
@@ -192,7 +192,7 @@ static void ndp_send(const struct ctx *c, const struct in6_addr *dst,
* @addr: IPv6 address to advertise
*/
static void ndp_na(const struct ctx *c, const struct in6_addr *dst,
- const void *addr)
+ const struct in6_addr *addr)
{
struct ndp_na na = {
.ih = {
@@ -202,6 +202,7 @@ static void ndp_na(const struct ctx *c, const struct in6_addr *dst,
.icmp6_solicited = 1,
.icmp6_override = 1,
},
+ .target_addr = *addr,
.target_l2_addr = {
.header = {
.type = OPT_TARGET_L2_ADDR,
@@ -210,7 +211,6 @@ static void ndp_na(const struct ctx *c, const struct in6_addr *dst,
}
};
- memcpy(&na.target_addr, addr, sizeof(na.target_addr));
memcpy(na.target_l2_addr.mac, c->our_tap_mac, ETH_ALEN);
ndp_send(c, dst, &na, sizeof(na));
@@ -242,6 +242,7 @@ static void ndp_ra(const struct ctx *c, const struct in6_addr *dst)
.valid_lifetime = ~0U,
.pref_lifetime = ~0U,
},
+ .prefix = c->ip6.addr,
.source_ll = {
.header = {
.type = OPT_SRC_L2_ADDR,
@@ -251,8 +252,6 @@ static void ndp_ra(const struct ctx *c, const struct in6_addr *dst)
};
unsigned char *ptr = NULL;
- memcpy(&ra.prefix, &c->ip6.addr, sizeof(ra.prefix));
-
ptr = &ra.var[0];
if (c->mtu != -1) {
@@ -282,8 +281,7 @@ static void ndp_ra(const struct ctx *c, const struct in6_addr *dst)
.lifetime = ~0U,
};
for (i = 0; i < n; i++) {
- memcpy(&rdnss->dns[i], &c->ip6.dns[i],
- sizeof(rdnss->dns[i]));
+ rdnss->dns[i] = c->ip6.dns[i];
}
ptr += offsetof(struct opt_rdnss, dns) +
i * sizeof(rdnss->dns[0]);
--
@@ -158,7 +158,7 @@ struct ndp_ra {
unsigned char var[sizeof(struct opt_mtu) + sizeof(struct opt_rdnss) +
sizeof(struct opt_dnssl)];
-} __attribute__((packed));
+} __attribute__((packed, aligned(__alignof__(struct in6_addr))));
/**
* struct ndp_ns - NDP Neighbor Solicitation (NS) message
@@ -168,7 +168,7 @@ struct ndp_ra {
struct ndp_ns {
struct icmp6hdr ih;
struct in6_addr target_addr;
-} __attribute__((packed));
+} __attribute__((packed, aligned(__alignof__(struct in6_addr))));
/**
* ndp_send() - Send an NDP message
@@ -192,7 +192,7 @@ static void ndp_send(const struct ctx *c, const struct in6_addr *dst,
* @addr: IPv6 address to advertise
*/
static void ndp_na(const struct ctx *c, const struct in6_addr *dst,
- const void *addr)
+ const struct in6_addr *addr)
{
struct ndp_na na = {
.ih = {
@@ -202,6 +202,7 @@ static void ndp_na(const struct ctx *c, const struct in6_addr *dst,
.icmp6_solicited = 1,
.icmp6_override = 1,
},
+ .target_addr = *addr,
.target_l2_addr = {
.header = {
.type = OPT_TARGET_L2_ADDR,
@@ -210,7 +211,6 @@ static void ndp_na(const struct ctx *c, const struct in6_addr *dst,
}
};
- memcpy(&na.target_addr, addr, sizeof(na.target_addr));
memcpy(na.target_l2_addr.mac, c->our_tap_mac, ETH_ALEN);
ndp_send(c, dst, &na, sizeof(na));
@@ -242,6 +242,7 @@ static void ndp_ra(const struct ctx *c, const struct in6_addr *dst)
.valid_lifetime = ~0U,
.pref_lifetime = ~0U,
},
+ .prefix = c->ip6.addr,
.source_ll = {
.header = {
.type = OPT_SRC_L2_ADDR,
@@ -251,8 +252,6 @@ static void ndp_ra(const struct ctx *c, const struct in6_addr *dst)
};
unsigned char *ptr = NULL;
- memcpy(&ra.prefix, &c->ip6.addr, sizeof(ra.prefix));
-
ptr = &ra.var[0];
if (c->mtu != -1) {
@@ -282,8 +281,7 @@ static void ndp_ra(const struct ctx *c, const struct in6_addr *dst)
.lifetime = ~0U,
};
for (i = 0; i < n; i++) {
- memcpy(&rdnss->dns[i], &c->ip6.dns[i],
- sizeof(rdnss->dns[i]));
+ rdnss->dns[i] = c->ip6.dns[i];
}
ptr += offsetof(struct opt_rdnss, dns) +
i * sizeof(rdnss->dns[0]);
--
2.47.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v2 5/8] ndp: Make route lifetime a #define
2024-11-14 3:33 [PATCH v2 0/8] ndp: Unsolicited RAs David Gibson
` (3 preceding siblings ...)
2024-11-14 3:33 ` [PATCH v2 4/8] ndp: Use struct assignment in preference to memcpy() for IPv6 addresses David Gibson
@ 2024-11-14 3:33 ` David Gibson
2024-11-14 3:33 ` [PATCH v2 6/8] util: Add general low-level random bytes helper David Gibson
` (3 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: David Gibson @ 2024-11-14 3:33 UTC (permalink / raw)
To: Stefano Brivio, passt-dev; +Cc: David Gibson
Currently we open-code the lifetime of the route we advertise via NDP to be
65535s (the maximum). Change it to a #define.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
ndp.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/ndp.c b/ndp.c
index fd512ae..09df8d6 100644
--- a/ndp.c
+++ b/ndp.c
@@ -33,6 +33,8 @@
#include "tap.h"
#include "log.h"
+#define RT_LIFETIME 65535
+
#define RS 133
#define RA 134
#define NS 135
@@ -229,7 +231,7 @@ static void ndp_ra(const struct ctx *c, const struct in6_addr *dst)
.icmp6_code = 0,
.icmp6_hop_limit = 255,
/* RFC 8319 */
- .icmp6_rt_lifetime = htons_constant(65535),
+ .icmp6_rt_lifetime = htons_constant(RT_LIFETIME),
.icmp6_addrconf_managed = 1,
},
.prefix_info = {
--
@@ -33,6 +33,8 @@
#include "tap.h"
#include "log.h"
+#define RT_LIFETIME 65535
+
#define RS 133
#define RA 134
#define NS 135
@@ -229,7 +231,7 @@ static void ndp_ra(const struct ctx *c, const struct in6_addr *dst)
.icmp6_code = 0,
.icmp6_hop_limit = 255,
/* RFC 8319 */
- .icmp6_rt_lifetime = htons_constant(65535),
+ .icmp6_rt_lifetime = htons_constant(RT_LIFETIME),
.icmp6_addrconf_managed = 1,
},
.prefix_info = {
--
2.47.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v2 6/8] util: Add general low-level random bytes helper
2024-11-14 3:33 [PATCH v2 0/8] ndp: Unsolicited RAs David Gibson
` (4 preceding siblings ...)
2024-11-14 3:33 ` [PATCH v2 5/8] ndp: Make route lifetime a #define David Gibson
@ 2024-11-14 3:33 ` David Gibson
2024-11-14 3:33 ` [PATCH v2 7/8] passt: Seed libc's pseudo random number generator David Gibson
` (2 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: David Gibson @ 2024-11-14 3:33 UTC (permalink / raw)
To: Stefano Brivio, passt-dev; +Cc: David Gibson
Currently secret_init() open codes getting good quality random bytes from
the OS, either via getrandom(2) or reading /dev/random. We're going to
add at least one more place that needs random data in future, so make a
general helper for getting random bytes. While we're there, fix a number
of minor bugs:
- getrandom() can theoretically return a "short read", so handle that case
- getrandom() as well as read can return a transient EINTR
- We would attempt to read data from /dev/random if we failed to open it
(open() returns -1), but not if we opened it as fd 0 (unlikely, but ok)
- More specific error reporting
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
passt.c | 30 +-----------------------------
util.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
util.h | 2 ++
3 files changed, 57 insertions(+), 29 deletions(-)
diff --git a/passt.c b/passt.c
index fac6101..73649de 100644
--- a/passt.c
+++ b/passt.c
@@ -36,9 +36,6 @@
#include <sys/prctl.h>
#include <netinet/if_ether.h>
#include <libgen.h>
-#ifdef HAS_GETRANDOM
-#include <sys/random.h>
-#endif
#include "util.h"
#include "passt.h"
@@ -118,32 +115,7 @@ static void post_handler(struct ctx *c, const struct timespec *now)
*/
static void secret_init(struct ctx *c)
{
-#ifndef HAS_GETRANDOM
- int dev_random = open("/dev/random", O_RDONLY);
- unsigned int random_read = 0;
-
- while (dev_random && random_read < sizeof(c->hash_secret)) {
- int ret = read(dev_random,
- (uint8_t *)&c->hash_secret + random_read,
- sizeof(c->hash_secret) - random_read);
-
- if (ret == -1 && errno == EINTR)
- continue;
-
- if (ret <= 0)
- break;
-
- random_read += ret;
- }
- if (dev_random >= 0)
- close(dev_random);
-
- if (random_read < sizeof(c->hash_secret))
-#else
- if (getrandom(&c->hash_secret, sizeof(c->hash_secret),
- GRND_RANDOM) < 0)
-#endif /* !HAS_GETRANDOM */
- die_perror("Failed to get random bytes for hash table and TCP");
+ raw_random(&c->hash_secret, sizeof(c->hash_secret));
}
/**
diff --git a/util.c b/util.c
index 126dedb..55cae3f 100644
--- a/util.c
+++ b/util.c
@@ -34,6 +34,9 @@
#include "passt.h"
#include "packet.h"
#include "log.h"
+#ifdef HAS_GETRANDOM
+#include <sys/random.h>
+#endif
/**
* sock_l4_sa() - Create and bind socket to socket address, add to epoll list
@@ -783,3 +786,54 @@ bool snprintf_check(char *str, size_t size, const char *format, ...)
return false;
}
+
+#define DEV_RANDOM "/dev/random"
+
+/**
+ * raw_random() - Get high quality random bytes
+ * @buf: Buffer to fill with random bytes
+ * @buflen: Number of bytes of random data to put in @buf
+ *
+ * Assumes that the random data is essential, and will die() if unable to obtain
+ * it.
+ */
+void raw_random(void *buf, size_t buflen)
+{
+ size_t random_read = 0;
+#ifndef HAS_GETRANDOM
+ int fd = open(DEV_RANDOM, O_RDONLY);
+
+ if (fd < 0)
+ die_perror("Couldn't open %s", DEV_RANDOM);
+#endif
+
+ while (random_read < buflen) {
+ ssize_t ret;
+
+#ifdef HAS_GETRANDOM
+ ret = getrandom((char *)buf + random_read,
+ buflen - random_read, GRND_RANDOM);
+#else
+ ret = read(dev_random, (char *)buf + random_read,
+ buflen - random_read);
+#endif
+
+ if (ret == -1 && errno == EINTR)
+ continue;
+
+ if (ret < 0)
+ die_perror("Error on random data source");
+
+ if (ret == 0)
+ break;
+
+ random_read += ret;
+ }
+
+#ifndef HAS_GETRANDOM
+ close(dev_random);
+#endif
+
+ if (random_read < buflen)
+ die("Unexpected EOF on random data source");
+}
diff --git a/util.h b/util.h
index 3616515..90428c4 100644
--- a/util.h
+++ b/util.h
@@ -263,6 +263,8 @@ static inline bool mod_between(unsigned x, unsigned i, unsigned j, unsigned m)
/* FPRINTF() intentionally silences cert-err33-c clang-tidy warnings */
#define FPRINTF(f, ...) (void)fprintf(f, __VA_ARGS__)
+void raw_random(void *buf, size_t buflen);
+
/*
* Workarounds for https://github.com/llvm/llvm-project/issues/58992
*
--
@@ -263,6 +263,8 @@ static inline bool mod_between(unsigned x, unsigned i, unsigned j, unsigned m)
/* FPRINTF() intentionally silences cert-err33-c clang-tidy warnings */
#define FPRINTF(f, ...) (void)fprintf(f, __VA_ARGS__)
+void raw_random(void *buf, size_t buflen);
+
/*
* Workarounds for https://github.com/llvm/llvm-project/issues/58992
*
--
2.47.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v2 7/8] passt: Seed libc's pseudo random number generator
2024-11-14 3:33 [PATCH v2 0/8] ndp: Unsolicited RAs David Gibson
` (5 preceding siblings ...)
2024-11-14 3:33 ` [PATCH v2 6/8] util: Add general low-level random bytes helper David Gibson
@ 2024-11-14 3:33 ` David Gibson
2024-11-14 3:33 ` [PATCH v2 8/8] ndp: Send unsolicited Router Advertisements David Gibson
2024-11-14 18:18 ` [PATCH v2 0/8] ndp: Unsolicited RAs Stefano Brivio
8 siblings, 0 replies; 10+ messages in thread
From: David Gibson @ 2024-11-14 3:33 UTC (permalink / raw)
To: Stefano Brivio, passt-dev; +Cc: David Gibson
We have an upcoming case where we need pseudo-random numbers to scatter
timings, but we don't need cryptographically strong random numbers. libc's
built in random() is fine for this purpose, but we should seed it. Extend
secret_init() - the only current user of random numbers - to do this as
well as generating the SipHash secret. Using /dev/random for a PRNG seed
is probably overkill, but it's simple and we only do it once, so we might
as well.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
passt.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/passt.c b/passt.c
index 73649de..83b26c5 100644
--- a/passt.c
+++ b/passt.c
@@ -110,12 +110,19 @@ static void post_handler(struct ctx *c, const struct timespec *now)
}
/**
- * secret_init() - Create secret value for SipHash calculations
+ * random_init() - Initialise things based on random data
* @c: Execution context
*/
-static void secret_init(struct ctx *c)
+static void random_init(struct ctx *c)
{
+ unsigned int seed;
+
+ /* Create secret value for SipHash calculations */
raw_random(&c->hash_secret, sizeof(c->hash_secret));
+
+ /* Seed pseudo-RNG for things that need non-cryptographic random */
+ raw_random(&seed, sizeof(seed));
+ srandom(seed);
}
/**
@@ -236,7 +243,7 @@ int main(int argc, char **argv)
tap_sock_init(&c);
- secret_init(&c);
+ random_init(&c);
if (clock_gettime(CLOCK_MONOTONIC, &now))
die_perror("Failed to get CLOCK_MONOTONIC time");
--
@@ -110,12 +110,19 @@ static void post_handler(struct ctx *c, const struct timespec *now)
}
/**
- * secret_init() - Create secret value for SipHash calculations
+ * random_init() - Initialise things based on random data
* @c: Execution context
*/
-static void secret_init(struct ctx *c)
+static void random_init(struct ctx *c)
{
+ unsigned int seed;
+
+ /* Create secret value for SipHash calculations */
raw_random(&c->hash_secret, sizeof(c->hash_secret));
+
+ /* Seed pseudo-RNG for things that need non-cryptographic random */
+ raw_random(&seed, sizeof(seed));
+ srandom(seed);
}
/**
@@ -236,7 +243,7 @@ int main(int argc, char **argv)
tap_sock_init(&c);
- secret_init(&c);
+ random_init(&c);
if (clock_gettime(CLOCK_MONOTONIC, &now))
die_perror("Failed to get CLOCK_MONOTONIC time");
--
2.47.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v2 8/8] ndp: Send unsolicited Router Advertisements
2024-11-14 3:33 [PATCH v2 0/8] ndp: Unsolicited RAs David Gibson
` (6 preceding siblings ...)
2024-11-14 3:33 ` [PATCH v2 7/8] passt: Seed libc's pseudo random number generator David Gibson
@ 2024-11-14 3:33 ` David Gibson
2024-11-14 18:18 ` [PATCH v2 0/8] ndp: Unsolicited RAs Stefano Brivio
8 siblings, 0 replies; 10+ messages in thread
From: David Gibson @ 2024-11-14 3:33 UTC (permalink / raw)
To: Stefano Brivio, passt-dev; +Cc: David Gibson
Currently, our NDP implementation only sends Router Advertisements (RA)
when it receives a Router Solicitation (RS) from the guest. However,
RFC 4861 requires that we periodically send unsolicited RAs.
Linux as a guest also requires this: it will send an RS when a link first
comes up, but the route it gets from this will have a finite lifetime (we
set this to 65535s, the maximum allowed, around 18 hours). When that
expires the guest will not send a new RS, but instead expects the route to
have been renewed (if still valid) by an unsolicited RA.
Implement sending unsolicited RAs on a partially randomised timer, as
required by RFC 4861. The RFC also specifies that solicited RAs should
also be delayed, or even omitted, if the next unsolicited RA is soon
enough. For now we don't do that, always sending an immediate RA in
response to an RS. We can get away with this because in our use cases
we expect to just have passt itself and the guest on the link, rather than
a large broadcast domain.
Link: https://github.com/kubevirt/kubevirt/issues/13191
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
ip.h | 9 +++++++++
ndp.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
ndp.h | 3 +++
passt.c | 3 +++
4 files changed, 69 insertions(+)
diff --git a/ip.h b/ip.h
index b8d4a5b..0742612 100644
--- a/ip.h
+++ b/ip.h
@@ -92,4 +92,13 @@ struct ipv6_opt_hdr {
char *ipv6_l4hdr(const struct pool *p, int idx, size_t offset, uint8_t *proto,
size_t *dlen);
+
+/* IPv6 link-local all-nodes multicast adddress, ff02::1 */
+static const struct in6_addr in6addr_ll_all_nodes = {
+ .s6_addr = {
+ 0xff, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01,
+ },
+};
+
#endif /* IP_H */
diff --git a/ndp.c b/ndp.c
index 09df8d6..7ee44b2 100644
--- a/ndp.c
+++ b/ndp.c
@@ -372,3 +372,57 @@ int ndp(const struct ctx *c, const struct icmp6hdr *ih,
return 1;
}
+
+/* Default interval between unsolicited RAs (seconds) */
+#define DEFAULT_MAX_RTR_ADV_INTERVAL 600 /* RFC 4861, 6.2.1 */
+
+/* Minimum required interval between RAs (seconds) */
+#define MIN_DELAY_BETWEEN_RAS 3 /* RFC 4861, 10 */
+
+static time_t next_ra;
+
+/**
+ * ndp_timer() - Send unsolicited NDP messages if necessary
+ * @c: Execution context
+ * @now: Current (monotonic) time
+ */
+void ndp_timer(const struct ctx *c, const struct timespec *now)
+{
+ time_t max_rtr_adv_interval = DEFAULT_MAX_RTR_ADV_INTERVAL;
+ time_t min_rtr_adv_interval, interval;
+
+ if (c->no_ra || now->tv_sec < next_ra)
+ return;
+
+ /* We must advertise before the route's lifetime expires */
+ max_rtr_adv_interval = MIN(max_rtr_adv_interval, RT_LIFETIME - 1);
+
+ /* But we must not go smaller than the minimum delay */
+ max_rtr_adv_interval = MAX(max_rtr_adv_interval, MIN_DELAY_BETWEEN_RAS);
+
+ /* RFC 4861, 6.2.1 */
+ min_rtr_adv_interval = MAX(max_rtr_adv_interval / 3,
+ MIN_DELAY_BETWEEN_RAS);
+
+ /* As required by RFC 4861, we randomise the interval between
+ * unsolicited RAs. This is to prevent multiple routers on a link
+ * getting synchronised (e.g. after booting a bunch of routers at once)
+ * and causing flurries of RAs at the same time.
+ *
+ * This random doesn't need to be cryptographically strong, so random(3)
+ * is fine. Other routers on the link also want to avoid
+ * synchronisation, and anything malicious has much easier ways to cause
+ * trouble.
+ *
+ * The modulus also makes this not strictly a uniform distribution, but,
+ * again, it's close enough for our purposes.
+ */
+ interval = min_rtr_adv_interval +
+ random() % (max_rtr_adv_interval - min_rtr_adv_interval);
+
+ info("NDP: sending unsolicited RA, next in %llds", (long long)interval);
+
+ ndp_ra(c, &in6addr_ll_all_nodes);
+
+ next_ra = now->tv_sec + interval;
+}
diff --git a/ndp.h b/ndp.h
index abe6d02..41c2000 100644
--- a/ndp.h
+++ b/ndp.h
@@ -6,7 +6,10 @@
#ifndef NDP_H
#define NDP_H
+struct icmp6hdr;
+
int ndp(const struct ctx *c, const struct icmp6hdr *ih,
const struct in6_addr *saddr, const struct pool *p);
+void ndp_timer(const struct ctx *c, const struct timespec *now);
#endif /* NDP_H */
diff --git a/passt.c b/passt.c
index 83b26c5..a51a4e1 100644
--- a/passt.c
+++ b/passt.c
@@ -49,6 +49,7 @@
#include "arch.h"
#include "log.h"
#include "tcp_splice.h"
+#include "ndp.h"
#define EPOLL_EVENTS 8
@@ -107,6 +108,8 @@ static void post_handler(struct ctx *c, const struct timespec *now)
flow_defer_handler(c, now);
#undef CALL_PROTO_HANDLER
+
+ ndp_timer(c, now);
}
/**
--
@@ -49,6 +49,7 @@
#include "arch.h"
#include "log.h"
#include "tcp_splice.h"
+#include "ndp.h"
#define EPOLL_EVENTS 8
@@ -107,6 +108,8 @@ static void post_handler(struct ctx *c, const struct timespec *now)
flow_defer_handler(c, now);
#undef CALL_PROTO_HANDLER
+
+ ndp_timer(c, now);
}
/**
--
2.47.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v2 0/8] ndp: Unsolicited RAs
2024-11-14 3:33 [PATCH v2 0/8] ndp: Unsolicited RAs David Gibson
` (7 preceding siblings ...)
2024-11-14 3:33 ` [PATCH v2 8/8] ndp: Send unsolicited Router Advertisements David Gibson
@ 2024-11-14 18:18 ` Stefano Brivio
8 siblings, 0 replies; 10+ messages in thread
From: Stefano Brivio @ 2024-11-14 18:18 UTC (permalink / raw)
To: David Gibson; +Cc: passt-dev
On Thu, 14 Nov 2024 14:33:02 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:
> The default route a guest obtains via NDP has an expiry. We set this
> to the maximum allowed, 65535s (about 18 hours). We missed, however,
> that after that expiry, the guest won't send a new Router
> Solicitation. Instead it expects an unsolicited Router Advertisement
> to have come at some point before the expiry. This means that on an
> IPv6 setup, the default will disappear after 18 hours.
>
> Correct this by sending unsolicited Router Advertisements as required
> by RFC 4861. Along the way we make a number of small cleanups to the
> NDP code.
>
> Link: https://github.com/kubevirt/kubevirt/issues/13191
>
> v2:
> * Some minor stylistic fixes
> * Use srandom()
> * Better explanation of the random requirements for the RAs
Applied.
--
Stefano
^ permalink raw reply [flat|nested] 10+ messages in thread