)]}'
{"neutron/agent/linux/ip_conntrack.py":[{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"825997f079dfb078b79e2bbfd824c9a9a9aa155a","unresolved":false,"context_lines":[{"line_number":117,"context_line":"        protocol \u003d rule.get(\u0027protocol\u0027)"},{"line_number":118,"context_line":"        direction \u003d rule.get(\u0027direction\u0027)"},{"line_number":119,"context_line":"        cmd \u003d [\u0027conntrack\u0027, \u0027-D\u0027]"},{"line_number":120,"context_line":"        if protocol:"},{"line_number":121,"context_line":"            # 0 is IP in /etc/protocols, but conntrack will throw an error"},{"line_number":122,"context_line":"            if protocol \u003d\u003d 0:"},{"line_number":123,"context_line":"                protocol \u003d \u0027ip\u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"5fc1f717_9807d882","line":120,"range":{"start_line":120,"start_character":11,"end_line":120,"end_character":19},"updated":"2019-03-28 14:24:32.000000000","message":"And maybe here, refactor to \n\"if protocol is not None\"\nto cover the case which protocol is integer 0.","commit_id":"58e6f9d77d60786726190ab4c92c5506298aba70"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"7d216a4928eb1af3a46ca526c5ec591c956b01bd","unresolved":false,"context_lines":[{"line_number":117,"context_line":"        protocol \u003d rule.get(\u0027protocol\u0027)"},{"line_number":118,"context_line":"        direction \u003d rule.get(\u0027direction\u0027)"},{"line_number":119,"context_line":"        cmd \u003d [\u0027conntrack\u0027, \u0027-D\u0027]"},{"line_number":120,"context_line":"        if protocol:"},{"line_number":121,"context_line":"            # 0 is IP in /etc/protocols, but conntrack will throw an error"},{"line_number":122,"context_line":"            if protocol \u003d\u003d 0:"},{"line_number":123,"context_line":"                protocol \u003d \u0027ip\u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fce034c_53560409","line":120,"range":{"start_line":120,"start_character":11,"end_line":120,"end_character":19},"in_reply_to":"5fc1f717_9807d882","updated":"2019-04-11 19:57:41.000000000","message":"Done","commit_id":"58e6f9d77d60786726190ab4c92c5506298aba70"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"e2e789eeeb2f91110f132ac98623f4fff53b8779","unresolved":false,"context_lines":[{"line_number":118,"context_line":"        direction \u003d rule.get(\u0027direction\u0027)"},{"line_number":119,"context_line":"        cmd \u003d [\u0027conntrack\u0027, \u0027-D\u0027]"},{"line_number":120,"context_line":"        if protocol:"},{"line_number":121,"context_line":"            # 0 is IP in /etc/protocols, but conntrack will throw an error"},{"line_number":122,"context_line":"            if protocol \u003d\u003d 0:"},{"line_number":123,"context_line":"                protocol \u003d \u0027ip\u0027"},{"line_number":124,"context_line":"            cmd.extend([\u0027-p\u0027, str(protocol)])"}],"source_content_type":"text/x-python","patch_set":1,"id":"5fc1f717_4314a6a0","line":121,"updated":"2019-03-22 18:14:06.000000000","message":"1) Actually, if protocol is \"0\", you should not specify protocol. \"0\" means no protocol [1].\n\n2) You should use the IPTABLES_PROTOCOL_NAME_MAP table [2] to convert the protocol number into its string.\n\n\nif protocol:\n  protocol \u003d IPTABLES_PROTOCOL_NAME_MAP.get(protocol, protocol)\n  if protocol:  # \u003c-- this is for the case \"0\": None\n    cmd.extend([\u0027-p\u0027, str(protocol)])\n\n\n[1] https://github.com/openstack/neutron/blob/002d9ee028d217131b6fe0529abdc4f203098d4f/neutron/common/constants.py#L82\n\n[2] https://github.com/openstack/neutron/blob/002d9ee028d217131b6fe0529abdc4f203098d4f/neutron/common/constants.py#L78","commit_id":"58e6f9d77d60786726190ab4c92c5506298aba70"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"31efc98c3dfb0d6eddea78b1b1e4449dcb0ff76b","unresolved":false,"context_lines":[{"line_number":118,"context_line":"        direction \u003d rule.get(\u0027direction\u0027)"},{"line_number":119,"context_line":"        cmd \u003d [\u0027conntrack\u0027, \u0027-D\u0027]"},{"line_number":120,"context_line":"        if protocol:"},{"line_number":121,"context_line":"            # 0 is IP in /etc/protocols, but conntrack will throw an error"},{"line_number":122,"context_line":"            if protocol \u003d\u003d 0:"},{"line_number":123,"context_line":"                protocol \u003d \u0027ip\u0027"},{"line_number":124,"context_line":"            cmd.extend([\u0027-p\u0027, str(protocol)])"}],"source_content_type":"text/x-python","patch_set":1,"id":"5fc1f717_b56edf93","line":121,"in_reply_to":"5fc1f717_4314a6a0","updated":"2019-03-22 20:11:05.000000000","message":"For iptables, protocol 0 produces rules without any other arguments, which is why that is None in the mapping.  And protocol 0 actually is \u0027ip\u0027 or \u0027hopopt\u0027, at least it will be when https://review.openstack.org/#/c/644979/ merges, it was an oversight when I created the table.\n\nSince this is calling conntrack I don\u0027t want to use the iptables mapping, just the protocol number from the rule, which conntrack accepts, except for 0 since there was a bad range check :(  I\u0027ve already had that change accepted in conntrack-tools :) but we have to work around it for older versions.","commit_id":"58e6f9d77d60786726190ab4c92c5506298aba70"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"71c0a38166a70cbc65503411e0305de04d882a91","unresolved":false,"context_lines":[{"line_number":118,"context_line":"        direction \u003d rule.get(\u0027direction\u0027)"},{"line_number":119,"context_line":"        cmd \u003d [\u0027conntrack\u0027, \u0027-D\u0027]"},{"line_number":120,"context_line":"        if protocol:"},{"line_number":121,"context_line":"            # 0 is IP in /etc/protocols, but conntrack will throw an error"},{"line_number":122,"context_line":"            if protocol \u003d\u003d 0:"},{"line_number":123,"context_line":"                protocol \u003d \u0027ip\u0027"},{"line_number":124,"context_line":"            cmd.extend([\u0027-p\u0027, str(protocol)])"}],"source_content_type":"text/x-python","patch_set":1,"id":"5fc1f717_3ac5f33c","line":121,"in_reply_to":"5fc1f717_b56edf93","updated":"2019-03-23 20:14:12.000000000","message":"I suggested this table because is the most complete one. The IP protocol numbers is the same for all services (iptables, conntrack, etc).\n\n0 is considered a pseudo protocol number in the kernel, but not in [1]. Because conntrack (correct me if I\u0027m wrong) only handles IP traffic, when deleting \"ip\" protocol rules what we are doing is deleting all rules.\n\nIn this change, we should not specify protocol \"ip\" for IP rules (IP is not an IP protocol itself).\n\nAnyway, I\u0027ll remove my -1.\n\n[1] https://www.iana.org/assignments/protocol-numbers/protocol-numbers.xml","commit_id":"58e6f9d77d60786726190ab4c92c5506298aba70"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"567a2274cbabee56369c68ead0b34d1397b12214","unresolved":false,"context_lines":[{"line_number":119,"context_line":"        cmd \u003d [\u0027conntrack\u0027, \u0027-D\u0027]"},{"line_number":120,"context_line":"        if protocol:"},{"line_number":121,"context_line":"            # 0 is IP in /etc/protocols, but conntrack will throw an error"},{"line_number":122,"context_line":"            if protocol \u003d\u003d 0:"},{"line_number":123,"context_line":"                protocol \u003d \u0027ip\u0027"},{"line_number":124,"context_line":"            cmd.extend([\u0027-p\u0027, str(protocol)])"},{"line_number":125,"context_line":"        cmd.extend([\u0027-f\u0027, str(ethertype).lower()])"}],"source_content_type":"text/x-python","patch_set":1,"id":"5fc1f717_a222a8a1","line":122,"updated":"2019-03-28 07:43:55.000000000","message":"value 0 will be False in boolean:\n\n\u003e\u003e\u003e protocol\u003d0\n\u003e\u003e\u003e bool(protocol)\nFalse\n\nSo it should never get into \"if\" from L120 in such case. I think that this should have changed indentation level. Or am I missing something?\nHowever in case when protocol is \"0\" then it will get into this \"if\" but Your new condition will not be True and You will still have same error as You had before this patch.","commit_id":"58e6f9d77d60786726190ab4c92c5506298aba70"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"7d216a4928eb1af3a46ca526c5ec591c956b01bd","unresolved":false,"context_lines":[{"line_number":119,"context_line":"        cmd \u003d [\u0027conntrack\u0027, \u0027-D\u0027]"},{"line_number":120,"context_line":"        if protocol:"},{"line_number":121,"context_line":"            # 0 is IP in /etc/protocols, but conntrack will throw an error"},{"line_number":122,"context_line":"            if protocol \u003d\u003d 0:"},{"line_number":123,"context_line":"                protocol \u003d \u0027ip\u0027"},{"line_number":124,"context_line":"            cmd.extend([\u0027-p\u0027, str(protocol)])"},{"line_number":125,"context_line":"        cmd.extend([\u0027-f\u0027, str(ethertype).lower()])"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fce034c_13407c35","line":122,"in_reply_to":"5fc1f717_588170fa","updated":"2019-04-11 19:57:41.000000000","message":"Done","commit_id":"58e6f9d77d60786726190ab4c92c5506298aba70"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"1cc2ce12140196a5905ea25ae733526590e06d2c","unresolved":false,"context_lines":[{"line_number":119,"context_line":"        cmd \u003d [\u0027conntrack\u0027, \u0027-D\u0027]"},{"line_number":120,"context_line":"        if protocol:"},{"line_number":121,"context_line":"            # 0 is IP in /etc/protocols, but conntrack will throw an error"},{"line_number":122,"context_line":"            if protocol \u003d\u003d 0:"},{"line_number":123,"context_line":"                protocol \u003d \u0027ip\u0027"},{"line_number":124,"context_line":"            cmd.extend([\u0027-p\u0027, str(protocol)])"},{"line_number":125,"context_line":"        cmd.extend([\u0027-f\u0027, str(ethertype).lower()])"}],"source_content_type":"text/x-python","patch_set":1,"id":"5fc1f717_588170fa","line":122,"in_reply_to":"5fc1f717_a222a8a1","updated":"2019-03-28 14:21:28.000000000","message":"Yes, I have same question.\nBut according to the trace of the bug, I can imagine this protocol here is string \u00270\u0027, not an integer 0.\n\nSo I\u0027d like to suggest we write this like the following to cover all int and str cases:\n\"\nif str(protocol) \u003d\u003d \u00270\u0027:\n\"","commit_id":"58e6f9d77d60786726190ab4c92c5506298aba70"}],"neutron/tests/unit/agent/linux/test_iptables_firewall.py":[{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"567a2274cbabee56369c68ead0b34d1397b12214","unresolved":false,"context_lines":[{"line_number":1390,"context_line":"                self.firewall.ipconntrack._process_queue()"},{"line_number":1391,"context_line":"            cmd \u003d [\u0027conntrack\u0027, \u0027-D\u0027]"},{"line_number":1392,"context_line":"            if protocol:"},{"line_number":1393,"context_line":"                if protocol \u003d\u003d 0:"},{"line_number":1394,"context_line":"                    protocol \u003d \u0027ip\u0027"},{"line_number":1395,"context_line":"                cmd.extend([\u0027-p\u0027, str(protocol)])"},{"line_number":1396,"context_line":"            if ethertype \u003d\u003d \u0027IPv4\u0027:"}],"source_content_type":"text/x-python","patch_set":1,"id":"5fc1f717_623840d2","line":1393,"updated":"2019-03-28 07:43:55.000000000","message":"the same comment here also","commit_id":"58e6f9d77d60786726190ab4c92c5506298aba70"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"7d216a4928eb1af3a46ca526c5ec591c956b01bd","unresolved":false,"context_lines":[{"line_number":1390,"context_line":"                self.firewall.ipconntrack._process_queue()"},{"line_number":1391,"context_line":"            cmd \u003d [\u0027conntrack\u0027, \u0027-D\u0027]"},{"line_number":1392,"context_line":"            if protocol:"},{"line_number":1393,"context_line":"                if protocol \u003d\u003d 0:"},{"line_number":1394,"context_line":"                    protocol \u003d \u0027ip\u0027"},{"line_number":1395,"context_line":"                cmd.extend([\u0027-p\u0027, str(protocol)])"},{"line_number":1396,"context_line":"            if ethertype \u003d\u003d \u0027IPv4\u0027:"}],"source_content_type":"text/x-python","patch_set":1,"id":"3fce034c_b3516811","line":1393,"in_reply_to":"5fc1f717_623840d2","updated":"2019-04-11 19:57:41.000000000","message":"Done","commit_id":"58e6f9d77d60786726190ab4c92c5506298aba70"}]}
