* [PATCH 0/2] ipv6: fix stuck RA-derived route in container after migration with criu
@ 2016-08-16 13:59 Andrew Yourtchenko
2016-08-16 13:59 ` [PATCH 1/2] ipv6: save route expiry in RTA_EXPIRES if RTF_EXPIRES set Andrew Yourtchenko
` (3 more replies)
0 siblings, 4 replies; 12+ messages in thread
From: Andrew Yourtchenko @ 2016-08-16 13:59 UTC (permalink / raw)
To: netdev; +Cc: Andrew Yourtchenko
This patchset fixes the connectivity problem for containers
with RA-derived default route, after they were migrated using criu:
the default routes would lose their "expires" value and become
stuck forever. The corresponding criu issue with the discussion
is at https://github.com/xemul/criu/issues/177
The latter uses "ip -6 route save" to save the routes during
the migration, and "ip -6 route restore" during restore, so the problem
is easily reproducible even without criu.
There are two problems, hence two patches in this patchset:
1) the expiry time for the route is saved in
RTA_CACHEINFO via rtnl_put_cacheinfo, but the code in rtm_to_fib6_config
expects the RTA_EXPIRES. Rather than cherrypicking in the restore code path
from RTA_CACHEINFO, adding RTA_EXPIRES upon save seemed like
a better option.
2) the restored route, even with the properly restored expires,
does not have the correct RTF_* flags set (RTF_ADDRCONF|RTF_DEFAULT),
preventing the incoming router advertisements from updating it.
During the code review I noticed RTF_ROUTEINFO would be also lost
during save/restore. This can be viewed as an operation symmetric
to that done in f0396f60d7c165018c9b203fb9b89fb224835578.
Tested both net and net-next with the patches.
Andrew Yourtchenko (2):
ipv6: save route expiry in RTA_EXPIRES if RTF_EXPIRES set
ipv6: fixup RTF_* flags when restoring RTPROT_RA route from rtnetlink
net/ipv6/route.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
--
2.7.4
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/2] ipv6: save route expiry in RTA_EXPIRES if RTF_EXPIRES set
2016-08-16 13:59 [PATCH 0/2] ipv6: fix stuck RA-derived route in container after migration with criu Andrew Yourtchenko
@ 2016-08-16 13:59 ` Andrew Yourtchenko
2016-08-16 13:59 ` [PATCH 2/2] ipv6: fixup RTF_* flags when restoring RTPROT_RA route from rtnetlink Andrew Yourtchenko
` (2 subsequent siblings)
3 siblings, 0 replies; 12+ messages in thread
From: Andrew Yourtchenko @ 2016-08-16 13:59 UTC (permalink / raw)
To: netdev; +Cc: Andrew Yourtchenko
This allows "ip -6 route save" to save the expiry for the routes
that have it, such that it can be correctly restored later by
"ip -6 route restore".
If a route has RTF_EXPIRES set, generate RTA_EXPIRES value which
will be used to restore the flag and expiry value by already
existing code in rtm_to_fib6_config.
The expiry was already being saved as part of RTA_CACHEINFO
in rtnl_put_cacheinfo(), but adding code to generate RTF_EXPIRES upon save
looked more appropriate than redundant cherrypicking from
RTA_CACHEINFO upon restore.
Signed-off-by: Andrew Yourtchenko <ayourtch@gmail.com>
---
net/ipv6/route.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 4981755..dc37017 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -3244,6 +3244,14 @@ static int rt6_fill_node(struct net *net,
if (rtnl_put_cacheinfo(skb, &rt->dst, 0, expires, rt->dst.error) < 0)
goto nla_put_failure;
+ /* Can't rely on expires == 0. It is zero if no expires flag,
+ * or if the timing is precisely at expiry. So, recheck the flag.
+ */
+ if (rt->rt6i_flags & RTF_EXPIRES)
+ if (nla_put_u32(skb, RTA_EXPIRES,
+ expires > 0 ? expires / HZ : 0))
+ goto nla_put_failure;
+
if (nla_put_u8(skb, RTA_PREF, IPV6_EXTRACT_PREF(rt->rt6i_flags)))
goto nla_put_failure;
--
2.7.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/2] ipv6: fixup RTF_* flags when restoring RTPROT_RA route from rtnetlink
2016-08-16 13:59 [PATCH 0/2] ipv6: fix stuck RA-derived route in container after migration with criu Andrew Yourtchenko
2016-08-16 13:59 ` [PATCH 1/2] ipv6: save route expiry in RTA_EXPIRES if RTF_EXPIRES set Andrew Yourtchenko
@ 2016-08-16 13:59 ` Andrew Yourtchenko
2016-08-19 4:09 ` [PATCH 0/2] ipv6: fix stuck RA-derived route in container after migration with criu David Miller
2016-08-19 16:41 ` [PATCH v2 " Andrew Yourtchenko
3 siblings, 0 replies; 12+ messages in thread
From: Andrew Yourtchenko @ 2016-08-16 13:59 UTC (permalink / raw)
To: netdev; +Cc: Andrew Yourtchenko
Fix the flags for RA-derived routes that were saved
via "ip -6 route save" and and subsequently restored via
"ip -6 route restore", allowing the incoming router advertisements
to update them, rather than complain about inability to do so.
Upon the restore of RA-derived saved routes, set the RTF_ADDRCONF
to indicate that the source of the route was originally
a router advertisement, and set the RTF_DEFAULT or RTF_ROUTEINFO
flag depending on prefix length. This can be considered a
sister change of f0396f60d7c165018c9b203fb9b89fb224835578, in
the other direction.
Signed-off-by: Andrew Yourtchenko <ayourtch@gmail.com>
---
net/ipv6/route.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index dc37017..4da7237 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -2769,6 +2769,16 @@ static int rtm_to_fib6_config(struct sk_buff *skb, struct nlmsghdr *nlh,
cfg->fc_protocol = rtm->rtm_protocol;
cfg->fc_type = rtm->rtm_type;
+ if (rtm->rtm_protocol == RTPROT_RA) {
+ /* RA-derived route: set flags accordingly. */
+ cfg->fc_flags |= RTF_ADDRCONF;
+ if (rtm->rtm_dst_len == 0) {
+ cfg->fc_flags |= RTF_DEFAULT;
+ } else {
+ cfg->fc_flags |= RTF_ROUTEINFO;
+ }
+ }
+
if (rtm->rtm_type == RTN_UNREACHABLE ||
rtm->rtm_type == RTN_BLACKHOLE ||
rtm->rtm_type == RTN_PROHIBIT ||
--
2.7.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 0/2] ipv6: fix stuck RA-derived route in container after migration with criu
2016-08-16 13:59 [PATCH 0/2] ipv6: fix stuck RA-derived route in container after migration with criu Andrew Yourtchenko
2016-08-16 13:59 ` [PATCH 1/2] ipv6: save route expiry in RTA_EXPIRES if RTF_EXPIRES set Andrew Yourtchenko
2016-08-16 13:59 ` [PATCH 2/2] ipv6: fixup RTF_* flags when restoring RTPROT_RA route from rtnetlink Andrew Yourtchenko
@ 2016-08-19 4:09 ` David Miller
2016-08-19 16:41 ` [PATCH v2 " Andrew Yourtchenko
3 siblings, 0 replies; 12+ messages in thread
From: David Miller @ 2016-08-19 4:09 UTC (permalink / raw)
To: ayourtch; +Cc: netdev
The indentation of both of your patches are incorrect.
You must indent each basic block by a full TAB character.
You must also line up multi-line function calls, such that
the second and subsequent lines start precisely at the first
column after the opennning parenthesis of the function call
itself. You must use the appropriate number of TAB and
SPC characters necessary to achieve this.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 0/2] ipv6: fix stuck RA-derived route in container after migration with criu
2016-08-16 13:59 [PATCH 0/2] ipv6: fix stuck RA-derived route in container after migration with criu Andrew Yourtchenko
` (2 preceding siblings ...)
2016-08-19 4:09 ` [PATCH 0/2] ipv6: fix stuck RA-derived route in container after migration with criu David Miller
@ 2016-08-19 16:41 ` Andrew Yourtchenko
2016-08-19 16:41 ` [PATCH v2 1/2] ipv6: save route expiry in RTA_EXPIRES if RTF_EXPIRES set Andrew Yourtchenko
2016-08-19 16:41 ` [PATCH v2 2/2] ipv6: fixup RTF_* flags when restoring RTPROT_RA route from rtnetlink Andrew Yourtchenko
3 siblings, 2 replies; 12+ messages in thread
From: Andrew Yourtchenko @ 2016-08-19 16:41 UTC (permalink / raw)
To: netdev; +Cc: Andrew Yourtchenko
This patchset fixes the connectivity problem for containers
with RA-derived default route, after they were migrated using criu:
the default routes would lose their "expires" value and become
stuck forever. The corresponding criu issue with the discussion
is at https://github.com/xemul/criu/issues/177
The latter uses "ip -6 route save" to save the routes during
the migration, and "ip -6 route restore" during restore, so the problem
is easily reproducible even without criu.
There are two problems, hence two patches in this patchset:
1) the expiry time for the route is saved in
RTA_CACHEINFO via rtnl_put_cacheinfo, but the code in rtm_to_fib6_config
expects the RTA_EXPIRES. Rather than cherrypicking in the restore code path
from RTA_CACHEINFO, adding RTA_EXPIRES upon save seemed like
a better option.
2) the restored route, even with the properly restored expires,
does not have the correct RTF_* flags set (RTF_ADDRCONF|RTF_DEFAULT),
preventing the incoming router advertisements from updating it.
During the code review I noticed RTF_ROUTEINFO would be also lost
during save/restore. This can be viewed as an operation symmetric
to that done in f0396f60d7c165018c9b203fb9b89fb224835578.
Tested both net and net-next with the patches.
Changes since v1 [1]:
* Fixed the indentation in both patches as per David Miller's review comments:
- basic block indented always by a TAB
- multiline function call second line to start at the first column
after the opening parenthesis of the function call, using
appropriate number of TABs (and SPC only at the end, if needed).
[1] v1: http://marc.info/?l=linux-netdev&m=147135599322286&w=2
Andrew Yourtchenko (2):
ipv6: save route expiry in RTA_EXPIRES if RTF_EXPIRES set
ipv6: fixup RTF_* flags when restoring RTPROT_RA route from rtnetlink
net/ipv6/route.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
--
2.7.4
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 1/2] ipv6: save route expiry in RTA_EXPIRES if RTF_EXPIRES set
2016-08-19 16:41 ` [PATCH v2 " Andrew Yourtchenko
@ 2016-08-19 16:41 ` Andrew Yourtchenko
2016-08-19 20:26 ` Eric Dumazet
2016-08-19 16:41 ` [PATCH v2 2/2] ipv6: fixup RTF_* flags when restoring RTPROT_RA route from rtnetlink Andrew Yourtchenko
1 sibling, 1 reply; 12+ messages in thread
From: Andrew Yourtchenko @ 2016-08-19 16:41 UTC (permalink / raw)
To: netdev; +Cc: Andrew Yourtchenko
This allows "ip -6 route save" to save the expiry for the routes
that have it, such that it can be correctly restored later by
"ip -6 route restore".
If a route has RTF_EXPIRES set, generate RTA_EXPIRES value which
will be used to restore the flag and expiry value by already
existing code in rtm_to_fib6_config.
The expiry was already being saved as part of RTA_CACHEINFO
in rtnl_put_cacheinfo(), but adding code to generate RTF_EXPIRES upon save
looked more appropriate than redundant cherrypicking from
RTA_CACHEINFO upon restore.
Signed-off-by: Andrew Yourtchenko <ayourtch@gmail.com>
---
Changes since v1 [1]:
* fixed the indentation in a multiline function call
as per David Miller's review
[1] v1: http://marc.info/?l=linux-netdev&m=147135597422280&w=2
net/ipv6/route.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 4981755..f5b987d 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -3244,6 +3244,14 @@ static int rt6_fill_node(struct net *net,
if (rtnl_put_cacheinfo(skb, &rt->dst, 0, expires, rt->dst.error) < 0)
goto nla_put_failure;
+ /* Can't rely on expires == 0. It is zero if no expires flag,
+ * or if the timing is precisely at expiry. So, recheck the flag.
+ */
+ if (rt->rt6i_flags & RTF_EXPIRES)
+ if (nla_put_u32(skb, RTA_EXPIRES,
+ expires > 0 ? expires / HZ : 0))
+ goto nla_put_failure;
+
if (nla_put_u8(skb, RTA_PREF, IPV6_EXTRACT_PREF(rt->rt6i_flags)))
goto nla_put_failure;
--
2.7.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 2/2] ipv6: fixup RTF_* flags when restoring RTPROT_RA route from rtnetlink
2016-08-19 16:41 ` [PATCH v2 " Andrew Yourtchenko
2016-08-19 16:41 ` [PATCH v2 1/2] ipv6: save route expiry in RTA_EXPIRES if RTF_EXPIRES set Andrew Yourtchenko
@ 2016-08-19 16:41 ` Andrew Yourtchenko
2016-08-19 18:07 ` Sergei Shtylyov
1 sibling, 1 reply; 12+ messages in thread
From: Andrew Yourtchenko @ 2016-08-19 16:41 UTC (permalink / raw)
To: netdev; +Cc: Andrew Yourtchenko
Fix the flags for RA-derived routes that were saved
via "ip -6 route save" and and subsequently restored via
"ip -6 route restore", allowing the incoming router advertisements
to update them, rather than complain about inability to do so.
Upon the restore of RA-derived saved routes, set the RTF_ADDRCONF
to indicate that the source of the route was originally
a router advertisement, and set the RTF_DEFAULT or RTF_ROUTEINFO
flag depending on prefix length. This can be considered a
sister change of f0396f60d7c165018c9b203fb9b89fb224835578, in
the other direction.
Signed-off-by: Andrew Yourtchenko <ayourtch@gmail.com>
---
Changes since v1 [1]:
* fixed the indentation of the basic blocks to be always a full TAB
as per David Miller's review
[1] v1: http://marc.info/?l=linux-netdev&m=147135599322285&w=2
net/ipv6/route.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index f5b987d..60d95cd 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -2769,6 +2769,16 @@ static int rtm_to_fib6_config(struct sk_buff *skb, struct nlmsghdr *nlh,
cfg->fc_protocol = rtm->rtm_protocol;
cfg->fc_type = rtm->rtm_type;
+ if (rtm->rtm_protocol == RTPROT_RA) {
+ /* RA-derived route: set flags accordingly. */
+ cfg->fc_flags |= RTF_ADDRCONF;
+ if (rtm->rtm_dst_len == 0) {
+ cfg->fc_flags |= RTF_DEFAULT;
+ } else {
+ cfg->fc_flags |= RTF_ROUTEINFO;
+ }
+ }
+
if (rtm->rtm_type == RTN_UNREACHABLE ||
rtm->rtm_type == RTN_BLACKHOLE ||
rtm->rtm_type == RTN_PROHIBIT ||
--
2.7.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/2] ipv6: fixup RTF_* flags when restoring RTPROT_RA route from rtnetlink
2016-08-19 16:41 ` [PATCH v2 2/2] ipv6: fixup RTF_* flags when restoring RTPROT_RA route from rtnetlink Andrew Yourtchenko
@ 2016-08-19 18:07 ` Sergei Shtylyov
2016-08-22 11:04 ` Andrew Yourtchenko
0 siblings, 1 reply; 12+ messages in thread
From: Sergei Shtylyov @ 2016-08-19 18:07 UTC (permalink / raw)
To: Andrew Yourtchenko, netdev
Hello.
On 08/19/2016 07:41 PM, Andrew Yourtchenko wrote:
> Fix the flags for RA-derived routes that were saved
> via "ip -6 route save" and and subsequently restored via
> "ip -6 route restore", allowing the incoming router advertisements
> to update them, rather than complain about inability to do so.
>
> Upon the restore of RA-derived saved routes, set the RTF_ADDRCONF
> to indicate that the source of the route was originally
> a router advertisement, and set the RTF_DEFAULT or RTF_ROUTEINFO
> flag depending on prefix length. This can be considered a
> sister change of f0396f60d7c165018c9b203fb9b89fb224835578, in
It's enough to specify 12 digits but you also need to specify the commit
summary enclosed in ("").
> the other direction.
>
> Signed-off-by: Andrew Yourtchenko <ayourtch@gmail.com>
> ---
> Changes since v1 [1]:
> * fixed the indentation of the basic blocks to be always a full TAB
> as per David Miller's review
>
> [1] v1: http://marc.info/?l=linux-netdev&m=147135599322285&w=2
>
> net/ipv6/route.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index f5b987d..60d95cd 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -2769,6 +2769,16 @@ static int rtm_to_fib6_config(struct sk_buff *skb, struct nlmsghdr *nlh,
> cfg->fc_protocol = rtm->rtm_protocol;
> cfg->fc_type = rtm->rtm_type;
>
> + if (rtm->rtm_protocol == RTPROT_RA) {
> + /* RA-derived route: set flags accordingly. */
> + cfg->fc_flags |= RTF_ADDRCONF;
> + if (rtm->rtm_dst_len == 0) {
> + cfg->fc_flags |= RTF_DEFAULT;
> + } else {
> + cfg->fc_flags |= RTF_ROUTEINFO;
> + }
{} not needed here and above.
[...]
MBR, Sergei
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] ipv6: save route expiry in RTA_EXPIRES if RTF_EXPIRES set
2016-08-19 16:41 ` [PATCH v2 1/2] ipv6: save route expiry in RTA_EXPIRES if RTF_EXPIRES set Andrew Yourtchenko
@ 2016-08-19 20:26 ` Eric Dumazet
2016-08-22 11:41 ` Andrew Yourtchenko
0 siblings, 1 reply; 12+ messages in thread
From: Eric Dumazet @ 2016-08-19 20:26 UTC (permalink / raw)
To: Andrew Yourtchenko; +Cc: netdev
On Fri, 2016-08-19 at 18:41 +0200, Andrew Yourtchenko wrote:
> This allows "ip -6 route save" to save the expiry for the routes
> that have it, such that it can be correctly restored later by
> "ip -6 route restore".
>
> If a route has RTF_EXPIRES set, generate RTA_EXPIRES value which
> will be used to restore the flag and expiry value by already
> existing code in rtm_to_fib6_config.
>
> The expiry was already being saved as part of RTA_CACHEINFO
> in rtnl_put_cacheinfo(), but adding code to generate RTF_EXPIRES upon save
> looked more appropriate than redundant cherrypicking from
> RTA_CACHEINFO upon restore.
>
> Signed-off-by: Andrew Yourtchenko <ayourtch@gmail.com>
> ---
> Changes since v1 [1]:
> * fixed the indentation in a multiline function call
> as per David Miller's review
>
> [1] v1: http://marc.info/?l=linux-netdev&m=147135597422280&w=2
>
> net/ipv6/route.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 4981755..f5b987d 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -3244,6 +3244,14 @@ static int rt6_fill_node(struct net *net,
> if (rtnl_put_cacheinfo(skb, &rt->dst, 0, expires, rt->dst.error) < 0)
> goto nla_put_failure;
>
> + /* Can't rely on expires == 0. It is zero if no expires flag,
> + * or if the timing is precisely at expiry. So, recheck the flag.
> + */
> + if (rt->rt6i_flags & RTF_EXPIRES)
> + if (nla_put_u32(skb, RTA_EXPIRES,
> + expires > 0 ? expires / HZ : 0))
> + goto nla_put_failure;
> +
Why sending (kernel -> user) the value in second units, while the other
direction (user -> kernel) expects hz ?
iproute2$ git grep -n RTA_EXPIRES
include/linux/rtnetlink.h:389: RTA_EXPIRES,
ip/iproute.c:930: addattr32(&req.n, sizeof(req), RTA_EXPIRES, expires*hz);
kernel patch : (32bc201e1974976b7d3fea9a9b17bb7392ca6394)
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -2809,6 +2810,15 @@ static int rtm_to_fib6_config(struct sk_buff *skb, struct nlmsghdr *nlh,
if (tb[RTA_ENCAP_TYPE])
cfg->fc_encap_type = nla_get_u16(tb[RTA_ENCAP_TYPE]);
+ if (tb[RTA_EXPIRES]) {
+ unsigned long timeout = addrconf_timeout_fixup(nla_get_u32(tb[RTA_EXPIRES]), HZ);
+
+ if (addrconf_finite_timeout(timeout)) {
+ cfg->fc_expires = jiffies_to_clock_t(timeout * HZ);
+ cfg->fc_flags |= RTF_EXPIRES;
+ }
+ }
+
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/2] ipv6: fixup RTF_* flags when restoring RTPROT_RA route from rtnetlink
2016-08-19 18:07 ` Sergei Shtylyov
@ 2016-08-22 11:04 ` Andrew Yourtchenko
2016-08-22 11:06 ` Sergei Shtylyov
0 siblings, 1 reply; 12+ messages in thread
From: Andrew Yourtchenko @ 2016-08-22 11:04 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: netdev
Hello,
Thanks for the review and feedback, one small clarification below.
On Fri, 19 Aug 2016, Sergei Shtylyov wrote:
> Hello.
>
> On 08/19/2016 07:41 PM, Andrew Yourtchenko wrote:
>
> > Fix the flags for RA-derived routes that were saved
> > via "ip -6 route save" and and subsequently restored via
> > "ip -6 route restore", allowing the incoming router advertisements
> > to update them, rather than complain about inability to do so.
> >
> > Upon the restore of RA-derived saved routes, set the RTF_ADDRCONF
> > to indicate that the source of the route was originally
> > a router advertisement, and set the RTF_DEFAULT or RTF_ROUTEINFO
> > flag depending on prefix length. This can be considered a
> > sister change of f0396f60d7c165018c9b203fb9b89fb224835578, in
>
> It's enough to specify 12 digits but you also need to specify the commit
> summary enclosed in ("").
Is just the first line of "ipv6: fix RTPROT_RA markup of RA routes
w/nexthops" enough, or do I need to include the additional two
paragraphs that follow ? (and can I keep the full hash rather than
truncate down to 12 digits?)
>
> > the other direction.
> >
> > Signed-off-by: Andrew Yourtchenko <ayourtch@gmail.com>
> > ---
> > Changes since v1 [1]:
> > * fixed the indentation of the basic blocks to be always a full TAB
> > as per David Miller's review
> >
> > [1] v1: http://marc.info/?l=linux-netdev&m=147135599322285&w=2
> >
> > net/ipv6/route.c | 10 ++++++++++
> > 1 file changed, 10 insertions(+)
> >
> > diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> > index f5b987d..60d95cd 100644
> > --- a/net/ipv6/route.c
> > +++ b/net/ipv6/route.c
> > @@ -2769,6 +2769,16 @@ static int rtm_to_fib6_config(struct sk_buff *skb,
> > struct nlmsghdr *nlh,
> > cfg->fc_protocol = rtm->rtm_protocol;
> > cfg->fc_type = rtm->rtm_type;
> >
> > + if (rtm->rtm_protocol == RTPROT_RA) {
> > + /* RA-derived route: set flags accordingly. */
> > + cfg->fc_flags |= RTF_ADDRCONF;
> > + if (rtm->rtm_dst_len == 0) {
> > + cfg->fc_flags |= RTF_DEFAULT;
> > + } else {
> > + cfg->fc_flags |= RTF_ROUTEINFO;
> > + }
>
> {} not needed here and above.
Will fix in v3, thanks!
--a
>
> [...]
>
> MBR, Sergei
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/2] ipv6: fixup RTF_* flags when restoring RTPROT_RA route from rtnetlink
2016-08-22 11:04 ` Andrew Yourtchenko
@ 2016-08-22 11:06 ` Sergei Shtylyov
0 siblings, 0 replies; 12+ messages in thread
From: Sergei Shtylyov @ 2016-08-22 11:06 UTC (permalink / raw)
To: Andrew Yourtchenko; +Cc: netdev
Hello.
On 8/22/2016 2:04 PM, Andrew Yourtchenko wrote:
>>> Fix the flags for RA-derived routes that were saved
>>> via "ip -6 route save" and and subsequently restored via
>>> "ip -6 route restore", allowing the incoming router advertisements
>>> to update them, rather than complain about inability to do so.
>>>
>>> Upon the restore of RA-derived saved routes, set the RTF_ADDRCONF
>>> to indicate that the source of the route was originally
>>> a router advertisement, and set the RTF_DEFAULT or RTF_ROUTEINFO
>>> flag depending on prefix length. This can be considered a
>>> sister change of f0396f60d7c165018c9b203fb9b89fb224835578, in
>>
>> It's enough to specify 12 digits but you also need to specify the commit
>> summary enclosed in ("").
>
> Is just the first line of "ipv6: fix RTPROT_RA markup of RA routes
> w/nexthops" enough, or do I need to include the additional two
Summary means the patch subject (and the first line of the commit log).
> paragraphs that follow ? (and can I keep the full hash rather than
> truncate down to 12 digits?)
Yes.
>>> the other direction.
>>>
>>> Signed-off-by: Andrew Yourtchenko <ayourtch@gmail.com>
[...]
MBR, Sergei
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] ipv6: save route expiry in RTA_EXPIRES if RTF_EXPIRES set
2016-08-19 20:26 ` Eric Dumazet
@ 2016-08-22 11:41 ` Andrew Yourtchenko
0 siblings, 0 replies; 12+ messages in thread
From: Andrew Yourtchenko @ 2016-08-22 11:41 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
On Fri, 19 Aug 2016, Eric Dumazet wrote:
> On Fri, 2016-08-19 at 18:41 +0200, Andrew Yourtchenko wrote:
> > This allows "ip -6 route save" to save the expiry for the routes
> > that have it, such that it can be correctly restored later by
> > "ip -6 route restore".
> >
> > If a route has RTF_EXPIRES set, generate RTA_EXPIRES value which
> > will be used to restore the flag and expiry value by already
> > existing code in rtm_to_fib6_config.
> >
> > The expiry was already being saved as part of RTA_CACHEINFO
> > in rtnl_put_cacheinfo(), but adding code to generate RTF_EXPIRES upon save
> > looked more appropriate than redundant cherrypicking from
> > RTA_CACHEINFO upon restore.
> >
> > Signed-off-by: Andrew Yourtchenko <ayourtch@gmail.com>
> > ---
> > Changes since v1 [1]:
> > * fixed the indentation in a multiline function call
> > as per David Miller's review
> >
> > [1] v1: http://marc.info/?l=linux-netdev&m=147135597422280&w=2
> >
> > net/ipv6/route.c | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> > index 4981755..f5b987d 100644
> > --- a/net/ipv6/route.c
> > +++ b/net/ipv6/route.c
> > @@ -3244,6 +3244,14 @@ static int rt6_fill_node(struct net *net,
> > if (rtnl_put_cacheinfo(skb, &rt->dst, 0, expires, rt->dst.error) < 0)
> > goto nla_put_failure;
> >
> > + /* Can't rely on expires == 0. It is zero if no expires flag,
> > + * or if the timing is precisely at expiry. So, recheck the flag.
> > + */
> > + if (rt->rt6i_flags & RTF_EXPIRES)
> > + if (nla_put_u32(skb, RTA_EXPIRES,
> > + expires > 0 ? expires / HZ : 0))
> > + goto nla_put_failure;
> > +
>
> Why sending (kernel -> user) the value in second units, while the other
> direction (user -> kernel) expects hz ?
No, the userland does expect seconds, see below:
>
> iproute2$ git grep -n RTA_EXPIRES
> include/linux/rtnetlink.h:389: RTA_EXPIRES,
> ip/iproute.c:930: addattr32(&req.n, sizeof(req), RTA_EXPIRES, expires*hz);
>
Fixed by commit eecc006952d6f3992b632974d0f04f995d2a176e
Author: Andrew Vagin <avagin@virtuozzo.com>
Date: Wed Jun 29 02:27:14 2016 +0300
ip route: timeout for routes has to be set in seconds
Currently a timeout is multiplied by HZ in user-space and
then it multiplied by HZ in kernel-space.
> kernel patch : (32bc201e1974976b7d3fea9a9b17bb7392ca6394)
>
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
>
> @@ -2809,6 +2810,15 @@ static int rtm_to_fib6_config(struct sk_buff *skb, struct nlmsghdr *nlh,
> if (tb[RTA_ENCAP_TYPE])
> cfg->fc_encap_type = nla_get_u16(tb[RTA_ENCAP_TYPE]);
>
> + if (tb[RTA_EXPIRES]) {
> + unsigned long timeout = addrconf_timeout_fixup(nla_get_u32(tb[RTA_EXPIRES]), HZ);
> +
> + if (addrconf_finite_timeout(timeout)) {
> + cfg->fc_expires = jiffies_to_clock_t(timeout * HZ);
> + cfg->fc_flags |= RTF_EXPIRES;
> + }
> + }
> +
>
Which is exactly the bug this snippet would trigger together with the
older git grep that you have enclosed - the resulting value from user
space assigned to fc_expires would have been:
jiffies_to_clock_t(timeout * HZ * hz).
So, the userland needs value in seconds. I suppose a good idea might be to
change my comment within the commit into something like:
/* Can't rely on expires == 0. It is zero if no expires flag,
* or if the timing is precisely at expiry. So, recheck the flag.
* The division by HZ is to obtain the value in seconds, which
* is needed by the userland, see commit
* eecc006952d6f3992b632974d0f04f995d2a176e
* "ip route: timeout for routes has to be set in seconds"
*/
What do you think ?
--a
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2016-08-22 11:41 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-08-16 13:59 [PATCH 0/2] ipv6: fix stuck RA-derived route in container after migration with criu Andrew Yourtchenko
2016-08-16 13:59 ` [PATCH 1/2] ipv6: save route expiry in RTA_EXPIRES if RTF_EXPIRES set Andrew Yourtchenko
2016-08-16 13:59 ` [PATCH 2/2] ipv6: fixup RTF_* flags when restoring RTPROT_RA route from rtnetlink Andrew Yourtchenko
2016-08-19 4:09 ` [PATCH 0/2] ipv6: fix stuck RA-derived route in container after migration with criu David Miller
2016-08-19 16:41 ` [PATCH v2 " Andrew Yourtchenko
2016-08-19 16:41 ` [PATCH v2 1/2] ipv6: save route expiry in RTA_EXPIRES if RTF_EXPIRES set Andrew Yourtchenko
2016-08-19 20:26 ` Eric Dumazet
2016-08-22 11:41 ` Andrew Yourtchenko
2016-08-19 16:41 ` [PATCH v2 2/2] ipv6: fixup RTF_* flags when restoring RTPROT_RA route from rtnetlink Andrew Yourtchenko
2016-08-19 18:07 ` Sergei Shtylyov
2016-08-22 11:04 ` Andrew Yourtchenko
2016-08-22 11:06 ` Sergei Shtylyov
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).