| Message ID | 3b977653c7ade52e180d754eb660ec66af212e94.1450259411.git.lucien.xin@gmail.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
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! -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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; -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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;
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(+)