Nits only: On Tue, 12 Nov 2024 15:06:15 +1100 David Gibson <david(a)gibson.dropbear.id.au> wrote: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(a)gibson.dropbear.id.au> --- ndp.c | 136 +++++++++++++++++++++++++++++++++------------------------- 1 file changed, 78 insertions(+), 58 deletions(-) diff --git a/ndp.c b/ndp.c index fa1b67a..e876c34 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; @@ -305,7 +286,7 @@ int ndp(const struct ctx *c, const struct icmp6hdr *ih, sizeof(rdnss->dns[i])); } ptr += offsetof(struct opt_rdnss, dns) + - i * sizeof(rdnss->dns[0]); + i * sizeof(rdnss->dns[0]);Unrelated change, less readable, probably from your new editor / clangd?for (n = 0; *c->dns_search[n].n; n++) dns_s_len += strlen(c->dns_search[n].n) + 2; @@ -329,7 +310,7 @@ int ndp(const struct ctx *c, const struct icmp6hdr *ih, *(ptr++) = '.'; len = sizeof(dnssl->domains) - - (ptr - dnssl->domains); + (ptr - dnssl->domains);Same here.strncpy((char *)ptr, c->dns_search[i].n, len); for (dot = (char *)ptr - 1; *dot; dot++) { @@ -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 = + packet_get(p, 0, 0, sizeof(struct ndp_ns), NULL);The assignment could go on its line here? We don't save any line this way.+ + 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;-- Stefano