On 5/27/24 20:31, Roberto Bartzen Acosta wrote: > Thanks Ilya! > > Em seg., 27 de mai. de 2024 às 10:28, Ilya Maximets <i.maxim...@ovn.org > <mailto:i.maxim...@ovn.org>> escreveu: > > On 5/27/24 18:22, Roberto Bartzen Acosta via discuss wrote: > > Hello everyone! > > > > I found an issue during testing with the OVN-IC daemon. I noticed that > applications that interact with python-idl show an error when deleting > datapath_binding when the table index changes. > > > > We can see the error in the logs below: > > > > 2024-05-27 09:49:38.779 23596 ERROR ovsdbapp.backend.ovs_idl.connection > [-] Datapath_Binding(uuid=UUID('498e66a2-70bc-4587-a66f-0433baf82f60'), > tunnel_key=16711683, load_balancers=[], external_ids={}) not in list: > ValueError: > Datapath_Binding(uuid=UUID('498e66a2-70bc-4587-a66f-0433baf82f60'), > tunnel_key=16711683, load_balancers=[], external_ids={}) not in list > > 2024-05-27 09:49:38.779 23596 ERROR ovsdbapp.backend.ovs_idl.connection > Traceback (most recent call last): > > 2024-05-27 09:49:38.779 23596 ERROR ovsdbapp.backend.ovs_idl.connection > File > "/usr/lib/python3/dist-packages/ovsdbapp/backend/ovs_idl/connection.py", line > 110, in run > > 2024-05-27 09:49:38.779 23596 ERROR ovsdbapp.backend.ovs_idl.connection > self.idl.run() > > 2024-05-27 09:49:38.779 23596 ERROR ovsdbapp.backend.ovs_idl.connection > File "/usr/lib/python3/dist-packages/ovs/db/idl.py", line 465, in run > > 2024-05-27 09:49:38.779 23596 ERROR ovsdbapp.backend.ovs_idl.connection > self.__parse_update(msg.params[2], OVSDB_UPDATE3) > > 2024-05-27 09:49:38.779 23596 ERROR ovsdbapp.backend.ovs_idl.connection > File "/usr/lib/python3/dist-packages/ovs/db/idl.py", line 924, in > __parse_update > > 2024-05-27 09:49:38.779 23596 ERROR ovsdbapp.backend.ovs_idl.connection > self.__do_parse_update(update, version, self.tables) > > 2024-05-27 09:49:38.779 23596 ERROR ovsdbapp.backend.ovs_idl.connection > File "/usr/lib/python3/dist-packages/ovs/db/idl.py", line 964, in > __do_parse_update > > 2024-05-27 09:49:38.779 23596 ERROR ovsdbapp.backend.ovs_idl.connection > changes = self.__process_update2(table, uuid, row_update) > > 2024-05-27 09:49:38.779 23596 ERROR ovsdbapp.backend.ovs_idl.connection > File "/usr/lib/python3/dist-packages/ovs/db/idl.py", line 991, in > __process_update2 > > 2024-05-27 09:49:38.779 23596 ERROR ovsdbapp.backend.ovs_idl.connection > del table.rows[uuid] > > 2024-05-27 09:49:38.779 23596 ERROR ovsdbapp.backend.ovs_idl.connection > File "/usr/lib/python3/dist-packages/ovs/db/custom_index.py", line 102, in > __delitem__ > > 2024-05-27 09:49:38.779 23596 ERROR ovsdbapp.backend.ovs_idl.connection > index.remove(val) > > 2024-05-27 09:49:38.779 23596 ERROR ovsdbapp.backend.ovs_idl.connection > File "/usr/lib/python3/dist-packages/ovs/db/custom_index.py", line 66, in > remove > > 2024-05-27 09:49:38.779 23596 ERROR ovsdbapp.backend.ovs_idl.connection > self.values.remove(self.index_entry_from_row(row)) > > 2024-05-27 09:49:38.779 23596 ERROR ovsdbapp.backend.ovs_idl.connection > File "/usr/lib/python3/dist-packages/sortedcontainers/sortedlist.py", line > 2015, in remove > > 2024-05-27 09:49:38.779 23596 ERROR ovsdbapp.backend.ovs_idl.connection > raise ValueError('{0!r} not in list'.format(value)) > > 2024-05-27 09:49:38.779 23596 ERROR ovsdbapp.backend.ovs_idl.connection > ValueError: > Datapath_Binding(uuid=UUID('498e66a2-70bc-4587-a66f-0433baf82f60'), > tunnel_key=16711683, load_balancers=[], external_ids={}) not in list > > 2024-05-27 09:49:38.779 23596 ERROR ovsdbapp.backend.ovs_idl.connection > > > > > > The complete logs showing the creation, update of the tunnel_key and > deletion of the datapath_binding can be accessed at [1]. > > > > Basically the problem occurs because of the tunnel_key update when we > create a transit-switch. If the table did not have this index the problem > would not be observed. > > > > This issue affects applications such as: neutron-server, > neutron-ovn-metadata-agent, and everyone that monitors the Datapath_Binding > table via python-idl. > > > > I've already talked to Ilya Maximets in the openvswitch IRC channel and > the issue seems to be in the row updating process: python/ovs/db/idl.py. > > > Thanks, Roberto! I beleive the fix I shared on IRC is mostly correct > though > I didn't test it. I need to refine it a little and add a unit test for > the > issue. With that I hope to post it for review somewhere soon. > > > > Just to update on the patch you proposed, the deletion of the entry fails > during the python-idl update execution. The content of the table row has > already been updated just above in the __apply_diff and/or __row_update > methods. If you request to delete the entry... you will delete an entry in > use from what I understand. > > The code below triggers an error when updating tunnel_key (error that didn't > happen before when updating rows):
Yeah, I figured that. :) > > + del table.rows[uuid] > + table.rows[uuid] = row > > > So I suggest just updating the table reference with the already updated row. > What do you think? The setattr method doesn't handle updates, so deletion is necessary. The fenied approach is temporary here: https://github.com/igsilya/ovs/commit/4792aee0dcb97d9aefd182cc3c97865aa501be71 I still need to create a test, so it might change if I find something else. > > $ git diff > diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py > index a80da84e7..08c5acffb 100644 > --- a/python/ovs/db/idl.py > +++ b/python/ovs/db/idl.py > @@ -1014,6 +1014,7 @@ class Idl(object): > raise error.Error('Modify non-existing row') > > old_row = self.__apply_diff(table, row, row_update['modify']) > + table.rows[uuid] = row > return Notice(ROW_UPDATE, row, Row(self, table, uuid, old_row)) > else: > raise error.Error('<row-update> unknown operation', > @@ -1062,6 +1063,7 @@ class Idl(object): > if op == ROW_CREATE: > table.rows[uuid] = row > if changed: > + table.rows[uuid] = row > return Notice(op, row, Row.from_json(self, table, uuid, old)) > return False > > > > CC: Terry. > > Best regards, Ilya Maximets. > > > > > Kind regards, > > Roberto > > > > > > [1] https://paste.openstack.org/show/bqLA7nxfk2C6loD5YlY1/ > <https://paste.openstack.org/show/bqLA7nxfk2C6loD5YlY1/> > > > > /‘Esta mensagem é direcionada apenas para os endereços constantes no > cabeçalho inicial. Se você não está listado nos endereços constantes no > cabeçalho, pedimos-lhe que desconsidere completamente o conteúdo dessa > mensagem e cuja cópia, encaminhamento e/ou execução das ações citadas estão > imediatamente anuladas e proibidas’./ > > / //‘Apesar do Magazine Luiza tomar todas as precauções razoáveis para > assegurar que nenhum vírus esteja presente nesse e-mail, a empresa não poderá > aceitar a responsabilidade por quaisquer perdas ou danos causados por esse > e-mail ou por seus anexos’./ > _______________________________________________ discuss mailing list disc...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-discuss
