)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"7110b4cb5cb8c9cd744009215c03114967128e77","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"9d9ad78a_d40975a4","updated":"2022-03-15 12:26:09.000000000","message":"We should change the fullstack tests to ensure we are adding the corresponding rules in table 0\n\n\"TestBwLimitQoSOvs._wait_for_bw_rule_applied\", for ingress direction should also check that we are adding this corresponding flow. \"_wait_for_min_bw_rule_applied\" should check that too.\n\n","commit_id":"811344565dba33a72bf46255c81731be7d9a1805"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"7a5dbd3b5082b0234097c25aba0ee286ffab99a6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"81d229fa_f83ce707","in_reply_to":"9d9ad78a_d40975a4","updated":"2022-03-15 19:55:40.000000000","message":"Done","commit_id":"811344565dba33a72bf46255c81731be7d9a1805"}],"neutron/agent/common/ovs_lib.py":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"a8756ffe4a8edacaa0e804f8f95575a4c79dea4d","unresolved":true,"context_lines":[{"line_number":819,"context_line":"        # The same reg4 is used for the minimum bandwidth rules in table\u003d0 but"},{"line_number":820,"context_line":"        # as in_port is always different in those 2 cases, same registry can be"},{"line_number":821,"context_line":"        # used for both types of rules"},{"line_number":822,"context_line":"        patch_ports \u003d self.get_bridge_patch_ports_ofports()"},{"line_number":823,"context_line":"        for patch_port in patch_ports:"},{"line_number":824,"context_line":"            self.add_flow("},{"line_number":825,"context_line":"                table\u003dconstants.LOCAL_SWITCHING,"}],"source_content_type":"text/x-python","patch_set":1,"id":"687dfc04_ac4bffb1","line":822,"range":{"start_line":822,"start_character":8,"end_line":822,"end_character":59},"updated":"2022-03-09 09:41:28.000000000","message":"I don\u0027t understand why we are applying this queue only to the traffic coming from patch ports. What if the traffic is coming from the same compute VM? This traffic won\u0027t leave the integration bridge and won\u0027t be marked with queue 0; thus, the ingress traffic for the second VM won\u0027t be limited.\n\nAm I missing something?","commit_id":"1ed9c47e67d2f3ad2b6128bb95c5df9d2cad9934"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"927f313a6e34ba0fc1276fc1b1ee2a50a14f4cc5","unresolved":false,"context_lines":[{"line_number":819,"context_line":"        # The same reg4 is used for the minimum bandwidth rules in table\u003d0 but"},{"line_number":820,"context_line":"        # as in_port is always different in those 2 cases, same registry can be"},{"line_number":821,"context_line":"        # used for both types of rules"},{"line_number":822,"context_line":"        patch_ports \u003d self.get_bridge_patch_ports_ofports()"},{"line_number":823,"context_line":"        for patch_port in patch_ports:"},{"line_number":824,"context_line":"            self.add_flow("},{"line_number":825,"context_line":"                table\u003dconstants.LOCAL_SWITCHING,"}],"source_content_type":"text/x-python","patch_set":1,"id":"3040cb3b_80bca0ac","line":822,"range":{"start_line":822,"start_character":8,"end_line":822,"end_character":59},"in_reply_to":"687dfc04_ac4bffb1","updated":"2022-03-15 11:44:27.000000000","message":"It\u0027s changed now 😊","commit_id":"1ed9c47e67d2f3ad2b6128bb95c5df9d2cad9934"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"7110b4cb5cb8c9cd744009215c03114967128e77","unresolved":true,"context_lines":[{"line_number":814,"context_line":"    def set_queue_for_ingress_bandwidth_limit(self):"},{"line_number":815,"context_line":"        # reg3 is used to memoize if queue was set or not. If it is first visit"},{"line_number":816,"context_line":"        # to table 0 for a packet (i.e. reg3 \u003d\u003d 0), set queue and memoize (i.e."},{"line_number":817,"context_line":"        # load 1 to reg4), then goto table 0 again. The packet will be handled"},{"line_number":818,"context_line":"        # as usual when the second visit to table 0."},{"line_number":819,"context_line":"        # For min bw reg4 is used for the same purpose. In case if there we"},{"line_number":820,"context_line":"        # would need one of those registries for something else in the future"}],"source_content_type":"text/x-python","patch_set":2,"id":"b6175baa_b1262864","line":817,"range":{"start_line":817,"start_character":20,"end_line":817,"end_character":24},"updated":"2022-03-15 12:26:09.000000000","message":"reg3","commit_id":"811344565dba33a72bf46255c81731be7d9a1805"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"7a5dbd3b5082b0234097c25aba0ee286ffab99a6","unresolved":false,"context_lines":[{"line_number":814,"context_line":"    def set_queue_for_ingress_bandwidth_limit(self):"},{"line_number":815,"context_line":"        # reg3 is used to memoize if queue was set or not. If it is first visit"},{"line_number":816,"context_line":"        # to table 0 for a packet (i.e. reg3 \u003d\u003d 0), set queue and memoize (i.e."},{"line_number":817,"context_line":"        # load 1 to reg4), then goto table 0 again. The packet will be handled"},{"line_number":818,"context_line":"        # as usual when the second visit to table 0."},{"line_number":819,"context_line":"        # For min bw reg4 is used for the same purpose. In case if there we"},{"line_number":820,"context_line":"        # would need one of those registries for something else in the future"}],"source_content_type":"text/x-python","patch_set":2,"id":"3539b45f_e6c2931b","line":817,"range":{"start_line":817,"start_character":20,"end_line":817,"end_character":24},"in_reply_to":"b6175baa_b1262864","updated":"2022-03-15 19:55:40.000000000","message":"Done","commit_id":"811344565dba33a72bf46255c81731be7d9a1805"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"7110b4cb5cb8c9cd744009215c03114967128e77","unresolved":true,"context_lines":[{"line_number":822,"context_line":"        # which sets pkt_mark for minimum bandwidth and play with bitmask"},{"line_number":823,"context_line":"        self.add_flow("},{"line_number":824,"context_line":"            table\u003dconstants.LOCAL_SWITCHING,"},{"line_number":825,"context_line":"            reg3\u003d0,"},{"line_number":826,"context_line":"            priority\u003d200,"},{"line_number":827,"context_line":"            actions\u003d(\"set_queue:%s,load:1-\u003eNXM_NX_REG3[0],\""},{"line_number":828,"context_line":"                     \"resubmit(,%s)\" % (QOS_DEFAULT_QUEUE,"}],"source_content_type":"text/x-python","patch_set":2,"id":"f8004f74_c0235260","line":825,"range":{"start_line":825,"start_character":12,"end_line":825,"end_character":16},"updated":"2022-03-15 12:26:09.000000000","message":"**nit**: I would add a follow-up patch to:\n1) Move \"neutron.agent.linux.openvswitch_firewall.firewall.create_reg_numbers\" to \"ovs_lib\"\n2) Move the FW constants REG_* to neutron-lib.plugins.ml2.ovs_constants, adding reg3 and reg4\n\nIn another follow-up patch, I would transform those methods into os-ken calls, instead of still using ovs-ofctl.\n\nBut as commented, this is just a nit.","commit_id":"811344565dba33a72bf46255c81731be7d9a1805"},{"author":{"_account_id":8313,"name":"Lajos Katona","display_name":"lajoskatona","email":"katonalala@gmail.com","username":"elajkat","status":"Ericsson Software Technology"},"change_message_id":"45f8265d928b818f8be9a668ae48761b822d8470","unresolved":true,"context_lines":[{"line_number":822,"context_line":"        # which sets pkt_mark for minimum bandwidth and play with bitmask"},{"line_number":823,"context_line":"        self.add_flow("},{"line_number":824,"context_line":"            table\u003dconstants.LOCAL_SWITCHING,"},{"line_number":825,"context_line":"            reg3\u003d0,"},{"line_number":826,"context_line":"            priority\u003d200,"},{"line_number":827,"context_line":"            actions\u003d(\"set_queue:%s,load:1-\u003eNXM_NX_REG3[0],\""},{"line_number":828,"context_line":"                     \"resubmit(,%s)\" % (QOS_DEFAULT_QUEUE,"}],"source_content_type":"text/x-python","patch_set":2,"id":"9acbf598_1f7c7557","line":825,"range":{"start_line":825,"start_character":12,"end_line":825,"end_character":16},"in_reply_to":"f8004f74_c0235260","updated":"2022-03-17 08:55:42.000000000","message":"\u003e **nit**: I would add a follow-up patch to:\n\u003e 1) Move \"neutron.agent.linux.openvswitch_firewall.firewall.create_reg_numbers\" to \"ovs_lib\"\n\n\u003e 2) Move the FW constants REG_* to neutron-lib.plugins.ml2.ovs_constants, adding reg3 and reg4\nI agree with this, these reg3 and reg4 are not visible in any doc and only these comments are helping.\n\n\u003e \n\u003e In another follow-up patch, I would transform those methods into os-ken calls, instead of still using ovs-ofctl.\n\u003e \n\u003e But as commented, this is just a nit.","commit_id":"811344565dba33a72bf46255c81731be7d9a1805"}]}