Re: [OpenWrt-Devel] [PATCH] [odhcp6c] ra: clear RA information and request new after link-up event or SIGUSR2

2019-03-20 Thread Pavel Merzlyakov
 >Can you share the network config you're using ?
I'm sorry I can't do this.

>I would prefer to use another signal to trigger the sending of RS.
Of course, it's up to you.
But I think there should be some signal witch restart both RA and DHCPv6.

Regarding to the patch
do you agree that we must clear STATE_RA_ on link-up event
before we trigger Router Solicitation message (see ra.c:247 ->
https://git.openwrt.org/?p=project/odhcp6c.git;a=blob;f=src/ra.c;h=898f449168e29bae08e48e9395f420d13b2ec899;hb=HEAD#l247
)?
Br,
Pavel

On Wed, 20 Mar 2019 at 10:52, Hans Dedecker  wrote:

> On Tue, Mar 19, 2019 at 1:15 PM Pavel Merzlyakov
>  wrote:
> >
> > >why is this not the case in your setup ?
> > Because sometimes odhcp6c miss restart, probably because of  Bridge Mode
> (it's attached to bridge interface).
> Can you share the network config you're using ?
>
> > Anyway, I use a hotplug script which sends SIGUSR2 signal to odhcp6c on
> link-up event.
> > Of course I could send SIGTERM, but I think it's not the proper way to
> solve problem.
> >
> > I thought that it is an obvious bug because I see a lack of consistency.
> > Why odhcp6c on SIGUSR2
> > clear STATE_IA_ and resend DHCPV6_MSG_
> > and don't do same with RA - clear STATE_RA_ and resend Router
> Solicitation message?
> This was the approach by the original author of the code to control
> only DHCPv6 via SIGUSR1 and SIGUSR2.
> I understand your need to trigger the sending of RS via a signal but
> I'm reluctant to change the current behavior of the SIGUSR2 signal as
> I would prefer to use another signal to trigger the sending of RS.
>
> Hans
> >
> > Br,
> > Pavel
> >
> > On Tue, 19 Mar 2019 at 12:36, Hans Dedecker  wrote:
> >>
> >> Hi,
> >>
> >> On Mon, Mar 18, 2019 at 8:51 PM Pavel Merzlyakov
> >>  wrote:
> >> >
> >> > Hi,
> >> >
> >> > >Can you be elaborate more in detail what use case you want to cover
> >> > >with this patch; in other words what is not working now ?
> >> > Ok,
> >> > my setup:
> >> > Two peer routers A and B which both connected to gateway C.
> >> > Routers A and B have public IPv6 addresses on WAN interfaces from
> same subnet (because of RA on C) and private IPv6 addresses on LAN
> interfaces.
> >> > and issue:
> >> > If I unplug router B from C and then plug it in LAN port of router A:
> >> > 1) Router B does not send Router Solicitation message. So router B
> does not have a valid gateway for some time.
> >> Since we've linksensing support in netifd the wan/wan6 interface
> >> should be brought down when the cable is unplugged; the wan/wan6
> >> interface will be brought up again when the cable is plugged in. This
> >> means odhcp6c will be restarted and will start sending RS; why is this
> >> not the case in your setup ?
> >>
> >> Hans
> >> > 2) Router B does not delete old IPv6 address on the WAN. After first
> Router Advertisement message router B is capable to send a request via new
> gateway A, but it can't receive responses because of invalid/old source
> address in request.
> >> >
> >> >
> >> > >Why do you want to restart RA if an SIGUSR2 signal is received ?
> >> > Because I assume that SIGUSR2 signal is equivalent to link-up event
> (see odhcp6c:674 ->
> https://git.openwrt.org/?p=project/odhcp6c.git;a=blob;f=src/odhcp6c.c;h=19a86f2654bf3c59b0f47cf0aedd87235187bf89;hb=d2e247d8d87ecf8c60fcf0acdad05667bd379521#l674
> ).
> >> > In my case I use hotplug script witch on link-up sends SIGUSR2 signal
> to odhcp6c (it's connected to bridge interface).
> >> > ra_restart () could be called without condition in beginning of the
> main loop.
> >> >
> >> > Br,
> >> > Pavel
> >> >
> >> > On Mon, 18 Mar 2019 at 18:26, Hans Dedecker 
> wrote:
> >> >>
> >> >> Hi,
> >> >>
> >> >> On Mon, Mar 18, 2019 at 2:43 PM  wrote:
> >> >> >
> >> >> > From: Pavel Merzlyakov 
> >> >> >
> >> >> > A subnet may be changed after link-up event
> >> >> Can you be elaborate more in detail what use case you want to cover
> >> >> with this patch; in other words what is not working now ?
> >> >> >
> >> >> > Signed-off-by: Pavel Merzlyakov 
> >> >> > ---
> >> >> >  src/odhcp6c.c |  3 +++
>

Re: [OpenWrt-Devel] [PATCH] [odhcp6c] ra: clear RA information and request new after link-up event or SIGUSR2

2019-03-19 Thread Pavel Merzlyakov
>why is this not the case in your setup ?
Because sometimes odhcp6c miss restart, probably because of  Bridge Mode
(it's attached to bridge interface).
Anyway, I use a hotplug script which sends SIGUSR2 signal to odhcp6c on
link-up event.
Of course I could send SIGTERM, but I think it's not the proper way to
solve problem.

I thought that it is an obvious bug because I see a lack of consistency.
Why odhcp6c on SIGUSR2
clear STATE_IA_ and resend DHCPV6_MSG_
and don't do same with RA - clear STATE_RA_ and resend Router Solicitation
message?

Br,
Pavel

On Tue, 19 Mar 2019 at 12:36, Hans Dedecker  wrote:

> Hi,
>
> On Mon, Mar 18, 2019 at 8:51 PM Pavel Merzlyakov
>  wrote:
> >
> > Hi,
> >
> > >Can you be elaborate more in detail what use case you want to cover
> > >with this patch; in other words what is not working now ?
> > Ok,
> > my setup:
> > Two peer routers A and B which both connected to gateway C.
> > Routers A and B have public IPv6 addresses on WAN interfaces from same
> subnet (because of RA on C) and private IPv6 addresses on LAN interfaces.
> > and issue:
> > If I unplug router B from C and then plug it in LAN port of router A:
> > 1) Router B does not send Router Solicitation message. So router B does
> not have a valid gateway for some time.
> Since we've linksensing support in netifd the wan/wan6 interface
> should be brought down when the cable is unplugged; the wan/wan6
> interface will be brought up again when the cable is plugged in. This
> means odhcp6c will be restarted and will start sending RS; why is this
> not the case in your setup ?
>
> Hans
> > 2) Router B does not delete old IPv6 address on the WAN. After first
> Router Advertisement message router B is capable to send a request via new
> gateway A, but it can't receive responses because of invalid/old source
> address in request.
> >
> >
> > >Why do you want to restart RA if an SIGUSR2 signal is received ?
> > Because I assume that SIGUSR2 signal is equivalent to link-up event
> (see odhcp6c:674 ->
> https://git.openwrt.org/?p=project/odhcp6c.git;a=blob;f=src/odhcp6c.c;h=19a86f2654bf3c59b0f47cf0aedd87235187bf89;hb=d2e247d8d87ecf8c60fcf0acdad05667bd379521#l674
> ).
> > In my case I use hotplug script witch on link-up sends SIGUSR2 signal to
> odhcp6c (it's connected to bridge interface).
> > ra_restart () could be called without condition in beginning of the main
> loop.
> >
> > Br,
> > Pavel
> >
> > On Mon, 18 Mar 2019 at 18:26, Hans Dedecker  wrote:
> >>
> >> Hi,
> >>
> >> On Mon, Mar 18, 2019 at 2:43 PM  wrote:
> >> >
> >> > From: Pavel Merzlyakov 
> >> >
> >> > A subnet may be changed after link-up event
> >> Can you be elaborate more in detail what use case you want to cover
> >> with this patch; in other words what is not working now ?
> >> >
> >> > Signed-off-by: Pavel Merzlyakov 
> >> > ---
> >> >  src/odhcp6c.c |  3 +++
> >> >  src/ra.c  | 20 +---
> >> >  src/ra.h  |  1 +
> >> >  3 files changed, 21 insertions(+), 3 deletions(-)
> >> >
> >> > diff --git a/src/odhcp6c.c b/src/odhcp6c.c
> >> > index 19a86f2..dd20f39 100644
> >> > --- a/src/odhcp6c.c
> >> > +++ b/src/odhcp6c.c
> >> > @@ -455,6 +455,9 @@ int main(_unused int argc, char* const argv[])
> >> >
> >> > syslog(LOG_NOTICE, "(re)starting transaction on %s",
> ifname);
> >> >
> >> > +   if (signal_usr2)
> >> > +   ra_restart();
> >> Why do you want to restart RA if an SIGUSR2 signal is received ?
> >>
> >> Hans
> >> > +
> >> > signal_usr1 = signal_usr2 = false;
> >> > int mode = dhcpv6_set_ia_mode(ia_na_mode, ia_pd_mode);
> >> > if (mode != DHCPV6_STATELESS)
> >> > diff --git a/src/ra.c b/src/ra.c
> >> > index 898f449..917df11 100644
> >> > --- a/src/ra.c
> >> > +++ b/src/ra.c
> >> > @@ -55,6 +55,7 @@ static int sock = -1, rtnl = -1;
> >> >  static int if_index = 0;
> >> >  static char if_name[IF_NAMESIZE] = {0};
> >> >  static volatile int rs_attempt = 0;
> >> > +static const int rs_attempt_limit = 4;
> >> >  static struct in6_addr lladdr = IN6ADDR_ANY_INIT;
> >> >  static unsigned int ra_options = 0;
> >> >  static unsigned int ra_holdoff_interval = 0;
>

Re: [OpenWrt-Devel] [PATCH] [odhcp6c] ra: clear RA information and request new after link-up event or SIGUSR2

2019-03-18 Thread Pavel Merzlyakov
Hi,

>Can you be elaborate more in detail what use case you want to cover
>with this patch; in other words what is not working now ?
Ok,
my setup:
Two peer routers A and B which both connected to gateway C.
Routers A and B have public IPv6 addresses on WAN interfaces from same
subnet (because of RA on C) and private IPv6 addresses on LAN interfaces.
and issue:
If I unplug router B from C and then plug it in LAN port of router A:
1) Router B does not send Router Solicitation message. So router B does not
have a valid gateway for some time.
2) Router B does not delete old IPv6 address on the WAN. After first Router
Advertisement message router B is capable to send a request via new gateway
A, but it can't receive responses because of invalid/old source address in
request.


>Why do you want to restart RA if an SIGUSR2 signal is received ?
Because I assume that SIGUSR2 signal is equivalent to link-up event  (see
odhcp6c:674 ->
https://git.openwrt.org/?p=project/odhcp6c.git;a=blob;f=src/odhcp6c.c;h=19a86f2654bf3c59b0f47cf0aedd87235187bf89;hb=d2e247d8d87ecf8c60fcf0acdad05667bd379521#l674
).
In my case I use hotplug script witch on link-up sends SIGUSR2 signal to
odhcp6c (it's connected to bridge interface).
ra_restart () could be called without condition in beginning of the main
loop.

Br,
Pavel

On Mon, 18 Mar 2019 at 18:26, Hans Dedecker  wrote:

> Hi,
>
> On Mon, Mar 18, 2019 at 2:43 PM  wrote:
> >
> > From: Pavel Merzlyakov 
> >
> > A subnet may be changed after link-up event
> Can you be elaborate more in detail what use case you want to cover
> with this patch; in other words what is not working now ?
> >
> > Signed-off-by: Pavel Merzlyakov 
> > ---
> >  src/odhcp6c.c |  3 +++
> >  src/ra.c  | 20 +---
> >  src/ra.h  |  1 +
> >  3 files changed, 21 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/odhcp6c.c b/src/odhcp6c.c
> > index 19a86f2..dd20f39 100644
> > --- a/src/odhcp6c.c
> > +++ b/src/odhcp6c.c
> > @@ -455,6 +455,9 @@ int main(_unused int argc, char* const argv[])
> >
> > syslog(LOG_NOTICE, "(re)starting transaction on %s",
> ifname);
> >
> > +   if (signal_usr2)
> > +   ra_restart();
> Why do you want to restart RA if an SIGUSR2 signal is received ?
>
> Hans
> > +
> > signal_usr1 = signal_usr2 = false;
> > int mode = dhcpv6_set_ia_mode(ia_na_mode, ia_pd_mode);
> > if (mode != DHCPV6_STATELESS)
> > diff --git a/src/ra.c b/src/ra.c
> > index 898f449..917df11 100644
> > --- a/src/ra.c
> > +++ b/src/ra.c
> > @@ -55,6 +55,7 @@ static int sock = -1, rtnl = -1;
> >  static int if_index = 0;
> >  static char if_name[IF_NAMESIZE] = {0};
> >  static volatile int rs_attempt = 0;
> > +static const int rs_attempt_limit = 4;
> >  static struct in6_addr lladdr = IN6ADDR_ANY_INIT;
> >  static unsigned int ra_options = 0;
> >  static unsigned int ra_holdoff_interval = 0;
> > @@ -179,6 +180,20 @@ failure:
> > return -1;
> >  }
> >
> > +void ra_restart(void)
> > +{
> > +   const int rs_attempt_old = rs_attempt;
> > +
> > +   odhcp6c_clear_state(STATE_RA_PREFIX);
> > +   odhcp6c_clear_state(STATE_RA_ROUTE);
> > +   odhcp6c_clear_state(STATE_RA_DNS);
> > +   odhcp6c_clear_state(STATE_RA_SEARCH);
> > +
> > +   rs_attempt = 0;
> > +   if (rs_attempt_old == 0 || rs_attempt_old >= rs_attempt_limit)
> > +   ra_send_rs(SIGALRM);
> > +}
> > +
> >  static void ra_send_rs(int signal __attribute__((unused)))
> >  {
> > const struct sockaddr_in6 dest = {AF_INET6, 0, 0,
> ALL_IPV6_ROUTERS, if_index};
> > @@ -193,7 +208,7 @@ static void ra_send_rs(int signal
> __attribute__((unused)))
> > if (sendto(sock, &rs, len, MSG_DONTWAIT, (struct
> sockaddr*)&dest, sizeof(dest)) < 0)
> > syslog(LOG_ERR, "Failed to send RS (%s)",
> strerror(errno));
> >
> > -   if (++rs_attempt <= 3)
> > +   if (++rs_attempt < rs_attempt_limit)
> > alarm(4);
> >  }
> >
> > @@ -243,8 +258,7 @@ bool ra_link_up(void)
> > if (ret) {
> > syslog(LOG_NOTICE, "carrier => %i event on %s",
> (int)!nocarrier, if_name);
> >
> > -   rs_attempt = 0;
> > -   ra_send_rs(SIGALRM);
> > +   ra_restart();
> > }
> >
> > return ret;
> > diff --git a/src/ra.h b/src/ra.h
> > index 9acc8cd..

[OpenWrt-Devel] [PATCH] [odhcp6c] ra: clear RA information and request new after link-up event or SIGUSR2

2019-03-18 Thread pavel . merzlyakov
From: Pavel Merzlyakov 

A subnet may be changed after link-up event

Signed-off-by: Pavel Merzlyakov 
---
 src/odhcp6c.c |  3 +++
 src/ra.c  | 20 +---
 src/ra.h  |  1 +
 3 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/src/odhcp6c.c b/src/odhcp6c.c
index 19a86f2..dd20f39 100644
--- a/src/odhcp6c.c
+++ b/src/odhcp6c.c
@@ -455,6 +455,9 @@ int main(_unused int argc, char* const argv[])
 
syslog(LOG_NOTICE, "(re)starting transaction on %s", ifname);
 
+   if (signal_usr2)
+   ra_restart();
+
signal_usr1 = signal_usr2 = false;
int mode = dhcpv6_set_ia_mode(ia_na_mode, ia_pd_mode);
if (mode != DHCPV6_STATELESS)
diff --git a/src/ra.c b/src/ra.c
index 898f449..917df11 100644
--- a/src/ra.c
+++ b/src/ra.c
@@ -55,6 +55,7 @@ static int sock = -1, rtnl = -1;
 static int if_index = 0;
 static char if_name[IF_NAMESIZE] = {0};
 static volatile int rs_attempt = 0;
+static const int rs_attempt_limit = 4;
 static struct in6_addr lladdr = IN6ADDR_ANY_INIT;
 static unsigned int ra_options = 0;
 static unsigned int ra_holdoff_interval = 0;
@@ -179,6 +180,20 @@ failure:
return -1;
 }
 
+void ra_restart(void)
+{
+   const int rs_attempt_old = rs_attempt;
+
+   odhcp6c_clear_state(STATE_RA_PREFIX);
+   odhcp6c_clear_state(STATE_RA_ROUTE);
+   odhcp6c_clear_state(STATE_RA_DNS);
+   odhcp6c_clear_state(STATE_RA_SEARCH);
+
+   rs_attempt = 0;
+   if (rs_attempt_old == 0 || rs_attempt_old >= rs_attempt_limit)
+   ra_send_rs(SIGALRM);
+}
+
 static void ra_send_rs(int signal __attribute__((unused)))
 {
const struct sockaddr_in6 dest = {AF_INET6, 0, 0, ALL_IPV6_ROUTERS, 
if_index};
@@ -193,7 +208,7 @@ static void ra_send_rs(int signal __attribute__((unused)))
if (sendto(sock, &rs, len, MSG_DONTWAIT, (struct sockaddr*)&dest, 
sizeof(dest)) < 0)
syslog(LOG_ERR, "Failed to send RS (%s)",  strerror(errno));
 
-   if (++rs_attempt <= 3)
+   if (++rs_attempt < rs_attempt_limit)
alarm(4);
 }
 
@@ -243,8 +258,7 @@ bool ra_link_up(void)
if (ret) {
syslog(LOG_NOTICE, "carrier => %i event on %s", 
(int)!nocarrier, if_name);
 
-   rs_attempt = 0;
-   ra_send_rs(SIGALRM);
+   ra_restart();
}
 
return ret;
diff --git a/src/ra.h b/src/ra.h
index 9acc8cd..4ec208f 100644
--- a/src/ra.h
+++ b/src/ra.h
@@ -46,5 +46,6 @@ struct icmpv6_opt_route_info {
 
 int ra_init(const char *ifname, const struct in6_addr *ifid,
unsigned int options, unsigned int holdoff_interval);
+void ra_restart(void);
 bool ra_link_up(void);
 bool ra_process(void);
-- 
2.21.0


___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


[OpenWrt-Devel] [PATCH] [ubox] kmodloader: fix and optimize loading of failed modules

2019-03-18 Thread pavel . merzlyakov
From: Pavel Merzlyakov 

1) Restore functionality which was lost in commit 876c7f5b.
   Again at boot time kmodloader can load all modules (/etc/modules.d/*)
   even if dependency information is completely missing.
   This functionality is important in case of hidden dependency (not symbol 
dependency).
   For example, in kernel 4.4.60 is hidden dependency between nf_nat_ipv6 and 
nf_conntrack_ipv6.
   We can't load nf_nat_ipv6 before nf_conntrack_ipv6 and modinfo do not show 
this dependency.
   Two sequential load attempts of nf_nat_ipv6 may not be enough (in my case 
it's definitely not enough).
   nf_nat_ipv4 has a similar problem.

2) Reduce count of attempts to load failed modules.
   Now kmodloader try to load failed modules after all others are loaded.

main_loader: Count of failed and successful attempts to load nf_nat_ipv6.ko 
(kernel 4.4.60) depend on ubox version:
   COMMITFAILED  SUCCESSFUL TOTAL
   128bc35f  53  1  54
   876c7f5b  2   0  2
   this  1   1  2

Signed-off-by: Pavel Merzlyakov 
---
 kmodloader.c | 39 ---
 1 file changed, 24 insertions(+), 15 deletions(-)

diff --git a/kmodloader.c b/kmodloader.c
index 3196deb..e0f4e6d 100644
--- a/kmodloader.c
+++ b/kmodloader.c
@@ -615,12 +615,13 @@ static void load_moddeps(struct module *_m)
}
 }
 
-static int iterations = 0;
-static int load_modprobe(void)
+static int load_modprobe(bool allow_load_retry)
 {
-   int loaded, todo;
+   int loaded, skipped, failed;
struct module_node *mn;
struct module *m;
+   bool load_retry = false;
+   static bool first_iteration = true;
 
avl_for_each_element(&modules, mn, avl) {
if (mn->is_alias)
@@ -632,12 +633,13 @@ static int load_modprobe(void)
 
do {
loaded = 0;
-   todo = 0;
+   skipped = 0;
+   failed = 0;
avl_for_each_element(&modules, mn, avl) {
if (mn->is_alias)
continue;
m = mn->m;
-   if ((m->state == PROBE) && (!deps_available(m, 0)) && 
m->error < 2) {
+   if ((m->state == PROBE) && (!deps_available(m, 0)) && 
(!m->error || load_retry)) {
if (!insert_module(get_module_path(m->name), 
(m->opts) ? (m->opts) : (""))) {
m->state = LOADED;
m->error = 0;
@@ -645,17 +647,24 @@ static int load_modprobe(void)
continue;
}
 
-   if (++m->error > 1)
-   ULOG_ERR("failed to load %s\n", 
m->name);
+   m->error = 1;
}
 
-   if ((m->state == PROBE) || m->error)
-   todo++;
+   if (m->error)
+   failed++;
+   else if (m->state == PROBE)
+   skipped++;
}
-   iterations++;
-   } while (loaded);
 
-   return todo;
+   if (allow_load_retry) {
+   /* if we can't load anything else let's try to load 
failed modules */
+   load_retry = loaded ? (failed && !skipped) : (failed && 
!load_retry && !first_iteration);
+   }
+
+   first_iteration = false;
+   } while (loaded || load_retry);
+
+   return skipped + failed;
 }
 
 static int print_insmod_usage(void)
@@ -884,7 +893,7 @@ static int main_modprobe(int argc, char **argv)
 
m->state = PROBE;
 
-   fail = load_modprobe();
+   fail = load_modprobe(true);
 
if (fail) {
ULOG_ERR("%d module%s could not be probed\n",
@@ -972,14 +981,14 @@ static int main_loader(int argc, char **argv)
m->opts = strdup(opts);
m->state = PROBE;
if (basename(gl.gl_pathv[j])[0] - '0' <= 9)
-   load_modprobe();
+   load_modprobe(false);
 
}
free(mod);
fclose(fp);
}
 
-   fail = load_modprobe();
+   fail = load_modprobe(true);
 
if (fail) {
ULOG_ERR("%d module%s could not be probed\n",
-- 
2.21.0


___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel