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

Darrell Ball dlu998 at gmail.com
Mon Aug 19 19:42:08 UTC 2019


On Mon, Aug 19, 2019 at 10:52 AM Yi-Hung Wei <yihung.wei at gmail.com> wrote:

> 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.
>

TBH, I am not sure we care lots about this information. I was even debating
changing it debug level.
We have 4 billion datapath timeout profile IDs, so it is unlikely we will
run out.
Eventually, they will get cleaned up by the retry thingy.

Also, I am not sure what action we will take by seeing these logs anyhow.

Spamming the log is more of a concern.


>
> Thanks,
>
> -Yi-Hung
>


More information about the dev mailing list