Hi Saurabh, Thanks for posting these patches!
We looked at the code in the last two days, running various tests and did comparisons with our porting. We tried to be as unbiased as possible in the review, with the goal of making the best out of both codebases and not a “us” vs “them” match. :) For the sake of simplicity I’ll refer to these patches as “VMWare” and to our patches as “Cloudbase” (available at [10]) in the rest of the text. General impressions (VMWare patches) The code is well written and easy to follow, which helped in the analysis. In general, at the current stage the patches look more like a proof of concept or an early work in progress and we experienced various blue screens (kernel panics) when using the compiled kernel extension while performing basic actions. Estimating the amount of time required to reach a maturity level comparable with the Cloudbase porting goes outside of the scope of this initial analysis, but it’s surely non negligible. Feature comparison Here's an initial raw feature comparison, please send any correction in case anything is missing. Feature | Cloudbase | VMWare | --------------------------------------------------------- VXLAN | X | X | VXLAN multiple vports | X | | VXLAN configurable UDP ports | X | | GRE | X | | IPv4 packet fragmentation | X | | LLC | | X | VLAN 802.1Q | X | X | Megaflows | X | | Multiple Hyper-V switches | X | | --------------------------------------------------------- Both implementations are limited to one single datapath at the moment. We have a WiP patch for multiple datapaths, but not yet public. #### General design #### Both porting efforts consists in an Hyper-V extensible switch NDIS forwarding extension. In both cases, external switches require //AllowManagementOS// set to true in order to manage tunnel endpoint traffic. This will change in future releases. Compatible platforms: Hyper-V Server / Windows Server 2012 and 2012 R2 and Windows 8 / 8.1 There are some important design differences between the two portings: ##### Interaction with userspace ###### Windows does not have a Netlink socket based equivalent, which is where all the communication between the user and kernel code is performed (current master branch). Cloudbase ========= We opted for the minimum impact in the userspace code, to minimize the code paths and maintenance required for the various platforms. To obtain this result, a Netlink equivalent has been created providing functions with the same semantic and an implementation that fits the Windows mode. Please see [1] for details. Advantages * Minimal impact in the current codebase (reduced maintenance) * Another advantage is that adapting the porting to multiple OVS versions is easy. * Command line compatibility Disadvantages * This design requires some additional marshalling between the userspace and kernel data structures, but from a first set of tests the performance impact is not relevant. VMWare ====== The patches contain separate Windows specific userspace functions, mostly contained in "dpif-windows.c" and "netdev-windows.c". The "DESIGN" file contains a reference to an additional component that possibly replaces ovs-vswitchd on Windows called ovs-wind, NOT present in the patches. Advantages * The main advantage consists in having less traffic between the components Disadvantages * Non trivial amount of code to be maintained in userspace. * Command line compatibility (?) ##### Hyper-V / OVS port matching ##### Cloudbase ========= Ports are created independently on the OVS and Hyper-V switches and matched with the //Set-VMNetworkAdapterOVSPort// Powershell cmdlet. See [2] for details. Advantages * Flexibility: OVS ports can have any name and can be created independently of attached VMs. * Compatibility with scripts using //ovs-vsctl// Disantages * One additional configuration step is required when a VM is attached to a switch VMWare ====== OVS ports are automatically created when a Hyper-V port is created on the switch. Ports have random names that also change between reboots, see [3]. There's no standard and easy way to determine which OVS port corresponds to a Hyper-V port. This can be improved by using the Hyper-V port GUID. Advantages * Less configuration Disadvantages * Determine which VM is connected to what port is not currently possible. * Security considerations: what OpenFlow rules are applied when a port is created? ##### Stability ##### Cloudbase ========= The driver is already in beta status and available for download [8]. A Jenkins job builds both userspace and kernel components at every build, the driver is signed and an installer (external) is provided for an easy deployment (both GUI and unattended). Stability is compatible with a beta release. A few issues need to be solved and performance can be improved as detailed in [8]. It'd be great if somebody in the community could do some testing and provide a separate opinion. VMWare ====== As noted before, based on the tests that we managed to perform the driver seems to be in an early development stage. We're definitely open to have a discussion on IRC for setting up an environment that allows more testing, as the crashes limit the actions that can be performed. Examples of issues we encountered: * Enabling the driver on a plain vanilla Hyper-V 2012 R2 having a single external switch with AllowManagementOS set, panics (blue screen) with the error reported at [4] (WInDbg, !analyze -v). * In the same OS, but with AllowManagementOS not set, it crashes with the error visible in [5]. * ovs-vsctl.exe --db=tcp:127.0.0.1:8888 add-br br0 fails with [6] * ovs-vsctl.exe --db=tcp:127.0.0.1:8888 show issues [7] * ovs-ofctl.exe benchmark / dump-ports-desc inconsistencies [11] ##### Performance ##### Due to the issues encountered during basic testing it was not possible to perform a proper performance comparison. ##### Notes ##### * The VMWare patches employ also Windows Filtering Platform (WFP) features for IP packet management, while the Cloudbase one performs those operations with ad hoc functions. * The Cloudbase porting must be rebased to the tip of the master branch (currently this requires around 1 week). ##### Conclusions ##### This is just a first quick comparison between both porting efforts including design, features and stability. A follow up conversation will also include notes on the implementation. Since the objective is to compare the two porting efforts and merge the best parts of each one, I'd like to propose a follow up via IRC, Hangout, Skype, etc. to discuss the details among the two teams. My IRC nick is: alexpilotti on FreeNode Thanks! Alessandro ##### Resources ##### [1] Cloudbase Netlink: http://wiki.cloudbase.it/ovs-hyperv-architecture#netlink_replacement [2] Cloudbase Powershell: http://wiki.cloudbase.it/ovs-hyperv-architecture#integration_with_hyper-v_networking [3] ovs-vsctl show output: http://paste.openstack.org/show/84918/ [4] Crash report 1: http://paste.openstack.org/show/84911/ [5] Crash report 2: http://paste.openstack.org/show/84913/ [6] ovs-vsctl error 1: http://paste.openstack.org/show/84916/ [7] ovs-vsctl error 2: http://paste.openstack.org/show/84917/ [8] Cloudbase MSI download: http://www.cloudbase.it/open-vswitch-on-hyper-v/ [9] Cloudbase port design: http://wiki.cloudbase.it/ovs-hyperv-architecture#integration_with_hyper-v_networking [10] Cloudbase port repos: http://openvswitch.org/pipermail/dev/2014-June/041432.html [11] ovs-ofctl inconsistency: http://paste.openstack.org/show/84925/ On 24 Jun 2014, at 03:34, Saurabh Shah <ssaur...@vmware.com> wrote: > This patch series adds OpenvSwitch datapath support for Windows platform. The > kernel switch extension has support for bridged back forwarding & tunneling > over VXLAN. > > The patch series is based on top of 6ddb631 ("datapath: keep mask array > compact > when deleting mask"). > > Saurabh Shah (4): > datapath-windows: Base code for developing the Hyper-V switch > entension. > datapath-windows: Prepare base code for the forwarding extension. > dpif-windows: Implement datapath interface for windows. > datapath-windows: Implement the virtual switch forwarding extension. > > AUTHORS | 2 + > BUILD.Windows | 95 -- > COPYING | 4 + > CodingStyle | 4 +- > INSTALL.Windows | 131 ++ > Makefile.am | 4 +- > datapath-windows/CodingStyle | 139 +++ > datapath-windows/DESIGN | 371 ++++++ > datapath-windows/Package/package.VcxProj | 100 ++ > datapath-windows/automake.mk | 60 + > datapath-windows/base/SxApi.h | 1196 +++++++++++++++++++ > datapath-windows/base/SxBase.c | 1389 +++++++++++++++++++++ > datapath-windows/base/SxBase.h | 170 +++ > datapath-windows/base/SxLibrary.c | 839 +++++++++++++ > datapath-windows/base/SxLibrary.h | 464 +++++++ > datapath-windows/base/precomp.h | 11 + > datapath-windows/base/precompsrc.c | 1 + > datapath-windows/base/sxbase.vcxproj | 130 ++ > datapath-windows/extensions.sln | 65 + > datapath-windows/include/OvsNetlink.h | 187 +++ > datapath-windows/include/OvsPub.h | 490 ++++++++ > datapath-windows/misc/install.cmd | 4 + > datapath-windows/misc/uninstall.cmd | 1 + > datapath-windows/ovsext/OvsActions.c | 1489 +++++++++++++++++++++++ > datapath-windows/ovsext/OvsAtomic.h | 32 + > datapath-windows/ovsext/OvsBufferMgmt.c | 1536 ++++++++++++++++++++++++ > datapath-windows/ovsext/OvsBufferMgmt.h | 125 ++ > datapath-windows/ovsext/OvsChecksum.c | 493 ++++++++ > datapath-windows/ovsext/OvsChecksum.h | 33 + > datapath-windows/ovsext/OvsDebug.c | 58 + > datapath-windows/ovsext/OvsDebug.h | 90 ++ > datapath-windows/ovsext/OvsEth.h | 450 +++++++ > datapath-windows/ovsext/OvsEvent.c | 514 ++++++++ > datapath-windows/ovsext/OvsEvent.h | 50 + > datapath-windows/ovsext/OvsExt.c | 375 ++++++ > datapath-windows/ovsext/OvsExt.h | 57 + > datapath-windows/ovsext/OvsFlow.c | 974 +++++++++++++++ > datapath-windows/ovsext/OvsFlow.h | 77 ++ > datapath-windows/ovsext/OvsIoctl.c | 742 ++++++++++++ > datapath-windows/ovsext/OvsIoctl.h | 40 + > datapath-windows/ovsext/OvsIpHelper.c | 1689 ++++++++++++++++++++++++++ > datapath-windows/ovsext/OvsIpHelper.h | 128 ++ > datapath-windows/ovsext/OvsNetProto.h | 367 ++++++ > datapath-windows/ovsext/OvsPacketParser.c | 294 +++++ > datapath-windows/ovsext/OvsPacketParser.h | 144 +++ > datapath-windows/ovsext/OvsProperty.c | 320 +++++ > datapath-windows/ovsext/OvsSwitch.c | 414 +++++++ > datapath-windows/ovsext/OvsSwitch.h | 148 +++ > datapath-windows/ovsext/OvsTunnel.c | 349 ++++++ > datapath-windows/ovsext/OvsTunnel.h | 57 + > datapath-windows/ovsext/OvsTunnelFilter.c | 343 ++++++ > datapath-windows/ovsext/OvsTunnelIntf.h | 25 + > datapath-windows/ovsext/OvsTypes.h | 32 + > datapath-windows/ovsext/OvsUser.c | 874 ++++++++++++++ > datapath-windows/ovsext/OvsUser.h | 114 ++ > datapath-windows/ovsext/OvsUtil.c | 85 ++ > datapath-windows/ovsext/OvsUtil.h | 78 ++ > datapath-windows/ovsext/OvsVport.c | 1685 ++++++++++++++++++++++++++ > datapath-windows/ovsext/OvsVport.h | 168 +++ > datapath-windows/ovsext/OvsVxlan.c | 442 +++++++ > datapath-windows/ovsext/OvsVxlan.h | 81 ++ > datapath-windows/ovsext/ovsext.inf | 85 ++ > datapath-windows/ovsext/ovsext.rc | 23 + > datapath-windows/ovsext/ovsext.vcxproj | 145 +++ > datapath-windows/ovsext/precomp.h | 35 + > datapath-windows/ovsext/precompsrc.c | 17 + > debian/copyright.in | 74 ++ > include/linux/openvswitch.h | 8 +- > lib/automake.mk | 3 + > lib/dpif-provider.h | 3 + > lib/dpif-windows.c | 1859 +++++++++++++++++++++++++++++ > lib/dpif-windows.h | 57 + > lib/dpif.c | 3 + > lib/jhash.c | 19 +- > lib/jhash.h | 7 + > lib/netdev-provider.h | 3 + > lib/netdev-windows.c | 895 ++++++++++++++ > lib/netdev.c | 5 + > lib/stream-tcp.c | 6 + > 79 files changed, 23977 insertions(+), 99 deletions(-) > delete mode 100644 BUILD.Windows > create mode 100644 INSTALL.Windows > create mode 100644 datapath-windows/CodingStyle > create mode 100644 datapath-windows/DESIGN > create mode 100755 datapath-windows/Package/package.VcxProj > create mode 100644 datapath-windows/automake.mk > create mode 100644 datapath-windows/base/SxApi.h > create mode 100644 datapath-windows/base/SxBase.c > create mode 100644 datapath-windows/base/SxBase.h > create mode 100644 datapath-windows/base/SxLibrary.c > create mode 100644 datapath-windows/base/SxLibrary.h > create mode 100644 datapath-windows/base/precomp.h > create mode 100644 datapath-windows/base/precompsrc.c > create mode 100755 datapath-windows/base/sxbase.vcxproj > create mode 100644 datapath-windows/extensions.sln > create mode 100644 datapath-windows/include/OvsNetlink.h > create mode 100644 datapath-windows/include/OvsPub.h > create mode 100755 datapath-windows/misc/install.cmd > create mode 100755 datapath-windows/misc/uninstall.cmd > create mode 100644 datapath-windows/ovsext/OvsActions.c > create mode 100644 datapath-windows/ovsext/OvsAtomic.h > create mode 100644 datapath-windows/ovsext/OvsBufferMgmt.c > create mode 100644 datapath-windows/ovsext/OvsBufferMgmt.h > create mode 100644 datapath-windows/ovsext/OvsChecksum.c > create mode 100644 datapath-windows/ovsext/OvsChecksum.h > create mode 100644 datapath-windows/ovsext/OvsDebug.c > create mode 100644 datapath-windows/ovsext/OvsDebug.h > create mode 100644 datapath-windows/ovsext/OvsEth.h > create mode 100644 datapath-windows/ovsext/OvsEvent.c > create mode 100644 datapath-windows/ovsext/OvsEvent.h > create mode 100644 datapath-windows/ovsext/OvsExt.c > create mode 100644 datapath-windows/ovsext/OvsExt.h > create mode 100644 datapath-windows/ovsext/OvsFlow.c > create mode 100644 datapath-windows/ovsext/OvsFlow.h > create mode 100644 datapath-windows/ovsext/OvsIoctl.c > create mode 100644 datapath-windows/ovsext/OvsIoctl.h > create mode 100644 datapath-windows/ovsext/OvsIpHelper.c > create mode 100644 datapath-windows/ovsext/OvsIpHelper.h > create mode 100644 datapath-windows/ovsext/OvsNetProto.h > create mode 100644 datapath-windows/ovsext/OvsPacketParser.c > create mode 100644 datapath-windows/ovsext/OvsPacketParser.h > create mode 100644 datapath-windows/ovsext/OvsProperty.c > create mode 100644 datapath-windows/ovsext/OvsSwitch.c > create mode 100644 datapath-windows/ovsext/OvsSwitch.h > create mode 100644 datapath-windows/ovsext/OvsTunnel.c > create mode 100644 datapath-windows/ovsext/OvsTunnel.h > create mode 100644 datapath-windows/ovsext/OvsTunnelFilter.c > create mode 100644 datapath-windows/ovsext/OvsTunnelIntf.h > create mode 100644 datapath-windows/ovsext/OvsTypes.h > create mode 100644 datapath-windows/ovsext/OvsUser.c > create mode 100644 datapath-windows/ovsext/OvsUser.h > create mode 100644 datapath-windows/ovsext/OvsUtil.c > create mode 100644 datapath-windows/ovsext/OvsUtil.h > create mode 100644 datapath-windows/ovsext/OvsVport.c > create mode 100644 datapath-windows/ovsext/OvsVport.h > create mode 100644 datapath-windows/ovsext/OvsVxlan.c > create mode 100644 datapath-windows/ovsext/OvsVxlan.h > create mode 100644 datapath-windows/ovsext/ovsext.inf > create mode 100644 datapath-windows/ovsext/ovsext.rc > create mode 100644 datapath-windows/ovsext/ovsext.vcxproj > create mode 100644 datapath-windows/ovsext/precomp.h > create mode 100644 datapath-windows/ovsext/precompsrc.c > create mode 100644 lib/dpif-windows.c > create mode 100644 lib/dpif-windows.h > create mode 100644 lib/netdev-windows.c > > -- > 1.7.9.5 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
