[patchv2] teamd: add a default value 1000 for link_watch.interval
diff mbox series

Message ID 20190305034156.7953-1-liuhangbin@gmail.com
State New
Headers show
Series
  • [patchv2] teamd: add a default value 1000 for link_watch.interval
Related show

Commit Message

Hangbin Liu March 5, 2019, 3:41 a.m. UTC
As we don't have a default value for link_watch.interval. If user
forgot to set this parameter, teamd will failed to init port priv and
exit at the end.

e.g.
teamd -g -c '{"runner":{"name":"activebackup"},"link_watch":{"name":"arp_ping","target_host":"198.51.100.1"}}'
teamdctl team0 port add p5p1
teamdctl team0 port add p5p2

teamd debug log shows:
p5p2: Got link watch from global config.
p5p2: Using sticky "0".
Failed to get "interval" link-watch option.
Failed to load options.
Failed to init port priv.
Callback named "lw_periodic" not found.
Callback named "lw_socket" not found.
Loop callback failed with: Invalid argument
Failed loop callback: libteam_events, 0x5624c28b9410
select() failed.
Exiting...
Removed loop callback: usock_acc_conn, 0x5624c28bab60
Removed loop callback: usock, 0x5624c28b9410
Removed loop callback: workq, 0x5624c28b9410
Removed loop callback: libteam_events, 0x5624c28b9410
Removed loop callback: daemon, 0x5624c28b9410
Failed: Bad file descriptor

Fix it by adding a default value for link_watch.interval.

v2: update default value to 1000, as Jamie Bainbridge suggested.

Reported-by: LiLiang <liali@redhat.com>
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 man/teamd.conf.5     | 10 ++++++++++
 teamd/teamd_lw_psr.c | 11 ++++++++---
 2 files changed, 18 insertions(+), 3 deletions(-)

Comments

Jamie Bainbridge March 5, 2019, 4:10 a.m. UTC | #1
Looks good to me.

Jamie

On Tue, 5 Mar 2019 at 13:42, Hangbin Liu <liuhangbin@gmail.com> wrote:
>
> As we don't have a default value for link_watch.interval. If user
> forgot to set this parameter, teamd will failed to init port priv and
> exit at the end.
>
> e.g.
> teamd -g -c '{"runner":{"name":"activebackup"},"link_watch":{"name":"arp_ping","target_host":"198.51.100.1"}}'
> teamdctl team0 port add p5p1
> teamdctl team0 port add p5p2
>
> teamd debug log shows:
> p5p2: Got link watch from global config.
> p5p2: Using sticky "0".
> Failed to get "interval" link-watch option.
> Failed to load options.
> Failed to init port priv.
> Callback named "lw_periodic" not found.
> Callback named "lw_socket" not found.
> Loop callback failed with: Invalid argument
> Failed loop callback: libteam_events, 0x5624c28b9410
> select() failed.
> Exiting...
> Removed loop callback: usock_acc_conn, 0x5624c28bab60
> Removed loop callback: usock, 0x5624c28b9410
> Removed loop callback: workq, 0x5624c28b9410
> Removed loop callback: libteam_events, 0x5624c28b9410
> Removed loop callback: daemon, 0x5624c28b9410
> Failed: Bad file descriptor
>
> Fix it by adding a default value for link_watch.interval.
>
> v2: update default value to 1000, as Jamie Bainbridge suggested.
>
> Reported-by: LiLiang <liali@redhat.com>
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> ---
>  man/teamd.conf.5     | 10 ++++++++++
>  teamd/teamd_lw_psr.c | 11 ++++++++---
>  2 files changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/man/teamd.conf.5 b/man/teamd.conf.5
> index 9bdf46a..43b09aa 100644
> --- a/man/teamd.conf.5
> +++ b/man/teamd.conf.5
> @@ -308,6 +308,11 @@ Default:
>  .TP
>  .BR "link_watch.interval "| " ports.PORTIFNAME.link_watch.interval " (int)
>  Value is a positive number in milliseconds. It is the interval between ARP requests being sent.
> +.RS 7
> +.PP
> +Default:
> +.BR "1000"
> +.RE
>  .TP
>  .BR "link_watch.init_wait "| " ports.PORTIFNAME.link_watch.init_wait " (int)
>  Value is a positive number in milliseconds. It is the delay between link watch initialization and the first ARP request being sent.
> @@ -371,6 +376,11 @@ Default:
>  .TP
>  .BR "link_watch.interval "| " ports.PORTIFNAME.link_watch.interval " (int)
>  Value is a positive number in milliseconds. It is the interval between sending NS packets.
> +.RS 7
> +.PP
> +Default:
> +.BR "1000"
> +.RE
>  .TP
>  .BR "link_watch.init_wait "| " ports.PORTIFNAME.link_watch.init_wait " (int)
>  Value is a positive number in milliseconds. It is the delay between link watch initialization and the first NS packet being sent.
> diff --git a/teamd/teamd_lw_psr.c b/teamd/teamd_lw_psr.c
> index c0772db..ad6e56b 100644
> --- a/teamd/teamd_lw_psr.c
> +++ b/teamd/teamd_lw_psr.c
> @@ -28,6 +28,7 @@
>   */
>
>  static const struct timespec lw_psr_default_init_wait = { 0, 1 };
> +#define LW_PSR_DEFAULT_INTERVAL   1000
>  #define LW_PSR_DEFAULT_MISSED_MAX 3
>
>  #define LW_PERIODIC_CB_NAME "lw_periodic"
> @@ -77,9 +78,13 @@ static int lw_psr_load_options(struct teamd_context *ctx,
>         int tmp;
>
>         err = teamd_config_int_get(ctx, &tmp, "@.interval", cpcookie);
> -       if (err) {
> -               teamd_log_err("Failed to get \"interval\" link-watch option.");
> -               return -EINVAL;
> +       if (!err) {
> +               if (tmp < 0) {
> +                       teamd_log_err("\"interval\" must not be negative number.");
> +                       return -EINVAL;
> +               }
> +       } else {
> +               tmp = LW_PSR_DEFAULT_INTERVAL;
>         }
>         teamd_log_dbg("interval \"%d\".", tmp);
>         ms_to_timespec(&psr_ppriv->interval, tmp);
> --
> 2.19.2
>
Jiri Pirko March 8, 2019, 8:55 a.m. UTC | #2
Tue, Mar 05, 2019 at 04:41:56AM CET, liuhangbin@gmail.com wrote:
>As we don't have a default value for link_watch.interval. If user
>forgot to set this parameter, teamd will failed to init port priv and
>exit at the end.
>
>e.g.
>teamd -g -c '{"runner":{"name":"activebackup"},"link_watch":{"name":"arp_ping","target_host":"198.51.100.1"}}'
>teamdctl team0 port add p5p1
>teamdctl team0 port add p5p2
>
>teamd debug log shows:
>p5p2: Got link watch from global config.
>p5p2: Using sticky "0".
>Failed to get "interval" link-watch option.
>Failed to load options.
>Failed to init port priv.
>Callback named "lw_periodic" not found.
>Callback named "lw_socket" not found.
>Loop callback failed with: Invalid argument
>Failed loop callback: libteam_events, 0x5624c28b9410
>select() failed.
>Exiting...
>Removed loop callback: usock_acc_conn, 0x5624c28bab60
>Removed loop callback: usock, 0x5624c28b9410
>Removed loop callback: workq, 0x5624c28b9410
>Removed loop callback: libteam_events, 0x5624c28b9410
>Removed loop callback: daemon, 0x5624c28b9410
>Failed: Bad file descriptor
>
>Fix it by adding a default value for link_watch.interval.
>
>v2: update default value to 1000, as Jamie Bainbridge suggested.
>
>Reported-by: LiLiang <liali@redhat.com>
>Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>


Applying: teamd: add a default value 1000 for link_watch.interval
error: patch failed: man/teamd.conf.5:308
error: man/teamd.conf.5: patch does not apply
error: patch failed: teamd/teamd_lw_psr.c:28
error: teamd/teamd_lw_psr.c: patch does not apply
Patch failed at 0001 teamd: add a default value 1000 for link_watch.interval
Use 'git am --show-current-patch' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Hangbin Liu March 8, 2019, 11:25 a.m. UTC | #3
On Fri, Mar 08, 2019 at 09:55:34AM +0100, Jiri Pirko wrote:
> Tue, Mar 05, 2019 at 04:41:56AM CET, liuhangbin@gmail.com wrote:
> >As we don't have a default value for link_watch.interval. If user
> >forgot to set this parameter, teamd will failed to init port priv and
> >exit at the end.
> >
> 
> Applying: teamd: add a default value 1000 for link_watch.interval
> error: patch failed: man/teamd.conf.5:308
> error: man/teamd.conf.5: patch does not apply
> error: patch failed: teamd/teamd_lw_psr.c:28
> error: teamd/teamd_lw_psr.c: patch does not apply
> Patch failed at 0001 teamd: add a default value 1000 for link_watch.interval
> Use 'git am --show-current-patch' to see the failed patch
> When you have resolved this problem, run "git am --continue".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".

That's weird, I can apply this patch correctly on current HEAD

547a4dc (HEAD -> master) teamd: add a default value 1000 for link_watch.interval
5f35530 (origin/master, origin/HEAD) teamd: lw: arp_ping: only check arp reply message

Thanks
Hangbin
Jiri Pirko March 8, 2019, 11:59 a.m. UTC | #4
Fri, Mar 08, 2019 at 12:25:20PM CET, liuhangbin@gmail.com wrote:
>On Fri, Mar 08, 2019 at 09:55:34AM +0100, Jiri Pirko wrote:
>> Tue, Mar 05, 2019 at 04:41:56AM CET, liuhangbin@gmail.com wrote:
>> >As we don't have a default value for link_watch.interval. If user
>> >forgot to set this parameter, teamd will failed to init port priv and
>> >exit at the end.
>> >
>> 
>> Applying: teamd: add a default value 1000 for link_watch.interval
>> error: patch failed: man/teamd.conf.5:308
>> error: man/teamd.conf.5: patch does not apply
>> error: patch failed: teamd/teamd_lw_psr.c:28
>> error: teamd/teamd_lw_psr.c: patch does not apply
>> Patch failed at 0001 teamd: add a default value 1000 for link_watch.interval
>> Use 'git am --show-current-patch' to see the failed patch
>> When you have resolved this problem, run "git am --continue".
>> If you prefer to skip this patch, run "git am --skip" instead.
>> To restore the original branch and stop patching, run "git am --abort".
>
>That's weird, I can apply this patch correctly on current HEAD
>
>547a4dc (HEAD -> master) teamd: add a default value 1000 for link_watch.interval
>5f35530 (origin/master, origin/HEAD) teamd: lw: arp_ping: only check arp reply message

Try now with head f36c191da3d65a4744582b2eb09fa297dd85f9ae


>
>Thanks
>Hangbin
Hangbin Liu March 8, 2019, 3:08 p.m. UTC | #5
On Fri, Mar 08, 2019 at 12:59:22PM +0100, Jiri Pirko wrote:
> >> Applying: teamd: add a default value 1000 for link_watch.interval
> >> error: patch failed: man/teamd.conf.5:308
> >> error: man/teamd.conf.5: patch does not apply
> >> error: patch failed: teamd/teamd_lw_psr.c:28
> >> error: teamd/teamd_lw_psr.c: patch does not apply
> >> Patch failed at 0001 teamd: add a default value 1000 for link_watch.interval
> >> Use 'git am --show-current-patch' to see the failed patch
> >> When you have resolved this problem, run "git am --continue".
> >> If you prefer to skip this patch, run "git am --skip" instead.
> >> To restore the original branch and stop patching, run "git am --abort".
> >
> >That's weird, I can apply this patch correctly on current HEAD
> >
> >547a4dc (HEAD -> master) teamd: add a default value 1000 for link_watch.interval
> >5f35530 (origin/master, origin/HEAD) teamd: lw: arp_ping: only check arp reply message
> 
> Try now with head f36c191da3d65a4744582b2eb09fa297dd85f9ae

Still works

$ git am ~/Mail/incoming/_patchv2_libteam_teamd_add_a_default_value_1000_for_link_watch.interva.patch
Applying: teamd: add a default value 1000 for link_watch.interval
$ git log --oneline | head -2
2863d2e teamd: add a default value 1000 for link_watch.interval
f36c191 man: fix runner.min_ports default value

Thanks
Hangbin
Hangbin Liu March 13, 2019, 7:06 a.m. UTC | #6
On Fri, Mar 08, 2019 at 11:08:21PM +0800, Hangbin Liu wrote:
> On Fri, Mar 08, 2019 at 12:59:22PM +0100, Jiri Pirko wrote:
> > >> Applying: teamd: add a default value 1000 for link_watch.interval
> > >> error: patch failed: man/teamd.conf.5:308
> > >> error: man/teamd.conf.5: patch does not apply
> > >> error: patch failed: teamd/teamd_lw_psr.c:28
> > >> error: teamd/teamd_lw_psr.c: patch does not apply
> > >> Patch failed at 0001 teamd: add a default value 1000 for link_watch.interval
> > >> Use 'git am --show-current-patch' to see the failed patch
> > >> When you have resolved this problem, run "git am --continue".
> > >> If you prefer to skip this patch, run "git am --skip" instead.
> > >> To restore the original branch and stop patching, run "git am --abort".
> > >
> > >That's weird, I can apply this patch correctly on current HEAD
> > >
> > >547a4dc (HEAD -> master) teamd: add a default value 1000 for link_watch.interval
> > >5f35530 (origin/master, origin/HEAD) teamd: lw: arp_ping: only check arp reply message
> > 
> > Try now with head f36c191da3d65a4744582b2eb09fa297dd85f9ae
> 
> Still works
> 
> $ git am ~/Mail/incoming/_patchv2_libteam_teamd_add_a_default_value_1000_for_link_watch.interva.patch
> Applying: teamd: add a default value 1000 for link_watch.interval
> $ git log --oneline | head -2
> 2863d2e teamd: add a default value 1000 for link_watch.interval
> f36c191 man: fix runner.min_ports default value

Hi Jiri,

Do I need to re-send the patch and let you have a try again?

Thanks
Hangbin
Jiri Pirko April 17, 2019, 7:34 a.m. UTC | #7
Wed, Mar 13, 2019 at 08:06:33AM CET, liuhangbin@gmail.com wrote:
>On Fri, Mar 08, 2019 at 11:08:21PM +0800, Hangbin Liu wrote:
>> On Fri, Mar 08, 2019 at 12:59:22PM +0100, Jiri Pirko wrote:
>> > >> Applying: teamd: add a default value 1000 for link_watch.interval
>> > >> error: patch failed: man/teamd.conf.5:308
>> > >> error: man/teamd.conf.5: patch does not apply
>> > >> error: patch failed: teamd/teamd_lw_psr.c:28
>> > >> error: teamd/teamd_lw_psr.c: patch does not apply
>> > >> Patch failed at 0001 teamd: add a default value 1000 for link_watch.interval
>> > >> Use 'git am --show-current-patch' to see the failed patch
>> > >> When you have resolved this problem, run "git am --continue".
>> > >> If you prefer to skip this patch, run "git am --skip" instead.
>> > >> To restore the original branch and stop patching, run "git am --abort".
>> > >
>> > >That's weird, I can apply this patch correctly on current HEAD
>> > >
>> > >547a4dc (HEAD -> master) teamd: add a default value 1000 for link_watch.interval
>> > >5f35530 (origin/master, origin/HEAD) teamd: lw: arp_ping: only check arp reply message
>> > 
>> > Try now with head f36c191da3d65a4744582b2eb09fa297dd85f9ae
>> 
>> Still works
>> 
>> $ git am ~/Mail/incoming/_patchv2_libteam_teamd_add_a_default_value_1000_for_link_watch.interva.patch
>> Applying: teamd: add a default value 1000 for link_watch.interval
>> $ git log --oneline | head -2
>> 2863d2e teamd: add a default value 1000 for link_watch.interval
>> f36c191 man: fix runner.min_ports default value
>
>Hi Jiri,
>
>Do I need to re-send the patch and let you have a try again?

Applying: teamd: add a default value 1000 for link_watch.interval
error: patch failed: man/teamd.conf.5:308
error: man/teamd.conf.5: patch does not apply
error: patch failed: teamd/teamd_lw_psr.c:28
error: teamd/teamd_lw_psr.c: patch does not apply
Patch failed at 0001 teamd: add a default value 1000 for link_watch.interval
hint: Use 'git am --show-current-patch' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Please generate and send again.


>
>Thanks
>Hangbin
Hangbin Liu April 17, 2019, 8:09 a.m. UTC | #8
On Wed, Apr 17, 2019 at 09:34:50AM +0200, Jiri Pirko wrote:
> >Hi Jiri,
> >
> >Do I need to re-send the patch and let you have a try again?
> 
> Applying: teamd: add a default value 1000 for link_watch.interval
> error: patch failed: man/teamd.conf.5:308
> error: man/teamd.conf.5: patch does not apply
> error: patch failed: teamd/teamd_lw_psr.c:28
> error: teamd/teamd_lw_psr.c: patch does not apply
> Patch failed at 0001 teamd: add a default value 1000 for link_watch.interval
> hint: Use 'git am --show-current-patch' to see the failed patch
> When you have resolved this problem, run "git am --continue".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".
> 
> Please generate and send again.

Hmm, that's really weird. I will ask Xin Long help post the patch.

Thanks
Hangbin

Patch
diff mbox series

diff --git a/man/teamd.conf.5 b/man/teamd.conf.5
index 9bdf46a..43b09aa 100644
--- a/man/teamd.conf.5
+++ b/man/teamd.conf.5
@@ -308,6 +308,11 @@  Default:
 .TP
 .BR "link_watch.interval "| " ports.PORTIFNAME.link_watch.interval " (int)
 Value is a positive number in milliseconds. It is the interval between ARP requests being sent.
+.RS 7
+.PP
+Default:
+.BR "1000"
+.RE
 .TP
 .BR "link_watch.init_wait "| " ports.PORTIFNAME.link_watch.init_wait " (int)
 Value is a positive number in milliseconds. It is the delay between link watch initialization and the first ARP request being sent.
@@ -371,6 +376,11 @@  Default:
 .TP
 .BR "link_watch.interval "| " ports.PORTIFNAME.link_watch.interval " (int)
 Value is a positive number in milliseconds. It is the interval between sending NS packets.
+.RS 7
+.PP
+Default:
+.BR "1000"
+.RE
 .TP
 .BR "link_watch.init_wait "| " ports.PORTIFNAME.link_watch.init_wait " (int)
 Value is a positive number in milliseconds. It is the delay between link watch initialization and the first NS packet being sent.
diff --git a/teamd/teamd_lw_psr.c b/teamd/teamd_lw_psr.c
index c0772db..ad6e56b 100644
--- a/teamd/teamd_lw_psr.c
+++ b/teamd/teamd_lw_psr.c
@@ -28,6 +28,7 @@ 
  */
 
 static const struct timespec lw_psr_default_init_wait = { 0, 1 };
+#define LW_PSR_DEFAULT_INTERVAL   1000
 #define LW_PSR_DEFAULT_MISSED_MAX 3
 
 #define LW_PERIODIC_CB_NAME "lw_periodic"
@@ -77,9 +78,13 @@  static int lw_psr_load_options(struct teamd_context *ctx,
 	int tmp;
 
 	err = teamd_config_int_get(ctx, &tmp, "@.interval", cpcookie);
-	if (err) {
-		teamd_log_err("Failed to get \"interval\" link-watch option.");
-		return -EINVAL;
+	if (!err) {
+		if (tmp < 0) {
+			teamd_log_err("\"interval\" must not be negative number.");
+			return -EINVAL;
+		}
+	} else {
+		tmp = LW_PSR_DEFAULT_INTERVAL;
 	}
 	teamd_log_dbg("interval \"%d\".", tmp);
 	ms_to_timespec(&psr_ppriv->interval, tmp);