Date:   Tue, 31 Jan 2023 18:13:50 +0200 (EET)
From:   Julian Anastasov <ja@ssi.bg>
To:     Zhang Changzhong <zhangchangzhong@huawei.com>
cc:     Network Development <netdev@vger.kernel.org>,
        open list <linux-kernel@vger.kernel.org>,
        "David S. Miller" <davem@davemloft.net>,
        Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>,
        David Ahern <dsahern@kernel.org>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        "Denis V. Lunev" <den@openvz.org>,
        Nikolay Aleksandrov <razor@blackwall.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        YueHaibing <yuehaibing@huawei.com>
Subject: Re: [Question] neighbor entry doesn't switch to the STALE state
 after the reachable timer expires
In-Reply-To: <1252089d-75db-a45c-d735-6883c772d95a@huawei.com>
Message-ID: <d95e4afc-3a95-a3a7-edd9-1bd13cdec90@ssi.bg>
References: <b1d8722e-5660-c38e-848f-3220d642889d@huawei.com> <99532c7f-161e-6d39-7680-ccc1f20349@ssi.bg> <9ebd0210-a4bb-afda-8a4d-5041b8395d78@huawei.com> <9ac5f4f6-36cc-cc6b-1220-f45db141656c@ssi.bg> <1252089d-75db-a45c-d735-6883c772d95a@huawei.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Precedence: bulk


	Hello,

On Tue, 31 Jan 2023, Zhang Changzhong wrote:

> Thanks for your fix!
> 
> I reproduced this problem by directly modifying the confirmed time of
> the neigh entry in STALE state. Your change can fix the problem in this
> scenario.

	I'm glad that you have a way to test it :) Thanks!

> Just curious, why did you choose 'jiffies - MAX_JIFFY_OFFSET + 86400 * HZ'
> as the value of 'mint'?

	It is too arbitrary :) Probably, just 'jiffies - MAX_JIFFY_OFFSET'
is enough or something depending on HZ/USER_HZ. I added 1 day for
timer to advance without leaving confirmed time behind the
jiffies - MAX_JIFFY_OFFSET zone but it is not needed.

	What limits play here:

- the HZ/USER_HZ difference: jiffies_to_clock_t reports the 3 times
to user space, so we want to display values as large as possible.
Any HZ > 100 for USER_HZ=100 works for the jiffies - MAX_JIFFY_OFFSET.
HZ=100 does not work.

- users can use large values for sysctl vars which can keep the timer
running for long time and reach some outdated confirmed time
before neigh_add_timer() is called to correct it

	If we choose mint = jiffies - MAX_JIFFY_OFFSET,
for 32-bit we will have:

Past                     Future
++++++++++++++++++++++++++++++++++++++++++++++++++++
|  49  days   |  49 days  |         99 days        |
++++++++++++++^+++++++++++^+++++++++++++++++++++++++
              ^           ^
DELAY+PROBE   |           |
            mint         now

- used/confirmed times should be up to 49 days behind jiffies but
we have 49 days to stay in timer without correcting them,
so they can go up to 99 days in the past before going in
the future and trigger the problem

- as we avoid the checks in neigh_timer_handler to save CPU cycles,
one needs crazy sysctl settings to keep the timer in DELAY+PROBE
states for 49 days. With default settings, it is no more than
half minute. In this case even
mint = jiffies - LONG_MAX + 86400 * HZ should work.

- REACHABLE state extends while confirmed time advances,
otherwise PROBE will need ARP reply to recheck the
times in neigh_add_timer while entering REACHABLE again

Regards

--
Julian Anastasov <ja@ssi.bg>