)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"f28e6de5badc697fc5ac84254e34a252b29922cc","unresolved":false,"context_lines":[{"line_number":10,"context_line":"the checksum correctly populated. This is necessary when the"},{"line_number":11,"context_line":"OVS datapath is \"netdev\"."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Because the overhead added in minimal and only applies to the"},{"line_number":14,"context_line":"metadata traffic inside the metadata namespace, this rule is"},{"line_number":15,"context_line":"always set."},{"line_number":16,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"1013265b_434231d7","line":13,"updated":"2020-11-30 10:15:15.000000000","message":"nitty nit: s/in/is","commit_id":"3216bd40eb99517ee650f9cfb85c1deca2b4d5d2"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"9bc38478a6b37c1e37fbd214c8d5e8424737f7cd","unresolved":false,"context_lines":[{"line_number":10,"context_line":"the checksum correctly populated. This is necessary when the"},{"line_number":11,"context_line":"OVS datapath is \"netdev\"."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Because the overhead added in minimal and only applies to the"},{"line_number":14,"context_line":"metadata traffic inside the metadata namespace, this rule is"},{"line_number":15,"context_line":"always set."},{"line_number":16,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"ce4204f2_a90b9968","line":13,"in_reply_to":"1013265b_434231d7","updated":"2020-12-01 09:46:27.000000000","message":"done","commit_id":"3216bd40eb99517ee650f9cfb85c1deca2b4d5d2"}],"neutron/agent/ovn/metadata/agent.py":[{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"9c9a931c1b0158e0761bfc0acee20f64f85b0877","unresolved":true,"context_lines":[{"line_number":484,"context_line":"            (\u0027external_ids\u0027, {\u0027iface-id\u0027: port.logical_port})).execute()"},{"line_number":485,"context_line":""},{"line_number":486,"context_line":"        # Ensure the correct checksum in the metadata traffic."},{"line_number":487,"context_line":"        self._ensure_datapath_checksum(namespace)"},{"line_number":488,"context_line":""},{"line_number":489,"context_line":"        # Spawn metadata proxy if it\u0027s not already running."},{"line_number":490,"context_line":"        metadata_driver.MetadataDriver.spawn_monitored_metadata_proxy("}],"source_content_type":"text/x-python","patch_set":8,"id":"84745b5c_8c3a8ca0","line":487,"range":{"start_line":487,"start_character":8,"end_line":487,"end_character":49},"updated":"2020-12-01 10:13:37.000000000","message":"We are connected to br-int, shall we populate a class attribute only when the OVS datapath is \u0027netdev\u0027? I understand there\u0027s not much of a hit if doing it unconditionally but as there is no need, I wonder if we should do it only for DPDK, which is when this is only needed.","commit_id":"30ef59a320f3a89fe31803ceed5075a0cdc8b225"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"4eab1e0fa0f61fc1688a54044cc82121ab5d61ec","unresolved":true,"context_lines":[{"line_number":484,"context_line":"            (\u0027external_ids\u0027, {\u0027iface-id\u0027: port.logical_port})).execute()"},{"line_number":485,"context_line":""},{"line_number":486,"context_line":"        # Ensure the correct checksum in the metadata traffic."},{"line_number":487,"context_line":"        self._ensure_datapath_checksum(namespace)"},{"line_number":488,"context_line":""},{"line_number":489,"context_line":"        # Spawn metadata proxy if it\u0027s not already running."},{"line_number":490,"context_line":"        metadata_driver.MetadataDriver.spawn_monitored_metadata_proxy("}],"source_content_type":"text/x-python","patch_set":8,"id":"eb08b61c_cb2d467f","line":487,"range":{"start_line":487,"start_character":8,"end_line":487,"end_character":49},"in_reply_to":"84745b5c_8c3a8ca0","updated":"2020-12-01 15:05:30.000000000","message":"That\u0027s what I comment in the commit message: the overhead is minimal and works for every datapath type. We don\u0027t need to make this conditional (although I documented it in the method description).","commit_id":"30ef59a320f3a89fe31803ceed5075a0cdc8b225"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"878be8d650308937e8d9e5771094b37ceeb405c1","unresolved":true,"context_lines":[{"line_number":484,"context_line":"            (\u0027external_ids\u0027, {\u0027iface-id\u0027: port.logical_port})).execute()"},{"line_number":485,"context_line":""},{"line_number":486,"context_line":"        # Ensure the correct checksum in the metadata traffic."},{"line_number":487,"context_line":"        self._ensure_datapath_checksum(namespace)"},{"line_number":488,"context_line":""},{"line_number":489,"context_line":"        # Spawn metadata proxy if it\u0027s not already running."},{"line_number":490,"context_line":"        metadata_driver.MetadataDriver.spawn_monitored_metadata_proxy("}],"source_content_type":"text/x-python","patch_set":8,"id":"c208db4f_712702f7","line":487,"range":{"start_line":487,"start_character":8,"end_line":487,"end_character":49},"in_reply_to":"8c35dd4e_5055c619","updated":"2020-12-04 09:04:32.000000000","message":"I understand what you say but it\u0027s probably also that the majority of the setups are non DPDK so a bug or an issue with this checksum fill would affect unnecessarily most of the environments.\n\nEspecially when the cost is so low I\u0027d do it conditionally 😊 I\u0027d send a follow up patch myself otherwise. Here it\u0027s how I think that the code would look like:\n\n\n\novs_dp \u003d self.ovs_idl.db_get(\u0027Bridge\u0027, self.ovn_bridge, \u0027datapath_type\u0027)\nif ovs_dp \u003d\u003d \u0027netdev\u0027:\n\n\n$ sudo ovs-vsctl get bridge br-int datapath_type                                                                                                                                           \nsystem","commit_id":"30ef59a320f3a89fe31803ceed5075a0cdc8b225"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"4ad67c526db488a5a81dcd8f0ae58b7704c3616e","unresolved":true,"context_lines":[{"line_number":484,"context_line":"            (\u0027external_ids\u0027, {\u0027iface-id\u0027: port.logical_port})).execute()"},{"line_number":485,"context_line":""},{"line_number":486,"context_line":"        # Ensure the correct checksum in the metadata traffic."},{"line_number":487,"context_line":"        self._ensure_datapath_checksum(namespace)"},{"line_number":488,"context_line":""},{"line_number":489,"context_line":"        # Spawn metadata proxy if it\u0027s not already running."},{"line_number":490,"context_line":"        metadata_driver.MetadataDriver.spawn_monitored_metadata_proxy("}],"source_content_type":"text/x-python","patch_set":8,"id":"bcf56cb6_e018273f","line":487,"range":{"start_line":487,"start_character":8,"end_line":487,"end_character":49},"in_reply_to":"a3dd49ab_391a9de4","updated":"2020-12-01 15:49:06.000000000","message":"How much is \"minimal\", 1% ? 2% ?\n\nDo we have any data points that measured this ?\n\nIf we do not, perhaps it would be good to take the safer path and only set this if it\u0027s DPDK. I don\u0027t know much about DPDK but based on daniel\u0027s comment it doesn\u0027t seem that difficult ?\n\nAnother point is the iptables vs nftables that was discussed in the PTG, I see we are using iptables for this so I assume we do have a similar command for nftables when time comes ?","commit_id":"30ef59a320f3a89fe31803ceed5075a0cdc8b225"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"6e3e51554c012276c4756dd7bd505bd8686559a3","unresolved":true,"context_lines":[{"line_number":484,"context_line":"            (\u0027external_ids\u0027, {\u0027iface-id\u0027: port.logical_port})).execute()"},{"line_number":485,"context_line":""},{"line_number":486,"context_line":"        # Ensure the correct checksum in the metadata traffic."},{"line_number":487,"context_line":"        self._ensure_datapath_checksum(namespace)"},{"line_number":488,"context_line":""},{"line_number":489,"context_line":"        # Spawn metadata proxy if it\u0027s not already running."},{"line_number":490,"context_line":"        metadata_driver.MetadataDriver.spawn_monitored_metadata_proxy("}],"source_content_type":"text/x-python","patch_set":8,"id":"8c35dd4e_5055c619","line":487,"range":{"start_line":487,"start_character":8,"end_line":487,"end_character":49},"in_reply_to":"bcf56cb6_e018273f","updated":"2020-12-04 08:35:54.000000000","message":"This is going to handle the metadata traffic. That happens only during the VM boot. I\u0027m not talking about 1% of traffic, but 0.000001%.\n\nWe still don\u0027t have the NFTables manager implemented so for now we\u0027ll use iptables.\n\nIn any case, I\u0027ll make this method conditional to the datapath.","commit_id":"30ef59a320f3a89fe31803ceed5075a0cdc8b225"},{"author":{"_account_id":23804,"name":"Daniel Alvarez","email":"dalvarez@redhat.com","username":"dalvarez"},"change_message_id":"c38aff041452dc9d522cb1c6743050188667ed0c","unresolved":true,"context_lines":[{"line_number":484,"context_line":"            (\u0027external_ids\u0027, {\u0027iface-id\u0027: port.logical_port})).execute()"},{"line_number":485,"context_line":""},{"line_number":486,"context_line":"        # Ensure the correct checksum in the metadata traffic."},{"line_number":487,"context_line":"        self._ensure_datapath_checksum(namespace)"},{"line_number":488,"context_line":""},{"line_number":489,"context_line":"        # Spawn metadata proxy if it\u0027s not already running."},{"line_number":490,"context_line":"        metadata_driver.MetadataDriver.spawn_monitored_metadata_proxy("}],"source_content_type":"text/x-python","patch_set":8,"id":"a3dd49ab_391a9de4","line":487,"range":{"start_line":487,"start_character":8,"end_line":487,"end_character":49},"in_reply_to":"eb08b61c_cb2d467f","updated":"2020-12-01 15:27:28.000000000","message":"Thanks Rodolfo for the answer. I understand but probably the majority of the use cases fall in the non DPDK datapath (ie. most of the times we won\u0027t need this) and the cost of checking the datapath type is minimal. IMHO, it\u0027d be best to fill the checksum only for DPDK especially since it\u0027s very easy (just check the bridge datapath and we\u0027re connecting already to its UNIX socket) and doesn\u0027t require any type of maintenance of migration path.\n\nAs I said, I understand there\u0027s not much of a hit if doing it unconditionally but since the right way to do is doing it only when needed and the cost of it doesn\u0027t seem too high, I\u0027d lean more towards this approach.","commit_id":"30ef59a320f3a89fe31803ceed5075a0cdc8b225"}]}