| Message ID | 20191108111537.10100.38432.stgit@dceara.remote.csb |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Improve ovn-detrace support for parsing OpenFlow cookies. | expand |
Hi Dumitru. Everything you've done looks good to me. However, when reviewing the patch, I educated myself on how ovn-detrace is implemented and I found a couple of problems. These problems aren't introduced by you. However, I think that since you have broadened the scope of the southbound database search area, they should probably be addressed. On 11/8/19 6:15 AM, Dumitru Ceara wrote: > Commit eb25a7da639e ("Improve debuggability of OVN to OpenFlow > translations.") added cookies for Port_Binding, Mac_Binding, > Multicast_Group, Chassis records to the OpenfFlow entries they > generate. > > Add support for parsing such cookies in ovn-detrace too. > Also, refactor ovn-detrace to allow a more generic way of defining > cookie handlers. > > Signed-off-by: Dumitru Ceara <dceara@redhat.com> > --- > utilities/ovn-detrace.in | 166 ++++++++++++++++++++++++++++++++-------------- > 1 file changed, 117 insertions(+), 49 deletions(-) > > diff --git a/utilities/ovn-detrace.in b/utilities/ovn-detrace.in > index 34b6b0e..79c6d3b 100755 > --- a/utilities/ovn-detrace.in > +++ b/utilities/ovn-detrace.in > @@ -98,48 +98,112 @@ class OVSDB(object): > def find_row_by_partial_uuid(self, table_name, value): > return self._find_row(table_name, lambda row: value in str(row.uuid)) (Note: this problem existed prior to the patch series) Since the cookie corresponds to the beginning of the southbound record's UUID, this lambda should probably be altered to: lambda row: str(row.uuid).startswith(value) The existing lambda could match the cookie with a later part of the UUID, returning an invalid match. > > - > -def get_lflow_from_cookie(ovnsb_db, cookie): > - return ovnsb_db.find_row_by_partial_uuid('Logical_Flow', cookie) > - > - > -def print_lflow(lflow, prefix): > - ldp_uuid = lflow.logical_datapath.uuid > - ldp_name = str(lflow.logical_datapath.external_ids.get('name')) > - > - print '%sLogical datapath: "%s" (%s) [%s]' % (prefix, > - ldp_name, > - ldp_uuid, > - lflow.pipeline) > - print "%sLogical flow: table=%s (%s), priority=%s, " \ > - "match=(%s), actions=(%s)" % (prefix, > - lflow.table_id, > - lflow.external_ids.get('stage-name'), > - lflow.priority, > - str(lflow.match).strip('"'), > - str(lflow.actions).strip('"')) > - > - > -def print_lflow_nb_hint(lflow, prefix, ovnnb_db): > - external_ids = lflow.external_ids > - if external_ids.get('stage-name') in ['ls_in_acl', > - 'ls_out_acl']: > - acl_hint = external_ids.get('stage-hint') > - if not acl_hint: > - return > - acl = ovnnb_db.find_row_by_partial_uuid('ACL', acl_hint) > - if not acl: > +class CookieHandler(object): > + def __init__(self, db, table): > + self._db = db > + self._table = table > + > + def get_record(self, cookie): > + return self._db.find_row_by_partial_uuid(self._table, cookie) > + > + def print_record(self, record, prefix): > + pass > + > + def print_hint(self, record, prefix, db): > + pass > + > +def datapath_str(datapath): > + return '"%s" (%s)' % (str(datapath.external_ids.get('name')), > + datapath.uuid) > + > +def chassis_str(chassis): > + if len(chassis) == 0: > + return '' > + ch = chassis[0] > + return 'chassis-name "%s", chassis-str "%s"' % (ch.name, ch.hostname) > + > +class LogicalFlowHandler(CookieHandler): > + def __init__(self, ovnsb_db): > + super(LogicalFlowHandler, self).__init__(ovnsb_db, 'Logical_Flow') > + > + def print_record(self, lflow, prefix): > + print('%sLogical datapath: %s [%s]' % > + (prefix, datapath_str(lflow.logical_datapath), lflow.pipeline)) > + print('%sLogical flow: table=%s (%s), priority=%s, ' > + 'match=(%s), actions=(%s)' % > + (prefix, lflow.table_id, lflow.external_ids.get('stage-name'), > + lflow.priority, > + str(lflow.match).strip('"'), > + str(lflow.actions).strip('"'))) > + > + def print_hint(self, lflow, prefix, ovnnb_db): > + external_ids = lflow.external_ids > + if external_ids.get('stage-name') in ['ls_in_acl', > + 'ls_out_acl']: > + acl_hint = external_ids.get('stage-hint') > + if not acl_hint: > + return > + acl = ovnnb_db.find_row_by_partial_uuid('ACL', acl_hint) > + if not acl: > + return > + output = '%sACL: %s, priority=%s, ' \ > + 'match=(%s), %s' % (prefix, > + acl.direction, > + acl.priority, > + acl.match.strip('"'), > + acl.action) > + if acl.log: > + output += ' (log)' > + print(output) > + > +class PortBindingHandler(CookieHandler): > + def __init__(self, ovnsb_db): > + super(PortBindingHandler, self).__init__(ovnsb_db, 'Port_Binding') > + > + def print_record(self, pb, prefix): > + ldp_uuid = pb.datapath.uuid > + ldp_name = str(pb.datapath.external_ids.get('name')) > + > + print('%sLogical datapath: %s' % (prefix, datapath_str(pb.datapath))) > + print('%sPort Binding: logical_port "%s", tunnel_key %ld, %s' % > + (prefix, pb.logical_port, pb.tunnel_key, > + chassis_str(pb.chassis))) > + > +class MacBindingHandler(CookieHandler): > + def __init__(self, ovnsb_db): > + super(MacBindingHandler, self).__init__(ovnsb_db, 'MAC_Binding') > + > + def print_record(self, mb, prefix): > + print('%sLogical datapath: %s' % (prefix, datapath_str(mb.datapath))) > + print('%sMAC Binding: ip "%s", logical_port "%s", mac "%s"' % > + (prefix, mb.ip, mb.logical_port, mb.mac)) > + > +class MulticastGroupHandler(CookieHandler): > + def __init__(self, ovnsb_db): > + super(MulticastGroupHandler, self).__init__(ovnsb_db, > + 'Multicast_Group') > + > + def print_record(self, mc, prefix): > + mc_ports = ', '.join([pb.logical_port for pb in mc.ports]) > + > + print('%sLogical datapath: %s' % (prefix, datapath_str(mc.datapath))) > + print('%sMulticast Group: name "%s", tunnel_key %ld ports: (%s)' % > + (prefix, mc.name, mc.tunnel_key, mc_ports)) > + > +class ChassisHandler(CookieHandler): > + def __init__(self, ovnsb_db): > + super(ChassisHandler, self).__init__(ovnsb_db, 'Chassis') > + > + def print_record(self, chassis, prefix): > + print('%sChassis: %s' % (prefix, chassis_str([chassis]))) > + > +def print_sb_record_from_cookie(ovnnb_db, ovnsb_db, cookie_handlers, cookie): > + for handler in cookie_handlers: > + sb_record = handler.get_record(cookie) (Note: this problem also existed before this patch series) handler.get_record() uses a generator expression to retrieve one of several potential matches for the cookie. If there are multiple partial matches in the database, then each subsequent call to handler.get_record() will return the next row with the same partial match. Perhaps it is a good idea to call handler.get_record() until it returns None each time. This way, if there is potential ambiguity we can at least present to the user that they're hitting one of several possible matching southbound database entries. > + if sb_record: > + handler.print_record(sb_record, " * ") > + handler.print_hint(sb_record, " * ", ovnnb_db) > return > - output = "%sACL: %s, priority=%s, " \ > - "match=(%s), %s" % (prefix, > - acl.direction, > - acl.priority, > - acl.match.strip('"'), > - acl.action) > - if acl.log: > - output += ' (log)' > - print output > - > > def main(): > try: > @@ -183,6 +247,14 @@ def main(): > ovsdb_ovnsb = OVSDB(ovnsb_db, 'OVN_Southbound') > ovsdb_ovnnb = OVSDB(ovnnb_db, 'OVN_Northbound') > > + cookie_handlers = [ > + LogicalFlowHandler(ovsdb_ovnsb), > + PortBindingHandler(ovsdb_ovnsb), > + MacBindingHandler(ovsdb_ovnsb), > + MulticastGroupHandler(ovsdb_ovnsb), > + ChassisHandler(ovsdb_ovnsb) > + ] > + > regex_cookie = re.compile(r'^.*cookie 0x([0-9a-fA-F]+)') > regex_table_id = re.compile(r'^[0-9]+\.') > cookie = None > @@ -194,14 +266,10 @@ def main(): > > line = line.strip() > > - if cookie: > - # print lflow info when the current flow block ends > - if regex_table_id.match(line): > - lflow = get_lflow_from_cookie(ovsdb_ovnsb, cookie) > - if lflow: > - print_lflow(lflow, " * ") > - print_lflow_nb_hint(lflow, " * ", ovsdb_ovnnb) > - cookie = None > + # Print SB record info when the current flow block ends. > + if cookie and regex_table_id.match(line): > + print_sb_record_from_cookie(ovsdb_ovnnb, ovsdb_ovnsb, > + cookie_handlers, cookie) > > print line > > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On Mon, Nov 11, 2019 at 8:02 PM Mark Michelson <mmichels@redhat.com> wrote: > > Hi Dumitru. Everything you've done looks good to me. > > However, when reviewing the patch, I educated myself on how ovn-detrace > is implemented and I found a couple of problems. These problems aren't > introduced by you. However, I think that since you have broadened the > scope of the southbound database search area, they should probably be > addressed. Hi Mark, Thanks for looking into this. I addressed your remarks and sent a v2: https://patchwork.ozlabs.org/project/openvswitch/list/?series=142383 I also refactored a bit more the printing so we don't have to explicitly specify the 'prefix' every time. Thanks, Dumitru > > On 11/8/19 6:15 AM, Dumitru Ceara wrote: > > Commit eb25a7da639e ("Improve debuggability of OVN to OpenFlow > > translations.") added cookies for Port_Binding, Mac_Binding, > > Multicast_Group, Chassis records to the OpenfFlow entries they > > generate. > > > > Add support for parsing such cookies in ovn-detrace too. > > Also, refactor ovn-detrace to allow a more generic way of defining > > cookie handlers. > > > > Signed-off-by: Dumitru Ceara <dceara@redhat.com> > > --- > > utilities/ovn-detrace.in | 166 ++++++++++++++++++++++++++++++++-------------- > > 1 file changed, 117 insertions(+), 49 deletions(-) > > > > diff --git a/utilities/ovn-detrace.in b/utilities/ovn-detrace.in > > index 34b6b0e..79c6d3b 100755 > > --- a/utilities/ovn-detrace.in > > +++ b/utilities/ovn-detrace.in > > @@ -98,48 +98,112 @@ class OVSDB(object): > > def find_row_by_partial_uuid(self, table_name, value): > > return self._find_row(table_name, lambda row: value in str(row.uuid)) > > (Note: this problem existed prior to the patch series) > > Since the cookie corresponds to the beginning of the southbound record's > UUID, this lambda should probably be altered to: > > lambda row: str(row.uuid).startswith(value) > > The existing lambda could match the cookie with a later part of the > UUID, returning an invalid match. > > > > > - > > -def get_lflow_from_cookie(ovnsb_db, cookie): > > - return ovnsb_db.find_row_by_partial_uuid('Logical_Flow', cookie) > > - > > - > > -def print_lflow(lflow, prefix): > > - ldp_uuid = lflow.logical_datapath.uuid > > - ldp_name = str(lflow.logical_datapath.external_ids.get('name')) > > - > > - print '%sLogical datapath: "%s" (%s) [%s]' % (prefix, > > - ldp_name, > > - ldp_uuid, > > - lflow.pipeline) > > - print "%sLogical flow: table=%s (%s), priority=%s, " \ > > - "match=(%s), actions=(%s)" % (prefix, > > - lflow.table_id, > > - lflow.external_ids.get('stage-name'), > > - lflow.priority, > > - str(lflow.match).strip('"'), > > - str(lflow.actions).strip('"')) > > - > > - > > -def print_lflow_nb_hint(lflow, prefix, ovnnb_db): > > - external_ids = lflow.external_ids > > - if external_ids.get('stage-name') in ['ls_in_acl', > > - 'ls_out_acl']: > > - acl_hint = external_ids.get('stage-hint') > > - if not acl_hint: > > - return > > - acl = ovnnb_db.find_row_by_partial_uuid('ACL', acl_hint) > > - if not acl: > > +class CookieHandler(object): > > + def __init__(self, db, table): > > + self._db = db > > + self._table = table > > + > > + def get_record(self, cookie): > > + return self._db.find_row_by_partial_uuid(self._table, cookie) > > + > > + def print_record(self, record, prefix): > > + pass > > + > > + def print_hint(self, record, prefix, db): > > + pass > > + > > +def datapath_str(datapath): > > + return '"%s" (%s)' % (str(datapath.external_ids.get('name')), > > + datapath.uuid) > > + > > +def chassis_str(chassis): > > + if len(chassis) == 0: > > + return '' > > + ch = chassis[0] > > + return 'chassis-name "%s", chassis-str "%s"' % (ch.name, ch.hostname) > > + > > +class LogicalFlowHandler(CookieHandler): > > + def __init__(self, ovnsb_db): > > + super(LogicalFlowHandler, self).__init__(ovnsb_db, 'Logical_Flow') > > + > > + def print_record(self, lflow, prefix): > > + print('%sLogical datapath: %s [%s]' % > > + (prefix, datapath_str(lflow.logical_datapath), lflow.pipeline)) > > + print('%sLogical flow: table=%s (%s), priority=%s, ' > > + 'match=(%s), actions=(%s)' % > > + (prefix, lflow.table_id, lflow.external_ids.get('stage-name'), > > + lflow.priority, > > + str(lflow.match).strip('"'), > > + str(lflow.actions).strip('"'))) > > + > > + def print_hint(self, lflow, prefix, ovnnb_db): > > + external_ids = lflow.external_ids > > + if external_ids.get('stage-name') in ['ls_in_acl', > > + 'ls_out_acl']: > > + acl_hint = external_ids.get('stage-hint') > > + if not acl_hint: > > + return > > + acl = ovnnb_db.find_row_by_partial_uuid('ACL', acl_hint) > > + if not acl: > > + return > > + output = '%sACL: %s, priority=%s, ' \ > > + 'match=(%s), %s' % (prefix, > > + acl.direction, > > + acl.priority, > > + acl.match.strip('"'), > > + acl.action) > > + if acl.log: > > + output += ' (log)' > > + print(output) > > + > > +class PortBindingHandler(CookieHandler): > > + def __init__(self, ovnsb_db): > > + super(PortBindingHandler, self).__init__(ovnsb_db, 'Port_Binding') > > + > > + def print_record(self, pb, prefix): > > + ldp_uuid = pb.datapath.uuid > > + ldp_name = str(pb.datapath.external_ids.get('name')) > > + > > + print('%sLogical datapath: %s' % (prefix, datapath_str(pb.datapath))) > > + print('%sPort Binding: logical_port "%s", tunnel_key %ld, %s' % > > + (prefix, pb.logical_port, pb.tunnel_key, > > + chassis_str(pb.chassis))) > > + > > +class MacBindingHandler(CookieHandler): > > + def __init__(self, ovnsb_db): > > + super(MacBindingHandler, self).__init__(ovnsb_db, 'MAC_Binding') > > + > > + def print_record(self, mb, prefix): > > + print('%sLogical datapath: %s' % (prefix, datapath_str(mb.datapath))) > > + print('%sMAC Binding: ip "%s", logical_port "%s", mac "%s"' % > > + (prefix, mb.ip, mb.logical_port, mb.mac)) > > + > > +class MulticastGroupHandler(CookieHandler): > > + def __init__(self, ovnsb_db): > > + super(MulticastGroupHandler, self).__init__(ovnsb_db, > > + 'Multicast_Group') > > + > > + def print_record(self, mc, prefix): > > + mc_ports = ', '.join([pb.logical_port for pb in mc.ports]) > > + > > + print('%sLogical datapath: %s' % (prefix, datapath_str(mc.datapath))) > > + print('%sMulticast Group: name "%s", tunnel_key %ld ports: (%s)' % > > + (prefix, mc.name, mc.tunnel_key, mc_ports)) > > + > > +class ChassisHandler(CookieHandler): > > + def __init__(self, ovnsb_db): > > + super(ChassisHandler, self).__init__(ovnsb_db, 'Chassis') > > + > > + def print_record(self, chassis, prefix): > > + print('%sChassis: %s' % (prefix, chassis_str([chassis]))) > > + > > +def print_sb_record_from_cookie(ovnnb_db, ovnsb_db, cookie_handlers, cookie): > > + for handler in cookie_handlers: > > + sb_record = handler.get_record(cookie) > > (Note: this problem also existed before this patch series) > > handler.get_record() uses a generator expression to retrieve one of > several potential matches for the cookie. If there are multiple partial > matches in the database, then each subsequent call to > handler.get_record() will return the next row with the same partial match. > > Perhaps it is a good idea to call handler.get_record() until it returns > None each time. This way, if there is potential ambiguity we can at > least present to the user that they're hitting one of several possible > matching southbound database entries. > > > + if sb_record: > > + handler.print_record(sb_record, " * ") > > + handler.print_hint(sb_record, " * ", ovnnb_db) > > return > > - output = "%sACL: %s, priority=%s, " \ > > - "match=(%s), %s" % (prefix, > > - acl.direction, > > - acl.priority, > > - acl.match.strip('"'), > > - acl.action) > > - if acl.log: > > - output += ' (log)' > > - print output > > - > > > > def main(): > > try: > > @@ -183,6 +247,14 @@ def main(): > > ovsdb_ovnsb = OVSDB(ovnsb_db, 'OVN_Southbound') > > ovsdb_ovnnb = OVSDB(ovnnb_db, 'OVN_Northbound') > > > > + cookie_handlers = [ > > + LogicalFlowHandler(ovsdb_ovnsb), > > + PortBindingHandler(ovsdb_ovnsb), > > + MacBindingHandler(ovsdb_ovnsb), > > + MulticastGroupHandler(ovsdb_ovnsb), > > + ChassisHandler(ovsdb_ovnsb) > > + ] > > + > > regex_cookie = re.compile(r'^.*cookie 0x([0-9a-fA-F]+)') > > regex_table_id = re.compile(r'^[0-9]+\.') > > cookie = None > > @@ -194,14 +266,10 @@ def main(): > > > > line = line.strip() > > > > - if cookie: > > - # print lflow info when the current flow block ends > > - if regex_table_id.match(line): > > - lflow = get_lflow_from_cookie(ovsdb_ovnsb, cookie) > > - if lflow: > > - print_lflow(lflow, " * ") > > - print_lflow_nb_hint(lflow, " * ", ovsdb_ovnnb) > > - cookie = None > > + # Print SB record info when the current flow block ends. > > + if cookie and regex_table_id.match(line): > > + print_sb_record_from_cookie(ovsdb_ovnnb, ovsdb_ovnsb, > > + cookie_handlers, cookie) > > > > print line > > > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > >
On Tue, Nov 12, 2019 at 5:21 PM Dumitru Ceara <dceara@redhat.com> wrote: > > On Mon, Nov 11, 2019 at 8:02 PM Mark Michelson <mmichels@redhat.com> wrote: > > > > Hi Dumitru. Everything you've done looks good to me. > > > > However, when reviewing the patch, I educated myself on how ovn-detrace > > is implemented and I found a couple of problems. These problems aren't > > introduced by you. However, I think that since you have broadened the > > scope of the southbound database search area, they should probably be > > addressed. > > Hi Mark, > > Thanks for looking into this. > > I addressed your remarks and sent a v2: > https://patchwork.ozlabs.org/project/openvswitch/list/?series=142383 > > I also refactored a bit more the printing so we don't have to > explicitly specify the 'prefix' every time. Sorry for the noise.. I had forgotten a stray "%s" in v2. Fixed in v3: https://patchwork.ozlabs.org/project/openvswitch/list/?series=142396 Thanks, Dumitru > > Thanks, > Dumitru > > > > > On 11/8/19 6:15 AM, Dumitru Ceara wrote: > > > Commit eb25a7da639e ("Improve debuggability of OVN to OpenFlow > > > translations.") added cookies for Port_Binding, Mac_Binding, > > > Multicast_Group, Chassis records to the OpenfFlow entries they > > > generate. > > > > > > Add support for parsing such cookies in ovn-detrace too. > > > Also, refactor ovn-detrace to allow a more generic way of defining > > > cookie handlers. > > > > > > Signed-off-by: Dumitru Ceara <dceara@redhat.com> > > > --- > > > utilities/ovn-detrace.in | 166 ++++++++++++++++++++++++++++++++-------------- > > > 1 file changed, 117 insertions(+), 49 deletions(-) > > > > > > diff --git a/utilities/ovn-detrace.in b/utilities/ovn-detrace.in > > > index 34b6b0e..79c6d3b 100755 > > > --- a/utilities/ovn-detrace.in > > > +++ b/utilities/ovn-detrace.in > > > @@ -98,48 +98,112 @@ class OVSDB(object): > > > def find_row_by_partial_uuid(self, table_name, value): > > > return self._find_row(table_name, lambda row: value in str(row.uuid)) > > > > (Note: this problem existed prior to the patch series) > > > > Since the cookie corresponds to the beginning of the southbound record's > > UUID, this lambda should probably be altered to: > > > > lambda row: str(row.uuid).startswith(value) > > > > The existing lambda could match the cookie with a later part of the > > UUID, returning an invalid match. > > > > > > > > - > > > -def get_lflow_from_cookie(ovnsb_db, cookie): > > > - return ovnsb_db.find_row_by_partial_uuid('Logical_Flow', cookie) > > > - > > > - > > > -def print_lflow(lflow, prefix): > > > - ldp_uuid = lflow.logical_datapath.uuid > > > - ldp_name = str(lflow.logical_datapath.external_ids.get('name')) > > > - > > > - print '%sLogical datapath: "%s" (%s) [%s]' % (prefix, > > > - ldp_name, > > > - ldp_uuid, > > > - lflow.pipeline) > > > - print "%sLogical flow: table=%s (%s), priority=%s, " \ > > > - "match=(%s), actions=(%s)" % (prefix, > > > - lflow.table_id, > > > - lflow.external_ids.get('stage-name'), > > > - lflow.priority, > > > - str(lflow.match).strip('"'), > > > - str(lflow.actions).strip('"')) > > > - > > > - > > > -def print_lflow_nb_hint(lflow, prefix, ovnnb_db): > > > - external_ids = lflow.external_ids > > > - if external_ids.get('stage-name') in ['ls_in_acl', > > > - 'ls_out_acl']: > > > - acl_hint = external_ids.get('stage-hint') > > > - if not acl_hint: > > > - return > > > - acl = ovnnb_db.find_row_by_partial_uuid('ACL', acl_hint) > > > - if not acl: > > > +class CookieHandler(object): > > > + def __init__(self, db, table): > > > + self._db = db > > > + self._table = table > > > + > > > + def get_record(self, cookie): > > > + return self._db.find_row_by_partial_uuid(self._table, cookie) > > > + > > > + def print_record(self, record, prefix): > > > + pass > > > + > > > + def print_hint(self, record, prefix, db): > > > + pass > > > + > > > +def datapath_str(datapath): > > > + return '"%s" (%s)' % (str(datapath.external_ids.get('name')), > > > + datapath.uuid) > > > + > > > +def chassis_str(chassis): > > > + if len(chassis) == 0: > > > + return '' > > > + ch = chassis[0] > > > + return 'chassis-name "%s", chassis-str "%s"' % (ch.name, ch.hostname) > > > + > > > +class LogicalFlowHandler(CookieHandler): > > > + def __init__(self, ovnsb_db): > > > + super(LogicalFlowHandler, self).__init__(ovnsb_db, 'Logical_Flow') > > > + > > > + def print_record(self, lflow, prefix): > > > + print('%sLogical datapath: %s [%s]' % > > > + (prefix, datapath_str(lflow.logical_datapath), lflow.pipeline)) > > > + print('%sLogical flow: table=%s (%s), priority=%s, ' > > > + 'match=(%s), actions=(%s)' % > > > + (prefix, lflow.table_id, lflow.external_ids.get('stage-name'), > > > + lflow.priority, > > > + str(lflow.match).strip('"'), > > > + str(lflow.actions).strip('"'))) > > > + > > > + def print_hint(self, lflow, prefix, ovnnb_db): > > > + external_ids = lflow.external_ids > > > + if external_ids.get('stage-name') in ['ls_in_acl', > > > + 'ls_out_acl']: > > > + acl_hint = external_ids.get('stage-hint') > > > + if not acl_hint: > > > + return > > > + acl = ovnnb_db.find_row_by_partial_uuid('ACL', acl_hint) > > > + if not acl: > > > + return > > > + output = '%sACL: %s, priority=%s, ' \ > > > + 'match=(%s), %s' % (prefix, > > > + acl.direction, > > > + acl.priority, > > > + acl.match.strip('"'), > > > + acl.action) > > > + if acl.log: > > > + output += ' (log)' > > > + print(output) > > > + > > > +class PortBindingHandler(CookieHandler): > > > + def __init__(self, ovnsb_db): > > > + super(PortBindingHandler, self).__init__(ovnsb_db, 'Port_Binding') > > > + > > > + def print_record(self, pb, prefix): > > > + ldp_uuid = pb.datapath.uuid > > > + ldp_name = str(pb.datapath.external_ids.get('name')) > > > + > > > + print('%sLogical datapath: %s' % (prefix, datapath_str(pb.datapath))) > > > + print('%sPort Binding: logical_port "%s", tunnel_key %ld, %s' % > > > + (prefix, pb.logical_port, pb.tunnel_key, > > > + chassis_str(pb.chassis))) > > > + > > > +class MacBindingHandler(CookieHandler): > > > + def __init__(self, ovnsb_db): > > > + super(MacBindingHandler, self).__init__(ovnsb_db, 'MAC_Binding') > > > + > > > + def print_record(self, mb, prefix): > > > + print('%sLogical datapath: %s' % (prefix, datapath_str(mb.datapath))) > > > + print('%sMAC Binding: ip "%s", logical_port "%s", mac "%s"' % > > > + (prefix, mb.ip, mb.logical_port, mb.mac)) > > > + > > > +class MulticastGroupHandler(CookieHandler): > > > + def __init__(self, ovnsb_db): > > > + super(MulticastGroupHandler, self).__init__(ovnsb_db, > > > + 'Multicast_Group') > > > + > > > + def print_record(self, mc, prefix): > > > + mc_ports = ', '.join([pb.logical_port for pb in mc.ports]) > > > + > > > + print('%sLogical datapath: %s' % (prefix, datapath_str(mc.datapath))) > > > + print('%sMulticast Group: name "%s", tunnel_key %ld ports: (%s)' % > > > + (prefix, mc.name, mc.tunnel_key, mc_ports)) > > > + > > > +class ChassisHandler(CookieHandler): > > > + def __init__(self, ovnsb_db): > > > + super(ChassisHandler, self).__init__(ovnsb_db, 'Chassis') > > > + > > > + def print_record(self, chassis, prefix): > > > + print('%sChassis: %s' % (prefix, chassis_str([chassis]))) > > > + > > > +def print_sb_record_from_cookie(ovnnb_db, ovnsb_db, cookie_handlers, cookie): > > > + for handler in cookie_handlers: > > > + sb_record = handler.get_record(cookie) > > > > (Note: this problem also existed before this patch series) > > > > handler.get_record() uses a generator expression to retrieve one of > > several potential matches for the cookie. If there are multiple partial > > matches in the database, then each subsequent call to > > handler.get_record() will return the next row with the same partial match. > > > > Perhaps it is a good idea to call handler.get_record() until it returns > > None each time. This way, if there is potential ambiguity we can at > > least present to the user that they're hitting one of several possible > > matching southbound database entries. > > > > > + if sb_record: > > > + handler.print_record(sb_record, " * ") > > > + handler.print_hint(sb_record, " * ", ovnnb_db) > > > return > > > - output = "%sACL: %s, priority=%s, " \ > > > - "match=(%s), %s" % (prefix, > > > - acl.direction, > > > - acl.priority, > > > - acl.match.strip('"'), > > > - acl.action) > > > - if acl.log: > > > - output += ' (log)' > > > - print output > > > - > > > > > > def main(): > > > try: > > > @@ -183,6 +247,14 @@ def main(): > > > ovsdb_ovnsb = OVSDB(ovnsb_db, 'OVN_Southbound') > > > ovsdb_ovnnb = OVSDB(ovnnb_db, 'OVN_Northbound') > > > > > > + cookie_handlers = [ > > > + LogicalFlowHandler(ovsdb_ovnsb), > > > + PortBindingHandler(ovsdb_ovnsb), > > > + MacBindingHandler(ovsdb_ovnsb), > > > + MulticastGroupHandler(ovsdb_ovnsb), > > > + ChassisHandler(ovsdb_ovnsb) > > > + ] > > > + > > > regex_cookie = re.compile(r'^.*cookie 0x([0-9a-fA-F]+)') > > > regex_table_id = re.compile(r'^[0-9]+\.') > > > cookie = None > > > @@ -194,14 +266,10 @@ def main(): > > > > > > line = line.strip() > > > > > > - if cookie: > > > - # print lflow info when the current flow block ends > > > - if regex_table_id.match(line): > > > - lflow = get_lflow_from_cookie(ovsdb_ovnsb, cookie) > > > - if lflow: > > > - print_lflow(lflow, " * ") > > > - print_lflow_nb_hint(lflow, " * ", ovsdb_ovnnb) > > > - cookie = None > > > + # Print SB record info when the current flow block ends. > > > + if cookie and regex_table_id.match(line): > > > + print_sb_record_from_cookie(ovsdb_ovnnb, ovsdb_ovnsb, > > > + cookie_handlers, cookie) > > > > > > print line > > > > > > > > > _______________________________________________ > > > dev mailing list > > > dev@openvswitch.org > > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > > >
diff --git a/utilities/ovn-detrace.in b/utilities/ovn-detrace.in index 34b6b0e..79c6d3b 100755 --- a/utilities/ovn-detrace.in +++ b/utilities/ovn-detrace.in @@ -98,48 +98,112 @@ class OVSDB(object): def find_row_by_partial_uuid(self, table_name, value): return self._find_row(table_name, lambda row: value in str(row.uuid)) - -def get_lflow_from_cookie(ovnsb_db, cookie): - return ovnsb_db.find_row_by_partial_uuid('Logical_Flow', cookie) - - -def print_lflow(lflow, prefix): - ldp_uuid = lflow.logical_datapath.uuid - ldp_name = str(lflow.logical_datapath.external_ids.get('name')) - - print '%sLogical datapath: "%s" (%s) [%s]' % (prefix, - ldp_name, - ldp_uuid, - lflow.pipeline) - print "%sLogical flow: table=%s (%s), priority=%s, " \ - "match=(%s), actions=(%s)" % (prefix, - lflow.table_id, - lflow.external_ids.get('stage-name'), - lflow.priority, - str(lflow.match).strip('"'), - str(lflow.actions).strip('"')) - - -def print_lflow_nb_hint(lflow, prefix, ovnnb_db): - external_ids = lflow.external_ids - if external_ids.get('stage-name') in ['ls_in_acl', - 'ls_out_acl']: - acl_hint = external_ids.get('stage-hint') - if not acl_hint: - return - acl = ovnnb_db.find_row_by_partial_uuid('ACL', acl_hint) - if not acl: +class CookieHandler(object): + def __init__(self, db, table): + self._db = db + self._table = table + + def get_record(self, cookie): + return self._db.find_row_by_partial_uuid(self._table, cookie) + + def print_record(self, record, prefix): + pass + + def print_hint(self, record, prefix, db): + pass + +def datapath_str(datapath): + return '"%s" (%s)' % (str(datapath.external_ids.get('name')), + datapath.uuid) + +def chassis_str(chassis): + if len(chassis) == 0: + return '' + ch = chassis[0] + return 'chassis-name "%s", chassis-str "%s"' % (ch.name, ch.hostname) + +class LogicalFlowHandler(CookieHandler): + def __init__(self, ovnsb_db): + super(LogicalFlowHandler, self).__init__(ovnsb_db, 'Logical_Flow') + + def print_record(self, lflow, prefix): + print('%sLogical datapath: %s [%s]' % + (prefix, datapath_str(lflow.logical_datapath), lflow.pipeline)) + print('%sLogical flow: table=%s (%s), priority=%s, ' + 'match=(%s), actions=(%s)' % + (prefix, lflow.table_id, lflow.external_ids.get('stage-name'), + lflow.priority, + str(lflow.match).strip('"'), + str(lflow.actions).strip('"'))) + + def print_hint(self, lflow, prefix, ovnnb_db): + external_ids = lflow.external_ids + if external_ids.get('stage-name') in ['ls_in_acl', + 'ls_out_acl']: + acl_hint = external_ids.get('stage-hint') + if not acl_hint: + return + acl = ovnnb_db.find_row_by_partial_uuid('ACL', acl_hint) + if not acl: + return + output = '%sACL: %s, priority=%s, ' \ + 'match=(%s), %s' % (prefix, + acl.direction, + acl.priority, + acl.match.strip('"'), + acl.action) + if acl.log: + output += ' (log)' + print(output) + +class PortBindingHandler(CookieHandler): + def __init__(self, ovnsb_db): + super(PortBindingHandler, self).__init__(ovnsb_db, 'Port_Binding') + + def print_record(self, pb, prefix): + ldp_uuid = pb.datapath.uuid + ldp_name = str(pb.datapath.external_ids.get('name')) + + print('%sLogical datapath: %s' % (prefix, datapath_str(pb.datapath))) + print('%sPort Binding: logical_port "%s", tunnel_key %ld, %s' % + (prefix, pb.logical_port, pb.tunnel_key, + chassis_str(pb.chassis))) + +class MacBindingHandler(CookieHandler): + def __init__(self, ovnsb_db): + super(MacBindingHandler, self).__init__(ovnsb_db, 'MAC_Binding') + + def print_record(self, mb, prefix): + print('%sLogical datapath: %s' % (prefix, datapath_str(mb.datapath))) + print('%sMAC Binding: ip "%s", logical_port "%s", mac "%s"' % + (prefix, mb.ip, mb.logical_port, mb.mac)) + +class MulticastGroupHandler(CookieHandler): + def __init__(self, ovnsb_db): + super(MulticastGroupHandler, self).__init__(ovnsb_db, + 'Multicast_Group') + + def print_record(self, mc, prefix): + mc_ports = ', '.join([pb.logical_port for pb in mc.ports]) + + print('%sLogical datapath: %s' % (prefix, datapath_str(mc.datapath))) + print('%sMulticast Group: name "%s", tunnel_key %ld ports: (%s)' % + (prefix, mc.name, mc.tunnel_key, mc_ports)) + +class ChassisHandler(CookieHandler): + def __init__(self, ovnsb_db): + super(ChassisHandler, self).__init__(ovnsb_db, 'Chassis') + + def print_record(self, chassis, prefix): + print('%sChassis: %s' % (prefix, chassis_str([chassis]))) + +def print_sb_record_from_cookie(ovnnb_db, ovnsb_db, cookie_handlers, cookie): + for handler in cookie_handlers: + sb_record = handler.get_record(cookie) + if sb_record: + handler.print_record(sb_record, " * ") + handler.print_hint(sb_record, " * ", ovnnb_db) return - output = "%sACL: %s, priority=%s, " \ - "match=(%s), %s" % (prefix, - acl.direction, - acl.priority, - acl.match.strip('"'), - acl.action) - if acl.log: - output += ' (log)' - print output - def main(): try: @@ -183,6 +247,14 @@ def main(): ovsdb_ovnsb = OVSDB(ovnsb_db, 'OVN_Southbound') ovsdb_ovnnb = OVSDB(ovnnb_db, 'OVN_Northbound') + cookie_handlers = [ + LogicalFlowHandler(ovsdb_ovnsb), + PortBindingHandler(ovsdb_ovnsb), + MacBindingHandler(ovsdb_ovnsb), + MulticastGroupHandler(ovsdb_ovnsb), + ChassisHandler(ovsdb_ovnsb) + ] + regex_cookie = re.compile(r'^.*cookie 0x([0-9a-fA-F]+)') regex_table_id = re.compile(r'^[0-9]+\.') cookie = None @@ -194,14 +266,10 @@ def main(): line = line.strip() - if cookie: - # print lflow info when the current flow block ends - if regex_table_id.match(line): - lflow = get_lflow_from_cookie(ovsdb_ovnsb, cookie) - if lflow: - print_lflow(lflow, " * ") - print_lflow_nb_hint(lflow, " * ", ovsdb_ovnnb) - cookie = None + # Print SB record info when the current flow block ends. + if cookie and regex_table_id.match(line): + print_sb_record_from_cookie(ovsdb_ovnnb, ovsdb_ovnsb, + cookie_handlers, cookie) print line
Commit eb25a7da639e ("Improve debuggability of OVN to OpenFlow translations.") added cookies for Port_Binding, Mac_Binding, Multicast_Group, Chassis records to the OpenfFlow entries they generate. Add support for parsing such cookies in ovn-detrace too. Also, refactor ovn-detrace to allow a more generic way of defining cookie handlers. Signed-off-by: Dumitru Ceara <dceara@redhat.com> --- utilities/ovn-detrace.in | 166 ++++++++++++++++++++++++++++++++-------------- 1 file changed, 117 insertions(+), 49 deletions(-)