)]}'
{"neutron/services/trunk/plugin.py":[{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"inactive contributor"},"change_message_id":"d4a556306a343da7edb6e3d9fa8d9376a3b7047b","unresolved":false,"context_lines":[{"line_number":88,"context_line":"    @staticmethod"},{"line_number":89,"context_line":"    def _handle_trunk_port_host_binding_change(context, updated_port, trigger):"},{"line_number":90,"context_line":"        host_id \u003d updated_port[portbindings.HOST_ID]"},{"line_number":91,"context_line":"        for subport in updated_port[\u0027trunk_details\u0027][\u0027sub_ports\u0027]:"},{"line_number":92,"context_line":"            port_req \u003d {\u0027port\u0027: {portbindings.HOST_ID: host_id}}"},{"line_number":93,"context_line":"            trigger.update_port(context, subport[\u0027port_id\u0027], port_req)"},{"line_number":94,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1aa78d24_9b824551","line":91,"range":{"start_line":91,"start_character":8,"end_line":91,"end_character":66},"updated":"2016-07-04 14:09:45.000000000","message":"This approach has a performance consequence.\n\nLast time I have profiled port update code (that was releases ago though) a single port update took some time t (roughly 0.1-1.0s). If we call update_port for each subport that means a trunk port update takes (subport_count + 1) * t. No problem for a handful of subports. But take a 1000 subports then we have 100-1000s for a trunk port update (the http client gave up way before). That is clearly unacceptable.\n\nIf we go this way we will have to limit the allowed maximum number of subports to get an acceptable trunk port update time.\n\nA trunk port and all of its subports are by definition on the same host. Storing that information multiple times is redundant. Processing it multiple times is wasted time.\n\nOne the other hand I see the lure of this approach, since this way we can readily reuse all port binding code.","commit_id":"d036cbdf341a57020e05c04ac09643ac1c396584"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"df5a15a8d79d797a32f2a2d9178516b34e201980","unresolved":false,"context_lines":[{"line_number":88,"context_line":"    @staticmethod"},{"line_number":89,"context_line":"    def _handle_trunk_port_host_binding_change(context, updated_port, trigger):"},{"line_number":90,"context_line":"        host_id \u003d updated_port[portbindings.HOST_ID]"},{"line_number":91,"context_line":"        for subport in updated_port[\u0027trunk_details\u0027][\u0027sub_ports\u0027]:"},{"line_number":92,"context_line":"            port_req \u003d {\u0027port\u0027: {portbindings.HOST_ID: host_id}}"},{"line_number":93,"context_line":"            trigger.update_port(context, subport[\u0027port_id\u0027], port_req)"},{"line_number":94,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1aa78d24_b088aec6","line":91,"range":{"start_line":91,"start_character":8,"end_line":91,"end_character":66},"in_reply_to":"1aa78d24_81ae0e7f","updated":"2016-07-06 02:55:46.000000000","message":"I tend to agree with Bence here somewhat. We need to figure out a way to batch the events pushed to the agent, as I can see ourselves needing this type of \u0027optimization\u0027 later in the future.\n\nOn a side note, we should be careful in adopting the \u0027works on my machine\u0027 mindset, we were burned in the past a few times :)","commit_id":"d036cbdf341a57020e05c04ac09643ac1c396584"},{"author":{"_account_id":16800,"name":"Rawlin Peters","email":"rawlin.peters@hpe.com","username":"rawlin"},"change_message_id":"c064af44747ce008bf3c5e231bc9f02e140b00cf","unresolved":false,"context_lines":[{"line_number":88,"context_line":"    @staticmethod"},{"line_number":89,"context_line":"    def _handle_trunk_port_host_binding_change(context, updated_port, trigger):"},{"line_number":90,"context_line":"        host_id \u003d updated_port[portbindings.HOST_ID]"},{"line_number":91,"context_line":"        for subport in updated_port[\u0027trunk_details\u0027][\u0027sub_ports\u0027]:"},{"line_number":92,"context_line":"            port_req \u003d {\u0027port\u0027: {portbindings.HOST_ID: host_id}}"},{"line_number":93,"context_line":"            trigger.update_port(context, subport[\u0027port_id\u0027], port_req)"},{"line_number":94,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1aa78d24_81ae0e7f","line":91,"range":{"start_line":91,"start_character":8,"end_line":91,"end_character":66},"in_reply_to":"1aa78d24_9b824551","updated":"2016-07-05 23:35:54.000000000","message":"My quick and dirty devstack benchmarking showed this takes roughly .25 sec currently, so it would take 1000 secs to perform the portbinding for 4k subports. Seventeen minutes does seem kind of crazy for an API call, but it is to connect a VM to 4k different networks, which makes the time seem less crazy.\n\nIf tenants want to create trunks with thousands of subports at a time, operators could also increase the client\u0027s timeout window to account for this extra time. I\u0027d say that beats creating a trunk port without subports then adding subports one-by-one from the API.\n\nI agree that this is redundant information for every subport on the same trunk, but I\u0027m not sure how else we can give subports a portbinding. Without a portbinding, the OVS agent won\u0027t process the patch ports on br-int that correspond to subports.","commit_id":"d036cbdf341a57020e05c04ac09643ac1c396584"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"inactive contributor"},"change_message_id":"6adfdd7192fb56742ba10a216d786273972bdf62","unresolved":false,"context_lines":[{"line_number":88,"context_line":"    @staticmethod"},{"line_number":89,"context_line":"    def _handle_trunk_port_host_binding_change(context, updated_port, trigger):"},{"line_number":90,"context_line":"        host_id \u003d updated_port[portbindings.HOST_ID]"},{"line_number":91,"context_line":"        for subport in updated_port[\u0027trunk_details\u0027][\u0027sub_ports\u0027]:"},{"line_number":92,"context_line":"            port_req \u003d {\u0027port\u0027: {portbindings.HOST_ID: host_id}}"},{"line_number":93,"context_line":"            trigger.update_port(context, subport[\u0027port_id\u0027], port_req)"},{"line_number":94,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1aa78d24_1cd70fee","line":91,"range":{"start_line":91,"start_character":8,"end_line":91,"end_character":66},"in_reply_to":"1aa78d24_b088aec6","updated":"2016-07-06 09:35:10.000000000","message":"Beware of handling the runtimes of the openvswitch mechanism driver as universal. For example people do deploy neutron with config like this:\n\n mechanism_drivers \u003d openvswitch,random-hw-switch-vendor\n\nAnd a hardware switch with a tiny control processor can be really slow. Also we should leave ample room for load from other sources on neutron-server.\n\nIMO what really matters is not the complete time required by the operation, but how long a component is blocked. For example if ovs-agent needs 1000 seconds to implement 4k subports AND it can process other ports in the middle of that, that\u0027s slow, but completely acceptable. If ovs-agent takes 1000 seconds AND it is blocked for 1000 seconds (ie. it cannot process other ports during that) then other nova boots (scheduled to the same host) will time out because of that. By default nova waits 600 seconds for a port notification.\n\nWhat we should absolutely avoid is to block neutron-server for such long periods. It is much better to block it for 4k*0.25 seconds than for 1*1000 seconds.\n\nOne possible way I see is to bind the trunk port with a only a few subports, then add subports to the running vm. However if we do that, then the guest needs to know *when* we finished adding all subports. That comes back to an earlier idea of exposing trunk and subport information over metadata. Maybe the guest could even create subports over the metadata interface.\n\nAnother way could be to bind a subport to the trunk port instead of binding it to the vswitch instance (that is today expressed by the host name). On a conceptual level I think that would make sense, because I see binding as binding-to-the-nearest-relevant-packet-forwarding-device. For a subport we could argue that the nearest relevant device is the trunk bridge, not the integration bridge. That would also get rid of the redundancy in the object model.\n\nAlso keep in mind that neutron\u0027s most relevant client for binding port updates is nova. I think today all calls from nova to neutron have the same global timeout, changing that could have far-reaching consequences.","commit_id":"d036cbdf341a57020e05c04ac09643ac1c396584"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"da3f330dc327a95a7916de3ce0ffd243ee608db4","unresolved":false,"context_lines":[{"line_number":227,"context_line":""},{"line_number":228,"context_line":"                trunk[\u0027sub_ports\u0027].append(obj)"},{"line_number":229,"context_line":""},{"line_number":230,"context_line":"        self._set_subport_portbinding(context, trunk[\u0027port_id\u0027], subports)"},{"line_number":231,"context_line":"        return trunk"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":3,"id":"1aa78d24_30c15e55","line":230,"updated":"2016-07-06 02:38:25.000000000","message":"we should simply emit a registry notifications about subport events and have this code wrapped up by the callback, because we\u0027d need that anyway to emit the RPC notification required for the agent to create the patch port on the right vlan. No?","commit_id":"d036cbdf341a57020e05c04ac09643ac1c396584"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"da3f330dc327a95a7916de3ce0ffd243ee608db4","unresolved":false,"context_lines":[{"line_number":231,"context_line":"        return trunk"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"    @staticmethod"},{"line_number":234,"context_line":"    def _set_subport_portbinding(context, trunk_port_id, subports):"},{"line_number":235,"context_line":"        core_plugin \u003d manager.NeutronManager.get_plugin()"},{"line_number":236,"context_line":"        trunk_port \u003d core_plugin.get_port(context, trunk_port_id)"},{"line_number":237,"context_line":"        trunk_host_id \u003d trunk_port[portbindings.HOST_ID]"}],"source_content_type":"text/x-python","patch_set":3,"id":"1aa78d24_50d50a1a","line":234,"updated":"2016-07-06 02:38:25.000000000","message":"If these do not use anything of the plugin why are they static methods? Not sure I see the point. I vote to get rid of these methods from here anyway, see previous comment.","commit_id":"d036cbdf341a57020e05c04ac09643ac1c396584"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"da3f330dc327a95a7916de3ce0ffd243ee608db4","unresolved":false,"context_lines":[{"line_number":270,"context_line":"                subport_obj.delete()"},{"line_number":271,"context_line":""},{"line_number":272,"context_line":"            trunk.sub_ports \u003d list(sub_ports.values())"},{"line_number":273,"context_line":"            self._clear_subport_portbinding(context, subports)"},{"line_number":274,"context_line":"            return trunk"},{"line_number":275,"context_line":""},{"line_number":276,"context_line":"    def get_subports(self, context, trunk_id, fields\u003dNone):"}],"source_content_type":"text/x-python","patch_set":3,"id":"1aa78d24_90b2d204","line":273,"updated":"2016-07-06 02:38:25.000000000","message":"ditto","commit_id":"d036cbdf341a57020e05c04ac09643ac1c396584"}]}
