diff mbox

[ovs-dev] datapath: Linux 4.9 compat.

Message ID 1481157078-22121-1-git-send-email-jarno@ovn.org
State Accepted
Headers show

Commit Message

Jarno Rajahalme Dec. 8, 2016, 12:31 a.m. UTC
This patch allows openvswitch kernel module in the OVS tree to be
compiled against the current net-next Linux kernel.  The changes are
due to these upstream commits:

56989f6d856 ("genetlink: mark families as __ro_after_init")
489111e5c25 ("genetlink: statically initialize families")
a07ea4d9941 ("genetlink: no longer support using static family IDs")

struct genl_family initialization is changed be completely static and
to include the new (in Linux 4.6) __ro_after_init attribute.  Compat
code defines it as an empty macro if not defined already.

GENL_ID_GENERATE is no longer defined, but since it was defined as 0,
it is safe to drop it from all initializers also on older Linux
versions.

Tested with current Linux net-next (4.9) and 3.16.

Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
---
 acinclude.m4                                |  4 ++--
 datapath/datapath.c                         | 18 +++++++++---------
 datapath/linux/Modules.mk                   |  1 +
 datapath/linux/compat/include/linux/cache.h | 10 ++++++++++
 4 files changed, 22 insertions(+), 11 deletions(-)
 create mode 100644 datapath/linux/compat/include/linux/cache.h

Comments

Andy Zhou Dec. 9, 2016, 5:38 a.m. UTC | #1
On Wed, Dec 7, 2016 at 4:31 PM, Jarno Rajahalme <jarno@ovn.org> wrote:

> This patch allows openvswitch kernel module in the OVS tree to be
> compiled against the current net-next Linux kernel.  The changes are
> due to these upstream commits:
>
> 56989f6d856 ("genetlink: mark families as __ro_after_init")
> 489111e5c25 ("genetlink: statically initialize families")
> a07ea4d9941 ("genetlink: no longer support using static family IDs")
>
> struct genl_family initialization is changed be completely static and
> to include the new (in Linux 4.6) __ro_after_init attribute.  Compat
> code defines it as an empty macro if not defined already.
>
> GENL_ID_GENERATE is no longer defined, but since it was defined as 0,
> it is safe to drop it from all initializers also on older Linux
> versions.
>
> Tested with current Linux net-next (4.9) and 3.16.
>
> Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
>

Tested building linux 4.0-4.9.

Acked-by: Andy Zhou <azhou@ovn.org>

When compiling with 4.0, I got the following warnning. It does not seem to
be related to this patch.

/home/azhou/projs/ovs/datapath/linux/datapath.c: In function
‘ovs_flow_cmd_set’:
/home/azhou/projs/ovs/datapath/linux/datapath.c:1232:1: warning: the frame
size of 1040 bytes is larger than 1024 bytes [-Wframe-larger-than=]
 }
 ^
Pravin Shelar Dec. 9, 2016, 10:36 p.m. UTC | #2
On Wed, Dec 7, 2016 at 4:31 PM, Jarno Rajahalme <jarno@ovn.org> wrote:
> This patch allows openvswitch kernel module in the OVS tree to be
> compiled against the current net-next Linux kernel.  The changes are
> due to these upstream commits:
>
> 56989f6d856 ("genetlink: mark families as __ro_after_init")
> 489111e5c25 ("genetlink: statically initialize families")
> a07ea4d9941 ("genetlink: no longer support using static family IDs")
>
> struct genl_family initialization is changed be completely static and
> to include the new (in Linux 4.6) __ro_after_init attribute.  Compat
> code defines it as an empty macro if not defined already.
>
> GENL_ID_GENERATE is no longer defined, but since it was defined as 0,
> it is safe to drop it from all initializers also on older Linux
> versions.
>

Patch looks good to me too.
Can you add assert for GENL_ID_GENERATE value, to catch case where it
is not the case.

Thanks.
Joe Stringer Dec. 9, 2016, 10:47 p.m. UTC | #3
On 7 December 2016 at 16:31, Jarno Rajahalme <jarno@ovn.org> wrote:
> This patch allows openvswitch kernel module in the OVS tree to be
> compiled against the current net-next Linux kernel.  The changes are
> due to these upstream commits:
>
> 56989f6d856 ("genetlink: mark families as __ro_after_init")
> 489111e5c25 ("genetlink: statically initialize families")
> a07ea4d9941 ("genetlink: no longer support using static family IDs")
>
> struct genl_family initialization is changed be completely static and
> to include the new (in Linux 4.6) __ro_after_init attribute.  Compat
> code defines it as an empty macro if not defined already.
>
> GENL_ID_GENERATE is no longer defined, but since it was defined as 0,
> it is safe to drop it from all initializers also on older Linux
> versions.
>
> Tested with current Linux net-next (4.9) and 3.16.
>
> Signed-off-by: Jarno Rajahalme <jarno@ovn.org>

Looks like others are happy with the change, I just had one question
(I mentioned offline) about whether we are now synchronised between
ovs tree datapath module and upstream v4.9 module code?
Jarno Rajahalme Dec. 10, 2016, 12:50 a.m. UTC | #4
> On Dec 9, 2016, at 2:47 PM, Joe Stringer <joe@ovn.org> wrote:
> 
> On 7 December 2016 at 16:31, Jarno Rajahalme <jarno@ovn.org <mailto:jarno@ovn.org>> wrote:
>> This patch allows openvswitch kernel module in the OVS tree to be
>> compiled against the current net-next Linux kernel.  The changes are
>> due to these upstream commits:
>> 
>> 56989f6d856 ("genetlink: mark families as __ro_after_init")
>> 489111e5c25 ("genetlink: statically initialize families")
>> a07ea4d9941 ("genetlink: no longer support using static family IDs")
>> 
>> struct genl_family initialization is changed be completely static and
>> to include the new (in Linux 4.6) __ro_after_init attribute.  Compat
>> code defines it as an empty macro if not defined already.
>> 
>> GENL_ID_GENERATE is no longer defined, but since it was defined as 0,
>> it is safe to drop it from all initializers also on older Linux
>> versions.
>> 
>> Tested with current Linux net-next (4.9) and 3.16.
>> 
>> Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
> 
> Looks like others are happy with the change, I just had one question
> (I mentioned offline) about whether we are now synchronised between
> ovs tree datapath module and upstream v4.9 module code?

I should remove “4.9” from the commit message. Either way, there are a lot of upstream patches not yet backported to the OVS tree:

- 018c1dda5f ("openvswitch: 802.1AD Flow handling, actions, vlan parsing, netlink attributes”)
  - 20ecf1e4e3 ("openvswitch: vlan: remove wrong likely statement”)
  - 72ec108d70 ("openvswitch: fix vlan subtraction from packet length”)
  - 3145c037e7 ("openvswitch: add NETIF_F_HW_VLAN_STAG_TX to internal dev”)
- 48d2ab609b ("net: mpls: Fixups for GSO”)
  - f7d49bce8e ("openvswitch: mpls: set network header correctly on key extract”)
  - c66549ffd6 ("openvswitch: correctly fragment packet with mpls headers”)
- 40773966cc ("openvswitch: fix flow stats accounting when node 0 is not possible”)
- db74a3335e ("openvswitch: use percpu flow stats”)
- 190aa3e778 (“openvswitch: Fix Frame-size larger than 1024 bytes warning.")
- 2279994d07 (“openvswitch: avoid resetting flow key while installing new flow.”)
- 85de4a2101 (“openvswitch: use mpls_hdr”)
- f33eb0cf99 ("openvswitch: remove unused functions”)
- 76e4cc7731 ("openvswitch: remove unnecessary EXPORT_SYMBOLs”)
- 91572088e3 (“net: use core MTU range checking in core net infra”)
- 08733a0cb7 ("netfilter: handle NF_REPEAT from nf_conntrack_in()”)
- 738314a084 ("openvswitch: use hard_header_len instead of hardcoded ETH_HLEN”)
- 329f45bc4f ("openvswitch: add mac_proto field to the flow key”)
- e2d9d8358c (“openvswitch: pass mac_proto to ovs_vport_send”)
- 1560a074df (“openvswitch: support MPLS push and pop for L3 packets”)
- 5108bbaddc (“openvswitch: add processing of L3 packets”)
- 0a6410fbde (“openvswitch: netlink: support L3 packets”)
- 91820da6ae ("openvswitch: add Ethernet push and pop actions”)
- 217ac77a3c (“openvswitch: allow L3 netdev ports”)
- c7d03a00b5 (“netns: make struct pernet_operations::id unsigned int”)

  Jarno
Jarno Rajahalme Dec. 10, 2016, 1:24 a.m. UTC | #5
Thanks for the reviews!

Pushed to master with a better title and a compiletime_assert() for GENL_ID_GENERATE, if it is defined.

  Jarno

> On Dec 9, 2016, at 2:36 PM, Pravin Shelar <pshelar@ovn.org> wrote:
> 
> On Wed, Dec 7, 2016 at 4:31 PM, Jarno Rajahalme <jarno@ovn.org <mailto:jarno@ovn.org>> wrote:
>> This patch allows openvswitch kernel module in the OVS tree to be
>> compiled against the current net-next Linux kernel.  The changes are
>> due to these upstream commits:
>> 
>> 56989f6d856 ("genetlink: mark families as __ro_after_init")
>> 489111e5c25 ("genetlink: statically initialize families")
>> a07ea4d9941 ("genetlink: no longer support using static family IDs")
>> 
>> struct genl_family initialization is changed be completely static and
>> to include the new (in Linux 4.6) __ro_after_init attribute.  Compat
>> code defines it as an empty macro if not defined already.
>> 
>> GENL_ID_GENERATE is no longer defined, but since it was defined as 0,
>> it is safe to drop it from all initializers also on older Linux
>> versions.
>> 
> 
> Patch looks good to me too.
> Can you add assert for GENL_ID_GENERATE value, to catch case where it
> is not the case.
> 
> Thanks.
Jarno Rajahalme Dec. 10, 2016, 1:27 a.m. UTC | #6
> On Dec 8, 2016, at 9:38 PM, Andy Zhou <azhou@ovn.org> wrote:
> 
> 
> 
> On Wed, Dec 7, 2016 at 4:31 PM, Jarno Rajahalme <jarno@ovn.org <mailto:jarno@ovn.org>> wrote:
> This patch allows openvswitch kernel module in the OVS tree to be
> compiled against the current net-next Linux kernel.  The changes are
> due to these upstream commits:
> 
> 56989f6d856 ("genetlink: mark families as __ro_after_init")
> 489111e5c25 ("genetlink: statically initialize families")
> a07ea4d9941 ("genetlink: no longer support using static family IDs")
> 
> struct genl_family initialization is changed be completely static and
> to include the new (in Linux 4.6) __ro_after_init attribute.  Compat
> code defines it as an empty macro if not defined already.
> 
> GENL_ID_GENERATE is no longer defined, but since it was defined as 0,
> it is safe to drop it from all initializers also on older Linux
> versions.
> 
> Tested with current Linux net-next (4.9) and 3.16.
> 
> Signed-off-by: Jarno Rajahalme <jarno@ovn.org <mailto:jarno@ovn.org>>
> 
> Tested building linux 4.0-4.9. 
> 
> Acked-by: Andy Zhou <azhou@ovn.org <mailto:azhou@ovn.org>>
> 

Thanks!

> When compiling with 4.0, I got the following warnning. It does not seem to be related to this patch.
> 
> /home/azhou/projs/ovs/datapath/linux/datapath.c: In function ‘ovs_flow_cmd_set’:
> /home/azhou/projs/ovs/datapath/linux/datapath.c:1232:1: warning: the frame size of 1040 bytes is larger than 1024 bytes [-Wframe-larger-than=]
>  }
>  ^

Right, not related. Upstream commit 190aa3e778 (“openvswitch: Fix Frame-size larger than 1024 bytes warning.”) fixes similar problem for ovs_flow_cmd_new().

  Jarno
diff mbox

Patch

diff --git a/acinclude.m4 b/acinclude.m4
index c4f331c..866e437 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -134,10 +134,10 @@  AC_DEFUN([OVS_CHECK_LINUX], [
     AC_MSG_RESULT([$kversion])
 
     if test "$version" -ge 4; then
-       if test "$version" = 4 && test "$patchlevel" -le 8; then
+       if test "$version" = 4 && test "$patchlevel" -le 9; then
           : # Linux 4.x
        else
-          AC_ERROR([Linux kernel in $KBUILD is version $kversion, but version newer than 4.8.x is not supported (please refer to the FAQ for advice)])
+          AC_ERROR([Linux kernel in $KBUILD is version $kversion, but version newer than 4.9.x is not supported (please refer to the FAQ for advice)])
        fi
     elif test "$version" = 3 && test "$patchlevel" -ge 10; then
        : # Linux 3.x
diff --git a/datapath/datapath.c b/datapath/datapath.c
index 3822129..ce2364a 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -682,8 +682,7 @@  static struct genl_ops dp_packet_genl_ops[] = {
 	}
 };
 
-static struct genl_family dp_packet_genl_family = {
-	.id = GENL_ID_GENERATE,
+static struct genl_family dp_packet_genl_family __ro_after_init = {
 	.hdrsize = sizeof(struct ovs_header),
 	.name = OVS_PACKET_FAMILY,
 	.version = OVS_PACKET_VERSION,
@@ -692,6 +691,7 @@  static struct genl_family dp_packet_genl_family = {
 	.parallel_ops = true,
 	.ops = dp_packet_genl_ops,
 	.n_ops = ARRAY_SIZE(dp_packet_genl_ops),
+	.module = THIS_MODULE,
 };
 
 static void get_dp_stats(const struct datapath *dp, struct ovs_dp_stats *stats,
@@ -1447,8 +1447,7 @@  static struct genl_ops dp_flow_genl_ops[] = {
 	},
 };
 
-static struct genl_family dp_flow_genl_family = {
-	.id = GENL_ID_GENERATE,
+static struct genl_family dp_flow_genl_family __ro_after_init = {
 	.hdrsize = sizeof(struct ovs_header),
 	.name = OVS_FLOW_FAMILY,
 	.version = OVS_FLOW_VERSION,
@@ -1459,6 +1458,7 @@  static struct genl_family dp_flow_genl_family = {
 	.n_ops = ARRAY_SIZE(dp_flow_genl_ops),
 	.mcgrps = &ovs_dp_flow_multicast_group,
 	.n_mcgrps = 1,
+	.module = THIS_MODULE,
 };
 
 static size_t ovs_dp_cmd_msg_size(void)
@@ -1832,8 +1832,7 @@  static struct genl_ops dp_datapath_genl_ops[] = {
 	},
 };
 
-static struct genl_family dp_datapath_genl_family = {
-	.id = GENL_ID_GENERATE,
+static struct genl_family dp_datapath_genl_family __ro_after_init = {
 	.hdrsize = sizeof(struct ovs_header),
 	.name = OVS_DATAPATH_FAMILY,
 	.version = OVS_DATAPATH_VERSION,
@@ -1844,6 +1843,7 @@  static struct genl_family dp_datapath_genl_family = {
 	.n_ops = ARRAY_SIZE(dp_datapath_genl_ops),
 	.mcgrps = &ovs_dp_datapath_multicast_group,
 	.n_mcgrps = 1,
+	.module = THIS_MODULE,
 };
 
 /* Called with ovs_mutex or RCU read lock. */
@@ -2254,8 +2254,7 @@  static struct genl_ops dp_vport_genl_ops[] = {
 	},
 };
 
-struct genl_family dp_vport_genl_family = {
-	.id = GENL_ID_GENERATE,
+struct genl_family dp_vport_genl_family __ro_after_init = {
 	.hdrsize = sizeof(struct ovs_header),
 	.name = OVS_VPORT_FAMILY,
 	.version = OVS_VPORT_VERSION,
@@ -2266,6 +2265,7 @@  struct genl_family dp_vport_genl_family = {
 	.n_ops = ARRAY_SIZE(dp_vport_genl_ops),
 	.mcgrps = &ovs_dp_vport_multicast_group,
 	.n_mcgrps = 1,
+	.module = THIS_MODULE,
 };
 
 static struct genl_family *dp_genl_families[] = {
@@ -2283,7 +2283,7 @@  static void dp_unregister_genl(int n_families)
 		genl_unregister_family(dp_genl_families[i]);
 }
 
-static int dp_register_genl(void)
+static int __init dp_register_genl(void)
 {
 	int err;
 	int i;
diff --git a/datapath/linux/Modules.mk b/datapath/linux/Modules.mk
index 26f6d22..985ffed 100644
--- a/datapath/linux/Modules.mk
+++ b/datapath/linux/Modules.mk
@@ -29,6 +29,7 @@  openvswitch_headers += \
 	linux/compat/gso.h \
 	linux/compat/include/linux/percpu.h \
 	linux/compat/include/linux/bug.h \
+	linux/compat/include/linux/cache.h \
 	linux/compat/include/linux/compiler.h \
 	linux/compat/include/linux/compiler-gcc.h \
 	linux/compat/include/linux/cpumask.h \
diff --git a/datapath/linux/compat/include/linux/cache.h b/datapath/linux/compat/include/linux/cache.h
new file mode 100644
index 0000000..917defa
--- /dev/null
+++ b/datapath/linux/compat/include/linux/cache.h
@@ -0,0 +1,10 @@ 
+#ifndef __LINUX_CACHE_WRAPPER_H
+#define __LINUX_CACHE_WRAPPER_H 1
+
+#include_next <linux/cache.h>
+
+#ifndef __ro_after_init
+#define __ro_after_init
+#endif
+
+#endif