tipc-discussion — User questions & problem reports, project news, developer discussion

[tipc-discussion] [net-next v2 04/16] tipc: simplify signature of tipc_namtbl_publish()

[<jm...@re...>]

From: Jon Maloy <jm...@re...>

Using the new address structure tipc_uaddr, we simplify the signature
of function tipc_sk_publish() and tipc_namtbl_publish() so that fewer
parameters need to be passed around.

Signed-off-by: Jon Maloy <jm...@re...>
Acked-by: Ying Xue <yin...@wi...>
Acked-by: Hoang Le <hoa...@de...>
---
 net/tipc/name_table.c | 10 +++---
 net/tipc/name_table.h |  6 ++--
 net/tipc/net.c        |  8 +++--
 net/tipc/node.c       | 29 ++++++++---------
 net/tipc/socket.c     | 73 +++++++++++++++++++++++--------------------
 5 files changed, 68 insertions(+), 58 deletions(-)

diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c
index c37cef09b54c..7b309fdd0090 100644
--- a/net/tipc/name_table.c
+++ b/net/tipc/name_table.c
@@ -740,9 +740,8 @@ void tipc_nametbl_build_group(struct net *net, struct tipc_group *grp,
 
 /* tipc_nametbl_publish - add service binding to name table
  */
-struct publication *tipc_nametbl_publish(struct net *net, u32 type, u32 lower,
-					 u32 upper, u32 scope, u32 port,
-					 u32 key)
+struct publication *tipc_nametbl_publish(struct net *net, struct tipc_uaddr *ua,
+					 struct tipc_socket_addr *sk, u32 key)
 {
 	struct name_table *nt = tipc_name_table(net);
 	struct tipc_net *tn = tipc_net(net);
@@ -757,8 +756,9 @@ struct publication *tipc_nametbl_publish(struct net *net, u32 type, u32 lower,
 		goto exit;
 	}
 
-	p = tipc_nametbl_insert_publ(net, type, lower, upper, scope,
-				     tipc_own_addr(net), port, key);
+	p = tipc_nametbl_insert_publ(net, ua->sr.type, ua->sr.lower,
+				     ua->sr.upper, ua->scope,
+				     sk->node, sk->ref, key);
 	if (p) {
 		nt->local_publ_count++;
 		skb = tipc_named_publish(net, p);
diff --git a/net/tipc/name_table.h b/net/tipc/name_table.h
index d9ad119f966b..47a8c266bcc8 100644
--- a/net/tipc/name_table.h
+++ b/net/tipc/name_table.h
@@ -42,6 +42,7 @@ struct tipc_subscription;
 struct tipc_plist;
 struct tipc_nlist;
 struct tipc_group;
+struct tipc_uaddr;
 
 /*
  * TIPC name types reserved for internal TIPC use (both current and planned)
@@ -120,9 +121,8 @@ void tipc_nametbl_lookup_dst_nodes(struct net *net, u32 type, u32 lower,
 bool tipc_nametbl_lookup(struct net *net, u32 type, u32 instance, u32 domain,
 			 struct list_head *dsts, int *dstcnt, u32 exclude,
 			 bool all);
-struct publication *tipc_nametbl_publish(struct net *net, u32 type, u32 lower,
-					 u32 upper, u32 scope, u32 port,
-					 u32 key);
+struct publication *tipc_nametbl_publish(struct net *net, struct tipc_uaddr *ua,
+					 struct tipc_socket_addr *sk, u32 key);
 int tipc_nametbl_withdraw(struct net *net, u32 type, u32 lower, u32 upper,
 			  u32 key);
 struct publication *tipc_nametbl_insert_publ(struct net *net, u32 type,
diff --git a/net/tipc/net.c b/net/tipc/net.c
index a129f661bee3..3f927949bb23 100644
--- a/net/tipc/net.c
+++ b/net/tipc/net.c
@@ -125,6 +125,11 @@ int tipc_net_init(struct net *net, u8 *node_id, u32 addr)
 static void tipc_net_finalize(struct net *net, u32 addr)
 {
 	struct tipc_net *tn = tipc_net(net);
+	struct tipc_socket_addr sk = {0, addr};
+	struct tipc_uaddr ua;
+
+	tipc_uaddr(&ua, TIPC_SERVICE_RANGE, TIPC_CLUSTER_SCOPE,
+		   TIPC_NODE_STATE, addr, addr);
 
 	if (cmpxchg(&tn->node_addr, 0, addr))
 		return;
@@ -132,8 +137,7 @@ static void tipc_net_finalize(struct net *net, u32 addr)
 	tipc_named_reinit(net);
 	tipc_sk_reinit(net);
 	tipc_mon_reinit_self(net);
-	tipc_nametbl_publish(net, TIPC_NODE_STATE, addr, addr,
-			     TIPC_CLUSTER_SCOPE, 0, addr);
+	tipc_nametbl_publish(net, &ua, &sk, addr);
 }
 
 void tipc_net_finalize_work(struct work_struct *work)
diff --git a/net/tipc/node.c b/net/tipc/node.c
index 008670d1f43e..7c12c9aa3926 100644
--- a/net/tipc/node.c
+++ b/net/tipc/node.c
@@ -393,21 +393,23 @@ static void tipc_node_write_unlock_fast(struct tipc_node *n)
 
 static void tipc_node_write_unlock(struct tipc_node *n)
 {
+	struct tipc_socket_addr sk;
 	struct net *net = n->net;
-	u32 addr = 0;
 	u32 flags = n->action_flags;
-	u32 link_id = 0;
-	u32 bearer_id;
 	struct list_head *publ_list;
+	struct tipc_uaddr ua;
+	u32 bearer_id;
 
 	if (likely(!flags)) {
 		write_unlock_bh(&n->lock);
 		return;
 	}
 
-	addr = n->addr;
-	link_id = n->link_id;
-	bearer_id = link_id & 0xffff;
+	tipc_uaddr(&ua, TIPC_SERVICE_RANGE, TIPC_NODE_SCOPE,
+		   TIPC_LINK_STATE, n->addr, n->addr);
+	sk.ref = n->link_id;
+	sk.node = n->addr;
+	bearer_id = n->link_id & 0xffff;
 	publ_list = &n->publ_list;
 
 	n->action_flags &= ~(TIPC_NOTIFY_NODE_DOWN | TIPC_NOTIFY_NODE_UP |
@@ -416,20 +418,19 @@ static void tipc_node_write_unlock(struct tipc_node *n)
 	write_unlock_bh(&n->lock);
 
 	if (flags & TIPC_NOTIFY_NODE_DOWN)
-		tipc_publ_notify(net, publ_list, addr, n->capabilities);
+		tipc_publ_notify(net, publ_list, n->addr, n->capabilities);
 
 	if (flags & TIPC_NOTIFY_NODE_UP)
-		tipc_named_node_up(net, addr, n->capabilities);
+		tipc_named_node_up(net, n->addr, n->capabilities);
 
 	if (flags & TIPC_NOTIFY_LINK_UP) {
-		tipc_mon_peer_up(net, addr, bearer_id);
-		tipc_nametbl_publish(net, TIPC_LINK_STATE, addr, addr,
-				     TIPC_NODE_SCOPE, link_id, link_id);
+		tipc_mon_peer_up(net, n->addr, bearer_id);
+		tipc_nametbl_publish(net, &ua, &sk, n->link_id);
 	}
 	if (flags & TIPC_NOTIFY_LINK_DOWN) {
-		tipc_mon_peer_down(net, addr, bearer_id);
-		tipc_nametbl_withdraw(net, TIPC_LINK_STATE, addr,
-				      addr, link_id);
+		tipc_mon_peer_down(net, n->addr, bearer_id);
+		tipc_nametbl_withdraw(net, TIPC_LINK_STATE, n->addr,
+				      n->addr, n->link_id);
 	}
 }
 
diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index fe522d49f747..94847252a7b7 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -111,7 +111,6 @@ struct tipc_sock {
 	struct sock sk;
 	u32 conn_type;
 	u32 conn_instance;
-	int published;
 	u32 max_pkt;
 	u32 maxnagle;
 	u32 portid;
@@ -141,6 +140,7 @@ struct tipc_sock {
 	bool expect_ack;
 	bool nodelay;
 	bool group_is_open;
+	bool published;
 };
 
 static int tipc_sk_backlog_rcv(struct sock *sk, struct sk_buff *skb);
@@ -151,8 +151,7 @@ static int tipc_release(struct socket *sock);
 static int tipc_accept(struct socket *sock, struct socket *new_sock, int flags,
 		       bool kern);
 static void tipc_sk_timeout(struct timer_list *t);
-static int tipc_sk_publish(struct tipc_sock *tsk, uint scope,
-			   struct tipc_service_range const *seq);
+static int tipc_sk_publish(struct tipc_sock *tsk, struct tipc_uaddr *ua);
 static int tipc_sk_withdraw(struct tipc_sock *tsk, uint scope,
 			    struct tipc_service_range const *seq);
 static int tipc_sk_leave(struct tipc_sock *tsk);
@@ -677,22 +676,31 @@ static int tipc_release(struct socket *sock)
  */
 static int __tipc_bind(struct socket *sock, struct sockaddr *skaddr, int alen)
 {
-	struct sockaddr_tipc *addr = (struct sockaddr_tipc *)skaddr;
+	struct tipc_uaddr *ua = (struct tipc_uaddr *)skaddr;
 	struct tipc_sock *tsk = tipc_sk(sock->sk);
+	bool unbind = false;
 
 	if (unlikely(!alen))
 		return tipc_sk_withdraw(tsk, 0, NULL);
 
-	if (addr->addrtype == TIPC_SERVICE_ADDR)
-		addr->addr.nameseq.upper = addr->addr.nameseq.lower;
+	if (ua->addrtype == TIPC_SERVICE_ADDR) {
+		ua->addrtype = TIPC_SERVICE_RANGE;
+		ua->sr.upper = ua->sr.lower;
+	}
+	if (ua->scope < 0) {
+		unbind = true;
+		ua->scope = -ua->scope;
+	}
+	/* Users may still use deprecated TIPC_ZONE_SCOPE */
+	if (ua->scope != TIPC_NODE_SCOPE)
+		ua->scope = TIPC_CLUSTER_SCOPE;
 
 	if (tsk->group)
 		return -EACCES;
 
-	if (addr->scope >= 0)
-		return tipc_sk_publish(tsk, addr->scope, &addr->addr.nameseq);
-	else
-		return tipc_sk_withdraw(tsk, -addr->scope, &addr->addr.nameseq);
+	if (unbind)
+		return tipc_sk_withdraw(tsk, ua->scope, &ua->sr);
+	return tipc_sk_publish(tsk, ua);
 }
 
 int tipc_sk_bind(struct socket *sock, struct sockaddr *skaddr, int alen)
@@ -707,18 +715,17 @@ int tipc_sk_bind(struct socket *sock, struct sockaddr *skaddr, int alen)
 
 static int tipc_bind(struct socket *sock, struct sockaddr *skaddr, int alen)
 {
-	struct sockaddr_tipc *addr = (struct sockaddr_tipc *)skaddr;
+	struct tipc_uaddr *ua = (struct tipc_uaddr *)skaddr;
+	u32 atype = ua->addrtype;
 
 	if (alen) {
-		if (alen < sizeof(struct sockaddr_tipc))
+		if (!tipc_uaddr_valid(ua, alen))
 			return -EINVAL;
-		if (addr->family != AF_TIPC)
+		if (atype == TIPC_SOCKET_ADDR)
 			return -EAFNOSUPPORT;
-		if (addr->addrtype > TIPC_SERVICE_ADDR)
-			return -EAFNOSUPPORT;
-		if (addr->addr.nameseq.type < TIPC_RESERVED_TYPES) {
+		if (ua->sr.type < TIPC_RESERVED_TYPES) {
 			pr_warn_once("Can't bind to reserved service type %u\n",
-				     addr->addr.nameseq.type);
+				     ua->sr.type);
 			return -EACCES;
 		}
 	}
@@ -2891,31 +2898,28 @@ static void tipc_sk_timeout(struct timer_list *t)
 	sock_put(sk);
 }
 
-static int tipc_sk_publish(struct tipc_sock *tsk, uint scope,
-			   struct tipc_service_range const *seq)
+static int tipc_sk_publish(struct tipc_sock *tsk, struct tipc_uaddr *ua)
 {
 	struct sock *sk = &tsk->sk;
 	struct net *net = sock_net(sk);
-	struct publication *publ;
+	struct tipc_socket_addr skaddr;
+	struct publication *p;
 	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;
 	if (key == tsk->portid)
 		return -EADDRINUSE;
-
-	publ = tipc_nametbl_publish(net, seq->type, seq->lower, seq->upper,
-				    scope, tsk->portid, key);
-	if (unlikely(!publ))
+	skaddr.ref = tsk->portid;
+	skaddr.node = tipc_own_addr(net);
+	p = tipc_nametbl_publish(net, ua, &skaddr, key);
+	if (unlikely(!p))
 		return -EINVAL;
 
-	list_add(&publ->binding_sock, &tsk->publications);
+	list_add(&p->binding_sock, &tsk->publications);
 	tsk->pub_count++;
-	tsk->published = 1;
+	tsk->published = true;
 	return 0;
 }
 
@@ -3067,13 +3071,15 @@ static int tipc_sk_join(struct tipc_sock *tsk, struct tipc_group_req *mreq)
 	struct net *net = sock_net(&tsk->sk);
 	struct tipc_group *grp = tsk->group;
 	struct tipc_msg *hdr = &tsk->phdr;
-	struct tipc_service_range seq;
+	struct tipc_uaddr ua;
 	int rc;
 
 	if (mreq->type < TIPC_RESERVED_TYPES)
 		return -EACCES;
 	if (mreq->scope > TIPC_NODE_SCOPE)
 		return -EINVAL;
+	if (mreq->scope != TIPC_NODE_SCOPE)
+		mreq->scope = TIPC_CLUSTER_SCOPE;
 	if (grp)
 		return -EACCES;
 	grp = tipc_group_create(net, tsk->portid, mreq, &tsk->group_is_open);
@@ -3083,11 +3089,10 @@ static int tipc_sk_join(struct tipc_sock *tsk, struct tipc_group_req *mreq)
 	msg_set_lookup_scope(hdr, mreq->scope);
 	msg_set_nametype(hdr, mreq->type);
 	msg_set_dest_droppable(hdr, true);
-	seq.type = mreq->type;
-	seq.lower = mreq->instance;
-	seq.upper = seq.lower;
 	tipc_nametbl_build_group(net, grp, mreq->type, mreq->scope);
-	rc = tipc_sk_publish(tsk, mreq->scope, &seq);
+	tipc_uaddr(&ua, TIPC_SERVICE_RANGE, mreq->scope,
+		   mreq->type, mreq->instance, mreq->instance);
+	rc = tipc_sk_publish(tsk, &ua);
 	if (rc) {
 		tipc_group_delete(net, grp);
 		tsk->group = NULL;
-- 
2.29.2

[tipc-discussion] [net-next v2 07/16] tipc: rename binding table lookup functions

[<jm...@re...>]

From: Jon Maloy <jm...@re...>

The binding table provides four different lookup functions, which
purpose is not obvious neither by their names nor by the (lack of)
descriptions.

We now give these functions names that better match their purposes,
and improve the comments that describe what they are doing.

Signed-off-by: Jon Maloy <jm...@re...>
Acked-by: Ying Xue <yin...@wi...>
Acked-by: Hoang Le <hoa...@de...>
---
 net/tipc/msg.c        |  4 ++--
 net/tipc/name_table.c | 51 ++++++++++++++++++++++++++++---------------
 net/tipc/name_table.h | 19 +++++++++-------
 net/tipc/socket.c     | 19 ++++++++--------
 4 files changed, 56 insertions(+), 37 deletions(-)

diff --git a/net/tipc/msg.c b/net/tipc/msg.c
index e9263280a2d4..25afb5949892 100644
--- a/net/tipc/msg.c
+++ b/net/tipc/msg.c
@@ -723,8 +723,8 @@ bool tipc_msg_lookup_dest(struct net *net, struct sk_buff *skb, int *err)
 	if (msg_reroute_cnt(msg))
 		return false;
 	dnode = tipc_scope2node(net, msg_lookup_scope(msg));
-	dport = tipc_nametbl_translate(net, msg_nametype(msg),
-				       msg_nameinst(msg), &dnode);
+	dport = tipc_nametbl_lookup_anycast(net, msg_nametype(msg),
+					    msg_nameinst(msg), &dnode);
 	if (!dport)
 		return false;
 	msg_incr_reroute_cnt(msg);
diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c
index 5676b8d4f08f..22616a943e70 100644
--- a/net/tipc/name_table.c
+++ b/net/tipc/name_table.c
@@ -544,24 +544,26 @@ struct publication *tipc_nametbl_remove_publ(struct net *net,
 }
 
 /**
- * tipc_nametbl_translate - perform service instance to socket translation
+ * tipc_nametbl_lookup_anycast - perform service instance to socket translation
  * @net: network namespace
  * @type: message type
  * @instance: message instance
  * @dnode: the search domain used during translation
  *
+ * On entry, 'dnode' is the search domain used during the lookup
+ *
  * On exit:
- * - if translation is deferred to another node, leave 'dnode' unchanged and
- * return 0
- * - if translation is attempted and succeeds, set 'dnode' to the publishing
- * node and return the published (non-zero) port number
- * - if translation is attempted and fails, set 'dnode' to 0 and return 0
+ * - if lookup is deferred to another node, leave 'dnode' unchanged and return 0
+ * - if lookup is attempted and succeeds, set 'dnode' to the publishing node and
+ *   return the published (non-zero) port number
+ * - if lookup is attempted and fails, set 'dnode' to 0 and return 0
  *
  * Note that for legacy users (node configured with Z.C.N address format) the
  * 'closest-first' lookup algorithm must be maintained, i.e., if dnode is 0
  * we must look in the local binding list first
  */
-u32 tipc_nametbl_translate(struct net *net, u32 type, u32 instance, u32 *dnode)
+u32 tipc_nametbl_lookup_anycast(struct net *net, u32 type,
+				u32 instance, u32 *dnode)
 {
 	struct tipc_net *tn = tipc_net(net);
 	bool legacy = tn->legacy_addr_format;
@@ -617,9 +619,15 @@ u32 tipc_nametbl_translate(struct net *net, u32 type, u32 instance, u32 *dnode)
 	return port;
 }
 
-bool tipc_nametbl_lookup(struct net *net, u32 type, u32 instance, u32 scope,
-			 struct list_head *dsts, int *dstcnt, u32 exclude,
-			 bool all)
+/* tipc_nametbl_lookup_group(): lookup destinaton(s) in a communication group
+ * Returns a list of one (== group anycast) or more (== group multicast)
+ * destination socket/node pairs matching the given address.
+ * The requester may or may not want to exclude himself from the list.
+ */
+bool tipc_nametbl_lookup_group(struct net *net, u32 type, u32 instance,
+			       u32 scope, struct list_head *dsts,
+			       int *dstcnt, u32 exclude,
+			       bool mcast)
 {
 	u32 self = tipc_own_addr(net);
 	struct service_range *sr;
@@ -646,7 +654,7 @@ bool tipc_nametbl_lookup(struct net *net, u32 type, u32 instance, u32 scope,
 			continue;
 		tipc_dest_push(dsts, p->sk.node, p->sk.ref);
 		(*dstcnt)++;
-		if (all)
+		if (mcast)
 			continue;
 		list_move_tail(&p->all_publ, &sr->all_publ);
 		break;
@@ -658,8 +666,14 @@ bool tipc_nametbl_lookup(struct net *net, u32 type, u32 instance, u32 scope,
 	return !list_empty(dsts);
 }
 
-void tipc_nametbl_mc_lookup(struct net *net, u32 type, u32 lower, u32 upper,
-			    u32 scope, bool exact, struct list_head *dports)
+/* tipc_nametbl_lookup_mcast_sockets(): look up node local destinaton sockets
+ *                                      matching the given address
+ * Used on nodes which have received a multicast/broadcast message
+ * Returns a list of local sockets
+ */
+void tipc_nametbl_lookup_mcast_sockets(struct net *net, u32 type, u32 lower,
+				       u32 upper, u32 scope, bool exact,
+				       struct list_head *dports)
 {
 	struct service_range *sr;
 	struct tipc_service *sc;
@@ -682,12 +696,13 @@ void tipc_nametbl_mc_lookup(struct net *net, u32 type, u32 lower, u32 upper,
 	rcu_read_unlock();
 }
 
-/* tipc_nametbl_lookup_dst_nodes - find broadcast destination nodes
- * - Creates list of nodes that overlap the given multicast address
- * - Determines if any node local destinations overlap
+/* tipc_nametbl_lookup_mcast_nodes(): look up all destination nodes matching
+ *                                    the given address. Used in sending node.
+ * Used on nodes which are sending out a multicast/broadcast message
+ * Returns a list of nodes, including own node if applicable
  */
-void tipc_nametbl_lookup_dst_nodes(struct net *net, u32 type, u32 lower,
-				   u32 upper, struct tipc_nlist *nodes)
+void tipc_nametbl_lookup_mcast_nodes(struct net *net, u32 type, u32 lower,
+				     u32 upper, struct tipc_nlist *nodes)
 {
 	struct service_range *sr;
 	struct tipc_service *sc;
diff --git a/net/tipc/name_table.h b/net/tipc/name_table.h
index 5a7c83d22ef9..07a297f22135 100644
--- a/net/tipc/name_table.h
+++ b/net/tipc/name_table.h
@@ -111,16 +111,19 @@ struct name_table {
 
 int tipc_nl_name_table_dump(struct sk_buff *skb, struct netlink_callback *cb);
 
-u32 tipc_nametbl_translate(struct net *net, u32 type, u32 instance, u32 *node);
-void tipc_nametbl_mc_lookup(struct net *net, u32 type, u32 lower, u32 upper,
-			    u32 scope, bool exact, struct list_head *dports);
+u32 tipc_nametbl_lookup_anycast(struct net *net, u32 type, u32 instance,
+				u32 *node);
+void tipc_nametbl_lookup_mcast_sockets(struct net *net, u32 type, u32 lower,
+				       u32 upper, u32 scope, bool exact,
+				   struct list_head *dports);
+void tipc_nametbl_lookup_mcast_nodes(struct net *net, u32 type, u32 lower,
+				     u32 upper, struct tipc_nlist *nodes);
+bool tipc_nametbl_lookup_group(struct net *net, u32 type, u32 instance,
+			       u32 domain, struct list_head *dsts,
+			       int *dstcnt, u32 exclude,
+			       bool all);
 void tipc_nametbl_build_group(struct net *net, struct tipc_group *grp,
 			      u32 type, u32 domain);
-void tipc_nametbl_lookup_dst_nodes(struct net *net, u32 type, u32 lower,
-				   u32 upper, struct tipc_nlist *nodes);
-bool tipc_nametbl_lookup(struct net *net, u32 type, u32 instance, u32 domain,
-			 struct list_head *dsts, int *dstcnt, u32 exclude,
-			 bool all);
 struct publication *tipc_nametbl_publish(struct net *net, struct tipc_uaddr *ua,
 					 struct tipc_socket_addr *sk, u32 key);
 void tipc_nametbl_withdraw(struct net *net, struct tipc_uaddr *ua,
diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 7f5722d3b3d0..b80c82d3ab87 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -863,8 +863,8 @@ static int tipc_sendmcast(struct  socket *sock, struct tipc_service_range *seq,
 
 	/* Lookup destination nodes */
 	tipc_nlist_init(&dsts, tipc_own_addr(net));
-	tipc_nametbl_lookup_dst_nodes(net, seq->type, seq->lower,
-				      seq->upper, &dsts);
+	tipc_nametbl_lookup_mcast_nodes(net, seq->type, seq->lower,
+					seq->upper, &dsts);
 	if (!dsts.local && !dsts.remote)
 		return -EHOSTUNREACH;
 
@@ -1032,8 +1032,9 @@ static int tipc_send_group_anycast(struct socket *sock, struct msghdr *m,
 
 		/* Look for a non-congested destination member, if any */
 		while (1) {
-			if (!tipc_nametbl_lookup(net, type, inst, scope, &dsts,
-						 &dstcnt, exclude, false))
+			if (!tipc_nametbl_lookup_group(net, type, inst, scope,
+						       &dsts, &dstcnt, exclude,
+						       false))
 				return -EHOSTUNREACH;
 			tipc_dest_pop(&dsts, &node, &port);
 			cong = tipc_group_cong(tsk->group, node, port, blks,
@@ -1179,8 +1180,8 @@ static int tipc_send_group_mcast(struct socket *sock, struct msghdr *m,
 	scope = msg_lookup_scope(hdr);
 	exclude = tipc_group_exclude(grp);
 
-	if (!tipc_nametbl_lookup(net, type, inst, scope, &dsts,
-				 &dstcnt, exclude, true))
+	if (!tipc_nametbl_lookup_group(net, type, inst, scope, &dsts,
+				       &dstcnt, exclude, true))
 		return -EHOSTUNREACH;
 
 	if (dstcnt == 1) {
@@ -1254,8 +1255,8 @@ void tipc_sk_mcast_rcv(struct net *net, struct sk_buff_head *arrvq,
 		}
 
 		/* Create destination port list: */
-		tipc_nametbl_mc_lookup(net, type, lower, upper,
-				       scope, exact, &dports);
+		tipc_nametbl_lookup_mcast_sockets(net, type, lower, upper,
+						  scope, exact, &dports);
 
 		/* Clone message per destination */
 		while (tipc_dest_pop(&dports, NULL, &portid)) {
@@ -1485,7 +1486,7 @@ static int __tipc_sendmsg(struct socket *sock, struct msghdr *m, size_t dlen)
 		type = dest->addr.name.name.type;
 		inst = dest->addr.name.name.instance;
 		dnode = dest->addr.name.domain;
-		dport = tipc_nametbl_translate(net, type, inst, &dnode);
+		dport = tipc_nametbl_lookup_anycast(net, type, inst, &dnode);
 		if (unlikely(!dport && !dnode))
 			return -EHOSTUNREACH;
 	} else if (dest->addrtype == TIPC_SOCKET_ADDR) {
-- 
2.29.2

[tipc-discussion] [net-next v2 01/16] tipc: re-organize members of struct publication

[<jm...@re...>]

From: Jon Maloy <jm...@re...>

In a future commit we will introduce more members to struct publication.
In order to keep this structure comprehensible we now group some of
its current fields into the sub-structures where they really belong,
- A struct tipc_service_range for the functional address the publication
  is representing.
- A struct tipc_socket_addr for the socket bound to that service range.

We also rename the stack variable 'publ' to just 'p' in a few places.
This is just as easy to understand in the given context, and keeps the
number of wrapped code lines to a minimum.

There are no functional changes in this commit.

Signed-off-by: Jon Maloy <jm...@re...>
Acked-by: Ying Xue <yin...@wi...>
Acked-by: Hoang Le <hoa...@de...>
---
 net/tipc/name_distr.c | 66 +++++++++++++++++++++----------------------
 net/tipc/name_table.c | 66 +++++++++++++++++++++----------------------
 net/tipc/name_table.h | 17 ++++-------
 net/tipc/socket.c     | 40 +++++++++++++-------------
 4 files changed, 92 insertions(+), 97 deletions(-)

diff --git a/net/tipc/name_distr.c b/net/tipc/name_distr.c
index 6cf57c3bfa27..1070b04d1126 100644
--- a/net/tipc/name_distr.c
+++ b/net/tipc/name_distr.c
@@ -1,8 +1,9 @@
 /*
  * net/tipc/name_distr.c: TIPC name distribution code
  *
- * Copyright (c) 2000-2006, 2014, Ericsson AB
+ * Copyright (c) 2000-2006, 2014-2019, Ericsson AB
  * Copyright (c) 2005, 2010-2011, Wind River Systems
+ * Copyright (c) 2020-2021, Red Hat Inc
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -55,10 +56,10 @@ struct distr_queue_item {
  */
 static void publ_to_item(struct distr_item *i, struct publication *p)
 {
-	i->type = htonl(p->type);
-	i->lower = htonl(p->lower);
-	i->upper = htonl(p->upper);
-	i->port = htonl(p->port);
+	i->type = htonl(p->sr.type);
+	i->lower = htonl(p->sr.lower);
+	i->upper = htonl(p->sr.upper);
+	i->port = htonl(p->sk.ref);
 	i->key = htonl(p->key);
 }
 
@@ -90,20 +91,20 @@ static struct sk_buff *named_prepare_buf(struct net *net, u32 type, u32 size,
 /**
  * tipc_named_publish - tell other nodes about a new publication by this node
  * @net: the associated network namespace
- * @publ: the new publication
+ * @p: the new publication
  */
-struct sk_buff *tipc_named_publish(struct net *net, struct publication *publ)
+struct sk_buff *tipc_named_publish(struct net *net, struct publication *p)
 {
 	struct name_table *nt = tipc_name_table(net);
 	struct distr_item *item;
 	struct sk_buff *skb;
 
-	if (publ->scope == TIPC_NODE_SCOPE) {
-		list_add_tail_rcu(&publ->binding_node, &nt->node_scope);
+	if (p->scope == TIPC_NODE_SCOPE) {
+		list_add_tail_rcu(&p->binding_node, &nt->node_scope);
 		return NULL;
 	}
 	write_lock_bh(&nt->cluster_scope_lock);
-	list_add_tail(&publ->binding_node, &nt->cluster_scope);
+	list_add_tail(&p->binding_node, &nt->cluster_scope);
 	write_unlock_bh(&nt->cluster_scope_lock);
 	skb = named_prepare_buf(net, PUBLICATION, ITEM_SIZE, 0);
 	if (!skb) {
@@ -113,25 +114,25 @@ struct sk_buff *tipc_named_publish(struct net *net, struct publication *publ)
 	msg_set_named_seqno(buf_msg(skb), nt->snd_nxt++);
 	msg_set_non_legacy(buf_msg(skb));
 	item = (struct distr_item *)msg_data(buf_msg(skb));
-	publ_to_item(item, publ);
+	publ_to_item(item, p);
 	return skb;
 }
 
 /**
  * tipc_named_withdraw - tell other nodes about a withdrawn publication by this node
  * @net: the associated network namespace
- * @publ: the withdrawn publication
+ * @p: the withdrawn publication
  */
-struct sk_buff *tipc_named_withdraw(struct net *net, struct publication *publ)
+struct sk_buff *tipc_named_withdraw(struct net *net, struct publication *p)
 {
 	struct name_table *nt = tipc_name_table(net);
 	struct distr_item *item;
 	struct sk_buff *skb;
 
 	write_lock_bh(&nt->cluster_scope_lock);
-	list_del(&publ->binding_node);
+	list_del(&p->binding_node);
 	write_unlock_bh(&nt->cluster_scope_lock);
-	if (publ->scope == TIPC_NODE_SCOPE)
+	if (p->scope == TIPC_NODE_SCOPE)
 		return NULL;
 
 	skb = named_prepare_buf(net, WITHDRAWAL, ITEM_SIZE, 0);
@@ -142,7 +143,7 @@ struct sk_buff *tipc_named_withdraw(struct net *net, struct publication *publ)
 	msg_set_named_seqno(buf_msg(skb), nt->snd_nxt++);
 	msg_set_non_legacy(buf_msg(skb));
 	item = (struct distr_item *)msg_data(buf_msg(skb));
-	publ_to_item(item, publ);
+	publ_to_item(item, p);
 	return skb;
 }
 
@@ -233,33 +234,32 @@ void tipc_named_node_up(struct net *net, u32 dnode, u16 capabilities)
 /**
  * tipc_publ_purge - remove publication associated with a failed node
  * @net: the associated network namespace
- * @publ: the publication to remove
+ * @p: the publication to remove
  * @addr: failed node's address
  *
  * Invoked for each publication issued by a newly failed node.
  * Removes publication structure from name table & deletes it.
  */
-static void tipc_publ_purge(struct net *net, struct publication *publ, u32 addr)
+static void tipc_publ_purge(struct net *net, struct publication *p, u32 addr)
 {
 	struct tipc_net *tn = tipc_net(net);
-	struct publication *p;
+	struct publication *_p;
 
 	spin_lock_bh(&tn->nametbl_lock);
-	p = tipc_nametbl_remove_publ(net, publ->type, publ->lower, publ->upper,
-				     publ->node, publ->key);
-	if (p)
-		tipc_node_unsubscribe(net, &p->binding_node, addr);
+	_p = tipc_nametbl_remove_publ(net, p->sr.type, p->sr.lower,
+				      p->sr.upper, p->sk.node, p->key);
+	if (_p)
+		tipc_node_unsubscribe(net, &_p->binding_node, addr);
 	spin_unlock_bh(&tn->nametbl_lock);
 
-	if (p != publ) {
+	if (_p != p) {
 		pr_err("Unable to remove publication from failed node\n"
 		       " (type=%u, lower=%u, node=0x%x, port=%u, key=%u)\n",
-		       publ->type, publ->lower, publ->node, publ->port,
-		       publ->key);
+		       p->sr.type, p->sr.lower, p->sk.node, p->sk.ref, p->key);
 	}
 
-	if (p)
-		kfree_rcu(p, rcu);
+	if (_p)
+		kfree_rcu(_p, rcu);
 }
 
 void tipc_publ_notify(struct net *net, struct list_head *nsub_list,
@@ -410,15 +410,15 @@ void tipc_named_reinit(struct net *net)
 {
 	struct name_table *nt = tipc_name_table(net);
 	struct tipc_net *tn = tipc_net(net);
-	struct publication *publ;
+	struct publication *p;
 	u32 self = tipc_own_addr(net);
 
 	spin_lock_bh(&tn->nametbl_lock);
 
-	list_for_each_entry_rcu(publ, &nt->node_scope, binding_node)
-		publ->node = self;
-	list_for_each_entry_rcu(publ, &nt->cluster_scope, binding_node)
-		publ->node = self;
+	list_for_each_entry_rcu(p, &nt->node_scope, binding_node)
+		p->sk.node = self;
+	list_for_each_entry_rcu(p, &nt->cluster_scope, binding_node)
+		p->sk.node = self;
 	nt->rc_dests = 0;
 	spin_unlock_bh(&tn->nametbl_lock);
 }
diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c
index ee5ac40ea2b6..c2410ba7be5c 100644
--- a/net/tipc/name_table.c
+++ b/net/tipc/name_table.c
@@ -3,7 +3,7 @@
  *
  * Copyright (c) 2000-2006, 2014-2018, Ericsson AB
  * Copyright (c) 2004-2008, 2010-2014, Wind River Systems
- * Copyright (c) 2020, Red Hat Inc
+ * Copyright (c) 2020-2021, Red Hat Inc
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -234,24 +234,24 @@ static struct publication *tipc_publ_create(u32 type, u32 lower, u32 upper,
 					    u32 scope, u32 node, u32 port,
 					    u32 key)
 {
-	struct publication *publ = kzalloc(sizeof(*publ), GFP_ATOMIC);
+	struct publication *p = kzalloc(sizeof(*p), GFP_ATOMIC);
 
-	if (!publ)
+	if (!p)
 		return NULL;
 
-	publ->type = type;
-	publ->lower = lower;
-	publ->upper = upper;
-	publ->scope = scope;
-	publ->node = node;
-	publ->port = port;
-	publ->key = key;
-	INIT_LIST_HEAD(&publ->binding_sock);
-	INIT_LIST_HEAD(&publ->binding_node);
-	INIT_LIST_HEAD(&publ->local_publ);
-	INIT_LIST_HEAD(&publ->all_publ);
-	INIT_LIST_HEAD(&publ->list);
-	return publ;
+	p->sr.type = type;
+	p->sr.lower = lower;
+	p->sr.upper = upper;
+	p->scope = scope;
+	p->sk.node = node;
+	p->sk.ref = port;
+	p->key = key;
+	INIT_LIST_HEAD(&p->binding_sock);
+	INIT_LIST_HEAD(&p->binding_node);
+	INIT_LIST_HEAD(&p->local_publ);
+	INIT_LIST_HEAD(&p->all_publ);
+	INIT_LIST_HEAD(&p->list);
+	return p;
 }
 
 /**
@@ -347,7 +347,7 @@ static struct publication *tipc_service_insert_publ(struct net *net,
 
 	/* Return if the publication already exists */
 	list_for_each_entry(p, &sr->all_publ, all_publ) {
-		if (p->key == key && (!p->node || p->node == node))
+		if (p->key == key && (!p->sk.node || p->sk.node == node))
 			return NULL;
 	}
 
@@ -363,8 +363,8 @@ static struct publication *tipc_service_insert_publ(struct net *net,
 
 	/* Any subscriptions waiting for notification?  */
 	list_for_each_entry_safe(sub, tmp, &sc->subscriptions, service_list) {
-		tipc_sub_report_overlap(sub, p->lower, p->upper, TIPC_PUBLISHED,
-					p->port, p->node, p->scope, first);
+		tipc_sub_report_overlap(sub, p->sr.lower, p->sr.upper, TIPC_PUBLISHED,
+					p->sk.ref, p->sk.node, p->scope, first);
 	}
 	return p;
 err:
@@ -384,7 +384,7 @@ static struct publication *tipc_service_remove_publ(struct service_range *sr,
 	struct publication *p;
 
 	list_for_each_entry(p, &sr->all_publ, all_publ) {
-		if (p->key != key || (node && node != p->node))
+		if (p->key != key || (node && node != p->sk.node))
 			continue;
 		list_del(&p->all_publ);
 		list_del(&p->local_publ);
@@ -452,8 +452,8 @@ static void tipc_service_subscribe(struct tipc_service *service,
 	/* Sort the publications before reporting */
 	list_sort(NULL, &publ_list, tipc_publ_sort);
 	list_for_each_entry_safe(p, tmp, &publ_list, list) {
-		tipc_sub_report_overlap(sub, p->lower, p->upper,
-					TIPC_PUBLISHED, p->port, p->node,
+		tipc_sub_report_overlap(sub, p->sr.lower, p->sr.upper,
+					TIPC_PUBLISHED, p->sk.ref, p->sk.node,
 					p->scope, true);
 		list_del_init(&p->list);
 	}
@@ -525,7 +525,7 @@ struct publication *tipc_nametbl_remove_publ(struct net *net, u32 type,
 	last = list_empty(&sr->all_publ);
 	list_for_each_entry_safe(sub, tmp, &sc->subscriptions, service_list) {
 		tipc_sub_report_overlap(sub, lower, upper, TIPC_WITHDRAWN,
-					p->port, node, p->scope, last);
+					p->sk.ref, node, p->scope, last);
 	}
 
 	/* Remove service range item if this was its last publication */
@@ -603,8 +603,8 @@ u32 tipc_nametbl_translate(struct net *net, u32 type, u32 instance, u32 *dnode)
 					     all_publ);
 			list_move_tail(&p->all_publ, &sr->all_publ);
 		}
-		port = p->port;
-		node = p->node;
+		port = p->sk.ref;
+		node = p->sk.node;
 		/* Todo: as for legacy, pick the first matching range only, a
 		 * "true" round-robin will be performed as needed.
 		 */
@@ -643,9 +643,9 @@ bool tipc_nametbl_lookup(struct net *net, u32 type, u32 instance, u32 scope,
 	list_for_each_entry(p, &sr->all_publ, all_publ) {
 		if (p->scope != scope)
 			continue;
-		if (p->port == exclude && p->node == self)
+		if (p->sk.ref == exclude && p->sk.node == self)
 			continue;
-		tipc_dest_push(dsts, p->node, p->port);
+		tipc_dest_push(dsts, p->sk.node, p->sk.ref);
 		(*dstcnt)++;
 		if (all)
 			continue;
@@ -675,7 +675,7 @@ void tipc_nametbl_mc_lookup(struct net *net, u32 type, u32 lower, u32 upper,
 	service_range_foreach_match(sr, sc, lower, upper) {
 		list_for_each_entry(p, &sr->local_publ, local_publ) {
 			if (p->scope == scope || (!exact && p->scope < scope))
-				tipc_dest_push(dports, 0, p->port);
+				tipc_dest_push(dports, 0, p->sk.ref);
 		}
 	}
 	spin_unlock_bh(&sc->lock);
@@ -702,7 +702,7 @@ void tipc_nametbl_lookup_dst_nodes(struct net *net, u32 type, u32 lower,
 	spin_lock_bh(&sc->lock);
 	service_range_foreach_match(sr, sc, lower, upper) {
 		list_for_each_entry(p, &sr->all_publ, all_publ) {
-			tipc_nlist_add(nodes, p->node);
+			tipc_nlist_add(nodes, p->sk.node);
 		}
 	}
 	spin_unlock_bh(&sc->lock);
@@ -731,7 +731,7 @@ void tipc_nametbl_build_group(struct net *net, struct tipc_group *grp,
 		list_for_each_entry(p, &sr->all_publ, all_publ) {
 			if (p->scope != scope)
 				continue;
-			tipc_group_add_member(grp, p->node, p->port, p->lower);
+			tipc_group_add_member(grp, p->sk.node, p->sk.ref, p->sr.lower);
 		}
 	}
 	spin_unlock_bh(&sc->lock);
@@ -909,7 +909,7 @@ static void tipc_service_delete(struct net *net, struct tipc_service *sc)
 	spin_lock_bh(&sc->lock);
 	rbtree_postorder_for_each_entry_safe(sr, tmpr, &sc->ranges, tree_node) {
 		list_for_each_entry_safe(p, tmp, &sr->all_publ, all_publ) {
-			tipc_service_remove_publ(sr, p->node, p->key);
+			tipc_service_remove_publ(sr, p->sk.node, p->key);
 			kfree_rcu(p, rcu);
 		}
 		rb_erase_augmented(&sr->tree_node, &sc->ranges, &sr_callbacks);
@@ -993,9 +993,9 @@ static int __tipc_nl_add_nametable_publ(struct tipc_nl_msg *msg,
 			goto publ_msg_full;
 		if (nla_put_u32(msg->skb, TIPC_NLA_PUBL_SCOPE, p->scope))
 			goto publ_msg_full;
-		if (nla_put_u32(msg->skb, TIPC_NLA_PUBL_NODE, p->node))
+		if (nla_put_u32(msg->skb, TIPC_NLA_PUBL_NODE, p->sk.node))
 			goto publ_msg_full;
-		if (nla_put_u32(msg->skb, TIPC_NLA_PUBL_REF, p->port))
+		if (nla_put_u32(msg->skb, TIPC_NLA_PUBL_REF, p->sk.ref))
 			goto publ_msg_full;
 		if (nla_put_u32(msg->skb, TIPC_NLA_PUBL_KEY, p->key))
 			goto publ_msg_full;
diff --git a/net/tipc/name_table.h b/net/tipc/name_table.h
index 5a82a01369d6..d9ad119f966b 100644
--- a/net/tipc/name_table.h
+++ b/net/tipc/name_table.h
@@ -3,6 +3,7 @@
  *
  * Copyright (c) 2000-2006, 2014-2018, Ericsson AB
  * Copyright (c) 2004-2005, 2010-2011, Wind River Systems
+ * Copyright (c) 2020-2021, Red Hat Inc
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -50,13 +51,10 @@ struct tipc_group;
 #define TIPC_NAMETBL_SIZE	1024	/* must be a power of 2 */
 
 /**
- * struct publication - info about a published (name or) name sequence
- * @type: name sequence type
- * @lower: name sequence lower bound
- * @upper: name sequence upper bound
+ * struct publication - info about a published service address or range
+ * @sr: service range represented by this publication
+ * @sk: address of socket bound to this publication
  * @scope: scope of publication, TIPC_NODE_SCOPE or TIPC_CLUSTER_SCOPE
- * @node: network address of publishing socket's node
- * @port: publishing port
  * @key: publication key, unique across the cluster
  * @id: publication id
  * @binding_node: all publications from the same node which bound this one
@@ -74,12 +72,9 @@ struct tipc_group;
  * @rcu: RCU callback head used for deferred freeing
  */
 struct publication {
-	u32 type;
-	u32 lower;
-	u32 upper;
+	struct tipc_service_range sr;
+	struct tipc_socket_addr sk;
 	u32 scope;
-	u32 node;
-	u32 port;
 	u32 key;
 	u32 id;
 	struct list_head binding_node;
diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index cebcc104dc70..fe522d49f747 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -3,7 +3,7 @@
  *
  * Copyright (c) 2001-2007, 2012-2019, Ericsson AB
  * Copyright (c) 2004-2008, 2010-2013, Wind River Systems
- * Copyright (c) 2020, Red Hat Inc
+ * Copyright (c) 2020-2021, Red Hat Inc
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -2923,30 +2923,30 @@ static int tipc_sk_withdraw(struct tipc_sock *tsk, uint scope,
 			    struct tipc_service_range const *seq)
 {
 	struct net *net = sock_net(&tsk->sk);
-	struct publication *publ;
+	struct publication *p;
 	struct publication *safe;
 	int rc = -EINVAL;
 
 	if (scope != TIPC_NODE_SCOPE)
 		scope = TIPC_CLUSTER_SCOPE;
 
-	list_for_each_entry_safe(publ, safe, &tsk->publications, binding_sock) {
+	list_for_each_entry_safe(p, safe, &tsk->publications, binding_sock) {
 		if (seq) {
-			if (publ->scope != scope)
+			if (p->scope != scope)
 				continue;
-			if (publ->type != seq->type)
+			if (p->sr.type != seq->type)
 				continue;
-			if (publ->lower != seq->lower)
+			if (p->sr.lower != seq->lower)
 				continue;
-			if (publ->upper != seq->upper)
+			if (p->sr.upper != seq->upper)
 				break;
-			tipc_nametbl_withdraw(net, publ->type, publ->lower,
-					      publ->upper, publ->key);
+			tipc_nametbl_withdraw(net, p->sr.type, p->sr.lower,
+					      p->sr.upper, p->key);
 			rc = 0;
 			break;
 		}
-		tipc_nametbl_withdraw(net, publ->type, publ->lower,
-				      publ->upper, publ->key);
+		tipc_nametbl_withdraw(net, p->sr.type, p->sr.lower,
+				      p->sr.upper, p->key);
 		rc = 0;
 	}
 	if (list_empty(&tsk->publications))
@@ -3711,11 +3711,11 @@ static int __tipc_nl_add_sk_publ(struct sk_buff *skb,
 
 	if (nla_put_u32(skb, TIPC_NLA_PUBL_KEY, publ->key))
 		goto attr_msg_cancel;
-	if (nla_put_u32(skb, TIPC_NLA_PUBL_TYPE, publ->type))
+	if (nla_put_u32(skb, TIPC_NLA_PUBL_TYPE, publ->sr.type))
 		goto attr_msg_cancel;
-	if (nla_put_u32(skb, TIPC_NLA_PUBL_LOWER, publ->lower))
+	if (nla_put_u32(skb, TIPC_NLA_PUBL_LOWER, publ->sr.lower))
 		goto attr_msg_cancel;
-	if (nla_put_u32(skb, TIPC_NLA_PUBL_UPPER, publ->upper))
+	if (nla_put_u32(skb, TIPC_NLA_PUBL_UPPER, publ->sr.upper))
 		goto attr_msg_cancel;
 
 	nla_nest_end(skb, attrs);
@@ -3863,9 +3863,9 @@ bool tipc_sk_filtering(struct sock *sk)
 		p = list_first_entry_or_null(&tsk->publications,
 					     struct publication, binding_sock);
 		if (p) {
-			type = p->type;
-			lower = p->lower;
-			upper = p->upper;
+			type = p->sr.type;
+			lower = p->sr.lower;
+			upper = p->sr.upper;
 		}
 	}
 
@@ -3964,9 +3964,9 @@ int tipc_sk_dump(struct sock *sk, u16 dqueues, char *buf)
 	if (tsk->published) {
 		p = list_first_entry_or_null(&tsk->publications,
 					     struct publication, binding_sock);
-		i += scnprintf(buf + i, sz - i, " %u", (p) ? p->type : 0);
-		i += scnprintf(buf + i, sz - i, " %u", (p) ? p->lower : 0);
-		i += scnprintf(buf + i, sz - i, " %u", (p) ? p->upper : 0);
+		i += scnprintf(buf + i, sz - i, " %u", (p) ? p->sr.type : 0);
+		i += scnprintf(buf + i, sz - i, " %u", (p) ? p->sr.lower : 0);
+		i += scnprintf(buf + i, sz - i, " %u", (p) ? p->sr.upper : 0);
 	}
 	i += scnprintf(buf + i, sz - i, " | %u", tsk->snd_win);
 	i += scnprintf(buf + i, sz - i, " %u", tsk->rcv_win);
-- 
2.29.2

[tipc-discussion] [net-next v2 06/16] tipc: simplify signature of tipc_nametbl_withdraw() functions

[<jm...@re...>]

From: Jon Maloy <jm...@re...>

Following the principles of the preceding commits, we reduce
the number of parameters passed along in tipc_sk_withdraw(),
tipc_nametbl_withdraw() and associated functions.

Signed-off-by: Jon Maloy <jm...@re...>
Acked-by: Ying Xue <yin...@wi...>
Acked-by: Hoang Le <hoa...@de...>
---
 net/tipc/name_distr.c | 11 ++++----
 net/tipc/name_table.c | 51 ++++++++++++++++++-----------------
 net/tipc/name_table.h | 11 ++++----
 net/tipc/node.c       |  3 +--
 net/tipc/socket.c     | 62 +++++++++++++++++++++----------------------
 5 files changed, 70 insertions(+), 68 deletions(-)

diff --git a/net/tipc/name_distr.c b/net/tipc/name_distr.c
index 727f8c54df6b..9e2fab3569b5 100644
--- a/net/tipc/name_distr.c
+++ b/net/tipc/name_distr.c
@@ -244,17 +244,19 @@ static void tipc_publ_purge(struct net *net, struct publication *p, u32 addr)
 {
 	struct tipc_net *tn = tipc_net(net);
 	struct publication *_p;
+	struct tipc_uaddr ua;
 
+	tipc_uaddr(&ua, TIPC_SERVICE_RANGE, p->scope, p->sr.type,
+		   p->sr.lower, p->sr.upper);
 	spin_lock_bh(&tn->nametbl_lock);
-	_p = tipc_nametbl_remove_publ(net, p->sr.type, p->sr.lower,
-				      p->sr.upper, p->sk.node, p->key);
+	_p = tipc_nametbl_remove_publ(net, &ua, &p->sk, p->key);
 	if (_p)
 		tipc_node_unsubscribe(net, &_p->binding_node, addr);
 	spin_unlock_bh(&tn->nametbl_lock);
 
 	if (_p != p) {
 		pr_err("Unable to remove publication from failed node\n"
-		       " (type=%u, lower=%u, node=0x%x, port=%u, key=%u)\n",
+		       " (type=%u, lower=%u, node=%u, port=%u, key=%u)\n",
 		       p->sr.type, p->sr.lower, p->sk.node, p->sk.ref, p->key);
 	}
 
@@ -309,8 +311,7 @@ static bool tipc_update_nametbl(struct net *net, struct distr_item *i,
 			return true;
 		}
 	} else if (dtype == WITHDRAWAL) {
-		p = tipc_nametbl_remove_publ(net, ua.sr.type, ua.sr.lower,
-					     ua.sr.upper, node, key);
+		p = tipc_nametbl_remove_publ(net, &ua, &sk, key);
 		if (p) {
 			tipc_node_unsubscribe(net, &p->binding_node, node);
 			kfree_rcu(p, rcu);
diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c
index 146c478143a6..5676b8d4f08f 100644
--- a/net/tipc/name_table.c
+++ b/net/tipc/name_table.c
@@ -366,16 +366,18 @@ static bool tipc_service_insert_publ(struct net *net,
 
 /**
  * tipc_service_remove_publ - remove a publication from a service
- * @sr: service_range to remove publication from
- * @node: target node
+ * @r: service_range to remove publication from
+ * @sk: address publishing socket
  * @key: target publication key
  */
-static struct publication *tipc_service_remove_publ(struct service_range *sr,
-						    u32 node, u32 key)
+static struct publication *tipc_service_remove_publ(struct service_range *r,
+						    struct tipc_socket_addr *sk,
+						    u32 key)
 {
 	struct publication *p;
+	u32 node = sk->node;
 
-	list_for_each_entry(p, &sr->all_publ, all_publ) {
+	list_for_each_entry(p, &r->all_publ, all_publ) {
 		if (p->key != key || (node && node != p->sk.node))
 			continue;
 		list_del(&p->all_publ);
@@ -493,16 +495,20 @@ struct publication *tipc_nametbl_insert_publ(struct net *net,
 	return NULL;
 }
 
-struct publication *tipc_nametbl_remove_publ(struct net *net, u32 type,
-					     u32 lower, u32 upper,
-					     u32 node, u32 key)
+struct publication *tipc_nametbl_remove_publ(struct net *net,
+					     struct tipc_uaddr *ua,
+					     struct tipc_socket_addr *sk,
+					     u32 key)
 {
-	struct tipc_service *sc = tipc_service_find(net, type);
 	struct tipc_subscription *sub, *tmp;
-	struct service_range *sr = NULL;
 	struct publication *p = NULL;
+	struct service_range *sr;
+	struct tipc_service *sc;
+	u32 upper = ua->sr.upper;
+	u32 lower = ua->sr.lower;
 	bool last;
 
+	sc = tipc_service_find(net, ua->sr.type);
 	if (!sc)
 		return NULL;
 
@@ -510,7 +516,7 @@ struct publication *tipc_nametbl_remove_publ(struct net *net, u32 type,
 	sr = tipc_service_find_range(sc, lower, upper);
 	if (!sr)
 		goto exit;
-	p = tipc_service_remove_publ(sr, node, key);
+	p = tipc_service_remove_publ(sr, sk, key);
 	if (!p)
 		goto exit;
 
@@ -518,7 +524,7 @@ struct publication *tipc_nametbl_remove_publ(struct net *net, u32 type,
 	last = list_empty(&sr->all_publ);
 	list_for_each_entry_safe(sub, tmp, &sc->subscriptions, service_list) {
 		tipc_sub_report_overlap(sub, lower, upper, TIPC_WITHDRAWN,
-					p->sk.ref, node, p->scope, last);
+					sk->ref, sk->node, ua->scope, last);
 	}
 
 	/* Remove service range item if this was its last publication */
@@ -768,24 +774,22 @@ struct publication *tipc_nametbl_publish(struct net *net, struct tipc_uaddr *ua,
 /**
  * tipc_nametbl_withdraw - withdraw a service binding
  * @net: network namespace
- * @type: service type
- * @lower: service range lower bound
- * @upper: service range upper bound
+ * @ua: service address/range being unbound
+ * @sk: address of the socket being unbound from
  * @key: target publication key
  */
-int tipc_nametbl_withdraw(struct net *net, u32 type, u32 lower,
-			  u32 upper, u32 key)
+void tipc_nametbl_withdraw(struct net *net, struct tipc_uaddr *ua,
+			   struct tipc_socket_addr *sk, u32 key)
 {
 	struct name_table *nt = tipc_name_table(net);
 	struct tipc_net *tn = tipc_net(net);
-	u32 self = tipc_own_addr(net);
 	struct sk_buff *skb = NULL;
 	struct publication *p;
 	u32 rc_dests;
 
 	spin_lock_bh(&tn->nametbl_lock);
 
-	p = tipc_nametbl_remove_publ(net, type, lower, upper, self, key);
+	p = tipc_nametbl_remove_publ(net, ua, sk, key);
 	if (p) {
 		nt->local_publ_count--;
 		skb = tipc_named_withdraw(net, p);
@@ -793,16 +797,13 @@ int tipc_nametbl_withdraw(struct net *net, u32 type, u32 lower,
 		kfree_rcu(p, rcu);
 	} else {
 		pr_err("Failed to remove local publication {%u,%u,%u}/%u\n",
-		       type, lower, upper, key);
+		       ua->sr.type, ua->sr.lower, ua->sr.upper, key);
 	}
 	rc_dests = nt->rc_dests;
 	spin_unlock_bh(&tn->nametbl_lock);
 
-	if (skb) {
+	if (skb)
 		tipc_node_broadcast(net, skb, rc_dests);
-		return 1;
-	}
-	return 0;
 }
 
 /**
@@ -900,7 +901,7 @@ static void tipc_service_delete(struct net *net, struct tipc_service *sc)
 	spin_lock_bh(&sc->lock);
 	rbtree_postorder_for_each_entry_safe(sr, tmpr, &sc->ranges, tree_node) {
 		list_for_each_entry_safe(p, tmp, &sr->all_publ, all_publ) {
-			tipc_service_remove_publ(sr, p->sk.node, p->key);
+			tipc_service_remove_publ(sr, &p->sk, p->key);
 			kfree_rcu(p, rcu);
 		}
 		rb_erase_augmented(&sr->tree_node, &sc->ranges, &sr_callbacks);
diff --git a/net/tipc/name_table.h b/net/tipc/name_table.h
index c8b026e56e81..5a7c83d22ef9 100644
--- a/net/tipc/name_table.h
+++ b/net/tipc/name_table.h
@@ -123,15 +123,16 @@ bool tipc_nametbl_lookup(struct net *net, u32 type, u32 instance, u32 domain,
 			 bool all);
 struct publication *tipc_nametbl_publish(struct net *net, struct tipc_uaddr *ua,
 					 struct tipc_socket_addr *sk, u32 key);
-int tipc_nametbl_withdraw(struct net *net, u32 type, u32 lower, u32 upper,
-			  u32 key);
+void tipc_nametbl_withdraw(struct net *net, struct tipc_uaddr *ua,
+			   struct tipc_socket_addr *sk, u32 key);
 struct publication *tipc_nametbl_insert_publ(struct net *net,
 					     struct tipc_uaddr *ua,
 					     struct tipc_socket_addr *sk,
 					     u32 key);
-struct publication *tipc_nametbl_remove_publ(struct net *net, u32 type,
-					     u32 lower, u32 upper,
-					     u32 node, u32 key);
+struct publication *tipc_nametbl_remove_publ(struct net *net,
+					     struct tipc_uaddr *ua,
+					     struct tipc_socket_addr *sk,
+					     u32 key);
 bool tipc_nametbl_subscribe(struct tipc_subscription *s);
 void tipc_nametbl_unsubscribe(struct tipc_subscription *s);
 int tipc_nametbl_init(struct net *net);
diff --git a/net/tipc/node.c b/net/tipc/node.c
index 7c12c9aa3926..f64e093a340b 100644
--- a/net/tipc/node.c
+++ b/net/tipc/node.c
@@ -429,8 +429,7 @@ static void tipc_node_write_unlock(struct tipc_node *n)
 	}
 	if (flags & TIPC_NOTIFY_LINK_DOWN) {
 		tipc_mon_peer_down(net, n->addr, bearer_id);
-		tipc_nametbl_withdraw(net, TIPC_LINK_STATE, n->addr,
-				      n->addr, n->link_id);
+		tipc_nametbl_withdraw(net, &ua, &sk, n->link_id);
 	}
 }
 
diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 94847252a7b7..7f5722d3b3d0 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -152,8 +152,7 @@ static int tipc_accept(struct socket *sock, struct socket *new_sock, int flags,
 		       bool kern);
 static void tipc_sk_timeout(struct timer_list *t);
 static int tipc_sk_publish(struct tipc_sock *tsk, struct tipc_uaddr *ua);
-static int tipc_sk_withdraw(struct tipc_sock *tsk, uint scope,
-			    struct tipc_service_range const *seq);
+static int tipc_sk_withdraw(struct tipc_sock *tsk, struct tipc_uaddr *ua);
 static int tipc_sk_leave(struct tipc_sock *tsk);
 static struct tipc_sock *tipc_sk_lookup(struct net *net, u32 portid);
 static int tipc_sk_insert(struct tipc_sock *tsk);
@@ -643,7 +642,7 @@ static int tipc_release(struct socket *sock)
 	__tipc_shutdown(sock, TIPC_ERR_NO_PORT);
 	sk->sk_shutdown = SHUTDOWN_MASK;
 	tipc_sk_leave(tsk);
-	tipc_sk_withdraw(tsk, 0, NULL);
+	tipc_sk_withdraw(tsk, NULL);
 	__skb_queue_purge(&tsk->mc_method.deferredq);
 	sk_stop_timer(sk, &sk->sk_timer);
 	tipc_sk_remove(tsk);
@@ -681,7 +680,7 @@ static int __tipc_bind(struct socket *sock, struct sockaddr *skaddr, int alen)
 	bool unbind = false;
 
 	if (unlikely(!alen))
-		return tipc_sk_withdraw(tsk, 0, NULL);
+		return tipc_sk_withdraw(tsk, NULL);
 
 	if (ua->addrtype == TIPC_SERVICE_ADDR) {
 		ua->addrtype = TIPC_SERVICE_RANGE;
@@ -699,7 +698,7 @@ static int __tipc_bind(struct socket *sock, struct sockaddr *skaddr, int alen)
 		return -EACCES;
 
 	if (unbind)
-		return tipc_sk_withdraw(tsk, ua->scope, &ua->sr);
+		return tipc_sk_withdraw(tsk, ua);
 	return tipc_sk_publish(tsk, ua);
 }
 
@@ -2923,38 +2922,37 @@ static int tipc_sk_publish(struct tipc_sock *tsk, struct tipc_uaddr *ua)
 	return 0;
 }
 
-static int tipc_sk_withdraw(struct tipc_sock *tsk, uint scope,
-			    struct tipc_service_range const *seq)
+static int tipc_sk_withdraw(struct tipc_sock *tsk, struct tipc_uaddr *ua)
 {
 	struct net *net = sock_net(&tsk->sk);
-	struct publication *p;
-	struct publication *safe;
+	struct publication *safe, *p;
+	struct tipc_uaddr _ua;
 	int rc = -EINVAL;
 
-	if (scope != TIPC_NODE_SCOPE)
-		scope = TIPC_CLUSTER_SCOPE;
-
 	list_for_each_entry_safe(p, safe, &tsk->publications, binding_sock) {
-		if (seq) {
-			if (p->scope != scope)
-				continue;
-			if (p->sr.type != seq->type)
-				continue;
-			if (p->sr.lower != seq->lower)
-				continue;
-			if (p->sr.upper != seq->upper)
-				break;
-			tipc_nametbl_withdraw(net, p->sr.type, p->sr.lower,
-					      p->sr.upper, p->key);
-			rc = 0;
-			break;
+		if (!ua) {
+			tipc_uaddr(&_ua, TIPC_SERVICE_RANGE, p->scope,
+				   p->sr.type, p->sr.lower, p->sr.upper);
+			tipc_nametbl_withdraw(net, &_ua, &p->sk, p->key);
+			continue;
 		}
-		tipc_nametbl_withdraw(net, p->sr.type, p->sr.lower,
-				      p->sr.upper, p->key);
+		/* Unbind specific publication */
+		if (p->scope != ua->scope)
+			continue;
+		if (p->sr.type != ua->sr.type)
+			continue;
+		if (p->sr.lower != ua->sr.lower)
+			continue;
+		if (p->sr.upper != ua->sr.upper)
+			break;
+		tipc_nametbl_withdraw(net, ua, &p->sk, p->key);
 		rc = 0;
+		break;
 	}
-	if (list_empty(&tsk->publications))
+	if (list_empty(&tsk->publications)) {
 		tsk->published = 0;
+		rc = 0;
+	}
 	return rc;
 }
 
@@ -3109,15 +3107,17 @@ static int tipc_sk_leave(struct tipc_sock *tsk)
 {
 	struct net *net = sock_net(&tsk->sk);
 	struct tipc_group *grp = tsk->group;
-	struct tipc_service_range seq;
+	struct tipc_uaddr ua;
 	int scope;
 
 	if (!grp)
 		return -EINVAL;
-	tipc_group_self(grp, &seq, &scope);
+	ua.addrtype = TIPC_SERVICE_RANGE;
+	tipc_group_self(grp, &ua.sr, &scope);
+	ua.scope = scope;
 	tipc_group_delete(net, grp);
 	tsk->group = NULL;
-	tipc_sk_withdraw(tsk, scope, &seq);
+	tipc_sk_withdraw(tsk, &ua);
 	return 0;
 }
 
-- 
2.29.2

[tipc-discussion] [net-next v2 02/16] tipc: move creation of publication item one level up in call chain

[<jm...@re...>]

From: Jon Maloy <jm...@re...>

We instantiate struct publication in tipc_nametbl_insert_publ()
instead of as currently in tipc_service_insert_publ(). This has the
advantage that we can pass a pointer to the publication struct to
the next call levels, instead of the numerous individual parameters
we pass on now. It also gives us a location to keep the contents of
the additional fields we will introduce in a later commit.

Signed-off-by: Jon Maloy <jm...@re...>
Acked-by: Ying Xue <yin...@wi...>
Acked-by: Hoang Le <hoa...@de...>
---
 net/tipc/name_table.c | 65 +++++++++++++++++++++----------------------
 1 file changed, 32 insertions(+), 33 deletions(-)

diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c
index c2410ba7be5c..c37cef09b54c 100644
--- a/net/tipc/name_table.c
+++ b/net/tipc/name_table.c
@@ -327,49 +327,48 @@ static struct service_range *tipc_service_create_range(struct tipc_service *sc,
 	return sr;
 }
 
-static struct publication *tipc_service_insert_publ(struct net *net,
-						    struct tipc_service *sc,
-						    u32 type, u32 lower,
-						    u32 upper, u32 scope,
-						    u32 node, u32 port,
-						    u32 key)
+static bool tipc_service_insert_publ(struct net *net,
+				     struct tipc_service *sc,
+				     struct publication *p)
 {
 	struct tipc_subscription *sub, *tmp;
 	struct service_range *sr;
-	struct publication *p;
+	struct publication *_p;
+	u32 node = p->sk.node;
 	bool first = false;
+	bool res = false;
 
-	sr = tipc_service_create_range(sc, lower, upper);
+	spin_lock_bh(&sc->lock);
+	sr = tipc_service_create_range(sc, p->sr.lower, p->sr.upper);
 	if (!sr)
-		goto  err;
+		goto  exit;
 
 	first = list_empty(&sr->all_publ);
 
 	/* Return if the publication already exists */
-	list_for_each_entry(p, &sr->all_publ, all_publ) {
-		if (p->key == key && (!p->sk.node || p->sk.node == node))
-			return NULL;
+	list_for_each_entry(_p, &sr->all_publ, all_publ) {
+		if (_p->key == p->key && (!_p->sk.node || _p->sk.node == node))
+			goto exit;
 	}
 
-	/* Create and insert publication */
-	p = tipc_publ_create(type, lower, upper, scope, node, port, key);
-	if (!p)
-		goto err;
-	/* Suppose there shouldn't be a huge gap btw publs i.e. >INT_MAX */
-	p->id = sc->publ_cnt++;
-	if (in_own_node(net, node))
+	if (in_own_node(net, p->sk.node))
 		list_add(&p->local_publ, &sr->local_publ);
 	list_add(&p->all_publ, &sr->all_publ);
+	p->id = sc->publ_cnt++;
 
 	/* Any subscriptions waiting for notification?  */
 	list_for_each_entry_safe(sub, tmp, &sc->subscriptions, service_list) {
-		tipc_sub_report_overlap(sub, p->sr.lower, p->sr.upper, TIPC_PUBLISHED,
-					p->sk.ref, p->sk.node, p->scope, first);
+		tipc_sub_report_overlap(sub, p->sr.lower, p->sr.upper,
+					TIPC_PUBLISHED, p->sk.ref, p->sk.node,
+					p->scope, first);
 	}
-	return p;
-err:
-	pr_warn("Failed to bind to %u,%u,%u, no memory\n", type, lower, upper);
-	return NULL;
+	res = true;
+exit:
+	if (!res)
+		pr_warn("Failed to bind to %u,%u,%u\n",
+			p->sr.type, p->sr.lower, p->sr.upper);
+	spin_unlock_bh(&sc->lock);
+	return res;
 }
 
 /**
@@ -482,6 +481,10 @@ struct publication *tipc_nametbl_insert_publ(struct net *net, u32 type,
 	struct tipc_service *sc;
 	struct publication *p;
 
+	p = tipc_publ_create(type, lower, upper, scope, node, port, key);
+	if (!p)
+		return NULL;
+
 	if (scope > TIPC_NODE_SCOPE || lower > upper) {
 		pr_debug("Failed to bind illegal {%u,%u,%u} with scope %u\n",
 			 type, lower, upper, scope);
@@ -490,14 +493,10 @@ struct publication *tipc_nametbl_insert_publ(struct net *net, u32 type,
 	sc = tipc_service_find(net, type);
 	if (!sc)
 		sc = tipc_service_create(type, &nt->services[hash(type)]);
-	if (!sc)
-		return NULL;
-
-	spin_lock_bh(&sc->lock);
-	p = tipc_service_insert_publ(net, sc, type, lower, upper,
-				     scope, node, port, key);
-	spin_unlock_bh(&sc->lock);
-	return p;
+	if (sc && tipc_service_insert_publ(net, sc, p))
+		return p;
+	kfree(p);
+	return NULL;
 }
 
 struct publication *tipc_nametbl_remove_publ(struct net *net, u32 type,
-- 
2.29.2

[tipc-discussion] [net-next v2 00/16] tipc: cleanups and simplifications

[<jm...@re...>]

From: Jon Maloy <jm...@re...>

We make a number of simplifications and cleanups, especially to call signatures
in the binding table. This makes the code easier to understand and serves as a
preparation for an upcoming functional addition.

v2:
   - Eliminated sparse warnings, as per feedback from Hoang
   - Fixed some typos, as per feedback from Tung
   - Fixed some issues noted by Xin:
     #2/16: Moved spinlock.
     #4/16: Added scope check and used tipc_uaddr() in tipc_sk_join()
     #5/16: Eliminated 'addrtype' from struct publication (for now).
     #6/16: 'node number' represented as decimal in some places and
            hex in others. I will fix that in a separate patch later.
     #8/16: Re-introduce use of node2scope(). This function is needed
            later.
     #13/16: This change does not look motivated right now, as stated
             in the commit log, but will be needed later. I kept it,
             just because of the consequences of removing it now and
             having to re-introduce it later.
     #14/16: Made tipc_sub_check_overlap() static, as also reported
             by sparse.

Jon Maloy (16):
  tipc: re-organize members of struct publication
  tipc: move creation of publication item one level up in call chain
  tipc: introduce new unified address type for internal use
  tipc: simplify signature of tipc_namtbl_publish()
  tipc: simplify call signatures for publication creation
  tipc: simplify signature of tipc_nametbl_withdraw() functions
  tipc: rename binding table lookup functions
  tipc: refactor tipc_sendmsg() and tipc_lookup_anycast()
  tipc: simplify signature of tipc_namtbl_lookup_mcast_sockets()
  tipc: simplify signature of tipc_nametbl_lookup_mcast_nodes()
  tipc: simplify signature of tipc_nametbl_lookup_group()
  tipc: simplify signature of tipc_service_find_range()
  tipc: simplify signature of tipc_find_service()
  tipc: simplify api between binding table and topology server
  tipc: add host-endian copy of user subscription to struct
    tipc_subscription
  tipc: remove some unnecessary warnings

 net/tipc/addr.h       |  44 +++++
 net/tipc/msg.c        |  23 ++-
 net/tipc/name_distr.c |  91 +++++----
 net/tipc/name_table.c | 426 +++++++++++++++++++++---------------------
 net/tipc/name_table.h |  63 +++----
 net/tipc/net.c        |   8 +-
 net/tipc/node.c       |  28 +--
 net/tipc/socket.c     | 319 ++++++++++++++++---------------
 net/tipc/subscr.c     |  84 +++++----
 net/tipc/subscr.h     |  12 +-
 10 files changed, 573 insertions(+), 525 deletions(-)

[tipc-discussion] [net-next v2 03/16] tipc: introduce new unified address type for internal use

[<jm...@re...>]

From: Jon Maloy <jm...@re...>

We introduce a simplified version of struct sockaddr_tipc, using
anonymous unions and structures. Apart from being nicer to work with,
this struct will come in handy when we in a later commit add another
address type.

Signed-off-by: Jon Maloy <jm...@re...>
Acked-by: Ying Xue <yin...@wi...>
Acked-by: Hoang Le <hoa...@de...>
---
 net/tipc/addr.c |  1 +
 net/tipc/addr.h | 46 +++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/net/tipc/addr.c b/net/tipc/addr.c
index abe29d1aa23a..fd0796269eed 100644
--- a/net/tipc/addr.c
+++ b/net/tipc/addr.c
@@ -3,6 +3,7 @@
  *
  * Copyright (c) 2000-2006, 2018, Ericsson AB
  * Copyright (c) 2004-2005, 2010-2011, Wind River Systems
+ * Copyright (c) 2020-2021, Red Hat Inc
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
diff --git a/net/tipc/addr.h b/net/tipc/addr.h
index 1a11831bef62..0772cfadaa0d 100644
--- a/net/tipc/addr.h
+++ b/net/tipc/addr.h
@@ -3,7 +3,7 @@
  *
  * Copyright (c) 2000-2006, 2018, Ericsson AB
  * Copyright (c) 2004-2005, Wind River Systems
- * Copyright (c) 2020, Red Hat Inc
+ * Copyright (c) 2020-2021, Red Hat Inc
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -44,6 +44,50 @@
 #include <net/netns/generic.h>
 #include "core.h"
 
+/* Struct tipc_uaddr: internal version of struct sockaddr_tipc.
+ * Must be kept aligned both regarding field positions and size.
+ */
+struct tipc_uaddr {
+	unsigned short family;
+	unsigned char addrtype;
+	signed char scope;
+	union {
+		struct {
+			struct tipc_service_addr sa;
+			u32 lookup_node;
+		};
+		struct tipc_service_range sr;
+		struct tipc_socket_addr sk;
+	};
+};
+
+static inline void tipc_uaddr(struct tipc_uaddr *ua, u32 atype, u32 scope,
+			      u32 type, u32 lower, u32 upper)
+{
+	ua->family = AF_TIPC;
+	ua->addrtype = atype;
+	ua->scope = scope;
+	ua->sr.type = type;
+	ua->sr.lower = lower;
+	ua->sr.upper = upper;
+}
+
+static inline bool tipc_uaddr_valid(struct tipc_uaddr *ua, int len)
+{
+	u32 atype;
+
+	if (len < sizeof(struct sockaddr_tipc))
+		return false;
+	atype = ua->addrtype;
+	if (ua->family != AF_TIPC)
+		return false;
+	if (atype == TIPC_SERVICE_ADDR || atype == TIPC_SOCKET_ADDR)
+		return true;
+	if (atype == TIPC_SERVICE_RANGE)
+		return ua->sr.upper >= ua->sr.lower;
+	return false;
+}
+
 static inline u32 tipc_own_addr(struct net *net)
 {
 	return tipc_net(net)->node_addr;
-- 
2.29.2

Re: [tipc-discussion] [net-next v2 1/2] tipc: convert dest node's address to network order

[Jon M. <jm...@re...>]

Hi Hoang,
I noticed that this series is not upstream yet. Did I forget to ack it?
Series
Acked-by: Jon Maloy <jm...@re...>

On 1/18/21 3:08 AM, Hoang Huu Le wrote:
> From: Hoang Le <hoa...@de...>
>
> (struct tipc_link_info)->dest is in network order (__be32), so we must
> convert the value to network order before assigning. The problem detected
> by sparse:
>
> net/tipc/netlink_compat.c:699:24: warning: incorrect type in assignment (different base types)
> net/tipc/netlink_compat.c:699:24:    expected restricted __be32 [usertype] dest
> net/tipc/netlink_compat.c:699:24:    got int
>
> Acked-by: Jon Maloy <jm...@re...>
> Signed-off-by: Hoang Le <hoa...@de...>
> ---
>   net/tipc/netlink_compat.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/tipc/netlink_compat.c b/net/tipc/netlink_compat.c
> index 5a1ce64039f7..0749df80454d 100644
> --- a/net/tipc/netlink_compat.c
> +++ b/net/tipc/netlink_compat.c
> @@ -696,7 +696,7 @@ static int tipc_nl_compat_link_dump(struct tipc_nl_compat_msg *msg,
>   	if (err)
>   		return err;
>   
> -	link_info.dest = nla_get_flag(link[TIPC_NLA_LINK_DEST]);
> +	link_info.dest = htonl(nla_get_flag(link[TIPC_NLA_LINK_DEST]));
>   	link_info.up = htonl(nla_get_flag(link[TIPC_NLA_LINK_UP]));
>   	nla_strscpy(link_info.str, link[TIPC_NLA_LINK_NAME],
>   		    TIPC_MAX_LINK_NAME);

Re: [tipc-discussion] [net-next 00/16] tipc: cleanups and simplifications

[Xin L. <luc...@gm...>]

Hi Jon,

Sorry for so late to check this patchset, I just came back from
Chinese New Year holidays.

Overall I like the idea to use one or two new structures to make the
parameters passing between functions more clear and readable.
Meanwhile I think if the parameters of some function are already
simple, we may just need to pass what the function only wants. After
all, ua->xxx will need CPU to access memory obj instead of read
registers only,  and we will have to build a ua object before calling
the function.

I just commented on some of the patches.

Thanks.

On Wed, Dec 9, 2020 at 2:51 AM <jm...@re...> wrote:
>
> From: Jon Maloy <jm...@re...>
>
> We make a number of simplifications and cleanups, especially to call signatures
> in the binding table. This makes the code easier to understand and serves as a
> preparation for an upcoming functional addition.
>
> Jon Maloy (16):
>   tipc: re-organize members of struct publication
>   tipc: move creation of publication item one level up in call chain
>   tipc: introduce new unified address type for internal use
>   tipc: simplify signature of tipc_namtbl_publish()
>   tipc: simplify call signatures for publication creation
>   tipc: simplify signature of tipc_nametbl_withdraw() functions
>   tipc: rename binding table lookup functions
>   tipc: refactor tipc_sendmsg() and tipc_lookup_anycast()
>   tipc: simplify signature of tipc_namtbl_lookup_mcast_sockets()
>   tipc: simplify signature of tipc_nametbl_lookup_mcast_nodes()
>   tipc: simplify signature of tipc_nametbl_lookup_group()
>   tipc: simplify signature of tipc_service_find_range()
>   tipc: simplify signature of tipc_find_service()
>   tipc: simplify api between binding table and topology server
>   tipc: add host-endian copy of user subscription to struct
>     tipc_subscription
>   tipc: remove some unnecessary warnings
>
>  net/tipc/addr.h       |  44 +++++
>  net/tipc/msg.c        |  23 ++-
>  net/tipc/name_distr.c |  89 +++++----
>  net/tipc/name_table.c | 419 ++++++++++++++++++++++--------------------
>  net/tipc/name_table.h |  64 ++++---
>  net/tipc/net.c        |   8 +-
>  net/tipc/node.c       |  28 +--
>  net/tipc/socket.c     | 313 +++++++++++++++----------------
>  net/tipc/subscr.c     |  84 +++++----
>  net/tipc/subscr.h     |  12 +-
>  10 files changed, 567 insertions(+), 517 deletions(-)
>
> --
> 2.28.0
>
>
>
> _______________________________________________
> tipc-discussion mailing list
> tip...@li...
> https://lists.sourceforge.net/lists/listinfo/tipc-discussion

Re: [tipc-discussion] [net-next 14/16] tipc: simplify api between binding table and topology server

[Xin L. <luc...@gm...>]

On Wed, Dec 9, 2020 at 2:51 AM <jm...@re...> wrote:
>
> From: Jon Maloy <jm...@re...>
>
> The function tipc_report_overlap() is called from the binding table
> with numerous parameters taken from an instance of struct publication.
> A closer look reveals that it always is safe to send along a pointer
> to the instance itself, and hence reduce the call signature. We do
> that in this commit.
>
> Signed-off-by: Jon Maloy <jm...@re...>
> ---
>  net/tipc/name_table.c | 20 ++++++---------
>  net/tipc/subscr.c     | 57 +++++++++++++++++++++++--------------------
>  net/tipc/subscr.h     |  9 +++----
>  3 files changed, 41 insertions(+), 45 deletions(-)
>
> diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c
> index 554a006d7c0d..4dfac3b9d26c 100644
> --- a/net/tipc/name_table.c
> +++ b/net/tipc/name_table.c
> @@ -357,9 +357,7 @@ static bool tipc_service_insert_publ(struct net *net,
>
>         /* Any subscriptions waiting for notification?  */
>         list_for_each_entry_safe(sub, tmp, &sc->subscriptions, service_list) {
> -               tipc_sub_report_overlap(sub, p->sr.lower, p->sr.upper,
> -                                       TIPC_PUBLISHED, p->sk.ref, p->sk.node,
> -                                       p->scope, first);
> +               tipc_sub_report_overlap(sub, p, TIPC_PUBLISHED, first);
>         }
>         return true;
>  err:
> @@ -451,9 +449,7 @@ static void tipc_service_subscribe(struct tipc_service *service,
>         /* Sort the publications before reporting */
>         list_sort(NULL, &publ_list, tipc_publ_sort);
>         list_for_each_entry_safe(p, tmp, &publ_list, list) {
> -               tipc_sub_report_overlap(sub, p->sr.lower, p->sr.upper,
> -                                       TIPC_PUBLISHED, p->sk.ref, p->sk.node,
> -                                       p->scope, true);
> +               tipc_sub_report_overlap(sub, p, TIPC_PUBLISHED, true);
>                 list_del_init(&p->list);
>         }
>  }
> @@ -532,9 +528,7 @@ struct publication *tipc_nametbl_remove_publ(struct net *net,
>         /* Notify any waiting subscriptions */
>         last = list_empty(&sr->all_publ);
>         list_for_each_entry_safe(sub, tmp, &sc->subscriptions, service_list) {
> -               tipc_sub_report_overlap(sub, ua->sr.lower, ua->sr.upper,
> -                                       TIPC_WITHDRAWN, sk->ref, sk->node,
> -                                       ua->scope, last);
> +               tipc_sub_report_overlap(sub, p, TIPC_WITHDRAWN, last);
>         }
>
>         /* Remove service range item if this was its last publication */
> @@ -543,7 +537,7 @@ struct publication *tipc_nametbl_remove_publ(struct net *net,
>                 kfree(sr);
>         }
>
> -       /* Delete service item if this no more publications and subscriptions */
> +       /* Delete service item if no more publications and subscriptions */
>         if (RB_EMPTY_ROOT(&sc->ranges) && list_empty(&sc->subscriptions)) {
>                 hlist_del_init_rcu(&sc->service_list);
>                 kfree_rcu(sc, rcu);
> @@ -842,7 +836,8 @@ bool tipc_nametbl_subscribe(struct tipc_subscription *sub)
>         struct tipc_uaddr ua;
>         bool res = true;
>
> -       tipc_uaddr(&ua, TIPC_SERVICE_RANGE, TIPC_NODE_SCOPE, type, 0, 0);
> +       tipc_uaddr(&ua, TIPC_SERVICE_RANGE, TIPC_NODE_SCOPE, type,
> +                  tipc_sub_read(s, seq.lower), tipc_sub_read(s, seq.upper));
>         spin_lock_bh(&tn->nametbl_lock);
>         sc = tipc_service_find(sub->net, &ua);
>         if (!sc)
> @@ -873,7 +868,8 @@ void tipc_nametbl_unsubscribe(struct tipc_subscription *sub)
>         struct tipc_service *sc;
>         struct tipc_uaddr ua;
>
> -       tipc_uaddr(&ua, TIPC_SERVICE_RANGE, TIPC_NODE_SCOPE, type, 0, 0);
> +       tipc_uaddr(&ua, TIPC_SERVICE_RANGE, TIPC_NODE_SCOPE, type,
> +                  tipc_sub_read(s, seq.lower), tipc_sub_read(s, seq.upper));
>         spin_lock_bh(&tn->nametbl_lock);
>         sc = tipc_service_find(sub->net, &ua);
>         if (!sc)
> diff --git a/net/tipc/subscr.c b/net/tipc/subscr.c
> index f6ad0005218c..feda0b6bbf1b 100644
> --- a/net/tipc/subscr.c
> +++ b/net/tipc/subscr.c
> @@ -40,18 +40,26 @@
>  #include "subscr.h"
>
>  static void tipc_sub_send_event(struct tipc_subscription *sub,
> -                               u32 found_lower, u32 found_upper,
> -                               u32 event, u32 port, u32 node)
> +                               struct publication *p,
> +                               u32 event)
>  {
> +       struct tipc_subscr *s = &sub->evt.s;
>         struct tipc_event *evt = &sub->evt;
>
>         if (sub->inactive)
>                 return;
>         tipc_evt_write(evt, event, event);
> -       tipc_evt_write(evt, found_lower, found_lower);
> -       tipc_evt_write(evt, found_upper, found_upper);
> -       tipc_evt_write(evt, port.ref, port);
> -       tipc_evt_write(evt, port.node, node);
> +       if (p) {
> +               tipc_evt_write(evt, found_lower, p->sr.lower);
> +               tipc_evt_write(evt, found_upper, p->sr.upper);
> +               tipc_evt_write(evt, port.ref, p->sk.ref);
> +               tipc_evt_write(evt, port.node, p->sk.node);
> +       } else {
> +               tipc_evt_write(evt, found_lower, s->seq.lower);
> +               tipc_evt_write(evt, found_upper, s->seq.upper);
> +               tipc_evt_write(evt, port.ref, 0);
> +               tipc_evt_write(evt, port.node, 0);
> +       }
>         tipc_topsrv_queue_evt(sub->net, sub->conid, event, evt);
>  }
>
> @@ -61,24 +69,23 @@ static void tipc_sub_send_event(struct tipc_subscription *sub,
>   * @found_lower: lower value to test
>   * @found_upper: upper value to test
>   *
> - * Return: 1 if there is overlap, otherwise 0.
> + * Returns true if there is overlap, otherwise false.
>   */
> -int tipc_sub_check_overlap(struct tipc_service_range *seq, u32 found_lower,
> -                          u32 found_upper)
> +bool tipc_sub_check_overlap(struct tipc_service_range *sr,
> +                           u32 found_lower, u32 found_upper)
This function should be changed to 'static' after its declaration is
deleted below.

>  {
> -       if (found_lower < seq->lower)
> -               found_lower = seq->lower;
> -       if (found_upper > seq->upper)
> -               found_upper = seq->upper;
> +       if (found_lower < sr->lower)
> +               found_lower = sr->lower;
> +       if (found_upper > sr->upper)
> +               found_upper = sr->upper;
>         if (found_lower > found_upper)
> -               return 0;
> -       return 1;
> +               return false;
> +       return true;
>  }
>
>  void tipc_sub_report_overlap(struct tipc_subscription *sub,
> -                            u32 found_lower, u32 found_upper,
> -                            u32 event, u32 port, u32 node,
> -                            u32 scope, int must)
> +                            struct publication *p,
> +                            u32 event, bool must)
>  {
>         struct tipc_subscr *s = &sub->evt.s;
>         u32 filter = tipc_sub_read(s, filter);
> @@ -88,29 +95,25 @@ void tipc_sub_report_overlap(struct tipc_subscription *sub,
>         seq.lower = tipc_sub_read(s, seq.lower);
>         seq.upper = tipc_sub_read(s, seq.upper);
>
> -       if (!tipc_sub_check_overlap(&seq, found_lower, found_upper))
> +       if (!tipc_sub_check_overlap(&seq, p->sr.lower, p->sr.upper))
>                 return;
> -
>         if (!must && !(filter & TIPC_SUB_PORTS))
>                 return;
> -       if (filter & TIPC_SUB_CLUSTER_SCOPE && scope == TIPC_NODE_SCOPE)
> +       if (filter & TIPC_SUB_CLUSTER_SCOPE && p->scope == TIPC_NODE_SCOPE)
>                 return;
> -       if (filter & TIPC_SUB_NODE_SCOPE && scope != TIPC_NODE_SCOPE)
> +       if (filter & TIPC_SUB_NODE_SCOPE && p->scope != TIPC_NODE_SCOPE)
>                 return;
>         spin_lock(&sub->lock);
> -       tipc_sub_send_event(sub, found_lower, found_upper,
> -                           event, port, node);
> +       tipc_sub_send_event(sub, p, event);
>         spin_unlock(&sub->lock);
>  }
>
>  static void tipc_sub_timeout(struct timer_list *t)
>  {
>         struct tipc_subscription *sub = from_timer(sub, t, timer);
> -       struct tipc_subscr *s = &sub->evt.s;
>
>         spin_lock(&sub->lock);
> -       tipc_sub_send_event(sub, s->seq.lower, s->seq.upper,
> -                           TIPC_SUBSCR_TIMEOUT, 0, 0);
> +       tipc_sub_send_event(sub, NULL, TIPC_SUBSCR_TIMEOUT);
>         sub->inactive = true;
>         spin_unlock(&sub->lock);
>  }
> diff --git a/net/tipc/subscr.h b/net/tipc/subscr.h
> index 3ded27391d54..5fce2e13fc1a 100644
> --- a/net/tipc/subscr.h
> +++ b/net/tipc/subscr.h
> @@ -43,6 +43,7 @@
>  #define TIPC_MAX_SUBSCR         65535
>  #define TIPC_MAX_PUBL           65535
>
> +struct publication;
>  struct tipc_subscription;
>  struct tipc_conn;
>
> @@ -74,13 +75,9 @@ struct tipc_subscription *tipc_sub_subscribe(struct net *net,
>                                              struct tipc_subscr *s,
>                                              int conid);
>  void tipc_sub_unsubscribe(struct tipc_subscription *sub);
> -
> -int tipc_sub_check_overlap(struct tipc_service_range *seq,
> -                          u32 found_lower, u32 found_upper);
>  void tipc_sub_report_overlap(struct tipc_subscription *sub,
> -                            u32 found_lower, u32 found_upper,
> -                            u32 event, u32 port, u32 node,
> -                            u32 scope, int must);
> +                            struct publication *p,
> +                            u32 event, bool must);
>
>  int __net_init tipc_topsrv_init_net(struct net *net);
>  void __net_exit tipc_topsrv_exit_net(struct net *net);
> --
> 2.28.0
>
>
>
> _______________________________________________
> tipc-discussion mailing list
> tip...@li...
> https://lists.sourceforge.net/lists/listinfo/tipc-discussion

Re: [tipc-discussion] [net-next 13/16] tipc: simplify signature of tipc_find_service()

[Xin L. <luc...@gm...>]

On Wed, Dec 9, 2020 at 2:51 AM <jm...@re...> wrote:
>
> From: Jon Maloy <jm...@re...>
>
> We reduce the signature of tipc_find_service() and
> tipc_create_service(). The reason for doing this might not
> be obvious, but we plan to let struct tipc_uaddr contain
> information that is relevant for these functions in a later
> commit.
>
> Signed-off-by: Jon Maloy <jm...@re...>
> ---
>  net/tipc/name_table.c | 58 ++++++++++++++++++++++++++-----------------
>  net/tipc/name_table.h |  2 +-
>  net/tipc/socket.c     |  2 +-
>  3 files changed, 37 insertions(+), 25 deletions(-)
>
> diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c
> index 57abed74d0d9..554a006d7c0d 100644
> --- a/net/tipc/name_table.c
> +++ b/net/tipc/name_table.c
> @@ -255,20 +255,25 @@ static struct publication *tipc_publ_create(struct tipc_uaddr *ua,
>   *
>   * Allocates a single range structure and sets it to all 0's.
>   */
> -static struct tipc_service *tipc_service_create(u32 type, struct hlist_head *hd)
> +static struct tipc_service *tipc_service_create(struct net *net,
> +                                               struct tipc_uaddr *ua)
is that too much to pass 'ua' pointer if only 'type' is being used here?

>  {
> -       struct tipc_service *service = kzalloc(sizeof(*service), GFP_ATOMIC);
> +       struct name_table *nt = tipc_name_table(net);
> +       struct tipc_service *service;
> +       struct hlist_head *hd;
>
> +       service = kzalloc(sizeof(*service), GFP_ATOMIC);
>         if (!service) {
>                 pr_warn("Service creation failed, no memory\n");
>                 return NULL;
>         }
>
>         spin_lock_init(&service->lock);
> -       service->type = type;
> +       service->type = ua->sr.type;
>         service->ranges = RB_ROOT;
>         INIT_HLIST_NODE(&service->service_list);
>         INIT_LIST_HEAD(&service->subscriptions);
> +       hd = &nt->services[hash(ua->sr.type)];
>         hlist_add_head_rcu(&service->service_list, hd);
>         return service;
>  }
> @@ -453,15 +458,16 @@ static void tipc_service_subscribe(struct tipc_service *service,
>         }
>  }
>
> -static struct tipc_service *tipc_service_find(struct net *net, u32 type)
> +static struct tipc_service *tipc_service_find(struct net *net,
> +                                             struct tipc_uaddr *ua)
>  {
>         struct name_table *nt = tipc_name_table(net);
>         struct hlist_head *service_head;
>         struct tipc_service *service;
>
> -       service_head = &nt->services[hash(type)];
> +       service_head = &nt->services[hash(ua->sr.type)];
>         hlist_for_each_entry_rcu(service, service_head, service_list) {
> -               if (service->type == type)
> +               if (service->type == ua->sr.type)
>                         return service;
>         }
>         return NULL;
> @@ -472,7 +478,6 @@ struct publication *tipc_nametbl_insert_publ(struct net *net,
>                                              struct tipc_socket_addr *sk,
>                                              u32 key)
>  {
> -       struct name_table *nt = tipc_name_table(net);
>         struct tipc_service *sc;
>         struct publication *p;
>         u32 type = ua->sr.type;
> @@ -487,9 +492,9 @@ struct publication *tipc_nametbl_insert_publ(struct net *net,
>                          type, ua->sr.lower, ua->sr.upper, sk->node);
>                 return NULL;
>         }
> -       sc = tipc_service_find(net, type);
> +       sc = tipc_service_find(net, ua);
>         if (!sc)
> -               sc = tipc_service_create(type, &nt->services[hash(type)]);
> +               sc = tipc_service_create(net, ua);
>         if (sc) {
>                 spin_lock_bh(&sc->lock);
>                 res = tipc_service_insert_publ(net, sc, p);
> @@ -512,7 +517,7 @@ struct publication *tipc_nametbl_remove_publ(struct net *net,
>         struct tipc_service *sc;
>         bool last;
>
> -       sc = tipc_service_find(net, ua->sr.type);
> +       sc = tipc_service_find(net, ua);
>         if (!sc)
>                 return NULL;
>
> @@ -585,7 +590,7 @@ bool tipc_nametbl_lookup_anycast(struct net *net,
>                 return true;
>
>         rcu_read_lock();
> -       sc = tipc_service_find(net, ua->sr.type);
> +       sc = tipc_service_find(net, ua);
>         if (unlikely(!sc))
>                 goto exit;
>
> @@ -638,7 +643,7 @@ bool tipc_nametbl_lookup_group(struct net *net, struct tipc_uaddr *ua,
>
>         *dstcnt = 0;
>         rcu_read_lock();
> -       sc = tipc_service_find(net, ua->sa.type);
> +       sc = tipc_service_find(net, ua);
>         if (unlikely(!sc))
>                 goto exit;
>
> @@ -682,7 +687,7 @@ void tipc_nametbl_lookup_mcast_sockets(struct net *net, struct tipc_uaddr *ua,
>         u32 scope = ua->scope;
>
>         rcu_read_lock();
> -       sc = tipc_service_find(net, ua->sr.type);
> +       sc = tipc_service_find(net, ua);
>         if (!sc)
>                 goto exit;
>
> @@ -711,7 +716,7 @@ void tipc_nametbl_lookup_mcast_nodes(struct net *net, struct tipc_uaddr *ua,
>         struct publication *p;
>
>         rcu_read_lock();
> -       sc = tipc_service_find(net, ua->sr.type);
> +       sc = tipc_service_find(net, ua);
>         if (!sc)
>                 goto exit;
>
> @@ -729,7 +734,7 @@ void tipc_nametbl_lookup_mcast_nodes(struct net *net, struct tipc_uaddr *ua,
>  /* tipc_nametbl_build_group - build list of communication group members
>   */
>  void tipc_nametbl_build_group(struct net *net, struct tipc_group *grp,
> -                             u32 type, u32 scope)
> +                             struct tipc_uaddr *ua)
>  {
>         struct service_range *sr;
>         struct tipc_service *sc;
> @@ -737,7 +742,7 @@ void tipc_nametbl_build_group(struct net *net, struct tipc_group *grp,
>         struct rb_node *n;
>
>         rcu_read_lock();
> -       sc = tipc_service_find(net, type);
> +       sc = tipc_service_find(net, ua);
>         if (!sc)
>                 goto exit;
>
> @@ -745,9 +750,10 @@ void tipc_nametbl_build_group(struct net *net, struct tipc_group *grp,
>         for (n = rb_first(&sc->ranges); n; n = rb_next(n)) {
>                 sr = container_of(n, struct service_range, tree_node);
>                 list_for_each_entry(p, &sr->all_publ, all_publ) {
> -                       if (p->scope != scope)
> +                       if (p->scope != ua->scope)
>                                 continue;
> -                       tipc_group_add_member(grp, p->sk.node, p->sk.ref, p->sr.lower);
> +                       tipc_group_add_member(grp, p->sk.node, p->sk.ref,
> +                                             p->sr.lower);
>                 }
>         }
>         spin_unlock_bh(&sc->lock);
> @@ -829,17 +835,18 @@ void tipc_nametbl_withdraw(struct net *net, struct tipc_uaddr *ua,
>   */
>  bool tipc_nametbl_subscribe(struct tipc_subscription *sub)
>  {
> -       struct name_table *nt = tipc_name_table(sub->net);
>         struct tipc_net *tn = tipc_net(sub->net);
>         struct tipc_subscr *s = &sub->evt.s;
>         u32 type = tipc_sub_read(s, seq.type);
>         struct tipc_service *sc;
> +       struct tipc_uaddr ua;
>         bool res = true;
>
> +       tipc_uaddr(&ua, TIPC_SERVICE_RANGE, TIPC_NODE_SCOPE, type, 0, 0);
>         spin_lock_bh(&tn->nametbl_lock);
> -       sc = tipc_service_find(sub->net, type);
> +       sc = tipc_service_find(sub->net, &ua);
>         if (!sc)
> -               sc = tipc_service_create(type, &nt->services[hash(type)]);
> +               sc = tipc_service_create(sub->net, &ua);
>         if (sc) {
>                 spin_lock_bh(&sc->lock);
>                 tipc_service_subscribe(sc, sub);
> @@ -864,9 +871,11 @@ void tipc_nametbl_unsubscribe(struct tipc_subscription *sub)
>         struct tipc_subscr *s = &sub->evt.s;
>         u32 type = tipc_sub_read(s, seq.type);
>         struct tipc_service *sc;
> +       struct tipc_uaddr ua;
>
> +       tipc_uaddr(&ua, TIPC_SERVICE_RANGE, TIPC_NODE_SCOPE, type, 0, 0);
>         spin_lock_bh(&tn->nametbl_lock);
> -       sc = tipc_service_find(sub->net, type);
> +       sc = tipc_service_find(sub->net, &ua);
>         if (!sc)
>                 goto exit;
>
> @@ -1058,6 +1067,7 @@ static int tipc_nl_service_list(struct net *net, struct tipc_nl_msg *msg,
>         struct tipc_net *tn = tipc_net(net);
>         struct tipc_service *service = NULL;
>         struct hlist_head *head;
> +       struct tipc_uaddr ua;
>         int err;
>         int i;
>
> @@ -1071,7 +1081,9 @@ static int tipc_nl_service_list(struct net *net, struct tipc_nl_msg *msg,
>
>                 if (*last_type ||
>                     (!i && *last_key && (*last_lower == *last_key))) {
> -                       service = tipc_service_find(net, *last_type);
> +                       tipc_uaddr(&ua, TIPC_SERVICE_RANGE, TIPC_NODE_SCOPE,
> +                                  *last_type, *last_lower, *last_lower);
> +                       service = tipc_service_find(net, &ua);
>                         if (!service)
>                                 return -EPIPE;
>                 } else {
> diff --git a/net/tipc/name_table.h b/net/tipc/name_table.h
> index 909eaf706553..5b8102e70f3b 100644
> --- a/net/tipc/name_table.h
> +++ b/net/tipc/name_table.h
> @@ -121,7 +121,7 @@ bool tipc_nametbl_lookup_group(struct net *net, struct tipc_uaddr *ua,
>                                struct list_head *dsts, int *dstcnt,
>                                u32 exclude, bool mcast);
>  void tipc_nametbl_build_group(struct net *net, struct tipc_group *grp,
> -                             u32 type, u32 domain);
> +                             struct tipc_uaddr *ua);
>  struct publication *tipc_nametbl_publish(struct net *net, struct tipc_uaddr *ua,
>                                          struct tipc_socket_addr *sk, u32 key);
>  void tipc_nametbl_withdraw(struct net *net, struct tipc_uaddr *ua,
> diff --git a/net/tipc/socket.c b/net/tipc/socket.c
> index a349160a5ae4..8f1083d3859d 100644
> --- a/net/tipc/socket.c
> +++ b/net/tipc/socket.c
> @@ -3073,9 +3073,9 @@ static int tipc_sk_join(struct tipc_sock *tsk, struct tipc_group_req *mreq)
>         msg_set_lookup_scope(hdr, mreq->scope);
>         msg_set_nametype(hdr, mreq->type);
>         msg_set_dest_droppable(hdr, true);
> -       tipc_nametbl_build_group(net, grp, mreq->type, mreq->scope);
>         tipc_uaddr(&ua, TIPC_SERVICE_RANGE, mreq->scope,
>                    mreq->type, mreq->instance, mreq->instance);
> +       tipc_nametbl_build_group(net, grp, &ua);
>         rc = tipc_sk_publish(tsk, &ua);
>         if (rc) {
>                 tipc_group_delete(net, grp);
> --
> 2.28.0
>
>
>
> _______________________________________________
> tipc-discussion mailing list
> tip...@li...
> https://lists.sourceforge.net/lists/listinfo/tipc-discussion

Re: [tipc-discussion] [net-next 05/16] tipc: simplify call signatures for publication creation

[Xin L. <luc...@gm...>]

On Wed, Dec 9, 2020 at 2:51 AM <jm...@re...> wrote:
>
> From: Jon Maloy <jm...@re...>
>
> We simplify the call signatures for tipc_nametbl_insert_publ() and
> tipc_publ_create() so that fewer parameters are passed around.
>
> Signed-off-by: Jon Maloy <jm...@re...>
> ---
>  net/tipc/name_distr.c | 23 ++++++++--------
>  net/tipc/name_table.c | 61 +++++++++++++++++++------------------------
>  net/tipc/name_table.h | 10 ++++---
>  net/tipc/socket.c     |  8 ++----
>  4 files changed, 47 insertions(+), 55 deletions(-)
>
> diff --git a/net/tipc/name_distr.c b/net/tipc/name_distr.c
> index 721d2fca3d6f..df42fc2b4536 100644
> --- a/net/tipc/name_distr.c
> +++ b/net/tipc/name_distr.c
> @@ -293,30 +293,31 @@ static bool tipc_update_nametbl(struct net *net, struct distr_item *i,
>                                 u32 node, u32 dtype)
>  {
>         struct publication *p = NULL;
> -       u32 lower = ntohl(i->lower);
> -       u32 upper = ntohl(i->upper);
> -       u32 type = ntohl(i->type);
> -       u32 port = ntohl(i->port);
> +       struct tipc_socket_addr sk;
> +       struct tipc_uaddr ua;
>         u32 key = ntohl(i->key);
>
> +       tipc_uaddr(&ua, TIPC_SERVICE_RANGE, TIPC_CLUSTER_SCOPE,
> +                  ntohl(i->type), ntohl(i->lower), ntohl(i->upper));
> +       sk.ref = ntohl(i->port);
> +       sk.node = node;
> +
>         if (dtype == PUBLICATION) {
> -               p = tipc_nametbl_insert_publ(net, type, lower, upper,
> -                                            TIPC_CLUSTER_SCOPE, node,
> -                                            port, key);
> +               p = tipc_nametbl_insert_publ(net, &ua, &sk, key);
>                 if (p) {
>                         tipc_node_subscribe(net, &p->binding_node, node);
>                         return true;
>                 }
>         } else if (dtype == WITHDRAWAL) {
> -               p = tipc_nametbl_remove_publ(net, type, lower,
> -                                            upper, node, key);
> +               p = tipc_nametbl_remove_publ(net, ua.sr.type, ua.sr.lower,
> +                                            ua.sr.upper, node, key);
>                 if (p) {
>                         tipc_node_unsubscribe(net, &p->binding_node, node);
>                         kfree_rcu(p, rcu);
>                         return true;
>                 }
> -               pr_warn_ratelimited("Failed to remove binding %u,%u from %x\n",
> -                                   type, lower, node);
> +               pr_warn_ratelimited("Failed to remove binding %u,%u from %u\n",
> +                                   ua.sr.type, ua.sr.lower, node);
>         } else {
>                 pr_warn("Unrecognized name table message received\n");
>         }
> diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c
> index d951e9345122..ba96d5fc57f3 100644
> --- a/net/tipc/name_table.c
> +++ b/net/tipc/name_table.c
> @@ -222,36 +222,30 @@ static int hash(int x)
>
>  /**
>   * tipc_publ_create - create a publication structure
> - * @type: name sequence type
> - * @lower: name sequence lower bound
> - * @upper: name sequence upper bound
> - * @scope: publication scope
> - * @node: network address of publishing socket
> - * @port: publishing port
> + * @ua: the service range the user is binding to
> + * @sk: the address of the socket thatis bound
>   * @key: publication key
>   */
> -static struct publication *tipc_publ_create(u32 type, u32 lower, u32 upper,
> -                                           u32 scope, u32 node, u32 port,
> +static struct publication *tipc_publ_create(struct tipc_uaddr *ua,
> +                                           struct tipc_socket_addr *sk,
>                                             u32 key)
>  {
> -       struct publication *publ = kzalloc(sizeof(*publ), GFP_ATOMIC);
> +       struct publication *p = kzalloc(sizeof(*p), GFP_ATOMIC);
>
> -       if (!publ)
> +       if (!p)
>                 return NULL;
>
> -       publ->sr.type = type;
> -       publ->sr.lower = lower;
> -       publ->sr.upper = upper;
> -       publ->scope = scope;
> -       publ->sk.node = node;
> -       publ->sk.ref = port;
> -       publ->key = key;
> -       INIT_LIST_HEAD(&publ->binding_sock);
> -       INIT_LIST_HEAD(&publ->binding_node);
> -       INIT_LIST_HEAD(&publ->local_publ);
> -       INIT_LIST_HEAD(&publ->all_publ);
> -       INIT_LIST_HEAD(&publ->list);
> -       return publ;
> +       p->sr = ua->sr;
> +       p->sk = *sk;
> +       p->addrtype = ua->addrtype;
> +       p->scope = ua->scope;
> +       p->key = key;
> +       INIT_LIST_HEAD(&p->binding_sock);
> +       INIT_LIST_HEAD(&p->binding_node);
> +       INIT_LIST_HEAD(&p->local_publ);
> +       INIT_LIST_HEAD(&p->all_publ);
> +       INIT_LIST_HEAD(&p->list);
> +       return p;
>  }
>
>  /**
> @@ -468,23 +462,24 @@ static struct tipc_service *tipc_service_find(struct net *net, u32 type)
>         return NULL;
>  };
>
> -struct publication *tipc_nametbl_insert_publ(struct net *net, u32 type,
> -                                            u32 lower, u32 upper,
> -                                            u32 scope, u32 node,
> -                                            u32 port, u32 key)
> +struct publication *tipc_nametbl_insert_publ(struct net *net,
> +                                            struct tipc_uaddr *ua,
> +                                            struct tipc_socket_addr *sk,
> +                                            u32 key)
>  {
>         struct name_table *nt = tipc_name_table(net);
>         struct tipc_service *sc;
>         struct publication *p;
> +       u32 type = ua->sr.type;
>         bool res = false;
>
> -       p = tipc_publ_create(type, lower, upper, scope, node, port, key);
> +       p = tipc_publ_create(ua, sk, key);
>         if (!p)
>                 return NULL;
>
> -       if (scope > TIPC_NODE_SCOPE || lower > upper) {
> -               pr_debug("Failed to bind illegal {%u,%u,%u} with scope %u\n",
> -                        type, lower, upper, scope);
> +       if (ua->sr.lower > ua->sr.upper) {
> +               pr_debug("Failed to bind illegal {%u,%u,%u} from node %u\n",
> +                        type, ua->sr.lower, ua->sr.upper, sk->node);
>                 return NULL;
>         }
>         sc = tipc_service_find(net, type);
> @@ -758,9 +753,7 @@ struct publication *tipc_nametbl_publish(struct net *net, struct tipc_uaddr *ua,
>                 goto exit;
>         }
>
> -       p = tipc_nametbl_insert_publ(net, ua->sr.type, ua->sr.lower,
> -                                    ua->sr.upper, ua->scope,
> -                                    sk->node, sk->ref, key);
> +       p = tipc_nametbl_insert_publ(net, ua, sk, key);
>         if (p) {
>                 nt->local_publ_count++;
>                 skb = tipc_named_publish(net, p);
> diff --git a/net/tipc/name_table.h b/net/tipc/name_table.h
> index 5e969e060509..e12b9eb2c7f1 100644
> --- a/net/tipc/name_table.h
> +++ b/net/tipc/name_table.h
> @@ -75,7 +75,8 @@ struct tipc_uaddr;
>  struct publication {
>         struct tipc_service_range sr;
>         struct tipc_socket_addr sk;
> -       u32 scope;
> +       u16 addrtype;
> +       u16 scope;
It would be nice to add some description for this change in the changelog.

>         u32 key;
>         u32 id;
>         struct list_head binding_node;
> @@ -125,9 +126,10 @@ struct publication *tipc_nametbl_publish(struct net *net, struct tipc_uaddr *ua,
>                                          struct tipc_socket_addr *sk, u32 key);
>  int tipc_nametbl_withdraw(struct net *net, u32 type, u32 lower, u32 upper,
>                           u32 key);
> -struct publication *tipc_nametbl_insert_publ(struct net *net, u32 type,
> -                                            u32 lower, u32 upper, u32 scope,
> -                                            u32 node, u32 ref, u32 key);
> +struct publication *tipc_nametbl_insert_publ(struct net *net,
> +                                            struct tipc_uaddr *ua,
> +                                            struct tipc_socket_addr *sk,
> +                                            u32 key);
>  struct publication *tipc_nametbl_remove_publ(struct net *net, u32 type,
>                                              u32 lower, u32 upper,
>                                              u32 node, u32 key);
> diff --git a/net/tipc/socket.c b/net/tipc/socket.c
> index 598c8611b75f..0a92ebdd096d 100644
> --- a/net/tipc/socket.c
> +++ b/net/tipc/socket.c
> @@ -3088,12 +3088,8 @@ static int tipc_sk_join(struct tipc_sock *tsk, struct tipc_group_req *mreq)
>         msg_set_nametype(hdr, mreq->type);
>         msg_set_dest_droppable(hdr, true);
>         tipc_nametbl_build_group(net, grp, mreq->type, mreq->scope);
> -
> -       ua.sr.type = mreq->type;
> -       ua.sr.lower = mreq->instance;
> -       ua.sr.upper = ua.sr.lower;
> -       ua.scope = mreq->scope;
> -       ua.addrtype = TIPC_SERVICE_RANGE;
> +       tipc_uaddr(&ua, TIPC_SERVICE_RANGE, mreq->scope,
> +                  mreq->type, mreq->instance, mreq->instance);
>         rc = tipc_sk_publish(tsk, &ua);
>         if (rc) {
>                 tipc_group_delete(net, grp);
> --
> 2.28.0
>
>
>
> _______________________________________________
> tipc-discussion mailing list
> tip...@li...
> https://lists.sourceforge.net/lists/listinfo/tipc-discussion

Re: [tipc-discussion] [net-next 08/16] tipc: refactor tipc_sendmsg() and tipc_lookup_anycast()

[Xin L. <luc...@gm...>]

On Wed, Dec 9, 2020 at 2:51 AM <jm...@re...> wrote:
>
> From: Jon Maloy <jm...@re...>
>
> We simplify the signature if function tipc_nametbl_lookup_anycast(),
> using address structures instead of dicrete integers.
>
> This also makes it possible to make some improvements to the functions
> __tipc_sendmsg() in socket.c and tipc_msg_lookup_dest() in msg.c.
>
> Signed-off-by: Jon Maloy <jm...@re...>
> ---
>  net/tipc/msg.c        | 23 ++++++------
>  net/tipc/name_table.c | 75 +++++++++++++++++++-------------------
>  net/tipc/name_table.h |  5 ++-
>  net/tipc/socket.c     | 83 +++++++++++++++++++++----------------------
>  4 files changed, 91 insertions(+), 95 deletions(-)
>
> diff --git a/net/tipc/msg.c b/net/tipc/msg.c
> index 9eddbddb2fec..931245e93830 100644
> --- a/net/tipc/msg.c
> +++ b/net/tipc/msg.c
> @@ -711,8 +711,11 @@ bool tipc_msg_skb_clone(struct sk_buff_head *msg, struct sk_buff_head *cpy)
>  bool tipc_msg_lookup_dest(struct net *net, struct sk_buff *skb, int *err)
>  {
>         struct tipc_msg *msg = buf_msg(skb);
> -       u32 dport, dnode;
> -       u32 onode = tipc_own_addr(net);
> +       u32 scope = msg_lookup_scope(msg);
> +       u32 self = tipc_own_addr(net);
> +       u32 inst = msg_nameinst(msg);
> +       struct tipc_socket_addr sk;
> +       struct tipc_uaddr ua;
>
>         if (!msg_isdata(msg))
>                 return false;
> @@ -726,16 +729,16 @@ 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 = tipc_scope2node(net, msg_lookup_scope(msg));
> -       dport = tipc_nametbl_lookup_anycast(net, msg_nametype(msg),
> -                                           msg_nameinst(msg), &dnode);
> -       if (!dport)
> +       tipc_uaddr(&ua, TIPC_SERVICE_RANGE, scope,
> +                  msg_nametype(msg), inst, inst);
> +       sk.node = tipc_scope2node(net, scope);
> +       if (!tipc_nametbl_lookup_anycast(net, &ua, &sk))
>                 return false;
>         msg_incr_reroute_cnt(msg);
> -       if (dnode != onode)
> -               msg_set_prevnode(msg, onode);
> -       msg_set_destnode(msg, dnode);
> -       msg_set_destport(msg, dport);
> +       if (sk.node != self)
> +               msg_set_prevnode(msg, self);
> +       msg_set_destnode(msg, sk.node);
> +       msg_set_destport(msg, sk.ref);
>         *err = TIPC_OK;
>
>         return true;
> diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c
> index e6177ccf1140..ed68db36bab9 100644
> --- a/net/tipc/name_table.c
> +++ b/net/tipc/name_table.c
> @@ -549,66 +549,64 @@ struct publication *tipc_nametbl_remove_publ(struct net *net,
>  /**
>   * tipc_nametbl_lookup_anycast - perform service instance to socket translation
>   * @net: network namespace
> - * @type: message type
> - * @instance: message instance
> - * @dnode: the search domain used during translation
> - *
> - * On entry, 'dnode' is the search domain used during the lookup
> + * @ua: service address to look ip
> + * @sk: address to socket we want to find
>   *
> + * On entry, a non-zero 'sk->node' indicates the node where we want lookup to be
> + * performed, which may not be this one.
>   * On exit:
> - * - if lookup is deferred to another node, leave 'dnode' unchanged and return 0
> - * - if lookup is attempted and succeeds, set 'dnode' to the publishing node and
> - *   return the published (non-zero) port number
> - * - if lookup is attempted and fails, set 'dnode' to 0 and return 0
> + * - If lookup is deferred to another node, leave 'sk->node' unchanged and
> + *   return 'true'.
> + * - If lookup is successful, set the 'sk->node' and 'sk->ref' (== portid) which
> + *   represent the bound socket and return 'true'.
> + * - If lookup fails, return 'false'
>   *
>   * Note that for legacy users (node configured with Z.C.N address format) the
> - * 'closest-first' lookup algorithm must be maintained, i.e., if dnode is 0
> + * 'closest-first' lookup algorithm must be maintained, i.e., if sk.node is 0
>   * we must look in the local binding list first
>   */
> -u32 tipc_nametbl_lookup_anycast(struct net *net, u32 type,
> -                               u32 instance, u32 *dnode)
> +bool tipc_nametbl_lookup_anycast(struct net *net,
> +                                struct tipc_uaddr *ua,
> +                                struct tipc_socket_addr *sk)
>  {
>         struct tipc_net *tn = tipc_net(net);
>         bool legacy = tn->legacy_addr_format;
>         u32 self = tipc_own_addr(net);
> -       struct service_range *sr;
> +       u32 inst = ua->sa.instance;
> +       struct service_range *r;
>         struct tipc_service *sc;
> -       struct list_head *list;
>         struct publication *p;
> -       u32 port = 0;
> -       u32 node = 0;
> +       struct list_head *l;
> +       bool res = false;
>
> -       if (!tipc_in_scope(legacy, *dnode, self))
> -               return 0;
> +       if (!tipc_in_scope(legacy, sk->node, self))
> +               return true;
>
>         rcu_read_lock();
> -       sc = tipc_service_find(net, type);
> +       sc = tipc_service_find(net, ua->sr.type);
>         if (unlikely(!sc))
>                 goto exit;
>
>         spin_lock_bh(&sc->lock);
> -       service_range_foreach_match(sr, sc, instance, instance) {
> +       service_range_foreach_match(r, sc, inst, inst) {
>                 /* Select lookup algo: local, closest-first or round-robin */
> -               if (*dnode == self) {
> -                       list = &sr->local_publ;
> -                       if (list_empty(list))
> +               if (sk->node == self) {
> +                       l = &r->local_publ;
> +                       if (list_empty(l))
>                                 continue;
> -                       p = list_first_entry(list, struct publication,
> -                                            local_publ);
> -                       list_move_tail(&p->local_publ, &sr->local_publ);
> -               } else if (legacy && !*dnode && !list_empty(&sr->local_publ)) {
> -                       list = &sr->local_publ;
> -                       p = list_first_entry(list, struct publication,
> -                                            local_publ);
> -                       list_move_tail(&p->local_publ, &sr->local_publ);
> +                       p = list_first_entry(l, struct publication, local_publ);
> +                       list_move_tail(&p->local_publ, &r->local_publ);
> +               } else if (legacy && !sk->node && !list_empty(&r->local_publ)) {
> +                       l = &r->local_publ;
> +                       p = list_first_entry(l, struct publication, local_publ);
> +                       list_move_tail(&p->local_publ, &r->local_publ);
>                 } else {
> -                       list = &sr->all_publ;
> -                       p = list_first_entry(list, struct publication,
> -                                            all_publ);
> -                       list_move_tail(&p->all_publ, &sr->all_publ);
> +                       l = &r->all_publ;
> +                       p = list_first_entry(l, struct publication, all_publ);
> +                       list_move_tail(&p->all_publ, &r->all_publ);
>                 }
> -               port = p->sk.ref;
> -               node = p->sk.node;
> +               *sk = p->sk;
> +               res = true;
>                 /* Todo: as for legacy, pick the first matching range only, a
>                  * "true" round-robin will be performed as needed.
>                  */
> @@ -618,8 +616,7 @@ u32 tipc_nametbl_lookup_anycast(struct net *net, u32 type,
>
>  exit:
>         rcu_read_unlock();
> -       *dnode = node;
> -       return port;
> +       return res;
>  }
>
>  /* tipc_nametbl_lookup_group(): lookup destinaton(s) in a communication group
> diff --git a/net/tipc/name_table.h b/net/tipc/name_table.h
> index 9f6e8efca00f..f5e37f90a2ba 100644
> --- a/net/tipc/name_table.h
> +++ b/net/tipc/name_table.h
> @@ -111,9 +111,8 @@ struct name_table {
>  };
>
>  int tipc_nl_name_table_dump(struct sk_buff *skb, struct netlink_callback *cb);
> -
> -u32 tipc_nametbl_lookup_anycast(struct net *net, u32 type, u32 instance,
> -                               u32 *node);
> +bool tipc_nametbl_lookup_anycast(struct net *net, struct tipc_uaddr *ua,
> +                                struct tipc_socket_addr *sk);
>  void tipc_nametbl_lookup_mcast_sockets(struct net *net, u32 type, u32 lower,
>                                        u32 upper, u32 scope, bool exact,
>                                    struct list_head *dports);
> diff --git a/net/tipc/socket.c b/net/tipc/socket.c
> index 05cfe179458e..913b1a7be25b 100644
> --- a/net/tipc/socket.c
> +++ b/net/tipc/socket.c
> @@ -1424,44 +1424,43 @@ static int __tipc_sendmsg(struct socket *sock, struct msghdr *m, size_t dlen)
>         struct sock *sk = sock->sk;
>         struct net *net = sock_net(sk);
>         struct tipc_sock *tsk = tipc_sk(sk);
> -       DECLARE_SOCKADDR(struct sockaddr_tipc *, dest, m->msg_name);
> +       struct tipc_uaddr *ua = (struct tipc_uaddr *)m->msg_name;
>         long timeout = sock_sndtimeo(sk, m->msg_flags & MSG_DONTWAIT);
>         struct list_head *clinks = &tsk->cong_links;
>         bool syn = !tipc_sk_type_connectionless(sk);
>         struct tipc_group *grp = tsk->group;
>         struct tipc_msg *hdr = &tsk->phdr;
> -       struct tipc_service_range *seq;
> +       struct tipc_socket_addr skaddr;
>         struct sk_buff_head pkts;
> -       u32 dport = 0, dnode = 0;
> -       u32 type = 0, inst = 0;
> -       int mtu, rc;
> +       int atype, mtu, rc;
>
>         if (unlikely(dlen > TIPC_MAX_USER_MSG_SIZE))
>                 return -EMSGSIZE;
>
> -       if (likely(dest)) {
> -               if (unlikely(m->msg_namelen < sizeof(*dest)))
> -                       return -EINVAL;
> -               if (unlikely(dest->family != AF_TIPC))
> +       if (ua) {
> +               if (!tipc_uaddr_valid(ua, m->msg_namelen))
>                         return -EINVAL;
> +                atype = ua->addrtype;
>         }
>
> +       /* If socket belongs to a communication group follow other paths */
>         if (grp) {
> -               if (!dest)
> +               if (!ua)
>                         return tipc_send_group_bcast(sock, m, dlen, timeout);
> -               if (dest->addrtype == TIPC_SERVICE_ADDR)
> +               if (atype == TIPC_SERVICE_ADDR)
>                         return tipc_send_group_anycast(sock, m, dlen, timeout);
> -               if (dest->addrtype == TIPC_SOCKET_ADDR)
> +               if (atype == TIPC_SOCKET_ADDR)
>                         return tipc_send_group_unicast(sock, m, dlen, timeout);
> -               if (dest->addrtype == TIPC_ADDR_MCAST)
> +               if (atype == TIPC_SERVICE_RANGE)
>                         return tipc_send_group_mcast(sock, m, dlen, timeout);
>                 return -EINVAL;
>         }
>
> -       if (unlikely(!dest)) {
> -               dest = &tsk->peer;
> -               if (!syn && dest->family != AF_TIPC)
> +       if (!ua) {
> +               ua = (struct tipc_uaddr *)&tsk->peer;
> +               if (!syn && ua->family != AF_TIPC)
>                         return -EDESTADDRREQ;
> +                atype = ua->addrtype;
>         }
>
>         if (unlikely(syn)) {
> @@ -1471,54 +1470,51 @@ static int __tipc_sendmsg(struct socket *sock, struct msghdr *m, size_t dlen)
>                         return -EISCONN;
>                 if (tsk->published)
>                         return -EOPNOTSUPP;
> -               if (dest->addrtype == TIPC_SERVICE_ADDR) {
> -                       tsk->conn_type = dest->addr.name.name.type;
> -                       tsk->conn_instance = dest->addr.name.name.instance;
> +               if (atype == TIPC_SERVICE_ADDR) {
> +                       tsk->conn_type = ua->sa.type;
> +                       tsk->conn_instance = ua->sa.instance;
>                 }
>                 msg_set_syn(hdr, 1);
>         }
>
> -       seq = &dest->addr.nameseq;
> -       if (dest->addrtype == TIPC_ADDR_MCAST)
> -               return tipc_sendmcast(sock, seq, m, dlen, timeout);
> -
> -       if (dest->addrtype == TIPC_SERVICE_ADDR) {
> -               type = dest->addr.name.name.type;
> -               inst = dest->addr.name.name.instance;
> -               dnode = dest->addr.name.domain;
> -               dport = tipc_nametbl_lookup_anycast(net, type, inst, &dnode);
> -               if (unlikely(!dport && !dnode))
> +       /* Determine destination */
> +       if (atype == TIPC_SERVICE_RANGE) {
> +               return tipc_sendmcast(sock, &ua->sr, m, dlen, timeout);
> +       } else if (atype == TIPC_SERVICE_ADDR) {
> +               skaddr.node = ua->lookup_node;
> +               ua->scope = skaddr.node ? TIPC_NODE_SCOPE : TIPC_CLUSTER_SCOPE;
tipc_node2scope() can be deleted if it's not used here.

> +               if (!tipc_nametbl_lookup_anycast(net, ua, &skaddr))
>                         return -EHOSTUNREACH;
> -       } else if (dest->addrtype == TIPC_SOCKET_ADDR) {
> -               dnode = dest->addr.id.node;
> +       } else if (atype == TIPC_SOCKET_ADDR) {
> +               skaddr = ua->sk;
>         } else {
>                 return -EINVAL;
>         }
>
>         /* Block or return if destination link is congested */
>         rc = tipc_wait_for_cond(sock, &timeout,
> -                               !tipc_dest_find(clinks, dnode, 0));
> +                               !tipc_dest_find(clinks, skaddr.node, 0));
>         if (unlikely(rc))
>                 return rc;
>
> -       if (dest->addrtype == TIPC_SERVICE_ADDR) {
> +       /* Finally build message header */
> +       msg_set_destnode(hdr, skaddr.node);
> +       msg_set_destport(hdr, skaddr.ref);
> +       if (atype == TIPC_SERVICE_ADDR) {
>                 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_node2scope(dnode));
> -               msg_set_destnode(hdr, dnode);
> -               msg_set_destport(hdr, dport);
> +               msg_set_nametype(hdr, ua->sa.type);
> +               msg_set_nameinst(hdr, ua->sa.instance);
> +               msg_set_lookup_scope(hdr, ua->scope);
>         } else { /* TIPC_SOCKET_ADDR */
>                 msg_set_type(hdr, TIPC_DIRECT_MSG);
>                 msg_set_lookup_scope(hdr, 0);
> -               msg_set_destnode(hdr, dnode);
> -               msg_set_destport(hdr, dest->addr.id.ref);
>                 msg_set_hdr_sz(hdr, BASIC_H_SIZE);
>         }
>
> +       /* Add message body */
>         __skb_queue_head_init(&pkts);
> -       mtu = tipc_node_get_mtu(net, dnode, tsk->portid, true);
> +       mtu = tipc_node_get_mtu(net, skaddr.node, tsk->portid, true);
>         rc = tipc_msg_build(hdr, m, 0, dlen, mtu, &pkts);
>         if (unlikely(rc != dlen))
>                 return rc;
> @@ -1527,10 +1523,11 @@ static int __tipc_sendmsg(struct socket *sock, struct msghdr *m, size_t dlen)
>                 return -ENOMEM;
>         }
>
> +       /* Send message */
>         trace_tipc_sk_sendmsg(sk, skb_peek(&pkts), TIPC_DUMP_SK_SNDQ, " ");
> -       rc = tipc_node_xmit(net, &pkts, dnode, tsk->portid);
> +       rc = tipc_node_xmit(net, &pkts, skaddr.node, tsk->portid);
>         if (unlikely(rc == -ELINKCONG)) {
> -               tipc_dest_push(clinks, dnode, 0);
> +               tipc_dest_push(clinks, skaddr.node, 0);
>                 tsk->cong_link_cnt++;
>                 rc = 0;
>         }
> --
> 2.28.0
>
>
>
> _______________________________________________
> tipc-discussion mailing list
> tip...@li...
> https://lists.sourceforge.net/lists/listinfo/tipc-discussion

Re: [tipc-discussion] [net-next 06/16] tipc: simplify signature of tipc_nametbl_withdraw() functions

[Xin L. <luc...@gm...>]

On Wed, Dec 9, 2020 at 2:51 AM <jm...@re...> wrote:
>
> From: Jon Maloy <jm...@re...>
>
> Following the priniciples of the preceding commits, we reduce
> the number of parameters passed along in tipc_sk_withdraw(),
> tipc_nametbl_withdraw() and associated functions.
>
> Signed-off-by: Jon Maloy <jm...@re...>
> ---
>  net/tipc/name_distr.c | 11 ++++----
>  net/tipc/name_table.c | 54 +++++++++++++++++++-----------------
>  net/tipc/name_table.h | 11 ++++----
>  net/tipc/node.c       |  3 +-
>  net/tipc/socket.c     | 64 +++++++++++++++++++++----------------------
>  5 files changed, 74 insertions(+), 69 deletions(-)
>
> diff --git a/net/tipc/name_distr.c b/net/tipc/name_distr.c
> index df42fc2b4536..fe5f39792323 100644
> --- a/net/tipc/name_distr.c
> +++ b/net/tipc/name_distr.c
> @@ -244,17 +244,19 @@ static void tipc_publ_purge(struct net *net, struct publication *p, u32 addr)
>  {
>         struct tipc_net *tn = tipc_net(net);
>         struct publication *_p;
> +       struct tipc_uaddr ua;
>
> +       tipc_uaddr(&ua, p->addrtype, p->scope, p->sr.type,
> +                  p->sr.lower, p->sr.upper);
>         spin_lock_bh(&tn->nametbl_lock);
> -       _p = tipc_nametbl_remove_publ(net, p->sr.type, p->sr.lower,
> -                                     p->sr.upper, p->sk.node, p->key);
> +       _p = tipc_nametbl_remove_publ(net, &ua, &p->sk, p->key);
>         if (_p)
>                 tipc_node_unsubscribe(net, &_p->binding_node, addr);
>         spin_unlock_bh(&tn->nametbl_lock);
>
>         if (_p != p) {
>                 pr_err("Unable to remove publication from failed node\n"
> -                      " (type=%u, lower=%u, node=0x%x, port=%u, key=%u)\n",
> +                      " (type=%u, lower=%u, node=%u, port=%u, key=%u)\n",
Currently, there seem still some places using 0x%x to print node value.

>                        p->sr.type, p->sr.lower, p->sk.node, p->sk.ref, p->key);
>         }
>
> @@ -309,8 +311,7 @@ static bool tipc_update_nametbl(struct net *net, struct distr_item *i,
>                         return true;
>                 }
>         } else if (dtype == WITHDRAWAL) {
> -               p = tipc_nametbl_remove_publ(net, ua.sr.type, ua.sr.lower,
> -                                            ua.sr.upper, node, key);
> +               p = tipc_nametbl_remove_publ(net, &ua, &sk, key);
>                 if (p) {
>                         tipc_node_unsubscribe(net, &p->binding_node, node);
>                         kfree_rcu(p, rcu);
> diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c
> index ba96d5fc57f3..50562d086016 100644
> --- a/net/tipc/name_table.c
> +++ b/net/tipc/name_table.c
> @@ -367,12 +367,15 @@ static bool tipc_service_insert_publ(struct net *net,
>   * @node: target node
>   * @key: target publication key
>   */
> -static struct publication *tipc_service_remove_publ(struct service_range *sr,
> -                                                   u32 node, u32 key)
> +static struct publication *tipc_service_remove_publ(struct service_range *r,
> +                                                   struct tipc_uaddr *ua,
> +                                                   struct tipc_socket_addr *sk,
> +                                                   u32 key)
>  {
>         struct publication *p;
> +       u32 node = sk->node;
>
> -       list_for_each_entry(p, &sr->all_publ, all_publ) {
> +       list_for_each_entry(p, &r->all_publ, all_publ) {
>                 if (p->key != key || (node && node != p->sk.node))
>                         continue;
>                 list_del(&p->all_publ);
> @@ -496,32 +499,35 @@ struct publication *tipc_nametbl_insert_publ(struct net *net,
>         return NULL;
>  }
>
> -struct publication *tipc_nametbl_remove_publ(struct net *net, u32 type,
> -                                            u32 lower, u32 upper,
> -                                            u32 node, u32 key)
> +struct publication *tipc_nametbl_remove_publ(struct net *net,
> +                                            struct tipc_uaddr *ua,
> +                                            struct tipc_socket_addr *sk,
> +                                            u32 key)
>  {
> -       struct tipc_service *sc = tipc_service_find(net, type);
>         struct tipc_subscription *sub, *tmp;
> -       struct service_range *sr = NULL;
>         struct publication *p = NULL;
> +       struct service_range *sr;
> +       struct tipc_service *sc;
>         bool last;
>
> +       sc = tipc_service_find(net, ua->sr.type);
>         if (!sc)
>                 return NULL;
>
>         spin_lock_bh(&sc->lock);
> -       sr = tipc_service_find_range(sc, lower, upper);
> +       sr = tipc_service_find_range(sc, ua->sr.lower, ua->sr.upper);
>         if (!sr)
>                 goto exit;
> -       p = tipc_service_remove_publ(sr, node, key);
> +       p = tipc_service_remove_publ(sr, ua, sk, key);
>         if (!p)
>                 goto exit;
>
>         /* Notify any waiting subscriptions */
>         last = list_empty(&sr->all_publ);
>         list_for_each_entry_safe(sub, tmp, &sc->subscriptions, service_list) {
> -               tipc_sub_report_overlap(sub, lower, upper, TIPC_WITHDRAWN,
> -                                       p->sk.ref, node, p->scope, last);
> +               tipc_sub_report_overlap(sub, ua->sr.lower, ua->sr.upper,
> +                                       TIPC_WITHDRAWN, sk->ref, sk->node,
> +                                       ua->scope, last);
>         }
>
>         /* Remove service range item if this was its last publication */
> @@ -771,24 +777,22 @@ struct publication *tipc_nametbl_publish(struct net *net, struct tipc_uaddr *ua,
>  /**
>   * tipc_nametbl_withdraw - withdraw a service binding
>   * @net: network namespace
> - * @type: service type
> - * @lower: service range lower bound
> - * @upper: service range upper bound
> + * @ua: service address/range being unbound
> + * @sk: address of socket bveing unbound from
>   * @key: target publication key
>   */
> -int tipc_nametbl_withdraw(struct net *net, u32 type, u32 lower,
> -                         u32 upper, u32 key)
> +void tipc_nametbl_withdraw(struct net *net, struct tipc_uaddr *ua,
> +                          struct tipc_socket_addr *sk, u32 key)
>  {
>         struct name_table *nt = tipc_name_table(net);
>         struct tipc_net *tn = tipc_net(net);
> -       u32 self = tipc_own_addr(net);
>         struct sk_buff *skb = NULL;
>         struct publication *p;
>         u32 rc_dests;
>
>         spin_lock_bh(&tn->nametbl_lock);
>
> -       p = tipc_nametbl_remove_publ(net, type, lower, upper, self, key);
> +       p = tipc_nametbl_remove_publ(net, ua, sk, key);
>         if (p) {
>                 nt->local_publ_count--;
>                 skb = tipc_named_withdraw(net, p);
> @@ -796,16 +800,13 @@ int tipc_nametbl_withdraw(struct net *net, u32 type, u32 lower,
>                 kfree_rcu(p, rcu);
>         } else {
>                 pr_err("Failed to remove local publication {%u,%u,%u}/%u\n",
> -                      type, lower, upper, key);
> +                      ua->sr.type, ua->sr.lower, ua->sr.upper, key);
>         }
>         rc_dests = nt->rc_dests;
>         spin_unlock_bh(&tn->nametbl_lock);
>
> -       if (skb) {
> +       if (skb)
>                 tipc_node_broadcast(net, skb, rc_dests);
> -               return 1;
> -       }
> -       return 0;
>  }
>
>  /**
> @@ -899,11 +900,14 @@ static void tipc_service_delete(struct net *net, struct tipc_service *sc)
>  {
>         struct service_range *sr, *tmpr;
>         struct publication *p, *tmp;
> +       struct tipc_uaddr ua;
>
>         spin_lock_bh(&sc->lock);
>         rbtree_postorder_for_each_entry_safe(sr, tmpr, &sc->ranges, tree_node) {
>                 list_for_each_entry_safe(p, tmp, &sr->all_publ, all_publ) {
> -                       tipc_service_remove_publ(sr, p->sk.node, p->key);
> +                       tipc_uaddr(&ua, p->addrtype, p->scope,
> +                                  p->sr.type, p->sr.lower, p->sr.upper);
> +                       tipc_service_remove_publ(sr, &ua, &p->sk, p->key);
>                         kfree_rcu(p, rcu);
>                 }
>                 rb_erase_augmented(&sr->tree_node, &sc->ranges, &sr_callbacks);
> diff --git a/net/tipc/name_table.h b/net/tipc/name_table.h
> index e12b9eb2c7f1..5f48f05b93be 100644
> --- a/net/tipc/name_table.h
> +++ b/net/tipc/name_table.h
> @@ -124,15 +124,16 @@ bool tipc_nametbl_lookup(struct net *net, u32 type, u32 instance, u32 domain,
>                          bool all);
>  struct publication *tipc_nametbl_publish(struct net *net, struct tipc_uaddr *ua,
>                                          struct tipc_socket_addr *sk, u32 key);
> -int tipc_nametbl_withdraw(struct net *net, u32 type, u32 lower, u32 upper,
> -                         u32 key);
> +void tipc_nametbl_withdraw(struct net *net, struct tipc_uaddr *ua,
> +                          struct tipc_socket_addr *sk, u32 key);
>  struct publication *tipc_nametbl_insert_publ(struct net *net,
>                                              struct tipc_uaddr *ua,
>                                              struct tipc_socket_addr *sk,
>                                              u32 key);
> -struct publication *tipc_nametbl_remove_publ(struct net *net, u32 type,
> -                                            u32 lower, u32 upper,
> -                                            u32 node, u32 key);
> +struct publication *tipc_nametbl_remove_publ(struct net *net,
> +                                            struct tipc_uaddr *ua,
> +                                            struct tipc_socket_addr *sk,
> +                                            u32 key);
>  bool tipc_nametbl_subscribe(struct tipc_subscription *s);
>  void tipc_nametbl_unsubscribe(struct tipc_subscription *s);
>  int tipc_nametbl_init(struct net *net);
> diff --git a/net/tipc/node.c b/net/tipc/node.c
> index 3a71e26c9509..24f3f5ea968d 100644
> --- a/net/tipc/node.c
> +++ b/net/tipc/node.c
> @@ -429,8 +429,7 @@ static void tipc_node_write_unlock(struct tipc_node *n)
>         }
>         if (flags & TIPC_NOTIFY_LINK_DOWN) {
>                 tipc_mon_peer_down(net, n->addr, bearer_id);
> -               tipc_nametbl_withdraw(net, TIPC_LINK_STATE, n->addr,
> -                                     n->addr, n->link_id);
> +               tipc_nametbl_withdraw(net, &ua, &sk, n->link_id);
>         }
>  }
>
> diff --git a/net/tipc/socket.c b/net/tipc/socket.c
> index 0a92ebdd096d..5a017a34fb5f 100644
> --- a/net/tipc/socket.c
> +++ b/net/tipc/socket.c
> @@ -152,8 +152,7 @@ static int tipc_accept(struct socket *sock, struct socket *new_sock, int flags,
>                        bool kern);
>  static void tipc_sk_timeout(struct timer_list *t);
>  static int tipc_sk_publish(struct tipc_sock *tsk, struct tipc_uaddr *ua);
> -static int tipc_sk_withdraw(struct tipc_sock *tsk, uint scope,
> -                           struct tipc_service_range const *seq);
> +static int tipc_sk_withdraw(struct tipc_sock *tsk, struct tipc_uaddr *ua);
>  static int tipc_sk_leave(struct tipc_sock *tsk);
>  static struct tipc_sock *tipc_sk_lookup(struct net *net, u32 portid);
>  static int tipc_sk_insert(struct tipc_sock *tsk);
> @@ -643,7 +642,7 @@ static int tipc_release(struct socket *sock)
>         __tipc_shutdown(sock, TIPC_ERR_NO_PORT);
>         sk->sk_shutdown = SHUTDOWN_MASK;
>         tipc_sk_leave(tsk);
> -       tipc_sk_withdraw(tsk, 0, NULL);
> +       tipc_sk_withdraw(tsk, NULL);
>         __skb_queue_purge(&tsk->mc_method.deferredq);
>         sk_stop_timer(sk, &sk->sk_timer);
>         tipc_sk_remove(tsk);
> @@ -681,7 +680,7 @@ static int __tipc_bind(struct socket *sock, struct sockaddr *skaddr, int alen)
>         bool unbind = false;
>
>         if (unlikely(!alen))
> -               return tipc_sk_withdraw(tsk, 0, NULL);
> +               return tipc_sk_withdraw(tsk, NULL);
>
>         if (ua->addrtype == TIPC_SERVICE_ADDR) {
>                 ua->addrtype = TIPC_SERVICE_RANGE;
> @@ -699,7 +698,7 @@ static int __tipc_bind(struct socket *sock, struct sockaddr *skaddr, int alen)
>                 return -EACCES;
>
>         if (unbind)
> -               return tipc_sk_withdraw(tsk, ua->scope, &ua->sr);
> +               return tipc_sk_withdraw(tsk, ua);
>         return tipc_sk_publish(tsk, ua);
>  }
>
> @@ -2923,38 +2922,37 @@ static int tipc_sk_publish(struct tipc_sock *tsk, struct tipc_uaddr *ua)
>         return 0;
>  }
>
> -static int tipc_sk_withdraw(struct tipc_sock *tsk, uint scope,
> -                           struct tipc_service_range const *seq)
> +static int tipc_sk_withdraw(struct tipc_sock *tsk, struct tipc_uaddr *ua)
>  {
>         struct net *net = sock_net(&tsk->sk);
> -       struct publication *publ;
> -       struct publication *safe;
> +       struct publication *safe, *p;
> +       struct tipc_uaddr _ua;
>         int rc = -EINVAL;
>
> -       if (scope != TIPC_NODE_SCOPE)
> -               scope = TIPC_CLUSTER_SCOPE;
> -
> -       list_for_each_entry_safe(publ, safe, &tsk->publications, binding_sock) {
> -               if (seq) {
> -                       if (publ->scope != scope)
> -                               continue;
> -                       if (publ->sr.type != seq->type)
> -                               continue;
> -                       if (publ->sr.lower != seq->lower)
> -                               continue;
> -                       if (publ->sr.upper != seq->upper)
> -                               break;
> -                       tipc_nametbl_withdraw(net, publ->sr.type, publ->sr.lower,
> -                                             publ->sr.upper, publ->key);
> -                       rc = 0;
> -                       break;
> +       list_for_each_entry_safe(p, safe, &tsk->publications, binding_sock) {
> +               if (!ua) {
> +                       tipc_uaddr(&_ua, p->addrtype, p->scope,
> +                                  p->sr.type, p->sr.lower, p->sr.upper);
> +                       tipc_nametbl_withdraw(net, &_ua, &p->sk, p->key);
> +                       continue;
>                 }
> -               tipc_nametbl_withdraw(net, publ->sr.type, publ->sr.lower,
> -                                     publ->sr.upper, publ->key);
> +               /* Unbind specific publication */
> +               if (p->scope != ua->scope)
> +                       continue;
> +               if (p->sr.type != ua->sr.type)
> +                       continue;
> +               if (p->sr.lower != ua->sr.lower)
> +                       continue;
> +               if (p->sr.upper != ua->sr.upper)
> +                       break;
> +               tipc_nametbl_withdraw(net, ua, &p->sk, p->key);
>                 rc = 0;
> +               break;
>         }
> -       if (list_empty(&tsk->publications))
> +       if (list_empty(&tsk->publications)) {
>                 tsk->published = 0;
> +               rc = 0;
> +       }
>         return rc;
>  }
>
> @@ -3107,15 +3105,17 @@ static int tipc_sk_leave(struct tipc_sock *tsk)
>  {
>         struct net *net = sock_net(&tsk->sk);
>         struct tipc_group *grp = tsk->group;
> -       struct tipc_service_range seq;
> +       struct tipc_uaddr ua;
>         int scope;
>
>         if (!grp)
>                 return -EINVAL;
> -       tipc_group_self(grp, &seq, &scope);
> +       ua.addrtype = TIPC_SERVICE_RANGE;
> +       tipc_group_self(grp, &ua.sr, &scope);
> +       ua.scope = scope;
I'm thinking it's possible to change grp->scope to 'signed char' type,
same as scope in struct tipc_uaddr/sockaddr_tipc. then do:

        tipc_group_self(grp, &ua.sr, &ua.scope);

>         tipc_group_delete(net, grp);
>         tsk->group = NULL;
> -       tipc_sk_withdraw(tsk, scope, &seq);
> +       tipc_sk_withdraw(tsk, &ua);
>         return 0;
>  }
>
> --
> 2.28.0
>
>
>
> _______________________________________________
> tipc-discussion mailing list
> tip...@li...
> https://lists.sourceforge.net/lists/listinfo/tipc-discussion

Re: [tipc-discussion] [net-next 04/16] tipc: simplify signature of tipc_namtbl_publish()

[Xin L. <luc...@gm...>]

On Wed, Dec 9, 2020 at 2:51 AM <jm...@re...> wrote:
>
> From: Jon Maloy <jm...@re...>
>
> Using the new address structure tipc_uaddr, we simplify the signature
> of function tipc_sk_publish() and tipc_namtbl_publish() so that fewer
> parameters need to be passed around.
>
> Signed-off-by: Jon Maloy <jm...@re...>
> ---
>  net/tipc/name_table.c | 10 +++---
>  net/tipc/name_table.h |  6 ++--
>  net/tipc/net.c        |  8 +++--
>  net/tipc/node.c       | 29 +++++++++--------
>  net/tipc/socket.c     | 75 +++++++++++++++++++++++--------------------
>  5 files changed, 70 insertions(+), 58 deletions(-)
>
> diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c
> index 68e269b49780..d951e9345122 100644
> --- a/net/tipc/name_table.c
> +++ b/net/tipc/name_table.c
> @@ -742,9 +742,8 @@ void tipc_nametbl_build_group(struct net *net, struct tipc_group *grp,
>
>  /* tipc_nametbl_publish - add service binding to name table
>   */
> -struct publication *tipc_nametbl_publish(struct net *net, u32 type, u32 lower,
> -                                        u32 upper, u32 scope, u32 port,
> -                                        u32 key)
> +struct publication *tipc_nametbl_publish(struct net *net, struct tipc_uaddr *ua,
> +                                        struct tipc_socket_addr *sk, u32 key)
>  {
>         struct name_table *nt = tipc_name_table(net);
>         struct tipc_net *tn = tipc_net(net);
> @@ -759,8 +758,9 @@ struct publication *tipc_nametbl_publish(struct net *net, u32 type, u32 lower,
>                 goto exit;
>         }
>
> -       p = tipc_nametbl_insert_publ(net, type, lower, upper, scope,
> -                                    tipc_own_addr(net), port, key);
> +       p = tipc_nametbl_insert_publ(net, ua->sr.type, ua->sr.lower,
> +                                    ua->sr.upper, ua->scope,
> +                                    sk->node, sk->ref, key);
>         if (p) {
>                 nt->local_publ_count++;
>                 skb = tipc_named_publish(net, p);
> diff --git a/net/tipc/name_table.h b/net/tipc/name_table.h
> index 3fff00440e1a..5e969e060509 100644
> --- a/net/tipc/name_table.h
> +++ b/net/tipc/name_table.h
> @@ -42,6 +42,7 @@ struct tipc_subscription;
>  struct tipc_plist;
>  struct tipc_nlist;
>  struct tipc_group;
> +struct tipc_uaddr;
>
>  /*
>   * TIPC name types reserved for internal TIPC use (both current and planned)
> @@ -120,9 +121,8 @@ void tipc_nametbl_lookup_dst_nodes(struct net *net, u32 type, u32 lower,
>  bool tipc_nametbl_lookup(struct net *net, u32 type, u32 instance, u32 domain,
>                          struct list_head *dsts, int *dstcnt, u32 exclude,
>                          bool all);
> -struct publication *tipc_nametbl_publish(struct net *net, u32 type, u32 lower,
> -                                        u32 upper, u32 scope, u32 port,
> -                                        u32 key);
> +struct publication *tipc_nametbl_publish(struct net *net, struct tipc_uaddr *ua,
> +                                        struct tipc_socket_addr *sk, u32 key);
>  int tipc_nametbl_withdraw(struct net *net, u32 type, u32 lower, u32 upper,
>                           u32 key);
>  struct publication *tipc_nametbl_insert_publ(struct net *net, u32 type,
> diff --git a/net/tipc/net.c b/net/tipc/net.c
> index a129f661bee3..3f927949bb23 100644
> --- a/net/tipc/net.c
> +++ b/net/tipc/net.c
> @@ -125,6 +125,11 @@ int tipc_net_init(struct net *net, u8 *node_id, u32 addr)
>  static void tipc_net_finalize(struct net *net, u32 addr)
>  {
>         struct tipc_net *tn = tipc_net(net);
> +       struct tipc_socket_addr sk = {0, addr};
> +       struct tipc_uaddr ua;
> +
> +       tipc_uaddr(&ua, TIPC_SERVICE_RANGE, TIPC_CLUSTER_SCOPE,
> +                  TIPC_NODE_STATE, addr, addr);
>
>         if (cmpxchg(&tn->node_addr, 0, addr))
>                 return;
> @@ -132,8 +137,7 @@ static void tipc_net_finalize(struct net *net, u32 addr)
>         tipc_named_reinit(net);
>         tipc_sk_reinit(net);
>         tipc_mon_reinit_self(net);
> -       tipc_nametbl_publish(net, TIPC_NODE_STATE, addr, addr,
> -                            TIPC_CLUSTER_SCOPE, 0, addr);
> +       tipc_nametbl_publish(net, &ua, &sk, addr);
>  }
>
>  void tipc_net_finalize_work(struct work_struct *work)
> diff --git a/net/tipc/node.c b/net/tipc/node.c
> index 86b4d7ffb47a..3a71e26c9509 100644
> --- a/net/tipc/node.c
> +++ b/net/tipc/node.c
> @@ -393,21 +393,23 @@ static void tipc_node_write_unlock_fast(struct tipc_node *n)
>
>  static void tipc_node_write_unlock(struct tipc_node *n)
>  {
> +       struct tipc_socket_addr sk;
>         struct net *net = n->net;
> -       u32 addr = 0;
>         u32 flags = n->action_flags;
> -       u32 link_id = 0;
> -       u32 bearer_id;
>         struct list_head *publ_list;
> +       struct tipc_uaddr ua;
> +       u32 bearer_id;
>
>         if (likely(!flags)) {
>                 write_unlock_bh(&n->lock);
>                 return;
>         }
>
> -       addr = n->addr;
> -       link_id = n->link_id;
> -       bearer_id = link_id & 0xffff;
> +       tipc_uaddr(&ua, TIPC_SERVICE_RANGE, TIPC_NODE_SCOPE,
> +                  TIPC_LINK_STATE, n->addr, n->addr);
> +       sk.ref = n->link_id;
> +       sk.node = n->addr;
> +       bearer_id = n->link_id & 0xffff;
>         publ_list = &n->publ_list;
>
>         n->action_flags &= ~(TIPC_NOTIFY_NODE_DOWN | TIPC_NOTIFY_NODE_UP |
> @@ -416,20 +418,19 @@ static void tipc_node_write_unlock(struct tipc_node *n)
>         write_unlock_bh(&n->lock);
>
>         if (flags & TIPC_NOTIFY_NODE_DOWN)
> -               tipc_publ_notify(net, publ_list, addr, n->capabilities);
> +               tipc_publ_notify(net, publ_list, n->addr, n->capabilities);
>
>         if (flags & TIPC_NOTIFY_NODE_UP)
> -               tipc_named_node_up(net, addr, n->capabilities);
> +               tipc_named_node_up(net, n->addr, n->capabilities);
>
>         if (flags & TIPC_NOTIFY_LINK_UP) {
> -               tipc_mon_peer_up(net, addr, bearer_id);
> -               tipc_nametbl_publish(net, TIPC_LINK_STATE, addr, addr,
> -                                    TIPC_NODE_SCOPE, link_id, link_id);
> +               tipc_mon_peer_up(net, n->addr, bearer_id);
> +               tipc_nametbl_publish(net, &ua, &sk, n->link_id);
>         }
>         if (flags & TIPC_NOTIFY_LINK_DOWN) {
> -               tipc_mon_peer_down(net, addr, bearer_id);
> -               tipc_nametbl_withdraw(net, TIPC_LINK_STATE, addr,
> -                                     addr, link_id);
> +               tipc_mon_peer_down(net, n->addr, bearer_id);
> +               tipc_nametbl_withdraw(net, TIPC_LINK_STATE, n->addr,
> +                                     n->addr, n->link_id);
>         }
>  }
>
> diff --git a/net/tipc/socket.c b/net/tipc/socket.c
> index bff14df40bc9..598c8611b75f 100644
> --- a/net/tipc/socket.c
> +++ b/net/tipc/socket.c
> @@ -111,7 +111,6 @@ struct tipc_sock {
>         struct sock sk;
>         u32 conn_type;
>         u32 conn_instance;
> -       int published;
>         u32 max_pkt;
>         u32 maxnagle;
>         u32 portid;
> @@ -141,6 +140,7 @@ struct tipc_sock {
>         bool expect_ack;
>         bool nodelay;
>         bool group_is_open;
> +       bool published;
>  };
>
>  static int tipc_sk_backlog_rcv(struct sock *sk, struct sk_buff *skb);
> @@ -151,8 +151,7 @@ static int tipc_release(struct socket *sock);
>  static int tipc_accept(struct socket *sock, struct socket *new_sock, int flags,
>                        bool kern);
>  static void tipc_sk_timeout(struct timer_list *t);
> -static int tipc_sk_publish(struct tipc_sock *tsk, uint scope,
> -                          struct tipc_service_range const *seq);
> +static int tipc_sk_publish(struct tipc_sock *tsk, struct tipc_uaddr *ua);
>  static int tipc_sk_withdraw(struct tipc_sock *tsk, uint scope,
>                             struct tipc_service_range const *seq);
>  static int tipc_sk_leave(struct tipc_sock *tsk);
> @@ -677,22 +676,31 @@ static int tipc_release(struct socket *sock)
>   */
>  static int __tipc_bind(struct socket *sock, struct sockaddr *skaddr, int alen)
>  {
> -       struct sockaddr_tipc *addr = (struct sockaddr_tipc *)skaddr;
> +       struct tipc_uaddr *ua = (struct tipc_uaddr *)skaddr;
>         struct tipc_sock *tsk = tipc_sk(sock->sk);
> +       bool unbind = false;
>
>         if (unlikely(!alen))
>                 return tipc_sk_withdraw(tsk, 0, NULL);
>
> -       if (addr->addrtype == TIPC_SERVICE_ADDR)
> -               addr->addr.nameseq.upper = addr->addr.nameseq.lower;
> +       if (ua->addrtype == TIPC_SERVICE_ADDR) {
> +               ua->addrtype = TIPC_SERVICE_RANGE;
> +               ua->sr.upper = ua->sr.lower;
> +       }
> +       if (ua->scope < 0) {
> +               unbind = true;
> +               ua->scope = -ua->scope;
> +       }
> +       /* Users may still use deprecated TIPC_ZONE_SCOPE */
> +       if (ua->scope != TIPC_NODE_SCOPE)
> +               ua->scope = TIPC_CLUSTER_SCOPE;
>
>         if (tsk->group)
>                 return -EACCES;
>
> -       if (addr->scope >= 0)
> -               return tipc_sk_publish(tsk, addr->scope, &addr->addr.nameseq);
> -       else
> -               return tipc_sk_withdraw(tsk, -addr->scope, &addr->addr.nameseq);
> +       if (unbind)
> +               return tipc_sk_withdraw(tsk, ua->scope, &ua->sr);
> +       return tipc_sk_publish(tsk, ua);
>  }
>
>  int tipc_sk_bind(struct socket *sock, struct sockaddr *skaddr, int alen)
> @@ -707,18 +715,17 @@ int tipc_sk_bind(struct socket *sock, struct sockaddr *skaddr, int alen)
>
>  static int tipc_bind(struct socket *sock, struct sockaddr *skaddr, int alen)
>  {
> -       struct sockaddr_tipc *addr = (struct sockaddr_tipc *)skaddr;
> +       struct tipc_uaddr *ua = (struct tipc_uaddr *)skaddr;
> +       u32 atype = ua->addrtype;
>
>         if (alen) {
> -               if (alen < sizeof(struct sockaddr_tipc))
> +               if (!tipc_uaddr_valid(ua, alen))
>                         return -EINVAL;
> -               if (addr->family != AF_TIPC)
> +               if (atype == TIPC_SOCKET_ADDR)
>                         return -EAFNOSUPPORT;
> -               if (addr->addrtype > TIPC_SERVICE_ADDR)
> -                       return -EAFNOSUPPORT;
> -               if (addr->addr.nameseq.type < TIPC_RESERVED_TYPES) {
> +               if (ua->sr.type < TIPC_RESERVED_TYPES) {
>                         pr_warn_once("Can't bind to reserved service type %u\n",
> -                                    addr->addr.nameseq.type);
> +                                    ua->sr.type);
>                         return -EACCES;
>                 }
>         }
> @@ -2891,31 +2898,28 @@ static void tipc_sk_timeout(struct timer_list *t)
>         sock_put(sk);
>  }
>
> -static int tipc_sk_publish(struct tipc_sock *tsk, uint scope,
> -                          struct tipc_service_range const *seq)
> +static int tipc_sk_publish(struct tipc_sock *tsk, struct tipc_uaddr *ua)
>  {
>         struct sock *sk = &tsk->sk;
>         struct net *net = sock_net(sk);
> -       struct publication *publ;
> +       struct tipc_socket_addr skaddr;
> +       struct publication *p;
>         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;
>         if (key == tsk->portid)
>                 return -EADDRINUSE;
> -
> -       publ = tipc_nametbl_publish(net, seq->type, seq->lower, seq->upper,
> -                                   scope, tsk->portid, key);
> -       if (unlikely(!publ))
> +       skaddr.ref = tsk->portid;
> +       skaddr.node = tipc_own_addr(net);
> +       p = tipc_nametbl_publish(net, ua, &skaddr, key);
> +       if (unlikely(!p))
>                 return -EINVAL;
>
> -       list_add(&publ->binding_sock, &tsk->publications);
> +       list_add(&p->binding_sock, &tsk->publications);
>         tsk->pub_count++;
> -       tsk->published = 1;
> +       tsk->published = true;
>         return 0;
>  }
>
> @@ -3067,7 +3071,7 @@ static int tipc_sk_join(struct tipc_sock *tsk, struct tipc_group_req *mreq)
>         struct net *net = sock_net(&tsk->sk);
>         struct tipc_group *grp = tsk->group;
>         struct tipc_msg *hdr = &tsk->phdr;
> -       struct tipc_service_range seq;
> +       struct tipc_uaddr ua;
>         int rc;
>
>         if (mreq->type < TIPC_RESERVED_TYPES)
> @@ -3083,11 +3087,14 @@ static int tipc_sk_join(struct tipc_sock *tsk, struct tipc_group_req *mreq)
>         msg_set_lookup_scope(hdr, mreq->scope);
>         msg_set_nametype(hdr, mreq->type);
>         msg_set_dest_droppable(hdr, true);
> -       seq.type = mreq->type;
> -       seq.lower = mreq->instance;
> -       seq.upper = seq.lower;
>         tipc_nametbl_build_group(net, grp, mreq->type, mreq->scope);
> -       rc = tipc_sk_publish(tsk, mreq->scope, &seq);
Do we also need this check for tipc_sk_join():
       if (ua->scope != TIPC_NODE_SCOPE)
               ua->scope = TIPC_CLUSTER_SCOPE;
since this check has been removed from tipc_sk_publish()?

> +
> +       ua.sr.type = mreq->type;
> +       ua.sr.lower = mreq->instance;
> +       ua.sr.upper = ua.sr.lower;
> +       ua.scope = mreq->scope;
> +       ua.addrtype = TIPC_SERVICE_RANGE;
Maybe just call:
        tipc_uaddr(&ua, TIPC_SERVICE_RANGE, mreq->scope, mreq->type,
mreq->instance, ua.sr.lower)

> +       rc = tipc_sk_publish(tsk, &ua);
>         if (rc) {
>                 tipc_group_delete(net, grp);
>                 tsk->group = NULL;
> --
> 2.28.0
>
>
>
> _______________________________________________
> tipc-discussion mailing list
> tip...@li...
> https://lists.sourceforge.net/lists/listinfo/tipc-discussion

Re: [tipc-discussion] [net-next 02/16] tipc: move creation of publication item one level up in call chain

[Xin L. <luc...@gm...>]

On Wed, Dec 9, 2020 at 2:51 AM <jm...@re...> wrote:
>
> From: Jon Maloy <jm...@re...>
>
> We instantiante struct publication in tipc_nametbl_insert_publ()
> instead of as currently in tipc_service_insert_publ(). This has the
> advantage that we can pass a pointer to the publication struct to
> the next call levels, instead of the numerous individual parameters
> we pass on now. It also gives us a location to keep the contents of
> the additional fields we will introduce in a later commit.
>
> Signed-off-by: Jon Maloy <jm...@re...>
> ---
>  net/tipc/name_table.c | 63 ++++++++++++++++++++++---------------------
>  1 file changed, 32 insertions(+), 31 deletions(-)
>
> diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c
> index c37a4a9c87ca..68e269b49780 100644
> --- a/net/tipc/name_table.c
> +++ b/net/tipc/name_table.c
> @@ -327,49 +327,44 @@ static struct service_range *tipc_service_create_range(struct tipc_service *sc,
>         return sr;
>  }
>
> -static struct publication *tipc_service_insert_publ(struct net *net,
> -                                                   struct tipc_service *sc,
> -                                                   u32 type, u32 lower,
> -                                                   u32 upper, u32 scope,
> -                                                   u32 node, u32 port,
> -                                                   u32 key)
> +static bool tipc_service_insert_publ(struct net *net,
> +                                    struct tipc_service *sc,
> +                                    struct publication *p)
>  {
>         struct tipc_subscription *sub, *tmp;
>         struct service_range *sr;
> -       struct publication *p;
> +       struct publication *_p;
> +       u32 node = p->sk.node;
>         bool first = false;
>
> -       sr = tipc_service_create_range(sc, lower, upper);
> +       sr = tipc_service_create_range(sc, p->sr.lower, p->sr.upper);
>         if (!sr)
>                 goto  err;
>
>         first = list_empty(&sr->all_publ);
>
>         /* Return if the publication already exists */
> -       list_for_each_entry(p, &sr->all_publ, all_publ) {
> -               if (p->key == key && (!p->sk.node || p->sk.node == node))
> -                       return NULL;
> +       list_for_each_entry(_p, &sr->all_publ, all_publ) {
> +               if (_p->key == p->key && (!_p->sk.node || _p->sk.node == node))
> +                       return false;
>         }
>
> -       /* Create and insert publication */
> -       p = tipc_publ_create(type, lower, upper, scope, node, port, key);
> -       if (!p)
> -               goto err;
> -       /* Suppose there shouldn't be a huge gap btw publs i.e. >INT_MAX */
> -       p->id = sc->publ_cnt++;
> -       if (in_own_node(net, node))
> +       if (in_own_node(net, p->sk.node))
>                 list_add(&p->local_publ, &sr->local_publ);
>         list_add(&p->all_publ, &sr->all_publ);
> +       p->id = sc->publ_cnt++;
>
>         /* Any subscriptions waiting for notification?  */
>         list_for_each_entry_safe(sub, tmp, &sc->subscriptions, service_list) {
> -               tipc_sub_report_overlap(sub, p->sr.lower, p->sr.upper, TIPC_PUBLISHED,
> -                                       p->sk.ref, p->sk.node, p->scope, first);
> +               tipc_sub_report_overlap(sub, p->sr.lower, p->sr.upper,
> +                                       TIPC_PUBLISHED, p->sk.ref, p->sk.node,
> +                                       p->scope, first);
>         }
> -       return p;
> +       return true;
>  err:
> -       pr_warn("Failed to bind to %u,%u,%u, no memory\n", type, lower, upper);
> -       return NULL;
> +       pr_warn("Failed to bind to %u,%u,%u, no memory\n",
> +               p->sr.type, p->sr.lower, p->sr.upper);
> +       return false;
>  }
>
>  /**
> @@ -481,6 +476,11 @@ struct publication *tipc_nametbl_insert_publ(struct net *net, u32 type,
>         struct name_table *nt = tipc_name_table(net);
>         struct tipc_service *sc;
>         struct publication *p;
> +       bool res = false;
> +
> +       p = tipc_publ_create(type, lower, upper, scope, node, port, key);
> +       if (!p)
> +               return NULL;
>
>         if (scope > TIPC_NODE_SCOPE || lower > upper) {
>                 pr_debug("Failed to bind illegal {%u,%u,%u} with scope %u\n",
> @@ -490,14 +490,15 @@ struct publication *tipc_nametbl_insert_publ(struct net *net, u32 type,
>         sc = tipc_service_find(net, type);
>         if (!sc)
>                 sc = tipc_service_create(type, &nt->services[hash(type)]);
> -       if (!sc)
> -               return NULL;
> -
> -       spin_lock_bh(&sc->lock);
> -       p = tipc_service_insert_publ(net, sc, type, lower, upper,
> -                                    scope, node, port, key);
> -       spin_unlock_bh(&sc->lock);
> -       return p;
> +       if (sc) {
> +               spin_lock_bh(&sc->lock);
> +               res = tipc_service_insert_publ(net, sc, p);
> +               spin_unlock_bh(&sc->lock);
If move spin_(un)lock_bh into tipc_service_insert_publ(), we can do:

        if (sc && tipc_service_insert_publ(net, sc, p))
                return p;

> +       }
> +       if (res)
> +               return p;
and remove 'bool res = false;' and this 'if'.

> +       kfree(p);
> +       return NULL;
>  }
>
>  struct publication *tipc_nametbl_remove_publ(struct net *net, u32 type,
> --
> 2.28.0
>
>
>
> _______________________________________________
> tipc-discussion mailing list
> tip...@li...
> https://lists.sourceforge.net/lists/listinfo/tipc-discussion

Re: [tipc-discussion] [net-next 12/16] tipc: simplify signature of tipc_service_find_range()

[Tung Q. N. <tun...@de...>]

Hi Jon,

Just a minor comment for this patch.

Best regards,
Tung Nguyen

-----Original Message-----
From: jm...@re... <jm...@re...> 
Sent: Wednesday, December 9, 2020 1:50 AM
To: tip...@li...
Cc: Tung Quang Nguyen <tun...@de...>; Hoang Huu Le <hoa...@de...>; Tuong Tong Lien <tuo...@de...>; jm...@re...; ma...@do...; xi...@re...; yin...@wi...; par...@gm...
Subject: [net-next 12/16] tipc: simplify signature of tipc_service_find_range()

From: Jon Maloy <jm...@re...>

We simplify the signatures of the functions tipc_service_create_range()
and tipc_service_finc_range().
[Tung]: Wrong function name, tipc_service_finc_range / tipc_service_find_range

Signed-off-by: Jon Maloy <jm...@re...>
---
 net/tipc/name_table.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c
index 7f2fb446233e..57abed74d0d9 100644
--- a/net/tipc/name_table.c
+++ b/net/tipc/name_table.c
@@ -276,13 +276,13 @@ static struct tipc_service *tipc_service_create(u32 type, struct hlist_head *hd)
 /*  tipc_service_find_range - find service range matching publication parameters
  */
 static struct service_range *tipc_service_find_range(struct tipc_service *sc,
-						     u32 lower, u32 upper)
+						     struct tipc_uaddr *ua)
 {
 	struct service_range *sr;
 
-	service_range_foreach_match(sr, sc, lower, upper) {
+	service_range_foreach_match(sr, sc, ua->sr.lower, ua->sr.upper) {
 		/* Look for exact match */
-		if (sr->lower == lower && sr->upper == upper)
+		if (sr->lower == ua->sr.lower && sr->upper == ua->sr.upper)
 			return sr;
 	}
 
@@ -290,10 +290,12 @@ static struct service_range *tipc_service_find_range(struct tipc_service *sc,
 }
 
 static struct service_range *tipc_service_create_range(struct tipc_service *sc,
-						       u32 lower, u32 upper)
+						       struct publication *p)
 {
 	struct rb_node **n, *parent = NULL;
 	struct service_range *sr;
+	u32 lower = p->sr.lower;
+	u32 upper = p->sr.upper;
 
 	n = &sc->ranges.rb_node;
 	while (*n) {
@@ -331,7 +333,7 @@ static bool tipc_service_insert_publ(struct net *net,
 	u32 node = p->sk.node;
 	bool first = false;
 
-	sr = tipc_service_create_range(sc, p->sr.lower, p->sr.upper);
+	sr = tipc_service_create_range(sc, p);
 	if (!sr)
 		goto  err;
 
@@ -515,7 +517,7 @@ struct publication *tipc_nametbl_remove_publ(struct net *net,
 		return NULL;
 
 	spin_lock_bh(&sc->lock);
-	sr = tipc_service_find_range(sc, ua->sr.lower, ua->sr.upper);
+	sr = tipc_service_find_range(sc, ua);
 	if (!sr)
 		goto exit;
 	p = tipc_service_remove_publ(sr, ua, sk, key);
-- 
2.28.0

Re: [tipc-discussion] [net-next 08/16] tipc: refactor tipc_sendmsg() and tipc_lookup_anycast()

[Tung Q. N. <tun...@de...>]

Hi Jon,

Some minor comments for this patch (Marked with [Tung]).

Best regards,
Tung Nguyen

-----Original Message-----
From: jm...@re... <jm...@re...> 
Sent: Wednesday, December 9, 2020 1:50 AM
To: tip...@li...
Cc: Tung Quang Nguyen <tun...@de...>; Hoang Huu Le <hoa...@de...>; Tuong Tong Lien <tuo...@de...>; jm...@re...; ma...@do...; xi...@re...; yin...@wi...; par...@gm...
Subject: [net-next 08/16] tipc: refactor tipc_sendmsg() and tipc_lookup_anycast()

From: Jon Maloy <jm...@re...>

We simplify the signature if function tipc_nametbl_lookup_anycast(),
using address structures instead of dicrete integers.
[Tung]: typo. if / of, dicrete / discrete
This also makes it possible to make some improvements to the functions
__tipc_sendmsg() in socket.c and tipc_msg_lookup_dest() in msg.c.

Signed-off-by: Jon Maloy <jm...@re...>
---
 net/tipc/msg.c        | 23 ++++++------
 net/tipc/name_table.c | 75 +++++++++++++++++++-------------------
 net/tipc/name_table.h |  5 ++-
 net/tipc/socket.c     | 83 +++++++++++++++++++++----------------------
 4 files changed, 91 insertions(+), 95 deletions(-)

diff --git a/net/tipc/msg.c b/net/tipc/msg.c
index 9eddbddb2fec..931245e93830 100644
--- a/net/tipc/msg.c
+++ b/net/tipc/msg.c
@@ -711,8 +711,11 @@ bool tipc_msg_skb_clone(struct sk_buff_head *msg, struct sk_buff_head *cpy)
 bool tipc_msg_lookup_dest(struct net *net, struct sk_buff *skb, int *err)
 {
 	struct tipc_msg *msg = buf_msg(skb);
-	u32 dport, dnode;
-	u32 onode = tipc_own_addr(net);
+	u32 scope = msg_lookup_scope(msg);
+	u32 self = tipc_own_addr(net);
+	u32 inst = msg_nameinst(msg);
+	struct tipc_socket_addr sk;
+	struct tipc_uaddr ua;
 
 	if (!msg_isdata(msg))
 		return false;
@@ -726,16 +729,16 @@ 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 = tipc_scope2node(net, msg_lookup_scope(msg));
-	dport = tipc_nametbl_lookup_anycast(net, msg_nametype(msg),
-					    msg_nameinst(msg), &dnode);
-	if (!dport)
+	tipc_uaddr(&ua, TIPC_SERVICE_RANGE, scope,
+		   msg_nametype(msg), inst, inst);
+	sk.node = tipc_scope2node(net, scope);
+	if (!tipc_nametbl_lookup_anycast(net, &ua, &sk))
 		return false;
 	msg_incr_reroute_cnt(msg);
-	if (dnode != onode)
-		msg_set_prevnode(msg, onode);
-	msg_set_destnode(msg, dnode);
-	msg_set_destport(msg, dport);
+	if (sk.node != self)
+		msg_set_prevnode(msg, self);
+	msg_set_destnode(msg, sk.node);
+	msg_set_destport(msg, sk.ref);
 	*err = TIPC_OK;
 
 	return true;
diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c
index e6177ccf1140..ed68db36bab9 100644
--- a/net/tipc/name_table.c
+++ b/net/tipc/name_table.c
@@ -549,66 +549,64 @@ struct publication *tipc_nametbl_remove_publ(struct net *net,
 /**
  * tipc_nametbl_lookup_anycast - perform service instance to socket translation
  * @net: network namespace
- * @type: message type
- * @instance: message instance
- * @dnode: the search domain used during translation
- *
- * On entry, 'dnode' is the search domain used during the lookup
+ * @ua: service address to look ip
[Tung]: typo. ip / up
+ * @sk: address to socket we want to find
  *
+ * On entry, a non-zero 'sk->node' indicates the node where we want lookup to be
+ * performed, which may not be this one.
  * On exit:
- * - if lookup is deferred to another node, leave 'dnode' unchanged and return 0
- * - if lookup is attempted and succeeds, set 'dnode' to the publishing node and
- *   return the published (non-zero) port number
- * - if lookup is attempted and fails, set 'dnode' to 0 and return 0
+ * - If lookup is deferred to another node, leave 'sk->node' unchanged and
+ *   return 'true'.
+ * - If lookup is successful, set the 'sk->node' and 'sk->ref' (== portid) which
+ *   represent the bound socket and return 'true'.
+ * - If lookup fails, return 'false'
  *
  * Note that for legacy users (node configured with Z.C.N address format) the
- * 'closest-first' lookup algorithm must be maintained, i.e., if dnode is 0
+ * 'closest-first' lookup algorithm must be maintained, i.e., if sk.node is 0
  * we must look in the local binding list first
  */
-u32 tipc_nametbl_lookup_anycast(struct net *net, u32 type,
-				u32 instance, u32 *dnode)
+bool tipc_nametbl_lookup_anycast(struct net *net,
+				 struct tipc_uaddr *ua,
+				 struct tipc_socket_addr *sk)
 {
 	struct tipc_net *tn = tipc_net(net);
 	bool legacy = tn->legacy_addr_format;
 	u32 self = tipc_own_addr(net);
-	struct service_range *sr;
+	u32 inst = ua->sa.instance;
+	struct service_range *r;
 	struct tipc_service *sc;
-	struct list_head *list;
 	struct publication *p;
-	u32 port = 0;
-	u32 node = 0;
+	struct list_head *l;
+	bool res = false;
 
-	if (!tipc_in_scope(legacy, *dnode, self))
-		return 0;
+	if (!tipc_in_scope(legacy, sk->node, self))
+		return true;
 
 	rcu_read_lock();
-	sc = tipc_service_find(net, type);
+	sc = tipc_service_find(net, ua->sr.type);
 	if (unlikely(!sc))
 		goto exit;
 
 	spin_lock_bh(&sc->lock);
-	service_range_foreach_match(sr, sc, instance, instance) {
+	service_range_foreach_match(r, sc, inst, inst) {
 		/* Select lookup algo: local, closest-first or round-robin */
-		if (*dnode == self) {
-			list = &sr->local_publ;
-			if (list_empty(list))
+		if (sk->node == self) {
+			l = &r->local_publ;
+			if (list_empty(l))
 				continue;
-			p = list_first_entry(list, struct publication,
-					     local_publ);
-			list_move_tail(&p->local_publ, &sr->local_publ);
-		} else if (legacy && !*dnode && !list_empty(&sr->local_publ)) {
-			list = &sr->local_publ;
-			p = list_first_entry(list, struct publication,
-					     local_publ);
-			list_move_tail(&p->local_publ, &sr->local_publ);
+			p = list_first_entry(l, struct publication, local_publ);
+			list_move_tail(&p->local_publ, &r->local_publ);
+		} else if (legacy && !sk->node && !list_empty(&r->local_publ)) {
+			l = &r->local_publ;
+			p = list_first_entry(l, struct publication, local_publ);
+			list_move_tail(&p->local_publ, &r->local_publ);
 		} else {
-			list = &sr->all_publ;
-			p = list_first_entry(list, struct publication,
-					     all_publ);
-			list_move_tail(&p->all_publ, &sr->all_publ);
+			l = &r->all_publ;
+			p = list_first_entry(l, struct publication, all_publ);
+			list_move_tail(&p->all_publ, &r->all_publ);
 		}
-		port = p->sk.ref;
-		node = p->sk.node;
+		*sk = p->sk;
+		res = true;
 		/* Todo: as for legacy, pick the first matching range only, a
 		 * "true" round-robin will be performed as needed.
 		 */
@@ -618,8 +616,7 @@ u32 tipc_nametbl_lookup_anycast(struct net *net, u32 type,
 
 exit:
 	rcu_read_unlock();
-	*dnode = node;
-	return port;
+	return res;
 }
 
 /* tipc_nametbl_lookup_group(): lookup destinaton(s) in a communication group
diff --git a/net/tipc/name_table.h b/net/tipc/name_table.h
index 9f6e8efca00f..f5e37f90a2ba 100644
--- a/net/tipc/name_table.h
+++ b/net/tipc/name_table.h
@@ -111,9 +111,8 @@ struct name_table {
 };
 
 int tipc_nl_name_table_dump(struct sk_buff *skb, struct netlink_callback *cb);
-
-u32 tipc_nametbl_lookup_anycast(struct net *net, u32 type, u32 instance,
-				u32 *node);
+bool tipc_nametbl_lookup_anycast(struct net *net, struct tipc_uaddr *ua,
+				 struct tipc_socket_addr *sk);
 void tipc_nametbl_lookup_mcast_sockets(struct net *net, u32 type, u32 lower,
 				       u32 upper, u32 scope, bool exact,
 				   struct list_head *dports);
diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 05cfe179458e..913b1a7be25b 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -1424,44 +1424,43 @@ static int __tipc_sendmsg(struct socket *sock, struct msghdr *m, size_t dlen)
 	struct sock *sk = sock->sk;
 	struct net *net = sock_net(sk);
 	struct tipc_sock *tsk = tipc_sk(sk);
-	DECLARE_SOCKADDR(struct sockaddr_tipc *, dest, m->msg_name);
+	struct tipc_uaddr *ua = (struct tipc_uaddr *)m->msg_name;
 	long timeout = sock_sndtimeo(sk, m->msg_flags & MSG_DONTWAIT);
 	struct list_head *clinks = &tsk->cong_links;
 	bool syn = !tipc_sk_type_connectionless(sk);
 	struct tipc_group *grp = tsk->group;
 	struct tipc_msg *hdr = &tsk->phdr;
-	struct tipc_service_range *seq;
+	struct tipc_socket_addr skaddr;
 	struct sk_buff_head pkts;
-	u32 dport = 0, dnode = 0;
-	u32 type = 0, inst = 0;
-	int mtu, rc;
+	int atype, mtu, rc;
 
 	if (unlikely(dlen > TIPC_MAX_USER_MSG_SIZE))
 		return -EMSGSIZE;
 
-	if (likely(dest)) {
-		if (unlikely(m->msg_namelen < sizeof(*dest)))
-			return -EINVAL;
-		if (unlikely(dest->family != AF_TIPC))
+	if (ua) {
+		if (!tipc_uaddr_valid(ua, m->msg_namelen))
 			return -EINVAL;
+		 atype = ua->addrtype;
 	}
 
+	/* If socket belongs to a communication group follow other paths */
 	if (grp) {
-		if (!dest)
+		if (!ua)
 			return tipc_send_group_bcast(sock, m, dlen, timeout);
-		if (dest->addrtype == TIPC_SERVICE_ADDR)
+		if (atype == TIPC_SERVICE_ADDR)
 			return tipc_send_group_anycast(sock, m, dlen, timeout);
-		if (dest->addrtype == TIPC_SOCKET_ADDR)
+		if (atype == TIPC_SOCKET_ADDR)
 			return tipc_send_group_unicast(sock, m, dlen, timeout);
-		if (dest->addrtype == TIPC_ADDR_MCAST)
+		if (atype == TIPC_SERVICE_RANGE)
 			return tipc_send_group_mcast(sock, m, dlen, timeout);
 		return -EINVAL;
 	}
 
-	if (unlikely(!dest)) {
-		dest = &tsk->peer;
-		if (!syn && dest->family != AF_TIPC)
+	if (!ua) {
+		ua = (struct tipc_uaddr *)&tsk->peer;
+		if (!syn && ua->family != AF_TIPC)
 			return -EDESTADDRREQ;
+		 atype = ua->addrtype;
 	}
 
 	if (unlikely(syn)) {
@@ -1471,54 +1470,51 @@ static int __tipc_sendmsg(struct socket *sock, struct msghdr *m, size_t dlen)
 			return -EISCONN;
 		if (tsk->published)
 			return -EOPNOTSUPP;
-		if (dest->addrtype == TIPC_SERVICE_ADDR) {
-			tsk->conn_type = dest->addr.name.name.type;
-			tsk->conn_instance = dest->addr.name.name.instance;
+		if (atype == TIPC_SERVICE_ADDR) {
+			tsk->conn_type = ua->sa.type;
+			tsk->conn_instance = ua->sa.instance;
 		}
 		msg_set_syn(hdr, 1);
 	}
 
-	seq = &dest->addr.nameseq;
-	if (dest->addrtype == TIPC_ADDR_MCAST)
-		return tipc_sendmcast(sock, seq, m, dlen, timeout);
-
-	if (dest->addrtype == TIPC_SERVICE_ADDR) {
-		type = dest->addr.name.name.type;
-		inst = dest->addr.name.name.instance;
-		dnode = dest->addr.name.domain;
-		dport = tipc_nametbl_lookup_anycast(net, type, inst, &dnode);
-		if (unlikely(!dport && !dnode))
+	/* Determine destination */
+	if (atype == TIPC_SERVICE_RANGE) {
+		return tipc_sendmcast(sock, &ua->sr, m, dlen, timeout);
+	} else if (atype == TIPC_SERVICE_ADDR) {
+		skaddr.node = ua->lookup_node;
+		ua->scope = skaddr.node ? TIPC_NODE_SCOPE : TIPC_CLUSTER_SCOPE;
+		if (!tipc_nametbl_lookup_anycast(net, ua, &skaddr))
 			return -EHOSTUNREACH;
-	} else if (dest->addrtype == TIPC_SOCKET_ADDR) {
-		dnode = dest->addr.id.node;
+	} else if (atype == TIPC_SOCKET_ADDR) {
+		skaddr = ua->sk;
 	} else {
 		return -EINVAL;
 	}
 
 	/* Block or return if destination link is congested */
 	rc = tipc_wait_for_cond(sock, &timeout,
-				!tipc_dest_find(clinks, dnode, 0));
+				!tipc_dest_find(clinks, skaddr.node, 0));
 	if (unlikely(rc))
 		return rc;
 
-	if (dest->addrtype == TIPC_SERVICE_ADDR) {
+	/* Finally build message header */
+	msg_set_destnode(hdr, skaddr.node);
+	msg_set_destport(hdr, skaddr.ref);
+	if (atype == TIPC_SERVICE_ADDR) {
 		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_node2scope(dnode));
-		msg_set_destnode(hdr, dnode);
-		msg_set_destport(hdr, dport);
+		msg_set_nametype(hdr, ua->sa.type);
+		msg_set_nameinst(hdr, ua->sa.instance);
+		msg_set_lookup_scope(hdr, ua->scope);
 	} else { /* TIPC_SOCKET_ADDR */
 		msg_set_type(hdr, TIPC_DIRECT_MSG);
 		msg_set_lookup_scope(hdr, 0);
-		msg_set_destnode(hdr, dnode);
-		msg_set_destport(hdr, dest->addr.id.ref);
 		msg_set_hdr_sz(hdr, BASIC_H_SIZE);
 	}
 
+	/* Add message body */
 	__skb_queue_head_init(&pkts);
-	mtu = tipc_node_get_mtu(net, dnode, tsk->portid, true);
+	mtu = tipc_node_get_mtu(net, skaddr.node, tsk->portid, true);
 	rc = tipc_msg_build(hdr, m, 0, dlen, mtu, &pkts);
 	if (unlikely(rc != dlen))
 		return rc;
@@ -1527,10 +1523,11 @@ static int __tipc_sendmsg(struct socket *sock, struct msghdr *m, size_t dlen)
 		return -ENOMEM;
 	}
 
+	/* Send message */
 	trace_tipc_sk_sendmsg(sk, skb_peek(&pkts), TIPC_DUMP_SK_SNDQ, " ");
-	rc = tipc_node_xmit(net, &pkts, dnode, tsk->portid);
+	rc = tipc_node_xmit(net, &pkts, skaddr.node, tsk->portid);
 	if (unlikely(rc == -ELINKCONG)) {
-		tipc_dest_push(clinks, dnode, 0);
+		tipc_dest_push(clinks, skaddr.node, 0);
 		tsk->cong_link_cnt++;
 		rc = 0;
 	}
-- 
2.28.0

Re: [tipc-discussion] [net-next 06/16] tipc: simplify signature of tipc_nametbl_withdraw() functions

[Tung Q. N. <tun...@de...>]

Hi Jon,

Some minor comments for this patch (Marked with [Tung]).

Best regards,
Tung Nguyen

-----Original Message-----
From: jm...@re... <jm...@re...> 
Sent: Wednesday, December 9, 2020 1:50 AM
To: tip...@li...
Cc: Tung Quang Nguyen <tun...@de...>; Hoang Huu Le <hoa...@de...>; Tuong Tong Lien <tuo...@de...>; jm...@re...; ma...@do...; xi...@re...; yin...@wi...; par...@gm...
Subject: [net-next 06/16] tipc: simplify signature of tipc_nametbl_withdraw() functions

From: Jon Maloy <jm...@re...>

Following the priniciples of the preceding commits, we reduce
[Tung]: typo. priniciples/principles
the number of parameters passed along in tipc_sk_withdraw(),
tipc_nametbl_withdraw() and associated functions.

Signed-off-by: Jon Maloy <jm...@re...>
---
 net/tipc/name_distr.c | 11 ++++----
 net/tipc/name_table.c | 54 +++++++++++++++++++-----------------
 net/tipc/name_table.h | 11 ++++----
 net/tipc/node.c       |  3 +-
 net/tipc/socket.c     | 64 +++++++++++++++++++++----------------------
 5 files changed, 74 insertions(+), 69 deletions(-)

diff --git a/net/tipc/name_distr.c b/net/tipc/name_distr.c
index df42fc2b4536..fe5f39792323 100644
--- a/net/tipc/name_distr.c
+++ b/net/tipc/name_distr.c
@@ -244,17 +244,19 @@ static void tipc_publ_purge(struct net *net, struct publication *p, u32 addr)
 {
 	struct tipc_net *tn = tipc_net(net);
 	struct publication *_p;
+	struct tipc_uaddr ua;
 
+	tipc_uaddr(&ua, p->addrtype, p->scope, p->sr.type,
+		   p->sr.lower, p->sr.upper);
 	spin_lock_bh(&tn->nametbl_lock);
-	_p = tipc_nametbl_remove_publ(net, p->sr.type, p->sr.lower,
-				      p->sr.upper, p->sk.node, p->key);
+	_p = tipc_nametbl_remove_publ(net, &ua, &p->sk, p->key);
 	if (_p)
 		tipc_node_unsubscribe(net, &_p->binding_node, addr);
 	spin_unlock_bh(&tn->nametbl_lock);
 
 	if (_p != p) {
 		pr_err("Unable to remove publication from failed node\n"
-		       " (type=%u, lower=%u, node=0x%x, port=%u, key=%u)\n",
+		       " (type=%u, lower=%u, node=%u, port=%u, key=%u)\n",
 		       p->sr.type, p->sr.lower, p->sk.node, p->sk.ref, p->key);
 	}
 
@@ -309,8 +311,7 @@ static bool tipc_update_nametbl(struct net *net, struct distr_item *i,
 			return true;
 		}
 	} else if (dtype == WITHDRAWAL) {
-		p = tipc_nametbl_remove_publ(net, ua.sr.type, ua.sr.lower,
-					     ua.sr.upper, node, key);
+		p = tipc_nametbl_remove_publ(net, &ua, &sk, key);
 		if (p) {
 			tipc_node_unsubscribe(net, &p->binding_node, node);
 			kfree_rcu(p, rcu);
diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c
index ba96d5fc57f3..50562d086016 100644
--- a/net/tipc/name_table.c
+++ b/net/tipc/name_table.c
@@ -367,12 +367,15 @@ static bool tipc_service_insert_publ(struct net *net,
  * @node: target node
  * @key: target publication key
  */
-static struct publication *tipc_service_remove_publ(struct service_range *sr,
-						    u32 node, u32 key)
+static struct publication *tipc_service_remove_publ(struct service_range *r,
+						    struct tipc_uaddr *ua,
+						    struct tipc_socket_addr *sk,
+						    u32 key)
 {
 	struct publication *p;
+	u32 node = sk->node;
 
-	list_for_each_entry(p, &sr->all_publ, all_publ) {
+	list_for_each_entry(p, &r->all_publ, all_publ) {
 		if (p->key != key || (node && node != p->sk.node))
 			continue;
 		list_del(&p->all_publ);
@@ -496,32 +499,35 @@ struct publication *tipc_nametbl_insert_publ(struct net *net,
 	return NULL;
 }
 
-struct publication *tipc_nametbl_remove_publ(struct net *net, u32 type,
-					     u32 lower, u32 upper,
-					     u32 node, u32 key)
+struct publication *tipc_nametbl_remove_publ(struct net *net,
+					     struct tipc_uaddr *ua,
+					     struct tipc_socket_addr *sk,
+					     u32 key)
 {
-	struct tipc_service *sc = tipc_service_find(net, type);
 	struct tipc_subscription *sub, *tmp;
-	struct service_range *sr = NULL;
 	struct publication *p = NULL;
+	struct service_range *sr;
+	struct tipc_service *sc;
 	bool last;
 
+	sc = tipc_service_find(net, ua->sr.type);
 	if (!sc)
 		return NULL;
 
 	spin_lock_bh(&sc->lock);
-	sr = tipc_service_find_range(sc, lower, upper);
+	sr = tipc_service_find_range(sc, ua->sr.lower, ua->sr.upper);
 	if (!sr)
 		goto exit;
-	p = tipc_service_remove_publ(sr, node, key);
+	p = tipc_service_remove_publ(sr, ua, sk, key);
 	if (!p)
 		goto exit;
 
 	/* Notify any waiting subscriptions */
 	last = list_empty(&sr->all_publ);
 	list_for_each_entry_safe(sub, tmp, &sc->subscriptions, service_list) {
-		tipc_sub_report_overlap(sub, lower, upper, TIPC_WITHDRAWN,
-					p->sk.ref, node, p->scope, last);
+		tipc_sub_report_overlap(sub, ua->sr.lower, ua->sr.upper,
+					TIPC_WITHDRAWN, sk->ref, sk->node,
+					ua->scope, last);
 	}
 
 	/* Remove service range item if this was its last publication */
@@ -771,24 +777,22 @@ struct publication *tipc_nametbl_publish(struct net *net, struct tipc_uaddr *ua,
 /**
  * tipc_nametbl_withdraw - withdraw a service binding
  * @net: network namespace
- * @type: service type
- * @lower: service range lower bound
- * @upper: service range upper bound
+ * @ua: service address/range being unbound
+ * @sk: address of socket bveing unbound from
[Tung]: typo. bveing /being
  * @key: target publication key
  */
-int tipc_nametbl_withdraw(struct net *net, u32 type, u32 lower,
-			  u32 upper, u32 key)
+void tipc_nametbl_withdraw(struct net *net, struct tipc_uaddr *ua,
+			   struct tipc_socket_addr *sk, u32 key)
 {
 	struct name_table *nt = tipc_name_table(net);
 	struct tipc_net *tn = tipc_net(net);
-	u32 self = tipc_own_addr(net);
 	struct sk_buff *skb = NULL;
 	struct publication *p;
 	u32 rc_dests;
 
 	spin_lock_bh(&tn->nametbl_lock);
 
-	p = tipc_nametbl_remove_publ(net, type, lower, upper, self, key);
+	p = tipc_nametbl_remove_publ(net, ua, sk, key);
 	if (p) {
 		nt->local_publ_count--;
 		skb = tipc_named_withdraw(net, p);
@@ -796,16 +800,13 @@ int tipc_nametbl_withdraw(struct net *net, u32 type, u32 lower,
 		kfree_rcu(p, rcu);
 	} else {
 		pr_err("Failed to remove local publication {%u,%u,%u}/%u\n",
-		       type, lower, upper, key);
+		       ua->sr.type, ua->sr.lower, ua->sr.upper, key);
 	}
 	rc_dests = nt->rc_dests;
 	spin_unlock_bh(&tn->nametbl_lock);
 
-	if (skb) {
+	if (skb)
 		tipc_node_broadcast(net, skb, rc_dests);
-		return 1;
-	}
-	return 0;
 }
 
 /**
@@ -899,11 +900,14 @@ static void tipc_service_delete(struct net *net, struct tipc_service *sc)
 {
 	struct service_range *sr, *tmpr;
 	struct publication *p, *tmp;
+	struct tipc_uaddr ua;
 
 	spin_lock_bh(&sc->lock);
 	rbtree_postorder_for_each_entry_safe(sr, tmpr, &sc->ranges, tree_node) {
 		list_for_each_entry_safe(p, tmp, &sr->all_publ, all_publ) {
-			tipc_service_remove_publ(sr, p->sk.node, p->key);
+			tipc_uaddr(&ua, p->addrtype, p->scope,
+				   p->sr.type, p->sr.lower, p->sr.upper);
+			tipc_service_remove_publ(sr, &ua, &p->sk, p->key);
 			kfree_rcu(p, rcu);
 		}
 		rb_erase_augmented(&sr->tree_node, &sc->ranges, &sr_callbacks);
diff --git a/net/tipc/name_table.h b/net/tipc/name_table.h
index e12b9eb2c7f1..5f48f05b93be 100644
--- a/net/tipc/name_table.h
+++ b/net/tipc/name_table.h
@@ -124,15 +124,16 @@ bool tipc_nametbl_lookup(struct net *net, u32 type, u32 instance, u32 domain,
 			 bool all);
 struct publication *tipc_nametbl_publish(struct net *net, struct tipc_uaddr *ua,
 					 struct tipc_socket_addr *sk, u32 key);
-int tipc_nametbl_withdraw(struct net *net, u32 type, u32 lower, u32 upper,
-			  u32 key);
+void tipc_nametbl_withdraw(struct net *net, struct tipc_uaddr *ua,
+			   struct tipc_socket_addr *sk, u32 key);
 struct publication *tipc_nametbl_insert_publ(struct net *net,
 					     struct tipc_uaddr *ua,
 					     struct tipc_socket_addr *sk,
 					     u32 key);
-struct publication *tipc_nametbl_remove_publ(struct net *net, u32 type,
-					     u32 lower, u32 upper,
-					     u32 node, u32 key);
+struct publication *tipc_nametbl_remove_publ(struct net *net,
+					     struct tipc_uaddr *ua,
+					     struct tipc_socket_addr *sk,
+					     u32 key);
 bool tipc_nametbl_subscribe(struct tipc_subscription *s);
 void tipc_nametbl_unsubscribe(struct tipc_subscription *s);
 int tipc_nametbl_init(struct net *net);
diff --git a/net/tipc/node.c b/net/tipc/node.c
index 3a71e26c9509..24f3f5ea968d 100644
--- a/net/tipc/node.c
+++ b/net/tipc/node.c
@@ -429,8 +429,7 @@ static void tipc_node_write_unlock(struct tipc_node *n)
 	}
 	if (flags & TIPC_NOTIFY_LINK_DOWN) {
 		tipc_mon_peer_down(net, n->addr, bearer_id);
-		tipc_nametbl_withdraw(net, TIPC_LINK_STATE, n->addr,
-				      n->addr, n->link_id);
+		tipc_nametbl_withdraw(net, &ua, &sk, n->link_id);
 	}
 }
 
diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 0a92ebdd096d..5a017a34fb5f 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -152,8 +152,7 @@ static int tipc_accept(struct socket *sock, struct socket *new_sock, int flags,
 		       bool kern);
 static void tipc_sk_timeout(struct timer_list *t);
 static int tipc_sk_publish(struct tipc_sock *tsk, struct tipc_uaddr *ua);
-static int tipc_sk_withdraw(struct tipc_sock *tsk, uint scope,
-			    struct tipc_service_range const *seq);
+static int tipc_sk_withdraw(struct tipc_sock *tsk, struct tipc_uaddr *ua);
 static int tipc_sk_leave(struct tipc_sock *tsk);
 static struct tipc_sock *tipc_sk_lookup(struct net *net, u32 portid);
 static int tipc_sk_insert(struct tipc_sock *tsk);
@@ -643,7 +642,7 @@ static int tipc_release(struct socket *sock)
 	__tipc_shutdown(sock, TIPC_ERR_NO_PORT);
 	sk->sk_shutdown = SHUTDOWN_MASK;
 	tipc_sk_leave(tsk);
-	tipc_sk_withdraw(tsk, 0, NULL);
+	tipc_sk_withdraw(tsk, NULL);
 	__skb_queue_purge(&tsk->mc_method.deferredq);
 	sk_stop_timer(sk, &sk->sk_timer);
 	tipc_sk_remove(tsk);
@@ -681,7 +680,7 @@ static int __tipc_bind(struct socket *sock, struct sockaddr *skaddr, int alen)
 	bool unbind = false;
 
 	if (unlikely(!alen))
-		return tipc_sk_withdraw(tsk, 0, NULL);
+		return tipc_sk_withdraw(tsk, NULL);
 
 	if (ua->addrtype == TIPC_SERVICE_ADDR) {
 		ua->addrtype = TIPC_SERVICE_RANGE;
@@ -699,7 +698,7 @@ static int __tipc_bind(struct socket *sock, struct sockaddr *skaddr, int alen)
 		return -EACCES;
 
 	if (unbind)
-		return tipc_sk_withdraw(tsk, ua->scope, &ua->sr);
+		return tipc_sk_withdraw(tsk, ua);
 	return tipc_sk_publish(tsk, ua);
 }
 
@@ -2923,38 +2922,37 @@ static int tipc_sk_publish(struct tipc_sock *tsk, struct tipc_uaddr *ua)
 	return 0;
 }
 
-static int tipc_sk_withdraw(struct tipc_sock *tsk, uint scope,
-			    struct tipc_service_range const *seq)
+static int tipc_sk_withdraw(struct tipc_sock *tsk, struct tipc_uaddr *ua)
 {
 	struct net *net = sock_net(&tsk->sk);
-	struct publication *publ;
-	struct publication *safe;
+	struct publication *safe, *p;
+	struct tipc_uaddr _ua;
 	int rc = -EINVAL;
 
-	if (scope != TIPC_NODE_SCOPE)
-		scope = TIPC_CLUSTER_SCOPE;
-
-	list_for_each_entry_safe(publ, safe, &tsk->publications, binding_sock) {
-		if (seq) {
-			if (publ->scope != scope)
-				continue;
-			if (publ->sr.type != seq->type)
-				continue;
-			if (publ->sr.lower != seq->lower)
-				continue;
-			if (publ->sr.upper != seq->upper)
-				break;
-			tipc_nametbl_withdraw(net, publ->sr.type, publ->sr.lower,
-					      publ->sr.upper, publ->key);
-			rc = 0;
-			break;
+	list_for_each_entry_safe(p, safe, &tsk->publications, binding_sock) {
+		if (!ua) {
+			tipc_uaddr(&_ua, p->addrtype, p->scope,
+				   p->sr.type, p->sr.lower, p->sr.upper);
+			tipc_nametbl_withdraw(net, &_ua, &p->sk, p->key);
+			continue;
 		}
-		tipc_nametbl_withdraw(net, publ->sr.type, publ->sr.lower,
-				      publ->sr.upper, publ->key);
+		/* Unbind specific publication */
+		if (p->scope != ua->scope)
+			continue;
+		if (p->sr.type != ua->sr.type)
+			continue;
+		if (p->sr.lower != ua->sr.lower)
+			continue;
+		if (p->sr.upper != ua->sr.upper)
+			break;
+		tipc_nametbl_withdraw(net, ua, &p->sk, p->key);
 		rc = 0;
+		break;
 	}
-	if (list_empty(&tsk->publications))
+	if (list_empty(&tsk->publications)) {
 		tsk->published = 0;
+		rc = 0;
+	}
 	return rc;
 }
 
@@ -3107,15 +3105,17 @@ static int tipc_sk_leave(struct tipc_sock *tsk)
 {
 	struct net *net = sock_net(&tsk->sk);
 	struct tipc_group *grp = tsk->group;
-	struct tipc_service_range seq;
+	struct tipc_uaddr ua;
 	int scope;
 
 	if (!grp)
 		return -EINVAL;
-	tipc_group_self(grp, &seq, &scope);
+	ua.addrtype = TIPC_SERVICE_RANGE;
+	tipc_group_self(grp, &ua.sr, &scope);
+	ua.scope = scope;
 	tipc_group_delete(net, grp);
 	tsk->group = NULL;
-	tipc_sk_withdraw(tsk, scope, &seq);
+	tipc_sk_withdraw(tsk, &ua);
 	return 0;
 }
 
-- 
2.28.0

Re: [tipc-discussion] [net-next 05/16] tipc: simplify call signatures for publication creation

[Tung Q. N. <tun...@de...>]

Hi Jon,

A minor comment for this patch (Marked with [Tung]).

Best regards,
Tung Nguyen

-----Original Message-----
From: jm...@re... <jm...@re...> 
Sent: Wednesday, December 9, 2020 1:50 AM
To: tip...@li...
Cc: Tung Quang Nguyen <tun...@de...>; Hoang Huu Le <hoa...@de...>; Tuong Tong Lien <tuo...@de...>; jm...@re...; ma...@do...; xi...@re...; yin...@wi...; par...@gm...
Subject: [net-next 05/16] tipc: simplify call signatures for publication creation

From: Jon Maloy <jm...@re...>

We simplify the call signatures for tipc_nametbl_insert_publ() and
tipc_publ_create() so that fewer parameters are passed around.

Signed-off-by: Jon Maloy <jm...@re...>
---
 net/tipc/name_distr.c | 23 ++++++++--------
 net/tipc/name_table.c | 61 +++++++++++++++++++------------------------
 net/tipc/name_table.h | 10 ++++---
 net/tipc/socket.c     |  8 ++----
 4 files changed, 47 insertions(+), 55 deletions(-)

diff --git a/net/tipc/name_distr.c b/net/tipc/name_distr.c
index 721d2fca3d6f..df42fc2b4536 100644
--- a/net/tipc/name_distr.c
+++ b/net/tipc/name_distr.c
@@ -293,30 +293,31 @@ static bool tipc_update_nametbl(struct net *net, struct distr_item *i,
 				u32 node, u32 dtype)
 {
 	struct publication *p = NULL;
-	u32 lower = ntohl(i->lower);
-	u32 upper = ntohl(i->upper);
-	u32 type = ntohl(i->type);
-	u32 port = ntohl(i->port);
+	struct tipc_socket_addr sk;
+	struct tipc_uaddr ua;
 	u32 key = ntohl(i->key);
 
+	tipc_uaddr(&ua, TIPC_SERVICE_RANGE, TIPC_CLUSTER_SCOPE,
+		   ntohl(i->type), ntohl(i->lower), ntohl(i->upper));
+	sk.ref = ntohl(i->port);
+	sk.node = node;
+
 	if (dtype == PUBLICATION) {
-		p = tipc_nametbl_insert_publ(net, type, lower, upper,
-					     TIPC_CLUSTER_SCOPE, node,
-					     port, key);
+		p = tipc_nametbl_insert_publ(net, &ua, &sk, key);
 		if (p) {
 			tipc_node_subscribe(net, &p->binding_node, node);
 			return true;
 		}
 	} else if (dtype == WITHDRAWAL) {
-		p = tipc_nametbl_remove_publ(net, type, lower,
-					     upper, node, key);
+		p = tipc_nametbl_remove_publ(net, ua.sr.type, ua.sr.lower,
+					     ua.sr.upper, node, key);
 		if (p) {
 			tipc_node_unsubscribe(net, &p->binding_node, node);
 			kfree_rcu(p, rcu);
 			return true;
 		}
-		pr_warn_ratelimited("Failed to remove binding %u,%u from %x\n",
-				    type, lower, node);
+		pr_warn_ratelimited("Failed to remove binding %u,%u from %u\n",
+				    ua.sr.type, ua.sr.lower, node);
 	} else {
 		pr_warn("Unrecognized name table message received\n");
 	}
diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c
index d951e9345122..ba96d5fc57f3 100644
--- a/net/tipc/name_table.c
+++ b/net/tipc/name_table.c
@@ -222,36 +222,30 @@ static int hash(int x)
 
 /**
  * tipc_publ_create - create a publication structure
- * @type: name sequence type
- * @lower: name sequence lower bound
- * @upper: name sequence upper bound
- * @scope: publication scope
- * @node: network address of publishing socket
- * @port: publishing port
+ * @ua: the service range the user is binding to
+ * @sk: the address of the socket thatis bound
[Tung]: typo. thatis/ that is
  * @key: publication key
  */
-static struct publication *tipc_publ_create(u32 type, u32 lower, u32 upper,
-					    u32 scope, u32 node, u32 port,
+static struct publication *tipc_publ_create(struct tipc_uaddr *ua,
+					    struct tipc_socket_addr *sk,
 					    u32 key)
 {
-	struct publication *publ = kzalloc(sizeof(*publ), GFP_ATOMIC);
+	struct publication *p = kzalloc(sizeof(*p), GFP_ATOMIC);
 
-	if (!publ)
+	if (!p)
 		return NULL;
 
-	publ->sr.type = type;
-	publ->sr.lower = lower;
-	publ->sr.upper = upper;
-	publ->scope = scope;
-	publ->sk.node = node;
-	publ->sk.ref = port;
-	publ->key = key;
-	INIT_LIST_HEAD(&publ->binding_sock);
-	INIT_LIST_HEAD(&publ->binding_node);
-	INIT_LIST_HEAD(&publ->local_publ);
-	INIT_LIST_HEAD(&publ->all_publ);
-	INIT_LIST_HEAD(&publ->list);
-	return publ;
+	p->sr = ua->sr;
+	p->sk = *sk;
+	p->addrtype = ua->addrtype;
+	p->scope = ua->scope;
+	p->key = key;
+	INIT_LIST_HEAD(&p->binding_sock);
+	INIT_LIST_HEAD(&p->binding_node);
+	INIT_LIST_HEAD(&p->local_publ);
+	INIT_LIST_HEAD(&p->all_publ);
+	INIT_LIST_HEAD(&p->list);
+	return p;
 }
 
 /**
@@ -468,23 +462,24 @@ static struct tipc_service *tipc_service_find(struct net *net, u32 type)
 	return NULL;
 };
 
-struct publication *tipc_nametbl_insert_publ(struct net *net, u32 type,
-					     u32 lower, u32 upper,
-					     u32 scope, u32 node,
-					     u32 port, u32 key)
+struct publication *tipc_nametbl_insert_publ(struct net *net,
+					     struct tipc_uaddr *ua,
+					     struct tipc_socket_addr *sk,
+					     u32 key)
 {
 	struct name_table *nt = tipc_name_table(net);
 	struct tipc_service *sc;
 	struct publication *p;
+	u32 type = ua->sr.type;
 	bool res = false;
 
-	p = tipc_publ_create(type, lower, upper, scope, node, port, key);
+	p = tipc_publ_create(ua, sk, key);
 	if (!p)
 		return NULL;
 
-	if (scope > TIPC_NODE_SCOPE || lower > upper) {
-		pr_debug("Failed to bind illegal {%u,%u,%u} with scope %u\n",
-			 type, lower, upper, scope);
+	if (ua->sr.lower > ua->sr.upper) {
+		pr_debug("Failed to bind illegal {%u,%u,%u} from node %u\n",
+			 type, ua->sr.lower, ua->sr.upper, sk->node);
 		return NULL;
 	}
 	sc = tipc_service_find(net, type);
@@ -758,9 +753,7 @@ struct publication *tipc_nametbl_publish(struct net *net, struct tipc_uaddr *ua,
 		goto exit;
 	}
 
-	p = tipc_nametbl_insert_publ(net, ua->sr.type, ua->sr.lower,
-				     ua->sr.upper, ua->scope,
-				     sk->node, sk->ref, key);
+	p = tipc_nametbl_insert_publ(net, ua, sk, key);
 	if (p) {
 		nt->local_publ_count++;
 		skb = tipc_named_publish(net, p);
diff --git a/net/tipc/name_table.h b/net/tipc/name_table.h
index 5e969e060509..e12b9eb2c7f1 100644
--- a/net/tipc/name_table.h
+++ b/net/tipc/name_table.h
@@ -75,7 +75,8 @@ struct tipc_uaddr;
 struct publication {
 	struct tipc_service_range sr;
 	struct tipc_socket_addr sk;
-	u32 scope;
+	u16 addrtype;
+	u16 scope;
 	u32 key;
 	u32 id;
 	struct list_head binding_node;
@@ -125,9 +126,10 @@ struct publication *tipc_nametbl_publish(struct net *net, struct tipc_uaddr *ua,
 					 struct tipc_socket_addr *sk, u32 key);
 int tipc_nametbl_withdraw(struct net *net, u32 type, u32 lower, u32 upper,
 			  u32 key);
-struct publication *tipc_nametbl_insert_publ(struct net *net, u32 type,
-					     u32 lower, u32 upper, u32 scope,
-					     u32 node, u32 ref, u32 key);
+struct publication *tipc_nametbl_insert_publ(struct net *net,
+					     struct tipc_uaddr *ua,
+					     struct tipc_socket_addr *sk,
+					     u32 key);
 struct publication *tipc_nametbl_remove_publ(struct net *net, u32 type,
 					     u32 lower, u32 upper,
 					     u32 node, u32 key);
diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 598c8611b75f..0a92ebdd096d 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -3088,12 +3088,8 @@ static int tipc_sk_join(struct tipc_sock *tsk, struct tipc_group_req *mreq)
 	msg_set_nametype(hdr, mreq->type);
 	msg_set_dest_droppable(hdr, true);
 	tipc_nametbl_build_group(net, grp, mreq->type, mreq->scope);
-
-	ua.sr.type = mreq->type;
-	ua.sr.lower = mreq->instance;
-	ua.sr.upper = ua.sr.lower;
-	ua.scope = mreq->scope;
-	ua.addrtype = TIPC_SERVICE_RANGE;
+	tipc_uaddr(&ua, TIPC_SERVICE_RANGE, mreq->scope,
+		   mreq->type, mreq->instance, mreq->instance);
 	rc = tipc_sk_publish(tsk, &ua);
 	if (rc) {
 		tipc_group_delete(net, grp);
-- 
2.28.0

Re: [tipc-discussion] [net-next 01/16] tipc: re-organize members of struct publication

[Jon M. <jm...@re...>]

On 1/27/21 2:57 AM, Tung Quang Nguyen wrote:
> Hi Jon,
>
> Some minor comments for this patch (marked with "[Tung]").
>
> Best regards,
> Tung Nguyen
>
> -----Original Message-----
> From: jm...@re... <jm...@re...>
> Sent: Wednesday, December 9, 2020 1:50 AM
> To: tip...@li...
> Cc: Tung Quang Nguyen <tun...@de...>; Hoang Huu Le <hoa...@de...>; Tuong Tong Lien <tuo...@de...>; jm...@re...; ma...@do...; xi...@re...; yin...@wi...; par...@gm...
> Subject: [net-next 01/16] tipc: re-organize members of struct publication
>
> From: Jon Maloy <jm...@re...>
>
> In a future commit we will introduce more members to struct publication.
> In order to keep this structure comprehensible we now group some of
> its current fields into the sub-structures where they really belong,
> - A struct tipc_service_range for the functional address the publication
>    is representing.
> - A struct tipc_socket_addr for the socket bound to that service range.
>
> We also rename the stack variable 'publ' to just 'p' in a couple of places.
> This is just as easy to understand in the given context, and keeps the
> number of wrapped code lines to a minimum.
>
> There are no functional changes in this commit.
>
> Signed-off-by: Jon Maloy <jm...@re...>
> ---
>   net/tipc/name_distr.c | 62 +++++++++++++++++++++----------------------
>   net/tipc/name_table.c | 44 +++++++++++++++---------------
>   net/tipc/name_table.h | 17 +++++-------
>   net/tipc/socket.c     | 32 +++++++++++-----------
>   4 files changed, 75 insertions(+), 80 deletions(-)
>
> diff --git a/net/tipc/name_distr.c b/net/tipc/name_distr.c
> index 6cf57c3bfa27..721d2fca3d6f 100644
> --- a/net/tipc/name_distr.c
> +++ b/net/tipc/name_distr.c
> @@ -3,6 +3,7 @@
>    *
>    * Copyright (c) 2000-2006, 2014, Ericsson AB
>    * Copyright (c) 2005, 2010-2011, Wind River Systems
> + * Copyright (c) 2020, Red Hat Inc
>    * All rights reserved.
>    *
>    * Redistribution and use in source and binary forms, with or without
> @@ -55,10 +56,10 @@ struct distr_queue_item {
>    */
>   static void publ_to_item(struct distr_item *i, struct publication *p)
>   {
> -	i->type = htonl(p->type);
> -	i->lower = htonl(p->lower);
> -	i->upper = htonl(p->upper);
> -	i->port = htonl(p->port);
> +	i->type = htonl(p->sr.type);
> +	i->lower = htonl(p->sr.lower);
> +	i->upper = htonl(p->sr.upper);
> +	i->port = htonl(p->sk.ref);
>   	i->key = htonl(p->key);
>   }
>   
> @@ -90,20 +91,20 @@ static struct sk_buff *named_prepare_buf(struct net *net, u32 type, u32 size,
>   /**
>    * tipc_named_publish - tell other nodes about a new publication by this node
>    * @net: the associated network namespace
> - * @publ: the new publication
> + * @p: the new publication
>    */
> -struct sk_buff *tipc_named_publish(struct net *net, struct publication *publ)
> +struct sk_buff *tipc_named_publish(struct net *net, struct publication *p)
>   {
>   	struct name_table *nt = tipc_name_table(net);
>   	struct distr_item *item;
>   	struct sk_buff *skb;
>   
> -	if (publ->scope == TIPC_NODE_SCOPE) {
> -		list_add_tail_rcu(&publ->binding_node, &nt->node_scope);
> +	if (p->scope == TIPC_NODE_SCOPE) {
> +		list_add_tail_rcu(&p->binding_node, &nt->node_scope);
>   		return NULL;
>   	}
>   	write_lock_bh(&nt->cluster_scope_lock);
> -	list_add_tail(&publ->binding_node, &nt->cluster_scope);
> +	list_add_tail(&p->binding_node, &nt->cluster_scope);
>   	write_unlock_bh(&nt->cluster_scope_lock);
>   	skb = named_prepare_buf(net, PUBLICATION, ITEM_SIZE, 0);
>   	if (!skb) {
> @@ -113,25 +114,25 @@ struct sk_buff *tipc_named_publish(struct net *net, struct publication *publ)
>   	msg_set_named_seqno(buf_msg(skb), nt->snd_nxt++);
>   	msg_set_non_legacy(buf_msg(skb));
>   	item = (struct distr_item *)msg_data(buf_msg(skb));
> -	publ_to_item(item, publ);
> +	publ_to_item(item, p);
>   	return skb;
>   }
>   
>   /**
>    * tipc_named_withdraw - tell other nodes about a withdrawn publication by this node
>    * @net: the associated network namespace
> - * @publ: the withdrawn publication
> + * @p: the withdrawn publication
>    */
> -struct sk_buff *tipc_named_withdraw(struct net *net, struct publication *publ)
> +struct sk_buff *tipc_named_withdraw(struct net *net, struct publication *p)
>   {
>   	struct name_table *nt = tipc_name_table(net);
>   	struct distr_item *item;
>   	struct sk_buff *skb;
>   
>   	write_lock_bh(&nt->cluster_scope_lock);
> -	list_del(&publ->binding_node);
> +	list_del(&p->binding_node);
>   	write_unlock_bh(&nt->cluster_scope_lock);
> -	if (publ->scope == TIPC_NODE_SCOPE)
> +	if (p->scope == TIPC_NODE_SCOPE)
>   		return NULL;
>   
>   	skb = named_prepare_buf(net, WITHDRAWAL, ITEM_SIZE, 0);
> @@ -142,7 +143,7 @@ struct sk_buff *tipc_named_withdraw(struct net *net, struct publication *publ)
>   	msg_set_named_seqno(buf_msg(skb), nt->snd_nxt++);
>   	msg_set_non_legacy(buf_msg(skb));
>   	item = (struct distr_item *)msg_data(buf_msg(skb));
> -	publ_to_item(item, publ);
> +	publ_to_item(item, p);
>   	return skb;
>   }
>   
> @@ -239,27 +240,26 @@ void tipc_named_node_up(struct net *net, u32 dnode, u16 capabilities)
>    * Invoked for each publication issued by a newly failed node.
>    * Removes publication structure from name table & deletes it.
>    */
> -static void tipc_publ_purge(struct net *net, struct publication *publ, u32 addr)
> +static void tipc_publ_purge(struct net *net, struct publication *p, u32 addr)
>   {
>   	struct tipc_net *tn = tipc_net(net);
> -	struct publication *p;
> +	struct publication *_p;
>   
>   	spin_lock_bh(&tn->nametbl_lock);
> -	p = tipc_nametbl_remove_publ(net, publ->type, publ->lower, publ->upper,
> -				     publ->node, publ->key);
> -	if (p)
> -		tipc_node_unsubscribe(net, &p->binding_node, addr);
> +	_p = tipc_nametbl_remove_publ(net, p->sr.type, p->sr.lower,
> +				      p->sr.upper, p->sk.node, p->key);
> +	if (_p)
> +		tipc_node_unsubscribe(net, &_p->binding_node, addr);
>   	spin_unlock_bh(&tn->nametbl_lock);
>   
> -	if (p != publ) {
> +	if (_p != p) {
>   		pr_err("Unable to remove publication from failed node\n"
>   		       " (type=%u, lower=%u, node=0x%x, port=%u, key=%u)\n",
> -		       publ->type, publ->lower, publ->node, publ->port,
> -		       publ->key);
> +		       p->sr.type, p->sr.lower, p->sk.node, p->sk.ref, p->key);
>   	}
>   
> -	if (p)
> -		kfree_rcu(p, rcu);
> +	if (_p)
> +		kfree_rcu(_p, rcu);
>   }
>   
>   void tipc_publ_notify(struct net *net, struct list_head *nsub_list,
> @@ -410,15 +410,15 @@ void tipc_named_reinit(struct net *net)
>   {
>   	struct name_table *nt = tipc_name_table(net);
>   	struct tipc_net *tn = tipc_net(net);
> -	struct publication *publ;
> +	struct publication *p;
>   	u32 self = tipc_own_addr(net);
>   
>   	spin_lock_bh(&tn->nametbl_lock);
>   
> -	list_for_each_entry_rcu(publ, &nt->node_scope, binding_node)
> -		publ->node = self;
> -	list_for_each_entry_rcu(publ, &nt->cluster_scope, binding_node)
> -		publ->node = self;
> +	list_for_each_entry_rcu(p, &nt->node_scope, binding_node)
> +		p->sk.node = self;
> +	list_for_each_entry_rcu(p, &nt->cluster_scope, binding_node)
> +		p->sk.node = self;
>   	nt->rc_dests = 0;
>   	spin_unlock_bh(&tn->nametbl_lock);
>   }
> diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c
> index ee5ac40ea2b6..c37a4a9c87ca 100644
> --- a/net/tipc/name_table.c
> +++ b/net/tipc/name_table.c
> @@ -239,12 +239,12 @@ static struct publication *tipc_publ_create(u32 type, u32 lower, u32 upper,
>   	if (!publ)
>   		return NULL;
>   
> -	publ->type = type;
> -	publ->lower = lower;
> -	publ->upper = upper;
> +	publ->sr.type = type;
> +	publ->sr.lower = lower;
> +	publ->sr.upper = upper;
>   	publ->scope = scope;
> -	publ->node = node;
> -	publ->port = port;
> +	publ->sk.node = node;
> +	publ->sk.ref = port;
>   	publ->key = key;
>   	INIT_LIST_HEAD(&publ->binding_sock);
>   	INIT_LIST_HEAD(&publ->binding_node);
> [Tung]: Should 'publ' be changed to 'p' ?
Yes, maybe.
I renamed this stack variable in the functions where there were many 
changes anyway, and kept the name where there were few changes.
This way, the patch doesn't get bloated with unmotivated changes.
But I'll take a new look, both at this one and the next one you mention.

///jon

> @@ -347,7 +347,7 @@ static struct publication *tipc_service_insert_publ(struct net *net,
>   
>   	/* Return if the publication already exists */
>   	list_for_each_entry(p, &sr->all_publ, all_publ) {
> -		if (p->key == key && (!p->node || p->node == node))
> +		if (p->key == key && (!p->sk.node || p->sk.node == node))
>   			return NULL;
>   	}
>   
> @@ -363,8 +363,8 @@ static struct publication *tipc_service_insert_publ(struct net *net,
>   
>   	/* Any subscriptions waiting for notification?  */
>   	list_for_each_entry_safe(sub, tmp, &sc->subscriptions, service_list) {
> -		tipc_sub_report_overlap(sub, p->lower, p->upper, TIPC_PUBLISHED,
> -					p->port, p->node, p->scope, first);
> +		tipc_sub_report_overlap(sub, p->sr.lower, p->sr.upper, TIPC_PUBLISHED,
> +					p->sk.ref, p->sk.node, p->scope, first);
>   	}
>   	return p;
>   err:
> @@ -384,7 +384,7 @@ static struct publication *tipc_service_remove_publ(struct service_range *sr,
>   	struct publication *p;
>   
>   	list_for_each_entry(p, &sr->all_publ, all_publ) {
> -		if (p->key != key || (node && node != p->node))
> +		if (p->key != key || (node && node != p->sk.node))
>   			continue;
>   		list_del(&p->all_publ);
>   		list_del(&p->local_publ);
> @@ -452,8 +452,8 @@ static void tipc_service_subscribe(struct tipc_service *service,
>   	/* Sort the publications before reporting */
>   	list_sort(NULL, &publ_list, tipc_publ_sort);
>   	list_for_each_entry_safe(p, tmp, &publ_list, list) {
> -		tipc_sub_report_overlap(sub, p->lower, p->upper,
> -					TIPC_PUBLISHED, p->port, p->node,
> +		tipc_sub_report_overlap(sub, p->sr.lower, p->sr.upper,
> +					TIPC_PUBLISHED, p->sk.ref, p->sk.node,
>   					p->scope, true);
>   		list_del_init(&p->list);
>   	}
> @@ -525,7 +525,7 @@ struct publication *tipc_nametbl_remove_publ(struct net *net, u32 type,
>   	last = list_empty(&sr->all_publ);
>   	list_for_each_entry_safe(sub, tmp, &sc->subscriptions, service_list) {
>   		tipc_sub_report_overlap(sub, lower, upper, TIPC_WITHDRAWN,
> -					p->port, node, p->scope, last);
> +					p->sk.ref, node, p->scope, last);
>   	}
>   
>   	/* Remove service range item if this was its last publication */
> @@ -603,8 +603,8 @@ u32 tipc_nametbl_translate(struct net *net, u32 type, u32 instance, u32 *dnode)
>   					     all_publ);
>   			list_move_tail(&p->all_publ, &sr->all_publ);
>   		}
> -		port = p->port;
> -		node = p->node;
> +		port = p->sk.ref;
> +		node = p->sk.node;
>   		/* Todo: as for legacy, pick the first matching range only, a
>   		 * "true" round-robin will be performed as needed.
>   		 */
> @@ -643,9 +643,9 @@ bool tipc_nametbl_lookup(struct net *net, u32 type, u32 instance, u32 scope,
>   	list_for_each_entry(p, &sr->all_publ, all_publ) {
>   		if (p->scope != scope)
>   			continue;
> -		if (p->port == exclude && p->node == self)
> +		if (p->sk.ref == exclude && p->sk.node == self)
>   			continue;
> -		tipc_dest_push(dsts, p->node, p->port);
> +		tipc_dest_push(dsts, p->sk.node, p->sk.ref);
>   		(*dstcnt)++;
>   		if (all)
>   			continue;
> @@ -675,7 +675,7 @@ void tipc_nametbl_mc_lookup(struct net *net, u32 type, u32 lower, u32 upper,
>   	service_range_foreach_match(sr, sc, lower, upper) {
>   		list_for_each_entry(p, &sr->local_publ, local_publ) {
>   			if (p->scope == scope || (!exact && p->scope < scope))
> -				tipc_dest_push(dports, 0, p->port);
> +				tipc_dest_push(dports, 0, p->sk.ref);
>   		}
>   	}
>   	spin_unlock_bh(&sc->lock);
> @@ -702,7 +702,7 @@ void tipc_nametbl_lookup_dst_nodes(struct net *net, u32 type, u32 lower,
>   	spin_lock_bh(&sc->lock);
>   	service_range_foreach_match(sr, sc, lower, upper) {
>   		list_for_each_entry(p, &sr->all_publ, all_publ) {
> -			tipc_nlist_add(nodes, p->node);
> +			tipc_nlist_add(nodes, p->sk.node);
>   		}
>   	}
>   	spin_unlock_bh(&sc->lock);
> @@ -731,7 +731,7 @@ void tipc_nametbl_build_group(struct net *net, struct tipc_group *grp,
>   		list_for_each_entry(p, &sr->all_publ, all_publ) {
>   			if (p->scope != scope)
>   				continue;
> -			tipc_group_add_member(grp, p->node, p->port, p->lower);
> +			tipc_group_add_member(grp, p->sk.node, p->sk.ref, p->sr.lower);
>   		}
>   	}
>   	spin_unlock_bh(&sc->lock);
> @@ -909,7 +909,7 @@ static void tipc_service_delete(struct net *net, struct tipc_service *sc)
>   	spin_lock_bh(&sc->lock);
>   	rbtree_postorder_for_each_entry_safe(sr, tmpr, &sc->ranges, tree_node) {
>   		list_for_each_entry_safe(p, tmp, &sr->all_publ, all_publ) {
> -			tipc_service_remove_publ(sr, p->node, p->key);
> +			tipc_service_remove_publ(sr, p->sk.node, p->key);
>   			kfree_rcu(p, rcu);
>   		}
>   		rb_erase_augmented(&sr->tree_node, &sc->ranges, &sr_callbacks);
> @@ -993,9 +993,9 @@ static int __tipc_nl_add_nametable_publ(struct tipc_nl_msg *msg,
>   			goto publ_msg_full;
>   		if (nla_put_u32(msg->skb, TIPC_NLA_PUBL_SCOPE, p->scope))
>   			goto publ_msg_full;
> -		if (nla_put_u32(msg->skb, TIPC_NLA_PUBL_NODE, p->node))
> +		if (nla_put_u32(msg->skb, TIPC_NLA_PUBL_NODE, p->sk.node))
>   			goto publ_msg_full;
> -		if (nla_put_u32(msg->skb, TIPC_NLA_PUBL_REF, p->port))
> +		if (nla_put_u32(msg->skb, TIPC_NLA_PUBL_REF, p->sk.ref))
>   			goto publ_msg_full;
>   		if (nla_put_u32(msg->skb, TIPC_NLA_PUBL_KEY, p->key))
>   			goto publ_msg_full;
> diff --git a/net/tipc/name_table.h b/net/tipc/name_table.h
> index 5a82a01369d6..3fff00440e1a 100644
> --- a/net/tipc/name_table.h
> +++ b/net/tipc/name_table.h
> @@ -3,6 +3,7 @@
>    *
>    * Copyright (c) 2000-2006, 2014-2018, Ericsson AB
>    * Copyright (c) 2004-2005, 2010-2011, Wind River Systems
> + * Copyright (c) 2020, Red Hat Inc
>    * All rights reserved.
>    *
>    * Redistribution and use in source and binary forms, with or without
> @@ -50,13 +51,10 @@ struct tipc_group;
>   #define TIPC_NAMETBL_SIZE	1024	/* must be a power of 2 */
>   
>   /**
> - * struct publication - info about a published (name or) name sequence
> - * @type: name sequence type
> - * @lower: name sequence lower bound
> - * @upper: name sequence upper bound
> + * struct publication - info about a published service address or range
> + * @sr: service range represented by this publication
> + * @sk: address of socket bound to this publication
>    * @scope: scope of publication, TIPC_NODE_SCOPE or TIPC_CLUSTER_SCOPE
> - * @node: network address of publishing socket's node
> - * @port: publishing port
>    * @key: publication key, unique across the cluster
>    * @id: publication id
>    * @binding_node: all publications from the same node which bound this one
> @@ -74,12 +72,9 @@ struct tipc_group;
>    * @rcu: RCU callback head used for deferred freeing
>    */
>   struct publication {
> -	u32 type;
> -	u32 lower;
> -	u32 upper;
> +	struct tipc_service_range sr;
> +	struct tipc_socket_addr sk;
>   	u32 scope;
> -	u32 node;
> -	u32 port;
>   	u32 key;
>   	u32 id;
>   	struct list_head binding_node;
> diff --git a/net/tipc/socket.c b/net/tipc/socket.c
> index cebcc104dc70..bff14df40bc9 100644
> --- a/net/tipc/socket.c
> +++ b/net/tipc/socket.c
> @@ -2934,19 +2934,19 @@ static int tipc_sk_withdraw(struct tipc_sock *tsk, uint scope,
>   		if (seq) {
>   			if (publ->scope != scope)
>   				continue;
> -			if (publ->type != seq->type)
> +			if (publ->sr.type != seq->type)
>   				continue;
> -			if (publ->lower != seq->lower)
> +			if (publ->sr.lower != seq->lower)
>   				continue;
> -			if (publ->upper != seq->upper)
> +			if (publ->sr.upper != seq->upper)
>   				break;
> -			tipc_nametbl_withdraw(net, publ->type, publ->lower,
> -					      publ->upper, publ->key);
> +			tipc_nametbl_withdraw(net, publ->sr.type, publ->sr.lower,
> +					      publ->sr.upper, publ->key);
>   			rc = 0;
>   			break;
>   		}
> -		tipc_nametbl_withdraw(net, publ->type, publ->lower,
> -				      publ->upper, publ->key);
> +		tipc_nametbl_withdraw(net, publ->sr.type, publ->sr.lower,
> +				      publ->sr.upper, publ->key);
>   		rc = 0;
>   	}
>   	if (list_empty(&tsk->publications))
> [Tung]: Should 'publ' be changed to 'p' ?
> @@ -3711,11 +3711,11 @@ static int __tipc_nl_add_sk_publ(struct sk_buff *skb,
>   
>   	if (nla_put_u32(skb, TIPC_NLA_PUBL_KEY, publ->key))
>   		goto attr_msg_cancel;
> -	if (nla_put_u32(skb, TIPC_NLA_PUBL_TYPE, publ->type))
> +	if (nla_put_u32(skb, TIPC_NLA_PUBL_TYPE, publ->sr.type))
>   		goto attr_msg_cancel;
> -	if (nla_put_u32(skb, TIPC_NLA_PUBL_LOWER, publ->lower))
> +	if (nla_put_u32(skb, TIPC_NLA_PUBL_LOWER, publ->sr.lower))
>   		goto attr_msg_cancel;
> -	if (nla_put_u32(skb, TIPC_NLA_PUBL_UPPER, publ->upper))
> +	if (nla_put_u32(skb, TIPC_NLA_PUBL_UPPER, publ->sr.upper))
>   		goto attr_msg_cancel;
>   
>   	nla_nest_end(skb, attrs);
> [Tung]: Should 'publ' be changed to 'p' ?
> @@ -3863,9 +3863,9 @@ bool tipc_sk_filtering(struct sock *sk)
>   		p = list_first_entry_or_null(&tsk->publications,
>   					     struct publication, binding_sock);
>   		if (p) {
> -			type = p->type;
> -			lower = p->lower;
> -			upper = p->upper;
> +			type = p->sr.type;
> +			lower = p->sr.lower;
> +			upper = p->sr.upper;
>   		}
>   	}
>   
> @@ -3964,9 +3964,9 @@ int tipc_sk_dump(struct sock *sk, u16 dqueues, char *buf)
>   	if (tsk->published) {
>   		p = list_first_entry_or_null(&tsk->publications,
>   					     struct publication, binding_sock);
> -		i += scnprintf(buf + i, sz - i, " %u", (p) ? p->type : 0);
> -		i += scnprintf(buf + i, sz - i, " %u", (p) ? p->lower : 0);
> -		i += scnprintf(buf + i, sz - i, " %u", (p) ? p->upper : 0);
> +		i += scnprintf(buf + i, sz - i, " %u", (p) ? p->sr.type : 0);
> +		i += scnprintf(buf + i, sz - i, " %u", (p) ? p->sr.lower : 0);
> +		i += scnprintf(buf + i, sz - i, " %u", (p) ? p->sr.upper : 0);
>   	}
>   	i += scnprintf(buf + i, sz - i, " | %u", tsk->snd_win);
>   	i += scnprintf(buf + i, sz - i, " %u", tsk->rcv_win);

Re: [tipc-discussion] [net-next 02/16] tipc: move creation of publication item one level up in call chain

[Tung Q. N. <tun...@de...>]

Hi Jon,

Just one minor comment for this patch (marked with [Tung]").

Best regards,
Tung Nguyen

-----Original Message-----
From: jm...@re... <jm...@re...> 
Sent: Wednesday, December 9, 2020 1:50 AM
To: tip...@li...
Cc: Tung Quang Nguyen <tun...@de...>; Hoang Huu Le <hoa...@de...>; Tuong Tong Lien <tuo...@de...>; jm...@re...; ma...@do...; xi...@re...; yin...@wi...; par...@gm...
Subject: [net-next 02/16] tipc: move creation of publication item one level up in call chain

From: Jon Maloy <jm...@re...>

We instantiante struct publication in tipc_nametbl_insert_publ()
[Tung]: typo. instantiante/Instantiate.
instead of as currently in tipc_service_insert_publ(). This has the
advantage that we can pass a pointer to the publication struct to
the next call levels, instead of the numerous individual parameters
we pass on now. It also gives us a location to keep the contents of
the additional fields we will introduce in a later commit.

Signed-off-by: Jon Maloy <jm...@re...>
---
 net/tipc/name_table.c | 63 ++++++++++++++++++++++---------------------
 1 file changed, 32 insertions(+), 31 deletions(-)

diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c
index c37a4a9c87ca..68e269b49780 100644
--- a/net/tipc/name_table.c
+++ b/net/tipc/name_table.c
@@ -327,49 +327,44 @@ static struct service_range *tipc_service_create_range(struct tipc_service *sc,
 	return sr;
 }
 
-static struct publication *tipc_service_insert_publ(struct net *net,
-						    struct tipc_service *sc,
-						    u32 type, u32 lower,
-						    u32 upper, u32 scope,
-						    u32 node, u32 port,
-						    u32 key)
+static bool tipc_service_insert_publ(struct net *net,
+				     struct tipc_service *sc,
+				     struct publication *p)
 {
 	struct tipc_subscription *sub, *tmp;
 	struct service_range *sr;
-	struct publication *p;
+	struct publication *_p;
+	u32 node = p->sk.node;
 	bool first = false;
 
-	sr = tipc_service_create_range(sc, lower, upper);
+	sr = tipc_service_create_range(sc, p->sr.lower, p->sr.upper);
 	if (!sr)
 		goto  err;
 
 	first = list_empty(&sr->all_publ);
 
 	/* Return if the publication already exists */
-	list_for_each_entry(p, &sr->all_publ, all_publ) {
-		if (p->key == key && (!p->sk.node || p->sk.node == node))
-			return NULL;
+	list_for_each_entry(_p, &sr->all_publ, all_publ) {
+		if (_p->key == p->key && (!_p->sk.node || _p->sk.node == node))
+			return false;
 	}
 
-	/* Create and insert publication */
-	p = tipc_publ_create(type, lower, upper, scope, node, port, key);
-	if (!p)
-		goto err;
-	/* Suppose there shouldn't be a huge gap btw publs i.e. >INT_MAX */
-	p->id = sc->publ_cnt++;
-	if (in_own_node(net, node))
+	if (in_own_node(net, p->sk.node))
 		list_add(&p->local_publ, &sr->local_publ);
 	list_add(&p->all_publ, &sr->all_publ);
+	p->id = sc->publ_cnt++;
 
 	/* Any subscriptions waiting for notification?  */
 	list_for_each_entry_safe(sub, tmp, &sc->subscriptions, service_list) {
-		tipc_sub_report_overlap(sub, p->sr.lower, p->sr.upper, TIPC_PUBLISHED,
-					p->sk.ref, p->sk.node, p->scope, first);
+		tipc_sub_report_overlap(sub, p->sr.lower, p->sr.upper,
+					TIPC_PUBLISHED, p->sk.ref, p->sk.node,
+					p->scope, first);
 	}
-	return p;
+	return true;
 err:
-	pr_warn("Failed to bind to %u,%u,%u, no memory\n", type, lower, upper);
-	return NULL;
+	pr_warn("Failed to bind to %u,%u,%u, no memory\n",
+		p->sr.type, p->sr.lower, p->sr.upper);
+	return false;
 }
 
 /**
@@ -481,6 +476,11 @@ struct publication *tipc_nametbl_insert_publ(struct net *net, u32 type,
 	struct name_table *nt = tipc_name_table(net);
 	struct tipc_service *sc;
 	struct publication *p;
+	bool res = false;
+
+	p = tipc_publ_create(type, lower, upper, scope, node, port, key);
+	if (!p)
+		return NULL;
 
 	if (scope > TIPC_NODE_SCOPE || lower > upper) {
 		pr_debug("Failed to bind illegal {%u,%u,%u} with scope %u\n",
@@ -490,14 +490,15 @@ struct publication *tipc_nametbl_insert_publ(struct net *net, u32 type,
 	sc = tipc_service_find(net, type);
 	if (!sc)
 		sc = tipc_service_create(type, &nt->services[hash(type)]);
-	if (!sc)
-		return NULL;
-
-	spin_lock_bh(&sc->lock);
-	p = tipc_service_insert_publ(net, sc, type, lower, upper,
-				     scope, node, port, key);
-	spin_unlock_bh(&sc->lock);
-	return p;
+	if (sc) {
+		spin_lock_bh(&sc->lock);
+		res = tipc_service_insert_publ(net, sc, p);
+		spin_unlock_bh(&sc->lock);
+	}
+	if (res)
+		return p;
+	kfree(p);
+	return NULL;
 }
 
 struct publication *tipc_nametbl_remove_publ(struct net *net, u32 type,
-- 
2.28.0

Re: [tipc-discussion] [net-next 01/16] tipc: re-organize members of struct publication

[Tung Q. N. <tun...@de...>]

Hi Jon,

Some minor comments for this patch (marked with "[Tung]").

Best regards,
Tung Nguyen

-----Original Message-----
From: jm...@re... <jm...@re...> 
Sent: Wednesday, December 9, 2020 1:50 AM
To: tip...@li...
Cc: Tung Quang Nguyen <tun...@de...>; Hoang Huu Le <hoa...@de...>; Tuong Tong Lien <tuo...@de...>; jm...@re...; ma...@do...; xi...@re...; yin...@wi...; par...@gm...
Subject: [net-next 01/16] tipc: re-organize members of struct publication

From: Jon Maloy <jm...@re...>

In a future commit we will introduce more members to struct publication.
In order to keep this structure comprehensible we now group some of
its current fields into the sub-structures where they really belong,
- A struct tipc_service_range for the functional address the publication
  is representing.
- A struct tipc_socket_addr for the socket bound to that service range.

We also rename the stack variable 'publ' to just 'p' in a couple of places.
This is just as easy to understand in the given context, and keeps the
number of wrapped code lines to a minimum.

There are no functional changes in this commit.

Signed-off-by: Jon Maloy <jm...@re...>
---
 net/tipc/name_distr.c | 62 +++++++++++++++++++++----------------------
 net/tipc/name_table.c | 44 +++++++++++++++---------------
 net/tipc/name_table.h | 17 +++++-------
 net/tipc/socket.c     | 32 +++++++++++-----------
 4 files changed, 75 insertions(+), 80 deletions(-)

diff --git a/net/tipc/name_distr.c b/net/tipc/name_distr.c
index 6cf57c3bfa27..721d2fca3d6f 100644
--- a/net/tipc/name_distr.c
+++ b/net/tipc/name_distr.c
@@ -3,6 +3,7 @@
  *
  * Copyright (c) 2000-2006, 2014, Ericsson AB
  * Copyright (c) 2005, 2010-2011, Wind River Systems
+ * Copyright (c) 2020, Red Hat Inc
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -55,10 +56,10 @@ struct distr_queue_item {
  */
 static void publ_to_item(struct distr_item *i, struct publication *p)
 {
-	i->type = htonl(p->type);
-	i->lower = htonl(p->lower);
-	i->upper = htonl(p->upper);
-	i->port = htonl(p->port);
+	i->type = htonl(p->sr.type);
+	i->lower = htonl(p->sr.lower);
+	i->upper = htonl(p->sr.upper);
+	i->port = htonl(p->sk.ref);
 	i->key = htonl(p->key);
 }
 
@@ -90,20 +91,20 @@ static struct sk_buff *named_prepare_buf(struct net *net, u32 type, u32 size,
 /**
  * tipc_named_publish - tell other nodes about a new publication by this node
  * @net: the associated network namespace
- * @publ: the new publication
+ * @p: the new publication
  */
-struct sk_buff *tipc_named_publish(struct net *net, struct publication *publ)
+struct sk_buff *tipc_named_publish(struct net *net, struct publication *p)
 {
 	struct name_table *nt = tipc_name_table(net);
 	struct distr_item *item;
 	struct sk_buff *skb;
 
-	if (publ->scope == TIPC_NODE_SCOPE) {
-		list_add_tail_rcu(&publ->binding_node, &nt->node_scope);
+	if (p->scope == TIPC_NODE_SCOPE) {
+		list_add_tail_rcu(&p->binding_node, &nt->node_scope);
 		return NULL;
 	}
 	write_lock_bh(&nt->cluster_scope_lock);
-	list_add_tail(&publ->binding_node, &nt->cluster_scope);
+	list_add_tail(&p->binding_node, &nt->cluster_scope);
 	write_unlock_bh(&nt->cluster_scope_lock);
 	skb = named_prepare_buf(net, PUBLICATION, ITEM_SIZE, 0);
 	if (!skb) {
@@ -113,25 +114,25 @@ struct sk_buff *tipc_named_publish(struct net *net, struct publication *publ)
 	msg_set_named_seqno(buf_msg(skb), nt->snd_nxt++);
 	msg_set_non_legacy(buf_msg(skb));
 	item = (struct distr_item *)msg_data(buf_msg(skb));
-	publ_to_item(item, publ);
+	publ_to_item(item, p);
 	return skb;
 }
 
 /**
  * tipc_named_withdraw - tell other nodes about a withdrawn publication by this node
  * @net: the associated network namespace
- * @publ: the withdrawn publication
+ * @p: the withdrawn publication
  */
-struct sk_buff *tipc_named_withdraw(struct net *net, struct publication *publ)
+struct sk_buff *tipc_named_withdraw(struct net *net, struct publication *p)
 {
 	struct name_table *nt = tipc_name_table(net);
 	struct distr_item *item;
 	struct sk_buff *skb;
 
 	write_lock_bh(&nt->cluster_scope_lock);
-	list_del(&publ->binding_node);
+	list_del(&p->binding_node);
 	write_unlock_bh(&nt->cluster_scope_lock);
-	if (publ->scope == TIPC_NODE_SCOPE)
+	if (p->scope == TIPC_NODE_SCOPE)
 		return NULL;
 
 	skb = named_prepare_buf(net, WITHDRAWAL, ITEM_SIZE, 0);
@@ -142,7 +143,7 @@ struct sk_buff *tipc_named_withdraw(struct net *net, struct publication *publ)
 	msg_set_named_seqno(buf_msg(skb), nt->snd_nxt++);
 	msg_set_non_legacy(buf_msg(skb));
 	item = (struct distr_item *)msg_data(buf_msg(skb));
-	publ_to_item(item, publ);
+	publ_to_item(item, p);
 	return skb;
 }
 
@@ -239,27 +240,26 @@ void tipc_named_node_up(struct net *net, u32 dnode, u16 capabilities)
  * Invoked for each publication issued by a newly failed node.
  * Removes publication structure from name table & deletes it.
  */
-static void tipc_publ_purge(struct net *net, struct publication *publ, u32 addr)
+static void tipc_publ_purge(struct net *net, struct publication *p, u32 addr)
 {
 	struct tipc_net *tn = tipc_net(net);
-	struct publication *p;
+	struct publication *_p;
 
 	spin_lock_bh(&tn->nametbl_lock);
-	p = tipc_nametbl_remove_publ(net, publ->type, publ->lower, publ->upper,
-				     publ->node, publ->key);
-	if (p)
-		tipc_node_unsubscribe(net, &p->binding_node, addr);
+	_p = tipc_nametbl_remove_publ(net, p->sr.type, p->sr.lower,
+				      p->sr.upper, p->sk.node, p->key);
+	if (_p)
+		tipc_node_unsubscribe(net, &_p->binding_node, addr);
 	spin_unlock_bh(&tn->nametbl_lock);
 
-	if (p != publ) {
+	if (_p != p) {
 		pr_err("Unable to remove publication from failed node\n"
 		       " (type=%u, lower=%u, node=0x%x, port=%u, key=%u)\n",
-		       publ->type, publ->lower, publ->node, publ->port,
-		       publ->key);
+		       p->sr.type, p->sr.lower, p->sk.node, p->sk.ref, p->key);
 	}
 
-	if (p)
-		kfree_rcu(p, rcu);
+	if (_p)
+		kfree_rcu(_p, rcu);
 }
 
 void tipc_publ_notify(struct net *net, struct list_head *nsub_list,
@@ -410,15 +410,15 @@ void tipc_named_reinit(struct net *net)
 {
 	struct name_table *nt = tipc_name_table(net);
 	struct tipc_net *tn = tipc_net(net);
-	struct publication *publ;
+	struct publication *p;
 	u32 self = tipc_own_addr(net);
 
 	spin_lock_bh(&tn->nametbl_lock);
 
-	list_for_each_entry_rcu(publ, &nt->node_scope, binding_node)
-		publ->node = self;
-	list_for_each_entry_rcu(publ, &nt->cluster_scope, binding_node)
-		publ->node = self;
+	list_for_each_entry_rcu(p, &nt->node_scope, binding_node)
+		p->sk.node = self;
+	list_for_each_entry_rcu(p, &nt->cluster_scope, binding_node)
+		p->sk.node = self;
 	nt->rc_dests = 0;
 	spin_unlock_bh(&tn->nametbl_lock);
 }
diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c
index ee5ac40ea2b6..c37a4a9c87ca 100644
--- a/net/tipc/name_table.c
+++ b/net/tipc/name_table.c
@@ -239,12 +239,12 @@ static struct publication *tipc_publ_create(u32 type, u32 lower, u32 upper,
 	if (!publ)
 		return NULL;
 
-	publ->type = type;
-	publ->lower = lower;
-	publ->upper = upper;
+	publ->sr.type = type;
+	publ->sr.lower = lower;
+	publ->sr.upper = upper;
 	publ->scope = scope;
-	publ->node = node;
-	publ->port = port;
+	publ->sk.node = node;
+	publ->sk.ref = port;
 	publ->key = key;
 	INIT_LIST_HEAD(&publ->binding_sock);
 	INIT_LIST_HEAD(&publ->binding_node);
[Tung]: Should 'publ' be changed to 'p' ?
@@ -347,7 +347,7 @@ static struct publication *tipc_service_insert_publ(struct net *net,
 
 	/* Return if the publication already exists */
 	list_for_each_entry(p, &sr->all_publ, all_publ) {
-		if (p->key == key && (!p->node || p->node == node))
+		if (p->key == key && (!p->sk.node || p->sk.node == node))
 			return NULL;
 	}
 
@@ -363,8 +363,8 @@ static struct publication *tipc_service_insert_publ(struct net *net,
 
 	/* Any subscriptions waiting for notification?  */
 	list_for_each_entry_safe(sub, tmp, &sc->subscriptions, service_list) {
-		tipc_sub_report_overlap(sub, p->lower, p->upper, TIPC_PUBLISHED,
-					p->port, p->node, p->scope, first);
+		tipc_sub_report_overlap(sub, p->sr.lower, p->sr.upper, TIPC_PUBLISHED,
+					p->sk.ref, p->sk.node, p->scope, first);
 	}
 	return p;
 err:
@@ -384,7 +384,7 @@ static struct publication *tipc_service_remove_publ(struct service_range *sr,
 	struct publication *p;
 
 	list_for_each_entry(p, &sr->all_publ, all_publ) {
-		if (p->key != key || (node && node != p->node))
+		if (p->key != key || (node && node != p->sk.node))
 			continue;
 		list_del(&p->all_publ);
 		list_del(&p->local_publ);
@@ -452,8 +452,8 @@ static void tipc_service_subscribe(struct tipc_service *service,
 	/* Sort the publications before reporting */
 	list_sort(NULL, &publ_list, tipc_publ_sort);
 	list_for_each_entry_safe(p, tmp, &publ_list, list) {
-		tipc_sub_report_overlap(sub, p->lower, p->upper,
-					TIPC_PUBLISHED, p->port, p->node,
+		tipc_sub_report_overlap(sub, p->sr.lower, p->sr.upper,
+					TIPC_PUBLISHED, p->sk.ref, p->sk.node,
 					p->scope, true);
 		list_del_init(&p->list);
 	}
@@ -525,7 +525,7 @@ struct publication *tipc_nametbl_remove_publ(struct net *net, u32 type,
 	last = list_empty(&sr->all_publ);
 	list_for_each_entry_safe(sub, tmp, &sc->subscriptions, service_list) {
 		tipc_sub_report_overlap(sub, lower, upper, TIPC_WITHDRAWN,
-					p->port, node, p->scope, last);
+					p->sk.ref, node, p->scope, last);
 	}
 
 	/* Remove service range item if this was its last publication */
@@ -603,8 +603,8 @@ u32 tipc_nametbl_translate(struct net *net, u32 type, u32 instance, u32 *dnode)
 					     all_publ);
 			list_move_tail(&p->all_publ, &sr->all_publ);
 		}
-		port = p->port;
-		node = p->node;
+		port = p->sk.ref;
+		node = p->sk.node;
 		/* Todo: as for legacy, pick the first matching range only, a
 		 * "true" round-robin will be performed as needed.
 		 */
@@ -643,9 +643,9 @@ bool tipc_nametbl_lookup(struct net *net, u32 type, u32 instance, u32 scope,
 	list_for_each_entry(p, &sr->all_publ, all_publ) {
 		if (p->scope != scope)
 			continue;
-		if (p->port == exclude && p->node == self)
+		if (p->sk.ref == exclude && p->sk.node == self)
 			continue;
-		tipc_dest_push(dsts, p->node, p->port);
+		tipc_dest_push(dsts, p->sk.node, p->sk.ref);
 		(*dstcnt)++;
 		if (all)
 			continue;
@@ -675,7 +675,7 @@ void tipc_nametbl_mc_lookup(struct net *net, u32 type, u32 lower, u32 upper,
 	service_range_foreach_match(sr, sc, lower, upper) {
 		list_for_each_entry(p, &sr->local_publ, local_publ) {
 			if (p->scope == scope || (!exact && p->scope < scope))
-				tipc_dest_push(dports, 0, p->port);
+				tipc_dest_push(dports, 0, p->sk.ref);
 		}
 	}
 	spin_unlock_bh(&sc->lock);
@@ -702,7 +702,7 @@ void tipc_nametbl_lookup_dst_nodes(struct net *net, u32 type, u32 lower,
 	spin_lock_bh(&sc->lock);
 	service_range_foreach_match(sr, sc, lower, upper) {
 		list_for_each_entry(p, &sr->all_publ, all_publ) {
-			tipc_nlist_add(nodes, p->node);
+			tipc_nlist_add(nodes, p->sk.node);
 		}
 	}
 	spin_unlock_bh(&sc->lock);
@@ -731,7 +731,7 @@ void tipc_nametbl_build_group(struct net *net, struct tipc_group *grp,
 		list_for_each_entry(p, &sr->all_publ, all_publ) {
 			if (p->scope != scope)
 				continue;
-			tipc_group_add_member(grp, p->node, p->port, p->lower);
+			tipc_group_add_member(grp, p->sk.node, p->sk.ref, p->sr.lower);
 		}
 	}
 	spin_unlock_bh(&sc->lock);
@@ -909,7 +909,7 @@ static void tipc_service_delete(struct net *net, struct tipc_service *sc)
 	spin_lock_bh(&sc->lock);
 	rbtree_postorder_for_each_entry_safe(sr, tmpr, &sc->ranges, tree_node) {
 		list_for_each_entry_safe(p, tmp, &sr->all_publ, all_publ) {
-			tipc_service_remove_publ(sr, p->node, p->key);
+			tipc_service_remove_publ(sr, p->sk.node, p->key);
 			kfree_rcu(p, rcu);
 		}
 		rb_erase_augmented(&sr->tree_node, &sc->ranges, &sr_callbacks);
@@ -993,9 +993,9 @@ static int __tipc_nl_add_nametable_publ(struct tipc_nl_msg *msg,
 			goto publ_msg_full;
 		if (nla_put_u32(msg->skb, TIPC_NLA_PUBL_SCOPE, p->scope))
 			goto publ_msg_full;
-		if (nla_put_u32(msg->skb, TIPC_NLA_PUBL_NODE, p->node))
+		if (nla_put_u32(msg->skb, TIPC_NLA_PUBL_NODE, p->sk.node))
 			goto publ_msg_full;
-		if (nla_put_u32(msg->skb, TIPC_NLA_PUBL_REF, p->port))
+		if (nla_put_u32(msg->skb, TIPC_NLA_PUBL_REF, p->sk.ref))
 			goto publ_msg_full;
 		if (nla_put_u32(msg->skb, TIPC_NLA_PUBL_KEY, p->key))
 			goto publ_msg_full;
diff --git a/net/tipc/name_table.h b/net/tipc/name_table.h
index 5a82a01369d6..3fff00440e1a 100644
--- a/net/tipc/name_table.h
+++ b/net/tipc/name_table.h
@@ -3,6 +3,7 @@
  *
  * Copyright (c) 2000-2006, 2014-2018, Ericsson AB
  * Copyright (c) 2004-2005, 2010-2011, Wind River Systems
+ * Copyright (c) 2020, Red Hat Inc
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -50,13 +51,10 @@ struct tipc_group;
 #define TIPC_NAMETBL_SIZE	1024	/* must be a power of 2 */
 
 /**
- * struct publication - info about a published (name or) name sequence
- * @type: name sequence type
- * @lower: name sequence lower bound
- * @upper: name sequence upper bound
+ * struct publication - info about a published service address or range
+ * @sr: service range represented by this publication
+ * @sk: address of socket bound to this publication
  * @scope: scope of publication, TIPC_NODE_SCOPE or TIPC_CLUSTER_SCOPE
- * @node: network address of publishing socket's node
- * @port: publishing port
  * @key: publication key, unique across the cluster
  * @id: publication id
  * @binding_node: all publications from the same node which bound this one
@@ -74,12 +72,9 @@ struct tipc_group;
  * @rcu: RCU callback head used for deferred freeing
  */
 struct publication {
-	u32 type;
-	u32 lower;
-	u32 upper;
+	struct tipc_service_range sr;
+	struct tipc_socket_addr sk;
 	u32 scope;
-	u32 node;
-	u32 port;
 	u32 key;
 	u32 id;
 	struct list_head binding_node;
diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index cebcc104dc70..bff14df40bc9 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -2934,19 +2934,19 @@ static int tipc_sk_withdraw(struct tipc_sock *tsk, uint scope,
 		if (seq) {
 			if (publ->scope != scope)
 				continue;
-			if (publ->type != seq->type)
+			if (publ->sr.type != seq->type)
 				continue;
-			if (publ->lower != seq->lower)
+			if (publ->sr.lower != seq->lower)
 				continue;
-			if (publ->upper != seq->upper)
+			if (publ->sr.upper != seq->upper)
 				break;
-			tipc_nametbl_withdraw(net, publ->type, publ->lower,
-					      publ->upper, publ->key);
+			tipc_nametbl_withdraw(net, publ->sr.type, publ->sr.lower,
+					      publ->sr.upper, publ->key);
 			rc = 0;
 			break;
 		}
-		tipc_nametbl_withdraw(net, publ->type, publ->lower,
-				      publ->upper, publ->key);
+		tipc_nametbl_withdraw(net, publ->sr.type, publ->sr.lower,
+				      publ->sr.upper, publ->key);
 		rc = 0;
 	}
 	if (list_empty(&tsk->publications))
[Tung]: Should 'publ' be changed to 'p' ?
@@ -3711,11 +3711,11 @@ static int __tipc_nl_add_sk_publ(struct sk_buff *skb,
 
 	if (nla_put_u32(skb, TIPC_NLA_PUBL_KEY, publ->key))
 		goto attr_msg_cancel;
-	if (nla_put_u32(skb, TIPC_NLA_PUBL_TYPE, publ->type))
+	if (nla_put_u32(skb, TIPC_NLA_PUBL_TYPE, publ->sr.type))
 		goto attr_msg_cancel;
-	if (nla_put_u32(skb, TIPC_NLA_PUBL_LOWER, publ->lower))
+	if (nla_put_u32(skb, TIPC_NLA_PUBL_LOWER, publ->sr.lower))
 		goto attr_msg_cancel;
-	if (nla_put_u32(skb, TIPC_NLA_PUBL_UPPER, publ->upper))
+	if (nla_put_u32(skb, TIPC_NLA_PUBL_UPPER, publ->sr.upper))
 		goto attr_msg_cancel;
 
 	nla_nest_end(skb, attrs);
[Tung]: Should 'publ' be changed to 'p' ?
@@ -3863,9 +3863,9 @@ bool tipc_sk_filtering(struct sock *sk)
 		p = list_first_entry_or_null(&tsk->publications,
 					     struct publication, binding_sock);
 		if (p) {
-			type = p->type;
-			lower = p->lower;
-			upper = p->upper;
+			type = p->sr.type;
+			lower = p->sr.lower;
+			upper = p->sr.upper;
 		}
 	}
 
@@ -3964,9 +3964,9 @@ int tipc_sk_dump(struct sock *sk, u16 dqueues, char *buf)
 	if (tsk->published) {
 		p = list_first_entry_or_null(&tsk->publications,
 					     struct publication, binding_sock);
-		i += scnprintf(buf + i, sz - i, " %u", (p) ? p->type : 0);
-		i += scnprintf(buf + i, sz - i, " %u", (p) ? p->lower : 0);
-		i += scnprintf(buf + i, sz - i, " %u", (p) ? p->upper : 0);
+		i += scnprintf(buf + i, sz - i, " %u", (p) ? p->sr.type : 0);
+		i += scnprintf(buf + i, sz - i, " %u", (p) ? p->sr.lower : 0);
+		i += scnprintf(buf + i, sz - i, " %u", (p) ? p->sr.upper : 0);
 	}
 	i += scnprintf(buf + i, sz - i, " | %u", tsk->snd_win);
 	i += scnprintf(buf + i, sz - i, " %u", tsk->rcv_win);
-- 
2.28.0

Re: [tipc-discussion] [net-next] tipc: remove duplicated code in tipc_msg_create

[Jon M. <jm...@re...>]

On 1/25/21 3:58 AM, Hoang Huu Le wrote:
> Remove a duplicate code checking for header size in tipc_msg_create() as
> it's already being done in tipc_msg_init().
>
> Signed-off-by: Hoang Huu Le <hoa...@de...>
> ---
>   net/tipc/msg.c | 4 ----
>   1 file changed, 4 deletions(-)
>
> diff --git a/net/tipc/msg.c b/net/tipc/msg.c
> index 2aca86021df5..e9263280a2d4 100644
> --- a/net/tipc/msg.c
> +++ b/net/tipc/msg.c
> @@ -117,10 +117,6 @@ struct sk_buff *tipc_msg_create(uint user, uint type,
>   	msg_set_origport(msg, oport);
>   	msg_set_destport(msg, dport);
>   	msg_set_errcode(msg, errcode);
> -	if (hdr_sz > SHORT_H_SIZE) {
> -		msg_set_orignode(msg, onode);
> -		msg_set_destnode(msg, dnode);
> -	}
>   	return buf;
>   }
>   
Acked-by: Jon Maloy <jm...@re...>

[tipc-discussion] [net-next] tipc: remove duplicated code in tipc_msg_create

[Hoang H. Le <hoa...@de...>]

Remove a duplicate code checking for header size in tipc_msg_create() as
it's already being done in tipc_msg_init().

Signed-off-by: Hoang Huu Le <hoa...@de...>
---
 net/tipc/msg.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/net/tipc/msg.c b/net/tipc/msg.c
index 2aca86021df5..e9263280a2d4 100644
--- a/net/tipc/msg.c
+++ b/net/tipc/msg.c
@@ -117,10 +117,6 @@ struct sk_buff *tipc_msg_create(uint user, uint type,
 	msg_set_origport(msg, oport);
 	msg_set_destport(msg, dport);
 	msg_set_errcode(msg, errcode);
-	if (hdr_sz > SHORT_H_SIZE) {
-		msg_set_orignode(msg, onode);
-		msg_set_destnode(msg, dnode);
-	}
 	return buf;
 }
 
-- 
2.25.1