Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH] ipv4: only allow increasing fib_info_hash_size
To:     =?UTF-8?B?5byg5Yev?= <zhangkaiheb@126.com>
Cc:     davem@davemloft.net, yoshfuji@linux-ipv6.org, dsahern@kernel.org,
        kuba@kernel.org, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <20211012110658.10166-1-zhangkaiheb@126.com>
 <23911752-3971-0230-cfd2-f8e30e8805db@gmail.com>
 <3bd88b51.6c7.17c77339c10.Coremail.zhangkaiheb@126.com>
From:   David Ahern <dsahern@gmail.com>
Message-ID: <9404e2d8-0976-1726-5f08-c277cdc14945@gmail.com>
Date:   Tue, 12 Oct 2021 20:33:19 -0600
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0)
 Gecko/20100101 Thunderbird/78.14.0
MIME-Version: 1.0
In-Reply-To: <3bd88b51.6c7.17c77339c10.Coremail.zhangkaiheb@126.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 10/12/21 7:10 PM, 张凯 wrote:
> 
> 1) Below shift operation will set fib_info_hash_size to zero if there
> are so many routes: 
>   unsigned int new_size = fib_info_hash_size << 1;
> 
>     This will wrap value of fib_info_hash_size, and a lot of fib_info
> will insert to a small hash bucket.
> so this patch disables above wrap.
> 2) Check whether fib_info_hash_size is zero only needed after allocation
> failed:
> if (!new_info_hash || !new_laddrhash) {
> 
> 

why not something simpler like this:

diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index 3364cb9c67e0..5c4bd1cebe0a 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -1401,6 +1401,10 @@ struct fib_info *fib_create_info(struct
fib_config *cfg,

                if (!new_size)
                        new_size = 16;
+
+               if (new_size < fib_info_hash_size)
+                       goto failure;
+
                bytes = new_size * sizeof(struct hlist_head *);
                new_info_hash = fib_info_hash_alloc(bytes);
                new_laddrhash = fib_info_hash_alloc(bytes);