Hi a.Hoang,
I have really tried to make it as "short" as possible because of the
"80-character" rule. Moreover, the value is not only a time value but also
playing as a "switch" to turn off/on the auto removal feature (see the commit
message). Anyway, it is just a name so we can change it to whatever. I will
wait for other opinions...
Regarding its unit, yes I thought about that (coding would be even one step
less) but 'milliseconds' does not seem to make sense, especially from a user
perspective, that a node can just need to be removed in minutes, like 5 or 10
minutes. So, I'd prefer to keep it in 'seconds' which is scalable enough.
BR/Tuong
-----Original Message-----
From: Hoang Huu Le <hoang.h...@dektech.com.au>
Sent: Tuesday, April 14, 2020 2:54 PM
To: Tuong Tong Lien <tuong.t.l...@dektech.com.au>; jma...@redhat.com;
ma...@donjonn.com; ying....@windriver.com; tipc-discussion@lists.sourceforge.net
Cc: tipc-dek <tipc-...@dektech.com.au>
Subject: RE: [PATCH RFC] tipc: make peer node cleanup timer configurable
Hi Tuong,
The 'node_cleanup' is not clear in meaning. I'd prefer using
'node_cleanup_time_secs'.
I think we should use 'milliseconds' for more popular than 'seconds' in
proc/sysctl.
Regards,
Hoang
-----Original Message-----
From: Tuong Tong Lien <tuong.t.l...@dektech.com.au>
Sent: Tuesday, April 14, 2020 2:30 PM
To: jma...@redhat.com; ma...@donjonn.com; ying....@windriver.com;
tipc-discussion@lists.sourceforge.net
Cc: tipc-dek <tipc-...@dektech.com.au>
Subject: [PATCH RFC] tipc: make peer node cleanup timer configurable
Since commit 6a939f365bdb ("tipc: Auto removal of peer down node
instance"), a peer node instance is automatically clean up after 5 mins
without contact. This changed behavior is not really expected from user
who would expect to see the node with the "down" state in the node list
as before instead.
Also, the timer value is said to be small that the peer node might just
be rebooting, so will come back soon. This is absolutely no problem but
one wants to extend it to 10 minutes.
This commit makes the timer configurable via the sysctl file:
/proc/sys/net/tipc/node_cleanup
which indicates in seconds how long a peer down node should be removed.
The default value is 300 i.e. 5 mins, while a value of '0' will disable
the auto removal feature.
Signed-off-by: Tuong Lien <tuong.t.l...@dektech.com.au>
---
net/tipc/core.h | 1 +
net/tipc/node.c | 15 +++++++++------
net/tipc/sysctl.c | 8 ++++++++
3 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/net/tipc/core.h b/net/tipc/core.h
index 631d83c9705f..7d07f9086936 100644
--- a/net/tipc/core.h
+++ b/net/tipc/core.h
@@ -89,6 +89,7 @@ struct tipc_crypto;
extern unsigned int tipc_net_id __read_mostly;
extern int sysctl_tipc_rmem[3] __read_mostly;
extern int sysctl_tipc_named_timeout __read_mostly;
+extern int sysctl_tipc_node_cleanup __read_mostly;
struct tipc_net {
u8 node_id[NODE_ID_LEN];
diff --git a/net/tipc/node.c b/net/tipc/node.c
index 2373a66f3b59..5cf01e182730 100644
--- a/net/tipc/node.c
+++ b/net/tipc/node.c
@@ -46,8 +46,11 @@
#include "trace.h"
#include "crypto.h"
+int sysctl_tipc_node_cleanup __read_mostly = 300; /* in seconds */
+
#define INVALID_NODE_SIG 0x10000
-#define NODE_CLEANUP_AFTER 300000
+#define NODE_CLEANUP_AFTER \
+ msecs_to_jiffies(sysctl_tipc_node_cleanup * 1000)
/* Flags used to take different actions according to flag type
* TIPC_NOTIFY_NODE_DOWN: notify node is down
@@ -130,7 +133,7 @@ struct tipc_node {
unsigned long keepalive_intv;
struct timer_list timer;
struct rcu_head rcu;
- unsigned long delete_at;
+ unsigned long checkpt;
struct net *peer_net;
u32 peer_hash_mix;
#ifdef CONFIG_TIPC_CRYPTO
@@ -537,7 +540,7 @@ struct tipc_node *tipc_node_create(struct net *net, u32
addr, u8 *peer_id,
for (i = 0; i < MAX_BEARERS; i++)
spin_lock_init(&n->links[i].lock);
n->state = SELF_DOWN_PEER_LEAVING;
- n->delete_at = jiffies + msecs_to_jiffies(NODE_CLEANUP_AFTER);
+ n->checkpt = jiffies;
n->signature = INVALID_NODE_SIG;
n->active_links[0] = INVALID_BEARER_ID;
n->active_links[1] = INVALID_BEARER_ID;
@@ -726,8 +729,8 @@ static bool tipc_node_cleanup(struct tipc_node *peer)
return false;
tipc_node_write_lock(peer);
-
- if (!node_is_up(peer) && time_after(jiffies, peer->delete_at)) {
+ if (!node_is_up(peer) && NODE_CLEANUP_AFTER &&
+ time_after(jiffies, peer->checkpt + NODE_CLEANUP_AFTER)) {
tipc_node_clear_links(peer);
tipc_node_delete_from_list(peer);
deleted = true;
@@ -1478,7 +1481,7 @@ static void node_lost_contact(struct tipc_node *n,
uint i;
pr_debug("Lost contact with %x\n", n->addr);
- n->delete_at = jiffies + msecs_to_jiffies(NODE_CLEANUP_AFTER);
+ n->checkpt = jiffies;
trace_tipc_node_lost_contact(n, true, " ");
/* Clean up broadcast state */
diff --git a/net/tipc/sysctl.c b/net/tipc/sysctl.c
index 58ab3d6dcdce..087a89b27b9a 100644
--- a/net/tipc/sysctl.c
+++ b/net/tipc/sysctl.c
@@ -75,6 +75,14 @@ static struct ctl_table tipc_table[] = {
.extra1 = SYSCTL_ONE,
},
#endif
+ {
+ .procname = "node_cleanup",
+ .data = &sysctl_tipc_node_cleanup,
+ .maxlen = sizeof(sysctl_tipc_node_cleanup),
+ .mode = 0644,
+ .proc_handler = proc_dointvec_minmax,
+ .extra1 = SYSCTL_ZERO,
+ },
{}
};
--
2.13.7
_______________________________________________
tipc-discussion mailing list
tipc-discussion@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tipc-discussion
