[ovs-dev] [PATCH 2/2] bridge: Only an inactivity_probe of 0 should turn off inactivity probes.

Ilya Maximets i.maximets at ovn.org
Mon Jun 14 19:08:58 UTC 2021


On 6/11/21 12:49 AM, Ben Pfaff wrote:
> The documentation for inactivity_probe says this:
> 
>        inactivity_probe: optional integer
>               Maximum  number  of milliseconds of idle time on connec‐
>               tion to controller before sending  an  inactivity  probe
>               message.  If  Open vSwitch does not communicate with the
>               controller for the specified number of seconds, it  will
>               send a probe. If a response is not received for the same
>               additional amount of time, Open vSwitch assumes the con‐
>               nection  has  been broken and attempts to reconnect. De‐
>               fault is implementation-specific. A value of 0  disables
>               inactivity probes.
> 
> This means that a value of 0 should disable inactivity probes and any
> other value should be in milliseconds.  The code in bridge.c was
> actually interpreting it as any value between 0 and 999 disabling
> inactivity probes.  That was surprising when I accidentally configured
> it to 5 or to 10, not remembering that it was in milliseconds, and
> disabled them entirely.  This fixes the problem.
> 
> Signed-off-by: Ben Pfaff <blp at ovn.org>
> ---
>  vswitchd/bridge.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> index 5ed7e8234354..9c1ee3cf06fe 100644
> --- a/vswitchd/bridge.c
> +++ b/vswitchd/bridge.c
> @@ -3965,8 +3965,10 @@ bridge_configure_remotes(struct bridge *br,
>          *oc = (struct ofproto_controller) {
>              .type = get_controller_ofconn_type(c->target, c->type),
>              .max_backoff = c->max_backoff ? *c->max_backoff / 1000 : 8,
> -            .probe_interval = (c->inactivity_probe
> -                               ? *c->inactivity_probe / 1000 : 5),
> +            .probe_interval = (!c->inactivity_probe ? 5
> +                               : !*c->inactivity_probe ? 0
> +                               : *c->inactivity_probe < 1000 ? 1
> +                               : *c->inactivity_probe / 1000),
>              .band = ((!c->connection_mode
>                        || !strcmp(c->connection_mode, "in-band"))
>                       && !disable_in_band
> 

This change looks fine in terms that it fixes disabling of the
inactivity probe, so:

  Acked-by: Ilya Maximets <i.maximets at ovn.org>

OTOH, it's unclear why the DB schema allows configuration in
milliseconds if the implementation doesn't honor the configured
value rounding it up or down.


More information about the dev mailing list