[ovs-dev] [PATCH v4 4/9] ct-dpif, dpif-netlink: Add conntrack timeout policy support

Yi-Hung Wei yihung.wei at gmail.com
Mon Aug 19 17:52:38 UTC 2019


On Fri, Aug 16, 2019 at 5:07 PM Darrell Ball <dlu998 at gmail.com> wrote:
>
> Thanks for the patch
>
> Pls let me know if this incremental works for you.
> Main change is logging fix for timeout policy deletion.
>
> Darrell
>
> diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
> index 1d4ee60..00d957b 100644
> --- a/lib/dpif-netlink.c
> +++ b/lib/dpif-netlink.c
> @@ -2822,11 +2822,10 @@ dpif_netlink_ct_dump_done(struct dpif *dpif OVS_UNUSED,
>                            struct ct_dpif_dump_state *dump_)
>  {
>      struct dpif_netlink_ct_dump_state *dump;
> -    int err;
>
>      INIT_CONTAINER(dump, dump_, up);
>
> -    err = nl_ct_dump_done(dump->nl_ct_dump);
> +    int err = nl_ct_dump_done(dump->nl_ct_dump);
>      free(dump);
>      return err;
>  }
> @@ -3335,7 +3334,8 @@ dpif_netlink_ct_del_timeout_policy(struct dpif *dpif OVS_UNUSED,
>              err = 0;
>          }
>          if (err) {
> -            VLOG_WARN_RL(&error_rl, "failed to delete timeout policy %s (%s)",
> +            static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);

Thanks for the diff.  It looks good in general.

I agree on the main concern of the proposed diff which is the original
rate limit in dpif-netlink (VLOG_RATE_LIMIT_INIT(9999, 5)) may log too
much duplicated information.  However, since we may delete more than
one one timeout policy in a minute, so lowering the rate limit to
VLOG_RATE_LIMIT_INIT(1, 1) may miss some useful information.   I would
use somewhere in the between (VLOG_RATE_LIMIT_INIT(5, 5)) in the next
version.

Thanks,

-Yi-Hung


More information about the dev mailing list