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=DGrx/okX; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by passt.top (Postfix) with ESMTPS id D24B35A0623 for ; Tue, 13 Jan 2026 23:12:33 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1768342352; 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=whPie6e/b8DDOld0h79Uw2+55Qm3DOUJtzBabL4OczQ=; b=DGrx/okX+ZvLsmkr9b+CABPUpsnTDj4kAwwupZTi5LUuTE/PJXkH/RvhJvknqr6XrPsKn3 YJ9Z0/TEe1pYkTBqpuS2k05SjrLT3unREpOW6M1yGN4gfnfX7bTeetrTYu11pMLqmzW4t5 QErm1Pt0EYy5z1/J6KCx38CEWdwOMfw= Received: from mail-wr1-f72.google.com (mail-wr1-f72.google.com [209.85.221.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-622-fMGzPc35M1qHYgwlV9UDoA-1; Tue, 13 Jan 2026 17:12:21 -0500 X-MC-Unique: fMGzPc35M1qHYgwlV9UDoA-1 X-Mimecast-MFC-AGG-ID: fMGzPc35M1qHYgwlV9UDoA_1768342340 Received: by mail-wr1-f72.google.com with SMTP id ffacd0b85a97d-430fd96b2f5so5863969f8f.3 for ; Tue, 13 Jan 2026 14:12:21 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1768342340; x=1768947140; h=content-transfer-encoding:mime-version:organization:references :in-reply-to:message-id:subject:cc:to:from:date:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=whPie6e/b8DDOld0h79Uw2+55Qm3DOUJtzBabL4OczQ=; b=b/k0lhOSHZ3oju33Fa7jpXDXnS410TKxG5p4/W8Cu6diP8niUsTxZaWtnij6F4YcB5 jOwukT6zXEdGkFWsDvSbCeoYXN8JCpTME2GZ7SqpKAwHrZt8ML/YFtk0xmM9nkJe9mtO 1mDxMasEhRZK8LdZajz6zuby9jXAABKPP3GFzpm6+X5IimWiLMkwB8grkb7TFWGT+3eg zXD1SlnK2Oec8EduuCYUpxBI7JXn0/QAKBAOBBn/C4eDQf6aSLzYsDYh80lNa0dkr553 2BID7VdMJ/pOBirwHMQaqTaVKGwYNZL1Yrm77TeDGSIfYH++ryMa5wx7thuQhkLAQt8K XYdw== X-Gm-Message-State: AOJu0Ywje6cAsKkxMtRs9grjK2olICTkp0jiafdlxPAP9wO3arE3GX4G 2twwK4XJwVGBzbZf/j+W5l+oWDIKkNcVM2ZE0E+lgRzrbVslbqLhxynYX3+z7iPFAHmzybYCqH1 7EVpwNHQ7bzE6ccLmcCwA5EUJCA6ngadW1nnEIrDu6zOr+GDWzLQQfA== X-Gm-Gg: AY/fxX508nu8qmh3COb8veYgvIpZfHlJ6qcDqyv6Xw5HRa83KXS3WOB41Y+l7Tj9lvO qhAPNJWOz6t5Hd8O3Hhxlfjrjbt4J/zPoN0xqdRJY3emgY8MYzYcvGEMuOXzPhrVHHQ7KVEL/J3 Q59vQCHChUAPAzZE7yXNeGQBXQ2JbQXFMarCtcosX4OGbT9VQi+Mm6O1kEuUDlW2p17sd/3c+9M gHdpQAHzcRqjPy3Qn1r6yhxM48Bjuwn3bTh1rgmr0HJyCZmh8SyE50OQuNcLX89svKEY8E5vd+w cmYkuV5naoc5t9LrK0VMqsLL79KbHFYzYVbfdkSRJdjEyj6FBjcpJHRc1XNDTWXB+hQuy1BHUTN wCFkVz6sLKsdwgwEt/TS+L9E1B3km3Cb+DLubug== X-Received: by 2002:a05:6000:290e:b0:430:f5dc:d34a with SMTP id ffacd0b85a97d-4342c543ba7mr458221f8f.29.1768342339923; Tue, 13 Jan 2026 14:12:19 -0800 (PST) X-Received: by 2002:a05:6000:290e:b0:430:f5dc:d34a with SMTP id ffacd0b85a97d-4342c543ba7mr458200f8f.29.1768342339335; Tue, 13 Jan 2026 14:12:19 -0800 (PST) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [176.103.220.4]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-432bd0860f5sm45980533f8f.0.2026.01.13.14.12.18 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 13 Jan 2026 14:12:18 -0800 (PST) Date: Tue, 13 Jan 2026 23:12:01 +0100 From: Stefano Brivio To: David Gibson Subject: Re: [PATCH v3 11/14] flow, fwd: Consult rules table when forwarding a new flow from socket Message-ID: <20260113231201.09f44967@elisabeth> In-Reply-To: <20260108022948.2657573-12-david@gibson.dropbear.id.au> References: <20260108022948.2657573-1-david@gibson.dropbear.id.au> <20260108022948.2657573-12-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 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: I-DCcuYOwwJpc63DMaXClOHyn83-2tS__7tEVI7zpLE_1768342340 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: NH7MOKD2M56FCVFYQAWE4OW7HH4MX7N6 X-Message-ID-Hash: NH7MOKD2M56FCVFYQAWE4OW7HH4MX7N6 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: Silly nits only and a couple of remarks that will probably be clarified at a later point: On Thu, 8 Jan 2026 13:29:45 +1100 David Gibson wrote: > We now have a formal array of forwarding rules. However, we don't actually > consult it when we forward a new flow. Instead we rely on (a) implicit > information (we wouldn't be here if there wasn't a listening socket for the > rule) and (b) the legacy delta[] data structure. > > Start addressing this, by searching for a matching forwarding rule when > attempting to forward a new flow. For now this is incomplete: > * We only do this for socket-initiated flows > * We make a potentially costly linear lookup > * We don't actually use the matching rule for anything yet > > We'll address each of those in later patches. > > Signed-off-by: David Gibson > --- > flow.c | 43 ++++++++++++++++++++++++++++++++++--------- > fwd.c | 33 +++++++++++++++++++++++++++++++++ > fwd.h | 2 ++ > 3 files changed, 69 insertions(+), 9 deletions(-) > > diff --git a/flow.c b/flow.c > index 4f534865..045e9712 100644 > --- a/flow.c > +++ b/flow.c > @@ -489,7 +489,7 @@ struct flowside *flow_initiate_sa(union flow *flow, uint8_t pif, > struct flowside *flow_target(const struct ctx *c, union flow *flow, > uint8_t proto) > { > - char estr[INANY_ADDRSTRLEN], fstr[INANY_ADDRSTRLEN]; > + char estr[INANY_ADDRSTRLEN], ostr[INANY_ADDRSTRLEN]; > struct flow_common *f = &flow->f; > const struct flowside *ini = &f->side[INISIDE]; > struct flowside *tgt = &f->side[TGTSIDE]; > @@ -500,6 +500,30 @@ struct flowside *flow_target(const struct ctx *c, union flow *flow, > ASSERT(f->pif[INISIDE] != PIF_NONE && f->pif[TGTSIDE] == PIF_NONE); > ASSERT(flow->f.state == FLOW_STATE_INI); > > + if (pif_is_socket(f->pif[INISIDE])) { > + const struct fwd_ports *fwd; > + > + if (f->pif[INISIDE] == PIF_HOST && proto == IPPROTO_TCP) > + fwd = &c->tcp.fwd_in; > + else if (f->pif[INISIDE] == PIF_HOST && proto == IPPROTO_UDP) > + fwd = &c->udp.fwd_in; > + else if (f->pif[INISIDE] == PIF_SPLICE && proto == IPPROTO_TCP) > + fwd = &c->tcp.fwd_out; > + else if (f->pif[INISIDE] == PIF_SPLICE && proto == IPPROTO_UDP) > + fwd = &c->udp.fwd_out; > + else > + goto nofwd; > + > + if (!fwd_rule_search(fwd, ini)) { > + /* This shouldn't happen, because if there's no rule for > + * it we should have no listening socket that would let > + * us get here > + */ > + flow_dbg(flow, "Unexpected missing forward rule"); > + goto nofwd; > + } > + } > + > switch (f->pif[INISIDE]) { > case PIF_TAP: > memcpy(f->tap_omac, MAC_UNDEF, ETH_ALEN); > @@ -514,22 +538,23 @@ struct flowside *flow_target(const struct ctx *c, union flow *flow, > tgtpif = fwd_nat_from_host(c, proto, ini, tgt); > fwd_neigh_mac_get(c, &tgt->oaddr, f->tap_omac); > break; > - > default: > - flow_err(flow, "No rules to forward %s [%s]:%hu -> [%s]:%hu", > - pif_name(f->pif[INISIDE]), > - inany_ntop(&ini->eaddr, estr, sizeof(estr)), > - ini->eport, > - inany_ntop(&ini->oaddr, fstr, sizeof(fstr)), > - ini->oport); > + goto nofwd; > } > > if (tgtpif == PIF_NONE) > - return NULL; > + goto nofwd; > > f->pif[TGTSIDE] = tgtpif; > flow_set_state(f, FLOW_STATE_TGT); > return tgt; > + > +nofwd: > + flow_err(flow, "No rules to forward %s %s [%s]:%hu -> [%s]:%hu", > + pif_name(f->pif[INISIDE]), ipproto_name(proto), > + inany_ntop(&ini->eaddr, estr, sizeof(estr)), ini->eport, > + inany_ntop(&ini->oaddr, ostr, sizeof(ostr)), ini->oport); This assumes we're still using this function for inbound forwards only (eaddr / eport -> oaddr / oport), perhaps we'll want a macro once it starts being used for the other way around as well (if at all). By the way, for rules, earlier in this series, you used "=>" to separate source and target of the forward, here it's still "->", I guess we should settle on one version (it just occurred to me while testing stuff: it might be useful to grep in logs). > + return NULL; > } > > /** > diff --git a/fwd.c b/fwd.c > index 3f088fd2..633ba5db 100644 > --- a/fwd.c > +++ b/fwd.c > @@ -409,6 +409,39 @@ void fwd_rule_add(struct fwd_ports *fwd, uint8_t flags, > } > } > > +/** > + * fwd_rule_match() - Does a prospective flow match a given forwarding rule Does [...]? > + * @rule: Forwarding rule > + * @ini: Initiating side flow information > + * > + * Returns: true if the rule applies to the flow, false otherwise > + */ > +static bool fwd_rule_match(const struct fwd_rule *rule, > + const struct flowside *ini) > +{ > + return inany_matches(&ini->oaddr, fwd_rule_addr(rule)) && > + ini->oport >= rule->first && ini->oport <= rule->last; The usual alignment is: return inany_matches(&ini->oaddr, fwd_rule_addr(rule)) && ini->oport >= rule->first && ini->oport <= rule->last; > +} > + > +/** > + * fwd_rule_search() - Find a rule which matches a prospective flow > + * @fwd: Forwarding table > + * @ini: Initiating side flow information > + * > + * Returns: first matching rule, or NULL if there is none I guess that this will eventually need to become a function matching the most specific rule first, tie breakers could be: 1. specific address given vs. wildcard 2. specific interface given vs. no interface 3. the day we support/need it: specific port/range vs. no port 4. smallest port range 5. the day we support/need something like this: longest prefix length and after this we should actually have an error on insertion (already guaranteed I think). > + */ > +const struct fwd_rule *fwd_rule_search(const struct fwd_ports *fwd, > + const struct flowside *ini) > +{ > + unsigned i; > + > + for (i = 0; i < fwd->count; i++) { > + if (fwd_rule_match(&fwd->rules[i], ini)) > + return &fwd->rules[i]; > + } Extra newline here to clearly separate the two outcomes. > + return NULL; > +} > + > /** > * fwd_rules_print() - Print forwarding rules for debugging > * @fwd: Table to print > diff --git a/fwd.h b/fwd.h > index f84e7c01..a10bdbb4 100644 > --- a/fwd.h > +++ b/fwd.h > @@ -103,6 +103,8 @@ struct fwd_ports { > void fwd_rule_add(struct fwd_ports *fwd, uint8_t flags, > const union inany_addr *addr, const char *ifname, > in_port_t first, in_port_t last, in_port_t to); > +const struct fwd_rule *fwd_rule_search(const struct fwd_ports *fwd, > + const struct flowside *ini); > void fwd_rules_print(const struct fwd_ports *fwd); > > void fwd_scan_ports_init(struct ctx *c); -- Stefano