Re: [PATCH net-next v1 1/2] tipc: move netlink policies to netlink.h

2016-01-07 Thread Richard Alpe
On 2016-01-05 22:47, David Miller wrote:
> From: Richard Alpe 
> Date: Tue, 5 Jan 2016 10:56:16 +0100
> 
>> Make the c files less cluttered and enable netlink attributes to be
>> shared between files. This will prove useful in a future patch where a
>> node message will contain a nested network.
>>
>> Signed-off-by: Richard Alpe 
>> Acked-by: Jon Maloy 
> 
> netlink.h is included by more than one file, which means the tables
> (might) be instantiated multiple times.
I thought about that, but I assumed unreferenced tables would be
optimized away by the compiler (?)
> 
> I'd really recommend not putting such tables in a header file that
> is used in this way.
Alright, I will drop the patch.

Do you have any suggestion on how to handle the case where a policy
(table) is shared between c files, like the net policy in the
subsequent patch? I could perhaps use extern in the c files (which
seems frown upon) or duplicate the policy? :-S

Thanks for the review
Richard
> 
> Thanks.
> 

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH net-next v1 1/2] tipc: move netlink policies to netlink.h

2016-01-05 Thread David Miller
From: Richard Alpe 
Date: Tue, 5 Jan 2016 10:56:16 +0100

> Make the c files less cluttered and enable netlink attributes to be
> shared between files. This will prove useful in a future patch where a
> node message will contain a nested network.
> 
> Signed-off-by: Richard Alpe 
> Acked-by: Jon Maloy 

netlink.h is included by more than one file, which means the tables
(might) be instantiated multiple times.

I'd really recommend not putting such tables in a header file that
is used in this way.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH net-next v1 1/2] tipc: move netlink policies to netlink.h

2016-01-05 Thread Richard Alpe
Make the c files less cluttered and enable netlink attributes to be
shared between files. This will prove useful in a future patch where a
node message will contain a nested network.

Signed-off-by: Richard Alpe 
Acked-by: Jon Maloy 
---
 net/tipc/bearer.c | 19 +---
 net/tipc/link.c   |  8 -
 net/tipc/name_table.c |  7 +
 net/tipc/net.c|  6 +---
 net/tipc/netlink.c| 13 +---
 net/tipc/netlink.h| 85 +++
 net/tipc/node.c   | 23 +-
 net/tipc/socket.c |  9 +-
 net/tipc/udp_media.c  |  9 +-
 9 files changed, 92 insertions(+), 87 deletions(-)

diff --git a/net/tipc/bearer.c b/net/tipc/bearer.c
index 802ffad..d654cfe 100644
--- a/net/tipc/bearer.c
+++ b/net/tipc/bearer.c
@@ -40,6 +40,7 @@
 #include "link.h"
 #include "discover.h"
 #include "bcast.h"
+#include "netlink.h"
 
 #define MAX_ADDR_STR 60
 
@@ -53,24 +54,6 @@ static struct tipc_media * const media_info_array[] = {
 #endif
NULL
 };
-
-static const struct nla_policy
-tipc_nl_bearer_policy[TIPC_NLA_BEARER_MAX + 1] = {
-   [TIPC_NLA_BEARER_UNSPEC]= { .type = NLA_UNSPEC },
-   [TIPC_NLA_BEARER_NAME] = {
-   .type = NLA_STRING,
-   .len = TIPC_MAX_BEARER_NAME
-   },
-   [TIPC_NLA_BEARER_PROP]  = { .type = NLA_NESTED },
-   [TIPC_NLA_BEARER_DOMAIN]= { .type = NLA_U32 }
-};
-
-static const struct nla_policy tipc_nl_media_policy[TIPC_NLA_MEDIA_MAX + 1] = {
-   [TIPC_NLA_MEDIA_UNSPEC] = { .type = NLA_UNSPEC },
-   [TIPC_NLA_MEDIA_NAME]   = { .type = NLA_STRING },
-   [TIPC_NLA_MEDIA_PROP]   = { .type = NLA_NESTED }
-};
-
 static void bearer_disable(struct net *net, struct tipc_bearer *b);
 
 /**
diff --git a/net/tipc/link.c b/net/tipc/link.c
index 0c2944f..cb807be 100644
--- a/net/tipc/link.c
+++ b/net/tipc/link.c
@@ -196,14 +196,6 @@ struct tipc_link {
 static const char *link_co_err = "Link tunneling error, ";
 static const char *link_rst_msg = "Resetting link ";
 
-/* Properties valid for media, bearar and link */
-static const struct nla_policy tipc_nl_prop_policy[TIPC_NLA_PROP_MAX + 1] = {
-   [TIPC_NLA_PROP_UNSPEC]  = { .type = NLA_UNSPEC },
-   [TIPC_NLA_PROP_PRIO]= { .type = NLA_U32 },
-   [TIPC_NLA_PROP_TOL] = { .type = NLA_U32 },
-   [TIPC_NLA_PROP_WIN] = { .type = NLA_U32 }
-};
-
 /* Send states for broadcast NACKs
  */
 enum {
diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c
index 91fce70..75992b5 100644
--- a/net/tipc/name_table.c
+++ b/net/tipc/name_table.c
@@ -44,15 +44,10 @@
 #include "addr.h"
 #include "node.h"
 #include 
+#include "netlink.h"
 
 #define TIPC_NAMETBL_SIZE 1024 /* must be a power of 2 */
 
-static const struct nla_policy
-tipc_nl_name_table_policy[TIPC_NLA_NAME_TABLE_MAX + 1] = {
-   [TIPC_NLA_NAME_TABLE_UNSPEC]= { .type = NLA_UNSPEC },
-   [TIPC_NLA_NAME_TABLE_PUBL]  = { .type = NLA_NESTED }
-};
-
 /**
  * struct name_info - name sequence publication info
  * @node_list: circular list of publications made by own node
diff --git a/net/tipc/net.c b/net/tipc/net.c
index 77bf911..d7a5c11 100644
--- a/net/tipc/net.c
+++ b/net/tipc/net.c
@@ -41,11 +41,7 @@
 #include "socket.h"
 #include "node.h"
 #include "bcast.h"
-
-static const struct nla_policy tipc_nl_net_policy[TIPC_NLA_NET_MAX + 1] = {
-   [TIPC_NLA_NET_UNSPEC]   = { .type = NLA_UNSPEC },
-   [TIPC_NLA_NET_ID]   = { .type = NLA_U32 }
-};
+#include "netlink.h"
 
 /*
  * The TIPC locking policy is designed to ensure a very fine locking
diff --git a/net/tipc/netlink.c b/net/tipc/netlink.c
index 8975b01..234cb93 100644
--- a/net/tipc/netlink.c
+++ b/net/tipc/netlink.c
@@ -42,18 +42,7 @@
 #include "node.h"
 #include "net.h"
 #include 
-
-static const struct nla_policy tipc_nl_policy[TIPC_NLA_MAX + 1] = {
-   [TIPC_NLA_UNSPEC]   = { .type = NLA_UNSPEC, },
-   [TIPC_NLA_BEARER]   = { .type = NLA_NESTED, },
-   [TIPC_NLA_SOCK] = { .type = NLA_NESTED, },
-   [TIPC_NLA_PUBL] = { .type = NLA_NESTED, },
-   [TIPC_NLA_LINK] = { .type = NLA_NESTED, },
-   [TIPC_NLA_MEDIA]= { .type = NLA_NESTED, },
-   [TIPC_NLA_NODE] = { .type = NLA_NESTED, },
-   [TIPC_NLA_NET]  = { .type = NLA_NESTED, },
-   [TIPC_NLA_NAME_TABLE]   = { .type = NLA_NESTED, }
-};
+#include "netlink.h"
 
 /* Users of the legacy API (tipc-config) can't handle that we add operations,
  * so we have a separate genl handling for the new API.
diff --git a/net/tipc/netlink.h b/net/tipc/netlink.h
index 08a1db6..ff7a39da 100644
--- a/net/tipc/netlink.h
+++ b/net/tipc/netlink.h
@@ -36,7 +36,92 @@
 #ifndef _TIPC_NETLINK_H
 #define _TIPC_NETLINK_H
 
+#include 
+
 extern struct genl_family tipc_genl_family;
+
+static const struct nla_policy tipc_nl_policy[TIPC_NLA_MAX + 1] = {
+   [TIPC_NLA_