neighbour: Annotate access to neigh_parms fields.

NEIGH_VAR() is read locklessly in the fast path, and IPv6 ndisc uses
NEIGH_VAR_SET() locklessly.

The next patch will convert neightbl_dump_info() to RCU.

Let's annotate accesses to neigh_param with READ_ONCE() and WRITE_ONCE().

Note that ndisc_ifinfo_sysctl_change() uses &NEIGH_VAR() and we cannot
use '&' with READ_ONCE(), so NEIGH_VAR_PTR() is introduced.

Note also that NEIGH_VAR_INIT() does not need WRITE_ONCE() as it is before
parms is published.  Also, the only user hippi_neigh_setup_dev() is no
longer called since commit e3804cbebb67 ("net: remove COMPAT_NET_DEV_OPS"),
which looks wrong, but probably no one uses HIPPI and RoadRunner.

Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Link: https://patch.msgid.link/20251022054004.2514876-3-kuniyu@google.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

diff --git a/include/net/neighbour.h b/include/net/neighbour.h
index 4a30bd458c5a921f316668a5d7cc42cd20ea7e57..998ff9eccebb7c957b39e81ce16296d89526dbd2 100644 (file)
--- a/include/net/neighbour.h
+++ b/include/net/neighbour.h
@@ -92,15 +92,17 @@ struct neigh_parms {
 static inline void neigh_var_set(struct neigh_parms *p, int index, int val)
 {
        set_bit(index, p->data_state);
-       p->data[index] = val;
+       WRITE_ONCE(p->data[index], val);
 }
 
-#define NEIGH_VAR(p, attr) ((p)->data[NEIGH_VAR_ ## attr])
+#define __NEIGH_VAR(p, attr) ((p)->data[NEIGH_VAR_ ## attr])
+#define NEIGH_VAR(p, attr) READ_ONCE(__NEIGH_VAR(p, attr))
+#define NEIGH_VAR_PTR(p, attr) (&(__NEIGH_VAR(p, attr)))
 
 /* In ndo_neigh_setup, NEIGH_VAR_INIT should be used.
  * In other cases, NEIGH_VAR_SET should be used.
  */
-#define NEIGH_VAR_INIT(p, attr, val) (NEIGH_VAR(p, attr) = val)
+#define NEIGH_VAR_INIT(p, attr, val) (__NEIGH_VAR(p, attr) = val)
 #define NEIGH_VAR_SET(p, attr, val) neigh_var_set(p, NEIGH_VAR_ ## attr, val)
 
 static inline void neigh_parms_data_state_setall(struct neigh_parms *p)
@@ -378,6 +380,13 @@ struct net *neigh_parms_net(const struct neigh_parms *parms)
 
 unsigned long neigh_rand_reach_time(unsigned long base);
 
+static inline void neigh_set_reach_time(struct neigh_parms *p)
+{
+       unsigned long base = NEIGH_VAR(p, BASE_REACHABLE_TIME);
+
+       WRITE_ONCE(p->reachable_time, neigh_rand_reach_time(base));
+}
+
 void pneigh_enqueue(struct neigh_table *tbl, struct neigh_parms *p,
                    struct sk_buff *skb);
 struct pneigh_entry *pneigh_lookup(struct neigh_table *tbl, struct net *net,

diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 98428f60731bd008109c38378294fe605b435a39..5bbebbfcba43b12d6719fff97802350d51c1c2c2 100644 (file)
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -995,8 +995,7 @@ static void neigh_periodic_work(struct work_struct *work)
 
                WRITE_ONCE(tbl->last_rand, jiffies);
                list_for_each_entry(p, &tbl->parms_list, list)
-                       p->reachable_time =
-                               neigh_rand_reach_time(NEIGH_VAR(p, BASE_REACHABLE_TIME));
+                       neigh_set_reach_time(p);
        }
 
        if (atomic_read(&tbl->entries) < READ_ONCE(tbl->gc_thresh1))
@@ -1749,8 +1748,7 @@ struct neigh_parms *neigh_parms_alloc(struct net_device *dev,
        if (p) {
                p->tbl            = tbl;
                refcount_set(&p->refcnt, 1);
-               p->reachable_time =
-                               neigh_rand_reach_time(NEIGH_VAR(p, BASE_REACHABLE_TIME));
+               neigh_set_reach_time(p);
                p->qlen = 0;
                netdev_hold(dev, &p->dev_tracker, GFP_KERNEL);
                p->dev = dev;
@@ -1810,8 +1808,7 @@ void neigh_table_init(int index, struct neigh_table *tbl)
        list_add(&tbl->parms.list, &tbl->parms_list);
        write_pnet(&tbl->parms.net, &init_net);
        refcount_set(&tbl->parms.refcnt, 1);
-       tbl->parms.reachable_time =
-                         neigh_rand_reach_time(NEIGH_VAR(&tbl->parms, BASE_REACHABLE_TIME));
+       neigh_set_reach_time(&tbl->parms);
        tbl->parms.qlen = 0;
 
        tbl->stats = alloc_percpu(struct neigh_statistics);
@@ -2194,7 +2191,7 @@ static int neightbl_fill_parms(struct sk_buff *skb, struct neigh_parms *parms)
                        NEIGH_VAR(parms, MCAST_PROBES)) ||
            nla_put_u32(skb, NDTPA_MCAST_REPROBES,
                        NEIGH_VAR(parms, MCAST_REPROBES)) ||
-           nla_put_msecs(skb, NDTPA_REACHABLE_TIME, parms->reachable_time,
+           nla_put_msecs(skb, NDTPA_REACHABLE_TIME, READ_ONCE(parms->reachable_time),
                          NDTPA_PAD) ||
            nla_put_msecs(skb, NDTPA_BASE_REACHABLE_TIME,
                          NEIGH_VAR(parms, BASE_REACHABLE_TIME), NDTPA_PAD) ||
@@ -2475,8 +2472,7 @@ static int neightbl_set(struct sk_buff *skb, struct nlmsghdr *nlh,
                                 * only be effective after the next time neigh_periodic_work
                                 * decides to recompute it (can be multiple minutes)
                                 */
-                               p->reachable_time =
-                                       neigh_rand_reach_time(NEIGH_VAR(p, BASE_REACHABLE_TIME));
+                               neigh_set_reach_time(p);
                                break;
                        case NDTPA_GC_STALETIME:
                                NEIGH_VAR_SET(p, GC_STALETIME,
@@ -3721,8 +3717,7 @@ static int neigh_proc_base_reachable_time(const struct ctl_table *ctl, int write
                 * only be effective after the next time neigh_periodic_work
                 * decides to recompute it
                 */
-               p->reachable_time =
-                       neigh_rand_reach_time(NEIGH_VAR(p, BASE_REACHABLE_TIME));
+               neigh_set_reach_time(p);
        }
        return ret;
 }

diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index f427e41e9c49bf342869bea4444f308a5ac03a26..59d17b6f06bfd19f4a5e0457f4f20ce7185894c1 100644 (file)
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -1449,7 +1449,7 @@ skip_defrtr:
                                              BASE_REACHABLE_TIME, rtime);
                                NEIGH_VAR_SET(in6_dev->nd_parms,
                                              GC_STALETIME, 3 * rtime);
-                               in6_dev->nd_parms->reachable_time = neigh_rand_reach_time(rtime);
+                               neigh_set_reach_time(in6_dev->nd_parms);
                                in6_dev->tstamp = jiffies;
                                send_ifinfo_notify = true;
                        }
@@ -1948,9 +1948,9 @@ int ndisc_ifinfo_sysctl_change(const struct ctl_table *ctl, int write, void *buf
                ret = -1;
 
        if (write && ret == 0 && dev && (idev = in6_dev_get(dev)) != NULL) {
-               if (ctl->data == &NEIGH_VAR(idev->nd_parms, BASE_REACHABLE_TIME))
-                       idev->nd_parms->reachable_time =
-                                       neigh_rand_reach_time(NEIGH_VAR(idev->nd_parms, BASE_REACHABLE_TIME));
+               if (ctl->data == NEIGH_VAR_PTR(idev->nd_parms, BASE_REACHABLE_TIME))
+                       neigh_set_reach_time(idev->nd_parms);
+
                WRITE_ONCE(idev->tstamp, jiffies);
                inet6_ifinfo_notify(RTM_NEWLINK, idev);
                in6_dev_put(idev);