[ovs-dev] [PATCH 00/12] Support zone-based conntrack timeout policy

Yi-Hung Wei yihung.wei at gmail.com
Mon Jul 29 18:53:29 UTC 2019

Hi Ilya,

Thanks for your comment.

On Mon, Jul 29, 2019 at 2:22 AM Ilya Maximets <i.maximets at samsung.com> wrote:
> Hi everyone,
> My 2 cents for the feature design:
> From the user's perspective:
> * 'add-dp'/'del-dp' commands looks very strange.
>   "I didn't add datapath into ovsdb, why it exists and switches packets?"
>   "I deleted the datapath from the OVS, why it still exists and switches packets?"
>   If you're implementing the configuration like this, 'datapath' should
>   own the bridges and interfaces, i.e. datapath should be created manually
>   on 'add-dp' and automatically on adding the first bridge on that datapath.
>   All the bridges and interfaces must be deleted/destroyed on 'del-dp'.
>   Or you need to rename your tables and commands to not look like this.
> From the developer's perspective:
> * Right now 'ofproto-dpif' is the only module that manages datapath interfaces
>   and it knows that (there are specific comments in the code). 'dpif's has
>   no reference counts and it's slightly unsafe to manage them outside of
>   'ofproto-dpif'.
>   You're adding the side module that allowed to open dpif (and it's not able
>   to delete it, that is the possible cause if issues) and use it without
>   noticing any other modules. This breaks the hierarchical structure of OVS.
> * Right now most of the datapath configuration is done via 'other_config'
>   and corresponding dpif_set_config() callback. Since you're introducing
>   datapath-config module, it should take care of all of this staff. And this
>   will require significant rework of the current datapath configuration scheme.
> * 'reconfigure_datapath' is an ambiguous name.
>   Solution for above issues might be not introducing the new modules at all.
>   Everything could be handled like we're handling meters, but with OVSDB as the
>   configuration source. On configuration change bridge layer will call ofproto
>   layer that will pass configuration to ofproto-dpif and, finally, dpif layer.
>   Inside 'struct dpif' in dpif.c module you could track all the configuration
>   and pass all the required changes to the dpif-provider via callbacks.
>   This way everything will work fine without breaking current OVS hierarchy.

Thanks for your suggestion about the datapath-config part. I think it
makes sense to implement what datapath-config does inside dpif.c
rather than introduce a new module.  I will make proper change for
that in v2.

> * DB scheme looks just overcomplicated. 3 additional tables which references
>   others just to store a string to integer map.
>   I think that it might be much easier to create a single 'CT_Zones' table
>   with all the required columns:
>       'id', 'tcp_syn_sent', 'tcp_syn_recv', ..., 'icmp_reply'.
>   This is not a big deal since you need to describe every single field in
>   vswitch.xml anyway. This will also allow you to check support of particular
>   field on the stage of adding value to the database.
>   If you really need to distinguish zones by the datapath type (which is not
>   obvious), you may add 'datapath_type' column, just like we have in a 'Bridge'
>   table.

As for the database schema, we intend to make CT_Zone table references
to CT_Timeout_Policy table because some other zone-based feature can
be configured through ovsdb later on. For example, we can have a new
column in CT_Zone table that stores 'limit' as an integer to support
the zone limit feature (limiting number of connection in a zone).  It
is currently configured through dpctl commands.

I understand your concern on the complication that introduced by the
datapath table.  Let me think about it more carefully and go back to
you later.



More information about the dev mailing list