[ovs-dev] [PATCH V4 3/9] compat: Move genl_ops policy to genl_family

Greg Rose gvrose8192 at gmail.com
Fri Mar 6 22:37:15 UTC 2020


From: Johannes Berg <johannes.berg at intel.com>

Upstream commit:
    commit 3b0f31f2b8c9fb348e4530b88f6b64f9621f83d6
    Author: Johannes Berg <johannes.berg at intel.com>
    Date:   Thu Mar 21 22:51:02 2019 +0100

    genetlink: make policy common to family

    Since maxattr is common, the policy can't really differ sanely,
    so make it common as well.

    The only user that did in fact manage to make a non-common policy
    is taskstats, which has to be really careful about it (since it's
    still using a common maxattr!). This is no longer supported, but
    we can fake it using pre_doit.

    This reduces the size of e.g. nl80211.o (which has lots of commands):

       text	   data	    bss	    dec	    hex	filename
     398745	  14323	   2240	 415308	  6564c	net/wireless/nl80211.o (before)
     397913	  14331	   2240	 414484	  65314	net/wireless/nl80211.o (after)
    --------------------------------
       -832      +8       0    -824

    Which is obviously just 8 bytes for each command, and an added 8
    bytes for the new policy pointer. I'm not sure why the ops list is
    counted as .text though.

    Most of the code transformations were done using the following spatch:
        @ops@
        identifier OPS;
        expression POLICY;
        @@
        struct genl_ops OPS[] = {
        ...,
         {
        -	.policy = POLICY,
         },
        ...
        };

        @@
        identifier ops.OPS;
        expression ops.POLICY;
        identifier fam;
        expression M;
        @@
        struct genl_family fam = {
                .ops = OPS,
                .maxattr = M,
        +       .policy = POLICY,
                ...
        };

    This also gets rid of devlink_nl_cmd_region_read_dumpit() accessing
    the cb->data as ops, which we want to change in a later genl patch.

    Signed-off-by: Johannes Berg <johannes.berg at intel.com>
    Signed-off-by: David S. Miller <davem at davemloft.net>

Since commit 3b0f31f2b8c9f ("genetlink: make policy common to family")
the policy field of the genl_ops structure has been moved into the
genl_family structure.  Add necessary compat layer infrastructure
to still support older kernels.

Acked-by: Yi-Hung Wei <yihung.wei at gmail.com>
Signed-off-by: Greg Rose <gvrose8192 at gmail.com>
---
V3 - Add policy back to genl_family instead of completely removing
     it.
---
 acinclude.m4         |  3 +++
 datapath/conntrack.c |  9 +++++++++
 datapath/datapath.c  | 38 ++++++++++++++++++++++++++++++++++++++
 datapath/meter.c     | 11 +++++++++++
 4 files changed, 61 insertions(+)

diff --git a/acinclude.m4 b/acinclude.m4
index 3a555ad..729d2c6 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -1050,6 +1050,9 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
                   [OVS_DEFINE([HAVE_DST_OPS_CONFIRM_NEIGH])])
   OVS_GREP_IFELSE([$KSRC/include/net/inet_frag.h], [fqdir],
                   [OVS_DEFINE([HAVE_INET_FRAG_FQDIR])])
+  OVS_FIND_FIELD_IFELSE([$KSRC/include/net/genetlink.h], [genl_ops],
+                        [policy],
+                        [OVS_DEFINE([HAVE_GENL_OPS_POLICY])])
 
   if cmp -s datapath/linux/kcompat.h.new \
             datapath/linux/kcompat.h >/dev/null 2>&1; then
diff --git a/datapath/conntrack.c b/datapath/conntrack.c
index 838cf63..b7eb53f 100644
--- a/datapath/conntrack.c
+++ b/datapath/conntrack.c
@@ -2312,7 +2312,9 @@ static struct genl_ops ct_limit_genl_ops[] = {
 #endif
 		.flags = GENL_ADMIN_PERM, /* Requires CAP_NET_ADMIN
 					   * privilege. */
+#ifdef HAVE_GENL_OPS_POLICY
 		.policy = ct_limit_policy,
+#endif
 		.doit = ovs_ct_limit_cmd_set,
 	},
 	{ .cmd = OVS_CT_LIMIT_CMD_DEL,
@@ -2321,7 +2323,9 @@ static struct genl_ops ct_limit_genl_ops[] = {
 #endif
 		.flags = GENL_ADMIN_PERM, /* Requires CAP_NET_ADMIN
 					   * privilege. */
+#ifdef HAVE_GENL_OPS_POLICY
 		.policy = ct_limit_policy,
+#endif
 		.doit = ovs_ct_limit_cmd_del,
 	},
 	{ .cmd = OVS_CT_LIMIT_CMD_GET,
@@ -2329,7 +2333,9 @@ static struct genl_ops ct_limit_genl_ops[] = {
 		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
 #endif
 		.flags = 0,		  /* OK for unprivileged users. */
+#ifdef HAVE_GENL_OPS_POLICY
 		.policy = ct_limit_policy,
+#endif
 		.doit = ovs_ct_limit_cmd_get,
 	},
 };
@@ -2343,6 +2349,9 @@ struct genl_family dp_ct_limit_genl_family __ro_after_init = {
 	.name = OVS_CT_LIMIT_FAMILY,
 	.version = OVS_CT_LIMIT_VERSION,
 	.maxattr = OVS_CT_LIMIT_ATTR_MAX,
+#ifndef HAVE_GENL_OPS_POLICY
+	.policy = ct_limit_policy,
+#endif
 	.netnsok = true,
 	.parallel_ops = true,
 	.ops = ct_limit_genl_ops,
diff --git a/datapath/datapath.c b/datapath/datapath.c
index 853bfb5..6f74c8f 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -657,7 +657,9 @@ static struct genl_ops dp_packet_genl_ops[] = {
 	  .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
 #endif
 	  .flags = GENL_UNS_ADMIN_PERM, /* Requires CAP_NET_ADMIN privilege. */
+#ifdef HAVE_GENL_OPS_POLICY
 	  .policy = packet_policy,
+#endif
 	  .doit = ovs_packet_cmd_execute
 	}
 };
@@ -667,6 +669,9 @@ static struct genl_family dp_packet_genl_family __ro_after_init = {
 	.name = OVS_PACKET_FAMILY,
 	.version = OVS_PACKET_VERSION,
 	.maxattr = OVS_PACKET_ATTR_MAX,
+#ifndef HAVE_GENL_OPS_POLICY
+	.policy = packet_policy,
+#endif
 	.netnsok = true,
 	.parallel_ops = true,
 	.ops = dp_packet_genl_ops,
@@ -1449,7 +1454,9 @@ static const struct genl_ops dp_flow_genl_ops[] = {
 	  .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
 #endif
 	  .flags = GENL_UNS_ADMIN_PERM, /* Requires CAP_NET_ADMIN privilege. */
+#ifdef HAVE_GENL_OPS_POLICY
 	  .policy = flow_policy,
+#endif
 	  .doit = ovs_flow_cmd_new
 	},
 	{ .cmd = OVS_FLOW_CMD_DEL,
@@ -1457,7 +1464,9 @@ static const struct genl_ops dp_flow_genl_ops[] = {
 	  .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
 #endif
 	  .flags = GENL_UNS_ADMIN_PERM, /* Requires CAP_NET_ADMIN privilege. */
+#ifdef HAVE_GENL_OPS_POLICY
 	  .policy = flow_policy,
+#endif
 	  .doit = ovs_flow_cmd_del
 	},
 	{ .cmd = OVS_FLOW_CMD_GET,
@@ -1465,7 +1474,9 @@ static const struct genl_ops dp_flow_genl_ops[] = {
 	  .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
 #endif
 	  .flags = 0,		    /* OK for unprivileged users. */
+#ifdef HAVE_GENL_OPS_POLICY
 	  .policy = flow_policy,
+#endif
 	  .doit = ovs_flow_cmd_get,
 	  .dumpit = ovs_flow_cmd_dump
 	},
@@ -1474,7 +1485,9 @@ static const struct genl_ops dp_flow_genl_ops[] = {
 	  .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
 #endif
 	  .flags = GENL_UNS_ADMIN_PERM, /* Requires CAP_NET_ADMIN privilege. */
+#ifdef HAVE_GENL_OPS_POLICY
 	  .policy = flow_policy,
+#endif
 	  .doit = ovs_flow_cmd_set,
 	},
 };
@@ -1484,6 +1497,9 @@ static struct genl_family dp_flow_genl_family __ro_after_init = {
 	.name = OVS_FLOW_FAMILY,
 	.version = OVS_FLOW_VERSION,
 	.maxattr = OVS_FLOW_ATTR_MAX,
+#ifndef HAVE_GENL_OPS_POLICY
+	.policy = flow_policy,
+#endif
 	.netnsok = true,
 	.parallel_ops = true,
 	.ops = dp_flow_genl_ops,
@@ -1853,7 +1869,9 @@ static const struct genl_ops dp_datapath_genl_ops[] = {
 	  .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
 #endif
 	  .flags = GENL_UNS_ADMIN_PERM, /* Requires CAP_NET_ADMIN privilege. */
+#ifdef HAVE_GENL_OPS_POLICY
 	  .policy = datapath_policy,
+#endif
 	  .doit = ovs_dp_cmd_new
 	},
 	{ .cmd = OVS_DP_CMD_DEL,
@@ -1861,7 +1879,9 @@ static const struct genl_ops dp_datapath_genl_ops[] = {
 	  .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
 #endif
 	  .flags = GENL_UNS_ADMIN_PERM, /* Requires CAP_NET_ADMIN privilege. */
+#ifdef HAVE_GENL_OPS_POLICY
 	  .policy = datapath_policy,
+#endif
 	  .doit = ovs_dp_cmd_del
 	},
 	{ .cmd = OVS_DP_CMD_GET,
@@ -1869,7 +1889,9 @@ static const struct genl_ops dp_datapath_genl_ops[] = {
 	  .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
 #endif
 	  .flags = 0,		    /* OK for unprivileged users. */
+#ifdef HAVE_GENL_OPS_POLICY
 	  .policy = datapath_policy,
+#endif
 	  .doit = ovs_dp_cmd_get,
 	  .dumpit = ovs_dp_cmd_dump
 	},
@@ -1878,7 +1900,9 @@ static const struct genl_ops dp_datapath_genl_ops[] = {
 	  .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
 #endif
 	  .flags = GENL_UNS_ADMIN_PERM, /* Requires CAP_NET_ADMIN privilege. */
+#ifdef HAVE_GENL_OPS_POLICY
 	  .policy = datapath_policy,
+#endif
 	  .doit = ovs_dp_cmd_set,
 	},
 };
@@ -1888,6 +1912,9 @@ static struct genl_family dp_datapath_genl_family __ro_after_init = {
 	.name = OVS_DATAPATH_FAMILY,
 	.version = OVS_DATAPATH_VERSION,
 	.maxattr = OVS_DP_ATTR_MAX,
+#ifndef HAVE_GENL_OPS_POLICY
+	.policy = datapath_policy,
+#endif
 	.netnsok = true,
 	.parallel_ops = true,
 	.ops = dp_datapath_genl_ops,
@@ -2310,7 +2337,9 @@ static const struct genl_ops dp_vport_genl_ops[] = {
 	  .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
 #endif
 	  .flags = GENL_UNS_ADMIN_PERM, /* Requires CAP_NET_ADMIN privilege. */
+#ifdef HAVE_GENL_OPS_POLICY
 	  .policy = vport_policy,
+#endif
 	  .doit = ovs_vport_cmd_new
 	},
 	{ .cmd = OVS_VPORT_CMD_DEL,
@@ -2318,7 +2347,9 @@ static const struct genl_ops dp_vport_genl_ops[] = {
 	  .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
 #endif
 	  .flags = GENL_UNS_ADMIN_PERM, /* Requires CAP_NET_ADMIN privilege. */
+#ifdef HAVE_GENL_OPS_POLICY
 	  .policy = vport_policy,
+#endif
 	  .doit = ovs_vport_cmd_del
 	},
 	{ .cmd = OVS_VPORT_CMD_GET,
@@ -2326,7 +2357,9 @@ static const struct genl_ops dp_vport_genl_ops[] = {
 	  .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
 #endif
 	  .flags = 0,		    /* OK for unprivileged users. */
+#ifdef HAVE_GENL_OPS_POLICY
 	  .policy = vport_policy,
+#endif
 	  .doit = ovs_vport_cmd_get,
 	  .dumpit = ovs_vport_cmd_dump
 	},
@@ -2335,7 +2368,9 @@ static const struct genl_ops dp_vport_genl_ops[] = {
 	  .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
 #endif
 	  .flags = GENL_UNS_ADMIN_PERM, /* Requires CAP_NET_ADMIN privilege. */
+#ifdef HAVE_GENL_OPS_POLICY
 	  .policy = vport_policy,
+#endif
 	  .doit = ovs_vport_cmd_set,
 	},
 };
@@ -2345,6 +2380,9 @@ struct genl_family dp_vport_genl_family __ro_after_init = {
 	.name = OVS_VPORT_FAMILY,
 	.version = OVS_VPORT_VERSION,
 	.maxattr = OVS_VPORT_ATTR_MAX,
+#ifndef HAVE_GENL_OPS_POLICY
+	.policy = vport_policy,
+#endif
 	.netnsok = true,
 	.parallel_ops = true,
 	.ops = dp_vport_genl_ops,
diff --git a/datapath/meter.c b/datapath/meter.c
index 7d8f51a..8cecd5a 100644
--- a/datapath/meter.c
+++ b/datapath/meter.c
@@ -542,7 +542,9 @@ static struct genl_ops dp_meter_genl_ops[] = {
 		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
 #endif
 		.flags = 0,		  /* OK for unprivileged users. */
+#ifdef HAVE_GENL_OPS_POLICY
 		.policy = meter_policy,
+#endif
 		.doit = ovs_meter_cmd_features
 	},
 	{ .cmd = OVS_METER_CMD_SET,
@@ -552,7 +554,9 @@ static struct genl_ops dp_meter_genl_ops[] = {
 		.flags = GENL_ADMIN_PERM, /* Requires CAP_NET_ADMIN
 					   *  privilege.
 					   */
+#ifdef HAVE_GENL_OPS_POLICY
 		.policy = meter_policy,
+#endif
 		.doit = ovs_meter_cmd_set,
 	},
 	{ .cmd = OVS_METER_CMD_GET,
@@ -560,7 +564,9 @@ static struct genl_ops dp_meter_genl_ops[] = {
 		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
 #endif
 		.flags = 0,		  /* OK for unprivileged users. */
+#ifdef HAVE_GENL_OPS_POLICY
 		.policy = meter_policy,
+#endif
 		.doit = ovs_meter_cmd_get,
 	},
 	{ .cmd = OVS_METER_CMD_DEL,
@@ -570,7 +576,9 @@ static struct genl_ops dp_meter_genl_ops[] = {
 		.flags = GENL_ADMIN_PERM, /* Requires CAP_NET_ADMIN
 					   *  privilege.
 					   */
+#ifdef HAVE_GENL_OPS_POLICY
 		.policy = meter_policy,
+#endif
 		.doit = ovs_meter_cmd_del
 	},
 };
@@ -584,6 +592,9 @@ struct genl_family dp_meter_genl_family __ro_after_init = {
 	.name = OVS_METER_FAMILY,
 	.version = OVS_METER_VERSION,
 	.maxattr = OVS_METER_ATTR_MAX,
+#ifndef HAVE_GENL_OPS_POLICY
+	.policy = meter_policy,
+#endif
 	.netnsok = true,
 	.parallel_ops = true,
 	.ops = dp_meter_genl_ops,
-- 
1.8.3.1



More information about the dev mailing list