* Re: [PATCH] ip route: timeout for routes has to be set in seconds
[not found] <5a205385ce594320936c4e330d52f532@HQ1WP-EXMB11.corp.brocade.com>
@ 2016-07-01 0:29 ` Stephen Hemminger
0 siblings, 0 replies; 4+ messages in thread
From: Stephen Hemminger @ 2016-07-01 0:29 UTC (permalink / raw)
To: Andrey Vagin; +Cc: netdev, Andrew Vagin, Xin Long, Hangbin Liu
On Tue, 28 Jun 2016 23:27:14 +0000
Andrey Vagin <avagin@openvz.org> wrote:
> From: Andrew Vagin <avagin@virtuozzo.com>
>
> Currently a timeout is multiplied by HZ in user-space and
> then it multiplied by HZ in kernel-space.
>
> $ ./ip/ip r add 2002::0/64 dev veth1 expires 10
> $ ./ip/ip -6 r
> 2002::/64 dev veth1 metric 1024 linkdown expires 996sec pref medium
>
> Cc: Xin Long <lucien.xin@gmail.com>
> Cc: Hangbin Liu <liuhangbin@gmail.com>
> Cc: Stephen Hemminger <shemming@brocade.com>
> Fixes: 68eede250500 ("route: allow routes to be configured with expire values")
> Signed-off-by: Andrew Vagin <avagin@virtuozzo.com>
Applied.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ip route: timeout for routes has to be set in seconds
[not found] ` <15a8916ec54d447b95cff28f99118342@HQ1WP-EXMB11.corp.brocade.com>
@ 2016-06-29 16:25 ` Stephen Hemminger
0 siblings, 0 replies; 4+ messages in thread
From: Stephen Hemminger @ 2016-06-29 16:25 UTC (permalink / raw)
To: Xin Long
Cc: Andrey Vagin, Hannes Frederic Sowa, network dev, Andrew Vagin,
Hangbin Liu
> There's another issue.
> now the expires transition with HZ happens in kernel when we add
> route with expire, but when we dump, the expires transition is in the
> user-space:
> if (ci->rta_expires != 0)
> fprintf(fp, " expires %dsec",
> ci->rta_expires/hz);
>
> I'm not sure if the *hz* value between kernel and user-space are
> always same. I'd like to do this transition both in kernel. just like
> ipaddr's
> valid_lft, preferred_lft.
All API's from applications are supposed to use USER_HZ which is 100hz
on most platforms. Actual kernel internal hz is configurable.
There were some cases legacy of early days where iproute2 had to deal
with kernel hz, mostly in the QoS stuff.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ip route: timeout for routes has to be set in seconds
2016-06-28 23:27 Andrey Vagin
@ 2016-06-29 10:26 ` Xin Long
[not found] ` <15a8916ec54d447b95cff28f99118342@HQ1WP-EXMB11.corp.brocade.com>
1 sibling, 0 replies; 4+ messages in thread
From: Xin Long @ 2016-06-29 10:26 UTC (permalink / raw)
To: Andrey Vagin, Hannes Frederic Sowa
Cc: network dev, Andrew Vagin, Hangbin Liu, Stephen Hemminger
Hi,
On Wed, Jun 29, 2016 at 7:27 AM, Andrey Vagin <avagin@openvz.org> wrote:
> From: Andrew Vagin <avagin@virtuozzo.com>
>
> Currently a timeout is multiplied by HZ in user-space and
> then it multiplied by HZ in kernel-space.
>
> $ ./ip/ip r add 2002::0/64 dev veth1 expires 10
> $ ./ip/ip -6 r
> 2002::/64 dev veth1 metric 1024 linkdown expires 996sec pref medium
>
> Cc: Xin Long <lucien.xin@gmail.com>
> Cc: Hangbin Liu <liuhangbin@gmail.com>
> Cc: Stephen Hemminger <shemming@brocade.com>
> Fixes: 68eede250500 ("route: allow routes to be configured with expire values")
> Signed-off-by: Andrew Vagin <avagin@virtuozzo.com>
> ---
> ip/iproute.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/ip/iproute.c b/ip/iproute.c
> index 8224d7f..7c0f5a4 100644
> --- a/ip/iproute.c
> +++ b/ip/iproute.c
> @@ -839,7 +839,6 @@ static int iproute_modify(int cmd, unsigned int flags, int argc, char **argv)
> int table_ok = 0;
> int raw = 0;
> int type_ok = 0;
> - static int hz;
>
> memset(&req, 0, sizeof(req));
>
> @@ -923,9 +922,7 @@ static int iproute_modify(int cmd, unsigned int flags, int argc, char **argv)
> NEXT_ARG();
> if (get_u32(&expires, *argv, 0))
> invarg("\"expires\" value is invalid\n", *argv);
> - if (!hz)
> - hz = get_user_hz();
> - addattr32(&req.n, sizeof(req), RTA_EXPIRES, expires*hz);
> + addattr32(&req.n, sizeof(req), RTA_EXPIRES, expires);
> } else if (matches(*argv, "metric") == 0 ||
> matches(*argv, "priority") == 0 ||
> strcmp(*argv, "preference") == 0) {
> --
> 2.7.4
>
This patch looks good to me.
There's another issue.
now the expires transition with HZ happens in kernel when we add
route with expire, but when we dump, the expires transition is in the
user-space:
if (ci->rta_expires != 0)
fprintf(fp, " expires %dsec",
ci->rta_expires/hz);
I'm not sure if the *hz* value between kernel and user-space are
always same. I'd like to do this transition both in kernel. just like
ipaddr's
valid_lft, preferred_lft.
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] ip route: timeout for routes has to be set in seconds
@ 2016-06-28 23:27 Andrey Vagin
2016-06-29 10:26 ` Xin Long
[not found] ` <15a8916ec54d447b95cff28f99118342@HQ1WP-EXMB11.corp.brocade.com>
0 siblings, 2 replies; 4+ messages in thread
From: Andrey Vagin @ 2016-06-28 23:27 UTC (permalink / raw)
To: netdev; +Cc: Andrew Vagin, Xin Long, Hangbin Liu, Stephen Hemminger
From: Andrew Vagin <avagin@virtuozzo.com>
Currently a timeout is multiplied by HZ in user-space and
then it multiplied by HZ in kernel-space.
$ ./ip/ip r add 2002::0/64 dev veth1 expires 10
$ ./ip/ip -6 r
2002::/64 dev veth1 metric 1024 linkdown expires 996sec pref medium
Cc: Xin Long <lucien.xin@gmail.com>
Cc: Hangbin Liu <liuhangbin@gmail.com>
Cc: Stephen Hemminger <shemming@brocade.com>
Fixes: 68eede250500 ("route: allow routes to be configured with expire values")
Signed-off-by: Andrew Vagin <avagin@virtuozzo.com>
---
ip/iproute.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/ip/iproute.c b/ip/iproute.c
index 8224d7f..7c0f5a4 100644
--- a/ip/iproute.c
+++ b/ip/iproute.c
@@ -839,7 +839,6 @@ static int iproute_modify(int cmd, unsigned int flags, int argc, char **argv)
int table_ok = 0;
int raw = 0;
int type_ok = 0;
- static int hz;
memset(&req, 0, sizeof(req));
@@ -923,9 +922,7 @@ static int iproute_modify(int cmd, unsigned int flags, int argc, char **argv)
NEXT_ARG();
if (get_u32(&expires, *argv, 0))
invarg("\"expires\" value is invalid\n", *argv);
- if (!hz)
- hz = get_user_hz();
- addattr32(&req.n, sizeof(req), RTA_EXPIRES, expires*hz);
+ addattr32(&req.n, sizeof(req), RTA_EXPIRES, expires);
} else if (matches(*argv, "metric") == 0 ||
matches(*argv, "priority") == 0 ||
strcmp(*argv, "preference") == 0) {
--
2.7.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-07-01 0:31 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <5a205385ce594320936c4e330d52f532@HQ1WP-EXMB11.corp.brocade.com>
2016-07-01 0:29 ` [PATCH] ip route: timeout for routes has to be set in seconds Stephen Hemminger
2016-06-28 23:27 Andrey Vagin
2016-06-29 10:26 ` Xin Long
[not found] ` <15a8916ec54d447b95cff28f99118342@HQ1WP-EXMB11.corp.brocade.com>
2016-06-29 16:25 ` Stephen Hemminger
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.