On Fri, Jul 10, 2026 at 05:31:23PM -0400, Jon Maloy wrote:
- if (!ip4->prefix_len) { - in_addr_t addr = ntohl(ip4->addr.s_addr); - if (IN_CLASSA(addr)) - ip4->prefix_len = (32 - IN_CLASSA_NSHIFT); - else if (IN_CLASSB(addr)) - ip4->prefix_len = (32 - IN_CLASSB_NSHIFT); - else if (IN_CLASSC(addr)) - ip4->prefix_len = (32 - IN_CLASSC_NSHIFT); - else - ip4->prefix_len = 32; + fwd_set_addr(c, &inany_from_v4(addr), CONF_ADDR_HOST, + prefix_len + 96); } - ip4->addr_seen = ip4->addr; - + a = fwd_get_addr(c, AF_INET, CONF_ADDR_ANY, 0);
Nit: maybe assert(a), since I doubt static checkers will be able to reason that the fwd_set_addr() above means this fwd_get_addr() can't fail. Also, I think that go inside the if block, since the other case means we already have a perfectly good a.
I think my conclusion was that the static checker ignores assert() as a conditonal test, but I can try it.
I'm not really sure what you mean by this. I don't know for certain with this particular case and each static checker. I do know that there are a *heap* of cases where an assert() can fix static checker warnings because it lets the checker know we can't have reached this point in the "bad" condition. [snip]
+/** + * struct guest_addr - Unified IPv4/IPv6 address entry + * @addr: IPv4 (as mapped) or IPv6 address + * @prefix_len: Prefix length in IPv6/IPv4-mapped [0,128]/[96,128] format + * @flags: CONF_ADDR_* flags + */ +struct guest_addr { + union inany_addr addr; + uint8_t prefix_len; + uint8_t flags; +#define CONF_ADDR_USER BIT(0) /* User set via -a */ +#define CONF_ADDR_HOST BIT(1) /* From host interface */ +#define CONF_ADDR_GENERATED BIT(2) /* Generated by PASST/PASTA */ +#define CONF_ADDR_LINKLOCAL BIT(3) /* Link-local address */ +#define CONF_ADDR_ANY 0xff /* Match any flag */
In general I like CONF_ADDR_ANY instead of 0 being special-cased. But with LINKLOCAL in the mix it doesn't quite work. ANY meaning "USER or HOST or GENERATED" makes sense. ANY meaning "USER or HOST or GENERATED or LINKLOCAL" is nonsense. I suggest you move LINKLOCAL into a different field, as Stefano has suggested to handle this (maybe "provenance" for USER/HOST/GENERATED and "flags" for LINKLOCAL?). Or you could remove the LINKLOCAL flag from the structure entirely: the information is already present in the address itself.
I was hoping to postpone this to a commit after this series, but I see your point. I can try with a 'scope' field, as suggested earlier, or just skip it altogheter and using an is_link_local() function.
Sorry, I thought there was already an inany_is_linklocal() function, but I see that there's only inany_is_linklocal6(). So that will need to be extended to handle IPv4 as well. [snip]
diff --git a/pasta.c b/pasta.c index 4e7ee542..9ef3ac00 100644 --- a/pasta.c +++ b/pasta.c @@ -330,6 +330,8 @@ void pasta_ns_conf(struct ctx *c) if (c->pasta_conf_ns) { unsigned int flags = IFF_UP; + const struct guest_addr *a; + int plen = 0; if (c->mtu) nl_link_set_mtu(nl_sock_ns, c->pasta_ifi, c->mtu); @@ -341,10 +343,17 @@ void pasta_ns_conf(struct ctx *c) if (c->ifi4) {
Nit: I think you can reduce the very deep nesting here, by moving the fwd_get_addr() and changing the if below to..
This is already done in a later patch, by introducing two subfuntions. However, this has caused lot of trouble at each rebase, so I think I will move it to a separate, standalone patch before this series.
Sounds like a good idea. -- 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