[ovs-dev] [PATCH] odp-netlink.h: Autogenerate a version of odp-netlink for windows kernel.

Ankur Sharma ankursharma at vmware.com
Thu Aug 7 18:25:07 UTC 2014


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 <aserdean at cloudbasesolutions.com>
Sent: Thursday, August 7, 2014 10:38 AM
To: Ankur Sharma; Nithin Raju
Cc: <dev at 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:ankursharma at vmware.com]
Trimis: Thursday, August 7, 2014 7:11 AM
Către: Alin Serdean; Nithin Raju
Cc: <dev at 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 <aserdean at cloudbasesolutions.com>
Sent: Wednesday, August 6, 2014 6:17 PM
To: Nithin Raju; Ankur Sharma
Cc: <dev at 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-bounces at openvswitch.org] În numele Nithin Raju
Trimis: Thursday, August 7, 2014 3:17 AM
Către: Ankur Sharma
Cc: <dev at 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 <nithin at vmware.com>

thanks,
Nithin


On Aug 6, 2014, at 4:30 PM, Ankur Sharma <ankursharma at 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 <ankursharma at vmware.com>
> Co-authored-by: Saurabh Shah <ssaurabh at vmware.com>
> Tested-by: Ankur Sharma <ankursharma at vmware.com>
> Reported-by: Alin Serdean <aserdean at cloudbasesolutions.com>
> Reported-by: Nithin Raju <nithin at 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 at 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 at 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


More information about the dev mailing list