On Fri, Jan 30, 2026 at 04:44:43PM -0500, Jon Maloy wrote: > We enable configuration of multiple IPv4 and IPv6 addresses by allowing > repeated use of the -a/--address option. > > - We update option parsing to append addresses to the unified addrs[] > array, with limit checks for IP4_MAX_ADDRS and IP6_MAX_ADDRS. I don't see any point to separate v4 and v6 limits, now there's a unified array. > - Each address specified via -a, but with no prefix length indicated, > gets a class-based default prefix length. That's not new, is it? I think we want to be careful with this. We need to maintain compatibility with previously working options, but I think we want to strongly encourage prefix lengths to be explicitly specified. Address classes are very anachronistic now, so I don't think we want to add any more uses of them than we have to for compatibility. > - If no -a option is given, addresses/prefix lengths are inherited from > the template interface. > - If a prefix length is to be added, it has to be done in CIDR format, > except for the very first address. I don't really follow what that means. > - We configure all indicated addresses in the namespace interface using > the for_each_addr() macro. > > Signed-off-by: Jon Maloy > > --- > v2: - Adapted to previous code changes > v3: - Adapted to single-array strategy > - Changes according to feedback from S. Brivio and G Gibson. > --- > conf.c | 23 ++++++++++++++++------- > pasta.c | 21 ++++++++++++++------- > 2 files changed, 30 insertions(+), 14 deletions(-) > > diff --git a/conf.c b/conf.c > index bb6bcf8..d73a3dd 100644 > --- a/conf.c > +++ b/conf.c > @@ -803,13 +803,13 @@ static unsigned int conf_ip6(unsigned int ifi, struct ctx *c) > } > > e = first_v6(c); > - c->ip6.addr_seen = e->addr.a6; > + if (e) > + c->ip6.addr_seen = e->addr.a6; This seems like it belongs in an earlier patch. > > if (IN6_IS_ADDR_LINKLOCAL(&c->ip6.guest_gw)) > c->ip6.our_tap_ll = c->ip6.guest_gw; > > - if (IN6_IS_ADDR_UNSPECIFIED(&e->addr.a6) || > - IN6_IS_ADDR_UNSPECIFIED(&c->ip6.our_tap_ll)) Missed this in an earlier patch, but !!e and an unspecified address shouldn't be possible, no? > + if (!count_v6(c) || IN6_IS_ADDR_UNSPECIFIED(&c->ip6.our_tap_ll)) > return 0; > > return ifi; > @@ -901,9 +901,11 @@ static void usage(const char *name, FILE *f, int status) > " default: 65520: maximum 802.3 MTU minus 802.3 header\n" > " length, rounded to 32 bits (IPv4 words)\n" > " -a, --address ADDR Assign IPv4 or IPv6 address ADDR[/PREFIXLEN]\n" > - " can be specified zero to two times (for IPv4 and IPv6)\n" > + " can be specified multiple times (limit: %d IPv4, %d IPv6)\n" > " default: use addresses from interface with default route\n" > - " -n, --netmask MASK Assign IPv4 MASK, dot-decimal or bits\n" > + " -n, --netmask MASK Assign IPv4 MASK, dot-decimal or bits\n", > + IP4_MAX_ADDRS, IP6_MAX_ADDRS); > + FPRINTF(f, > " default: netmask from matching address on the host\n" > " -M, --mac-addr ADDR Use source MAC address ADDR\n" > " default: 9a:55:9a:55:9a:55 (locally administered)\n" > @@ -1836,6 +1838,9 @@ void conf(struct ctx *c, int argc, char **argv) > die("Can't mix CIDR with -n"); > > if (af == AF_INET) { > + if (count_v4(c) >= IP4_MAX_ADDRS) > + die("Too many IPv4 addresses"); > + > e = &c->addrs[c->addr_count]; > e->addr = addr; > e->prefix_len = prefix_len ? prefix_len : > @@ -1845,6 +1850,9 @@ void conf(struct ctx *c, int argc, char **argv) > if (c->mode == MODE_PASTA) > c->ip4.no_copy_addrs = true; > } else if (af == AF_INET6) { > + if (count_v6(c) >= IP6_MAX_ADDRS) > + die("Too many IPv6 addresses"); > + > e = &c->addrs[c->addr_count]; > e->addr = addr; > e->prefix_len = prefix_len ? prefix_len : 64; > @@ -1861,6 +1869,8 @@ void conf(struct ctx *c, int argc, char **argv) > struct inany_addr_entry *e; > int plen; > > + if (count_v4(c) > 1) > + die("-n can only be used with first address"); > if (prefix_from_cidr) > die("Can't use both -n and CIDR prefix length"); > plen = conf_ip4_prefix(optarg); > @@ -2156,8 +2166,7 @@ void conf(struct ctx *c, int argc, char **argv) > if (!c->ifi6) { > c->no_ndp = 1; > c->no_dhcpv6 = 1; > - } else if (!first_v6(c) || > - IN6_IS_ADDR_UNSPECIFIED(&first_v6(c)->addr.a6)) { > + } else if (!count_v6(c)) { > c->no_dhcpv6 = 1; > } > > diff --git a/pasta.c b/pasta.c > index de0ba14..8cb5873 100644 > --- a/pasta.c > +++ b/pasta.c > @@ -312,10 +312,14 @@ static void pasta_ns_conf_ip4(struct ctx *c) > int rc = 0; > > if (c->ip4.no_copy_addrs) { > - struct inany_addr_entry *e = first_v4(c); > + const struct inany_addr_entry *e; > > - rc = nl_addr_set(nl_sock_ns, c->pasta_ifi, AF_INET, > - inany_v4(&e->addr), e->prefix_len - 96); > + for_each_addr(c, e, AF_INET) { > + rc = nl_addr_set(nl_sock_ns, c->pasta_ifi, AF_INET, > + inany_v4(&e->addr), e->prefix_len - 96); > + if (rc < 0) > + break; > + } As noted on the previous patch, I think a single pass through the array makes sense, only changing the inner part of the loop for v4 vs. v6 (we could potentially update nl_addr_set() to take an inany, which might make it cleaner). > } else { > rc = nl_addr_dup(nl_sock, c->ifi4, > nl_sock_ns, c->pasta_ifi, AF_INET); > @@ -346,7 +350,6 @@ static void pasta_ns_conf_ip4(struct ctx *c) > */ > static void pasta_ns_conf_ip6(struct ctx *c) > { > - struct inany_addr_entry *e; > int rc = 0; > > rc = nl_addr_get_ll(nl_sock_ns, c->pasta_ifi, &c->ip6.addr_ll_seen); > @@ -365,11 +368,15 @@ static void pasta_ns_conf_ip6(struct ctx *c) > nl_link_set_flags(nl_sock_ns, c->pasta_ifi, 0, IFF_NOARP); > > if (c->ip6.no_copy_addrs) { > - e = first_v6(c); > + const struct inany_addr_entry *e; > > - if (e && !IN6_IS_ADDR_UNSPECIFIED(&e->addr.a6)) { > + for_each_addr(c, e, AF_INET6) { > + if (IN6_IS_ADDR_UNSPECIFIED(&e->addr.a6)) > + continue; > rc = nl_addr_set(nl_sock_ns, c->pasta_ifi, > - AF_INET6, &e->addr.a6, 64); > + AF_INET6, &e->addr.a6, e->prefix_len); > + if (rc < 0) > + break; > } > } else { > rc = nl_addr_dup(nl_sock, c->ifi6, > -- > 2.52.0 > -- David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson