[ovs-dev] [PATCH v3 07/11] ct-dpif: Helper functions for conntrack zone limit

[Yi-Hung Wei]

On Tue, Aug 14, 2018 at 4:29 PM Justin Pettit <jpettit at ovn.org> wrote:
>
>
> > On Aug 14, 2018, at 11:56 AM, Yi-Hung Wei <yihung.wei at gmail.com> wrote:
> >
> > +/* The caller takes ownership of 'struct ct_dpif_zone_limit *', and is
> > + * responsible to free that struct. */
> > +struct ct_dpif_zone_limit *
> > +ct_dpif_pop_zone_limit(struct ovs_list *zone_limits)
> > +{
> > +    struct ct_dpif_zone_limit *zone_limit;
> > +    LIST_FOR_EACH_POP (zone_limit, node, zone_limits) {
> > +        return zone_limit;
> > +    }
> > +    OVS_NOT_REACHED();
> > +}
>
> Aborting seems like a pretty serious action to take on calling this when it's not empty--especially if there's not a comment about it in the description.  I think we would normally just return NULL if it's not empty.  However, I think the function may not be needed since it's only used by ct_dpif_free_zone_limits() and could be simplified somewhat.
>
> What do you think of the incremental at the end?  (I've only compile-tested it.)
>
> --Justin

Thanks for the improvement.  I think the changes make sense.  I tested
it with the system traffic test and it works fine.

Thanks,

-Yi-Hung