* [PATCH net-next v3 0/2] net, neigh: introduce interval_probe_time for periodic probe @ 2022-06-09 10:57 Yuwei Wang 2022-06-09 10:57 ` [PATCH net-next v3 1/2] sysctl: add proc_dointvec_jiffies_minmax Yuwei Wang 2022-06-09 10:57 ` [PATCH net-next v3 2/2] net, neigh: introduce interval_probe_time for periodic probe Yuwei Wang 0 siblings, 2 replies; 12+ messages in thread From: Yuwei Wang @ 2022-06-09 10:57 UTC (permalink / raw) To: davem, kuba, edumazet, pabeni Cc: daniel, roopa, dsahern, qindi, netdev, Yuwei Wang This series adds a new option `interval_probe_time` in net, neigh for periodic probe, and add a limitation to prevent it set to 0 Yuwei Wang (2): sysctl: add proc_dointvec_jiffies_minmax net, neigh: introduce interval_probe_time for periodic probe include/linux/sysctl.h | 2 ++ include/net/neighbour.h | 1 + include/uapi/linux/neighbour.h | 1 + include/uapi/linux/sysctl.h | 37 +++++++++++++++++----------------- kernel/sysctl.c | 36 +++++++++++++++++++++++++++++++++ net/core/neighbour.c | 33 ++++++++++++++++++++++++++++-- net/decnet/dn_neigh.c | 1 + net/ipv4/arp.c | 1 + net/ipv6/ndisc.c | 1 + 9 files changed, 93 insertions(+), 20 deletions(-) base-commit: da6e113ff010815fdd21ee1e9af2e8d179a2680f -- 2.34.1 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH net-next v3 1/2] sysctl: add proc_dointvec_jiffies_minmax 2022-06-09 10:57 [PATCH net-next v3 0/2] net, neigh: introduce interval_probe_time for periodic probe Yuwei Wang @ 2022-06-09 10:57 ` Yuwei Wang 2022-06-14 5:58 ` Jakub Kicinski 2022-06-09 10:57 ` [PATCH net-next v3 2/2] net, neigh: introduce interval_probe_time for periodic probe Yuwei Wang 1 sibling, 1 reply; 12+ messages in thread From: Yuwei Wang @ 2022-06-09 10:57 UTC (permalink / raw) To: davem, kuba, edumazet, pabeni Cc: daniel, roopa, dsahern, qindi, netdev, Yuwei Wang add proc_dointvec_jiffies_minmax to fit jiffies param with a limited range of values Signed-off-by: Yuwei Wang <wangyuweihx@gmail.com> --- include/linux/sysctl.h | 2 ++ kernel/sysctl.c | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h index 80263f7cdb77..0e1c05244350 100644 --- a/include/linux/sysctl.h +++ b/include/linux/sysctl.h @@ -75,6 +75,8 @@ int proc_douintvec_minmax(struct ctl_table *table, int write, void *buffer, int proc_dou8vec_minmax(struct ctl_table *table, int write, void *buffer, size_t *lenp, loff_t *ppos); int proc_dointvec_jiffies(struct ctl_table *, int, void *, size_t *, loff_t *); +int proc_dointvec_jiffies_minmax(struct ctl_table *table, int write, + void *buffer, size_t *lenp, loff_t *ppos); int proc_dointvec_userhz_jiffies(struct ctl_table *, int, void *, size_t *, loff_t *); int proc_dointvec_ms_jiffies(struct ctl_table *, int, void *, size_t *, diff --git a/kernel/sysctl.c b/kernel/sysctl.c index e52b6e372c60..4187c389a1eb 100644 --- a/kernel/sysctl.c +++ b/kernel/sysctl.c @@ -1189,6 +1189,31 @@ static int do_proc_dointvec_jiffies_conv(bool *negp, unsigned long *lvalp, return 0; } +static int do_proc_dointvec_jiffies_minmax_conv(bool *negp, unsigned long *lvalp, + int *valp, int write, void *data) +{ + int tmp, ret; + struct do_proc_dointvec_minmax_conv_param *param = data; + /* + * If writing, first do so via a temporary local int so we can + * bounds-check it before touching *valp. + */ + int *ip = write ? &tmp : valp; + + ret = do_proc_dointvec_jiffies_conv(negp, lvalp, ip, write, data); + if (ret) + return ret; + + if (write) { + if ((param->min && *param->min > tmp) || + (param->max && *param->max < tmp)) + return -EINVAL; + *valp = tmp; +} + +return 0; +} + static int do_proc_dointvec_userhz_jiffies_conv(bool *negp, unsigned long *lvalp, int *valp, int write, void *data) @@ -1259,6 +1284,17 @@ int proc_dointvec_jiffies(struct ctl_table *table, int write, do_proc_dointvec_jiffies_conv,NULL); } +int proc_dointvec_jiffies_minmax(struct ctl_table *table, int write, + void *buffer, size_t *lenp, loff_t *ppos) +{ + struct do_proc_dointvec_minmax_conv_param param = { + .min = (int *) table->extra1, + .max = (int *) table->extra2, + }; + return do_proc_dointvec(table, write, buffer, lenp, ppos, + do_proc_dointvec_jiffies_minmax_conv, ¶m); +} + /** * proc_dointvec_userhz_jiffies - read a vector of integers as 1/USER_HZ seconds * @table: the sysctl table -- 2.34.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH net-next v3 1/2] sysctl: add proc_dointvec_jiffies_minmax 2022-06-09 10:57 ` [PATCH net-next v3 1/2] sysctl: add proc_dointvec_jiffies_minmax Yuwei Wang @ 2022-06-14 5:58 ` Jakub Kicinski 0 siblings, 0 replies; 12+ messages in thread From: Jakub Kicinski @ 2022-06-14 5:58 UTC (permalink / raw) To: Yuwei Wang; +Cc: davem, edumazet, pabeni, daniel, roopa, dsahern, qindi, netdev On Thu, 9 Jun 2022 10:57:24 +0000 Yuwei Wang wrote: > + *valp = tmp; > +} > + > +return 0; > +} > + Looks whitespace damaged ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH net-next v3 2/2] net, neigh: introduce interval_probe_time for periodic probe 2022-06-09 10:57 [PATCH net-next v3 0/2] net, neigh: introduce interval_probe_time for periodic probe Yuwei Wang 2022-06-09 10:57 ` [PATCH net-next v3 1/2] sysctl: add proc_dointvec_jiffies_minmax Yuwei Wang @ 2022-06-09 10:57 ` Yuwei Wang 2022-06-14 6:02 ` Jakub Kicinski 2022-06-14 9:10 ` Nikolay Aleksandrov 1 sibling, 2 replies; 12+ messages in thread From: Yuwei Wang @ 2022-06-09 10:57 UTC (permalink / raw) To: davem, kuba, edumazet, pabeni Cc: daniel, roopa, dsahern, qindi, netdev, Yuwei Wang commit ed6cd6a17896 ("net, neigh: Set lower cap for neigh_managed_work rearming") fixed a case when DELAY_PROBE_TIME is configured to 0, the processing of the system work queue hog CPU to 100%, and further more we should introduce a new option used by periodic probe Signed-off-by: Yuwei Wang <wangyuweihx@gmail.com> --- v3: - add limitation to prevent `INTERVAL_PROBE_TIME` to 0 - remove `NETEVENT_INTERVAL_PROBE_TIME_UPDATE` - add .min to NDTPA_INTERVAL_PROBE_TIME include/net/neighbour.h | 1 + include/uapi/linux/neighbour.h | 1 + include/uapi/linux/sysctl.h | 37 +++++++++++++++++----------------- net/core/neighbour.c | 33 ++++++++++++++++++++++++++++-- net/decnet/dn_neigh.c | 1 + net/ipv4/arp.c | 1 + net/ipv6/ndisc.c | 1 + 7 files changed, 55 insertions(+), 20 deletions(-) diff --git a/include/net/neighbour.h b/include/net/neighbour.h index 87419f7f5421..c6a6e652d442 100644 --- a/include/net/neighbour.h +++ b/include/net/neighbour.h @@ -48,6 +48,7 @@ enum { NEIGH_VAR_RETRANS_TIME, NEIGH_VAR_BASE_REACHABLE_TIME, NEIGH_VAR_DELAY_PROBE_TIME, + NEIGH_VAR_INTERVAL_PROBE_TIME, NEIGH_VAR_GC_STALETIME, NEIGH_VAR_QUEUE_LEN_BYTES, NEIGH_VAR_PROXY_QLEN, diff --git a/include/uapi/linux/neighbour.h b/include/uapi/linux/neighbour.h index 39c565e460c7..8713c3ea81b2 100644 --- a/include/uapi/linux/neighbour.h +++ b/include/uapi/linux/neighbour.h @@ -154,6 +154,7 @@ enum { NDTPA_QUEUE_LENBYTES, /* u32 */ NDTPA_MCAST_REPROBES, /* u32 */ NDTPA_PAD, + NDTPA_INTERVAL_PROBE_TIME, /* u64, msecs */ __NDTPA_MAX }; #define NDTPA_MAX (__NDTPA_MAX - 1) diff --git a/include/uapi/linux/sysctl.h b/include/uapi/linux/sysctl.h index 6a3b194c50fe..53f06bfd2a37 100644 --- a/include/uapi/linux/sysctl.h +++ b/include/uapi/linux/sysctl.h @@ -584,24 +584,25 @@ enum { /* /proc/sys/net/<protocol>/neigh/<dev> */ enum { - NET_NEIGH_MCAST_SOLICIT=1, - NET_NEIGH_UCAST_SOLICIT=2, - NET_NEIGH_APP_SOLICIT=3, - NET_NEIGH_RETRANS_TIME=4, - NET_NEIGH_REACHABLE_TIME=5, - NET_NEIGH_DELAY_PROBE_TIME=6, - NET_NEIGH_GC_STALE_TIME=7, - NET_NEIGH_UNRES_QLEN=8, - NET_NEIGH_PROXY_QLEN=9, - NET_NEIGH_ANYCAST_DELAY=10, - NET_NEIGH_PROXY_DELAY=11, - NET_NEIGH_LOCKTIME=12, - NET_NEIGH_GC_INTERVAL=13, - NET_NEIGH_GC_THRESH1=14, - NET_NEIGH_GC_THRESH2=15, - NET_NEIGH_GC_THRESH3=16, - NET_NEIGH_RETRANS_TIME_MS=17, - NET_NEIGH_REACHABLE_TIME_MS=18, + NET_NEIGH_MCAST_SOLICIT = 1, + NET_NEIGH_UCAST_SOLICIT = 2, + NET_NEIGH_APP_SOLICIT = 3, + NET_NEIGH_RETRANS_TIME = 4, + NET_NEIGH_REACHABLE_TIME = 5, + NET_NEIGH_DELAY_PROBE_TIME = 6, + NET_NEIGH_GC_STALE_TIME = 7, + NET_NEIGH_UNRES_QLEN = 8, + NET_NEIGH_PROXY_QLEN = 9, + NET_NEIGH_ANYCAST_DELAY = 10, + NET_NEIGH_PROXY_DELAY = 11, + NET_NEIGH_LOCKTIME = 12, + NET_NEIGH_GC_INTERVAL = 13, + NET_NEIGH_GC_THRESH1 = 14, + NET_NEIGH_GC_THRESH2 = 15, + NET_NEIGH_GC_THRESH3 = 16, + NET_NEIGH_RETRANS_TIME_MS = 17, + NET_NEIGH_REACHABLE_TIME_MS = 18, + NET_NEIGH_INTERVAL_PROBE_TIME = 19, }; /* /proc/sys/net/dccp */ diff --git a/net/core/neighbour.c b/net/core/neighbour.c index 54625287ee5b..845fad952ce2 100644 --- a/net/core/neighbour.c +++ b/net/core/neighbour.c @@ -1579,7 +1579,7 @@ static void neigh_managed_work(struct work_struct *work) list_for_each_entry(neigh, &tbl->managed_list, managed_list) neigh_event_send_probe(neigh, NULL, false); queue_delayed_work(system_power_efficient_wq, &tbl->managed_work, - max(NEIGH_VAR(&tbl->parms, DELAY_PROBE_TIME), HZ)); + NEIGH_VAR(&tbl->parms, INTERVAL_PROBE_TIME)); write_unlock_bh(&tbl->lock); } @@ -2100,7 +2100,9 @@ static int neightbl_fill_parms(struct sk_buff *skb, struct neigh_parms *parms) nla_put_msecs(skb, NDTPA_PROXY_DELAY, NEIGH_VAR(parms, PROXY_DELAY), NDTPA_PAD) || nla_put_msecs(skb, NDTPA_LOCKTIME, - NEIGH_VAR(parms, LOCKTIME), NDTPA_PAD)) + NEIGH_VAR(parms, LOCKTIME), NDTPA_PAD) || + nla_put_msecs(skb, NDTPA_INTERVAL_PROBE_TIME, + NEIGH_VAR(parms, INTERVAL_PROBE_TIME), NDTPA_PAD)) goto nla_put_failure; return nla_nest_end(skb, nest); @@ -2255,6 +2257,7 @@ static const struct nla_policy nl_ntbl_parm_policy[NDTPA_MAX+1] = { [NDTPA_ANYCAST_DELAY] = { .type = NLA_U64 }, [NDTPA_PROXY_DELAY] = { .type = NLA_U64 }, [NDTPA_LOCKTIME] = { .type = NLA_U64 }, + [NDTPA_INTERVAL_PROBE_TIME] = { .type = NLA_U64, .min = 1 }, }; static int neightbl_set(struct sk_buff *skb, struct nlmsghdr *nlh, @@ -2373,6 +2376,10 @@ static int neightbl_set(struct sk_buff *skb, struct nlmsghdr *nlh, nla_get_msecs(tbp[i])); call_netevent_notifiers(NETEVENT_DELAY_PROBE_TIME_UPDATE, p); break; + case NDTPA_INTERVAL_PROBE_TIME: + NEIGH_VAR_SET(p, INTERVAL_PROBE_TIME, + nla_get_msecs(tbp[i])); + break; case NDTPA_RETRANS_TIME: NEIGH_VAR_SET(p, RETRANS_TIME, nla_get_msecs(tbp[i])); @@ -3562,6 +3569,24 @@ static int neigh_proc_dointvec_zero_intmax(struct ctl_table *ctl, int write, return ret; } +static int neigh_proc_dointvec_jiffies_positive(struct ctl_table *ctl, int write, + void *buffer, size_t *lenp, + loff_t *ppos) +{ + struct ctl_table tmp = *ctl; + int ret; + + int min = HZ; + int max = INT_MAX; + + tmp.extra1 = &min; + tmp.extra2 = &max; + + ret = proc_dointvec_jiffies_minmax(&tmp, write, buffer, lenp, ppos); + neigh_proc_update(ctl, write); + return ret; +} + int neigh_proc_dointvec(struct ctl_table *ctl, int write, void *buffer, size_t *lenp, loff_t *ppos) { @@ -3658,6 +3683,9 @@ static int neigh_proc_base_reachable_time(struct ctl_table *ctl, int write, #define NEIGH_SYSCTL_USERHZ_JIFFIES_ENTRY(attr, name) \ NEIGH_SYSCTL_ENTRY(attr, attr, name, 0644, neigh_proc_dointvec_userhz_jiffies) +#define NEIGH_SYSCTL_JIFFIES_POSITIVE_ENTRY(attr, name) \ + NEIGH_SYSCTL_ENTRY(attr, attr, name, 0644, neigh_proc_dointvec_jiffies_positive) + #define NEIGH_SYSCTL_MS_JIFFIES_REUSED_ENTRY(attr, data_attr, name) \ NEIGH_SYSCTL_ENTRY(attr, data_attr, name, 0644, neigh_proc_dointvec_ms_jiffies) @@ -3676,6 +3704,7 @@ static struct neigh_sysctl_table { NEIGH_SYSCTL_USERHZ_JIFFIES_ENTRY(RETRANS_TIME, "retrans_time"), NEIGH_SYSCTL_JIFFIES_ENTRY(BASE_REACHABLE_TIME, "base_reachable_time"), NEIGH_SYSCTL_JIFFIES_ENTRY(DELAY_PROBE_TIME, "delay_first_probe_time"), + NEIGH_SYSCTL_JIFFIES_POSITIVE_ENTRY(INTERVAL_PROBE_TIME, "interval_probe_time"), NEIGH_SYSCTL_JIFFIES_ENTRY(GC_STALETIME, "gc_stale_time"), NEIGH_SYSCTL_ZERO_INTMAX_ENTRY(QUEUE_LEN_BYTES, "unres_qlen_bytes"), NEIGH_SYSCTL_ZERO_INTMAX_ENTRY(PROXY_QLEN, "proxy_qlen"), diff --git a/net/decnet/dn_neigh.c b/net/decnet/dn_neigh.c index fbd98ac853ea..995b22841ebf 100644 --- a/net/decnet/dn_neigh.c +++ b/net/decnet/dn_neigh.c @@ -94,6 +94,7 @@ struct neigh_table dn_neigh_table = { [NEIGH_VAR_RETRANS_TIME] = 1 * HZ, [NEIGH_VAR_BASE_REACHABLE_TIME] = 30 * HZ, [NEIGH_VAR_DELAY_PROBE_TIME] = 5 * HZ, + [NEIGH_VAR_INTERVAL_PROBE_TIME] = 5 * HZ, [NEIGH_VAR_GC_STALETIME] = 60 * HZ, [NEIGH_VAR_QUEUE_LEN_BYTES] = SK_WMEM_MAX, [NEIGH_VAR_PROXY_QLEN] = 0, diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c index ab4a5601c82a..dbea1f7a7e2b 100644 --- a/net/ipv4/arp.c +++ b/net/ipv4/arp.c @@ -168,6 +168,7 @@ struct neigh_table arp_tbl = { [NEIGH_VAR_RETRANS_TIME] = 1 * HZ, [NEIGH_VAR_BASE_REACHABLE_TIME] = 30 * HZ, [NEIGH_VAR_DELAY_PROBE_TIME] = 5 * HZ, + [NEIGH_VAR_INTERVAL_PROBE_TIME] = 5 * HZ, [NEIGH_VAR_GC_STALETIME] = 60 * HZ, [NEIGH_VAR_QUEUE_LEN_BYTES] = SK_WMEM_MAX, [NEIGH_VAR_PROXY_QLEN] = 64, diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c index b0dfe97ea4ee..aa9dc032bac5 100644 --- a/net/ipv6/ndisc.c +++ b/net/ipv6/ndisc.c @@ -128,6 +128,7 @@ struct neigh_table nd_tbl = { [NEIGH_VAR_RETRANS_TIME] = ND_RETRANS_TIMER, [NEIGH_VAR_BASE_REACHABLE_TIME] = ND_REACHABLE_TIME, [NEIGH_VAR_DELAY_PROBE_TIME] = 5 * HZ, + [NEIGH_VAR_INTERVAL_PROBE_TIME] = 5 * HZ, [NEIGH_VAR_GC_STALETIME] = 60 * HZ, [NEIGH_VAR_QUEUE_LEN_BYTES] = SK_WMEM_MAX, [NEIGH_VAR_PROXY_QLEN] = 64, -- 2.34.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH net-next v3 2/2] net, neigh: introduce interval_probe_time for periodic probe 2022-06-09 10:57 ` [PATCH net-next v3 2/2] net, neigh: introduce interval_probe_time for periodic probe Yuwei Wang @ 2022-06-14 6:02 ` Jakub Kicinski 2022-06-15 15:33 ` Yuwei Wang 2022-06-14 9:10 ` Nikolay Aleksandrov 1 sibling, 1 reply; 12+ messages in thread From: Jakub Kicinski @ 2022-06-14 6:02 UTC (permalink / raw) To: Yuwei Wang; +Cc: davem, edumazet, pabeni, daniel, roopa, dsahern, qindi, netdev On Thu, 9 Jun 2022 10:57:25 +0000 Yuwei Wang wrote: > commit ed6cd6a17896 ("net, neigh: Set lower cap for neigh_managed_work rearming") > fixed a case when DELAY_PROBE_TIME is configured to 0, the processing of the > system work queue hog CPU to 100%, and further more we should introduce > a new option used by periodic probe > > Signed-off-by: Yuwei Wang <wangyuweihx@gmail.com> The new sysctl needs to be documented in Documentation/networking/ip-sysctl ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next v3 2/2] net, neigh: introduce interval_probe_time for periodic probe 2022-06-14 6:02 ` Jakub Kicinski @ 2022-06-15 15:33 ` Yuwei Wang 0 siblings, 0 replies; 12+ messages in thread From: Yuwei Wang @ 2022-06-15 15:33 UTC (permalink / raw) To: Jakub Kicinski Cc: davem, Eric Dumazet, Paolo Abeni, Daniel Borkmann, roopa, dsahern, 秦迪, netdev On Tue, 14 Jun 2022 at 14:02, Jakub Kicinski <kuba@kernel.org> wrote: > > On Thu, 9 Jun 2022 10:57:25 +0000 Yuwei Wang wrote: > > commit ed6cd6a17896 ("net, neigh: Set lower cap for neigh_managed_work rearming") > > fixed a case when DELAY_PROBE_TIME is configured to 0, the processing of the > > system work queue hog CPU to 100%, and further more we should introduce > > a new option used by periodic probe > > > > Signed-off-by: Yuwei Wang <wangyuweihx@gmail.com> > > The new sysctl needs to be documented in > Documentation/networking/ip-sysctl Oops……I will fix the whitespace and add the doc in the next version ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next v3 2/2] net, neigh: introduce interval_probe_time for periodic probe 2022-06-09 10:57 ` [PATCH net-next v3 2/2] net, neigh: introduce interval_probe_time for periodic probe Yuwei Wang 2022-06-14 6:02 ` Jakub Kicinski @ 2022-06-14 9:10 ` Nikolay Aleksandrov 2022-06-15 15:28 ` Yuwei Wang 1 sibling, 1 reply; 12+ messages in thread From: Nikolay Aleksandrov @ 2022-06-14 9:10 UTC (permalink / raw) To: Yuwei Wang, davem, kuba, edumazet, pabeni Cc: daniel, roopa, dsahern, qindi, netdev On 09/06/2022 13:57, Yuwei Wang wrote: > commit ed6cd6a17896 ("net, neigh: Set lower cap for neigh_managed_work rearming") > fixed a case when DELAY_PROBE_TIME is configured to 0, the processing of the > system work queue hog CPU to 100%, and further more we should introduce > a new option used by periodic probe > > Signed-off-by: Yuwei Wang <wangyuweihx@gmail.com> > --- > v3: > - add limitation to prevent `INTERVAL_PROBE_TIME` to 0 > - remove `NETEVENT_INTERVAL_PROBE_TIME_UPDATE` > - add .min to NDTPA_INTERVAL_PROBE_TIME > > include/net/neighbour.h | 1 + > include/uapi/linux/neighbour.h | 1 + > include/uapi/linux/sysctl.h | 37 +++++++++++++++++----------------- > net/core/neighbour.c | 33 ++++++++++++++++++++++++++++-- > net/decnet/dn_neigh.c | 1 + > net/ipv4/arp.c | 1 + > net/ipv6/ndisc.c | 1 + > 7 files changed, 55 insertions(+), 20 deletions(-) > [snip] > diff --git a/include/uapi/linux/neighbour.h b/include/uapi/linux/neighbour.h > index 39c565e460c7..8713c3ea81b2 100644 > --- a/include/uapi/linux/neighbour.h > +++ b/include/uapi/linux/neighbour.h > @@ -154,6 +154,7 @@ enum { > NDTPA_QUEUE_LENBYTES, /* u32 */ > NDTPA_MCAST_REPROBES, /* u32 */ > NDTPA_PAD, > + NDTPA_INTERVAL_PROBE_TIME, /* u64, msecs */ > __NDTPA_MAX > }; > #define NDTPA_MAX (__NDTPA_MAX - 1) [snip] > /* /proc/sys/net/dccp */ > diff --git a/net/core/neighbour.c b/net/core/neighbour.c > index 54625287ee5b..845fad952ce2 100644 > --- a/net/core/neighbour.c > +++ b/net/core/neighbour.c > @@ -1579,7 +1579,7 @@ static void neigh_managed_work(struct work_struct *work) > list_for_each_entry(neigh, &tbl->managed_list, managed_list) > neigh_event_send_probe(neigh, NULL, false); > queue_delayed_work(system_power_efficient_wq, &tbl->managed_work, > - max(NEIGH_VAR(&tbl->parms, DELAY_PROBE_TIME), HZ)); > + NEIGH_VAR(&tbl->parms, INTERVAL_PROBE_TIME)); > write_unlock_bh(&tbl->lock); > } > > @@ -2100,7 +2100,9 @@ static int neightbl_fill_parms(struct sk_buff *skb, struct neigh_parms *parms) > nla_put_msecs(skb, NDTPA_PROXY_DELAY, > NEIGH_VAR(parms, PROXY_DELAY), NDTPA_PAD) || > nla_put_msecs(skb, NDTPA_LOCKTIME, > - NEIGH_VAR(parms, LOCKTIME), NDTPA_PAD)) > + NEIGH_VAR(parms, LOCKTIME), NDTPA_PAD) || > + nla_put_msecs(skb, NDTPA_INTERVAL_PROBE_TIME, > + NEIGH_VAR(parms, INTERVAL_PROBE_TIME), NDTPA_PAD)) > goto nla_put_failure; > return nla_nest_end(skb, nest); > > @@ -2255,6 +2257,7 @@ static const struct nla_policy nl_ntbl_parm_policy[NDTPA_MAX+1] = { > [NDTPA_ANYCAST_DELAY] = { .type = NLA_U64 }, > [NDTPA_PROXY_DELAY] = { .type = NLA_U64 }, > [NDTPA_LOCKTIME] = { .type = NLA_U64 }, > + [NDTPA_INTERVAL_PROBE_TIME] = { .type = NLA_U64, .min = 1 }, shouldn't the min be MSEC_PER_SEC (1 sec minimum) ? > }; > > static int neightbl_set(struct sk_buff *skb, struct nlmsghdr *nlh, > @@ -2373,6 +2376,10 @@ static int neightbl_set(struct sk_buff *skb, struct nlmsghdr *nlh, > nla_get_msecs(tbp[i])); > call_netevent_notifiers(NETEVENT_DELAY_PROBE_TIME_UPDATE, p); > break; > + case NDTPA_INTERVAL_PROBE_TIME: > + NEIGH_VAR_SET(p, INTERVAL_PROBE_TIME, > + nla_get_msecs(tbp[i])); > + break; > case NDTPA_RETRANS_TIME: > NEIGH_VAR_SET(p, RETRANS_TIME, > nla_get_msecs(tbp[i])); > @@ -3562,6 +3569,24 @@ static int neigh_proc_dointvec_zero_intmax(struct ctl_table *ctl, int write, > return ret; > } > > +static int neigh_proc_dointvec_jiffies_positive(struct ctl_table *ctl, int write, > + void *buffer, size_t *lenp, > + loff_t *ppos) Do we need the proc entry to be in jiffies when the netlink option is in ms? Why not make it directly in ms (with _ms similar to other neigh _ms time options) ? IMO, it would be better to be consistent with the netlink option which sets it in ms. It seems the _ms options were added later and usually people want a more understandable value, I haven't seen anyone wanting a jiffies version of a ms interval variable. :) > +{ > + struct ctl_table tmp = *ctl; > + int ret; > + > + int min = HZ; > + int max = INT_MAX; > + > + tmp.extra1 = &min; > + tmp.extra2 = &max; hmm, I don't think these min/max match the netlink attribute's min/max. > + > + ret = proc_dointvec_jiffies_minmax(&tmp, write, buffer, lenp, ppos); > + neigh_proc_update(ctl, write); > + return ret; > +} > + > int neigh_proc_dointvec(struct ctl_table *ctl, int write, void *buffer, > size_t *lenp, loff_t *ppos) > { > @@ -3658,6 +3683,9 @@ static int neigh_proc_base_reachable_time(struct ctl_table *ctl, int write, > #define NEIGH_SYSCTL_USERHZ_JIFFIES_ENTRY(attr, name) \ > NEIGH_SYSCTL_ENTRY(attr, attr, name, 0644, neigh_proc_dointvec_userhz_jiffies) > [snip] Cheers, Nik ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next v3 2/2] net, neigh: introduce interval_probe_time for periodic probe 2022-06-14 9:10 ` Nikolay Aleksandrov @ 2022-06-15 15:28 ` Yuwei Wang 2022-06-15 15:39 ` Nikolay Aleksandrov 0 siblings, 1 reply; 12+ messages in thread From: Yuwei Wang @ 2022-06-15 15:28 UTC (permalink / raw) To: Nikolay Aleksandrov Cc: davem, Jakub Kicinski, Eric Dumazet, Paolo Abeni, Daniel Borkmann, roopa, dsahern, 秦迪, netdev, wangyuweihx On Tue, 14 Jun 2022 at 17:10, Nikolay Aleksandrov <razor@blackwall.org> wrote: > > @@ -2255,6 +2257,7 @@ static const struct nla_policy nl_ntbl_parm_policy[NDTPA_MAX+1] = { > > [NDTPA_ANYCAST_DELAY] = { .type = NLA_U64 }, > > [NDTPA_PROXY_DELAY] = { .type = NLA_U64 }, > > [NDTPA_LOCKTIME] = { .type = NLA_U64 }, > > + [NDTPA_INTERVAL_PROBE_TIME] = { .type = NLA_U64, .min = 1 }, > > shouldn't the min be MSEC_PER_SEC (1 sec minimum) ? thanks, I will make it match the option ;) > > > > +static int neigh_proc_dointvec_jiffies_positive(struct ctl_table *ctl, int write, > > + void *buffer, size_t *lenp, > > + loff_t *ppos) > > Do we need the proc entry to be in jiffies when the netlink option is in ms? > Why not make it directly in ms (with _ms similar to other neigh _ms time options) ? > > IMO, it would be better to be consistent with the netlink option which sets it in ms. > > It seems the _ms options were added later and usually people want a more understandable > value, I haven't seen anyone wanting a jiffies version of a ms interval variable. :) > It was in jiffies because this entry was separated from `DELAY_PROBE_TIME`, it keeps nearly all the things the same as `DELAY_PROBE_TIME`, they are both configured by seconds and read to jiffies, was `ms` in netlink attribute, I think it's ok to keep this consistency, and is there a demand required to configure it by ms? If there is that demand, we can make it configured as ms. > > +{ > > + struct ctl_table tmp = *ctl; > > + int ret; > > + > > + int min = HZ; > > + int max = INT_MAX; > > + > > + tmp.extra1 = &min; > > + tmp.extra2 = &max; > > hmm, I don't think these min/max match the netlink attribute's min/max. thanks, I will make it match the attribute ;) > > > + > > + ret = proc_dointvec_jiffies_minmax(&tmp, write, buffer, lenp, ppos); > > + neigh_proc_update(ctl, write); > > + return ret; > > +} > > + > > int neigh_proc_dointvec(struct ctl_table *ctl, int write, void *buffer, > > size_t *lenp, loff_t *ppos) > > { > > @@ -3658,6 +3683,9 @@ static int neigh_proc_base_reachable_time(struct ctl_table *ctl, int write, > > #define NEIGH_SYSCTL_USERHZ_JIFFIES_ENTRY(attr, name) \ > > NEIGH_SYSCTL_ENTRY(attr, attr, name, 0644, neigh_proc_dointvec_userhz_jiffies) > > > [snip] > Cheers, > Nik Thanks, Yuwei Wang ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next v3 2/2] net, neigh: introduce interval_probe_time for periodic probe 2022-06-15 15:28 ` Yuwei Wang @ 2022-06-15 15:39 ` Nikolay Aleksandrov 2022-06-15 23:33 ` Jakub Kicinski 0 siblings, 1 reply; 12+ messages in thread From: Nikolay Aleksandrov @ 2022-06-15 15:39 UTC (permalink / raw) To: Yuwei Wang Cc: davem, Jakub Kicinski, Eric Dumazet, Paolo Abeni, Daniel Borkmann, roopa, dsahern, 秦迪, netdev, wangyuweihx On 15 June 2022 18:28:27 EEST, Yuwei Wang <wangyuweihx@gmail.com> wrote: >On Tue, 14 Jun 2022 at 17:10, Nikolay Aleksandrov <razor@blackwall.org> wrote: >> > @@ -2255,6 +2257,7 @@ static const struct nla_policy nl_ntbl_parm_policy[NDTPA_MAX+1] = { >> > [NDTPA_ANYCAST_DELAY] = { .type = NLA_U64 }, >> > [NDTPA_PROXY_DELAY] = { .type = NLA_U64 }, >> > [NDTPA_LOCKTIME] = { .type = NLA_U64 }, >> > + [NDTPA_INTERVAL_PROBE_TIME] = { .type = NLA_U64, .min = 1 }, >> >> shouldn't the min be MSEC_PER_SEC (1 sec minimum) ? > >thanks, I will make it match the option ;) > >> > >> > +static int neigh_proc_dointvec_jiffies_positive(struct ctl_table *ctl, int write, >> > + void *buffer, size_t *lenp, >> > + loff_t *ppos) >> >> Do we need the proc entry to be in jiffies when the netlink option is in ms? >> Why not make it directly in ms (with _ms similar to other neigh _ms time options) ? >> >> IMO, it would be better to be consistent with the netlink option which sets it in ms. >> >> It seems the _ms options were added later and usually people want a more understandable >> value, I haven't seen anyone wanting a jiffies version of a ms interval variable. :) >> > >It was in jiffies because this entry was separated from `DELAY_PROBE_TIME`, >it keeps nearly all the things the same as `DELAY_PROBE_TIME`, >they are both configured by seconds and read to jiffies, was `ms` in >netlink attribute, >I think it's ok to keep this consistency, and is there a demand >required to configure it by ms? >If there is that demand, we can make it configured as ms. > no, no demand, just out of user-friendliness :) but I get it keeping it as jiffies is also fine >> > +{ >> > + struct ctl_table tmp = *ctl; >> > + int ret; >> > + >> > + int min = HZ; >> > + int max = INT_MAX; >> > + >> > + tmp.extra1 = &min; >> > + tmp.extra2 = &max; >> >> hmm, I don't think these min/max match the netlink attribute's min/max. > >thanks, I will make it match the attribute ;) > >> >> > + >> > + ret = proc_dointvec_jiffies_minmax(&tmp, write, buffer, lenp, ppos); >> > + neigh_proc_update(ctl, write); >> > + return ret; >> > +} >> > + >> > int neigh_proc_dointvec(struct ctl_table *ctl, int write, void *buffer, >> > size_t *lenp, loff_t *ppos) >> > { >> > @@ -3658,6 +3683,9 @@ static int neigh_proc_base_reachable_time(struct ctl_table *ctl, int write, >> > #define NEIGH_SYSCTL_USERHZ_JIFFIES_ENTRY(attr, name) \ >> > NEIGH_SYSCTL_ENTRY(attr, attr, name, 0644, neigh_proc_dointvec_userhz_jiffies) >> > >> [snip] >> Cheers, >> Nik > >Thanks, >Yuwei Wang ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next v3 2/2] net, neigh: introduce interval_probe_time for periodic probe 2022-06-15 15:39 ` Nikolay Aleksandrov @ 2022-06-15 23:33 ` Jakub Kicinski 2022-06-18 10:24 ` Yuwei Wang 0 siblings, 1 reply; 12+ messages in thread From: Jakub Kicinski @ 2022-06-15 23:33 UTC (permalink / raw) To: Nikolay Aleksandrov Cc: Yuwei Wang, davem, Eric Dumazet, Paolo Abeni, Daniel Borkmann, roopa, dsahern, 秦迪, netdev On Wed, 15 Jun 2022 18:39:53 +0300 Nikolay Aleksandrov wrote: > >> Do we need the proc entry to be in jiffies when the netlink option is in ms? > >> Why not make it directly in ms (with _ms similar to other neigh _ms time options) ? > >> > >> IMO, it would be better to be consistent with the netlink option which sets it in ms. > >> > >> It seems the _ms options were added later and usually people want a more understandable > >> value, I haven't seen anyone wanting a jiffies version of a ms interval variable. :) > >> > > > >It was in jiffies because this entry was separated from `DELAY_PROBE_TIME`, > >it keeps nearly all the things the same as `DELAY_PROBE_TIME`, > >they are both configured by seconds and read to jiffies, was `ms` in > >netlink attribute, > >I think it's ok to keep this consistency, and is there a demand > >required to configure it by ms? > >If there is that demand, we can make it configured as ms. > > no, no demand, just out of user-friendliness :) but > I get it keeping it as jiffies is also fine +1 to using ms ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next v3 2/2] net, neigh: introduce interval_probe_time for periodic probe 2022-06-15 23:33 ` Jakub Kicinski @ 2022-06-18 10:24 ` Yuwei Wang 2022-06-18 10:29 ` Yuwei Wang 0 siblings, 1 reply; 12+ messages in thread From: Yuwei Wang @ 2022-06-18 10:24 UTC (permalink / raw) To: Jakub Kicinski Cc: Nikolay Aleksandrov, davem, Eric Dumazet, Paolo Abeni, Daniel Borkmann, roopa, dsahern, 秦迪, netdev, wangyuweihx On Thu, 16 Jun 2022 at 07:33, Jakub Kicinski <kuba@kernel.org> wrote: > > On Wed, 15 Jun 2022 18:39:53 +0300 Nikolay Aleksandrov wrote: > > >> Do we need the proc entry to be in jiffies when the netlink option is in ms? > > >> Why not make it directly in ms (with _ms similar to other neigh _ms time options) ? > > >> > > >> IMO, it would be better to be consistent with the netlink option which sets it in ms. > > >> > > >> It seems the _ms options were added later and usually people want a more understandable > > >> value, I haven't seen anyone wanting a jiffies version of a ms interval variable. :) > > >> > > > > > >It was in jiffies because this entry was separated from `DELAY_PROBE_TIME`, > > >it keeps nearly all the things the same as `DELAY_PROBE_TIME`, > > >they are both configured by seconds and read to jiffies, was `ms` in > > >netlink attribute, > > >I think it's ok to keep this consistency, and is there a demand > > >required to configure it by ms? > > >If there is that demand, we can make it configured as ms. > > > > no, no demand, just out of user-friendliness :) but > > I get it keeping it as jiffies is also fine > > +1 to using ms If no one is against this, we can make configured as ms? so, there will be updates as follow in the next version patch: - make this option configured as ms, and rename it to `interval_probe_time` - add documentation to Documentation/networking/ip-sysctl - fix damaged whitespace - fix missing `proc_*_jiffies_minmax` on `CONFIG_PROC_SYSCTL` is not defined ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next v3 2/2] net, neigh: introduce interval_probe_time for periodic probe 2022-06-18 10:24 ` Yuwei Wang @ 2022-06-18 10:29 ` Yuwei Wang 0 siblings, 0 replies; 12+ messages in thread From: Yuwei Wang @ 2022-06-18 10:29 UTC (permalink / raw) To: wangyuweihx Cc: Nikolay Aleksandrov, davem, Eric Dumazet, Paolo Abeni, Daniel Borkmann, roopa, dsahern, 秦迪, netdev, Jakub Kicinski On Sat, 18 Jun 2022 at 18:24, Yuwei Wang <wangyuweihx@gmail.com> wrote: > > - make this option configured as ms, and rename it to `interval_probe_time` typo: make this option configured as ms, and rename it to `interval_probe_time_ms` > - add documentation to Documentation/networking/ip-sysctl > - fix damaged whitespace > - fix missing `proc_*_jiffies_minmax` on `CONFIG_PROC_SYSCTL` is not defined ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2022-06-18 10:31 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-06-09 10:57 [PATCH net-next v3 0/2] net, neigh: introduce interval_probe_time for periodic probe Yuwei Wang 2022-06-09 10:57 ` [PATCH net-next v3 1/2] sysctl: add proc_dointvec_jiffies_minmax Yuwei Wang 2022-06-14 5:58 ` Jakub Kicinski 2022-06-09 10:57 ` [PATCH net-next v3 2/2] net, neigh: introduce interval_probe_time for periodic probe Yuwei Wang 2022-06-14 6:02 ` Jakub Kicinski 2022-06-15 15:33 ` Yuwei Wang 2022-06-14 9:10 ` Nikolay Aleksandrov 2022-06-15 15:28 ` Yuwei Wang 2022-06-15 15:39 ` Nikolay Aleksandrov 2022-06-15 23:33 ` Jakub Kicinski 2022-06-18 10:24 ` Yuwei Wang 2022-06-18 10:29 ` Yuwei Wang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).