[ovs-dev] [PATCH ovn 3/3] ovn-detrace: Add support for other types of SB cookies.
Dumitru Ceara
dceara at redhat.com
Tue Nov 12 16:50:36 UTC 2019
On Tue, Nov 12, 2019 at 5:21 PM Dumitru Ceara <dceara at redhat.com> wrote:
>
> On Mon, Nov 11, 2019 at 8:02 PM Mark Michelson <mmichels at 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 at 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 at openvswitch.org
> > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> > >
> >
More information about the dev
mailing list