[PATCHv3 net-next] ipv6: allow routes to be configured with expire values

[PATCHv3 net-next] ipv6: allow routes to be configured with expire values

[Xin Long]

Add the support for adding expire value to routes,  requested by
Tom Gundersen <teg@jklm.no> for systemd-networkd, and NetworkManager
wants it too.

implement it by adding the new RTNETLINK attribute RTA_EXPIRES.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 include/uapi/linux/rtnetlink.h |  1 +
 net/ipv6/route.c               | 10 ++++++++++
 2 files changed, 11 insertions(+)

diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
index 123a5af..ca764b5 100644
--- a/include/uapi/linux/rtnetlink.h
+++ b/include/uapi/linux/rtnetlink.h
@@ -311,6 +311,7 @@ enum rtattr_type_t {
 	RTA_PREF,
 	RTA_ENCAP_TYPE,
 	RTA_ENCAP,
+	RTA_EXPIRES,
 	__RTA_MAX
 };
 
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index c83b6a5..3c8834b 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -2709,6 +2709,7 @@ static const struct nla_policy rtm_ipv6_policy[RTA_MAX+1] = {
 	[RTA_PREF]              = { .type = NLA_U8 },
 	[RTA_ENCAP_TYPE]	= { .type = NLA_U16 },
 	[RTA_ENCAP]		= { .type = NLA_NESTED },
+	[RTA_EXPIRES]		= { .type = NLA_U32 },
 };
 
 static int rtm_to_fib6_config(struct sk_buff *skb, struct nlmsghdr *nlh,
@@ -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;
+		}
+	}
+
 	err = 0;
 errout:
 	return err;
-- 
2.1.0

Re: [PATCHv3 net-next] ipv6: allow routes to be configured with expire values

[Hannes Frederic Sowa]

On 16.12.2015 10:50, Xin Long wrote:
> Add the support for adding expire value to routes,  requested by
> Tom Gundersen <teg@jklm.no> for systemd-networkd, and NetworkManager
> wants it too.
> 
> implement it by adding the new RTNETLINK attribute RTA_EXPIRES.
> 
> Signed-off-by: Xin Long <lucien.xin@gmail.com>

Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>

Thanks, Xin Long!

Re: [PATCHv3 net-next] ipv6: allow routes to be configured with expire values

[Dan Williams]

On Wed, 2015-12-16 at 17:50 +0800, Xin Long wrote:
> Add the support for adding expire value to routes,  requested by
> Tom Gundersen <teg@jklm.no> for systemd-networkd, and NetworkManager
> wants it too.
> 
> implement it by adding the new RTNETLINK attribute RTA_EXPIRES.

Could you also add bits to send RTA_EXPIRES back to userspace in the
route dump in rt6_fill_node(), so that userspace can figure out when
RTA_EXPIRES is supported or not?

(obviously having it there isn't foolproof as if there are no routes on
the system yet userspace can't figure out support, but it's better than
nothing...)

Thanks!
Dan

> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---
>  include/uapi/linux/rtnetlink.h |  1 +
>  net/ipv6/route.c               | 10 ++++++++++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/include/uapi/linux/rtnetlink.h
> b/include/uapi/linux/rtnetlink.h
> index 123a5af..ca764b5 100644
> --- a/include/uapi/linux/rtnetlink.h
> +++ b/include/uapi/linux/rtnetlink.h
> @@ -311,6 +311,7 @@ enum rtattr_type_t {
>  	RTA_PREF,
>  	RTA_ENCAP_TYPE,
>  	RTA_ENCAP,
> +	RTA_EXPIRES,
>  	__RTA_MAX
>  };
>  
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index c83b6a5..3c8834b 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -2709,6 +2709,7 @@ static const struct nla_policy
> rtm_ipv6_policy[RTA_MAX+1] = {
>  	[RTA_PREF]              = { .type = NLA_U8 },
>  	[RTA_ENCAP_TYPE]	= { .type = NLA_U16 },
>  	[RTA_ENCAP]		= { .type = NLA_NESTED },
> +	[RTA_EXPIRES]		= { .type = NLA_U32 },
>  };
>  
>  static int rtm_to_fib6_config(struct sk_buff *skb, struct nlmsghdr
> *nlh,
> @@ -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;
> +		}
> +	}
> +
>  	err = 0;
>  errout:
>  	return err;

Re: [PATCHv3 net-next] ipv6: allow routes to be configured with expire values

[David Miller]

From: Dan Williams <dcbw@redhat.com>
Date: Wed, 16 Dec 2015 11:03:52 -0600

> On Wed, 2015-12-16 at 17:50 +0800, Xin Long wrote:
>> Add the support for adding expire value to routes,  requested by
>> Tom Gundersen <teg@jklm.no> for systemd-networkd, and NetworkManager
>> wants it too.
>> 
>> implement it by adding the new RTNETLINK attribute RTA_EXPIRES.
> 
> Could you also add bits to send RTA_EXPIRES back to userspace in the
> route dump in rt6_fill_node(), so that userspace can figure out when
> RTA_EXPIRES is supported or not?
> 
> (obviously having it there isn't foolproof as if there are no routes on
> the system yet userspace can't figure out support, but it's better than
> nothing...)

That brings up an interesting issue, and I do not agree that we should
publish the value for the purpose of determining if the kernel supports
it or not.

We need to come up with a policy for handling unknown attributes
because what we do now doesn't work.

I'm almost positive that the right thing to do is to unilaterally
making nlmsg_parse() error out on out-of-range attribute type numbers,
and then backport that to all -stable branches.

Re: [PATCHv3 net-next] ipv6: allow routes to be configured with expire values

[Dan Williams]

On Thu, 2015-12-17 at 15:08 -0500, David Miller wrote:
> From: Dan Williams <dcbw@redhat.com>
> Date: Wed, 16 Dec 2015 11:03:52 -0600
> 
> > On Wed, 2015-12-16 at 17:50 +0800, Xin Long wrote:
> >> Add the support for adding expire value to routes,  requested by
> >> Tom Gundersen <teg@jklm.no> for systemd-networkd, and
> NetworkManager
> >> wants it too.
> >> 
> >> implement it by adding the new RTNETLINK attribute RTA_EXPIRES.
> > 
> > Could you also add bits to send RTA_EXPIRES back to userspace in
> the
> > route dump in rt6_fill_node(), so that userspace can figure out
> when
> > RTA_EXPIRES is supported or not?
> > 
> > (obviously having it there isn't foolproof as if there are no
> routes on
> > the system yet userspace can't figure out support, but it's better
> than
> > nothing...)
> 
> That brings up an interesting issue, and I do not agree that we
> should
> publish the value for the purpose of determining if the kernel
> supports
> it or not.

That said, userspace still needs to read back the EXPIRES attribute, if
only for iproute.  The program setting RTA_EXPIRES isn't the only thing
that wants to know about the route's details.

> We need to come up with a policy for handling unknown attributes
> because what we do now doesn't work.

Definitely agree.

> I'm almost positive that the right thing to do is to unilaterally
> making nlmsg_parse() error out on out-of-range attribute type
> numbers,
> and then backport that to all -stable branches.

This works for one attribute because then userspace gets an error like
EOPNOTSUPP or something.  But which attribute caused it?  Does
userspace then have to retry the operation a couple times with all the
different combinations of potentially unsupported options?

If we're going to error out on unrecognized options, I'd really like to
see some kind of netlink features bitmap or something that positively
indicates which options the kernel will accept.

Dan

Re: [PATCHv3 net-next] ipv6: allow routes to be configured with expire values

[Hannes Frederic Sowa]

On 17.12.2015 21:23, Dan Williams wrote:
> On Thu, 2015-12-17 at 15:08 -0500, David Miller wrote:
>> From: Dan Williams <dcbw@redhat.com>
>> Date: Wed, 16 Dec 2015 11:03:52 -0600
>>
>>> On Wed, 2015-12-16 at 17:50 +0800, Xin Long wrote:
>>>> Add the support for adding expire value to routes,  requested by
>>>> Tom Gundersen <teg@jklm.no> for systemd-networkd, and
>> NetworkManager
>>>> wants it too.
>>>>
>>>> implement it by adding the new RTNETLINK attribute RTA_EXPIRES.
>>>
>>> Could you also add bits to send RTA_EXPIRES back to userspace in
>> the
>>> route dump in rt6_fill_node(), so that userspace can figure out
>> when
>>> RTA_EXPIRES is supported or not?
>>>
>>> (obviously having it there isn't foolproof as if there are no
>> routes on
>>> the system yet userspace can't figure out support, but it's better
>> than
>>> nothing...)
>>
>> That brings up an interesting issue, and I do not agree that we
>> should
>> publish the value for the purpose of determining if the kernel
>> supports
>> it or not.
> 
> That said, userspace still needs to read back the EXPIRES attribute, if
> only for iproute.  The program setting RTA_EXPIRES isn't the only thing
> that wants to know about the route's details.
>
>> We need to come up with a policy for handling unknown attributes
>> because what we do now doesn't work.
> 
> Definitely agree.
> 
>> I'm almost positive that the right thing to do is to unilaterally
>> making nlmsg_parse() error out on out-of-range attribute type
>> numbers,
>> and then backport that to all -stable branches.
> 
> This works for one attribute because then userspace gets an error like
> EOPNOTSUPP or something.  But which attribute caused it?  Does
> userspace then have to retry the operation a couple times with all the
> different combinations of potentially unsupported options?
> 
> If we're going to error out on unrecognized options, I'd really like to
> see some kind of netlink features bitmap or something that positively
> indicates which options the kernel will accept.

Based on your mail I started to look if we can simply publish the
nla_policy maps to user space, which get fed to nlmsg_parse. I am
working on a rtnl_annotate function which adds this information along
with a new netlink flag NLM_F_DUMP_POLICY to query those.

Right now I am struggeling with nested attributes and if it is safe to
move NLA_UNSPEC to the value 1 so we can determine if a specific
attribute is set in the policy or not...

Also nested attributes seem to be quite hairy, maybe there is no reason
to inform user space about them, I don't yet know.

This infrastructure should be safe to use also when features get backported.

Bye,
Hannes

Re: [PATCHv3 net-next] ipv6: allow routes to be configured with expire values

[David Miller]

From: Dan Williams <dcbw@redhat.com>
Date: Thu, 17 Dec 2015 14:23:57 -0600

> On Thu, 2015-12-17 at 15:08 -0500, David Miller wrote:
>> That brings up an interesting issue, and I do not agree that we
>> should
>> publish the value for the purpose of determining if the kernel
>> supports
>> it or not.
> 
> That said, userspace still needs to read back the EXPIRES attribute, if
> only for iproute.  The program setting RTA_EXPIRES isn't the only thing
> that wants to know about the route's details.

Agreed.

>> I'm almost positive that the right thing to do is to unilaterally
>> making nlmsg_parse() error out on out-of-range attribute type
>> numbers,
>> and then backport that to all -stable branches.
> 
> This works for one attribute because then userspace gets an error like
> EOPNOTSUPP or something.  But which attribute caused it?  Does
> userspace then have to retry the operation a couple times with all the
> different combinations of potentially unsupported options?
> 
> If we're going to error out on unrecognized options, I'd really like to
> see some kind of netlink features bitmap or something that positively
> indicates which options the kernel will accept.

Also agree.  But it has to be really simple and trivial so that -stable
backports are possible.

Re: [PATCHv3 net-next] ipv6: allow routes to be configured with expire values

[David Miller]

From: Xin Long <lucien.xin@gmail.com>
Date: Wed, 16 Dec 2015 17:50:11 +0800

> Add the support for adding expire value to routes,  requested by
> Tom Gundersen <teg@jklm.no> for systemd-networkd, and NetworkManager
> wants it too.
> 
> implement it by adding the new RTNETLINK attribute RTA_EXPIRES.
> 
> Signed-off-by: Xin Long <lucien.xin@gmail.com>

Applied, thanks.