Hi Alin, Thanks a lot for your reply.
========================================================================== [COMMENT]: I would prefer to enhance odp-netlink.h if possible. [REPLY]: Sure I had a discussion on this with nithin. I'll talk to ben and confirm if he is ok with having ifdefs in odp-netlink.h. We'll go with one odp-netlink.h as long as ben is fine with it. =========================================================================== [COMMENT]: One other idea that comes to mind is to create mockup files of the missing header files openvswitch.h(odp-netlink.h) is including and adding them to a directory. This directory can be used under the forwarding extension solution. This can help us in allowing us to put extra defines if needed in the future. [REPLY]: Thanks a lot for proposing an alternate approach. I can think over it but i am not sure if it is going to give us something much better then what we plan to do. Having said that i would like to think over it more and then we can make a call. Meanwhile if you are ok then i would to submit a V2 of the patch with comment 1 incorporated and we can check it in. Comment 2 can be thought over and taken care of later. =========================================================================== Thanks. Regards, Ankur ________________________________________ From: Alin Serdean <aserd...@cloudbasesolutions.com> Sent: Thursday, August 7, 2014 10:38 AM To: Ankur Sharma; Nithin Raju Cc: <dev@openvswitch.org> Subject: RE: [ovs-dev] [PATCH] odp-netlink.h: Autogenerate a version of odp-netlink for windows kernel. Hi Ankur, I would prefer to enhance odp-netlink.h if possible. One other idea that comes to mind is to create mockup files of the missing header files openvswitch.h(odp-netlink.h) is including and adding them to a directory. This directory can be used under the forwarding extension solution. This can help us in allowing us to put extra defines if needed in the future. Alin. -----Mesaj original----- De la: Ankur Sharma [mailto:ankursha...@vmware.com] Trimis: Thursday, August 7, 2014 7:11 AM Către: Alin Serdean; Nithin Raju Cc: <dev@openvswitch.org> Subiect: RE: [ovs-dev] [PATCH] odp-netlink.h: Autogenerate a version of odp-netlink for windows kernel. Hi Alin, Thanks a lot for the review. Comment: I was wondering why not generate the #ifdef over the file that already generates a copy of openvswitch.h (odp-netlink.h) needed for the extension? Reply: Yes sounds good. I did not update the original sed script as it would be full of ifdef based checks than, having a separate script looked cleaner to me. But i can make this change if you want to. ====================================================================================== Comment: And as Nithin pointed out I think this can be solved in a more elegant solution. Reply: Yes, overall we discussed following approaches. 1. Have a seperate sed script to create odp-netlink for windows(submitted in patch). The script would be executed as a part ovs user space build, adhering to generation of odp-netlink.h. https://urldefense.proofpoint.com/v1/url?u=https://github.com/openvswitch/ovs/commit/837eefc76b3c79bb790a4c4c2d0a314d81b71a28&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=f6EhnZ0ORGZNt5QbYmRaOxfWfx%2Bqd3KEiPf3%2FYaollU%3D%0A&m=6guioO%2FCDHM51PWGnmLdFqKKVNr06663Pzst8zz1ex4%3D%0A&s=5c4a7e31d3fff03c2332c004885ef37fd0392a5c6d3c7740367101441346433b 2. Same as above, but script will be executed as a part of windows driver build. 3. Create a static odp-netlink-windows.h and make it a part of git repo. We ruled out 3 as it would not automatically sync openvswitch.h. 2 is still under consideration as a long term solution but we did not do it as it would require that kernel build machine has sed as a powershell command (implying that mingw or some other package providing sed installation is a must on kernel build machines), also executing a pre build script as a part of vc project did not look straightforward when i tried it last time (for some other issue). Why we picked approach 1: a. It alligns with the odp-netlink.h generation. b. It will unblock the windows kernel build easily. c. After committing this patch i'll investigate approach 2 and implement that if everyone want to. Please let me know your thoughts, we can approach the issue accordingly. Regards, Ankur ________________________________________ From: Alin Serdean <aserd...@cloudbasesolutions.com> Sent: Wednesday, August 6, 2014 6:17 PM To: Nithin Raju; Ankur Sharma Cc: <dev@openvswitch.org> Subject: RE: [ovs-dev] [PATCH] odp-netlink.h: Autogenerate a version of odp-netlink for windows kernel. I was wondering why not generate the #ifdef over the file that already generates a copy of openvswitch.h (odp-netlink.h) needed for the extension? And as Nithin pointed out I think this can be solved in a more elegant solution. Thanks, Alin. -----Mesaj original----- De la: dev [mailto:dev-boun...@openvswitch.org] În numele Nithin Raju Trimis: Thursday, August 7, 2014 3:17 AM Către: Ankur Sharma Cc: <dev@openvswitch.org> Subiect: Re: [ovs-dev] [PATCH] odp-netlink.h: Autogenerate a version of odp-netlink for windows kernel. hi Ankur, This is not a perfect solution, but a step in the right direction of auto-generating a file for the Windows DP. Only comment I had is that we should get rid of definition of ETH_ALEN in OvsTypes.h. You can do that in another patch instead of re-spinning this one. Unless we change the approach, this patch looks good to me. Acked-by: Nithin Raju <nit...@vmware.com> thanks, Nithin On Aug 6, 2014, at 4:30 PM, Ankur Sharma <ankursha...@vmware.com> wrote: > odp-netlink.h: Autogenerate a version of odp-netlink for windows kernel. > > Autogenerated odp-netlink.h will not compile with windows kernel, as > it refers to some userspace files like openvswitch/types.h and > packets.h which hyperv extension does not access. Due to this the > windows datapath compilation is broken on tip of tree. This patch > intends to fix that. > > In this patch we add a new sed script "extract-odp-netlink-windows-h" > to create odp-netlink-windows-dp.h. It works on similar lines as > extract-odp-netlink-h, but avoids including the header files which are > not available for driver. > > Also, added saurabh's fix to not to include some header files in > lib/netlink-protocol.h not needed by windows driver. > > After this fix, a userspace build will be needed before windows kernel > datapath can be built. > > Tested that hyperv extension could be built after building the > userspace. Verified vxlan tunnel based ping across hypervisors. > Verified that odp-netlink-windows-dp.h is not built for linux > platform. Ran 'make distcheck' to verify that nothing is broken on > linux. > > Signed-off-by: Ankur Sharma <ankursha...@vmware.com> > Co-authored-by: Saurabh Shah <ssaur...@vmware.com> > Tested-by: Ankur Sharma <ankursha...@vmware.com> > Reported-by: Alin Serdean <aserd...@cloudbasesolutions.com> > Reported-by: Nithin Raju <nit...@vmware.com> > Reported-at: > https://urldefense.proofpoint.com/v1/url?u=https://github.com/openvswi > tch/ovs-issues/issues/21&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=ubrOpIW > avCMqX4l4j1LEVpTfDj%2FD5Qyn8KCoJIBGvzo%3D%0A&m=j8DgXwkf5X0TTs0%2B5C5wB > eHu5N%2BL38609QgXuIyYlvA%3D%0A&s=08ba33ff599cf904d6adfe602f69b542c6e87 > 57acacc80a2c2cc8074965197c2 > --- > build-aux/extract-odp-netlink-windows-h | 24 ++++++++++++++++++++++++ > datapath-windows/ovsext/precomp.h | 2 +- > include/automake.mk | 16 ++++++++++++++++ > lib/netlink-protocol.h | 3 ++- > 4 files changed, 43 insertions(+), 2 deletions(-) create mode 100755 > build-aux/extract-odp-netlink-windows-h > > diff --git a/build-aux/extract-odp-netlink-windows-h > b/build-aux/extract-odp-netlink-windows-h > new file mode 100755 > index 0000000..dbb2c98 > --- /dev/null > +++ b/build-aux/extract-odp-netlink-windows-h > @@ -0,0 +1,24 @@ > +# This is a "sed" script that transforms <linux/openvswitch.h> into a > +# form that is suitable for inclusion within the Open vSwitch tree on > +# windows system. The transformed header file can be included by > +windows # driver modules. > + > +# Add a header warning that this is a generated file. > +1i\ > +/* -*- mode: c; buffer-read-only: t -*- */\ > +/* Generated automatically from <linux/openvswitch.h> -- do not > +modify! */\ \ \ > + > +# Avoid using reserved names in header guards. > +s/_LINUX_OPENVSWITCH_H/ODP_NETLINK_WINDOWS_DP_H/ > + > +# and use the appropriate userspace header. > +s,<linux/types\.h>,"OvsTypes.h", > + > +# Add ETH_ADDR_LEN macro to avoid including userspace packet.h > +s,#include <linux/if_ether\.h>,\n#ifndef ETH_ADDR_LEN \ #define > +ETH_ADDR_LEN 6 \n#endif, > + > +# Use OVS's own ETH_ADDR_LEN instead of Linux-specific ETH_ALEN. > +s/ETH_ALEN/ETH_ADDR_LEN/ > diff --git a/datapath-windows/ovsext/precomp.h > b/datapath-windows/ovsext/precomp.h > index 45e72de..4c81323 100644 > --- a/datapath-windows/ovsext/precomp.h > +++ b/datapath-windows/ovsext/precomp.h > @@ -28,4 +28,4 @@ > * Include openvswitch.h from userspace. Changing the location the > file from > * include/linux is pending discussion. > */ > -#include "include\linux\openvswitch.h" > +#include "include\odp-netlink-windows-dp.h" > diff --git a/include/automake.mk b/include/automake.mk index > 55cb353..233eb52 100644 > --- a/include/automake.mk > +++ b/include/automake.mk > @@ -1,9 +1,25 @@ > BUILT_SOURCES += include/odp-netlink.h > > +if WIN32 > +BUILT_SOURCES += include/odp-netlink-windows-dp.h endif > + > include/odp-netlink.h: datapath/linux/compat/include/linux/openvswitch.h \ > build-aux/extract-odp-netlink-h > sed -f $(srcdir)/build-aux/extract-odp-netlink-h < $< > $@ > + > +if WIN32 > +include/odp-netlink-windows-dp.h: > datapath/linux/compat/include/linux/openvswitch.h \ > + build-aux/extract-odp-netlink-windows-h > + sed -f $(srcdir)/build-aux/extract-odp-netlink-windows-h < $< > > +$@ endif > + > EXTRA_DIST += build-aux/extract-odp-netlink-h > + > +if WIN32 > +EXTRA_DIST += build-aux/extract-odp-netlink-windows-h > +endif > + > CLEANFILES += include/odp-netlink.h > > include include/openflow/automake.mk > diff --git a/lib/netlink-protocol.h b/lib/netlink-protocol.h index > 8938055..3ce18f0 100644 > --- a/lib/netlink-protocol.h > +++ b/lib/netlink-protocol.h > @@ -28,10 +28,11 @@ > * regardless of platform. On Linux, it includes the proper headers > directly; > * on other platforms it directly defines the structures and macros itself. > */ > - > +#ifndef OVS_WIN_DP > #include <stdint.h> > #include <sys/socket.h> > #include "util.h" > +#endif > > #ifdef HAVE_NETLINK > #include <linux/netlink.h> > -- > 1.9.1 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://urldefense.proofpoint.com/v1/url?u=http://openvswitch.org/mail > man/listinfo/dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=ubrOpIWavCMqX4l > 4j1LEVpTfDj%2FD5Qyn8KCoJIBGvzo%3D%0A&m=j8DgXwkf5X0TTs0%2B5C5wBeHu5N%2B > L38609QgXuIyYlvA%3D%0A&s=fc9e474fcf7c7aeddc0665718843f221c2a35aaf3ee65 > 49845a83e346c435081 _______________________________________________ dev mailing list dev@openvswitch.org https://urldefense.proofpoint.com/v1/url?u=http://openvswitch.org/mailman/listinfo/dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=f6EhnZ0ORGZNt5QbYmRaOxfWfx%2Bqd3KEiPf3%2FYaollU%3D%0A&m=6nuuBTgvQAvbKg9%2FFSBQ4uorWLl4D3NdXeeb02JA0UE%3D%0A&s=6f54e0317445639fa75630147d04943dfc54412864da19aaf78f983dc7aa9954 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
