All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next] neigh: support smaller retrans_time settting
@ 2020-03-31  3:33 Hangbin Liu
  2020-03-31 16:54 ` David Miller
  2020-04-01  2:07 ` [PATCHv2 " Hangbin Liu
  0 siblings, 2 replies; 6+ messages in thread
From: Hangbin Liu @ 2020-03-31  3:33 UTC (permalink / raw)
  To: netdev; +Cc: Eric Dumazet, David S . Miller, Hangbin Liu

Currently, we limited the retrans_time to be greater than HZ/2. For
example, if the HZ = 1000, setting retrans_time less than 500ms will
not work. This makes the user unable to achieve a more accurate control
for bonding arp fast failover.

But remove the sanity check would make the retransmission immediately
if user set retrans_time to 0 by wrong config. So I hard code the sanity
check to 10, which is 10ms if HZ is 1000 and 100ms if HZ is 100.
This number should be enough for user to get more accurate neigh
control.

Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 net/core/neighbour.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 789a73aa7bd8..245bb5bd30e2 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -1125,7 +1125,7 @@ int __neigh_event_send(struct neighbour *neigh, struct sk_buff *skb)
 			neigh->nud_state     = NUD_INCOMPLETE;
 			neigh->updated = now;
 			next = now + max(NEIGH_VAR(neigh->parms, RETRANS_TIME),
-					 HZ/2);
+					 10);
 			neigh_add_timer(neigh, next);
 			immediate_probe = true;
 		} else {
-- 
2.19.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH net-next] neigh: support smaller retrans_time settting
  2020-03-31  3:33 [PATCH net-next] neigh: support smaller retrans_time settting Hangbin Liu
@ 2020-03-31 16:54 ` David Miller
  2020-04-01  2:07 ` [PATCHv2 " Hangbin Liu
  1 sibling, 0 replies; 6+ messages in thread
From: David Miller @ 2020-03-31 16:54 UTC (permalink / raw)
  To: liuhangbin; +Cc: netdev, eric.dumazet

From: Hangbin Liu <liuhangbin@gmail.com>
Date: Tue, 31 Mar 2020 11:33:56 +0800

> Currently, we limited the retrans_time to be greater than HZ/2. For
> example, if the HZ = 1000, setting retrans_time less than 500ms will
> not work. This makes the user unable to achieve a more accurate control
> for bonding arp fast failover.
> 
> But remove the sanity check would make the retransmission immediately
> if user set retrans_time to 0 by wrong config. So I hard code the sanity
> check to 10, which is 10ms if HZ is 1000 and 100ms if HZ is 100.
> This number should be enough for user to get more accurate neigh
> control.
> 
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>

It only makes sense to make the safety limit be constant (time wise),
and therefore relative to HZ.

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCHv2 net-next] neigh: support smaller retrans_time settting
  2020-03-31  3:33 [PATCH net-next] neigh: support smaller retrans_time settting Hangbin Liu
  2020-03-31 16:54 ` David Miller
@ 2020-04-01  2:07 ` Hangbin Liu
  2020-04-01  2:36   ` Eric Dumazet
  2020-04-01  6:46   ` [PATCHv3 " Hangbin Liu
  1 sibling, 2 replies; 6+ messages in thread
From: Hangbin Liu @ 2020-04-01  2:07 UTC (permalink / raw)
  To: netdev; +Cc: David Miller, eric.dumazet, Hangbin Liu

Currently, we limited the retrans_time to be greater than HZ/2. i.e.
setting retrans_time less than 500ms will not work. This makes the user
unable to achieve a more accurate control for bonding arp fast failover.

Update the sanity check to HZ/100, which is 10ms, to let users have more
ability on the retrans_time control.

v2: use HZ instead of hard code number

Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 net/core/neighbour.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 5bf8d22a47ec..46a5611a9f3d 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -1125,7 +1125,7 @@ int __neigh_event_send(struct neighbour *neigh, struct sk_buff *skb)
 			neigh->nud_state     = NUD_INCOMPLETE;
 			neigh->updated = now;
 			next = now + max(NEIGH_VAR(neigh->parms, RETRANS_TIME),
-					 HZ/2);
+					 HZ/100);
 			neigh_add_timer(neigh, next);
 			immediate_probe = true;
 		} else {
-- 
2.19.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCHv2 net-next] neigh: support smaller retrans_time settting
  2020-04-01  2:07 ` [PATCHv2 " Hangbin Liu
@ 2020-04-01  2:36   ` Eric Dumazet
  2020-04-01  6:46   ` [PATCHv3 " Hangbin Liu
  1 sibling, 0 replies; 6+ messages in thread
From: Eric Dumazet @ 2020-04-01  2:36 UTC (permalink / raw)
  To: Hangbin Liu, netdev; +Cc: David Miller



On 3/31/20 7:07 PM, Hangbin Liu wrote:
> Currently, we limited the retrans_time to be greater than HZ/2. i.e.
> setting retrans_time less than 500ms will not work. This makes the user
> unable to achieve a more accurate control for bonding arp fast failover.
> 
> Update the sanity check to HZ/100, which is 10ms, to let users have more
> ability on the retrans_time control.
> 
> v2: use HZ instead of hard code number
> 
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> ---
>  net/core/neighbour.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> index 5bf8d22a47ec..46a5611a9f3d 100644
> --- a/net/core/neighbour.c
> +++ b/net/core/neighbour.c
> @@ -1125,7 +1125,7 @@ int __neigh_event_send(struct neighbour *neigh, struct sk_buff *skb)
>  			neigh->nud_state     = NUD_INCOMPLETE;
>  			neigh->updated = now;
>  			next = now + max(NEIGH_VAR(neigh->parms, RETRANS_TIME),
> -					 HZ/2);
> +					 HZ/100);
>  			neigh_add_timer(neigh, next);
>  			immediate_probe = true;
>  		} else {
> 

Note that IPv6 has a different limit (HZ/10)

It would be nice to have converged values.



^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCHv3 net-next] neigh: support smaller retrans_time settting
  2020-04-01  2:07 ` [PATCHv2 " Hangbin Liu
  2020-04-01  2:36   ` Eric Dumazet
@ 2020-04-01  6:46   ` Hangbin Liu
  2020-04-03  0:55     ` David Miller
  1 sibling, 1 reply; 6+ messages in thread
From: Hangbin Liu @ 2020-04-01  6:46 UTC (permalink / raw)
  To: netdev; +Cc: David Miller, Eric Dumazet, Hangbin Liu

Currently, we limited the retrans_time to be greater than HZ/2. i.e.
setting retrans_time less than 500ms will not work. This makes the user
unable to achieve a more accurate control for bonding arp fast failover.

Update the sanity check to HZ/100, which is 10ms, to let users have more
ability on the retrans_time control.

v3: sync the behavior with IPv6 and update all the timer handler
v2: use HZ instead of hard code number

Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 net/core/neighbour.c | 10 ++++++----
 net/ipv6/addrconf.c  |  7 ++++---
 net/ipv6/ndisc.c     |  4 ++--
 3 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 5bf8d22a47ec..39d37d0ef575 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -1065,11 +1065,12 @@ static void neigh_timer_handler(struct timer_list *t)
 			neigh->updated = jiffies;
 			atomic_set(&neigh->probes, 0);
 			notify = 1;
-			next = now + NEIGH_VAR(neigh->parms, RETRANS_TIME);
+			next = now + max(NEIGH_VAR(neigh->parms, RETRANS_TIME),
+					 HZ/100);
 		}
 	} else {
 		/* NUD_PROBE|NUD_INCOMPLETE */
-		next = now + NEIGH_VAR(neigh->parms, RETRANS_TIME);
+		next = now + max(NEIGH_VAR(neigh->parms, RETRANS_TIME), HZ/100);
 	}
 
 	if ((neigh->nud_state & (NUD_INCOMPLETE | NUD_PROBE)) &&
@@ -1125,7 +1126,7 @@ int __neigh_event_send(struct neighbour *neigh, struct sk_buff *skb)
 			neigh->nud_state     = NUD_INCOMPLETE;
 			neigh->updated = now;
 			next = now + max(NEIGH_VAR(neigh->parms, RETRANS_TIME),
-					 HZ/2);
+					 HZ/100);
 			neigh_add_timer(neigh, next);
 			immediate_probe = true;
 		} else {
@@ -1427,7 +1428,8 @@ void __neigh_set_probe_once(struct neighbour *neigh)
 	neigh->nud_state = NUD_INCOMPLETE;
 	atomic_set(&neigh->probes, neigh_max_probes(neigh));
 	neigh_add_timer(neigh,
-			jiffies + NEIGH_VAR(neigh->parms, RETRANS_TIME));
+			jiffies + max(NEIGH_VAR(neigh->parms, RETRANS_TIME),
+				      HZ/100));
 }
 EXPORT_SYMBOL(__neigh_set_probe_once);
 
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index a11fd4d67832..89aa6da8661d 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -1357,7 +1357,7 @@ static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp,
 
 	regen_advance = idev->cnf.regen_max_retry *
 			idev->cnf.dad_transmits *
-			NEIGH_VAR(idev->nd_parms, RETRANS_TIME) / HZ;
+			max(NEIGH_VAR(idev->nd_parms, RETRANS_TIME), HZ/100) / HZ;
 
 	/* recalculate max_desync_factor each time and update
 	 * idev->desync_factor if it's larger
@@ -4117,7 +4117,8 @@ static void addrconf_dad_work(struct work_struct *w)
 
 	ifp->dad_probes--;
 	addrconf_mod_dad_work(ifp,
-			      NEIGH_VAR(ifp->idev->nd_parms, RETRANS_TIME));
+			      max(NEIGH_VAR(ifp->idev->nd_parms, RETRANS_TIME),
+				  HZ/100));
 	spin_unlock(&ifp->lock);
 	write_unlock_bh(&idev->lock);
 
@@ -4523,7 +4524,7 @@ static void addrconf_verify_rtnl(void)
 				   !(ifp->flags&IFA_F_TENTATIVE)) {
 				unsigned long regen_advance = ifp->idev->cnf.regen_max_retry *
 					ifp->idev->cnf.dad_transmits *
-					NEIGH_VAR(ifp->idev->nd_parms, RETRANS_TIME) / HZ;
+					max(NEIGH_VAR(ifp->idev->nd_parms, RETRANS_TIME), HZ/100) / HZ;
 
 				if (age >= ifp->prefered_lft - regen_advance) {
 					struct inet6_ifaddr *ifpub = ifp->ifpub;
diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index 6ffa153e5166..1ecd4e9b0bdf 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -1359,8 +1359,8 @@ static void ndisc_router_discovery(struct sk_buff *skb)
 
 		if (rtime && rtime/1000 < MAX_SCHEDULE_TIMEOUT/HZ) {
 			rtime = (rtime*HZ)/1000;
-			if (rtime < HZ/10)
-				rtime = HZ/10;
+			if (rtime < HZ/100)
+				rtime = HZ/100;
 			NEIGH_VAR_SET(in6_dev->nd_parms, RETRANS_TIME, rtime);
 			in6_dev->tstamp = jiffies;
 			send_ifinfo_notify = true;
-- 
2.19.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCHv3 net-next] neigh: support smaller retrans_time settting
  2020-04-01  6:46   ` [PATCHv3 " Hangbin Liu
@ 2020-04-03  0:55     ` David Miller
  0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2020-04-03  0:55 UTC (permalink / raw)
  To: liuhangbin; +Cc: netdev, eric.dumazet

From: Hangbin Liu <liuhangbin@gmail.com>
Date: Wed,  1 Apr 2020 14:46:20 +0800

> Currently, we limited the retrans_time to be greater than HZ/2. i.e.
> setting retrans_time less than 500ms will not work. This makes the user
> unable to achieve a more accurate control for bonding arp fast failover.
> 
> Update the sanity check to HZ/100, which is 10ms, to let users have more
> ability on the retrans_time control.
> 
> v3: sync the behavior with IPv6 and update all the timer handler
> v2: use HZ instead of hard code number
> 
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>

Applied, I like the consistency between ipv4 and ipv6 now.

Thanks.

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-04-03  0:55 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-31  3:33 [PATCH net-next] neigh: support smaller retrans_time settting Hangbin Liu
2020-03-31 16:54 ` David Miller
2020-04-01  2:07 ` [PATCHv2 " Hangbin Liu
2020-04-01  2:36   ` Eric Dumazet
2020-04-01  6:46   ` [PATCHv3 " Hangbin Liu
2020-04-03  0:55     ` David Miller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.