Re: [PATCH v4 02/14] conf, fwd: Keep a table of our port forwarding configuration

16 Jan 2026


      On Fri, 16 Jan 2026 11:20:43 +1100
David Gibson  wrote:
...
On Fri, Jan 16, 2026 at 12:01:27AM +0100, Stefano Brivio wrote:
...
On Thu, 15 Jan 2026 19:50:33 +1100
David Gibson  wrote:
...
@@ -313,6 +330,90 @@ bool fwd_port_is_ephemeral(in_port_t port)
  return (port >= fwd_ephemeral_min) && (port <= fwd_ephemeral_max);
 }
+/**
+ * fwd_rule_add() - Add a rule to a forwarding table
+ * @fwd:	Table to add to
+ * @flags:	Flags for this entry
+ * @addr:	Our address to forward (NULL for both 0.0.0.0 and ::)
+ * @ifname:	Only forward from this interface name, if non-empty
+ * @first:	First port number to forward
+ * @last:	Last port number to forward
+ * @to:		First port of target port range to map to
+ */
+void fwd_rule_add(struct fwd_ports *fwd, uint8_t flags,
+		  const union inany_addr *addr, const char *ifname,
+		  in_port_t first, in_port_t last, in_port_t to)
+{
+	/* Flags which can be set from the caller */
+	const uint8_t allowed_flags = FWD_WEAK;
+	struct fwd_rule *new;
+	unsigned port;
+
+	ASSERT(!(flags & ~allowed_flags));
+
+	if (fwd->count >= ARRAY_SIZE(fwd->rules))
+		die("Too many port forwarding ranges");
+
+	new = &fwd->rules[fwd->count++];
+	new->flags = flags;
+
+	if (addr) {
+		new->addr = *addr;
+	} else {
+		new->addr = inany_any6;
+		new->flags |= FWD_DUAL_STACK_ANY;
+	}
+
+	memset(new->ifname, 0, sizeof(new->ifname));
+	if (ifname) {
+		if (strlen(ifname) + 1 > sizeof(new->ifname))
+			die("Interface name %s is too long", ifname);
+		strncpy(new->ifname, ifname, sizeof(new->ifname));
+	}
This looks safe to me now, but:
/home/sbrivio/passt/fwd.c:394:3:
  Type: Buffer not null terminated (BUFFER_SIZE)  
[snip]
...perhaps worth switching to the usual snprintf() approach with return
check (see handling of c->ip4.ifname_out in conf()) and be done with it?
Good idea, not sure why it didn't occur to me earlier.
I've done that, and verified it fixes the coverity error (thanks for
resending the instructions for that).
...
I'd be slightly more confident if Coverity Scan didn't complain at all
(and happier without the noise, too).
Other than that, this version looks good to me. I would make a new
release just before merging it (with this "fixed") so that we can debug
things a bit more conveniently should something go wrong with it.
That sounds wise.  Do you want a new spin with the coverity fix?  Just
this patch?  Something else?
Yes, thanks, a respin would be nice, so that we have a reasonable
"permalink" to it, given it's a quite fundamental feature you're adding
and we might want to refer to the series as posted / applied.

-- 
Stefano

    