[ovs-dev] [PATCH 08/12] datapath-config: Consume datapath, CT_Zone, and CT_Timeout_Policy tables

Yi-Hung Wei yihung.wei at gmail.com
Mon Jul 29 22:52:34 UTC 2019


On Fri, Jul 26, 2019 at 1:22 PM William Tu <u9012063 at gmail.com> wrote:
> > +static void
> > +datapath_update_ct_zone_config(struct datapath *dp, struct dpif *dpif,
> > +                               unsigned int idl_seqno)
> > +{
> > +    const struct ovsrec_datapath *dp_cfg = dp->cfg;
> > +    struct ovsrec_ct_timeout_policy *tp_cfg;
> > +    struct ovsrec_ct_zone *zone_cfg;
> > +    struct ct_timeout_policy *tp;
> > +    struct ct_zone *zone;
> > +    uint16_t zone_id;
> > +    bool new_zone;
> > +    size_t i;
> > +
> > +    for (i = 0; i < dp_cfg->n_ct_zones; i++) {
> > +        /* Update ct_zone config */
> > +        zone_cfg = dp_cfg->value_ct_zones[i];
> > +        zone_id = dp_cfg->key_ct_zones[i];
> > +        zone = ct_zone_lookup(&dp->ct_zones, zone_id);
> > +        if (!zone) {
> > +            new_zone = true;
> > +            zone = ct_zone_alloc(zone_id);
> > +        } else {
> > +            new_zone = false;
> > +        }
> > +        zone->last_used_seqno = idl_seqno;
> > +
> > +        /* Update timeout policy */
> missing .
> > +        tp_cfg = zone_cfg->timeout_policy;
> > +        tp = ct_timeout_policy_lookup(&dp->ct_tps, &tp_cfg->header_.uuid);
> > +        if (!tp) {
> > +            tp = ct_timeout_policy_alloc(tp_cfg, idl_seqno);
> > +            cmap_insert(&dp->ct_tps, &tp->node, uuid_hash(&tp->uuid));
> > +            if (dpif) {
> > +                ct_dpif_add_timeout_policy(dpif, false, &tp->cdtp);
> > +            }
> > +        } else {
> > +            if (ct_timeout_policy_update(tp_cfg, tp, idl_seqno)) {
> > +                if (dpif) {
> > +                    ct_dpif_add_timeout_policy(dpif, false, &tp->cdtp);
> > +                }
> > +            }
> > +        }
> > +        tp->last_used_seqno = idl_seqno;
> > +
> > +        /* Update default timeout policy */
>
> missing . and some places below
>
> > +        if (!zone_id && tp->last_updated_seqno == idl_seqno) {
> > +            ct_dpif_add_timeout_policy(dpif, true, &tp->cdtp);
>
> Do we need to check 'if (dpif)' here?
> You have the checking above, but not here. Or should we just return
> error when dpif is NULL.
Thanks for review. I will check if dpif is NULL and address the
comment issue in v2.

Thanks,

-Yi-Hung


More information about the dev mailing list