conf_ports() is extremely long, but we want to refactor it so that parts can be shared with the upcoming configuration client. Make a small start by separating out the section that parses just the port specification (not including address and/or interface). This also allows us to constify a few extra things, and while we're there replace a few vague error messages with more specific ones. Signed-off-by: David Gibson --- conf.c | 203 ++++++++++++++++++++++++++++++++------------------------- 1 file changed, 116 insertions(+), 87 deletions(-) diff --git a/conf.c b/conf.c index ae37bf96..c515480b 100644 --- a/conf.c +++ b/conf.c @@ -74,7 +74,7 @@ const char *pasta_default_ifn = "tap0"; * character *after* the delimiter, if no further @c is in @s, * return NULL */ -static char *next_chunk(const char *s, char c) +static const char *next_chunk(const char *s, char c) { char *sep = strchr(s, c); return sep ? sep + 1 : NULL; @@ -101,18 +101,19 @@ struct port_range { * Return: -EINVAL on parsing error, -ERANGE on out of range port * numbers, 0 on success */ -static int parse_port_range(const char *s, char **endptr, +static int parse_port_range(const char *s, const char **endptr, struct port_range *range) { unsigned long first, last; + char *ep; - last = first = strtoul(s, endptr, 10); - if (*endptr == s) /* Parsed nothing */ + last = first = strtoul(s, &ep, 10); + if (ep == s) /* Parsed nothing */ return -EINVAL; - if (**endptr == '-') { /* we have a last value too */ - const char *lasts = *endptr + 1; - last = strtoul(lasts, endptr, 10); - if (*endptr == lasts) /* Parsed nothing */ + if (*ep == '-') { /* we have a last value too */ + const char *lasts = ep + 1; + last = strtoul(lasts, &ep, 10); + if (ep == lasts) /* Parsed nothing */ return -EINVAL; } @@ -121,6 +122,7 @@ static int parse_port_range(const char *s, char **endptr, range->first = first; range->last = last; + *endptr = ep; return 0; } @@ -213,6 +215,101 @@ enum fwd_mode { FWD_MODE_ALL, }; +/** + * conf_ports_spec() - Parse port range(s) specifier + * @c: Execution context + * @optname: Short option name, t, T, u, or U + * @optarg: Option argument (port specification) + * @fwd: Forwarding table to be updated + * @addr: Listening address for forwarding + * @ifname: Interface name for listening + * @spec: Port range(s) specifier + */ +static void conf_ports_spec(const struct ctx *c, + char optname, const char *optarg, + struct fwd_table *fwd, + const union inany_addr *addr, const char *ifname, + const char *spec) +{ + uint8_t exclude[PORT_BITMAP_SIZE] = { 0 }; + bool exclude_only = true; + const char *p; + unsigned i; + + /* Mark all exclusions first, they might be given after base ranges */ + p = spec; + do { + struct port_range xrange; + + if (*p != '~') { + /* Not an exclude range, parse later */ + exclude_only = false; + continue; + } + p++; + + if (parse_port_range(p, &p, &xrange)) + goto bad; + if ((*p != '\0') && (*p != ',')) /* Garbage after the range */ + goto bad; + + for (i = xrange.first; i <= xrange.last; i++) + bitmap_set(exclude, i); + } while ((p = next_chunk(p, ','))); + + if (exclude_only) { + /* Exclude ephemeral ports */ + fwd_port_map_ephemeral(exclude); + + conf_ports_range_except(c, optname, optarg, fwd, + addr, ifname, + 1, NUM_PORTS - 1, exclude, + 1, FWD_WEAK); + return; + } + + /* Now process base ranges, skipping exclusions */ + p = spec; + do { + struct port_range orig_range, mapped_range; + + if (*p == '~') + /* Exclude range, already parsed */ + continue; + + if (parse_port_range(p, &p, &orig_range)) + goto bad; + + if (*p == ':') { /* There's a range to map to as well */ + if (parse_port_range(p + 1, &p, &mapped_range)) + goto bad; + if ((mapped_range.last - mapped_range.first) != + (orig_range.last - orig_range.first)) + goto bad; + } else { + mapped_range = orig_range; + } + + if ((*p != '\0') && (*p != ',')) /* Garbage after the ranges */ + goto bad; + + if (orig_range.first == 0) { + die("Can't forward port 0 for option '-%c %s'", + optname, optarg); + } + + conf_ports_range_except(c, optname, optarg, fwd, + addr, ifname, + orig_range.first, orig_range.last, + exclude, + mapped_range.first, 0); + } while ((p = next_chunk(p, ','))); + + return; +bad: + die("Invalid port specifier %s", optarg); +} + /** * conf_ports() - Parse port configuration options, initialise UDP/TCP sockets * @c: Execution context @@ -226,9 +323,6 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg, { union inany_addr addr_buf = inany_any6, *addr = &addr_buf; char buf[BUFSIZ], *spec, *ifname = NULL, *p; - uint8_t exclude[PORT_BITMAP_SIZE] = { 0 }; - bool exclude_only = true; - unsigned i; if (!strcmp(optarg, "none")) { if (*mode) @@ -255,6 +349,8 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg, } if (!strcmp(optarg, "all")) { + uint8_t exclude[PORT_BITMAP_SIZE] = { 0 }; + if (*mode) goto mode_conflict; @@ -285,7 +381,8 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg, spec++; if (optname != 't' && optname != 'u') - goto bad; + die("Listening address not allowed for -%c %s", + optname, optarg); if ((ifname = strchr(buf, '%'))) { *ifname = 0; @@ -295,9 +392,10 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg, * so the length of the given ifname is: * (spec - ifname - 1) */ - if (spec - ifname - 1 >= IFNAMSIZ) - goto bad; - + if (spec - ifname - 1 >= IFNAMSIZ) { + die("Interface name '%s' is too long (max %u)", + ifname, IFNAMSIZ - 1); + } } if (ifname == buf + 1) { /* Interface without address */ @@ -312,7 +410,7 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg, } if (!inany_pton(p, addr)) - goto bad; + die("Bad forwarding address '%s'", p); } } else { spec = buf; @@ -330,27 +428,6 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg, } } - /* Mark all exclusions first, they might be given after base ranges */ - p = spec; - do { - struct port_range xrange; - - if (*p != '~') { - /* Not an exclude range, parse later */ - exclude_only = false; - continue; - } - p++; - - if (parse_port_range(p, &p, &xrange)) - goto bad; - if ((*p != '\0') && (*p != ',')) /* Garbage after the range */ - goto bad; - - for (i = xrange.first; i <= xrange.last; i++) - bitmap_set(exclude, i); - } while ((p = next_chunk(p, ','))); - if (ifname && c->no_bindtodevice) { die( "Device binding for '-%c %s' unsupported (requires kernel 5.7+)", @@ -360,57 +437,9 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg, if ((optname == 'T' || optname == 'U') && !ifname) ifname = "lo"; - if (exclude_only) { - /* Exclude ephemeral ports */ - fwd_port_map_ephemeral(exclude); - - conf_ports_range_except(c, optname, optarg, fwd, - addr, ifname, - 1, NUM_PORTS - 1, exclude, - 1, FWD_WEAK); - return; - } - - /* Now process base ranges, skipping exclusions */ - p = spec; - do { - struct port_range orig_range, mapped_range; - - if (*p == '~') - /* Exclude range, already parsed */ - continue; - - if (parse_port_range(p, &p, &orig_range)) - goto bad; - - if (*p == ':') { /* There's a range to map to as well */ - if (parse_port_range(p + 1, &p, &mapped_range)) - goto bad; - if ((mapped_range.last - mapped_range.first) != - (orig_range.last - orig_range.first)) - goto bad; - } else { - mapped_range = orig_range; - } - - if ((*p != '\0') && (*p != ',')) /* Garbage after the ranges */ - goto bad; - - if (orig_range.first == 0) { - die("Can't forward port 0 for option '-%c %s'", - optname, optarg); - } - - conf_ports_range_except(c, optname, optarg, fwd, - addr, ifname, - orig_range.first, orig_range.last, - exclude, - mapped_range.first, 0); - } while ((p = next_chunk(p, ','))); - + conf_ports_spec(c, optname, optarg, fwd, addr, ifname, spec); return; -bad: - die("Invalid port specifier %s", optarg); + mode_conflict: die("Port forwarding mode '%s' conflicts with previous mode", optarg); } -- 2.53.0

For passt, the default forwarding mode is "none", which falls out naturally from the other handling: if we don't get any options, we get empty forwarding tables, which corresponds to "none" behaviour. However, for pasta the default is "auto". This is handled a bit oddly: in conf_ports() we set the mode variable, but don't set up the rules we need for "auto" mode. Instead we want until nearly the end of conf() and if the mode is FWD_MODE_AUTO or unset, we make conf_ports_range_except() calls to set up the "auto" rules. Simplify this a bit, by creating the rules within conf_ports() itself when we parse -[tuTU] auto. For the case of no forwarding options we call into conf_ports() itself with synthetic arguments. As well as making the code a little shorter, this makes it more obvious that giving no arguments really is equivalent to -[tuTU] auto. Signed-off-by: David Gibson --- conf.c | 54 ++++++++++++++++++++++-------------------------------- 1 file changed, 22 insertions(+), 32 deletions(-) diff --git a/conf.c b/conf.c index c515480b..7d718f91 100644 --- a/conf.c +++ b/conf.c @@ -345,6 +345,10 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg, die("'auto' port forwarding is only allowed for pasta"); *mode = FWD_MODE_AUTO; + + conf_ports_range_except(c, optname, optarg, fwd, NULL, NULL, + 1, NUM_PORTS - 1, NULL, 1, FWD_SCAN); + return; } @@ -1576,7 +1580,6 @@ void conf(struct ctx *c, int argc, char **argv) enum fwd_mode udp_out_mode = FWD_MODE_UNSET; enum fwd_mode tcp_in_mode = FWD_MODE_UNSET; enum fwd_mode udp_in_mode = FWD_MODE_UNSET; - enum fwd_mode fwd_default = FWD_MODE_NONE; bool v4_only = false, v6_only = false; unsigned dns4_idx = 0, dns6_idx = 0; unsigned long max_mtu = IP_MAX_MTU; @@ -1593,10 +1596,8 @@ void conf(struct ctx *c, int argc, char **argv) gid_t gid; - if (c->mode == MODE_PASTA) { + if (c->mode == MODE_PASTA) c->no_dhcp_dns = c->no_dhcp_dns_search = 1; - fwd_default = FWD_MODE_AUTO; - } if (tap_l2_max_len(c) - ETH_HLEN < max_mtu) max_mtu = tap_l2_max_len(c) - ETH_HLEN; @@ -2244,34 +2245,23 @@ void conf(struct ctx *c, int argc, char **argv) if_indextoname(c->ifi6, c->pasta_ifn); } - if (!tcp_in_mode) - tcp_in_mode = fwd_default; - if (!tcp_out_mode) - tcp_out_mode = fwd_default; - if (!udp_in_mode) - udp_in_mode = fwd_default; - if (!udp_out_mode) - udp_out_mode = fwd_default; - - if (tcp_in_mode == FWD_MODE_AUTO) { - conf_ports_range_except(c, 't', "auto", c->fwd[PIF_HOST], - NULL, NULL, 1, NUM_PORTS - 1, NULL, 1, - FWD_SCAN); - } - if (tcp_out_mode == FWD_MODE_AUTO) { - conf_ports_range_except(c, 'T', "auto", c->fwd[PIF_SPLICE], - NULL, "lo", 1, NUM_PORTS - 1, NULL, 1, - FWD_SCAN); - } - if (udp_in_mode == FWD_MODE_AUTO) { - conf_ports_range_except(c, 'u', "auto", c->fwd[PIF_HOST], - NULL, NULL, 1, NUM_PORTS - 1, NULL, 1, - FWD_SCAN); - } - if (udp_out_mode == FWD_MODE_AUTO) { - conf_ports_range_except(c, 'U', "auto", c->fwd[PIF_SPLICE], - NULL, "lo", 1, NUM_PORTS - 1, NULL, 1, - FWD_SCAN); + if (c->mode == MODE_PASTA) { + if (!tcp_in_mode) { + conf_ports(c, 't', "auto", + c->fwd[PIF_HOST], &tcp_in_mode); + } + if (!tcp_out_mode) { + conf_ports(c, 'T', "auto", + c->fwd[PIF_SPLICE], &tcp_out_mode); + } + if (!udp_in_mode) { + conf_ports(c, 'u', "auto", + c->fwd[PIF_HOST], &udp_in_mode); + } + if (!udp_out_mode) { + conf_ports(c, 'U', "auto", + c->fwd[PIF_SPLICE], &udp_out_mode); + } } if (!c->quiet) -- 2.53.0

6dad076df037 ("fwd: Split forwarding rule specification from its implementation state") created struct fwd_rule_state with a forwarding rule plus the table of sockets used for its implementation. It turns out this is quite awkward for sharing rule parsing code between passt and the upcoming configuration client. Instead keep the index of listening sockets in a parallel array in struct fwd_table. Signed-off-by: David Gibson --- fwd.c | 73 ++++++++++++++++++++++++++++++----------------------------- fwd.h | 20 ++++++---------- 2 files changed, 44 insertions(+), 49 deletions(-) diff --git a/fwd.c b/fwd.c index e09b42fe..7e0edc38 100644 --- a/fwd.c +++ b/fwd.c @@ -363,7 +363,7 @@ void fwd_rule_add(struct fwd_table *fwd, uint8_t proto, uint8_t flags, /* Flags which can be set from the caller */ const uint8_t allowed_flags = FWD_WEAK | FWD_SCAN | FWD_DUAL_STACK_ANY; unsigned num = (unsigned)last - first + 1; - struct fwd_rule_state *new; + struct fwd_rule *new; unsigned i, port; assert(!(flags & ~allowed_flags)); @@ -379,7 +379,7 @@ void fwd_rule_add(struct fwd_table *fwd, uint8_t proto, uint8_t flags, /* Check for any conflicting entries */ for (i = 0; i < fwd->count; i++) { char newstr[INANY_ADDRSTRLEN], rulestr[INANY_ADDRSTRLEN]; - const struct fwd_rule *rule = &fwd->rules[i].rule; + const struct fwd_rule *rule = &fwd->rules[i]; if (proto != rule->proto) /* Non-conflicting protocols */ @@ -399,38 +399,38 @@ void fwd_rule_add(struct fwd_table *fwd, uint8_t proto, uint8_t flags, rule->first, rule->last); } - new = &fwd->rules[fwd->count++]; - new->rule.proto = proto; - new->rule.flags = flags; + new = &fwd->rules[fwd->count]; + new->proto = proto; + new->flags = flags; if (addr) { - new->rule.addr = *addr; + new->addr = *addr; } else { - new->rule.addr = inany_any6; - new->rule.flags |= FWD_DUAL_STACK_ANY; + new->addr = inany_any6; + new->flags |= FWD_DUAL_STACK_ANY; } - memset(new->rule.ifname, 0, sizeof(new->rule.ifname)); + memset(new->ifname, 0, sizeof(new->ifname)); if (ifname) { int ret; - ret = snprintf(new->rule.ifname, sizeof(new->rule.ifname), + ret = snprintf(new->ifname, sizeof(new->ifname), "%s", ifname); - if (ret <= 0 || (size_t)ret >= sizeof(new->rule.ifname)) + if (ret <= 0 || (size_t)ret >= sizeof(new->ifname)) die("Invalid interface name: %s", ifname); } assert(first <= last); - new->rule.first = first; - new->rule.last = last; + new->first = first; + new->last = last; + new->to = to; - new->rule.to = to; + fwd->rulesocks[fwd->count] = &fwd->socks[fwd->sock_count]; + for (port = new->first; port <= new->last; port++) + fwd->rulesocks[fwd->count][port - new->first] = -1; - new->socks = &fwd->socks[fwd->sock_count]; + fwd->count++; fwd->sock_count += num; - - for (port = new->rule.first; port <= new->rule.last; port++) - new->socks[port - new->rule.first] = -1; } /** @@ -466,7 +466,7 @@ const struct fwd_rule *fwd_rule_search(const struct fwd_table *fwd, if (hint >= 0) { char ostr[INANY_ADDRSTRLEN], rstr[INANY_ADDRSTRLEN]; - const struct fwd_rule *rule = &fwd->rules[hint].rule; + const struct fwd_rule *rule = &fwd->rules[hint]; assert((unsigned)hint < fwd->count); if (fwd_rule_match(rule, ini, proto)) @@ -480,8 +480,8 @@ const struct fwd_rule *fwd_rule_search(const struct fwd_table *fwd, } for (i = 0; i < fwd->count; i++) { - if (fwd_rule_match(&fwd->rules[i].rule, ini, proto)) - return &fwd->rules[i].rule; + if (fwd_rule_match(&fwd->rules[i], ini, proto)) + return &fwd->rules[i]; } return NULL; @@ -496,7 +496,7 @@ void fwd_rules_print(const struct fwd_table *fwd) unsigned i; for (i = 0; i < fwd->count; i++) { - const struct fwd_rule *rule = &fwd->rules[i].rule; + const struct fwd_rule *rule = &fwd->rules[i]; const char *percent = *rule->ifname ? "%" : ""; const char *weak = "", *scan = ""; char addr[INANY_ADDRSTRLEN]; @@ -533,9 +533,9 @@ void fwd_rules_print(const struct fwd_table *fwd) static int fwd_sync_one(const struct ctx *c, uint8_t pif, unsigned idx, const uint8_t *tcp, const uint8_t *udp) { - const struct fwd_rule_state *rs = &c->fwd[pif]->rules[idx]; - const struct fwd_rule *rule = &rs->rule; + const struct fwd_rule *rule = &c->fwd[pif]->rules[idx]; const union inany_addr *addr = fwd_rule_addr(rule); + int *socks = c->fwd[pif]->rulesocks[idx]; const char *ifname = rule->ifname; const uint8_t *map = NULL; bool bound_one = false; @@ -555,7 +555,7 @@ static int fwd_sync_one(const struct ctx *c, uint8_t pif, unsigned idx, } for (port = rule->first; port <= rule->last; port++) { - int fd = rs->socks[port - rule->first]; + int fd = socks[port - rule->first]; if (map && !bitmap_isset(map, port)) { /* We don't want to listen on this port */ @@ -563,7 +563,7 @@ static int fwd_sync_one(const struct ctx *c, uint8_t pif, unsigned idx, /* We already are, so stop */ epoll_del(c->epollfd, fd); close(fd); - rs->socks[port - rule->first] = -1; + socks[port - rule->first] = -1; } continue; } @@ -595,7 +595,7 @@ static int fwd_sync_one(const struct ctx *c, uint8_t pif, unsigned idx, continue; } - rs->socks[port - rule->first] = fd; + socks[port - rule->first] = fd; bound_one = true; } @@ -685,11 +685,12 @@ void fwd_listen_close(const struct fwd_table *fwd) unsigned i; for (i = 0; i < fwd->count; i++) { - const struct fwd_rule_state *rs = &fwd->rules[i]; + const struct fwd_rule *rule = &fwd->rules[i]; + int *socks = fwd->rulesocks[i]; unsigned port; - for (port = rs->rule.first; port <= rs->rule.last; port++) { - int *fdp = &rs->socks[port - rs->rule.first]; + for (port = rule->first; port <= rule->last; port++) { + int *fdp = &socks[port - rule->first]; if (*fdp >= 0) { close(*fdp); *fdp = -1; @@ -769,8 +770,8 @@ static bool has_scan_rules(const struct fwd_table *fwd, uint8_t proto) unsigned i; for (i = 0; i < fwd->count; i++) { - if (fwd->rules[i].rule.proto == proto && - fwd->rules[i].rule.flags & FWD_SCAN) + if (fwd->rules[i].proto == proto && + fwd->rules[i].flags & FWD_SCAN) return true; } return false; @@ -838,14 +839,14 @@ static void current_listen_map(uint8_t *map, const struct fwd_table *fwd, memset(map, 0, PORT_BITMAP_SIZE); for (i = 0; i < fwd->count; i++) { - const struct fwd_rule_state *rs = &fwd->rules[i]; + const struct fwd_rule *rule = &fwd->rules[i]; unsigned port; - if (rs->rule.proto != proto) + if (rule->proto != proto) continue; - for (port = rs->rule.first; port <= rs->rule.last; port++) { - if (rs->socks[port - rs->rule.first] >= 0) + for (port = rule->first; port <= rule->last; port++) { + if (fwd->rulesocks[i][port - rule->first] >= 0) bitmap_set(map, port); } } diff --git a/fwd.h b/fwd.h index 33600cbf..7e9ec49e 100644 --- a/fwd.h +++ b/fwd.h @@ -26,16 +26,6 @@ struct flowside; void fwd_probe_ephemeral(void); void fwd_port_map_ephemeral(uint8_t *map); -/** - * struct fwd_rule_state - Forwarding rule and associated state - * @rule: Rule specification - * @socks: Array of listening sockets for this entry - */ -struct fwd_rule_state { - struct fwd_rule rule; - int *socks; -}; - #define FWD_RULE_BITS 8 #define MAX_FWD_RULES MAX_FROM_BITS(FWD_RULE_BITS) #define FWD_NO_HINT (-1) @@ -61,15 +51,19 @@ struct fwd_listen_ref { #define MAX_LISTEN_SOCKS (NUM_PORTS * 5) /** - * struct fwd_table - Table of forwarding rules (per initiating pif) + * struct fwd_table - Forwarding state (per initiating pif) * @count: Number of forwarding rules * @rules: Array of forwarding rules - * @sock_count: Number of entries used in @socks + * @rulesocks: Parallel array of @rules (@count valid entries) of pointers to + * @socks entries giving the start of the corresponding rule's + * sockets within the larger array + * @sock_count: Number of entries used in @socks (for all rules combined) * @socks: Listening sockets for forwarding */ struct fwd_table { unsigned count; - struct fwd_rule_state rules[MAX_FWD_RULES]; + struct fwd_rule rules[MAX_FWD_RULES]; + int *rulesocks[MAX_FWD_RULES]; unsigned sock_count; int socks[MAX_LISTEN_SOCKS]; }; -- 2.53.0

In order to be shared with the upcoming configuration client, we want to split code which deals with forwarding rules as standalone objects from those which deal with forwarding rules as they're actually used to implement forwarding in passt/pasta. Create fwd_rule.c to contain code from the first category, and start off by moving code to format rules into text for human display into it. While we're at it, we rework that formatting code a little to make it more flexible. Signed-off-by: David Gibson --- Makefile | 10 +++--- conf.c | 2 +- fwd.c | 48 --------------------------- fwd.h | 1 - fwd_rule.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ fwd_rule.h | 12 +++++++ 6 files changed, 113 insertions(+), 55 deletions(-) create mode 100644 fwd_rule.c diff --git a/Makefile b/Makefile index dd647b10..f697c12b 100644 --- a/Makefile +++ b/Makefile @@ -38,11 +38,11 @@ FLAGS += -DVERSION=\"$(VERSION)\" FLAGS += -DDUAL_STACK_SOCKETS=$(DUAL_STACK_SOCKETS) PASST_SRCS = arch.c arp.c bitmap.c checksum.c conf.c dhcp.c dhcpv6.c \ - epoll_ctl.c flow.c fwd.c icmp.c igmp.c inany.c iov.c ip.c isolation.c \ - lineread.c log.c mld.c ndp.c netlink.c migrate.c packet.c passt.c \ - pasta.c pcap.c pif.c repair.c serialise.c tap.c tcp.c tcp_buf.c \ - tcp_splice.c tcp_vu.c udp.c udp_flow.c udp_vu.c util.c vhost_user.c \ - virtio.c vu_common.c + epoll_ctl.c flow.c fwd.c fwd_rule.c icmp.c igmp.c inany.c iov.c ip.c \ + isolation.c lineread.c log.c mld.c ndp.c netlink.c migrate.c packet.c \ + passt.c pasta.c pcap.c pif.c repair.c serialise.c tap.c tcp.c \ + tcp_buf.c tcp_splice.c tcp_vu.c udp.c udp_flow.c udp_vu.c util.c \ + vhost_user.c virtio.c vu_common.c QRAP_SRCS = qrap.c PASST_REPAIR_SRCS = passt-repair.c SRCS = $(PASST_SRCS) $(QRAP_SRCS) $(PASST_REPAIR_SRCS) diff --git a/conf.c b/conf.c index 15044f3c..318bb915 100644 --- a/conf.c +++ b/conf.c @@ -1269,7 +1269,7 @@ dns6: dir = "Inbound"; info("%s forwarding rules (%s):", dir, pif_name(i)); - fwd_rules_print(c->fwd[i]); + fwd_rules_info(c->fwd[i]->rules, c->fwd[i]->count); } } diff --git a/fwd.c b/fwd.c index 7e0edc38..39a14c40 100644 --- a/fwd.c +++ b/fwd.c @@ -304,20 +304,6 @@ parse_err: warn("Unable to parse %s", PORT_RANGE_SYSCTL); } -/** - * fwd_rule_addr() - Return match address for a rule - * @rule: Forwarding rule - * - * Return: matching address for rule, NULL if it matches all addresses - */ -static const union inany_addr *fwd_rule_addr(const struct fwd_rule *rule) -{ - if (rule->flags & FWD_DUAL_STACK_ANY) - return NULL; - - return &rule->addr; -} - /** * fwd_port_map_ephemeral() - Mark ephemeral ports in a bitmap * @map: Bitmap to update @@ -487,40 +473,6 @@ const struct fwd_rule *fwd_rule_search(const struct fwd_table *fwd, return NULL; } -/** - * fwd_rules_print() - Print forwarding rules for debugging - * @fwd: Table to print - */ -void fwd_rules_print(const struct fwd_table *fwd) -{ - unsigned i; - - for (i = 0; i < fwd->count; i++) { - const struct fwd_rule *rule = &fwd->rules[i]; - const char *percent = *rule->ifname ? "%" : ""; - const char *weak = "", *scan = ""; - char addr[INANY_ADDRSTRLEN]; - - inany_ntop(fwd_rule_addr(rule), addr, sizeof(addr)); - if (rule->flags & FWD_WEAK) - weak = " (best effort)"; - if (rule->flags & FWD_SCAN) - scan = " (auto-scan)"; - - if (rule->first == rule->last) { - info(" %s [%s]%s%s:%hu => %hu %s%s", - ipproto_name(rule->proto), addr, percent, - rule->ifname, rule->first, rule->to, weak, scan); - } else { - info(" %s [%s]%s%s:%hu-%hu => %hu-%hu %s%s", - ipproto_name(rule->proto), addr, percent, - rule->ifname, rule->first, rule->last, - rule->to, rule->last - rule->first + rule->to, - weak, scan); - } - } -} - /** fwd_sync_one() - Create or remove listening sockets for a forward entry * @c: Execution context * @pif: Interface to create listening sockets for diff --git a/fwd.h b/fwd.h index 7e9ec49e..805fabd0 100644 --- a/fwd.h +++ b/fwd.h @@ -91,7 +91,6 @@ void fwd_rule_add(struct fwd_table *fwd, uint8_t proto, uint8_t flags, const struct fwd_rule *fwd_rule_search(const struct fwd_table *fwd, const struct flowside *ini, uint8_t proto, int hint); -void fwd_rules_print(const struct fwd_table *fwd); void fwd_scan_ports_init(struct ctx *c); void fwd_scan_ports_timer(struct ctx * c, const struct timespec *now); diff --git a/fwd_rule.c b/fwd_rule.c new file mode 100644 index 00000000..a034d5d1 --- /dev/null +++ b/fwd_rule.c @@ -0,0 +1,95 @@ +// SPDX-License-Identifier: GPL-2.0-or-later + +/* PASST - Plug A Simple Socket Transport + * for qemu/UNIX domain socket mode + * + * PASTA - Pack A Subtle Tap Abstraction + * for network namespace/tap device mode + * + * PESTO - Programmable Extensible Socket Translation Orchestrator + * front-end for passt(1) and pasta(1) forwarding configuration + * + * fwd_rule.c - Helpers for working with forwarding rule specifications + * + * Copyright Red Hat + * Author: David Gibson + */ + +#include + +#include "fwd_rule.h" + +/** + * fwd_rule_addr() - Return match address for a rule + * @rule: Forwarding rule + * + * Return: matching address for rule, NULL if it matches all addresses + */ +const union inany_addr *fwd_rule_addr(const struct fwd_rule *rule) +{ + if (rule->flags & FWD_DUAL_STACK_ANY) + return NULL; + + return &rule->addr; +} + +/** + * fwd_rule_fmt() - Prettily format forwarding rule as a string + * @rule: Rule to format + * @dst: Buffer to store output (should have FWD_RULE_STRLEN bytes) + * @size: Size of @dst + */ +#if defined(__GNUC__) && __GNUC__ < 15 +/* Workaround bug in gcc 12, 13 & 14 (at least) which gives a false positive + * -Wformat-overflow message if this function is inlined. + */ +__attribute__((noinline)) +#endif +static const char *fwd_rule_fmt(const struct fwd_rule *rule, + char *dst, size_t size) +{ + const char *percent = *rule->ifname ? "%" : ""; + const char *weak = "", *scan = ""; + char addr[INANY_ADDRSTRLEN]; + int len; + + inany_ntop(fwd_rule_addr(rule), addr, sizeof(addr)); + if (rule->flags & FWD_WEAK) + weak = " (best effort)"; + if (rule->flags & FWD_SCAN) + scan = " (auto-scan)"; + + if (rule->first == rule->last) { + len = snprintf(dst, size, + "%s [%s]%s%s:%hu => %hu %s%s", + ipproto_name(rule->proto), addr, percent, + rule->ifname, rule->first, rule->to, weak, scan); + } else { + in_port_t tolast = rule->last - rule->first + rule->to; + len = snprintf(dst, size, + "%s [%s]%s%s:%hu-%hu => %hu-%hu %s%s", + ipproto_name(rule->proto), addr, percent, + rule->ifname, rule->first, rule->last, + rule->to, tolast, weak, scan); + } + + if (len < 0 || (size_t)len >= size) + return NULL; + + return dst; +} + +/** + * fwd_rules_info() - Print forwarding rules for debugging + * @fwd: Table to print + */ +void fwd_rules_info(const struct fwd_rule *rules, size_t count) +{ + unsigned i; + + for (i = 0; i < count; i++) { + char buf[FWD_RULE_STRLEN]; + + info(" %s", fwd_rule_fmt(&rules[i], buf, sizeof(buf))); + } +} diff --git a/fwd_rule.h b/fwd_rule.h index ae0e1d61..e92efb6d 100644 --- a/fwd_rule.h +++ b/fwd_rule.h @@ -13,7 +13,9 @@ #include #include +#include "ip.h" #include "inany.h" +#include "bitmap.h" /** * struct fwd_rule - Forwarding rule governing a range of ports @@ -41,4 +43,14 @@ struct fwd_rule { uint8_t flags; }; +#define FWD_RULE_STRLEN \ + (IPPROTO_STRLEN - 1 \ + + INANY_ADDRSTRLEN - 1 \ + + IFNAMSIZ - 1 \ + + 4 * (UINT16_STRLEN - 1) \ + + sizeof(" []%:- => - (best effort) (auto-scan)")) + +const union inany_addr *fwd_rule_addr(const struct fwd_rule *rule); +void fwd_rules_info(const struct fwd_rule *rules, size_t count); + #endif /* FWD_RULE_H */ -- 2.53.0

Currently conf_ports_range_except() deduces the protocol to use from the option name. This is correct, but a DRY violation, since we check the option name at several points in the callchain. Instead pass the protocol explicitly to conf_ports_range_except() and conf_ports_spec(), computing it from the option name in conf_ports(). This is redundant for now, but means the optname and optarg parameters to the lower-level functions are used only for debugging output and will be removable in future. Signed-off-by: David Gibson --- conf.c | 37 ++++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/conf.c b/conf.c index 318bb915..a0cf9454 100644 --- a/conf.c +++ b/conf.c @@ -134,6 +134,7 @@ static int parse_port_range(const char *s, const char **endptr, * @optname: Short option name, t, T, u, or U * @optarg: Option argument (port specification) * @fwd: Forwarding table to be updated + * @proto: Protocol to forward * @addr: Listening address * @ifname: Listening interface * @first: First port to forward @@ -144,7 +145,7 @@ static int parse_port_range(const char *s, const char **endptr, */ static void conf_ports_range_except(const struct ctx *c, char optname, const char *optarg, struct fwd_table *fwd, - const union inany_addr *addr, + uint8_t proto, const union inany_addr *addr, const char *ifname, uint16_t first, uint16_t last, const uint8_t *exclude, uint16_t to, @@ -152,17 +153,9 @@ static void conf_ports_range_except(const struct ctx *c, char optname, { unsigned delta = to - first; unsigned base, i; - uint8_t proto; assert(first != 0); - if (optname == 't' || optname == 'T') - proto = IPPROTO_TCP; - else if (optname == 'u' || optname == 'U') - proto = IPPROTO_UDP; - else - assert(0); - for (base = first; base <= last; base++) { if (exclude && bitmap_isset(exclude, base)) continue; @@ -221,13 +214,14 @@ enum fwd_mode { * @optname: Short option name, t, T, u, or U * @optarg: Option argument (port specification) * @fwd: Forwarding table to be updated + * @proto: Protocol to forward * @addr: Listening address for forwarding * @ifname: Interface name for listening * @spec: Port range(s) specifier */ static void conf_ports_spec(const struct ctx *c, char optname, const char *optarg, - struct fwd_table *fwd, + struct fwd_table *fwd, uint8_t proto, const union inany_addr *addr, const char *ifname, const char *spec) { @@ -262,7 +256,7 @@ static void conf_ports_spec(const struct ctx *c, fwd_port_map_ephemeral(exclude); conf_ports_range_except(c, optname, optarg, fwd, - addr, ifname, + proto, addr, ifname, 1, NUM_PORTS - 1, exclude, 1, FWD_WEAK); return; @@ -299,7 +293,7 @@ static void conf_ports_spec(const struct ctx *c, } conf_ports_range_except(c, optname, optarg, fwd, - addr, ifname, + proto, addr, ifname, orig_range.first, orig_range.last, exclude, mapped_range.first, 0); @@ -323,6 +317,14 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg, { union inany_addr addr_buf = inany_any6, *addr = &addr_buf; char buf[BUFSIZ], *spec, *ifname = NULL, *p; + uint8_t proto; + + if (optname == 't' || optname == 'T') + proto = IPPROTO_TCP; + else if (optname == 'u' || optname == 'U') + proto = IPPROTO_UDP; + else + assert(0); if (!strcmp(optarg, "none")) { if (*mode) @@ -332,9 +334,9 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg, return; } - if ((optname == 't' || optname == 'T') && c->no_tcp) + if (proto == IPPROTO_TCP && c->no_tcp) die("TCP port forwarding requested but TCP is disabled"); - if ((optname == 'u' || optname == 'U') && c->no_udp) + if (proto == IPPROTO_UDP && c->no_udp) die("UDP port forwarding requested but UDP is disabled"); if (!strcmp(optarg, "auto")) { @@ -346,7 +348,8 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg, *mode = FWD_MODE_AUTO; - conf_ports_range_except(c, optname, optarg, fwd, NULL, NULL, + conf_ports_range_except(c, optname, optarg, fwd, + proto, NULL, NULL, 1, NUM_PORTS - 1, NULL, 1, FWD_SCAN); return; @@ -364,7 +367,7 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg, fwd_port_map_ephemeral(exclude); conf_ports_range_except(c, optname, optarg, fwd, - NULL, NULL, + proto, NULL, NULL, 1, NUM_PORTS - 1, exclude, 1, FWD_WEAK); return; @@ -438,7 +441,7 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg, if ((optname == 'T' || optname == 'U') && !ifname) ifname = "lo"; - conf_ports_spec(c, optname, optarg, fwd, addr, ifname, spec); + conf_ports_spec(c, optname, optarg, fwd, proto, addr, ifname, spec); return; mode_conflict: -- 2.53.0

After parsing port ranges conf_ports_spec() checks if we've reached a chunk delimiter (',') to verify that there isn't extra garbage there. Rework how we do this to use the recently introduced chunk-end pointer. This has two advantages: 1) Small, but practical: we don't need to repeat what the valid delimiters are, that's already handled in the chunk splitting code. 2) Large, if theoretical: this will also give an error if port parsing overruns a chunk boundary. We don't really expect that to happen, but it would be very confusing if it did. strtoul(3), on which parse_port_range() is based does say it may accept thousands separators based on locale which means we can't be sure it will only accept strings of digits. Signed-off-by: David Gibson --- conf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/conf.c b/conf.c index 0bc74e95..c3655824 100644 --- a/conf.c +++ b/conf.c @@ -264,7 +264,7 @@ static void conf_ports_spec(const struct ctx *c, if (parse_port_range(p, &p, &xrange)) goto bad; - if ((*p != '\0') && (*p != ',')) /* Garbage after the range */ + if (p != ep) /* Garbage after the range */ goto bad; for (i = xrange.first; i <= xrange.last; i++) @@ -303,7 +303,7 @@ static void conf_ports_spec(const struct ctx *c, mapped_range = orig_range; } - if ((*p != '\0') && (*p != ',')) /* Garbage after the ranges */ + if (p != ep) /* Garbage after the ranges */ goto bad; if (orig_range.first == 0) { -- 2.53.0

Currently fwd_rule_add() both builds the struct fwd_rule and inserts it into the table. This will be inconvenient when we want to dynamically add rules from a configuration client. Alter fwd_rule_add() to take a pre-constructed struct fwd_rule, which we build in the caller. Signed-off-by: David Gibson --- conf.c | 42 ++++++++++++++++++++++++++++++++-------- fwd.c | 61 ++++++++++++++-------------------------------------------- fwd.h | 4 +--- 3 files changed, 49 insertions(+), 58 deletions(-) diff --git a/conf.c b/conf.c index a0cf9454..027bbac9 100644 --- a/conf.c +++ b/conf.c @@ -151,9 +151,26 @@ static void conf_ports_range_except(const struct ctx *c, char optname, const uint8_t *exclude, uint16_t to, uint8_t flags) { + struct fwd_rule rule = { + .addr = addr ? *addr : inany_any6, + .ifname = { 0 }, + .proto = proto, + .flags = flags, + }; unsigned delta = to - first; unsigned base, i; + if (!addr) + rule.flags |= FWD_DUAL_STACK_ANY; + if (ifname) { + int ret; + + ret = snprintf(rule.ifname, sizeof(rule.ifname), + "%s", ifname); + if (ret <= 0 || (size_t)ret >= sizeof(rule.ifname)) + die("Invalid interface name: %s", ifname); + } + assert(first != 0); for (base = first; base <= last; base++) { @@ -165,28 +182,37 @@ static void conf_ports_range_except(const struct ctx *c, char optname, break; } + rule.first = base; + rule.last = i - 1; + rule.to = base + delta; + if ((optname == 'T' || optname == 'U') && c->no_bindtodevice) { /* FIXME: Once the fwd bitmaps are removed, move this * workaround to the caller */ + struct fwd_rule rulev = { + .ifname = { 0 }, + .flags = flags, + .first = base, + .last = i - 1, + .to = base + delta, + }; + assert(!addr && ifname && !strcmp(ifname, "lo")); warn( "SO_BINDTODEVICE unavailable, forwarding only 127.0.0.1 and ::1 for '-%c %s'", optname, optarg); if (c->ifi4) { - fwd_rule_add(fwd, proto, flags, - &inany_loopback4, NULL, - base, i - 1, base + delta); + rulev.addr = inany_loopback4; + fwd_rule_add(fwd, &rulev); } if (c->ifi6) { - fwd_rule_add(fwd, proto, flags, - &inany_loopback6, NULL, - base, i - 1, base + delta); + rulev.addr = inany_loopback6; + fwd_rule_add(fwd, &rulev); } } else { - fwd_rule_add(fwd, proto, flags, addr, ifname, - base, i - 1, base + delta); + fwd_rule_add(fwd, &rule); } base = i - 1; } diff --git a/fwd.c b/fwd.c index 39a14c40..c05107d1 100644 --- a/fwd.c +++ b/fwd.c @@ -332,30 +332,22 @@ void fwd_rule_init(struct ctx *c) } /** - * fwd_rule_add() - Add a rule to a forwarding table + * fwd_rule_add() - Validate and add a rule to a forwarding table * @fwd: Table to add to - * @proto: Protocol to forward - * @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 + * @new: Rule to add */ -void fwd_rule_add(struct fwd_table *fwd, uint8_t proto, uint8_t flags, - const union inany_addr *addr, const char *ifname, - in_port_t first, in_port_t last, in_port_t to) +void fwd_rule_add(struct fwd_table *fwd, const struct fwd_rule *new) { /* Flags which can be set from the caller */ const uint8_t allowed_flags = FWD_WEAK | FWD_SCAN | FWD_DUAL_STACK_ANY; - unsigned num = (unsigned)last - first + 1; - struct fwd_rule *new; + unsigned num = (unsigned)new->last - new->first + 1; unsigned i, port; - assert(!(flags & ~allowed_flags)); + assert(!(new->flags & ~allowed_flags)); /* Passing a non-wildcard address with DUAL_STACK_ANY is a bug */ - assert(!(flags & FWD_DUAL_STACK_ANY) || !addr || - inany_equals(addr, &inany_any6)); + assert(!(new->flags & FWD_DUAL_STACK_ANY) || + inany_equals(&new->addr, &inany_any6)); + assert(new->first <= new->last); if (fwd->count >= ARRAY_SIZE(fwd->rules)) die("Too many port forwarding ranges"); @@ -367,55 +359,30 @@ void fwd_rule_add(struct fwd_table *fwd, uint8_t proto, uint8_t flags, char newstr[INANY_ADDRSTRLEN], rulestr[INANY_ADDRSTRLEN]; const struct fwd_rule *rule = &fwd->rules[i]; - if (proto != rule->proto) + if (new->proto != rule->proto) /* Non-conflicting protocols */ continue; - if (!inany_matches(addr, fwd_rule_addr(rule))) + if (!inany_matches(fwd_rule_addr(new), fwd_rule_addr(rule))) /* Non-conflicting addresses */ continue; - if (last < rule->first || rule->last < first) + if (new->last < rule->first || rule->last < new->first) /* Port ranges don't overlap */ continue; die("Forwarding configuration conflict: %s/%u-%u versus %s/%u-%u", - inany_ntop(addr, newstr, sizeof(newstr)), first, last, + inany_ntop(fwd_rule_addr(new), newstr, sizeof(newstr)), + new->first, new->last, inany_ntop(fwd_rule_addr(rule), rulestr, sizeof(rulestr)), rule->first, rule->last); } - new = &fwd->rules[fwd->count]; - new->proto = proto; - 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) { - int ret; - - ret = snprintf(new->ifname, sizeof(new->ifname), - "%s", ifname); - if (ret <= 0 || (size_t)ret >= sizeof(new->ifname)) - die("Invalid interface name: %s", ifname); - } - - assert(first <= last); - new->first = first; - new->last = last; - new->to = to; - fwd->rulesocks[fwd->count] = &fwd->socks[fwd->sock_count]; for (port = new->first; port <= new->last; port++) fwd->rulesocks[fwd->count][port - new->first] = -1; - fwd->count++; + fwd->rules[fwd->count++] = *new; fwd->sock_count += num; } diff --git a/fwd.h b/fwd.h index 805fabd0..96b8c608 100644 --- a/fwd.h +++ b/fwd.h @@ -85,9 +85,7 @@ struct fwd_scan { #define FWD_PORT_SCAN_INTERVAL 1000 /* ms */ void fwd_rule_init(struct ctx *c); -void fwd_rule_add(struct fwd_table *fwd, uint8_t proto, uint8_t flags, - const union inany_addr *addr, const char *ifname, - in_port_t first, in_port_t last, in_port_t to); +void fwd_rule_add(struct fwd_table *fwd, const struct fwd_rule *new); const struct fwd_rule *fwd_rule_search(const struct fwd_table *fwd, const struct flowside *ini, uint8_t proto, int hint); -- 2.53.0

Currently the man page describes the internal syntax of port specifiers in prose, which isn't particularly easy to follow. Rework it to use more syntax "diagrams" to show how it works. This will also allow us to more easily update the manual page for some coming changes in syntax. usage() output is updated similarly, though more briefly. Signed-off-by: David Gibson --- conf.c | 10 +++++----- passt.1 | 32 ++++++++++++++++++++++---------- 2 files changed, 27 insertions(+), 15 deletions(-) diff --git a/conf.c b/conf.c index c3655824..5d6517c3 100644 --- a/conf.c +++ b/conf.c @@ -1041,11 +1041,11 @@ static void usage(const char *name, FILE *f, int status) " 'none': don't forward any ports\n" " 'all': forward all unbound, non-ephemeral ports\n" "%s" - " a comma-separated list, optionally ranged with '-'\n" - " and optional target ports after ':', with optional\n" - " address specification suffixed by '/' and optional\n" - " interface prefixed by '%%'. Ranges can be reduced by\n" - " excluding ports or ranges prefixed by '~'\n" + " [ADDR[%%IFACE]/]PORTS: forward specific ports\n" + " PORTS is a comma-separated list of ports, optionally\n" + " ranged with '-' and optional target ports after ':'.\n" + " Ranges can be reduced by excluding ports or ranges\n" + " prefixed by '~'\n" " Examples:\n" " -t 22 Forward local port 22 to 22 on %s\n" " -t 22:23 Forward local port 22 to 23 on %s\n" diff --git a/passt.1 b/passt.1 index 7da4fe5f..d329f8f0 100644 --- a/passt.1 +++ b/passt.1 @@ -447,16 +447,28 @@ periodically derived (every second) from listening sockets reported by \fI/proc/net/tcp\fR and \fI/proc/net/tcp6\fR, see \fBproc\fR(5). .TP -.BR ports -A comma-separated list of ports, optionally ranged with \fI-\fR, and, -optionally, with target ports after \fI:\fR, if they differ. Specific addresses -can be bound as well, separated by \fI/\fR, and also, since Linux 5.7, limited -to specific interfaces, prefixed by \fI%\fR. Within given ranges, selected ports -and ranges can be excluded by an additional specification prefixed by \fI~\fR. - -Specifying excluded ranges only implies that all other ports are forwarded. In -this case, no failures are reported for unavailable ports, unless no ports could -be forwarded at all. +[\fIaddress\fR[\fB%\fR\fIinterface\fR]\fB/\fR]\fIports\fR ... +Specific ports to forward. Optionally, a specific listening address +and interface name (since Linux 5.7) can be specified. \fIports\fR is +a comma-separated list of entries which may be any of: +.RS +.TP +\fIfirst\fR[\fB-\fR\fIlast\fR][\fB:\fR\fItofirst\fR[\fB-\fR\fItolast\fR]] +Include range. Forward port numbers between \fIfirst\fR and \fIlast\fR +(inclusive) to ports between \fItofirst\fR and \fItolast\fR. If +\fItofirst\fR and \fItolast\fR are omitted, assume the same as +\fIfirst\fR and \fIlast\fR. If \fIlast\fR is omitted, assume the same +as \fIfirst\fR. + +.TP +\fB~\fR\fIfirst\fR[\fB-\fR\fIlast\fR] +Exclude range. Exclude port numbers between \fIfirst\fR and +\fIlast\fR from. This takes precedences over include ranges. +.RE + +Specifying excluded ranges only implies that all other non-ephemeral +ports are forwarded. In this case, no failures are reported for +unavailable ports, unless no ports could be forwarded at all. Examples: .RS -- 2.53.0

Currently -[tTuU] all is handled separately in conf_ports() before calling conf_ports_spec(). Earlier changes mean we can now move this handling to conf_ports_spec(). This makes the code slightly simpler, but more importantly it allows some useful combinations we couldn't previously do, such as -t 127.0.0.1/all or -u %eth2/all Signed-off-by: David Gibson --- conf.c | 25 ++++++++++--------------- passt.1 | 28 ++++++++++++++++++++-------- 2 files changed, 30 insertions(+), 23 deletions(-) diff --git a/conf.c b/conf.c index 5d6517c3..f62109b5 100644 --- a/conf.c +++ b/conf.c @@ -251,6 +251,11 @@ static void conf_ports_spec(const struct ctx *c, const char *p, *ep; unsigned i; + if (!strcmp(spec, "all")) { + /* Treat "all" as equivalent to "": all non-ephemeral ports */ + spec = ""; + } + /* Mark all exclusions first, they might be given after base ranges */ for_each_chunk(p, ep, spec, ",") { struct port_range xrange; @@ -372,19 +377,6 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg, return; } - if (!strcmp(optarg, "all")) { - uint8_t exclude[PORT_BITMAP_SIZE] = { 0 }; - - /* Exclude ephemeral ports */ - fwd_port_map_ephemeral(exclude); - - conf_ports_range_except(c, optname, optarg, fwd, - proto, NULL, NULL, - 1, NUM_PORTS - 1, exclude, - 1, FWD_WEAK); - return; - } - strncpy(buf, optarg, sizeof(buf) - 1); if ((spec = strchr(buf, '/'))) { @@ -1039,14 +1031,17 @@ static void usage(const char *name, FILE *f, int status) " can be specified multiple times\n" " SPEC can be:\n" " 'none': don't forward any ports\n" - " 'all': forward all unbound, non-ephemeral ports\n" "%s" " [ADDR[%%IFACE]/]PORTS: forward specific ports\n" - " PORTS is a comma-separated list of ports, optionally\n" + " PORTS is either 'all' (forward all unbound, non-ephemeral\n" + " ports), or a comma-separated list of ports, optionally\n" " ranged with '-' and optional target ports after ':'.\n" " Ranges can be reduced by excluding ports or ranges\n" " prefixed by '~'\n" " Examples:\n" + " -t all Forward all ports\n" + " -t 127.0.0.1/all Forward all ports from local address\n" + " 127.0.0.1\n" " -t 22 Forward local port 22 to 22 on %s\n" " -t 22:23 Forward local port 22 to 23 on %s\n" " -t 22,25 Forward ports 22, 25 to ports 22, 25\n" diff --git a/passt.1 b/passt.1 index d329f8f0..3ba447d5 100644 --- a/passt.1 +++ b/passt.1 @@ -434,12 +434,6 @@ Configure TCP port forwarding to guest or namespace. \fIspec\fR can be one of: .BR none Don't forward any ports -.TP -.BR all -Forward all unbound, non-ephemeral ports, as permitted by current capabilities. -For low (< 1024) ports, see \fBNOTES\fR. No failures are reported for -unavailable ports, unless no ports could be forwarded at all. - .TP .BR auto " " (\fBpasta\fR " " only) Dynamically forward ports bound in the namespace. The list of ports is @@ -449,10 +443,20 @@ periodically derived (every second) from listening sockets reported by .TP [\fIaddress\fR[\fB%\fR\fIinterface\fR]\fB/\fR]\fIports\fR ... Specific ports to forward. Optionally, a specific listening address -and interface name (since Linux 5.7) can be specified. \fIports\fR is -a comma-separated list of entries which may be any of: +and interface name (since Linux 5.7) can be specified. \fIports\fR +may be either: .RS .TP +\fBall\fR +Forward all unbound, non-ephemeral ports, as permitted by current +capabilities. For low (< 1024) ports, see \fBNOTES\fR. No failures +are reported for unavailable ports, unless no ports could be forwarded +at all. +.RE + +.RS +or a comma-separated list of entries which may be any of: +.TP \fIfirst\fR[\fB-\fR\fIlast\fR][\fB:\fR\fItofirst\fR[\fB-\fR\fItolast\fR]] Include range. Forward port numbers between \fIfirst\fR and \fIlast\fR (inclusive) to ports between \fItofirst\fR and \fItolast\fR. If @@ -473,6 +477,14 @@ unavailable ports, unless no ports could be forwarded at all. Examples: .RS .TP +-t all +Forward all unbound, non-ephemeral ports as permitted by current +capabilities to the corresponding port on the guest or namespace +.TP +-t 127.0.0.1/all +For the local address 127.0.0.1, forward all unbound, non-ephemeral +ports as permitted by current capabilities. +.TP -t 22 Forward local port 22 to port 22 on the guest or namespace .TP -- 2.53.0

We only use the optname and optarg parameters for printing error messages, and they're not even particularly necessary there. Remove them. Signed-off-by: David Gibson --- conf.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/conf.c b/conf.c index 5629b3dc..89f68de7 100644 --- a/conf.c +++ b/conf.c @@ -219,8 +219,6 @@ fail: /** * conf_ports_spec() - Parse port range(s) specifier * @c: Execution context - * @optname: Short option name, t, T, u, or U - * @optarg: Option argument (port specification) * @fwd: Forwarding table to be updated * @proto: Protocol to forward * @addr: Listening address for forwarding @@ -228,7 +226,6 @@ fail: * @spec: Port range(s) specifier */ static void conf_ports_spec(const struct ctx *c, - char optname, const char *optarg, struct fwd_table *fwd, uint8_t proto, const union inany_addr *addr, const char *ifname, const char *spec) @@ -316,8 +313,7 @@ static void conf_ports_spec(const struct ctx *c, goto bad; if (orig_range.first == 0) { - die("Can't forward port 0 for option '-%c %s'", - optname, optarg); + die("Can't forward port 0 included in '%s'", spec); } conf_ports_range_except(fwd, proto, addr, ifname, @@ -328,7 +324,7 @@ static void conf_ports_spec(const struct ctx *c, return; bad: - die("Invalid port specifier %s", optarg); + die("Invalid port specifier '%s'", spec); } /** @@ -432,11 +428,11 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg, optname, optarg); if (c->ifi4) { - conf_ports_spec(c, optname, optarg, fwd, proto, + conf_ports_spec(c, fwd, proto, &inany_loopback4, NULL, spec); } if (c->ifi6) { - conf_ports_spec(c, optname, optarg, fwd, proto, + conf_ports_spec(c, fwd, proto, &inany_loopback6, NULL, spec); } return; @@ -451,7 +447,7 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg, optname, optarg); } - conf_ports_spec(c, optname, optarg, fwd, proto, addr, ifname, spec); + conf_ports_spec(c, fwd, proto, addr, ifname, spec); } /** -- 2.53.0

Although fwd_rule_add() performs some sanity checks on the rule it is given, there are invalid rules we don't check for, assuming that its callers will do that. That won't be enough when we can get rules inserted by a dynamic update client without going through the existing parsing code. So, add stricter checks to fwd_rule_add(), which is now possible thanks to the capabilities bits in the struct fwd_table. Where those duplicate existing checks in the callers, remove the old copies. Signed-off-by: David Gibson --- conf.c | 19 ------------------- fwd.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 46 insertions(+), 24 deletions(-) diff --git a/conf.c b/conf.c index b7a4459e..31627209 100644 --- a/conf.c +++ b/conf.c @@ -310,10 +310,6 @@ static void conf_ports_spec(struct fwd_table *fwd, uint8_t proto, if (p != ep) /* Garbage after the ranges */ goto bad; - if (orig_range.first == 0) { - die("Can't forward port 0 included in '%s'", spec); - } - conf_ports_range_except(fwd, proto, addr, ifname, orig_range.first, orig_range.last, exclude, @@ -356,11 +352,6 @@ static void conf_ports(char optname, const char *optarg, struct fwd_table *fwd) return; } - if (proto == IPPROTO_TCP && !(fwd->caps & FWD_CAP_TCP)) - die("TCP port forwarding requested but TCP is disabled"); - if (proto == IPPROTO_UDP && !(fwd->caps & FWD_CAP_UDP)) - die("UDP port forwarding requested but UDP is disabled"); - strncpy(buf, optarg, sizeof(buf) - 1); if ((spec = strchr(buf, '/'))) { @@ -405,16 +396,6 @@ static void conf_ports(char optname, const char *optarg, struct fwd_table *fwd) addr = NULL; } - if (addr) { - if (!(fwd->caps & FWD_CAP_IPV4) && inany_v4(addr)) { - die("IPv4 is disabled, can't use -%c %s", - optname, optarg); - } else if (!(fwd->caps & FWD_CAP_IPV6) && !inany_v4(addr)) { - die("IPv6 is disabled, can't use -%c %s", - optname, optarg); - } - } - if (optname == 'T' || optname == 'U') { assert(!addr && !ifname); diff --git a/fwd.c b/fwd.c index c7fd1a9d..98b04d0c 100644 --- a/fwd.c +++ b/fwd.c @@ -367,17 +367,58 @@ int fwd_rule_add(struct fwd_table *fwd, const struct fwd_rule *new) new->first, new->last); return -EINVAL; } + if (!new->first) { + warn("Forwarding rule atttempts to map from port 0"); + return -EINVAL; + } + if (!new->to || (new->to + new->last - new->first) < new->to) { + warn("Forwarding rule atttempts to map to port 0"); + return -EINVAL; + } if (new->flags & ~allowed_flags) { warn("Rule has invalid flags 0x%hhx", new->flags & ~allowed_flags); return -EINVAL; } - if (new->flags & FWD_DUAL_STACK_ANY && - !inany_equals(&new->addr, &inany_any6)) { - char astr[INANY_ADDRSTRLEN]; + if (new->flags & FWD_DUAL_STACK_ANY) { + if (!inany_equals(&new->addr, &inany_any6)) { + char astr[INANY_ADDRSTRLEN]; - warn("Dual stack rule has non-wildcard address %s", - inany_ntop(&new->addr, astr, sizeof(astr))); + warn("Dual stack rule has non-wildcard address %s", + inany_ntop(&new->addr, astr, sizeof(astr))); + return -EINVAL; + } + if (!(fwd->caps & FWD_CAP_IPV4)) { + warn("Dual stack forward, but IPv4 not enabled"); + return -EINVAL; + } + if (!(fwd->caps & FWD_CAP_IPV6)) { + warn("Dual stack forward, but IPv6 not enabled"); + return -EINVAL; + } + } else { + if (inany_v4(&new->addr) && !(fwd->caps & FWD_CAP_IPV4)) { + warn("IPv4 forward, but IPv4 not enabled"); + return -EINVAL; + } + if (!inany_v4(&new->addr) && !(fwd->caps & FWD_CAP_IPV6)) { + warn("IPv6 forward, but IPv6 not enabled"); + return -EINVAL; + } + } + if (new->proto == IPPROTO_TCP) { + if (!(fwd->caps & FWD_CAP_TCP)) { + warn("Can't add TCP forwarding rule, TCP not enabled"); + return -EINVAL; + } + } else if (new->proto == IPPROTO_UDP) { + if (!(fwd->caps & FWD_CAP_UDP)) { + warn("Can't add UDP forwarding rule, UDP not enabled"); + return -EINVAL; + } + } else { + warn("Unsupported protocol 0x%hhx (%s) for forwarding rule", + new->proto, ipproto_name(new->proto)); return -EINVAL; } -- 2.53.0

The code parsing command line options into forwarding rules has now been decoupled from most of passt/pasta's internals. This is good, because we'll soon want to share it with a configuration update client. Make the next step by moving this code into fwd_rule.[ch]. Signed-off-by: David Gibson --- conf.c | 378 +------------------------------------------ fwd.c | 93 ----------- fwd.h | 33 ---- fwd_rule.c | 465 ++++++++++++++++++++++++++++++++++++++++++++++++++++- fwd_rule.h | 36 ++++- 5 files changed, 503 insertions(+), 502 deletions(-) diff --git a/conf.c b/conf.c index 31627209..5f38a287 100644 --- a/conf.c +++ b/conf.c @@ -13,7 +13,6 @@ */ #include -#include #include #include #include @@ -66,367 +65,6 @@ const char *pasta_default_ifn = "tap0"; -/** - * port_range() - Represents a non-empty range of ports - * @first: First port number in the range - * @last: Last port number in the range (inclusive) - * - * Invariant: @last >= @first - */ -struct port_range { - in_port_t first, last; -}; - -/** - * parse_port_range() - Parse a range of port numbers '<first>[-<last>]' - * @s: String to parse - * @endptr: Update to the character after the parsed range (similar to - * strtol() etc.) - * @range: Update with the parsed values on success - * - * Return: -EINVAL on parsing error, -ERANGE on out of range port - * numbers, 0 on success - */ -static int parse_port_range(const char *s, const char **endptr, - struct port_range *range) -{ - unsigned long first, last; - char *ep; - - last = first = strtoul(s, &ep, 10); - if (ep == s) /* Parsed nothing */ - return -EINVAL; - if (*ep == '-') { /* we have a last value too */ - const char *lasts = ep + 1; - last = strtoul(lasts, &ep, 10); - if (ep == lasts) /* Parsed nothing */ - return -EINVAL; - } - - if ((last < first) || (last >= NUM_PORTS)) - return -ERANGE; - - range->first = first; - range->last = last; - *endptr = ep; - - return 0; -} - -/** - * parse_keyword() - Parse a literal keyword - * @s: String to parse - * @endptr: Update to the character after the keyword - * @kw: Keyword to accept - * - * Return: 0, if @s starts with @kw, -EINVAL if it does not - */ -static int parse_keyword(const char *s, const char **endptr, const char *kw) -{ - size_t len = strlen(kw); - - if (strlen(s) < len) - return -EINVAL; - - if (memcmp(s, kw, len)) - return -EINVAL; - - *endptr = s + len; - return 0; -} - -/** - * conf_ports_range_except() - Set up forwarding for a range of ports minus a - * bitmap of exclusions - * @fwd: Forwarding table to be updated - * @proto: Protocol to forward - * @addr: Listening address - * @ifname: Listening interface - * @first: First port to forward - * @last: Last port to forward - * @exclude: Bitmap of ports to exclude (may be NULL) - * @to: Port to translate @first to when forwarding - * @flags: Flags for forwarding entries - */ -static void conf_ports_range_except(struct fwd_table *fwd, uint8_t proto, - const union inany_addr *addr, - const char *ifname, - uint16_t first, uint16_t last, - const uint8_t *exclude, uint16_t to, - uint8_t flags) -{ - struct fwd_rule rule = { - .addr = addr ? *addr : inany_any6, - .ifname = { 0 }, - .proto = proto, - .flags = flags, - }; - char rulestr[FWD_RULE_STRLEN]; - unsigned delta = to - first; - unsigned base, i; - - if (!addr) - rule.flags |= FWD_DUAL_STACK_ANY; - if (ifname) { - int ret; - - ret = snprintf(rule.ifname, sizeof(rule.ifname), - "%s", ifname); - if (ret <= 0 || (size_t)ret >= sizeof(rule.ifname)) - die("Invalid interface name: %s", ifname); - } - - assert(first != 0); - - for (base = first; base <= last; base++) { - if (exclude && bitmap_isset(exclude, base)) - continue; - - for (i = base; i <= last; i++) { - if (exclude && bitmap_isset(exclude, i)) - break; - } - - rule.first = base; - rule.last = i - 1; - rule.to = base + delta; - - fwd_rule_conflict_check(&rule, fwd->rules, fwd->count); - if (fwd_rule_add(fwd, &rule) < 0) - goto fail; - - base = i - 1; - } - return; - -fail: - die("Unable to add rule %s", - fwd_rule_fmt(&rule, rulestr, sizeof(rulestr))); -} - -/* - * for_each_chunk - Step through delimited chunks of a string - * @p_: Pointer to start of each chunk (updated) - * @ep_: Pointer to end of each chunk (updated) - * @s_: String to step through - * @sep_: String of all allowed delimiters - */ -#define for_each_chunk(p_, ep_, s_, sep_) \ - for ((p_) = (s_); \ - (ep_) = (p_) + strcspn((p_), (sep_)), *(p_); \ - (p_) = *(ep_) ? (ep_) + 1 : (ep_)) - -/** - * conf_ports_spec() - Parse port range(s) specifier - * @fwd: Forwarding table to be updated - * @proto: Protocol to forward - * @addr: Listening address for forwarding - * @ifname: Interface name for listening - * @spec: Port range(s) specifier - */ -static void conf_ports_spec(struct fwd_table *fwd, uint8_t proto, - const union inany_addr *addr, const char *ifname, - const char *spec) -{ - uint8_t exclude[PORT_BITMAP_SIZE] = { 0 }; - bool exclude_only = true; - const char *p, *ep; - uint8_t flags = 0; - unsigned i; - - if (!strcmp(spec, "all")) { - /* Treat "all" as equivalent to "": all non-ephemeral ports */ - spec = ""; - } - - /* Parse excluded ranges and "auto" in the first pass */ - for_each_chunk(p, ep, spec, ",") { - struct port_range xrange; - - if (isdigit(*p)) { - /* Include range, parse later */ - exclude_only = false; - continue; - } - - if (parse_keyword(p, &p, "auto") == 0) { - if (p != ep) /* Garbage after the keyword */ - goto bad; - - if (!(fwd->caps & FWD_CAP_SCAN)) { - die( -"'auto' port forwarding is only allowed for pasta"); - } - - flags |= FWD_SCAN; - continue; - } - - /* Should be an exclude range */ - if (*p != '~') - goto bad; - p++; - - if (parse_port_range(p, &p, &xrange)) - goto bad; - if (p != ep) /* Garbage after the range */ - goto bad; - - for (i = xrange.first; i <= xrange.last; i++) - bitmap_set(exclude, i); - } - - if (exclude_only) { - /* Exclude ephemeral ports */ - fwd_port_map_ephemeral(exclude); - - conf_ports_range_except(fwd, proto, addr, ifname, - 1, NUM_PORTS - 1, exclude, - 1, flags | FWD_WEAK); - return; - } - - /* Now process base ranges, skipping exclusions */ - for_each_chunk(p, ep, spec, ",") { - struct port_range orig_range, mapped_range; - - if (!isdigit(*p)) - /* Already parsed */ - continue; - - if (parse_port_range(p, &p, &orig_range)) - goto bad; - - if (*p == ':') { /* There's a range to map to as well */ - if (parse_port_range(p + 1, &p, &mapped_range)) - goto bad; - if ((mapped_range.last - mapped_range.first) != - (orig_range.last - orig_range.first)) - goto bad; - } else { - mapped_range = orig_range; - } - - if (p != ep) /* Garbage after the ranges */ - goto bad; - - conf_ports_range_except(fwd, proto, addr, ifname, - orig_range.first, orig_range.last, - exclude, - mapped_range.first, flags); - } - - return; -bad: - die("Invalid port specifier '%s'", spec); -} - -/** - * conf_ports() - Parse port configuration options, initialise UDP/TCP sockets - * @optname: Short option name, t, T, u, or U - * @optarg: Option argument (port specification) - * @fwd: Forwarding table to be updated - */ -static void conf_ports(char optname, const char *optarg, struct fwd_table *fwd) -{ - union inany_addr addr_buf = inany_any6, *addr = &addr_buf; - char buf[BUFSIZ], *spec, *ifname = NULL; - uint8_t proto; - - if (optname == 't' || optname == 'T') - proto = IPPROTO_TCP; - else if (optname == 'u' || optname == 'U') - proto = IPPROTO_UDP; - else - assert(0); - - if (!strcmp(optarg, "none")) { - unsigned i; - - for (i = 0; i < fwd->count; i++) { - if (fwd->rules[i].proto == proto) { - die("-%c none conflicts with previous options", - optname); - } - } - return; - } - - strncpy(buf, optarg, sizeof(buf) - 1); - - if ((spec = strchr(buf, '/'))) { - *spec = 0; - spec++; - - if (optname != 't' && optname != 'u') - die("Listening address not allowed for -%c %s", - optname, optarg); - - if ((ifname = strchr(buf, '%'))) { - *ifname = 0; - ifname++; - - /* spec is already advanced one past the '/', - * so the length of the given ifname is: - * (spec - ifname - 1) - */ - if (spec - ifname - 1 >= IFNAMSIZ) { - die("Interface name '%s' is too long (max %u)", - ifname, IFNAMSIZ - 1); - } - } - - if (ifname == buf + 1) { /* Interface without address */ - addr = NULL; - } else { - char *p = buf; - - /* Allow square brackets for IPv4 too for convenience */ - if (*p == '[' && p[strlen(p) - 1] == ']') { - p[strlen(p) - 1] = '\0'; - p++; - } - - if (!inany_pton(p, addr)) - die("Bad forwarding address '%s'", p); - } - } else { - spec = buf; - - addr = NULL; - } - - if (optname == 'T' || optname == 'U') { - assert(!addr && !ifname); - - if (!(fwd->caps & FWD_CAP_IFNAME)) { - warn( -"SO_BINDTODEVICE unavailable, forwarding only 127.0.0.1 and ::1 for '-%c %s'", - optname, optarg); - - if (fwd->caps & FWD_CAP_IPV4) { - conf_ports_spec(fwd, proto, - &inany_loopback4, NULL, spec); - } - if (fwd->caps & FWD_CAP_IPV6) { - conf_ports_spec(fwd, proto, - &inany_loopback6, NULL, spec); - } - return; - } - - ifname = "lo"; - } - - if (ifname && !(fwd->caps & FWD_CAP_IFNAME)) { - die( -"Device binding for '-%c %s' unsupported (requires kernel 5.7+)", - optname, optarg); - } - - conf_ports_spec(fwd, proto, addr, ifname, spec); -} - /** * add_dns4() - Possibly add the IPv4 address of a DNS resolver to configuration * @c: Execution context @@ -2163,16 +1801,16 @@ void conf(struct ctx *c, int argc, char **argv) if (name == 't') { opt_t = true; - conf_ports(name, optarg, c->fwd[PIF_HOST]); + fwd_rule_parse(name, optarg, c->fwd[PIF_HOST]); } else if (name == 'u') { opt_u = true; - conf_ports(name, optarg, c->fwd[PIF_HOST]); + fwd_rule_parse(name, optarg, c->fwd[PIF_HOST]); } else if (name == 'T') { opt_T = true; - conf_ports(name, optarg, c->fwd[PIF_SPLICE]); + fwd_rule_parse(name, optarg, c->fwd[PIF_SPLICE]); } else if (name == 'U') { opt_U = true; - conf_ports(name, optarg, c->fwd[PIF_SPLICE]); + fwd_rule_parse(name, optarg, c->fwd[PIF_SPLICE]); } } while (name != -1); @@ -2224,13 +1862,13 @@ void conf(struct ctx *c, int argc, char **argv) if (c->mode == MODE_PASTA) { if (!opt_t) - conf_ports('t', "auto", c->fwd[PIF_HOST]); + fwd_rule_parse('t', "auto", c->fwd[PIF_HOST]); if (!opt_T) - conf_ports('T', "auto", c->fwd[PIF_SPLICE]); + fwd_rule_parse('T', "auto", c->fwd[PIF_SPLICE]); if (!opt_u) - conf_ports('u', "auto", c->fwd[PIF_HOST]); + fwd_rule_parse('u', "auto", c->fwd[PIF_HOST]); if (!opt_U) - conf_ports('U', "auto", c->fwd[PIF_SPLICE]); + fwd_rule_parse('U', "auto", c->fwd[PIF_SPLICE]); } if (!c->quiet) diff --git a/fwd.c b/fwd.c index f2f7b648..728a783c 100644 --- a/fwd.c +++ b/fwd.c @@ -275,99 +275,6 @@ void fwd_rule_init(struct ctx *c) c->fwd[PIF_SPLICE] = &fwd_out; } -/** - * fwd_rule_add() - Validate and add a rule to a forwarding table - * @fwd: Table to add to - * @new: Rule to add - * - * Return: 0 on success, negative error code on failure - */ -int fwd_rule_add(struct fwd_table *fwd, const struct fwd_rule *new) -{ - /* Flags which can be set from the caller */ - const uint8_t allowed_flags = FWD_WEAK | FWD_SCAN | FWD_DUAL_STACK_ANY; - unsigned num = (unsigned)new->last - new->first + 1; - unsigned port; - - if (new->first > new->last) { - warn("Rule has invalid port range %u-%u", - new->first, new->last); - return -EINVAL; - } - if (!new->first) { - warn("Forwarding rule atttempts to map from port 0"); - return -EINVAL; - } - if (!new->to || (new->to + new->last - new->first) < new->to) { - warn("Forwarding rule atttempts to map to port 0"); - return -EINVAL; - } - if (new->flags & ~allowed_flags) { - warn("Rule has invalid flags 0x%hhx", - new->flags & ~allowed_flags); - return -EINVAL; - } - if (new->flags & FWD_DUAL_STACK_ANY) { - if (!inany_equals(&new->addr, &inany_any6)) { - char astr[INANY_ADDRSTRLEN]; - - warn("Dual stack rule has non-wildcard address %s", - inany_ntop(&new->addr, astr, sizeof(astr))); - return -EINVAL; - } - if (!(fwd->caps & FWD_CAP_IPV4)) { - warn("Dual stack forward, but IPv4 not enabled"); - return -EINVAL; - } - if (!(fwd->caps & FWD_CAP_IPV6)) { - warn("Dual stack forward, but IPv6 not enabled"); - return -EINVAL; - } - } else { - if (inany_v4(&new->addr) && !(fwd->caps & FWD_CAP_IPV4)) { - warn("IPv4 forward, but IPv4 not enabled"); - return -EINVAL; - } - if (!inany_v4(&new->addr) && !(fwd->caps & FWD_CAP_IPV6)) { - warn("IPv6 forward, but IPv6 not enabled"); - return -EINVAL; - } - } - if (new->proto == IPPROTO_TCP) { - if (!(fwd->caps & FWD_CAP_TCP)) { - warn("Can't add TCP forwarding rule, TCP not enabled"); - return -EINVAL; - } - } else if (new->proto == IPPROTO_UDP) { - if (!(fwd->caps & FWD_CAP_UDP)) { - warn("Can't add UDP forwarding rule, UDP not enabled"); - return -EINVAL; - } - } else { - warn("Unsupported protocol 0x%hhx (%s) for forwarding rule", - new->proto, ipproto_name(new->proto)); - return -EINVAL; - } - - if (fwd->count >= ARRAY_SIZE(fwd->rules)) { - warn("Too many rules (maximum %u)", ARRAY_SIZE(fwd->rules)); - return -ENOSPC; - } - if ((fwd->sock_count + num) > ARRAY_SIZE(fwd->socks)) { - warn("Rules require too many listening sockets (maximum %u)", - ARRAY_SIZE(fwd->socks)); - return -ENOSPC; - } - - fwd->rulesocks[fwd->count] = &fwd->socks[fwd->sock_count]; - for (port = new->first; port <= new->last; port++) - fwd->rulesocks[fwd->count][port - new->first] = -1; - - fwd->rules[fwd->count++] = *new; - fwd->sock_count += num; - return 0; -} - /** * fwd_rule_match() - Does a prospective flow match a given forwarding rule? * @rule: Forwarding rule diff --git a/fwd.h b/fwd.h index e664d1d0..8f845d09 100644 --- a/fwd.h +++ b/fwd.h @@ -20,8 +20,6 @@ struct flowside; -#define FWD_RULE_BITS 8 -#define MAX_FWD_RULES MAX_FROM_BITS(FWD_RULE_BITS) #define FWD_NO_HINT (-1) /** @@ -36,36 +34,6 @@ struct fwd_listen_ref { unsigned rule :FWD_RULE_BITS; }; -/* Maximum number of listening sockets (per pif) - * - * Rationale: This lets us listen on every port for two addresses and two - * protocols (which we need for -T auto -U auto without SO_BINDTODEVICE), plus a - * comfortable number of extras. - */ -#define MAX_LISTEN_SOCKS (NUM_PORTS * 5) - -/** - * struct fwd_table - Forwarding state (per initiating pif) - * @caps: Forwarding capabilities for this initiating pif - * @count: Number of forwarding rules - * @rules: Array of forwarding rules - * @rulesocks: Parallel array of @rules (@count valid entries) of pointers to - * @socks entries giving the start of the corresponding rule's - * sockets within the larger array - * @sock_count: Number of entries used in @socks (for all rules combined) - * @socks: Listening sockets for forwarding - */ -struct fwd_table { - uint32_t caps; - unsigned count; - struct fwd_rule rules[MAX_FWD_RULES]; - int *rulesocks[MAX_FWD_RULES]; - unsigned sock_count; - int socks[MAX_LISTEN_SOCKS]; -}; - -#define PORT_BITMAP_SIZE DIV_ROUND_UP(NUM_PORTS, 8) - /** * struct fwd_scan - Port scanning state for a protocol+direction * @scan4: /proc/net fd to scan for IPv4 ports when in AUTO mode @@ -81,7 +49,6 @@ struct fwd_scan { #define FWD_PORT_SCAN_INTERVAL 1000 /* ms */ void fwd_rule_init(struct ctx *c); -int fwd_rule_add(struct fwd_table *fwd, const struct fwd_rule *new); const struct fwd_rule *fwd_rule_search(const struct fwd_table *fwd, const struct flowside *ini, uint8_t proto, int hint); diff --git a/fwd_rule.c b/fwd_rule.c index 9d489827..2b0bd4db 100644 --- a/fwd_rule.c +++ b/fwd_rule.c @@ -15,6 +15,7 @@ * Author: David Gibson */ +#include #include #include #include @@ -89,7 +90,7 @@ parse_err: * fwd_port_map_ephemeral() - Mark ephemeral ports in a bitmap * @map: Bitmap to update */ -void fwd_port_map_ephemeral(uint8_t *map) +static void fwd_port_map_ephemeral(uint8_t *map) { unsigned port; @@ -123,6 +124,7 @@ const union inany_addr *fwd_rule_addr(const struct fwd_rule *rule) */ __attribute__((noinline)) #endif +/* cppcheck-suppress staticFunction */ const char *fwd_rule_fmt(const struct fwd_rule *rule, char *dst, size_t size) { const char *percent = *rule->ifname ? "%" : ""; @@ -199,8 +201,8 @@ static bool fwd_rule_conflicts(const struct fwd_rule *a, const struct fwd_rule * * @rules: Existing rules against which to test * @count: Number of rules in @rules */ -void fwd_rule_conflict_check(const struct fwd_rule *new, - const struct fwd_rule *rules, size_t count) +static void fwd_rule_conflict_check(const struct fwd_rule *new, + const struct fwd_rule *rules, size_t count) { unsigned i; @@ -215,3 +217,460 @@ void fwd_rule_conflict_check(const struct fwd_rule *new, fwd_rule_fmt(&rules[i], rulestr, sizeof(rulestr))); } } + +/** + * fwd_rule_add() - Validate and add a rule to a forwarding table + * @fwd: Table to add to + * @new: Rule to add + * + * Return: 0 on success, negative error code on failure + */ +static int fwd_rule_add(struct fwd_table *fwd, const struct fwd_rule *new) +{ + /* Flags which can be set from the caller */ + const uint8_t allowed_flags = FWD_WEAK | FWD_SCAN | FWD_DUAL_STACK_ANY; + unsigned num = (unsigned)new->last - new->first + 1; + unsigned port; + + if (new->first > new->last) { + warn("Rule has invalid port range %u-%u", + new->first, new->last); + return -EINVAL; + } + if (!new->first) { + warn("Forwarding rule atttempts to map from port 0"); + return -EINVAL; + } + if (!new->to || (new->to + new->last - new->first) < new->to) { + warn("Forwarding rule atttempts to map to port 0"); + return -EINVAL; + } + if (new->flags & ~allowed_flags) { + warn("Rule has invalid flags 0x%hhx", + new->flags & ~allowed_flags); + return -EINVAL; + } + if (new->flags & FWD_DUAL_STACK_ANY) { + if (!inany_equals(&new->addr, &inany_any6)) { + char astr[INANY_ADDRSTRLEN]; + + warn("Dual stack rule has non-wildcard address %s", + inany_ntop(&new->addr, astr, sizeof(astr))); + return -EINVAL; + } + if (!(fwd->caps & FWD_CAP_IPV4)) { + warn("Dual stack forward, but IPv4 not enabled"); + return -EINVAL; + } + if (!(fwd->caps & FWD_CAP_IPV6)) { + warn("Dual stack forward, but IPv6 not enabled"); + return -EINVAL; + } + } else { + if (inany_v4(&new->addr) && !(fwd->caps & FWD_CAP_IPV4)) { + warn("IPv4 forward, but IPv4 not enabled"); + return -EINVAL; + } + if (!inany_v4(&new->addr) && !(fwd->caps & FWD_CAP_IPV6)) { + warn("IPv6 forward, but IPv6 not enabled"); + return -EINVAL; + } + } + if (new->proto == IPPROTO_TCP) { + if (!(fwd->caps & FWD_CAP_TCP)) { + warn("Can't add TCP forwarding rule, TCP not enabled"); + return -EINVAL; + } + } else if (new->proto == IPPROTO_UDP) { + if (!(fwd->caps & FWD_CAP_UDP)) { + warn("Can't add UDP forwarding rule, UDP not enabled"); + return -EINVAL; + } + } else { + warn("Unsupported protocol 0x%hhx (%s) for forwarding rule", + new->proto, ipproto_name(new->proto)); + return -EINVAL; + } + + if (fwd->count >= ARRAY_SIZE(fwd->rules)) { + warn("Too many rules (maximum %u)", ARRAY_SIZE(fwd->rules)); + return -ENOSPC; + } + if ((fwd->sock_count + num) > ARRAY_SIZE(fwd->socks)) { + warn("Rules require too many listening sockets (maximum %u)", + ARRAY_SIZE(fwd->socks)); + return -ENOSPC; + } + + fwd->rulesocks[fwd->count] = &fwd->socks[fwd->sock_count]; + for (port = new->first; port <= new->last; port++) + fwd->rulesocks[fwd->count][port - new->first] = -1; + + fwd->rules[fwd->count++] = *new; + fwd->sock_count += num; + return 0; +} + +/** + * port_range() - Represents a non-empty range of ports + * @first: First port number in the range + * @last: Last port number in the range (inclusive) + * + * Invariant: @last >= @first + */ +struct port_range { + in_port_t first, last; +}; + +/** + * parse_port_range() - Parse a range of port numbers '<first>[-<last>]' + * @s: String to parse + * @endptr: Update to the character after the parsed range (similar to + * strtol() etc.) + * @range: Update with the parsed values on success + * + * Return: -EINVAL on parsing error, -ERANGE on out of range port + * numbers, 0 on success + */ +static int parse_port_range(const char *s, const char **endptr, + struct port_range *range) +{ + unsigned long first, last; + char *ep; + + last = first = strtoul(s, &ep, 10); + if (ep == s) /* Parsed nothing */ + return -EINVAL; + if (*ep == '-') { /* we have a last value too */ + const char *lasts = ep + 1; + last = strtoul(lasts, &ep, 10); + if (ep == lasts) /* Parsed nothing */ + return -EINVAL; + } + + if ((last < first) || (last >= NUM_PORTS)) + return -ERANGE; + + range->first = first; + range->last = last; + *endptr = ep; + + return 0; +} + +/** + * parse_keyword() - Parse a literal keyword + * @s: String to parse + * @endptr: Update to the character after the keyword + * @kw: Keyword to accept + * + * Return: 0, if @s starts with @kw, -EINVAL if it does not + */ +static int parse_keyword(const char *s, const char **endptr, const char *kw) +{ + size_t len = strlen(kw); + + if (strlen(s) < len) + return -EINVAL; + + if (memcmp(s, kw, len)) + return -EINVAL; + + *endptr = s + len; + return 0; +} + +/** + * fwd_rule_range_except() - Set up forwarding for a range of ports minus a + * bitmap of exclusions + * @fwd: Forwarding table to be updated + * @proto: Protocol to forward + * @addr: Listening address + * @ifname: Listening interface + * @first: First port to forward + * @last: Last port to forward + * @exclude: Bitmap of ports to exclude (may be NULL) + * @to: Port to translate @first to when forwarding + * @flags: Flags for forwarding entries + */ +static void fwd_rule_range_except(struct fwd_table *fwd, uint8_t proto, + const union inany_addr *addr, + const char *ifname, + uint16_t first, uint16_t last, + const uint8_t *exclude, uint16_t to, + uint8_t flags) +{ + struct fwd_rule rule = { + .addr = addr ? *addr : inany_any6, + .ifname = { 0 }, + .proto = proto, + .flags = flags, + }; + char rulestr[FWD_RULE_STRLEN]; + unsigned delta = to - first; + unsigned base, i; + + if (!addr) + rule.flags |= FWD_DUAL_STACK_ANY; + if (ifname) { + int ret; + + ret = snprintf(rule.ifname, sizeof(rule.ifname), + "%s", ifname); + if (ret <= 0 || (size_t)ret >= sizeof(rule.ifname)) + die("Invalid interface name: %s", ifname); + } + + assert(first != 0); + + for (base = first; base <= last; base++) { + if (exclude && bitmap_isset(exclude, base)) + continue; + + for (i = base; i <= last; i++) { + if (exclude && bitmap_isset(exclude, i)) + break; + } + + rule.first = base; + rule.last = i - 1; + rule.to = base + delta; + + fwd_rule_conflict_check(&rule, fwd->rules, fwd->count); + if (fwd_rule_add(fwd, &rule) < 0) + goto fail; + + base = i - 1; + } + return; + +fail: + die("Unable to add rule %s", + fwd_rule_fmt(&rule, rulestr, sizeof(rulestr))); +} + +/* + * for_each_chunk - Step through delimited chunks of a string + * @p_: Pointer to start of each chunk (updated) + * @ep_: Pointer to end of each chunk (updated) + * @s_: String to step through + * @sep_: String of all allowed delimiters + */ +#define for_each_chunk(p_, ep_, s_, sep_) \ + for ((p_) = (s_); \ + (ep_) = (p_) + strcspn((p_), (sep_)), *(p_); \ + (p_) = *(ep_) ? (ep_) + 1 : (ep_)) + +/** + * fwd_rule_parse_ports() - Parse port range(s) specifier + * @fwd: Forwarding table to be updated + * @proto: Protocol to forward + * @addr: Listening address for forwarding + * @ifname: Interface name for listening + * @spec: Port range(s) specifier + */ +static void fwd_rule_parse_ports(struct fwd_table *fwd, uint8_t proto, + const union inany_addr *addr, + const char *ifname, + const char *spec) +{ + uint8_t exclude[PORT_BITMAP_SIZE] = { 0 }; + bool exclude_only = true; + const char *p, *ep; + uint8_t flags = 0; + unsigned i; + + if (!strcmp(spec, "all")) { + /* Treat "all" as equivalent to "": all non-ephemeral ports */ + spec = ""; + } + + /* Parse excluded ranges and "auto" in the first pass */ + for_each_chunk(p, ep, spec, ",") { + struct port_range xrange; + + if (isdigit(*p)) { + /* Include range, parse later */ + exclude_only = false; + continue; + } + + if (parse_keyword(p, &p, "auto") == 0) { + if (p != ep) /* Garbage after the keyword */ + goto bad; + + if (!(fwd->caps & FWD_CAP_SCAN)) { + die( +"'auto' port forwarding is only allowed for pasta"); + } + + flags |= FWD_SCAN; + continue; + } + + /* Should be an exclude range */ + if (*p != '~') + goto bad; + p++; + + if (parse_port_range(p, &p, &xrange)) + goto bad; + if (p != ep) /* Garbage after the range */ + goto bad; + + for (i = xrange.first; i <= xrange.last; i++) + bitmap_set(exclude, i); + } + + if (exclude_only) { + /* Exclude ephemeral ports */ + fwd_port_map_ephemeral(exclude); + + fwd_rule_range_except(fwd, proto, addr, ifname, + 1, NUM_PORTS - 1, exclude, + 1, flags | FWD_WEAK); + return; + } + + /* Now process base ranges, skipping exclusions */ + for_each_chunk(p, ep, spec, ",") { + struct port_range orig_range, mapped_range; + + if (!isdigit(*p)) + /* Already parsed */ + continue; + + if (parse_port_range(p, &p, &orig_range)) + goto bad; + + if (*p == ':') { /* There's a range to map to as well */ + if (parse_port_range(p + 1, &p, &mapped_range)) + goto bad; + if ((mapped_range.last - mapped_range.first) != + (orig_range.last - orig_range.first)) + goto bad; + } else { + mapped_range = orig_range; + } + + if (p != ep) /* Garbage after the ranges */ + goto bad; + + fwd_rule_range_except(fwd, proto, addr, ifname, + orig_range.first, orig_range.last, + exclude, + mapped_range.first, flags); + } + + return; +bad: + die("Invalid port specifier '%s'", spec); +} + +/** + * fwd_rule_parse() - Parse port configuration option + * @optname: Short option name, t, T, u, or U + * @optarg: Option argument (port specification) + * @fwd: Forwarding table to be updated + */ +void fwd_rule_parse(char optname, const char *optarg, struct fwd_table *fwd) +{ + union inany_addr addr_buf = inany_any6, *addr = &addr_buf; + char buf[BUFSIZ], *spec, *ifname = NULL; + uint8_t proto; + + if (optname == 't' || optname == 'T') + proto = IPPROTO_TCP; + else if (optname == 'u' || optname == 'U') + proto = IPPROTO_UDP; + else + assert(0); + + if (!strcmp(optarg, "none")) { + unsigned i; + + for (i = 0; i < fwd->count; i++) { + if (fwd->rules[i].proto == proto) { + die("-%c none conflicts with previous options", + optname); + } + } + return; + } + + strncpy(buf, optarg, sizeof(buf) - 1); + + if ((spec = strchr(buf, '/'))) { + *spec = 0; + spec++; + + if (optname != 't' && optname != 'u') + die("Listening address not allowed for -%c %s", + optname, optarg); + + if ((ifname = strchr(buf, '%'))) { + *ifname = 0; + ifname++; + + /* spec is already advanced one past the '/', + * so the length of the given ifname is: + * (spec - ifname - 1) + */ + if (spec - ifname - 1 >= IFNAMSIZ) { + die("Interface name '%s' is too long (max %u)", + ifname, IFNAMSIZ - 1); + } + } + + if (ifname == buf + 1) { /* Interface without address */ + addr = NULL; + } else { + char *p = buf; + + /* Allow square brackets for IPv4 too for convenience */ + if (*p == '[' && p[strlen(p) - 1] == ']') { + p[strlen(p) - 1] = '\0'; + p++; + } + + if (!inany_pton(p, addr)) + die("Bad forwarding address '%s'", p); + } + } else { + spec = buf; + + addr = NULL; + } + + if (optname == 'T' || optname == 'U') { + assert(!addr && !ifname); + + if (!(fwd->caps & FWD_CAP_IFNAME)) { + warn( +"SO_BINDTODEVICE unavailable, forwarding only 127.0.0.1 and ::1 for '-%c %s'", + optname, optarg); + + if (fwd->caps & FWD_CAP_IPV4) { + fwd_rule_parse_ports(fwd, proto, + &inany_loopback4, NULL, + spec); + } + if (fwd->caps & FWD_CAP_IPV6) { + fwd_rule_parse_ports(fwd, proto, + &inany_loopback6, NULL, + spec); + } + return; + } + + ifname = "lo"; + } + + if (ifname && !(fwd->caps & FWD_CAP_IFNAME)) { + die( +"Device binding for '-%c %s' unsupported (requires kernel 5.7+)", + optname, optarg); + } + + fwd_rule_parse_ports(fwd, proto, addr, ifname, spec); +} diff --git a/fwd_rule.h b/fwd_rule.h index 5c7b67aa..f0f4efda 100644 --- a/fwd_rule.h +++ b/fwd_rule.h @@ -19,6 +19,7 @@ /* Number of ports for both TCP and UDP */ #define NUM_PORTS (1U << 16) +#define PORT_BITMAP_SIZE DIV_ROUND_UP(NUM_PORTS, 8) /* Forwarding capability bits */ #define FWD_CAP_IPV4 BIT(0) @@ -54,8 +55,38 @@ struct fwd_rule { uint8_t flags; }; +#define FWD_RULE_BITS 8 +#define MAX_FWD_RULES MAX_FROM_BITS(FWD_RULE_BITS) + +/* Maximum number of listening sockets (per pif) + * + * Rationale: This lets us listen on every port for two addresses and two + * protocols (which we need for -T auto -U auto without SO_BINDTODEVICE), plus a + * comfortable number of extras. + */ +#define MAX_LISTEN_SOCKS (NUM_PORTS * 5) + +/** + * struct fwd_table - Forwarding state (per initiating pif) + * @caps: Forwarding capabilities for this initiating pif + * @count: Number of forwarding rules + * @rules: Array of forwarding rules + * @rulesocks: Parallel array of @rules (@count valid entries) of pointers to + * @socks entries giving the start of the corresponding rule's + * sockets within the larger array + * @sock_count: Number of entries used in @socks (for all rules combined) + * @socks: Listening sockets for forwarding + */ +struct fwd_table { + uint32_t caps; + unsigned count; + struct fwd_rule rules[MAX_FWD_RULES]; + int *rulesocks[MAX_FWD_RULES]; + unsigned sock_count; + int socks[MAX_LISTEN_SOCKS]; +}; + void fwd_probe_ephemeral(void); -void fwd_port_map_ephemeral(uint8_t *map); #define FWD_RULE_STRLEN \ (IPPROTO_STRLEN - 1 \ @@ -67,7 +98,6 @@ void fwd_port_map_ephemeral(uint8_t *map); const union inany_addr *fwd_rule_addr(const struct fwd_rule *rule); const char *fwd_rule_fmt(const struct fwd_rule *rule, char *dst, size_t size); void fwd_rules_info(const struct fwd_rule *rules, size_t count); -void fwd_rule_conflict_check(const struct fwd_rule *new, - const struct fwd_rule *rules, size_t count); +void fwd_rule_parse(char optname, const char *optarg, struct fwd_table *fwd); #endif /* FWD_RULE_H */ -- 2.53.0

On Fri, 10 Apr 2026 11:02:57 +1000 David Gibson wrote:

fwd_rule_add() sanity checks the given rule, however all errors are fatal: either they're assert()s in the case of things that callers should have already verified, or die()s if we run out of space for the new rule.

This won't suffice any more when we allow rule updates from a configuration client. We don't want to trust the input we get from the client any more than we have to.

Replace the assert()s and die()s with a return value. Also include warn()s so that the user gets a more specific idea of the problem in the logs or stderr.

Signed-off-by: David Gibson --- conf.c | 15 ++++++++++++--- fwd.c | 41 +++++++++++++++++++++++++++++++---------- fwd.h | 2 +- fwd_rule.c | 3 +-- fwd_rule.h | 1 + 5 files changed, 46 insertions(+), 16 deletions(-)

diff --git a/conf.c b/conf.c index b871646f..5c913820 100644 --- a/conf.c +++ b/conf.c @@ -157,6 +157,7 @@ static void conf_ports_range_except(const struct ctx *c, char optname, .proto = proto, .flags = flags, }; + char rulestr[FWD_RULE_STRLEN]; unsigned delta = to - first; unsigned base, i;

@@ -207,20 +208,28 @@ static void conf_ports_range_except(const struct ctx *c, char optname, rulev.addr = inany_loopback4; fwd_rule_conflict_check(&rulev, fwd->rules, fwd->count); - fwd_rule_add(fwd, &rulev); + if (fwd_rule_add(fwd, &rulev) < 0) + goto fail; } if (c->ifi6) { rulev.addr = inany_loopback6; fwd_rule_conflict_check(&rulev, fwd->rules, fwd->count); - fwd_rule_add(fwd, &rulev); + if (fwd_rule_add(fwd, &rulev) < 0) + goto fail; } } else { fwd_rule_conflict_check(&rule, fwd->rules, fwd->count); - fwd_rule_add(fwd, &rule); + if (fwd_rule_add(fwd, &rule) < 0) + goto fail; } base = i - 1; } + return; + +fail: + die("Unable to add rule %s", + fwd_rule_fmt(&rule, rulestr, sizeof(rulestr)));

Similar to my comment to 10/23: I expected this die() to be gone by the end of the series, but it's still there. Is it just a placeholder die() that will go once pesto(1) is added? -- Stefano

On Fri, 10 Apr 2026 11:03:02 +1000 David Gibson wrote:

Currently -[tTuU] all is handled separately in conf_ports() before calling conf_ports_spec(). Earlier changes mean we can now move this handling to conf_ports_spec(). This makes the code slightly simpler, but more importantly it allows some useful combinations we couldn't previously do, such as -t 127.0.0.1/all or -u %eth2/all

Signed-off-by: David Gibson --- conf.c | 25 ++++++++++--------------- passt.1 | 28 ++++++++++++++++++++-------- 2 files changed, 30 insertions(+), 23 deletions(-)

diff --git a/conf.c b/conf.c index 5d6517c3..f62109b5 100644 --- a/conf.c +++ b/conf.c @@ -251,6 +251,11 @@ static void conf_ports_spec(const struct ctx *c, const char *p, *ep; unsigned i;

+ if (!strcmp(spec, "all")) { + /* Treat "all" as equivalent to "": all non-ephemeral ports */ + spec = ""; + } + /* Mark all exclusions first, they might be given after base ranges */ for_each_chunk(p, ep, spec, ",") { struct port_range xrange; @@ -372,19 +377,6 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg, return; }

- if (!strcmp(optarg, "all")) { - uint8_t exclude[PORT_BITMAP_SIZE] = { 0 }; - - /* Exclude ephemeral ports */ - fwd_port_map_ephemeral(exclude); - - conf_ports_range_except(c, optname, optarg, fwd, - proto, NULL, NULL, - 1, NUM_PORTS - 1, exclude, - 1, FWD_WEAK); - return; - } - strncpy(buf, optarg, sizeof(buf) - 1);

if ((spec = strchr(buf, '/'))) { @@ -1039,14 +1031,17 @@ static void usage(const char *name, FILE *f, int status) " can be specified multiple times\n" " SPEC can be:\n" " 'none': don't forward any ports\n" - " 'all': forward all unbound, non-ephemeral ports\n" "%s" " [ADDR[%%IFACE]/]PORTS: forward specific ports\n" - " PORTS is a comma-separated list of ports, optionally\n" + " PORTS is either 'all' (forward all unbound, non-ephemeral\n" + " ports), or a comma-separated list of ports, optionally\n" " ranged with '-' and optional target ports after ':'.\n" " Ranges can be reduced by excluding ports or ranges\n" " prefixed by '~'\n" " Examples:\n" + " -t all Forward all ports\n"

Nit: the examples below have a tab as a separator, which makes it slightly easier to ensure we indent them properly.

+ " -t 127.0.0.1/all Forward all ports from local address\n" + " 127.0.0.1\n"

This makes things pretty hard on eyes as it's not consistent with the rest of the "table". Could we perhaps do: " -t ::1/all Forward all ports from ::1\n" ?

" -t 22 Forward local port 22 to 22 on %s\n" " -t 22:23 Forward local port 22 to 23 on %s\n" " -t 22,25 Forward ports 22, 25 to ports 22, 25\n" diff --git a/passt.1 b/passt.1 index d329f8f0..3ba447d5 100644 --- a/passt.1 +++ b/passt.1 @@ -434,12 +434,6 @@ Configure TCP port forwarding to guest or namespace. \fIspec\fR can be one of: .BR none Don't forward any ports

-.TP -.BR all -Forward all unbound, non-ephemeral ports, as permitted by current capabilities. -For low (< 1024) ports, see \fBNOTES\fR. No failures are reported for -unavailable ports, unless no ports could be forwarded at all. - .TP .BR auto " " (\fBpasta\fR " " only) Dynamically forward ports bound in the namespace. The list of ports is @@ -449,10 +443,20 @@ periodically derived (every second) from listening sockets reported by .TP [\fIaddress\fR[\fB%\fR\fIinterface\fR]\fB/\fR]\fIports\fR ... Specific ports to forward. Optionally, a specific listening address -and interface name (since Linux 5.7) can be specified. \fIports\fR is -a comma-separated list of entries which may be any of: +and interface name (since Linux 5.7) can be specified. \fIports\fR +may be either: .RS .TP +\fBall\fR +Forward all unbound, non-ephemeral ports, as permitted by current +capabilities. For low (< 1024) ports, see \fBNOTES\fR. No failures +are reported for unavailable ports, unless no ports could be forwarded +at all. +.RE + +.RS +or a comma-separated list of entries which may be any of: +.TP \fIfirst\fR[\fB-\fR\fIlast\fR][\fB:\fR\fItofirst\fR[\fB-\fR\fItolast\fR]] Include range. Forward port numbers between \fIfirst\fR and \fIlast\fR (inclusive) to ports between \fItofirst\fR and \fItolast\fR. If @@ -473,6 +477,14 @@ unavailable ports, unless no ports could be forwarded at all. Examples: .RS .TP +-t all +Forward all unbound, non-ephemeral ports as permitted by current +capabilities to the corresponding port on the guest or namespace +.TP +-t 127.0.0.1/all +For the local address 127.0.0.1, forward all unbound, non-ephemeral +ports as permitted by current capabilities.

Nit: all the other examples have no dot at the end (I tend to think it fits better this type of list, but all I care about is that it's consistent).

+.TP -t 22 Forward local port 22 to port 22 on the guest or namespace .TP

-- Stefano

On Fri, 10 Apr 2026 11:03:07 +1000 David Gibson wrote:

Although fwd_rule_add() performs some sanity checks on the rule it is given, there are invalid rules we don't check for, assuming that its callers will do that.

That won't be enough when we can get rules inserted by a dynamic update client without going through the existing parsing code. So, add stricter checks to fwd_rule_add(), which is now possible thanks to the capabilities bits in the struct fwd_table. Where those duplicate existing checks in the callers, remove the old copies.

Signed-off-by: David Gibson --- conf.c | 19 ------------------- fwd.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 46 insertions(+), 24 deletions(-)

diff --git a/conf.c b/conf.c index b7a4459e..31627209 100644 --- a/conf.c +++ b/conf.c @@ -310,10 +310,6 @@ static void conf_ports_spec(struct fwd_table *fwd, uint8_t proto, if (p != ep) /* Garbage after the ranges */ goto bad;

- if (orig_range.first == 0) { - die("Can't forward port 0 included in '%s'", spec); - } - conf_ports_range_except(fwd, proto, addr, ifname, orig_range.first, orig_range.last, exclude, @@ -356,11 +352,6 @@ static void conf_ports(char optname, const char *optarg, struct fwd_table *fwd) return; }

- if (proto == IPPROTO_TCP && !(fwd->caps & FWD_CAP_TCP)) - die("TCP port forwarding requested but TCP is disabled"); - if (proto == IPPROTO_UDP && !(fwd->caps & FWD_CAP_UDP)) - die("UDP port forwarding requested but UDP is disabled"); - strncpy(buf, optarg, sizeof(buf) - 1);

if ((spec = strchr(buf, '/'))) { @@ -405,16 +396,6 @@ static void conf_ports(char optname, const char *optarg, struct fwd_table *fwd) addr = NULL; }

- if (addr) { - if (!(fwd->caps & FWD_CAP_IPV4) && inany_v4(addr)) { - die("IPv4 is disabled, can't use -%c %s", - optname, optarg); - } else if (!(fwd->caps & FWD_CAP_IPV6) && !inany_v4(addr)) { - die("IPv6 is disabled, can't use -%c %s", - optname, optarg); - } - } - if (optname == 'T' || optname == 'U') { assert(!addr && !ifname);

diff --git a/fwd.c b/fwd.c index c7fd1a9d..98b04d0c 100644 --- a/fwd.c +++ b/fwd.c @@ -367,17 +367,58 @@ int fwd_rule_add(struct fwd_table *fwd, const struct fwd_rule *new) new->first, new->last); return -EINVAL; } + if (!new->first) { + warn("Forwarding rule atttempts to map from port 0"); + return -EINVAL; + } + if (!new->to || (new->to + new->last - new->first) < new->to) { + warn("Forwarding rule atttempts to map to port 0");

Nit: attempts

+ return -EINVAL; + } if (new->flags & ~allowed_flags) { warn("Rule has invalid flags 0x%hhx", new->flags & ~allowed_flags); return -EINVAL; } - if (new->flags & FWD_DUAL_STACK_ANY && - !inany_equals(&new->addr, &inany_any6)) { - char astr[INANY_ADDRSTRLEN]; + if (new->flags & FWD_DUAL_STACK_ANY) { + if (!inany_equals(&new->addr, &inany_any6)) { + char astr[INANY_ADDRSTRLEN];

- warn("Dual stack rule has non-wildcard address %s", - inany_ntop(&new->addr, astr, sizeof(astr))); + warn("Dual stack rule has non-wildcard address %s", + inany_ntop(&new->addr, astr, sizeof(astr))); + return -EINVAL; + } + if (!(fwd->caps & FWD_CAP_IPV4)) { + warn("Dual stack forward, but IPv4 not enabled"); + return -EINVAL; + } + if (!(fwd->caps & FWD_CAP_IPV6)) { + warn("Dual stack forward, but IPv6 not enabled"); + return -EINVAL; + } + } else { + if (inany_v4(&new->addr) && !(fwd->caps & FWD_CAP_IPV4)) { + warn("IPv4 forward, but IPv4 not enabled"); + return -EINVAL; + } + if (!inany_v4(&new->addr) && !(fwd->caps & FWD_CAP_IPV6)) { + warn("IPv6 forward, but IPv6 not enabled"); + return -EINVAL; + } + } + if (new->proto == IPPROTO_TCP) { + if (!(fwd->caps & FWD_CAP_TCP)) { + warn("Can't add TCP forwarding rule, TCP not enabled"); + return -EINVAL; + } + } else if (new->proto == IPPROTO_UDP) { + if (!(fwd->caps & FWD_CAP_UDP)) { + warn("Can't add UDP forwarding rule, UDP not enabled"); + return -EINVAL; + } + } else { + warn("Unsupported protocol 0x%hhx (%s) for forwarding rule", + new->proto, ipproto_name(new->proto)); return -EINVAL; }

-- Stefano

On Thu, Apr 16, 2026 at 12:04:28AM +0200, Stefano Brivio wrote:

On Fri, 10 Apr 2026 11:02:56 +1000 David Gibson wrote:

...

Amongst other checks, fwd_rule_add() checks that the newly added rule doesn't conflict with any existing rules. However, unlike the other things we verify, this isn't really required for safe operation. Rule conflicts are a useful thing for the user to know about, but the forwarding logic is perfectly sound with conflicting rules (the first one will win).

In order to support dynamic rule updates, we want fwd_rule_add() to become a more low-level function, only checking the things it really needs to. So, move rule conflict checking to its caller via new helpers in fwd_rule.c.

Signed-off-by: David Gibson --- conf.c | 5 +++++ fwd.c | 26 +------------------------- fwd_rule.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ fwd_rule.h | 2 ++ 4 files changed, 53 insertions(+), 25 deletions(-)

diff --git a/conf.c b/conf.c index 027bbac9..b871646f 100644 --- a/conf.c +++ b/conf.c @@ -205,13 +205,18 @@ static void conf_ports_range_except(const struct ctx *c, char optname,

if (c->ifi4) { rulev.addr = inany_loopback4; + fwd_rule_conflict_check(&rulev, + fwd->rules, fwd->count); fwd_rule_add(fwd, &rulev); } if (c->ifi6) { rulev.addr = inany_loopback6; + fwd_rule_conflict_check(&rulev, + fwd->rules, fwd->count); fwd_rule_add(fwd, &rulev); } } else { + fwd_rule_conflict_check(&rule, fwd->rules, fwd->count); fwd_rule_add(fwd, &rule); } base = i - 1; diff --git a/fwd.c b/fwd.c index c05107d1..c9637525 100644 --- a/fwd.c +++ b/fwd.c @@ -341,7 +341,7 @@ void fwd_rule_add(struct fwd_table *fwd, const struct fwd_rule *new) /* Flags which can be set from the caller */ const uint8_t allowed_flags = FWD_WEAK | FWD_SCAN | FWD_DUAL_STACK_ANY; unsigned num = (unsigned)new->last - new->first + 1; - unsigned i, port; + unsigned port;

assert(!(new->flags & ~allowed_flags)); /* Passing a non-wildcard address with DUAL_STACK_ANY is a bug */ @@ -354,30 +354,6 @@ void fwd_rule_add(struct fwd_table *fwd, const struct fwd_rule *new) if ((fwd->sock_count + num) > ARRAY_SIZE(fwd->socks)) die("Too many listening sockets");

- /* Check for any conflicting entries */ - for (i = 0; i < fwd->count; i++) { - char newstr[INANY_ADDRSTRLEN], rulestr[INANY_ADDRSTRLEN]; - const struct fwd_rule *rule = &fwd->rules[i]; - - if (new->proto != rule->proto) - /* Non-conflicting protocols */ - continue; - - if (!inany_matches(fwd_rule_addr(new), fwd_rule_addr(rule))) - /* Non-conflicting addresses */ - continue; - - if (new->last < rule->first || rule->last < new->first) - /* Port ranges don't overlap */ - continue; - - die("Forwarding configuration conflict: %s/%u-%u versus %s/%u-%u", - inany_ntop(fwd_rule_addr(new), newstr, sizeof(newstr)), - new->first, new->last, - inany_ntop(fwd_rule_addr(rule), rulestr, sizeof(rulestr)), - rule->first, rule->last); - } - fwd->rulesocks[fwd->count] = &fwd->socks[fwd->sock_count]; for (port = new->first; port <= new->last; port++) fwd->rulesocks[fwd->count][port - new->first] = -1; diff --git a/fwd_rule.c b/fwd_rule.c index a034d5d1..5bc94efe 100644 --- a/fwd_rule.c +++ b/fwd_rule.c @@ -93,3 +93,48 @@ void fwd_rules_info(const struct fwd_rule *rules, size_t count) info(" %s", fwd_rule_fmt(&rules[i], buf, sizeof(buf))); } } + +/** + * fwd_rule_conflicts() - Test if two rules conflict with each other + * @a, @b: Rules to test + */ +static bool fwd_rule_conflicts(const struct fwd_rule *a, const struct fwd_rule *b) +{ + if (a->proto != b->proto) + /* Non-conflicting protocols */ + return false; + + if (!inany_matches(fwd_rule_addr(a), fwd_rule_addr(b))) + /* Non-conflicting addresses */ + return false; + + assert(a->first <= a->last && b->first <= b->last);

I expected this assert() to be gone by the end of the series, like the ones dropped in 11/23, but it's still there. Is this something that the client can't specifically trigger for some reason?

Oops, yeah, that's a mistake. What I had in mind is that the rule is first checked for internal consistency (including first <= last), and only then do we conflict check. But then I changed things so that's no longer the case. I'll fix it with followup patches in the next spin. -- 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

On Thu, Apr 16, 2026 at 12:04:51AM +0200, Stefano Brivio wrote:

On Fri, 10 Apr 2026 11:03:03 +1000 David Gibson wrote:

...

The forwarding table now allows for arbitrary port ranges to be marked as FWD_SCAN, meaning we don't open sockets for every port, but only those we scan as listening on the target side. However, there's currently no way to create such rules, except -[tTuU] auto which always scans every port with an unspecified listening address and interface.

Allow user-specified "auto" ranges by moving the parsing of the "auto" keyword from conf_ports(), to conf_ports_spec() as part of the port specified. "auto" can be combined freely with other port ranges, e.g. -t 127.0.0.1/auto -u %lo/5000-7000,auto -T auto,12345 -U auto,~1-9000

Note that any address and interface given only affects where the automatic forwards listen, not what addresses we consider when scanning. That is, if the target side is listening on *any* address, we will create a forward on the specified address.

Link: https://bugs.passt.top/show_bug.cgi?id=180

Signed-off-by: David Gibson --- conf.c | 85 ++++++++++++++++++++++++++++++++++++++++++--------------- passt.1 | 30 ++++++++++++++------ 2 files changed, 85 insertions(+), 30 deletions(-)

diff --git a/conf.c b/conf.c index f62109b5..8e3b4b20 100644 --- a/conf.c +++ b/conf.c @@ -13,6 +13,7 @@ */

#include +#include #include #include #include @@ -112,6 +113,28 @@ static int parse_port_range(const char *s, const char **endptr, return 0; }

+/** + * parse_keyword() - Parse a literal keyword + * @s: String to parse + * @endptr: Update to the character after the keyword + * @kw: Keyword to accept + * + * Return: 0, if @s starts with @kw, -EINVAL if it does not + */ +static int parse_keyword(const char *s, const char **endptr, const char *kw) +{ + size_t len = strlen(kw); + + if (strlen(s) < len) + return -EINVAL; + + if (memcmp(s, kw, len)) + return -EINVAL; + + *endptr = s + len; + return 0; +} + /** * conf_ports_range_except() - Set up forwarding for a range of ports minus a * bitmap of exclusions @@ -249,6 +272,7 @@ static void conf_ports_spec(const struct ctx *c, uint8_t exclude[PORT_BITMAP_SIZE] = { 0 }; bool exclude_only = true; const char *p, *ep; + uint8_t flags = 0; unsigned i;

if (!strcmp(spec, "all")) { @@ -256,15 +280,32 @@ static void conf_ports_spec(const struct ctx *c, spec = ""; }

- /* Mark all exclusions first, they might be given after base ranges */ + /* Parse excluded ranges and "auto" in the first pass */ for_each_chunk(p, ep, spec, ",") { struct port_range xrange;

- if (*p != '~') { - /* Not an exclude range, parse later */ + if (isdigit(*p)) { + /* Include range, parse later */ exclude_only = false; continue; } + + if (parse_keyword(p, &p, "auto") == 0) { + if (p != ep) /* Garbage after the keyword */ + goto bad; + + if (c->mode != MODE_PASTA) { + die( +"'auto' port forwarding is only allowed for pasta"); + } + + flags |= FWD_SCAN; + continue; + } + + /* Should be an exclude range */ + if (*p != '~') + goto bad; p++;

if (parse_port_range(p, &p, &xrange)) @@ -283,7 +324,7 @@ static void conf_ports_spec(const struct ctx *c, conf_ports_range_except(c, optname, optarg, fwd, proto, addr, ifname, 1, NUM_PORTS - 1, exclude, - 1, FWD_WEAK); + 1, flags | FWD_WEAK); return; }

@@ -291,8 +332,8 @@ static void conf_ports_spec(const struct ctx *c, for_each_chunk(p, ep, spec, ",") { struct port_range orig_range, mapped_range;

- if (*p == '~') - /* Exclude range, already parsed */ + if (!isdigit(*p)) + /* Already parsed */ continue;

if (parse_port_range(p, &p, &orig_range)) @@ -320,7 +361,7 @@ static void conf_ports_spec(const struct ctx *c, proto, addr, ifname, orig_range.first, orig_range.last, exclude, - mapped_range.first, 0); + mapped_range.first, flags); }

return; @@ -366,17 +407,6 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg, if (proto == IPPROTO_UDP && c->no_udp) die("UDP port forwarding requested but UDP is disabled");

- if (!strcmp(optarg, "auto")) { - if (c->mode != MODE_PASTA) - die("'auto' port forwarding is only allowed for pasta"); - - conf_ports_range_except(c, optname, optarg, fwd, - proto, NULL, NULL, - 1, NUM_PORTS - 1, NULL, 1, FWD_SCAN); - - return; - } - strncpy(buf, optarg, sizeof(buf) - 1);

if ((spec = strchr(buf, '/'))) { @@ -1031,13 +1061,13 @@ static void usage(const char *name, FILE *f, int status) " can be specified multiple times\n" " SPEC can be:\n" " 'none': don't forward any ports\n" - "%s" " [ADDR[%%IFACE]/]PORTS: forward specific ports\n" " PORTS is either 'all' (forward all unbound, non-ephemeral\n" " ports), or a comma-separated list of ports, optionally\n" " ranged with '-' and optional target ports after ':'.\n" " Ranges can be reduced by excluding ports or ranges\n" - " prefixed by '~'\n" + " prefixed by '~'.\n" + "%s" " Examples:\n" " -t all Forward all ports\n" " -t 127.0.0.1/all Forward all ports from local address\n" @@ -1051,15 +1081,26 @@ static void usage(const char *name, FILE *f, int status) " -t 192.0.2.1/5 Bind port 5 of 192.0.2.1 to %s\n" " -t 5-25,~10-20 Forward ports 5 to 9, and 21 to 25\n" " -t ~25 Forward all ports except for 25\n" + "%s" " default: %s\n" " -u, --udp-ports SPEC UDP port forwarding to %s\n" " SPEC is as described for TCP above\n" " default: %s\n", guest, strstr(name, "pasta") ? - " 'auto': forward all ports currently bound in namespace\n" + " The 'auto' keyword may be given to only forward\n" + " ports which are bound in the target namespace\n" + : "", + guest, guest, guest, + strstr(name, "pasta") ? + " -t auto Forward all ports bound in namespace\n" + " -t 192.0.2.2/auto Forward ports from 192.0.2.2 if\n" + " they are bound in the namespace\n" + " -t 8000-8010,auto Forward ports 8000-8010 if they\n" + " are bound in the namespace\n"

The whole thing is now rendered as:

Examples: -t all Forward all ports -t 127.0.0.1/all Forward all ports from local address 127.0.0.1 -t 22 Forward local port 22 to 22 on namespace -t 22:23 Forward local port 22 to 23 on namespace -t 22,25 Forward ports 22, 25 to ports 22, 25 -t 22-80 Forward ports 22 to 80 -t 22-80:32-90 Forward ports 22 to 80 to corresponding port numbers plus 10 -t 192.0.2.1/5 Bind port 5 of 192.0.2.1 to namespace -t 5-25,~10-20 Forward ports 5 to 9, and 21 to 25 -t ~25 Forward all ports except for 25 -t auto Forward all ports bound in namespace -t 192.0.2.2/auto Forward ports from 192.0.2.2 if they are bound in the namespace -t 8000-8010,auto Forward ports 8000-8010 if they are bound in the namespace

I think this could be:

" -t auto\t Forward all ports bound in namespace\n" " -t ::1/auto Forward ports from ::1 if bound in\n" " namespace\n" " -t 80-82,auto Forward ports 80-82 if bound in\n" " namespace\n"

instead.

Good ideas, done. -- 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

On Thu, Apr 16, 2026 at 12:04:38AM +0200, Stefano Brivio wrote:

On Fri, 10 Apr 2026 11:03:01 +1000 David Gibson wrote:

...

Currently the man page describes the internal syntax of port specifiers in prose, which isn't particularly easy to follow. Rework it to use more syntax "diagrams" to show how it works. This will also allow us to more easily update the manual page for some coming changes in syntax.

usage() output is updated similarly, though more briefly.

Signed-off-by: David Gibson --- conf.c | 10 +++++----- passt.1 | 32 ++++++++++++++++++++++---------- 2 files changed, 27 insertions(+), 15 deletions(-)

diff --git a/conf.c b/conf.c index c3655824..5d6517c3 100644 --- a/conf.c +++ b/conf.c @@ -1041,11 +1041,11 @@ static void usage(const char *name, FILE *f, int status) " 'none': don't forward any ports\n" " 'all': forward all unbound, non-ephemeral ports\n" "%s" - " a comma-separated list, optionally ranged with '-'\n" - " and optional target ports after ':', with optional\n" - " address specification suffixed by '/' and optional\n" - " interface prefixed by '%%'. Ranges can be reduced by\n" - " excluding ports or ranges prefixed by '~'\n" + " [ADDR[%%IFACE]/]PORTS: forward specific ports\n" + " PORTS is a comma-separated list of ports, optionally\n" + " ranged with '-' and optional target ports after ':'.\n" + " Ranges can be reduced by excluding ports or ranges\n" + " prefixed by '~'\n" " Examples:\n" " -t 22 Forward local port 22 to 22 on %s\n" " -t 22:23 Forward local port 22 to 23 on %s\n" diff --git a/passt.1 b/passt.1 index 7da4fe5f..d329f8f0 100644 --- a/passt.1 +++ b/passt.1 @@ -447,16 +447,28 @@ periodically derived (every second) from listening sockets reported by \fI/proc/net/tcp\fR and \fI/proc/net/tcp6\fR, see \fBproc\fR(5).

.TP -.BR ports -A comma-separated list of ports, optionally ranged with \fI-\fR, and, -optionally, with target ports after \fI:\fR, if they differ. Specific addresses -can be bound as well, separated by \fI/\fR, and also, since Linux 5.7, limited -to specific interfaces, prefixed by \fI%\fR. Within given ranges, selected ports -and ranges can be excluded by an additional specification prefixed by \fI~\fR. - -Specifying excluded ranges only implies that all other ports are forwarded. In -this case, no failures are reported for unavailable ports, unless no ports could -be forwarded at all. +[\fIaddress\fR[\fB%\fR\fIinterface\fR]\fB/\fR]\fIports\fR ... +Specific ports to forward. Optionally, a specific listening address +and interface name (since Linux 5.7) can be specified. \fIports\fR is +a comma-separated list of entries which may be any of: +.RS +.TP +\fIfirst\fR[\fB-\fR\fIlast\fR][\fB:\fR\fItofirst\fR[\fB-\fR\fItolast\fR]] +Include range. Forward port numbers between \fIfirst\fR and \fIlast\fR +(inclusive) to ports between \fItofirst\fR and \fItolast\fR. If +\fItofirst\fR and \fItolast\fR are omitted, assume the same as +\fIfirst\fR and \fIlast\fR. If \fIlast\fR is omitted, assume the same +as \fIfirst\fR. + +.TP +\fB~\fR\fIfirst\fR[\fB-\fR\fIlast\fR] +Exclude range. Exclude port numbers between \fIfirst\fR and +\fIlast\fR from. This takes precedences over include ranges.

..."from the set of all non-ephemeral ports permitted by current capabilities"?

Or simply drop " from", because it should be clear from the paragraph below?

Uh, yeah, not entirely sure what I was going for there. I've changed this to \fB~\fR\fIfirst\fR[\fB-\fR\fIlast\fR] Exclude range. Don't forward port numbers between \fIfirst\fR and \fIlast\fR. This takes precedences over include ranges.

...

+.RE + +Specifying excluded ranges only implies that all other non-ephemeral +ports are forwarded. In this case, no failures are reported for +unavailable ports, unless no ports could be forwarded at all.

Examples: .RS

-- Stefano

-- 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