Re: [patchv2 libteam] teamd: add a default value 1000 for link_watch.interval

[Jamie Bainbridge]

Looks good to me.

Jamie

On Tue, 5 Mar 2019 at 13:42, Hangbin Liu  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 
> Signed-off-by: Hangbin Liu
>
> 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