netdev - Re: [PATCH] net, neigh: introduce interval_probe

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CANmJ_FOkC=YZhex+fGyVsP_1bUBU3MsHMLbPzdixgU5PUGpONQ@mail.gmail.com>
Date:   Sun, 22 May 2022 02:12:35 +0800
From:   王禹惟 <wangyuweihx@...il.com>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     davem@...emloft.net, Eric Dumazet <edumazet@...gle.com>,
        daniel@...earbox.net, roopa@...dia.com, dsahern@...nel.org,
        秦迪 <qindi@...ff.weibo.com>,
        netdev@...r.kernel.org, wangyuweihx <wangyuweihx@...il.com>
Subject: Re: [PATCH] net, neigh: introduce interval_probe_time for periodic probe

On Sat, May 21, 2022 at 8:56 AM Jakub Kicinski <kuba@...nel.org> wrote:
>
> On Fri, 20 May 2022 05:51:04 +0000 Yuwei Wang wrote:
> > commit 7482e3841d52 ("net, neigh: Add NTF_MANAGED flag for managed neighbor entries")
> > neighbor entries which with NTF_EXT_MANAGED flags will periodically call neigh_event_send()
> > for performing the resolution. and the interval was set to DELAY_PROBE_TIME
> >
> > DELAY_PROBE_TIME was configured as the first probe time delay, and it makes sense to set it to `0`.
> >
> > when DELAY_PROBE_TIME is `0`, the resolution of neighbor entries with NTF_EXT_MANAGED will
> > trap in an infinity recursion.
>
> Recursion or will constantly get re-resolved?

Recursive call `neigh_event_send_probe` with no interval,
which means threads of `system_power_efficient_wq` will consume 100% cpu.
Re-resolved or not depend on `neigh->nud_state`, if `neigh->nud_state`
is NUD_REACHABLE,
`neigh_event_send_probe` will return immediately with no action
>
> > as commit messages mentioned in the above commit, we should introduce a new option which means resolution interval.
> >
> > Signed-off-by: Yuwei Wang <wangyuweihx@...il.com>
>
> > diff --git a/include/uapi/linux/neighbour.h b/include/uapi/linux/neighbour.h
> > index 39c565e460c7..5ae538be64b9 100644
> > --- a/include/uapi/linux/neighbour.h
> > +++ b/include/uapi/linux/neighbour.h
> > @@ -143,6 +143,7 @@ enum {
> >       NDTPA_RETRANS_TIME,             /* u64, msecs */
> >       NDTPA_GC_STALETIME,             /* u64, msecs */
> >       NDTPA_DELAY_PROBE_TIME,         /* u64, msecs */
> > +     NDTPA_INTERVAL_PROBE_TIME,      /* u64, msecs */
> >       NDTPA_QUEUE_LEN,                /* u32 */
> >       NDTPA_APP_PROBES,               /* u32 */
> >       NDTPA_UCAST_PROBES,             /* u32 */
>
> You can't insert values in the middle of a uAPI enum,
> you'll break binary compatibility with older kernels.

Thanks, I will move it to the tail in patch v2