mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: David Laight <David.Laight@ACULAB.COM>
To: 'Dan Carpenter' <dan.carpenter@linaro.org>, Julian Anastasov <ja@ssi.bg>
Cc: Simon Horman <horms@verge.net.au>,
	Pablo Neira Ayuso <pablo@netfilter.org>,
	Jozsef Kadlecsik <kadlec@netfilter.org>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	"Jakub Kicinski" <kuba@kernel.org>,
	Paolo Abeni <pabeni@redhat.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"lvs-devel@vger.kernel.org" <lvs-devel@vger.kernel.org>,
	"netfilter-devel@vger.kernel.org"
	<netfilter-devel@vger.kernel.org>,
	"coreteam@netfilter.org" <coreteam@netfilter.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"Bartosz Golaszewski" <brgl@bgdev.pl>
Subject: RE: [PATCH net] ipvs: Fix clamp() order in ip_vs_conn_init()
Date: Wed, 11 Dec 2024 14:27:06 +0000	[thread overview]
Message-ID: <7e01a62a5cb4435198f13be27c19de26@AcuMS.aculab.com> (raw)
In-Reply-To: <1e0cf09d-406f-4b66-8ff5-25ddc2345e54@stanley.mountain>

From: Dan Carpenter
> Sent: 11 December 2024 13:17
> 
> We recently added some build time asserts to detect incorrect calls to
> clamp and it detected this bug which breaks the build.  The variable
> in this clamp is "max_avail" and it should be the first argument.  The
> code currently is the equivalent to max = max(max_avail, max).

The fix is correct but the description above is wrong.
When run max_avail is always larger than min so the result is correct.
But the compiler does some constant propagation (for something that
can't happen) and wants to calculate the constant 'clamp(max, min, 0)'
Both max and min are known values so the build assert trips.

I posted the same patch (with a different message) last week.

	David

> 
> Reported-by: Linux Kernel Functional Testing <lkft@linaro.org>
> Closes:
> https://lore.kernel.org/all/CA+G9fYsT34UkGFKxus63H6UVpYi5GRZkezT9MRLfAbM3f6ke0g@mail.gmail.com/
> Fixes: 4f325e26277b ("ipvs: dynamically limit the connection hash table")
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> ---
> I've been trying to add stable CC's to my commits but I'm not sure the
> netdev policy on this.  Do you prefer to add them yourself?
> 
>  net/netfilter/ipvs/ip_vs_conn.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/netfilter/ipvs/ip_vs_conn.c b/net/netfilter/ipvs/ip_vs_conn.c
> index 98d7dbe3d787..9f75ac801301 100644
> --- a/net/netfilter/ipvs/ip_vs_conn.c
> +++ b/net/netfilter/ipvs/ip_vs_conn.c
> @@ -1495,7 +1495,7 @@ int __init ip_vs_conn_init(void)
>  	max_avail -= 2;		/* ~4 in hash row */
>  	max_avail -= 1;		/* IPVS up to 1/2 of mem */
>  	max_avail -= order_base_2(sizeof(struct ip_vs_conn));
> -	max = clamp(max, min, max_avail);
> +	max = clamp(max_avail, min, max);
>  	ip_vs_conn_tab_bits = clamp_val(ip_vs_conn_tab_bits, min, max);
>  	ip_vs_conn_tab_size = 1 << ip_vs_conn_tab_bits;
>  	ip_vs_conn_tab_mask = ip_vs_conn_tab_size - 1;
> --
> 2.45.2

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)


  parent reply	other threads:[~2024-12-11 14:28 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-11 13:16 Dan Carpenter
2024-12-11 13:53 ` Bartosz Golaszewski
2024-12-11 14:27 ` David Laight [this message]
2024-12-11 15:49   ` Dan Carpenter
2024-12-11 16:46   ` Julian Anastasov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7e01a62a5cb4435198f13be27c19de26@AcuMS.aculab.com \
    --to=david.laight@aculab.com \
    --cc=brgl@bgdev.pl \
    --cc=coreteam@netfilter.org \
    --cc=dan.carpenter@linaro.org \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=horms@verge.net.au \
    --cc=ja@ssi.bg \
    --cc=kadlec@netfilter.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lvs-devel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=pablo@netfilter.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®