| Message ID | 20201023080719.21492.49223.stgit@dceara.remote.csb |
|---|---|
| State | Accepted |
| Headers | show |
| Series | ovn-detrace: Fix OVS interface decoding and error reporting. | expand |
On Fri, Oct 23, 2020 at 1:37 PM Dumitru Ceara <dceara@redhat.com> wrote: > > Do not assume 'ofport' is unique for all OVS interfaces in the system. This > is true only for interfaces within the same OVS bridge. Also, only decode > br-int related interfaces. > > Also, fix printing of potential duplicate UUIDs decoded from cookies. > > Reported-by: Michael Cambria <mcambria@redhat.com> > Reported-at: https://bugzilla.redhat.com/1890803 > Fixes: 8051499a6c1b ("ovn-detrace: Add support for other types of SB cookies.") > Signed-off-by: Dumitru Ceara <dceara@redhat.com> Thanks Dumitru. The patches LGTM. I applied both the patches in this series to master. Numan > --- > utilities/ovn-detrace.in | 35 ++++++++++++++++++++++++++++++----- > 1 file changed, 30 insertions(+), 5 deletions(-) > > diff --git a/utilities/ovn-detrace.in b/utilities/ovn-detrace.in > index 4f8dd5f..2344f52 100755 > --- a/utilities/ovn-detrace.in > +++ b/utilities/ovn-detrace.in > @@ -117,18 +117,27 @@ class OVSDB(object): > def _find_rows(self, table_name, find_fn): > return filter(find_fn, self.get_table(table_name).rows.values()) > > - def _find_rows_by_name(self, table_name, value): > + def find_rows_by_name(self, table_name, value): > return self._find_rows(table_name, lambda row: row.name == value) > > def find_rows_by_partial_uuid(self, table_name, value): > return self._find_rows(table_name, > lambda row: str(row.uuid).startswith(value)) > > + def get_first_record(self, table_name): > + table_rows = self.get_table(table_name).rows.values() > + if len(table_rows) == 0: > + return None > + return next(iter(table_rows)) > + > class CookieHandler(object): > def __init__(self, db, table): > self._db = db > self._table = table > > + def print(self, msg): > + print_h(msg) > + > def get_records(self, cookie): > return [] > > @@ -320,10 +329,25 @@ class OvsInterfaceHandler(CookieHandler): > def __init__(self, ovs_db): > super(OvsInterfaceHandler, self).__init__(ovs_db, 'Interface') > > + # Store the interfaces connected to the integration bridge in a dict > + # indexed by ofport. > + br = self.get_br_int() > + self._intfs = { > + i.ofport[0] : i for p in br.ports > + for i in p.interfaces if len(i.ofport) > 0 > + } > + > + def get_br_int(self): > + ovsrec = self._db.get_first_record('Open_vSwitch') > + if ovsrec: > + br_name = ovsrec.external_ids.get('ovn-bridge', 'br-int') > + else: > + br_name = 'br-int' > + return next(iter(self._db.find_rows_by_name('Bridge', br_name))) > + > def get_records(self, ofport): > - return self._db._find_rows(self._table, > - lambda intf: len(intf.ofport) > 0 and > - str(intf.ofport[0]) == ofport) > + intf = self._intfs.get(int(ofport)) > + return [intf] if intf else [] > > def print_record(self, intf): > print_p('OVS Interface: %s (%s)' % > @@ -331,7 +355,8 @@ class OvsInterfaceHandler(CookieHandler): > > def print_record_from_cookie(ovnnb_db, cookie_handlers, cookie): > for handler in cookie_handlers: > - for i, record in enumerate(handler.get_records(cookie)): > + records = list(handler.get_records(cookie)) > + for i, record in enumerate(records): > if i > 0: > handler.print('[Duplicate uuid cookie]') > handler.print_record(record) > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On 10/30/20 10:17 AM, Numan Siddique wrote: > On Fri, Oct 23, 2020 at 1:37 PM Dumitru Ceara <dceara@redhat.com> wrote: >> >> Do not assume 'ofport' is unique for all OVS interfaces in the system. This >> is true only for interfaces within the same OVS bridge. Also, only decode >> br-int related interfaces. >> >> Also, fix printing of potential duplicate UUIDs decoded from cookies. >> >> Reported-by: Michael Cambria <mcambria@redhat.com> >> Reported-at: https://bugzilla.redhat.com/1890803 >> Fixes: 8051499a6c1b ("ovn-detrace: Add support for other types of SB cookies.") >> Signed-off-by: Dumitru Ceara <dceara@redhat.com> > > Thanks Dumitru. > > The patches LGTM. I applied both the patches in this series to master. > > Numan > Thanks!
On 10/30/20 10:26 AM, Dumitru Ceara wrote: > On 10/30/20 10:17 AM, Numan Siddique wrote: >> On Fri, Oct 23, 2020 at 1:37 PM Dumitru Ceara <dceara@redhat.com> wrote: >>> >>> Do not assume 'ofport' is unique for all OVS interfaces in the system. This >>> is true only for interfaces within the same OVS bridge. Also, only decode >>> br-int related interfaces. >>> >>> Also, fix printing of potential duplicate UUIDs decoded from cookies. >>> >>> Reported-by: Michael Cambria <mcambria@redhat.com> >>> Reported-at: https://bugzilla.redhat.com/1890803 >>> Fixes: 8051499a6c1b ("ovn-detrace: Add support for other types of SB cookies.") >>> Signed-off-by: Dumitru Ceara <dceara@redhat.com> >> >> Thanks Dumitru. >> >> The patches LGTM. I applied both the patches in this series to master. >> >> Numan >> > > Thanks! > Actually, would it also be possible to backport this series to stable branches as it's a bug fix? Patch 1/2 could go down to branch-20.03. Patch 2/2 could go down to branch-20.09. Thanks, Dumitru
On Fri, Oct 30, 2020 at 3:32 PM Dumitru Ceara <dceara@redhat.com> wrote: > > On 10/30/20 10:26 AM, Dumitru Ceara wrote: > > On 10/30/20 10:17 AM, Numan Siddique wrote: > >> On Fri, Oct 23, 2020 at 1:37 PM Dumitru Ceara <dceara@redhat.com> wrote: > >>> > >>> Do not assume 'ofport' is unique for all OVS interfaces in the system. This > >>> is true only for interfaces within the same OVS bridge. Also, only decode > >>> br-int related interfaces. > >>> > >>> Also, fix printing of potential duplicate UUIDs decoded from cookies. > >>> > >>> Reported-by: Michael Cambria <mcambria@redhat.com> > >>> Reported-at: https://bugzilla.redhat.com/1890803 > >>> Fixes: 8051499a6c1b ("ovn-detrace: Add support for other types of SB cookies.") > >>> Signed-off-by: Dumitru Ceara <dceara@redhat.com> > >> > >> Thanks Dumitru. > >> > >> The patches LGTM. I applied both the patches in this series to master. > >> > >> Numan > >> > > > > Thanks! > > > > Actually, would it also be possible to backport this series to stable > branches as it's a bug fix? > > Patch 1/2 could go down to branch-20.03. > Patch 2/2 could go down to branch-20.09. Done. Thanks Numan > > Thanks, > Dumitru > > _______________________________________________ > 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 4f8dd5f..2344f52 100755 --- a/utilities/ovn-detrace.in +++ b/utilities/ovn-detrace.in @@ -117,18 +117,27 @@ class OVSDB(object): def _find_rows(self, table_name, find_fn): return filter(find_fn, self.get_table(table_name).rows.values()) - def _find_rows_by_name(self, table_name, value): + def find_rows_by_name(self, table_name, value): return self._find_rows(table_name, lambda row: row.name == value) def find_rows_by_partial_uuid(self, table_name, value): return self._find_rows(table_name, lambda row: str(row.uuid).startswith(value)) + def get_first_record(self, table_name): + table_rows = self.get_table(table_name).rows.values() + if len(table_rows) == 0: + return None + return next(iter(table_rows)) + class CookieHandler(object): def __init__(self, db, table): self._db = db self._table = table + def print(self, msg): + print_h(msg) + def get_records(self, cookie): return [] @@ -320,10 +329,25 @@ class OvsInterfaceHandler(CookieHandler): def __init__(self, ovs_db): super(OvsInterfaceHandler, self).__init__(ovs_db, 'Interface') + # Store the interfaces connected to the integration bridge in a dict + # indexed by ofport. + br = self.get_br_int() + self._intfs = { + i.ofport[0] : i for p in br.ports + for i in p.interfaces if len(i.ofport) > 0 + } + + def get_br_int(self): + ovsrec = self._db.get_first_record('Open_vSwitch') + if ovsrec: + br_name = ovsrec.external_ids.get('ovn-bridge', 'br-int') + else: + br_name = 'br-int' + return next(iter(self._db.find_rows_by_name('Bridge', br_name))) + def get_records(self, ofport): - return self._db._find_rows(self._table, - lambda intf: len(intf.ofport) > 0 and - str(intf.ofport[0]) == ofport) + intf = self._intfs.get(int(ofport)) + return [intf] if intf else [] def print_record(self, intf): print_p('OVS Interface: %s (%s)' % @@ -331,7 +355,8 @@ class OvsInterfaceHandler(CookieHandler): def print_record_from_cookie(ovnnb_db, cookie_handlers, cookie): for handler in cookie_handlers: - for i, record in enumerate(handler.get_records(cookie)): + records = list(handler.get_records(cookie)) + for i, record in enumerate(records): if i > 0: handler.print('[Duplicate uuid cookie]') handler.print_record(record)
Do not assume 'ofport' is unique for all OVS interfaces in the system. This is true only for interfaces within the same OVS bridge. Also, only decode br-int related interfaces. Also, fix printing of potential duplicate UUIDs decoded from cookies. Reported-by: Michael Cambria <mcambria@redhat.com> Reported-at: https://bugzilla.redhat.com/1890803 Fixes: 8051499a6c1b ("ovn-detrace: Add support for other types of SB cookies.") Signed-off-by: Dumitru Ceara <dceara@redhat.com> --- utilities/ovn-detrace.in | 35 ++++++++++++++++++++++++++++++----- 1 file changed, 30 insertions(+), 5 deletions(-)