[ovs-dev] [PATCH v2 8/9] ofproto-dpif-xlate: Translate timeout policy in ct action

[Darrell Ball]

On Fri, Aug 9, 2019 at 1:10 PM Justin Pettit <jpettit at ovn.org> wrote:

>
> > On Aug 1, 2019, at 3:07 PM, Yi-Hung Wei <yihung.wei at gmail.com> wrote:
> >
> > diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h
> > index 79a2314500cf..57b32ccb610f 100644
> > --- a/lib/dpif-provider.h
> > +++ b/lib/dpif-provider.h
> > @@ -536,6 +536,11 @@ struct dpif_class {
> >                                        struct ct_dpif_timeout_policy
> *tp);
> >     int (*ct_timeout_policy_dump_done)(struct dpif *, void *state);
> >
> > +    /* Get timeout policy name (OVS_CT_ATTR_TIMEOUT) from datapath. */
> > +    int (*ct_format_timeout_policy_name)(struct dpif *, uint32_t tp_id,
> > +                                         uint16_t dl_type, uint8_t
> nw_proto,
> > +                                         struct ds *ds);
>
> To Darrell's point about wanting the ability to implement timeout policies
> in the userspace implementation without unwildcarding the dl_type and
> nw_proto, I'd suggest adding a "bool *unwildcard" argument that the
> provider can indicate whether those bits should be unwildcarded or not.
>

Sounds good

A related point is that some timeout profile expansion supporting code is
presently located in common code in ct-dpif.*
This is the code that helps convert a profile from the database form and
expands it to 6 Netfilter profiles -
(TCP4, TCP6, UDP4, UDP6, ICMP4, ICMP6).
I did mention this in some other comments, but I wanted to reiterate to
save some of the further churn.

One other thing that concerns me is that datapath rules referencing timeout
profiles can linger in the fast path after the associated
timeout profiles are supposed to be gone (eg) short timers/ct-flush).
Although, presently, the communication of timeout profiles via datapath
rules is
needed for Netfilter support, this lingering of timeout profile context is
not ideal. It would be good to document it.

The above being said, I do think that for userspace datapath timeout
profiles could be communicated more simply to userspace conntrack.
After reading ct-zone/timeout profile info from the database, the
ct-zone/timeout profile delta can be sent via _type_set_config() API to
the userspace datapath/conntrack.
conntrack can associate timeout profiles to ct/commit related rules via
ct-zone. This would also satisfy the present requirements.
This would mean the timeout profile specifiers would not need to be
directly associated with datapath rules.


>
> > diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
> > index 41e07f0ee23e..1a2fc4a6a084 100644
> > --- a/ofproto/ofproto-provider.h
> > +++ b/ofproto/ofproto-provider.h
> > @@ -1880,6 +1880,11 @@ struct ofproto_class {
> >             const struct ovsrec_datapath *dp_cfg, unsigned int
> idl_seqno);
> >     /* Cleans up the to be deleted timeout policy in the pending kill
> list. */
> >     void (*ct_zone_timeout_policy_sweep)(const struct ofproto *ofproto_);
> > +
> > +    /* Returns true if timeout policy for 'zone' is configured and
> stores the
> > +     * timeout policy id in '*tp_id'. */
> > +    bool (*ct_zone_timeout_policy_get)(const struct ofproto *ofproto_,
> > +                                       uint16_t zone, uint32_t *tp_id);
> > };
>
> As we discuss off-line, by pulling the OVSDB information out of the dpif
> layer, I think we'll want a more tradition get/set ofproto interface.  I
> believe that will be done in v3.
>
> Thanks,
>
> --Justin
>
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>