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

Message ID db93a31a64a50d3593c9812ee131529d621d202c.1555498342.git.lucien.xin@gmail.com
State New
Headers show
Series
  • [PATCHv3] teamd: add a default value 1000 for link_watch.interval
Related show

Commit Message

Xin Long April 17, 2019, 10:52 a.m. UTC
From: Hangbin Liu <liuhangbin@gmail.com>

As we don't have a default value for link_watch.interval. If a user
forgets to set this parameter, teamd will fail to init ports' priv
and exit in 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.
v3: fix the changelog to pass checkpatch.pl.

Reported-by: LiLiang <liali@redhat.com>
Reviewed-by: Jamie Bainbridge <jamie.bainbridge@gmail.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

Hangbin Liu April 22, 2019, 8:50 a.m. UTC | #1
On Wed, Apr 17, 2019 at 06:52:22PM +0800, Xin Long wrote:
> From: Hangbin Liu <liuhangbin@gmail.com>
> 
> As we don't have a default value for link_watch.interval. If a user
> forgets to set this parameter, teamd will fail to init ports' priv
> and exit in 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.
> v3: fix the changelog to pass checkpatch.pl.
> 
> Reported-by: LiLiang <liali@redhat.com>
> Reviewed-by: Jamie Bainbridge <jamie.bainbridge@gmail.com>
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> ---

Hi Jiri,

Would you please help try this one?

Thanks
Hangbin
Jiri Pirko May 2, 2019, 6:42 a.m. UTC | #2
Wed, Apr 17, 2019 at 12:52:22PM CEST, lucien.xin@gmail.com wrote:
>From: Hangbin Liu <liuhangbin@gmail.com>
>
>As we don't have a default value for link_watch.interval. If a user
>forgets to set this parameter, teamd will fail to init ports' priv
>and exit in 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.
>v3: fix the changelog to pass checkpatch.pl.
>
>Reported-by: LiLiang <liali@redhat.com>
>Reviewed-by: Jamie Bainbridge <jamie.bainbridge@gmail.com>
>Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>

applied, thanks!

Patch
diff mbox series

diff --git a/man/teamd.conf.5 b/man/teamd.conf.5
index 5b0f3e9..9090b4a 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);