* [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
* [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 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
* 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-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-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-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).