On 29.07.2019 12:21, Ilya Maximets wrote:
> On 26.07.2019 2:24, Yi-Hung Wei wrote:
>> This patch series enables zone-based conntrack timeout policy support in OVS.
>> Timeout policy is a set of timeout attributes that can be associated with a
>> connection when it is committed. Then, the connection tracking system will
>> expire a connection based on its connection state. For example, one use
>> case would be to extend the timeout of TCP connection in the established
>> state to avoid re-connect overhead. Or use case is to shorten the connection
>> timeout so that the system can reclaim resources faster.
>> The idea of zone-based conntrack timeout policy is to group connections
>> with similar characteristics in a conntrack zone, and assign timeout policy
>> to the conntrack zone. Therefore, all the connections in that zone will share
>> the same timeout policy.
>>
>> For zone-based timeout policy configuration, the association of conntrack
>> zone and conntrack timeout policy is defined per datapath in vswitch ovsdb
>> schema. User can program the database through ovs-vsctl or using ovsdb
>> protocol directly. Once the zone-based timeout policy configuration is
>> in the database, vswitchd will read those configuration and orgaznie it
>> in internal datapath strcture, and push the timeout policy into datapath.
>> Currenlty, only the kernel datapath supports customized timeout policy.
>
> 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.
>
> * 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.
Another option is a single 'CT_Zones' table with 'id' and 'timeouts' columns.
Where 'timeouts' is a map {string --> integer}.
So, something like this will work:
ovs-vsctl create CT_Zones 1 -- set CT_Zones 1 timeouts:icmp_first=60
timeouts:icmp_reply=60
> 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.
>
>
> Best regards, Ilya Maximets.
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
