From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: passt.top; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=eaqlgzmB; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by passt.top (Postfix) with ESMTPS id D46275A0262 for ; Wed, 08 Apr 2026 01:16:34 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1775603793; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Vt4Nxn0hEPwPJJFij9EmFrSYae3Idryc0DXe45ejjLM=; b=eaqlgzmBM/n5YqgtVtMAnrybqO5L1aA0VPlgYtXi//WR1diS13Ky9xMxoUUsCeQ35jPfSI NEHszJPL36kBu9A4KuR+xufnABX3nAj66FBT/RcCclM7swaHo8FNUTbMT1eBsiiBDpFf9R TOkhHJ1a4u44a0BpUbdy25VP+W13DnY= Received: from mail-wr1-f70.google.com (mail-wr1-f70.google.com [209.85.221.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-159-F4aTidJkPqun3andZS5-pQ-1; Tue, 07 Apr 2026 19:16:32 -0400 X-MC-Unique: F4aTidJkPqun3andZS5-pQ-1 X-Mimecast-MFC-AGG-ID: F4aTidJkPqun3andZS5-pQ_1775603791 Received: by mail-wr1-f70.google.com with SMTP id ffacd0b85a97d-43d034589d0so5592280f8f.1 for ; Tue, 07 Apr 2026 16:16:32 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1775603791; x=1776208591; h=date:content-transfer-encoding:mime-version:organization:references :in-reply-to:message-id:subject:cc:to:from:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=Vt4Nxn0hEPwPJJFij9EmFrSYae3Idryc0DXe45ejjLM=; b=KPUyNb8Ivb+iXX18S3lLUffRr2FEIt+SDhZKHzZVnPgLTtl8hujFv4j69MVj82KPSQ DW9evVgMdBtGE8i8I4s3meCU8tlKERLOE6VnFgnqnWHUKrNay8/nHgIv69YPX3fW0cEe +o7aIIjvB6jtPcTRdSd3rMStrzuSo429pKFZJDSKLJcbgyA7v1+hisiTwI2egOipTrLG qq4uaBmqTmRQ8FHHKx6oJotVEy1/2UD6KG9QqOpNaAJCPDWQK67byVXIXLFY9FgQs7OT O/ZY05nJJlJ0lAJBy7AKfC81oIFsvEqRhaCVgp5JS8Paibgihs3i0ftLnRTtldzJ2tzE lv7A== X-Gm-Message-State: AOJu0Yw1q75oL01g2eOVyAE84+GeqH6kY2wYiIRXFbRk+fyJERY96R8P GKC2PbNYUrytER57YvlTIkHmeZ8MHvbIbMKFwDMO/Dl3fY6Eok4vaEr6IEcuTkEi+synXzmtG2q HG1QJJwcQZXiGz6sHgACbsrXtWQ2R5tAbO9MZNtYWs2kkRPLk/jRapFsHNk1UVg== X-Gm-Gg: AeBDieskUFjTz8sdXVIUArzjs3H3UkW4/3HNieUmfPxoTyjPOMUnungH5JCmjS4o7IT 0c9MAkJIWSZXzTAfKT6sKyP3EGYi1Z6lBUJV0dFDmZzCMv9SKZhtmFgKIb0vIxte9VqmckgZ739 93A1k0JF6o718i2dONISP9HYCWF6ogheaFpeQBC+3TcRObbjh1vBynIvq/TIbT6zA1WrYDSkxIg mhxfQ6l93QECxfkjGWYWZ+7/hxX4ts65dDBJmPUWT+9nJ3V6Bo3dV0RYoXCdLlXKIsZ5m1cQhzT ChsTUdqTwGxVdKsCL6IbT5qdwk/+cpGYi1EhFuZ7fQBtkcznH7nAMKJuNq7Sae09mADN+5F/XS3 nVMHjhZkZ5iAa/Uz9GoKxFuybXsCO3s67xbHqaacr6Y7xoH3qlA== X-Received: by 2002:a05:6000:25ca:b0:43d:b99:bdc3 with SMTP id ffacd0b85a97d-43d292cba48mr28573328f8f.26.1775603790607; Tue, 07 Apr 2026 16:16:30 -0700 (PDT) X-Received: by 2002:a05:6000:25ca:b0:43d:b99:bdc3 with SMTP id ffacd0b85a97d-43d292cba48mr28573294f8f.26.1775603790012; Tue, 07 Apr 2026 16:16:30 -0700 (PDT) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [176.103.220.4]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-43d1e4d58e5sm54131097f8f.23.2026.04.07.16.15.01 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 07 Apr 2026 16:15:42 -0700 (PDT) From: Stefano Brivio To: David Gibson Subject: Re: [PATCH 06/18] fwd: Better split forwarding rule specification from associated sockets Message-ID: <20260408011445.275ff479@elisabeth> In-Reply-To: <20260407031630.2457081-7-david@gibson.dropbear.id.au> References: <20260407031630.2457081-1-david@gibson.dropbear.id.au> <20260407031630.2457081-7-david@gibson.dropbear.id.au> Organization: Red Hat X-Mailer: Claws Mail 4.2.0 (GTK 3.24.49; x86_64-pc-linux-gnu) MIME-Version: 1.0 Date: Wed, 08 Apr 2026 01:14:46 +0200 (CEST) X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: kkruRs2iluomK2tvnsB2FEu2kSj1viDr--ADREjCgEc_1775603791 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: SXP6DYUYXE5K2XWTOYBSNL4BY7ZUJDGZ X-Message-ID-Hash: SXP6DYUYXE5K2XWTOYBSNL4BY7ZUJDGZ X-MailFrom: sbrivio@redhat.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On Tue, 7 Apr 2026 13:16:18 +1000 David Gibson wrote: > 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. Indeed, I hated it, in that short moment I had to fiddle with it. Thanks for coming up with a cleaner solution. > > 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 | 16 ++++--------- > 2 files changed, 41 insertions(+), 48 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..83ee9b2e 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,17 @@ 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 > + * @rulesocks: Pointers to socket arrays per-rule I don't see this as particularly descriptive (which sockets? What's the array size?). I'm thinking of something like: @socks_ref: Per-rule pointers to associated @socks, @sock_count of them > * @sock_count: Number of entries used in @socks > * @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]; > }; -- Stefano