| Message ID | 1489077180-12946-1-git-send-email-dedeckeh@gmail.com |
|---|---|
| State | Rejected |
| Headers | show |
On 03/09/2017 05:32 PM, Hans Dedecker wrote: > Trigger interface update event when IPv6 address lifetime changes by setting > the address indicator flag to inform external subsystems about IPv6 address > lifetime change. > > Signed-off-by: Hans Dedecker <dedeckeh@gmail.com> AFAICT, this will cause a lot of ifupdate events in big IPv6 networks (for example, some large mesh networks based on Gluon have more than 4000 client and 5-10 radvds, often causing more than one RA per second, each updating valid/preferred lifetimes). We *really* want to avoid causing lots of reloads for services that set triggers on interface updates; the majority of services is not interested in the lifetimes of addresses at all. Matthias > --- > interface-ip.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/interface-ip.c b/interface-ip.c > index ddca5d2..366f69a 100644 > --- a/interface-ip.c > +++ b/interface-ip.c > @@ -563,8 +563,10 @@ interface_update_proto_addr(struct vlist_tree *tree, > keep = false; > > if (a_old->valid_until != a_new->valid_until || > - a_old->preferred_until != a_new->preferred_until) > + a_old->preferred_until != a_new->preferred_until) { > + iface->updated |= IUF_ADDRESS; > replace = true; > + } > > if ((a_new->flags & DEVADDR_FAMILY) == DEVADDR_INET4 && > a_new->broadcast != a_old->broadcast) >
On Thu, Mar 9, 2017 at 10:20 PM, Matthias Schiffer <mschiffer@universe-factory.net> wrote: > On 03/09/2017 05:32 PM, Hans Dedecker wrote: >> Trigger interface update event when IPv6 address lifetime changes by setting >> the address indicator flag to inform external subsystems about IPv6 address >> lifetime change. >> >> Signed-off-by: Hans Dedecker <dedeckeh@gmail.com> > > AFAICT, this will cause a lot of ifupdate events in big IPv6 networks (for > example, some large mesh networks based on Gluon have more than 4000 client > and 5-10 radvds, often causing more than one RA per second, each updating > valid/preferred lifetimes). We *really* want to avoid causing lots of > reloads for services that set triggers on interface updates; the majority > of services is not interested in the lifetimes of addresses at all. The netifd patch set is a result of a reported hnet issue (https://github.com/openwrt/openwrt/issues/346). The problem is triggered by netifd commit https://git.lede-project.org/?p=project/netifd.git;a=commit;h=b8ef742bd04ebef324ae11aee56c6e1d2cb7e0ad; before this commit an interface update event was always triggered independant from the interface updated flag and thus indeed causing lots of service reloads that set triggers on interface updates. Now it seems by restricting the interface update events hnet package is broken as the lifetime of the IPv6 addresses is not refreshed anymore. After doing code review in hnet package I noticed it relies on interface update notifications; hnet certainly picks up the delegated prefix via interface update notifications I'm not 100% sure about IPv6 addresses as I'm not a hnet package expert nor do I have a setup on which I can test it. I'm fine leaving this patch out but leaving the other netifd patch (http://lists.infradead.org/pipermail/lede-dev/2017-March/006605.html) out will certainly keep the hnet package broken. Hans > > Matthias > > >> --- >> interface-ip.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/interface-ip.c b/interface-ip.c >> index ddca5d2..366f69a 100644 >> --- a/interface-ip.c >> +++ b/interface-ip.c >> @@ -563,8 +563,10 @@ interface_update_proto_addr(struct vlist_tree *tree, >> keep = false; >> >> if (a_old->valid_until != a_new->valid_until || >> - a_old->preferred_until != a_new->preferred_until) >> + a_old->preferred_until != a_new->preferred_until) { >> + iface->updated |= IUF_ADDRESS; >> replace = true; >> + } >> >> if ((a_new->flags & DEVADDR_FAMILY) == DEVADDR_INET4 && >> a_new->broadcast != a_old->broadcast) >> > >
On 03/09/2017 11:03 PM, Hans Dedecker wrote: > On Thu, Mar 9, 2017 at 10:20 PM, Matthias Schiffer > <mschiffer@universe-factory.net> wrote: >> On 03/09/2017 05:32 PM, Hans Dedecker wrote: >>> Trigger interface update event when IPv6 address lifetime changes by setting >>> the address indicator flag to inform external subsystems about IPv6 address >>> lifetime change. >>> >>> Signed-off-by: Hans Dedecker <dedeckeh@gmail.com> >> >> AFAICT, this will cause a lot of ifupdate events in big IPv6 networks (for >> example, some large mesh networks based on Gluon have more than 4000 client >> and 5-10 radvds, often causing more than one RA per second, each updating >> valid/preferred lifetimes). We *really* want to avoid causing lots of >> reloads for services that set triggers on interface updates; the majority >> of services is not interested in the lifetimes of addresses at all. > The netifd patch set is a result of a reported hnet issue > (https://github.com/openwrt/openwrt/issues/346). > The problem is triggered by netifd commit > https://git.lede-project.org/?p=project/netifd.git;a=commit;h=b8ef742bd04ebef324ae11aee56c6e1d2cb7e0ad; > before this commit an interface update event was always triggered > independant from the interface updated flag and thus indeed causing > lots of service reloads that set triggers on interface updates. Now it > seems by restricting the interface update events hnet package is > broken as the lifetime of the IPv6 addresses is not refreshed anymore. > After doing code review in hnet package I noticed it relies on > interface update notifications; hnet certainly picks up the delegated > prefix via interface update notifications I'm not 100% sure about IPv6 > addresses as I'm not a hnet package expert nor do I have a setup on > which I can test it. I'm fine leaving this patch out but leaving the > other netifd patch > (http://lists.infradead.org/pipermail/lede-dev/2017-March/006605.html) > out will certainly keep the hnet package broken. > > Hans >> Hmm, what exactly are these prefix objects in netifd? Is this only used for prefix delegation via DHCPv6? If it is, the second patch should not trigger updates for each RA, right? If that's the case, I don't see an issue with it. Matthias
On Fri, Mar 10, 2017 at 12:08 AM, Matthias Schiffer <mschiffer@universe-factory.net> wrote: > On 03/09/2017 11:03 PM, Hans Dedecker wrote: >> On Thu, Mar 9, 2017 at 10:20 PM, Matthias Schiffer >> <mschiffer@universe-factory.net> wrote: >>> On 03/09/2017 05:32 PM, Hans Dedecker wrote: >>>> Trigger interface update event when IPv6 address lifetime changes by setting >>>> the address indicator flag to inform external subsystems about IPv6 address >>>> lifetime change. >>>> >>>> Signed-off-by: Hans Dedecker <dedeckeh@gmail.com> >>> >>> AFAICT, this will cause a lot of ifupdate events in big IPv6 networks (for >>> example, some large mesh networks based on Gluon have more than 4000 client >>> and 5-10 radvds, often causing more than one RA per second, each updating >>> valid/preferred lifetimes). We *really* want to avoid causing lots of >>> reloads for services that set triggers on interface updates; the majority >>> of services is not interested in the lifetimes of addresses at all. >> The netifd patch set is a result of a reported hnet issue >> (https://github.com/openwrt/openwrt/issues/346). >> The problem is triggered by netifd commit >> https://git.lede-project.org/?p=project/netifd.git;a=commit;h=b8ef742bd04ebef324ae11aee56c6e1d2cb7e0ad; >> before this commit an interface update event was always triggered >> independant from the interface updated flag and thus indeed causing >> lots of service reloads that set triggers on interface updates. Now it >> seems by restricting the interface update events hnet package is >> broken as the lifetime of the IPv6 addresses is not refreshed anymore. >> After doing code review in hnet package I noticed it relies on >> interface update notifications; hnet certainly picks up the delegated >> prefix via interface update notifications I'm not 100% sure about IPv6 >> addresses as I'm not a hnet package expert nor do I have a setup on >> which I can test it. I'm fine leaving this patch out but leaving the >> other netifd patch >> (http://lists.infradead.org/pipermail/lede-dev/2017-March/006605.html) >> out will certainly keep the hnet package broken. >> >> Hans >>> > > Hmm, what exactly are these prefix objects in netifd? Is this only used for > prefix delegation via DHCPv6? If it is, the second patch should not trigger > updates for each RA, right? If that's the case, I don't see an issue with it. It's indeed used for prefix delegation via DHCPv6 thus it should not trigger an interface update for each RA Hans > > Matthias >
diff --git a/interface-ip.c b/interface-ip.c index ddca5d2..366f69a 100644 --- a/interface-ip.c +++ b/interface-ip.c @@ -563,8 +563,10 @@ interface_update_proto_addr(struct vlist_tree *tree, keep = false; if (a_old->valid_until != a_new->valid_until || - a_old->preferred_until != a_new->preferred_until) + a_old->preferred_until != a_new->preferred_until) { + iface->updated |= IUF_ADDRESS; replace = true; + } if ((a_new->flags & DEVADDR_FAMILY) == DEVADDR_INET4 && a_new->broadcast != a_old->broadcast)
Trigger interface update event when IPv6 address lifetime changes by setting the address indicator flag to inform external subsystems about IPv6 address lifetime change. Signed-off-by: Hans Dedecker <dedeckeh@gmail.com> --- interface-ip.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)