On Tue, Jun 13, 2017 at 4:40 PM, Russell Bryant <russ...@ovn.org> wrote: > On Fri, Jun 2, 2017 at 8:31 AM, <majop...@redhat.com> wrote: >> From: Miguel Angel Ajo <majop...@redhat.com> >> >> This patch handles multiple gateways with priorities in chassisredirect >> ports, any gateway with a chassis redirect port will implement the >> rules to de-encapsulate incomming packets for such port. >> >> And hosts targetting a remote chassisredirect port will setup a >> bundle(active_backup, ..) action to each tunnel port, in the given >> priority order. >> >> Signed-off-by: Miguel Angel Ajo <majop...@redhat.com> >> --- >> ovn/controller/binding.c | 9 +-- >> ovn/controller/lflow.c | 6 +- >> ovn/controller/lport.c | 119 >> ++++++++++++++++++++++++++++++++++++++++ >> ovn/controller/lport.h | 28 ++++++++++ >> ovn/controller/ovn-controller.c | 5 +- >> ovn/controller/physical.c | 114 ++++++++++++++++++++++++++++++++------ >> 6 files changed, 255 insertions(+), 26 deletions(-) > > Some high level comments to start ... > > Ideally with a patch series, each patch should be applicable on its > own. With this patch applied, some tests are failing for me. > > Documentation should also be included with whatever patch first > introduces functionality, so I'd expect docs on the updated > redirect-chassis format here. > > Please read over > Documentation/internals/contributing/coding-style.rst. There are some > minor style issues throughout the patch. I can point them out in a > more detailed pass. > > The patch makes me wonder if we should introduce a more structured > format for specifying chassis associated with a router port. It feels > like we're encoding too much in a single option string. Maybe we > should add a new "chassis" column to Logical_Router_Port, that can > include a list of chassis, which would have to be a new record type in > OVN northbound, containing much less info than the southbound > counterpart. We'd have to add a similar new column to the > Port_Binding table in OVN southbound. I'm curious what you and others > think about this, or if the parsed option string is fine.
Sorry, I replied to v1, but all comments apply to v2. -- Russell Bryant _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
