diff mbox series

[net-next,1/5] tipc: obsolete TIPC_ZONE_SCOPE

Message ID 1521128935-6141-3-git-send-email-jon.maloy@ericsson.com
State Accepted, archived
Delegated to: David Miller
Headers show
Series tipc: obsolete zone concept | expand

Commit Message

Jon Maloy March 15, 2018, 3:48 p.m. UTC
Publications for TIPC_CLUSTER_SCOPE and TIPC_ZONE_SCOPE are in all
aspects handled the same way, both on the publishing node and on the
receiving nodes.

Despite previous ambitions to the contrary, this is never going to change,
so we take the conseqeunce of this and obsolete TIPC_ZONE_SCOPE and related
macros/functions. Whenever a user is doing a bind() or a sendmsg() attempt
using ZONE_SCOPE we translate this internally to CLUSTER_SCOPE, while we
remain compatible with users and remote nodes still using ZONE_SCOPE.

Furthermore, the non-formalized scope value 0 has always been permitted
for use during lookup, with the same meaning as ZONE_SCOPE/CLUSTER_SCOPE.
We now permit it even as binding scope, but for compatibility reasons we
choose to not change the value of TIPC_CLUSTER_SCOPE.

Acked-by: Ying Xue <ying.xue@windriver.com>
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
 include/uapi/linux/tipc.h | 102 ++++++++++++++++++++++++----------------------
 net/tipc/addr.c           |  31 --------------
 net/tipc/addr.h           |  10 +++++
 net/tipc/msg.c            |   2 +-
 net/tipc/name_table.c     |   3 +-
 net/tipc/net.c            |   2 +-
 net/tipc/socket.c         |  15 ++++---
 7 files changed, 77 insertions(+), 88 deletions(-)

Comments

Jiri Pirko March 15, 2018, 4:11 p.m. UTC | #1
Thu, Mar 15, 2018 at 04:48:51PM CET, jon.maloy@ericsson.com wrote:
>Publications for TIPC_CLUSTER_SCOPE and TIPC_ZONE_SCOPE are in all
>aspects handled the same way, both on the publishing node and on the
>receiving nodes.
>
>Despite previous ambitions to the contrary, this is never going to change,
>so we take the conseqeunce of this and obsolete TIPC_ZONE_SCOPE and related
>macros/functions. Whenever a user is doing a bind() or a sendmsg() attempt
>using ZONE_SCOPE we translate this internally to CLUSTER_SCOPE, while we
>remain compatible with users and remote nodes still using ZONE_SCOPE.
>
>Furthermore, the non-formalized scope value 0 has always been permitted
>for use during lookup, with the same meaning as ZONE_SCOPE/CLUSTER_SCOPE.
>We now permit it even as binding scope, but for compatibility reasons we
>choose to not change the value of TIPC_CLUSTER_SCOPE.
>
>Acked-by: Ying Xue <ying.xue@windriver.com>
>Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>

[...]


>diff --git a/include/uapi/linux/tipc.h b/include/uapi/linux/tipc.h
>index 14bacc7..4ac9f1f 100644
>--- a/include/uapi/linux/tipc.h
>+++ b/include/uapi/linux/tipc.h
>@@ -61,50 +61,6 @@ struct tipc_name_seq {
> 	__u32 upper;
> };
> 
>-/* TIPC Address Size, Offset, Mask specification for Z.C.N
>- */
>-#define TIPC_NODE_BITS          12
>-#define TIPC_CLUSTER_BITS       12
>-#define TIPC_ZONE_BITS          8
>-
>-#define TIPC_NODE_OFFSET        0
>-#define TIPC_CLUSTER_OFFSET     TIPC_NODE_BITS
>-#define TIPC_ZONE_OFFSET        (TIPC_CLUSTER_OFFSET + TIPC_CLUSTER_BITS)
>-
>-#define TIPC_NODE_SIZE          ((1UL << TIPC_NODE_BITS) - 1)
>-#define TIPC_CLUSTER_SIZE       ((1UL << TIPC_CLUSTER_BITS) - 1)
>-#define TIPC_ZONE_SIZE          ((1UL << TIPC_ZONE_BITS) - 1)
>-
>-#define TIPC_NODE_MASK		(TIPC_NODE_SIZE << TIPC_NODE_OFFSET)
>-#define TIPC_CLUSTER_MASK	(TIPC_CLUSTER_SIZE << TIPC_CLUSTER_OFFSET)
>-#define TIPC_ZONE_MASK		(TIPC_ZONE_SIZE << TIPC_ZONE_OFFSET)
>-
>-#define TIPC_ZONE_CLUSTER_MASK (TIPC_ZONE_MASK | TIPC_CLUSTER_MASK)
>-
>-static inline __u32 tipc_addr(unsigned int zone,
>-			      unsigned int cluster,
>-			      unsigned int node)
>-{
>-	return (zone << TIPC_ZONE_OFFSET) |
>-		(cluster << TIPC_CLUSTER_OFFSET) |
>-		node;
>-}
>-
>-static inline unsigned int tipc_zone(__u32 addr)
>-{
>-	return addr >> TIPC_ZONE_OFFSET;
>-}
>-
>-static inline unsigned int tipc_cluster(__u32 addr)
>-{
>-	return (addr & TIPC_CLUSTER_MASK) >> TIPC_CLUSTER_OFFSET;
>-}
>-
>-static inline unsigned int tipc_node(__u32 addr)
>-{
>-	return addr & TIPC_NODE_MASK;
>-}

If someone includes tipc.h and uses any of this, your patch is going to
break his compilation. Would anyone have good reason to use any of this?
Jon Maloy March 15, 2018, 4:27 p.m. UTC | #2
No, it won't. I just moved those functions and #defines to the bottom of the same file, and marked them as 'deprecated'.

BR
///jon

> -----Original Message-----
> From: Jiri Pirko [mailto:jiri@resnulli.us]
> Sent: Thursday, March 15, 2018 12:11
> To: Jon Maloy <jon.maloy@ericsson.com>
> Cc: davem@davemloft.net; netdev@vger.kernel.org; Mohan Krishna Ghanta
> Krishnamurthy <mohan.krishna.ghanta.krishnamurthy@ericsson.com>; Tung
> Quang Nguyen <tung.q.nguyen@dektech.com.au>; Hoang Huu Le
> <hoang.h.le@dektech.com.au>; Canh Duc Luu
> <canh.d.luu@dektech.com.au>; Ying Xue <ying.xue@windriver.com>; tipc-
> discussion@lists.sourceforge.net
> Subject: Re: [net-next 1/5] tipc: obsolete TIPC_ZONE_SCOPE
> 
> Thu, Mar 15, 2018 at 04:48:51PM CET, jon.maloy@ericsson.com wrote:
> >Publications for TIPC_CLUSTER_SCOPE and TIPC_ZONE_SCOPE are in all
> >aspects handled the same way, both on the publishing node and on the
> >receiving nodes.
> >
> >Despite previous ambitions to the contrary, this is never going to
> >change, so we take the conseqeunce of this and obsolete
> TIPC_ZONE_SCOPE
> >and related macros/functions. Whenever a user is doing a bind() or a
> >sendmsg() attempt using ZONE_SCOPE we translate this internally to
> >CLUSTER_SCOPE, while we remain compatible with users and remote nodes
> still using ZONE_SCOPE.
> >
> >Furthermore, the non-formalized scope value 0 has always been permitted
> >for use during lookup, with the same meaning as
> ZONE_SCOPE/CLUSTER_SCOPE.
> >We now permit it even as binding scope, but for compatibility reasons
> >we choose to not change the value of TIPC_CLUSTER_SCOPE.
> >
> >Acked-by: Ying Xue <ying.xue@windriver.com>
> >Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
> 
> [...]
> 
> 
> >diff --git a/include/uapi/linux/tipc.h b/include/uapi/linux/tipc.h
> >index 14bacc7..4ac9f1f 100644
> >--- a/include/uapi/linux/tipc.h
> >+++ b/include/uapi/linux/tipc.h
> >@@ -61,50 +61,6 @@ struct tipc_name_seq {
> > 	__u32 upper;
> > };
> >
> >-/* TIPC Address Size, Offset, Mask specification for Z.C.N
> >- */
> >-#define TIPC_NODE_BITS          12
> >-#define TIPC_CLUSTER_BITS       12
> >-#define TIPC_ZONE_BITS          8
> >-
> >-#define TIPC_NODE_OFFSET        0
> >-#define TIPC_CLUSTER_OFFSET     TIPC_NODE_BITS
> >-#define TIPC_ZONE_OFFSET        (TIPC_CLUSTER_OFFSET +
> TIPC_CLUSTER_BITS)
> >-
> >-#define TIPC_NODE_SIZE          ((1UL << TIPC_NODE_BITS) - 1)
> >-#define TIPC_CLUSTER_SIZE       ((1UL << TIPC_CLUSTER_BITS) - 1)
> >-#define TIPC_ZONE_SIZE          ((1UL << TIPC_ZONE_BITS) - 1)
> >-
> >-#define TIPC_NODE_MASK		(TIPC_NODE_SIZE <<
> TIPC_NODE_OFFSET)
> >-#define TIPC_CLUSTER_MASK	(TIPC_CLUSTER_SIZE <<
> TIPC_CLUSTER_OFFSET)
> >-#define TIPC_ZONE_MASK		(TIPC_ZONE_SIZE <<
> TIPC_ZONE_OFFSET)
> >-
> >-#define TIPC_ZONE_CLUSTER_MASK (TIPC_ZONE_MASK |
> TIPC_CLUSTER_MASK)
> >-
> >-static inline __u32 tipc_addr(unsigned int zone,
> >-			      unsigned int cluster,
> >-			      unsigned int node)
> >-{
> >-	return (zone << TIPC_ZONE_OFFSET) |
> >-		(cluster << TIPC_CLUSTER_OFFSET) |
> >-		node;
> >-}
> >-
> >-static inline unsigned int tipc_zone(__u32 addr) -{
> >-	return addr >> TIPC_ZONE_OFFSET;
> >-}
> >-
> >-static inline unsigned int tipc_cluster(__u32 addr) -{
> >-	return (addr & TIPC_CLUSTER_MASK) >> TIPC_CLUSTER_OFFSET;
> >-}
> >-
> >-static inline unsigned int tipc_node(__u32 addr) -{
> >-	return addr & TIPC_NODE_MASK;
> >-}
> 
> If someone includes tipc.h and uses any of this, your patch is going to break
> his compilation. Would anyone have good reason to use any of this?
Jiri Pirko March 15, 2018, 4:37 p.m. UTC | #3
Thu, Mar 15, 2018 at 05:27:17PM CET, jon.maloy@ericsson.com wrote:
>No, it won't. I just moved those functions and #defines to the bottom of the same file, and marked them as 'deprecated'.

Ah. That I missed. Thanks!
diff mbox series

Patch

diff --git a/include/uapi/linux/tipc.h b/include/uapi/linux/tipc.h
index 14bacc7..4ac9f1f 100644
--- a/include/uapi/linux/tipc.h
+++ b/include/uapi/linux/tipc.h
@@ -61,50 +61,6 @@  struct tipc_name_seq {
 	__u32 upper;
 };
 
-/* TIPC Address Size, Offset, Mask specification for Z.C.N
- */
-#define TIPC_NODE_BITS          12
-#define TIPC_CLUSTER_BITS       12
-#define TIPC_ZONE_BITS          8
-
-#define TIPC_NODE_OFFSET        0
-#define TIPC_CLUSTER_OFFSET     TIPC_NODE_BITS
-#define TIPC_ZONE_OFFSET        (TIPC_CLUSTER_OFFSET + TIPC_CLUSTER_BITS)
-
-#define TIPC_NODE_SIZE          ((1UL << TIPC_NODE_BITS) - 1)
-#define TIPC_CLUSTER_SIZE       ((1UL << TIPC_CLUSTER_BITS) - 1)
-#define TIPC_ZONE_SIZE          ((1UL << TIPC_ZONE_BITS) - 1)
-
-#define TIPC_NODE_MASK		(TIPC_NODE_SIZE << TIPC_NODE_OFFSET)
-#define TIPC_CLUSTER_MASK	(TIPC_CLUSTER_SIZE << TIPC_CLUSTER_OFFSET)
-#define TIPC_ZONE_MASK		(TIPC_ZONE_SIZE << TIPC_ZONE_OFFSET)
-
-#define TIPC_ZONE_CLUSTER_MASK (TIPC_ZONE_MASK | TIPC_CLUSTER_MASK)
-
-static inline __u32 tipc_addr(unsigned int zone,
-			      unsigned int cluster,
-			      unsigned int node)
-{
-	return (zone << TIPC_ZONE_OFFSET) |
-		(cluster << TIPC_CLUSTER_OFFSET) |
-		node;
-}
-
-static inline unsigned int tipc_zone(__u32 addr)
-{
-	return addr >> TIPC_ZONE_OFFSET;
-}
-
-static inline unsigned int tipc_cluster(__u32 addr)
-{
-	return (addr & TIPC_CLUSTER_MASK) >> TIPC_CLUSTER_OFFSET;
-}
-
-static inline unsigned int tipc_node(__u32 addr)
-{
-	return addr & TIPC_NODE_MASK;
-}
-
 /*
  * Application-accessible port name types
  */
@@ -117,9 +73,10 @@  static inline unsigned int tipc_node(__u32 addr)
 /*
  * Publication scopes when binding port names and port name sequences
  */
-#define TIPC_ZONE_SCOPE         1
-#define TIPC_CLUSTER_SCOPE      2
-#define TIPC_NODE_SCOPE         3
+enum tipc_scope {
+	TIPC_CLUSTER_SCOPE = 2, /* 0 can also be used */
+	TIPC_NODE_SCOPE    = 3
+};
 
 /*
  * Limiting values for messages
@@ -243,7 +200,7 @@  struct sockaddr_tipc {
 struct tipc_group_req {
 	__u32 type;      /* group id */
 	__u32 instance;  /* member id */
-	__u32 scope;     /* zone/cluster/node */
+	__u32 scope;     /* cluster/node */
 	__u32 flags;
 };
 
@@ -268,4 +225,53 @@  struct tipc_sioc_ln_req {
 	__u32 bearer_id;
 	char linkname[TIPC_MAX_LINK_NAME];
 };
+
+
+/* The macros and functions below are deprecated:
+ */
+
+#define TIPC_ZONE_SCOPE         1
+
+#define TIPC_NODE_BITS          12
+#define TIPC_CLUSTER_BITS       12
+#define TIPC_ZONE_BITS          8
+
+#define TIPC_NODE_OFFSET        0
+#define TIPC_CLUSTER_OFFSET     TIPC_NODE_BITS
+#define TIPC_ZONE_OFFSET        (TIPC_CLUSTER_OFFSET + TIPC_CLUSTER_BITS)
+
+#define TIPC_NODE_SIZE          ((1UL << TIPC_NODE_BITS) - 1)
+#define TIPC_CLUSTER_SIZE       ((1UL << TIPC_CLUSTER_BITS) - 1)
+#define TIPC_ZONE_SIZE          ((1UL << TIPC_ZONE_BITS) - 1)
+
+#define TIPC_NODE_MASK		(TIPC_NODE_SIZE << TIPC_NODE_OFFSET)
+#define TIPC_CLUSTER_MASK	(TIPC_CLUSTER_SIZE << TIPC_CLUSTER_OFFSET)
+#define TIPC_ZONE_MASK		(TIPC_ZONE_SIZE << TIPC_ZONE_OFFSET)
+
+#define TIPC_ZONE_CLUSTER_MASK (TIPC_ZONE_MASK | TIPC_CLUSTER_MASK)
+
+static inline __u32 tipc_addr(unsigned int zone,
+			      unsigned int cluster,
+			      unsigned int node)
+{
+	return (zone << TIPC_ZONE_OFFSET) |
+		(cluster << TIPC_CLUSTER_OFFSET) |
+		node;
+}
+
+static inline unsigned int tipc_zone(__u32 addr)
+{
+	return addr >> TIPC_ZONE_OFFSET;
+}
+
+static inline unsigned int tipc_cluster(__u32 addr)
+{
+	return (addr & TIPC_CLUSTER_MASK) >> TIPC_CLUSTER_OFFSET;
+}
+
+static inline unsigned int tipc_node(__u32 addr)
+{
+	return addr & TIPC_NODE_MASK;
+}
+
 #endif
diff --git a/net/tipc/addr.c b/net/tipc/addr.c
index 48fd3b5..97cd857 100644
--- a/net/tipc/addr.c
+++ b/net/tipc/addr.c
@@ -64,23 +64,6 @@  int in_own_node(struct net *net, u32 addr)
 }
 
 /**
- * addr_domain - convert 2-bit scope value to equivalent message lookup domain
- *
- * Needed when address of a named message must be looked up a second time
- * after a network hop.
- */
-u32 addr_domain(struct net *net, u32 sc)
-{
-	struct tipc_net *tn = net_generic(net, tipc_net_id);
-
-	if (likely(sc == TIPC_NODE_SCOPE))
-		return tn->own_addr;
-	if (sc == TIPC_CLUSTER_SCOPE)
-		return tipc_cluster_mask(tn->own_addr);
-	return tipc_zone_mask(tn->own_addr);
-}
-
-/**
  * tipc_addr_domain_valid - validates a network domain address
  *
  * Accepts <Z.C.N>, <Z.C.0>, <Z.0.0>, and <0.0.0>,
@@ -124,20 +107,6 @@  int tipc_in_scope(u32 domain, u32 addr)
 	return 0;
 }
 
-/**
- * tipc_addr_scope - convert message lookup domain to a 2-bit scope value
- */
-int tipc_addr_scope(u32 domain)
-{
-	if (likely(!domain))
-		return TIPC_ZONE_SCOPE;
-	if (tipc_node(domain))
-		return TIPC_NODE_SCOPE;
-	if (tipc_cluster(domain))
-		return TIPC_CLUSTER_SCOPE;
-	return TIPC_ZONE_SCOPE;
-}
-
 char *tipc_addr_string_fill(char *string, u32 addr)
 {
 	snprintf(string, 16, "<%u.%u.%u>",
diff --git a/net/tipc/addr.h b/net/tipc/addr.h
index bebb347..2ecf5a5 100644
--- a/net/tipc/addr.h
+++ b/net/tipc/addr.h
@@ -60,6 +60,16 @@  static inline u32 tipc_cluster_mask(u32 addr)
 	return addr & TIPC_ZONE_CLUSTER_MASK;
 }
 
+static inline int tipc_node2scope(u32 node)
+{
+	return node ? TIPC_NODE_SCOPE : TIPC_CLUSTER_SCOPE;
+}
+
+static inline int tipc_scope2node(struct net *net, int sc)
+{
+	return sc != TIPC_NODE_SCOPE ? 0 : tipc_own_addr(net);
+}
+
 u32 tipc_own_addr(struct net *net);
 int in_own_cluster(struct net *net, u32 addr);
 int in_own_cluster_exact(struct net *net, u32 addr);
diff --git a/net/tipc/msg.c b/net/tipc/msg.c
index 4e1c6f6..b6c45dc 100644
--- a/net/tipc/msg.c
+++ b/net/tipc/msg.c
@@ -580,7 +580,7 @@  bool tipc_msg_lookup_dest(struct net *net, struct sk_buff *skb, int *err)
 	msg = buf_msg(skb);
 	if (msg_reroute_cnt(msg))
 		return false;
-	dnode = addr_domain(net, msg_lookup_scope(msg));
+	dnode = tipc_scope2node(net, msg_lookup_scope(msg));
 	dport = tipc_nametbl_translate(net, msg_nametype(msg),
 				       msg_nameinst(msg), &dnode);
 	if (!dport)
diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c
index e01c9c6..6772390 100644
--- a/net/tipc/name_table.c
+++ b/net/tipc/name_table.c
@@ -473,8 +473,7 @@  struct publication *tipc_nametbl_insert_publ(struct net *net, u32 type,
 	struct name_seq *seq = nametbl_find_seq(net, type);
 	int index = hash(type);
 
-	if ((scope < TIPC_ZONE_SCOPE) || (scope > TIPC_NODE_SCOPE) ||
-	    (lower > upper)) {
+	if (scope > TIPC_NODE_SCOPE || lower > upper) {
 		pr_debug("Failed to publish illegal {%u,%u,%u} with scope %u\n",
 			 type, lower, upper, scope);
 		return NULL;
diff --git a/net/tipc/net.c b/net/tipc/net.c
index 1a2fde0..5c4c440 100644
--- a/net/tipc/net.c
+++ b/net/tipc/net.c
@@ -118,7 +118,7 @@  int tipc_net_start(struct net *net, u32 addr)
 	tipc_sk_reinit(net);
 
 	tipc_nametbl_publish(net, TIPC_CFG_SRV, tn->own_addr, tn->own_addr,
-			     TIPC_ZONE_SCOPE, 0, tn->own_addr);
+			     TIPC_CLUSTER_SCOPE, 0, tn->own_addr);
 
 	pr_info("Started in network mode\n");
 	pr_info("Own node address %s, network identity %u\n",
diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 8b04e60..910d382 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -644,7 +644,7 @@  static int tipc_bind(struct socket *sock, struct sockaddr *uaddr,
 		goto exit;
 	}
 
-	res = (addr->scope > 0) ?
+	res = (addr->scope >= 0) ?
 		tipc_sk_publish(tsk, addr->scope, &addr->addr.nameseq) :
 		tipc_sk_withdraw(tsk, -addr->scope, &addr->addr.nameseq);
 exit:
@@ -1280,8 +1280,8 @@  static int __tipc_sendmsg(struct socket *sock, struct msghdr *m, size_t dlen)
 	struct tipc_msg *hdr = &tsk->phdr;
 	struct tipc_name_seq *seq;
 	struct sk_buff_head pkts;
-	u32 type, inst, domain;
 	u32 dnode, dport;
+	u32 type, inst;
 	int mtu, rc;
 
 	if (unlikely(dlen > TIPC_MAX_USER_MSG_SIZE))
@@ -1332,13 +1332,12 @@  static int __tipc_sendmsg(struct socket *sock, struct msghdr *m, size_t dlen)
 	if (dest->addrtype == TIPC_ADDR_NAME) {
 		type = dest->addr.name.name.type;
 		inst = dest->addr.name.name.instance;
-		domain = dest->addr.name.domain;
-		dnode = domain;
+		dnode = dest->addr.name.domain;
 		msg_set_type(hdr, TIPC_NAMED_MSG);
 		msg_set_hdr_sz(hdr, NAMED_H_SIZE);
 		msg_set_nametype(hdr, type);
 		msg_set_nameinst(hdr, inst);
-		msg_set_lookup_scope(hdr, tipc_addr_scope(domain));
+		msg_set_lookup_scope(hdr, tipc_node2scope(dnode));
 		dport = tipc_nametbl_translate(net, type, inst, &dnode);
 		msg_set_destnode(hdr, dnode);
 		msg_set_destport(hdr, dport);
@@ -2592,6 +2591,9 @@  static int tipc_sk_publish(struct tipc_sock *tsk, uint scope,
 	struct publication *publ;
 	u32 key;
 
+	if (scope != TIPC_NODE_SCOPE)
+		scope = TIPC_CLUSTER_SCOPE;
+
 	if (tipc_sk_connected(sk))
 		return -EINVAL;
 	key = tsk->portid + tsk->pub_count + 1;
@@ -2617,6 +2619,9 @@  static int tipc_sk_withdraw(struct tipc_sock *tsk, uint scope,
 	struct publication *safe;
 	int rc = -EINVAL;
 
+	if (scope != TIPC_NODE_SCOPE)
+		scope = TIPC_CLUSTER_SCOPE;
+
 	list_for_each_entry_safe(publ, safe, &tsk->publications, pport_list) {
 		if (seq) {
 			if (publ->scope != scope)