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=GEWb/p6E; 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 B29305A0265 for ; Thu, 16 Apr 2026 00:04:33 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1776290672; 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=9Ho7uWAYxDr9gOfMLhnA2QH2xYBxO3m7/n1o23FiuLM=; b=GEWb/p6EkvNE4lz/c51N0QnCenWOyd+g7YMIltukQwKBHpmbd5JadySVfyIUZ/IYfBMJRQ +WrMMOSTPyya6TCsScvwj/UrEIjtDkUWFXfAam98qGhADsFmjoJ0pU/hX805zB+rJHSJCe 4/8yUx7R0+APNQKV+J7ShhPiA33kEIc= 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-625-xCUgiTyMOC6-lz-XCePdiw-1; Wed, 15 Apr 2026 18:04:31 -0400 X-MC-Unique: xCUgiTyMOC6-lz-XCePdiw-1 X-Mimecast-MFC-AGG-ID: xCUgiTyMOC6-lz-XCePdiw_1776290670 Received: by mail-wr1-f70.google.com with SMTP id ffacd0b85a97d-43d1fec59c9so17917f8f.0 for ; Wed, 15 Apr 2026 15:04:31 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1776290670; x=1776895470; 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=9Ho7uWAYxDr9gOfMLhnA2QH2xYBxO3m7/n1o23FiuLM=; b=KLer/hoJGlXHmOgNnBsrwjY/HP7qgdddyk08t+A6ptxJVy1BYOqJFzVYgEXwb4exnU uCpPR15WoGDmmoFF3ejIhBOe5rlOLPeAwOZDU37+G3meWIuM96uZ/0WZpjuL+ZkwLECY qjbtPJg+0ibClFa04MDapFinEJgEbdW0R34Qwcd1rlboU3EPVzJKqKEaJZYMPmE50I0k +4D1qXkqdw8uH9EbX04ZVWE9mckaizNeL/e4DP0oJCQvJOwYWyPBCOUTryeNONP+bKq6 BOQBkVCT8WlpIHUnIJSVKwMRvG7jAAs6vDVJbzW0B08kPqW3gCBP7lI0ERioO9QdHixN 6qPg== X-Gm-Message-State: AOJu0YzovNjtBlsCz76j6hTt7t5Cf5gqIEXkJbj4lu/XylyvNe0eDjv6 Uq3A8utBZY7smBvzRh/Ta+4Pg3QGVEeTHUFfIYpHt39QR9z8zVqQkYqeEir9+rb6kDaqbVHXLXI 4ugpC4nVkRcJkgzN04HDGvR0dF/PJ7CDJVX0iQh3f6GbWkvj8/bIEsXTz2optIA== X-Gm-Gg: AeBDievzvDrWwbpNIoFQ9EB23NLgjEGcjX/qCZ20MtukkZKcQVRodccTfYmkQpXguTT nZ1SoMg+PDrkLmACAje9N/Tg1IHUZM1D6DJdIMFBWFY5hedLGTeSRaGCxSqQEwsn5Xj2imDcAkf tcNfT+Rsb1KfoKSJ5eeMncA836ySxs+Snv6ghIec0Nuqahn4ww1UnyqyWjIC+27ehHe2NmHzxWm vucNd63Nb2SkhbnvCsBcvy80Lvc0yWO7Zu7Y2gnK/k55ZQ4MmVU/qZoz0EguVKwq7hOorKBKrQs DZRMKPy2g/zY1LTkNsHkHB2ygWS5AVmUFvFF2L8UwgO0XEbGt8e+czRqfCL5kvgEJLXLn+7jZBN 9fYgwAq1tGjAp0vOF1DZEMSqnalr3w3bNAqKiXvelIbAvtoNx7w== X-Received: by 2002:adf:f7c6:0:b0:43d:160:c226 with SMTP id ffacd0b85a97d-43eb10ed603mr1037345f8f.24.1776290669709; Wed, 15 Apr 2026 15:04:29 -0700 (PDT) X-Received: by 2002:adf:f7c6:0:b0:43d:160:c226 with SMTP id ffacd0b85a97d-43eb10ed603mr1037331f8f.24.1776290669094; Wed, 15 Apr 2026 15:04:29 -0700 (PDT) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [176.103.220.4]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-43ead33d6d3sm8736457f8f.8.2026.04.15.15.04.28 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 15 Apr 2026 15:04:28 -0700 (PDT) From: Stefano Brivio To: David Gibson Subject: Re: [PATCH v2 10/23] fwd_rule: Move rule conflict checking from fwd_rule_add() to caller Message-ID: <20260416000427.64745c9c@elisabeth> In-Reply-To: <20260410010309.736855-11-david@gibson.dropbear.id.au> References: <20260410010309.736855-1-david@gibson.dropbear.id.au> <20260410010309.736855-11-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: Thu, 16 Apr 2026 00:04:28 +0200 (CEST) X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: FKsNvXOgLr_tFcsiEfF861Fjw_MCL22OtDidfQbtago_1776290670 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: VLK2CRUBIYGAITAUAW4OAMWV7W5OO5P2 X-Message-ID-Hash: VLK2CRUBIYGAITAUAW4OAMWV7W5OO5P2 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 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? -- Stefano