On Sep 30, 2014, at 7:40 AM, Samuel Ghinet <sghi...@cloudbasesolutions.com> wrote:
> Hello guys, > > This patch number 00 is an introduction to the patch series. > > I am sorry we could not provide the design behind this much faster. This > series of patches is based on that design. > I have tested these vport commands with both VMs and vxlan, with both VMs > connecting and reconnecting and netlink vport add-s and deletes. > > In order to work properly, this patch requires a few fixes in parts that are > outside of this series' scope: > 1. netdev-windows - I received from Alin a draft patch, which was enough to > allow the userspace use the netlink commands vport new, set and delete hi Samuel/Alin, Thanks for this huge contribution. I believe, this patch is taking things in the right direction. There are corner cases to handle, and I also suspect the code will become a little unstable with these patches. But, we'll fix the bugs once the feature is in :) Pls. address the comments in the patches, and we don't have to fix everything at once. Here are some high level comments: 1) Why do we need to store 2 separate names: ovsName and portFriendlyName? If the goal is that we want to allow flexibility in changing the portFriendlyName on a HV port while it is connected to an OVS DP port, it should be stated clearly. I am not a big fan of making the two names go out of sync while the OVS DP port exists. IMHO, we should fail the OID that tries to change portFriendlyName of a HV port while its OVS DP counterpart exists. It is completely legitimate to fail an OID, esp. when we know that the name change was issued from the WMI script that the OVS distribution is providing. 2) Creation and connection of the External port/NIC is a little bit of a complex state machine as I have seen on my machine. There is one port create, and 2 NIC creates. The sequence of call is something like this: OvsCreatePort(): nicIndex = 0 OvsCreateNic(): nicIndex = 0 OvsCreateNic(): nicIndex = 1 OvsConnectNic(): nicIndex = 1 I don't know if the code handles this. The previous code handed this in a hacky way, by making sure that the external port doesn't get added to the portHashArray, nor was it set in the vportArray. 3) If the name is not unique, we should fail the OID to add the NIC or to change the name. We should also enforce a size limit on the portFriendlyName. 4) Fixes to nl_sock_transact_multiple__() to always pass an output buffer are not part of the changes, though you talk about it in the cover letter. Pls. address as many comments as you can. We need not get this right in the first try itself. We can fix any bugs and fallout after the code goes in. thanks, -- Nithin _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
