On Mon, Jul 29, 2019 at 11:53 AM Yi-Hung Wei <yihung....@gmail.com> wrote:
> Hi Ilya, > > Thanks for your comment. > > On Mon, Jul 29, 2019 at 2:22 AM Ilya Maximets <i.maxim...@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. > I agree that a "datapath" should be auto-created when bridge of that type is created. This came up in internal e-mails and I lost track of it. It is a bit strange to call this column 'datapath'; 'datapath-config' might be a bit better and reflects what it is really is. I think renaming would clear things up considerably and avoid confusion. > > > > 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. > > Thanks, > > -Yi-Hung > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
