)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7776,"name":"Paul Carver","email":"pcarver@att.com","username":"pcarver"},"change_message_id":"19188c2c5c4965e755b952be5ab00969748c6328","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"fa31d9ce_6f996c1f","updated":"2017-02-17 13:55:29.000000000","message":"Please review https://wiki.openstack.org/wiki/GitCommitMessages\n\nIn particular:\n\nThe first line should be limited to 50 characters and should not end with a period.\nSubsequent lines should be wrapped at 72 characters.\n\nBut also, your first line probably ought to be clearer.","commit_id":"59f04c0c28f0f5e9496d91ee9c70a5285dbde0cc"}],"networking_sfc/db/flowclassifier_db.py":[{"author":{"_account_id":7776,"name":"Paul Carver","email":"pcarver@att.com","username":"pcarver"},"change_message_id":"8666a1e68279f4be8c2c309813eff68a9a618bc8","unresolved":false,"context_lines":[{"line_number":233,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":234,"context_line":"            if logical_source_port is not None:"},{"line_number":235,"context_line":"                port_src \u003d self._get_port(context, logical_source_port)"},{"line_number":236,"context_line":"                if source_ip_prefix is None:"},{"line_number":237,"context_line":"                    src_ipaddresses \u003d port_src[\u0027fixed_ips\u0027]"},{"line_number":238,"context_line":"                    if len(src_ipaddresses) \u003d\u003d 1:"},{"line_number":239,"context_line":"                        source_ip_prefix \u003d src_ipaddresses[0][\u0027ip_address\u0027]"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa31d9ce_87abdc1d","line":236,"updated":"2017-02-17 17:30:18.000000000","message":"Ok, I missed this when trying to rush through a review earlier before running to a meeting. It\u0027s good that you are checking for None before modifying.\n\nHowever, I\u0027m still unclear on what the current behavior is and what the new behavior is. I currently have these prefix fields as not required in the API reference. What\u0027s the benefit of populating them if the user doesn\u0027t? Does it make something work better to have the non-required fields populated?","commit_id":"59f04c0c28f0f5e9496d91ee9c70a5285dbde0cc"},{"author":{"_account_id":19948,"name":"Farhad Sunavala","email":"fsbiz@yahoo.com","username":"fsunaval"},"change_message_id":"1fdc8f7d2b92db8ca9de2ba250d7f146d672b1ed","unresolved":false,"context_lines":[{"line_number":233,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":234,"context_line":"            if logical_source_port is not None:"},{"line_number":235,"context_line":"                port_src \u003d self._get_port(context, logical_source_port)"},{"line_number":236,"context_line":"                if source_ip_prefix is None:"},{"line_number":237,"context_line":"                    src_ipaddresses \u003d port_src[\u0027fixed_ips\u0027]"},{"line_number":238,"context_line":"                    if len(src_ipaddresses) \u003d\u003d 1:"},{"line_number":239,"context_line":"                        source_ip_prefix \u003d src_ipaddresses[0][\u0027ip_address\u0027]"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa31d9ce_1ceca228","line":236,"in_reply_to":"fa31d9ce_5970ecdc","updated":"2017-02-17 18:15:43.000000000","message":"Yes, I couldn\u0027t have said it better on both counts.\n\nIt is a bug and this commit will not fix it completely.\nI only attempt it if there is a single IP address.\n\nThis is why I have the check \n                    if len(dst_ipaddresses) \u003d\u003d 1:\n\nIf there are multiple IP addresses, then this fix is not enough. This would require creating multiple flow classifiers for the same logical source port with each flow classifier having one of the IP addresses.","commit_id":"59f04c0c28f0f5e9496d91ee9c70a5285dbde0cc"},{"author":{"_account_id":19948,"name":"Farhad Sunavala","email":"fsbiz@yahoo.com","username":"fsunaval"},"change_message_id":"29831049464fe33de659d6eb5cb5286b79e2be20","unresolved":false,"context_lines":[{"line_number":233,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":234,"context_line":"            if logical_source_port is not None:"},{"line_number":235,"context_line":"                port_src \u003d self._get_port(context, logical_source_port)"},{"line_number":236,"context_line":"                if source_ip_prefix is None:"},{"line_number":237,"context_line":"                    src_ipaddresses \u003d port_src[\u0027fixed_ips\u0027]"},{"line_number":238,"context_line":"                    if len(src_ipaddresses) \u003d\u003d 1:"},{"line_number":239,"context_line":"                        source_ip_prefix \u003d src_ipaddresses[0][\u0027ip_address\u0027]"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa31d9ce_d915bce5","line":236,"in_reply_to":"fa31d9ce_87abdc1d","updated":"2017-02-17 17:50:40.000000000","message":"Current behaviour:\n\nConfigure a simple portchain as follows:\n\nsrc --- sf1 --- sf2 ---- dst\n\nConfigure your flow-classifer as follows:\n\nflow-classifier-create --logical-source-port \u003csrc\u003e --protocol icmp  fc\n\nYour portchain will work just as you intended. So far so good.\nNow, ping sf1 from another source (say src2).\nThe ICMP responses from sf1 back to src2 will also go through the chain even though in your flowclassifier you only specified ICMP traffic for src.\n\nThe reason is the flow rule programmed on sf1 is as follows:\n\ninport\u003d\u003csf1\u003e, prot\u003dicmp, actions\u003dredirect to sf2\n\nThe logical-source-port specified in the flowclassifier has no relevance down the chain.\n\nNew behaviour:\n\nConsider same portchain and flowclassifier as above.\n\nsrc --- sf1 --- sf2 ---- dst\nflow-classifier-create --logical-source-port \u003csrc\u003e --protocol icmp  fc\n\nSay, src IP is 10.0.0.10\nWe now get the SRC IP (10.0.0.10) and put it in the flowclassifier.\nNow, the rules programmed at sf1 will appear as follows:\ninport\u003d\u003csf1\u003e, ip.src\u003d10.0.0.10/32, prot\u003dicmp, actions \u003d redirect to sf2\n\nNow, only icmp traffic from src will traverse through the chain. Pings from src2 to sf1 will no longer traverse through the chain which is what you wanted.","commit_id":"59f04c0c28f0f5e9496d91ee9c70a5285dbde0cc"},{"author":{"_account_id":7776,"name":"Paul Carver","email":"pcarver@att.com","username":"pcarver"},"change_message_id":"77073ccd6abd1258349f798cf73c3270ab2d0ee2","unresolved":false,"context_lines":[{"line_number":233,"context_line":"        with context.session.begin(subtransactions\u003dTrue):"},{"line_number":234,"context_line":"            if logical_source_port is not None:"},{"line_number":235,"context_line":"                port_src \u003d self._get_port(context, logical_source_port)"},{"line_number":236,"context_line":"                if source_ip_prefix is None:"},{"line_number":237,"context_line":"                    src_ipaddresses \u003d port_src[\u0027fixed_ips\u0027]"},{"line_number":238,"context_line":"                    if len(src_ipaddresses) \u003d\u003d 1:"},{"line_number":239,"context_line":"                        source_ip_prefix \u003d src_ipaddresses[0][\u0027ip_address\u0027]"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa31d9ce_5970ecdc","line":236,"in_reply_to":"fa31d9ce_d915bce5","updated":"2017-02-17 18:04:58.000000000","message":"Thank you. That helps. I would say that the behavior before this change where traffic from SF2 get picked up by the chain is a bug. Would you agree?\n\nBut if logical_source_port has multiple IPs then your change doesn\u0027t do anything. So I would say that we should have a bug logged for the fact that traffic from SF2 can unintentionally be funneled into the port chain and your commit message for this change should contain a partial-bug: line to reflect that it addresses the unintended behavior but only in the case where the source has only one IP.\n\nThe bug should get further attention. In particular, we could probably address a dual stack scenario where the source has one IPv4 address and one IPv6 address.\n\nI\u0027m not saying you have to solve everything in this commit. But my impression is that your commit fixes part of a larger problem and we should capture that fact.","commit_id":"59f04c0c28f0f5e9496d91ee9c70a5285dbde0cc"},{"author":{"_account_id":9396,"name":"igordcard","email":"igordcard@gmail.com","username":"igordcard"},"change_message_id":"7d3ce1024ac6f8b1fac94fed721c19c1f2c0ab8f","unresolved":false,"context_lines":[{"line_number":239,"context_line":"                            port_src[\u0027device_owner\u0027] not in router_ints):"},{"line_number":240,"context_line":"                    src_ipadds \u003d port_src[\u0027fixed_ips\u0027]"},{"line_number":241,"context_line":"                    if len(src_ipadds) \u003d\u003d 1:"},{"line_number":242,"context_line":"                        source_ip_prefix \u003d src_ipadds[0][\u0027ip_address\u0027]"},{"line_number":243,"context_line":""},{"line_number":244,"context_line":"            if logical_destination_port is not None:"},{"line_number":245,"context_line":"                port_dst \u003d self._get_port(context, logical_destination_port)"}],"source_content_type":"text/x-python","patch_set":3,"id":"da36d5c6_3a119fad","line":242,"range":{"start_line":242,"start_character":24,"end_line":242,"end_character":70},"updated":"2017-02-19 12:09:48.000000000","message":"This is effectively overriding something in the API, even though only when it wasn\u0027t specified manually. The flow classifier should be influenced at the driver level, not the API/DB level. Indeed, drivers other than OVS one - or even a future enhanced OVS driver - need not suffer from this issue.","commit_id":"3d6d9e46b4c448294f6a201c1aa3dbf70a22902e"},{"author":{"_account_id":19948,"name":"Farhad Sunavala","email":"fsbiz@yahoo.com","username":"fsunaval"},"change_message_id":"8f2f5593fa88d8ce15d2f9f155797120f43155fd","unresolved":false,"context_lines":[{"line_number":239,"context_line":"                            port_src[\u0027device_owner\u0027] not in router_ints):"},{"line_number":240,"context_line":"                    src_ipadds \u003d port_src[\u0027fixed_ips\u0027]"},{"line_number":241,"context_line":"                    if len(src_ipadds) \u003d\u003d 1:"},{"line_number":242,"context_line":"                        source_ip_prefix \u003d src_ipadds[0][\u0027ip_address\u0027]"},{"line_number":243,"context_line":""},{"line_number":244,"context_line":"            if logical_destination_port is not None:"},{"line_number":245,"context_line":"                port_dst \u003d self._get_port(context, logical_destination_port)"}],"source_content_type":"text/x-python","patch_set":3,"id":"da36d5c6_6077e6d4","line":242,"range":{"start_line":242,"start_character":24,"end_line":242,"end_character":70},"in_reply_to":"da36d5c6_3a119fad","updated":"2017-02-19 18:40:07.000000000","message":"I see your point and originally wanted to fix it in the OVS driver as you suggest.   However, after thinking more about it, decided to fix it higher up. \n\nIrrespective of which driver you take, the fundamental issue remains.  If you specify only the logical-src-port (neutron entity) and another field (say, protocol\u003dicmp), the logical-src-port information is irrelevant down the chain and the only information available for reclassification is the protocol information.  \nAs long as classification needs to be done again at the SF, this problem will exist.\n\nThe problem persists with OVN as well.  OVN will change the logical destination port to match that of the SF initially, but when the packet comes out of the SF, it still needs to be reclassified.  \n\nLet me know your thoughts.","commit_id":"3d6d9e46b4c448294f6a201c1aa3dbf70a22902e"}]}