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

[tipc-discussion] [net-next 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 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);
@@ -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))
@@ -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.28.0

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

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

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)
 {
-	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] [net-next 13/16] tipc: simplify signature of tipc_find_service()

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

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)
 {
-	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] [net-next 02/16] tipc: move creation of publication item one level up in call chain

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

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 (res)
+		return p;
+	kfree(p);
+	return NULL;
 }
 
 struct publication *tipc_nametbl_remove_publ(struct net *net, u32 type,
-- 
2.28.0

[tipc-discussion] [net-next 15/16] tipc: add host-endian copy of user subscription to struct tipc_subscription

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

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

We reduce and localize the usage of the tipc_sub_xx() macros by adding a
corresponding member, with fields set in host-endian format, to struct
tipc_subscription.

Signed-off-by: Jon Maloy <jm...@re...>
---
 net/tipc/name_table.c | 29 +++++++++++-----------------
 net/tipc/subscr.c     | 45 +++++++++++++++++++++++--------------------
 net/tipc/subscr.h     |  3 ++-
 3 files changed, 37 insertions(+), 40 deletions(-)

diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c
index 4dfac3b9d26c..9085c41a1709 100644
--- a/net/tipc/name_table.c
+++ b/net/tipc/name_table.c
@@ -414,17 +414,14 @@ static int tipc_publ_sort(void *priv, struct list_head *a,
 static void tipc_service_subscribe(struct tipc_service *service,
 				   struct tipc_subscription *sub)
 {
-	struct tipc_subscr *sb = &sub->evt.s;
 	struct publication *p, *first, *tmp;
 	struct list_head publ_list;
 	struct service_range *sr;
-	struct tipc_service_range r;
-	u32 filter;
+	u32 filter, lower, upper;
 
-	r.type = tipc_sub_read(sb, seq.type);
-	r.lower = tipc_sub_read(sb, seq.lower);
-	r.upper = tipc_sub_read(sb, seq.upper);
-	filter = tipc_sub_read(sb, filter);
+	filter = sub->s.filter;
+	lower = sub->s.seq.lower;
+	upper = sub->s.seq.upper;
 
 	tipc_sub_get(sub);
 	list_add(&sub->service_list, &service->subscriptions);
@@ -433,7 +430,7 @@ static void tipc_service_subscribe(struct tipc_service *service,
 		return;
 
 	INIT_LIST_HEAD(&publ_list);
-	service_range_foreach_match(sr, service, r.lower, r.upper) {
+	service_range_foreach_match(sr, service, lower, upper) {
 		first = NULL;
 		list_for_each_entry(p, &sr->all_publ, all_publ) {
 			if (filter & TIPC_SUB_PORTS)
@@ -830,14 +827,13 @@ void tipc_nametbl_withdraw(struct net *net, struct tipc_uaddr *ua,
 bool tipc_nametbl_subscribe(struct tipc_subscription *sub)
 {
 	struct tipc_net *tn = tipc_net(sub->net);
-	struct tipc_subscr *s = &sub->evt.s;
-	u32 type = tipc_sub_read(s, seq.type);
+	u32 type = sub->s.seq.type;
 	struct tipc_service *sc;
 	struct tipc_uaddr ua;
 	bool res = true;
 
 	tipc_uaddr(&ua, TIPC_SERVICE_RANGE, TIPC_NODE_SCOPE, type,
-		   tipc_sub_read(s, seq.lower), tipc_sub_read(s, seq.upper));
+		   sub->s.seq.lower, sub->s.seq.upper);
 	spin_lock_bh(&tn->nametbl_lock);
 	sc = tipc_service_find(sub->net, &ua);
 	if (!sc)
@@ -847,9 +843,8 @@ bool tipc_nametbl_subscribe(struct tipc_subscription *sub)
 		tipc_service_subscribe(sc, sub);
 		spin_unlock_bh(&sc->lock);
 	} else {
-		pr_warn("Failed to subscribe for {%u,%u,%u}\n", type,
-			tipc_sub_read(s, seq.lower),
-			tipc_sub_read(s, seq.upper));
+		pr_warn("Failed to subscribe for {%u,%u,%u}\n",
+			type, sub->s.seq.lower, sub->s.seq.upper);
 		res = false;
 	}
 	spin_unlock_bh(&tn->nametbl_lock);
@@ -863,13 +858,11 @@ bool tipc_nametbl_subscribe(struct tipc_subscription *sub)
 void tipc_nametbl_unsubscribe(struct tipc_subscription *sub)
 {
 	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;
 
-	tipc_uaddr(&ua, TIPC_SERVICE_RANGE, TIPC_NODE_SCOPE, type,
-		   tipc_sub_read(s, seq.lower), tipc_sub_read(s, seq.upper));
+	tipc_uaddr(&ua, TIPC_SERVICE_RANGE, TIPC_NODE_SCOPE,
+		   sub->s.seq.type, sub->s.seq.lower, sub->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 feda0b6bbf1b..b6d883512f93 100644
--- a/net/tipc/subscr.c
+++ b/net/tipc/subscr.c
@@ -65,37 +65,32 @@ static void tipc_sub_send_event(struct tipc_subscription *sub,
 
 /**
  * tipc_sub_check_overlap - test for subscription overlap with the given values
- * @seq: tipc_name_seq to check
- * @found_lower: lower value to test
- * @found_upper: upper value to test
+ * @subscribed: the service range subscribed for
+ * @found: the service range we are checning for match
  *
  * Returns true if there is overlap, otherwise false.
  */
-bool tipc_sub_check_overlap(struct tipc_service_range *sr,
-			    u32 found_lower, u32 found_upper)
+bool tipc_sub_check_overlap(struct tipc_service_range *subscribed,
+			    struct tipc_service_range *found)
 {
-	if (found_lower < sr->lower)
-		found_lower = sr->lower;
-	if (found_upper > sr->upper)
-		found_upper = sr->upper;
-	if (found_lower > found_upper)
-		return false;
-	return true;
+	u32 found_lower = found->lower;
+	u32 found_upper = found->upper;
+
+	if (found_lower < subscribed->lower)
+		found_lower = subscribed->lower;
+	if (found_upper > subscribed->upper)
+		found_upper = subscribed->upper;
+	return found_lower <= found_upper;
 }
 
 void tipc_sub_report_overlap(struct tipc_subscription *sub,
 			     struct publication *p,
 			     u32 event, bool must)
 {
-	struct tipc_subscr *s = &sub->evt.s;
-	u32 filter = tipc_sub_read(s, filter);
-	struct tipc_service_range seq;
-
-	seq.type = tipc_sub_read(s, seq.type);
-	seq.lower = tipc_sub_read(s, seq.lower);
-	seq.upper = tipc_sub_read(s, seq.upper);
+	struct tipc_service_range *sr = &sub->s.seq;
+	u32 filter = sub->s.filter;
 
-	if (!tipc_sub_check_overlap(&seq, p->sr.lower, p->sr.upper))
+	if (!tipc_sub_check_overlap(sr, &p->sr))
 		return;
 	if (!must && !(filter & TIPC_SUB_PORTS))
 		return;
@@ -137,12 +132,14 @@ struct tipc_subscription *tipc_sub_subscribe(struct net *net,
 					     struct tipc_subscr *s,
 					     int conid)
 {
+	u32 lower = tipc_sub_read(s, seq.lower);
+	u32 upper = tipc_sub_read(s, seq.upper);
 	u32 filter = tipc_sub_read(s, filter);
 	struct tipc_subscription *sub;
 	u32 timeout;
 
 	if ((filter & TIPC_SUB_PORTS && filter & TIPC_SUB_SERVICE) ||
-	    (tipc_sub_read(s, seq.lower) > tipc_sub_read(s, seq.upper))) {
+	    lower > upper) {
 		pr_warn("Subscription rejected, illegal request\n");
 		return NULL;
 	}
@@ -157,6 +154,12 @@ struct tipc_subscription *tipc_sub_subscribe(struct net *net,
 	sub->conid = conid;
 	sub->inactive = false;
 	memcpy(&sub->evt.s, s, sizeof(*s));
+	sub->s.seq.type = tipc_sub_read(s, seq.type);
+	sub->s.seq.lower = lower;
+	sub->s.seq.upper = upper;
+	sub->s.filter = filter;
+	sub->s.timeout = tipc_sub_read(s, timeout);
+	memcpy(sub->s.usr_handle, s->usr_handle, 8);
 	spin_lock_init(&sub->lock);
 	kref_init(&sub->kref);
 	if (!tipc_nametbl_subscribe(sub)) {
diff --git a/net/tipc/subscr.h b/net/tipc/subscr.h
index 5fce2e13fc1a..dcadb376b860 100644
--- a/net/tipc/subscr.h
+++ b/net/tipc/subscr.h
@@ -60,12 +60,13 @@ struct tipc_conn;
  * @lock: serialize up/down and timer events
  */
 struct tipc_subscription {
+	struct tipc_subscr s;
+	struct tipc_event evt;
 	struct kref kref;
 	struct net *net;
 	struct timer_list timer;
 	struct list_head service_list;
 	struct list_head sub_list;
-	struct tipc_event evt;
 	int conid;
 	bool inactive;
 	spinlock_t lock;
-- 
2.28.0

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

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

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

We simplify the signatures of the functions tipc_service_create_range()
and tipc_service_finc_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

[tipc-discussion] [net-next 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 that match their purpose, and add comments
that further describe what they are doing.

Signed-off-by: Jon Maloy <jm...@re...>
---
 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 2aca86021df5..9eddbddb2fec 100644
--- a/net/tipc/msg.c
+++ b/net/tipc/msg.c
@@ -727,8 +727,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 50562d086016..e6177ccf1140 100644
--- a/net/tipc/name_table.c
+++ b/net/tipc/name_table.c
@@ -547,24 +547,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;
@@ -620,9 +622,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;
@@ -649,7 +657,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;
@@ -661,8 +669,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;
@@ -685,12 +699,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 5f48f05b93be..9f6e8efca00f 100644
--- a/net/tipc/name_table.h
+++ b/net/tipc/name_table.h
@@ -112,16 +112,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 5a017a34fb5f..05cfe179458e 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.28.0

[tipc-discussion] [net-next 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.

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] [net-next 10/16] tipc: simplify signature of tipc_nametbl_lookup_mcast_nodes()

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

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

We follow up the preceding commits by reducing the signature of
the function tipc_nametbl_lookup_mcast_nodes().

Signed-off-by: Jon Maloy <jm...@re...>
---
 net/tipc/name_table.c |  8 ++++----
 net/tipc/name_table.h |  4 ++--
 net/tipc/socket.c     | 16 +++++++---------
 3 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c
index ea4356dfb47d..23af099370fb 100644
--- a/net/tipc/name_table.c
+++ b/net/tipc/name_table.c
@@ -701,20 +701,20 @@ void tipc_nametbl_lookup_mcast_sockets(struct net *net, struct tipc_uaddr *ua,
  * 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_mcast_nodes(struct net *net, u32 type, u32 lower,
-				     u32 upper, struct tipc_nlist *nodes)
+void tipc_nametbl_lookup_mcast_nodes(struct net *net, struct tipc_uaddr *ua,
+				     struct tipc_nlist *nodes)
 {
 	struct service_range *sr;
 	struct tipc_service *sc;
 	struct publication *p;
 
 	rcu_read_lock();
-	sc = tipc_service_find(net, type);
+	sc = tipc_service_find(net, ua->sr.type);
 	if (!sc)
 		goto exit;
 
 	spin_lock_bh(&sc->lock);
-	service_range_foreach_match(sr, sc, lower, upper) {
+	service_range_foreach_match(sr, sc, ua->sr.lower, ua->sr.upper) {
 		list_for_each_entry(p, &sr->all_publ, all_publ) {
 			tipc_nlist_add(nodes, p->sk.node);
 		}
diff --git a/net/tipc/name_table.h b/net/tipc/name_table.h
index 368a76f73892..259ba514193e 100644
--- a/net/tipc/name_table.h
+++ b/net/tipc/name_table.h
@@ -115,8 +115,8 @@ 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, struct tipc_uaddr *ua,
 				       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);
+void tipc_nametbl_lookup_mcast_nodes(struct net *net, struct tipc_uaddr *ua,
+				     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,
diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 68d457c41c89..7d4807d0e2d1 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -840,7 +840,7 @@ static __poll_t tipc_poll(struct file *file, struct socket *sock,
  * Called from function tipc_sendmsg(), which has done all sanity checks
  * Return: the number of bytes sent on success, or errno
  */
-static int tipc_sendmcast(struct  socket *sock, struct tipc_service_range *seq,
+static int tipc_sendmcast(struct  socket *sock, struct tipc_uaddr *ua,
 			  struct msghdr *msg, size_t dlen, long timeout)
 {
 	struct sock *sk = sock->sk;
@@ -848,7 +848,6 @@ static int tipc_sendmcast(struct  socket *sock, struct tipc_service_range *seq,
 	struct tipc_msg *hdr = &tsk->phdr;
 	struct net *net = sock_net(sk);
 	int mtu = tipc_bcast_get_mtu(net);
-	struct tipc_mc_method *method = &tsk->mc_method;
 	struct sk_buff_head pkts;
 	struct tipc_nlist dsts;
 	int rc;
@@ -863,8 +862,7 @@ 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_mcast_nodes(net, seq->type, seq->lower,
-					seq->upper, &dsts);
+	tipc_nametbl_lookup_mcast_nodes(net, ua, &dsts);
 	if (!dsts.local && !dsts.remote)
 		return -EHOSTUNREACH;
 
@@ -874,9 +872,9 @@ static int tipc_sendmcast(struct  socket *sock, struct tipc_service_range *seq,
 	msg_set_lookup_scope(hdr, TIPC_CLUSTER_SCOPE);
 	msg_set_destport(hdr, 0);
 	msg_set_destnode(hdr, 0);
-	msg_set_nametype(hdr, seq->type);
-	msg_set_namelower(hdr, seq->lower);
-	msg_set_nameupper(hdr, seq->upper);
+	msg_set_nametype(hdr, ua->sr.type);
+	msg_set_namelower(hdr, ua->sr.lower);
+	msg_set_nameupper(hdr, ua->sr.upper);
 
 	/* Build message as chain of buffers */
 	__skb_queue_head_init(&pkts);
@@ -886,7 +884,7 @@ static int tipc_sendmcast(struct  socket *sock, struct tipc_service_range *seq,
 	if (unlikely(rc == dlen)) {
 		trace_tipc_sk_sendmcast(sk, skb_peek(&pkts),
 					TIPC_DUMP_SK_SNDQ, " ");
-		rc = tipc_mcast_xmit(net, &pkts, method, &dsts,
+		rc = tipc_mcast_xmit(net, &pkts, &tsk->mc_method, &dsts,
 				     &tsk->cong_link_cnt);
 	}
 
@@ -1479,7 +1477,7 @@ static int __tipc_sendmsg(struct socket *sock, struct msghdr *m, size_t dlen)
 
 	/* Determine destination */
 	if (atype == TIPC_SERVICE_RANGE) {
-		return tipc_sendmcast(sock, &ua->sr, m, dlen, timeout);
+		return tipc_sendmcast(sock, ua, m, dlen, timeout);
 	} else if (atype == TIPC_SERVICE_ADDR) {
 		skaddr.node = ua->lookup_node;
 		ua->scope = skaddr.node ? TIPC_NODE_SCOPE : TIPC_CLUSTER_SCOPE;
-- 
2.28.0

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

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

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;
 	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] [net-next 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...>
---
 net/tipc/addr.h | 44 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/net/tipc/addr.h b/net/tipc/addr.h
index 1a11831bef62..2d7992e5da64 100644
--- a/net/tipc/addr.h
+++ b/net/tipc/addr.h
@@ -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.28.0

[tipc-discussion] [net-next 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...>
---
 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);
+
+	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;
+	rc = tipc_sk_publish(tsk, &ua);
 	if (rc) {
 		tipc_group_delete(net, grp);
 		tsk->group = NULL;
-- 
2.28.0

[tipc-discussion] [net-next] tipc: support 128bit node identity for peer removing

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

From: Hoang Le <hoa...@de...>

We add the support to remove a specific node down with 128bit
node identifier, as an alternative to legacy 32-bit node address.

example:
$tipc peer remove identiy <1001002|16777777>

Acked-by: Jon Maloy <jm...@re...>
Signed-off-by: Hoang Le <hoa...@de...>
---
 net/tipc/node.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/net/tipc/node.c b/net/tipc/node.c
index 032d8fc09894..81d69779016c 100644
--- a/net/tipc/node.c
+++ b/net/tipc/node.c
@@ -2220,6 +2220,9 @@ int tipc_nl_peer_rm(struct sk_buff *skb, struct genl_info *info)
 	struct tipc_net *tn = net_generic(net, tipc_net_id);
 	struct nlattr *attrs[TIPC_NLA_NET_MAX + 1];
 	struct tipc_node *peer, *temp_node;
+	u8 node_id[NODE_ID_LEN];
+	u64 *w0 = (u64 *)&node_id[0];
+	u64 *w1 = (u64 *)&node_id[8];
 	u32 addr;
 	int err;
 
@@ -2233,10 +2236,22 @@ int tipc_nl_peer_rm(struct sk_buff *skb, struct genl_info *info)
 	if (err)
 		return err;
 
-	if (!attrs[TIPC_NLA_NET_ADDR])
-		return -EINVAL;
+	/* attrs[TIPC_NLA_NET_NODEID] and attrs[TIPC_NLA_NET_ADDR] are
+	 * mutually exclusive cases
+	 */
+	if (attrs[TIPC_NLA_NET_ADDR]) {
+		addr = nla_get_u32(attrs[TIPC_NLA_NET_ADDR]);
+		if (!addr)
+			return -EINVAL;
+	}
 
-	addr = nla_get_u32(attrs[TIPC_NLA_NET_ADDR]);
+	if (attrs[TIPC_NLA_NET_NODEID]) {
+		if (!attrs[TIPC_NLA_NET_NODEID_W1])
+			return -EINVAL;
+		*w0 = nla_get_u64(attrs[TIPC_NLA_NET_NODEID]);
+		*w1 = nla_get_u64(attrs[TIPC_NLA_NET_NODEID_W1]);
+		addr = hash128to32(node_id);
+	}
 
 	if (in_own_node(net, addr))
 		return -ENOTSUPP;
-- 
2.25.1

Re: [tipc-discussion] [net-next] tipc: support 128bit node identity for peer removing

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

On 12/1/20 4:01 AM, Hoang Huu Le wrote:
> From: Hoang Le <hoa...@de...>
>
> We add the support to remove a specific node down with 128bit
> node identifier, as an alternative to legacy 32-bit node address.
>
> example:
> $tipc peer remove identiy <1001002|16777777>
>
> Signed-off-by: Hoang Le <hoa...@de...>
> ---
>   net/tipc/node.c | 21 ++++++++++++++++++---
>   1 file changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/net/tipc/node.c b/net/tipc/node.c
> index cd67b7d5169f..a7479f68a146 100644
> --- a/net/tipc/node.c
> +++ b/net/tipc/node.c
> @@ -2195,6 +2195,9 @@ int tipc_nl_peer_rm(struct sk_buff *skb, struct genl_info *info)
>   	struct tipc_net *tn = net_generic(net, tipc_net_id);
>   	struct nlattr *attrs[TIPC_NLA_NET_MAX + 1];
>   	struct tipc_node *peer, *temp_node;
> +	u8 node_id[NODE_ID_LEN];
> +	u64 *w0 = (u64 *)&node_id[0];
> +	u64 *w1 = (u64 *)&node_id[8];
>   	u32 addr;
>   	int err;
>   
> @@ -2208,10 +2211,22 @@ int tipc_nl_peer_rm(struct sk_buff *skb, struct genl_info *info)
>   	if (err)
>   		return err;
>   
> -	if (!attrs[TIPC_NLA_NET_ADDR])
> -		return -EINVAL;
> +	/* attrs[TIPC_NLA_NET_NODEID] and attrs[TIPC_NLA_NET_ADDR] are
> +	 * mutually exclusive case
s/case/cases
> +	 */
> +	if (attrs[TIPC_NLA_NET_ADDR]) {
> +		addr = nla_get_u32(attrs[TIPC_NLA_NET_ADDR]);
> +		if (!addr)
> +			return -EINVAL;
> +	}
>   
> -	addr = nla_get_u32(attrs[TIPC_NLA_NET_ADDR]);
> +	if (attrs[TIPC_NLA_NET_NODEID]) {
> +		if (!attrs[TIPC_NLA_NET_NODEID_W1])
> +			return -EINVAL;
> +		*w0 = nla_get_u64(attrs[TIPC_NLA_NET_NODEID]);
> +		*w1 = nla_get_u64(attrs[TIPC_NLA_NET_NODEID_W1]);
> +		addr = hash128to32(node_id);
> +	}
>   
>   	if (in_own_node(net, addr))
>   		return -ENOTSUPP;
Acked-by: Jon Maloy <jm...@re...>

[tipc-discussion] [net-next] tipc: support 128bit node identity for peer removing

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

From: Hoang Le <hoa...@de...>

We add the support to remove a specific node down with 128bit
node identifier, as an alternative to legacy 32-bit node address.

example:
$tipc peer remove identiy <1001002|16777777>

Signed-off-by: Hoang Le <hoa...@de...>
---
 net/tipc/node.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/net/tipc/node.c b/net/tipc/node.c
index cd67b7d5169f..a7479f68a146 100644
--- a/net/tipc/node.c
+++ b/net/tipc/node.c
@@ -2195,6 +2195,9 @@ int tipc_nl_peer_rm(struct sk_buff *skb, struct genl_info *info)
 	struct tipc_net *tn = net_generic(net, tipc_net_id);
 	struct nlattr *attrs[TIPC_NLA_NET_MAX + 1];
 	struct tipc_node *peer, *temp_node;
+	u8 node_id[NODE_ID_LEN];
+	u64 *w0 = (u64 *)&node_id[0];
+	u64 *w1 = (u64 *)&node_id[8];
 	u32 addr;
 	int err;
 
@@ -2208,10 +2211,22 @@ int tipc_nl_peer_rm(struct sk_buff *skb, struct genl_info *info)
 	if (err)
 		return err;
 
-	if (!attrs[TIPC_NLA_NET_ADDR])
-		return -EINVAL;
+	/* attrs[TIPC_NLA_NET_NODEID] and attrs[TIPC_NLA_NET_ADDR] are
+	 * mutually exclusive case
+	 */
+	if (attrs[TIPC_NLA_NET_ADDR]) {
+		addr = nla_get_u32(attrs[TIPC_NLA_NET_ADDR]);
+		if (!addr)
+			return -EINVAL;
+	}
 
-	addr = nla_get_u32(attrs[TIPC_NLA_NET_ADDR]);
+	if (attrs[TIPC_NLA_NET_NODEID]) {
+		if (!attrs[TIPC_NLA_NET_NODEID_W1])
+			return -EINVAL;
+		*w0 = nla_get_u64(attrs[TIPC_NLA_NET_NODEID]);
+		*w1 = nla_get_u64(attrs[TIPC_NLA_NET_NODEID_W1]);
+		addr = hash128to32(node_id);
+	}
 
 	if (in_own_node(net, addr))
 		return -ENOTSUPP;
-- 
2.25.1

Re: [tipc-discussion] [PATCH 01/10 net-next] net/tipc: fix tipc header files for kernel-doc

[Ying X. <yin...@wi...>]

Please see my comments about parameters marked with "FIXME":

On 11/25/20 12:20 PM, Randy Dunlap wrote:
> Fix tipc header files for adding to the networking docbook.
> 
> Remove some uses of "/**" that were not kernel-doc notation.
> 
> Fix some source formatting to eliminate Sphinx warnings.
> 
> Add missing struct member and function argument kernel-doc descriptions.
> 
> Documentation/networking/tipc:18: ../net/tipc/name_table.h:65: WARNING: Unexpected indentation.
> Documentation/networking/tipc:18: ../net/tipc/name_table.h:66: WARNING: Block quote ends without a blank line; unexpected unindent.
> 
> ../net/tipc/bearer.h:128: warning: Function parameter or member 'min_win' not described in 'tipc_media'
> ../net/tipc/bearer.h:128: warning: Function parameter or member 'max_win' not described in 'tipc_media'
> 
> ../net/tipc/bearer.h:171: warning: Function parameter or member 'min_win' not described in 'tipc_bearer'
> ../net/tipc/bearer.h:171: warning: Function parameter or member 'max_win' not described in 'tipc_bearer'
> ../net/tipc/bearer.h:171: warning: Function parameter or member 'disc' not described in 'tipc_bearer'
> ../net/tipc/bearer.h:171: warning: Function parameter or member 'up' not described in 'tipc_bearer'
> ../net/tipc/bearer.h:171: warning: Function parameter or member 'refcnt' not described in 'tipc_bearer'
> 
> ../net/tipc/name_distr.h:68: warning: Function parameter or member 'port' not described in 'distr_item'
> 
> ../net/tipc/name_table.h:111: warning: Function parameter or member 'services' not described in 'name_table'
> ../net/tipc/name_table.h:111: warning: Function parameter or member 'cluster_scope_lock' not described in 'name_table'
> ../net/tipc/name_table.h:111: warning: Function parameter or member 'rc_dests' not described in 'name_table'
> ../net/tipc/name_table.h:111: warning: Function parameter or member 'snd_nxt' not described in 'name_table'
> 
> ../net/tipc/subscr.h:67: warning: Function parameter or member 'kref' not described in 'tipc_subscription'
> ../net/tipc/subscr.h:67: warning: Function parameter or member 'net' not described in 'tipc_subscription'
> ../net/tipc/subscr.h:67: warning: Function parameter or member 'service_list' not described in 'tipc_subscription'
> ../net/tipc/subscr.h:67: warning: Function parameter or member 'conid' not described in 'tipc_subscription'
> ../net/tipc/subscr.h:67: warning: Function parameter or member 'inactive' not described in 'tipc_subscription'
> ../net/tipc/subscr.h:67: warning: Function parameter or member 'lock' not described in 'tipc_subscription'
> 
> Signed-off-by: Randy Dunlap <rd...@in...>
> Cc: Jon Maloy <jm...@re...>
> Cc: Ying Xue <yin...@wi...>
> Cc: ne...@vg...
> Cc: tip...@li...
> Cc: "David S. Miller" <da...@da...>
> Cc: Jakub Kicinski <ku...@ke...>
> ---
>  net/tipc/bearer.h     |   10 +++++++---
>  net/tipc/crypto.h     |    6 +++---
>  net/tipc/name_distr.h |    2 +-
>  net/tipc/name_table.h |    9 ++++++---
>  net/tipc/subscr.h     |   11 +++++++----
>  5 files changed, 24 insertions(+), 14 deletions(-)
> 
> --- linux-next-20201102.orig/net/tipc/bearer.h
> +++ linux-next-20201102/net/tipc/bearer.h
> @@ -93,7 +93,8 @@ struct tipc_bearer;
>   * @raw2addr: convert from raw addr format to media addr format
>   * @priority: default link (and bearer) priority
>   * @tolerance: default time (in ms) before declaring link failure
> - * @window: default window (in packets) before declaring link congestion
> + * @min_win: minimum window (in packets) before declaring link congestion
> + * @max_win: maximum window (in packets) before declaring link congestion
>   * @mtu: max packet size bearer can support for media type not dependent on
>   * underlying device MTU
>   * @type_id: TIPC media identifier
> @@ -138,12 +139,15 @@ struct tipc_media {
>   * @pt: packet type for bearer
>   * @rcu: rcu struct for tipc_bearer
>   * @priority: default link priority for bearer
> - * @window: default window size for bearer
> + * @min_win: minimum window (in packets) before declaring link congestion
> + * @max_win: maximum window (in packets) before declaring link congestion
>   * @tolerance: default link tolerance for bearer
>   * @domain: network domain to which links can be established
>   * @identity: array index of this bearer within TIPC bearer array
> - * @link_req: ptr to (optional) structure making periodic link setup requests
> + * @disc: ptr to link setup request
>   * @net_plane: network plane ('A' through 'H') currently associated with bearer
> + * @up: bearer up flag (bit 0)
> + * @refcnt: tipc_bearer reference counter
>   *
>   * Note: media-specific code is responsible for initialization of the fields
>   * indicated below when a bearer is enabled; TIPC's generic bearer code takes
> --- linux-next-20201102.orig/net/tipc/crypto.h
> +++ linux-next-20201102/net/tipc/crypto.h
> @@ -1,5 +1,5 @@
>  /* SPDX-License-Identifier: GPL-2.0 */
> -/**
> +/*
>   * net/tipc/crypto.h: Include file for TIPC crypto
>   *
>   * Copyright (c) 2019, Ericsson AB
> @@ -53,7 +53,7 @@
>  #define TIPC_AES_GCM_IV_SIZE		12
>  #define TIPC_AES_GCM_TAG_SIZE		16
>  
> -/**
> +/*
>   * TIPC crypto modes:
>   * - CLUSTER_KEY:
>   *	One single key is used for both TX & RX in all nodes in the cluster.
> @@ -69,7 +69,7 @@ enum {
>  extern int sysctl_tipc_max_tfms __read_mostly;
>  extern int sysctl_tipc_key_exchange_enabled __read_mostly;
>  
> -/**
> +/*
>   * TIPC encryption message format:
>   *
>   *     3 3 2 2 2 2 2 2 2 2 2 2 1 1 1 1 1 1 1 1 1 1 0 0 0 0 0 0 0 0 0 0
> --- linux-next-20201102.orig/net/tipc/name_distr.h
> +++ linux-next-20201102/net/tipc/name_distr.h
> @@ -46,7 +46,7 @@
>   * @type: name sequence type
>   * @lower: name sequence lower bound
>   * @upper: name sequence upper bound
> - * @ref: publishing port reference
> + * @port: publishing port reference
>   * @key: publication key
>   *
>   * ===> All fields are stored in network byte order. <===
> --- linux-next-20201102.orig/net/tipc/name_table.h
> +++ linux-next-20201102/net/tipc/name_table.h
> @@ -60,8 +60,8 @@ struct tipc_group;
>   * @key: publication key, unique across the cluster
>   * @id: publication id
>   * @binding_node: all publications from the same node which bound this one
> - * - Remote publications: in node->publ_list
> - *   Used by node/name distr to withdraw publications when node is lost
> + * - Remote publications: in node->publ_list;
> + * Used by node/name distr to withdraw publications when node is lost
>   * - Local/node scope publications: in name_table->node_scope list
>   * - Local/cluster scope publications: in name_table->cluster_scope list
>   * @binding_sock: all publications from the same socket which bound this one
> @@ -92,13 +92,16 @@ struct publication {
>  
>  /**
>   * struct name_table - table containing all existing port name publications
> - * @seq_hlist: name sequence hash lists
> + * @services: name sequence hash lists
>   * @node_scope: all local publications with node scope
>   *               - used by name_distr during re-init of name table
>   * @cluster_scope: all local publications with cluster scope
>   *               - used by name_distr to send bulk updates to new nodes
>   *               - used by name_distr during re-init of name table
> + * @cluster_scope_lock: lock for accessing @cluster_scope
>   * @local_publ_count: number of publications issued by this node
> + * @rc_dests: broadcast destinations counter (FIXME)

@rc_dests: destination node counter

> + * @snd_nxt: next sequence number to be used
>   */
>  struct name_table {
>  	struct hlist_head services[TIPC_NAMETBL_SIZE];
> --- linux-next-20201102.orig/net/tipc/subscr.h
> +++ linux-next-20201102/net/tipc/subscr.h
> @@ -47,12 +47,15 @@ struct tipc_conn;
>  
>  /**
>   * struct tipc_subscription - TIPC network topology subscription object
> - * @subscriber: pointer to its subscriber
> - * @seq: name sequence associated with subscription
> + * @kref: reference count for this subscription
> + * @net: network namespace associated with subscription
>   * @timer: timer governing subscription duration (optional)
> - * @nameseq_list: adjacent subscriptions in name sequence's subscription list
> + * @service_list: adjacent subscriptions in name sequence's subscription list
>   * @sub_list: adjacent subscriptions in subscriber's subscription list
>   * @evt: template for events generated by subscription
> + * @conid: connection ID for this subscription (FIXME)

@conid: connection identifier of topology server

> + * @inactive: true if this subscription is inactive
> + * @lock: serialize up/down and timer events
>   */
>  struct tipc_subscription {
>  	struct kref kref;
> @@ -63,7 +66,7 @@ struct tipc_subscription {
>  	struct tipc_event evt;
>  	int conid;
>  	bool inactive;
> -	spinlock_t lock; /* serialize up/down and timer events */
> +	spinlock_t lock;
>  };
>  
>  struct tipc_subscription *tipc_sub_subscribe(struct net *net,
> 

Re: [tipc-discussion] [PATCH 00/10 net-next] net/tipc: fix all kernel-doc and add TIPC networking chapter

[Ying X. <yin...@wi...>]

On 11/25/20 12:20 PM, Randy Dunlap wrote:
> 
> Question: is net/tipc/discover.c, in tipc_disc_delete() kernel-doc,
> what is the word "duest"?  Should it be changed?

The "duest" is a typo, and it should be "dest" defined as below:
struct tipc_discoverer {
        u32 bearer_id;
        struct tipc_media_addr dest; ===> "dest"
        struct net *net;
        u32 domain;
        int num_nodes;
        spinlock_t lock;
        struct sk_buff *skb;
        struct timer_list timer;
        unsigned long timer_intv;
};

Re: [tipc-discussion] [net] tipc: fix incompatiable mtu of transmission

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

s/incompatiable/incompatible


Still acked
///jon

On 11/27/20 10:14 AM, Jon Maloy wrote:
>
>
> On 11/27/20 12:01 AM, Hoang Huu Le wrote:
>> From: Hoang Le <hoa...@de...>
>>
>> In commit 682cd3cf946b6
>> ("tipc: confgiure and apply UDP bearer MTU on running links"), we
>> introduced a function to change UDP bearer MTU and applied this new 
>> value
>> across existing per-link. However, we did not apply this new MTU 
>> value at
>> node level. This lead to packet dropped at link level if its size is
>> greater than new MTU value.
>>
>> To fix this issue, we also apply this new MTU value for node level.
>>
>> Fixes: 682cd3cf946b6 ("tipc: confgiure and apply UDP bearer MTU on 
>> running links")
>> Signed-off-by: Hoang Le <hoa...@de...>
>> ---
>>   net/tipc/node.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/net/tipc/node.c b/net/tipc/node.c
>> index cd67b7d5169f..9f6975dd7873 100644
>> --- a/net/tipc/node.c
>> +++ b/net/tipc/node.c
>> @@ -2182,6 +2182,8 @@ void tipc_node_apply_property(struct net *net, 
>> struct tipc_bearer *b,
>>               else if (prop == TIPC_NLA_PROP_MTU)
>>                   tipc_link_set_mtu(e->link, b->mtu);
>>           }
>> +        /* Update MTU for node link entry */
>> +        e->mtu = tipc_link_mss(e->link);
>>           tipc_node_write_unlock(n);
>>           tipc_bearer_xmit(net, bearer_id, &xmitq, &e->maddr, NULL);
>>       }
> Acked-by: Jon Maloy <jm...@re...>
>
>
>
> _______________________________________________
> tipc-discussion mailing list
> tip...@li...
> https://lists.sourceforge.net/lists/listinfo/tipc-discussion
>

Re: [tipc-discussion] [net] tipc: fix incompatiable mtu of transmission

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

On 11/27/20 12:01 AM, Hoang Huu Le wrote:
> From: Hoang Le <hoa...@de...>
>
> In commit 682cd3cf946b6
> ("tipc: confgiure and apply UDP bearer MTU on running links"), we
> introduced a function to change UDP bearer MTU and applied this new value
> across existing per-link. However, we did not apply this new MTU value at
> node level. This lead to packet dropped at link level if its size is
> greater than new MTU value.
>
> To fix this issue, we also apply this new MTU value for node level.
>
> Fixes: 682cd3cf946b6 ("tipc: confgiure and apply UDP bearer MTU on running links")
> Signed-off-by: Hoang Le <hoa...@de...>
> ---
>   net/tipc/node.c | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/net/tipc/node.c b/net/tipc/node.c
> index cd67b7d5169f..9f6975dd7873 100644
> --- a/net/tipc/node.c
> +++ b/net/tipc/node.c
> @@ -2182,6 +2182,8 @@ void tipc_node_apply_property(struct net *net, struct tipc_bearer *b,
>   			else if (prop == TIPC_NLA_PROP_MTU)
>   				tipc_link_set_mtu(e->link, b->mtu);
>   		}
> +		/* Update MTU for node link entry */
> +		e->mtu = tipc_link_mss(e->link);
>   		tipc_node_write_unlock(n);
>   		tipc_bearer_xmit(net, bearer_id, &xmitq, &e->maddr, NULL);
>   	}
Acked-by: Jon Maloy <jm...@re...>

[tipc-discussion] [net] tipc: fix incompatiable mtu of transmission

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

From: Hoang Le <hoa...@de...>

In commit 682cd3cf946b6
("tipc: confgiure and apply UDP bearer MTU on running links"), we
introduced a function to change UDP bearer MTU and applied this new value
across existing per-link. However, we did not apply this new MTU value at
node level. This lead to packet dropped at link level if its size is
greater than new MTU value.

To fix this issue, we also apply this new MTU value for node level.

Fixes: 682cd3cf946b6 ("tipc: confgiure and apply UDP bearer MTU on running links")
Signed-off-by: Hoang Le <hoa...@de...>
---
 net/tipc/node.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/tipc/node.c b/net/tipc/node.c
index cd67b7d5169f..9f6975dd7873 100644
--- a/net/tipc/node.c
+++ b/net/tipc/node.c
@@ -2182,6 +2182,8 @@ void tipc_node_apply_property(struct net *net, struct tipc_bearer *b,
 			else if (prop == TIPC_NLA_PROP_MTU)
 				tipc_link_set_mtu(e->link, b->mtu);
 		}
+		/* Update MTU for node link entry */
+		e->mtu = tipc_link_mss(e->link);
 		tipc_node_write_unlock(n);
 		tipc_bearer_xmit(net, bearer_id, &xmitq, &e->maddr, NULL);
 	}
-- 
2.25.1

Re: [tipc-discussion] refcount warnings with TIPC in combination with a TAP device and fragmented messages

[Holger B. <hol...@hi...>]

Hi Tung Nguyen,
I said we first encountered the problem when migrating to 5.4. But the
backtrace dump and the ftrace log are done with kernel 5.10.0-rc4-17824.
So we were able to reproduce this behavior with latest  mainline kernel,
without any local modification of the kernel.

Best regards
Holger Brunck
 

> -----Original Message-----
> From: Tung Quang Nguyen <tun...@de...>
> Sent: Dienstag, 24. November 2020 11:05
> To: Holger Brunck <hol...@hi...>
> Cc: CC: Xin Long <luc...@gm...>; Matteo Ghidoni
> <mat...@hi...>; Jon Maloy
> <jm...@re...>; tip...@li...
> Subject: RE: refcount warnings with TIPC in combination with a TAP device
> and fragmented messages
> 
> 
> Hi Holger,
> 
> You said you met this stack trace after migrating to "kernel 5.4". Can you give
> us a full version ?
> 
> Thanks.
> Tung Nguyen
> 
> -----Original Message-----
> From: Holger Brunck <hol...@hi...>
> Sent: Monday, November 23, 2020 10:00 PM
> To: Jon Maloy <jm...@re...>; tip...@li...
> Cc: Tung Quang Nguyen <tun...@de...>; CC: Xin Long
> <luc...@gm...>; Matteo Ghidoni <matteo.ghidoni@hitachi-
> powergrids.com>
> Subject: Re: refcount warnings with TIPC in combination with a TAP device
> and fragmented messages
> 
> Hi Jon,
> thanks.  To add some more information I was now using ftrace to see the call
> stack within TIPC/TAP and the related skb operations to narrow this down.
> Maybe someone more familiar with the receive flow in the TIPC stack sees
> already something suspicious.
> 
> Here is the trace starting from our program (main) sending the first fragment
> to the TAP device until the first WARN_ON sequence is written out when our
> client (tipc_ping) is receiving the reassembled packet:
> 
>      main-91      ....    84.4851: tun_chr_write_iter <-vfs_write              <-- first
> fragment arrives
>      main-91      ....    84.4851:   tun_get_user <-tun_chr_write_iter
>      main-91      ....    84.4851:     sock_alloc_send_pskb <-tun_get_user
>      main-91      ....    84.4851:       skb_set_owner_w <-sock_alloc_send_pskb
> <-- refcount add
>      main-91      ....    84.4851:     skb_put <-tun_get_user
>      main-91      ....    84.4852:     skb_copy_datagram_from_iter <-tun_get_user
> <-- copy data
>      main-91      ....    84.4852: tipc_l2_rcv_msg <-__netif_receive_skb_one_core
>      main-91      ....    84.4852:   tipc_rcv <-tipc_l2_rcv_msg
>      main-91      ...2    84.4852:     tipc_link_rcv <-tipc_rcv
>      main-91      ...2    84.4852:       tipc_link_advance_transmq <-tipc_link_rcv
>      main-91      ...2    84.4852:         kfree_skb <-tipc_link_advance_transmq
>      main-91      ...2    84.4852:           skb_release_all <-kfree_skb
>      main-91      ...2    84.4852:             skb_release_head_state <-
> skb_release_all
>      main-91      ...2    84.4852:             skb_release_data <-skb_release_all
>      main-91      ...2    84.4852:               skb_free_head <-skb_release_data
>      main-91      ...2    84.4852:           kfree_skbmem <-kfree_skb
>      main-91      ...2    84.4852:         kfree_skb <-tipc_link_advance_transmq
>      main-91      ...2    84.4852:           skb_release_all <-kfree_skb
>      main-91      ...2    84.4852:             skb_release_head_state <-
> skb_release_all
>      main-91      ...2    84.4852:             skb_release_data <-skb_release_all
>      main-91      ...2    84.4852:               skb_free_head <-skb_release_data
>      main-91      ...2    84.4852:           kfree_skbmem <-kfree_skb
>      main-91      ...2    84.4852:       tipc_data_input <-tipc_link_rcv
>      main-91      ...2    84.4852:       tipc_link_input <-tipc_link_rcv
>      main-91      ...2    84.4852:         tipc_buf_append <-tipc_link_input
>      main-91      ...2    84.4852:           skb_pull <-tipc_buf_append
>      main-91      ...2    84.4853:       tipc_link_advance_backlog <-tipc_link_rcv
>      main-91      ....    84.4853:     kfree_skb <-tipc_rcv
>      main-91      ....    84.4854: tun_chr_write_iter <-vfs_write       <--- second
> fragment
>      main-91      ....    84.4854:   tun_get_user <-tun_chr_write_iter
>      main-91      ....    84.4854:     sock_alloc_send_pskb <-tun_get_user
>      main-91      ....    84.4854:       skb_set_owner_w <-sock_alloc_send_pskb
>      main-91      ....    84.4854:     skb_put <-tun_get_user
>      main-91      ....    84.4854:     skb_copy_datagram_from_iter <-tun_get_user
>      main-91      ....    84.4854: tipc_l2_rcv_msg <-__netif_receive_skb_one_core
>      main-91      ....    84.4854:   tipc_rcv <-tipc_l2_rcv_msg
>      main-91      ...2    84.4854:     tipc_link_advance_transmq <-tipc_link_rcv
>      main-91      ...2    84.4854:     tipc_data_input <-tipc_link_rcv
>      main-91      ...2    84.4854:       tipc_link_input <-tipc_link_rcv
>      main-91      ...2    84.4854:         tipc_buf_append <-tipc_link_input
>      main-91      ...2    84.4854:           skb_pull <-tipc_buf_append
>      main-91      ...2    84.4854:           skb_try_coalesce <-tipc_buf_append
>      main-91      ...2    84.4854:         tipc_data_input <-tipc_link_input
>      main-91      ...2    84.4854:       skb_queue_tail <-tipc_data_input
>      main-91      ....    84.4854:     tipc_sk_rcv <-tipc_rcv
>      main-91      ....    84.4854:       tipc_sk_lookup <-tipc_sk_rcv
>      main-91      ...1    84.4854:       tipc_sk_filter_rcv <-tipc_sk_rcv
>      main-91      ...1    84.4854:         sock_wfree <-tipc_sk_filter_rcv
>      main-91      ...1    84.4854:           tun_sock_write_space <-sock_wfree
>      main-91      ...1    84.4854:         tipc_data_ready <-tipc_sk_filter_rcv
>      main-91      ....    84.4855:         tipc_node_distr_xmit <-tipc_sk_rcv
>      main-91      ....    84.4855:     kfree_skb <-tipc_rcv
> tipc_ping-97      ....    84.4855: tipc_poll <-sock_poll                <-- tipc_ping
> receives the packet
> tipc_ping-97      ....    84.4859: tipc_recvmsg <-__sys_recvfrom
> tipc_ping-97      ....    84.4859:   tipc_wait_for_rcvmsg <-tipc_recvmsg
> tipc_ping-97      ....    84.4859:   tipc_sk_set_orig_addr <-tipc_recvmsg
>                                                    skb_copy_datagram_msg
> tipc_ping-97      ....    84.4859:       skb_copy_datagram_iter <-tipc_recvmsg
> tipc_ping-97      ....    84.4859:   kfree_skb <-tsk_advance_rx_queue
> tipc_ping-97      ....    84.4859:     skb_release_all <-kfree_skb
> tipc_ping-97      ....    84.4859:       skb_release_head_state <-skb_release_all
> tipc_ping-97      ....    84.4859:       skb_release_data <-skb_release_all
> tipc_ping-97      ....    84.4859:         kfree_skb <-skb_release_data
> tipc_ping-97      ....    84.4859:           skb_release_all <-kfree_skb
> tipc_ping-97      ....    84.4859:             skb_release_head_state <-
> skb_release_all
> tipc_ping-97      ....    84.4859:             sock_wfree <-skb_release_head_state
> tipc_ping-97      ....    84.4859:               warn_slowpath_fmt <-
> refcount_warn_saturate  <--- first WARN_ON dump
> tipc_ping-97      ....    84.4897:               __warn <-warn_slowpath_fmt
> tipc_ping-97      ....    84.5227:               dump_backtrace <-show_stack
> 
> 
> Best regards
> Holger Brunck
> 
> 
> From: Jon Maloy <jm...@re...>
> Sent: 23 November 2020 15:25
> To: Holger Brunck <hol...@hi...>; tipc-
> dis...@li... <tip...@li...>
> Cc: CC: Tung Nguyen <tun...@de...>; CC: Xin Long
> <luc...@gm...>; Matteo Ghidoni <matteo.ghidoni@hitachi-
> powergrids.com>
> Subject: Re: refcount warnings with TIPC in combination with a TAP device
> and fragmented messages
> 
> 
> 
> 
> Hi Holger,
> We will look into this. It is a new setup for us, so it has to be prepared first.
> 
> ///jon
> 
> 
> On 11/19/20 9:33 AM, Holger Brunck wrote:
> > Hi all,
> > we have currently problems with TIPC in combination with a TAP
> > interface and fragmented messages. We have a Kirkwood board getting
> > TIPC ethernet packets from a third party stack in user space which we
> > fill into a TAP device to pass this packets to the kernel. On the
> > remote side we have another board with a physical ethernet device.
> > After migrating to kernel 5.4 we saw kernel crashes as soon as we send
> fragmented TIPC messages via this link. Before the kernel crash we see
> "refcount_t: underflow; use-after-free"
> > warnings coming either from the TIPC or the driver/net/tun.
> > For smaller packets everything work fine. To track this down we wrote
> > a small test program which receives raw ethernet packets in user space
> > and send them back to kernel space via a TAP interface, to be able to
> reproduce it with a mainline kernel only.
> > So instead of a link e.g. eth0:eth0 we have a link from eth0:tap0.
> >
> > I now tested this on latest mainline kernel and still see warnings
> > coming from the kernel, but not the crash.
> > So in the testcase we have TIPC server running on a remote board
> > having a ethernet bearer on a interface. The ethernet interface is
> > connected to the ethernet interface of the kirkwood board. The incoming
> messages are passed to user space and filled into the TAP device.
> > So we have a TIPC link between tap0 and eth0. If I send now back and
> > forth  TIPC connectionless messages to the server via the TAP device
> > the kernel print the following messages as soon as the packets are
> > larger then the MTU and TIPC need to reassemble the fragmented
> messages coming from the TAP driver:
> >
> > root@km_kirkwood:~#
> > ------------[ cut here ]------------
> > WARNING: CPU: 0 PID: 94 at lib/refcount.c:28 sock_wfree+0xd8/0xe0
> > refcount_t: underflow; use-after-free.
> > Modules linked in:
> > CPU: 0 PID: 94 Comm: tipc_ping Tainted: G        W         5.10.0-rc4-17824-
> g0fa8ee0d9ab9 #112
> > Hardware name: Marvell Kirkwood (Flattened Device Tree) [<c000f924>]
> > (unwind_backtrace) from [<c000c980>] (show_stack+0x10/0x14)
> > [<c000c980>] (show_stack) from [<c001c27c>] (__warn+0xe8/0xfc)
> > [<c001c27c>] (__warn) from [<c001c2fc>] (warn_slowpath_fmt+0x6c/0xb8)
> > [<c001c2fc>] (warn_slowpath_fmt) from [<c0564b78>]
> > (sock_wfree+0xd8/0xe0) [<c0564b78>] (sock_wfree) from [<c056b730>]
> > (skb_release_all+0xc/0x28) [<c056b730>] (skb_release_all) from
> > [<c056b784>] (kfree_skb+0x20/0x6c) [<c056b784>] (kfree_skb) from
> > [<c056be44>] (skb_release_data+0xc0/0x140) [<c056be44>]
> > (skb_release_data) from [<c056b784>] (kfree_skb+0x20/0x6c)
> > [<c056b784>] (kfree_skb) from [<c0720ea4>] (tipc_recvmsg+0x270/0x3c4)
> > [<c0720ea4>] (tipc_recvmsg) from [<c0560428>]
> > (__sys_recvfrom+0xb0/0x128) [<c0560428>] (__sys_recvfrom) from
> > [<c05604bc>] (sys_recv+0x18/0x20) [<c05604bc>] (sys_recv) from
> [<c0008240>] (ret_fast_syscall+0x0/0x50) Exception stack(0xc1615fa8 to
> 0xc1615ff0)
> > 5fa0:                   00000001 00000000 00000003 00025010 0000fa00
> 00000000
> > 5fc0: 00000001 00000000 00010888 00000123 00000000 00000000
> 49cb0000
> > be939bec
> > 5fe0: 00000000 be93997c 00011114 49d9666c ---[ end trace
> > e7c3a1366c544d79 ]--- ------------[ cut here ]------------
> > WARNING: CPU: 0 PID: 74 at lib/refcount.c:22
> > sock_alloc_send_pskb+0x240/0x258
> > refcount_t: saturated; leaking memory.
> > Modules linked in:
> > CPU: 0 PID: 74 Comm: main Tainted: G        W         5.10.0-rc4-17824-
> g0fa8ee0d9ab9 #112
> > Hardware name: Marvell Kirkwood (Flattened Device Tree) [<c000f924>]
> > (unwind_backtrace) from [<c000c980>] (show_stack+0x10/0x14)
> > [<c000c980>] (show_stack) from [<c001c27c>] (__warn+0xe8/0xfc)
> > [<c001c27c>] (__warn) from [<c001c2fc>] (warn_slowpath_fmt+0x6c/0xb8)
> > [<c001c2fc>] (warn_slowpath_fmt) from [<c0563b40>]
> > (sock_alloc_send_pskb+0x240/0x258)
> > [<c0563b40>] (sock_alloc_send_pskb) from [<c043d500>]
> > (tun_get_user+0x224/0x13f8) [<c043d500>] (tun_get_user) from
> > [<c043e718>] (tun_chr_write_iter+0x44/0x78) [<c043e718>]
> > (tun_chr_write_iter) from [<c012f1bc>] (vfs_write+0x1fc/0x448)
> > [<c012f1bc>] (vfs_write) from [<c012f52c>] (ksys_write+0x54/0xcc)
> > [<c012f52c>] (ksys_write) from [<c0008240>] (ret_fast_syscall+0x0/0x50)
> Exception stack(0xc158bfa8 to 0xc158bff0)
> > bfa0:                   00011244 00000000 00000004 00022098 000005c2
> 00000004
> > bfc0: 00011244 00000000 000108bc 00000004 00000000 00000000
> 49cb0000
> > becf3c94
> > bfe0: 00000000 becf3bb0 49e21ce0 49e223e4 ---[ end trace
> > e7c3a1366c544d7a ]---
> >
> > Kernel version:
> > root@km_kirkwood:~# cat /proc/version
> > Linux version 5.10.0-rc4-17824-g0fa8ee0d9ab9
> >
> > If I simply use ethernet bearer on both sides everything works fine,
> > so I need the TAP interface to be involved to see the problem.
> >
> > It seems that we have a problem in the skb_buf allocation or handling
> > between the TAP driver and the TIPC layer as soon it comes to
> fragmentation.
> > Does anyone have an idea what could cause the problem and how to track
> > this down? Any help is appreciated.
> >
> > Best regards
> > Holger Brunck

Re: [tipc-discussion] refcount warnings with TIPC in combination with a TAP device and fragmented messages

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

Hi Holger,

You said you met this stack trace after migrating to "kernel 5.4". Can you give us a full version ?

Thanks.
Tung Nguyen

-----Original Message-----
From: Holger Brunck <hol...@hi...> 
Sent: Monday, November 23, 2020 10:00 PM
To: Jon Maloy <jm...@re...>; tip...@li...
Cc: Tung Quang Nguyen <tun...@de...>; CC: Xin Long <luc...@gm...>; Matteo Ghidoni <mat...@hi...>
Subject: Re: refcount warnings with TIPC in combination with a TAP device and fragmented messages

Hi Jon,
thanks.  To add some more information I was now using ftrace to see the call stack within TIPC/TAP and the related
skb operations to narrow this down. Maybe someone more familiar with the receive flow in the TIPC stack sees already something suspicious. 

Here is the trace starting from our program (main) sending the first fragment to the TAP device until the first WARN_ON sequence
is written out when our client (tipc_ping) is receiving the reassembled packet:

     main-91      ....    84.4851: tun_chr_write_iter <-vfs_write              <-- first fragment arrives
     main-91      ....    84.4851:   tun_get_user <-tun_chr_write_iter
     main-91      ....    84.4851:     sock_alloc_send_pskb <-tun_get_user
     main-91      ....    84.4851:       skb_set_owner_w <-sock_alloc_send_pskb     <-- refcount add
     main-91      ....    84.4851:     skb_put <-tun_get_user
     main-91      ....    84.4852:     skb_copy_datagram_from_iter <-tun_get_user  <-- copy data
     main-91      ....    84.4852: tipc_l2_rcv_msg <-__netif_receive_skb_one_core
     main-91      ....    84.4852:   tipc_rcv <-tipc_l2_rcv_msg
     main-91      ...2    84.4852:     tipc_link_rcv <-tipc_rcv
     main-91      ...2    84.4852:       tipc_link_advance_transmq <-tipc_link_rcv
     main-91      ...2    84.4852:         kfree_skb <-tipc_link_advance_transmq
     main-91      ...2    84.4852:           skb_release_all <-kfree_skb
     main-91      ...2    84.4852:             skb_release_head_state <-skb_release_all
     main-91      ...2    84.4852:             skb_release_data <-skb_release_all
     main-91      ...2    84.4852:               skb_free_head <-skb_release_data
     main-91      ...2    84.4852:           kfree_skbmem <-kfree_skb
     main-91      ...2    84.4852:         kfree_skb <-tipc_link_advance_transmq
     main-91      ...2    84.4852:           skb_release_all <-kfree_skb
     main-91      ...2    84.4852:             skb_release_head_state <-skb_release_all
     main-91      ...2    84.4852:             skb_release_data <-skb_release_all
     main-91      ...2    84.4852:               skb_free_head <-skb_release_data
     main-91      ...2    84.4852:           kfree_skbmem <-kfree_skb
     main-91      ...2    84.4852:       tipc_data_input <-tipc_link_rcv
     main-91      ...2    84.4852:       tipc_link_input <-tipc_link_rcv
     main-91      ...2    84.4852:         tipc_buf_append <-tipc_link_input
     main-91      ...2    84.4852:           skb_pull <-tipc_buf_append
     main-91      ...2    84.4853:       tipc_link_advance_backlog <-tipc_link_rcv
     main-91      ....    84.4853:     kfree_skb <-tipc_rcv
     main-91      ....    84.4854: tun_chr_write_iter <-vfs_write       <--- second fragment
     main-91      ....    84.4854:   tun_get_user <-tun_chr_write_iter
     main-91      ....    84.4854:     sock_alloc_send_pskb <-tun_get_user
     main-91      ....    84.4854:       skb_set_owner_w <-sock_alloc_send_pskb
     main-91      ....    84.4854:     skb_put <-tun_get_user
     main-91      ....    84.4854:     skb_copy_datagram_from_iter <-tun_get_user
     main-91      ....    84.4854: tipc_l2_rcv_msg <-__netif_receive_skb_one_core
     main-91      ....    84.4854:   tipc_rcv <-tipc_l2_rcv_msg
     main-91      ...2    84.4854:     tipc_link_advance_transmq <-tipc_link_rcv
     main-91      ...2    84.4854:     tipc_data_input <-tipc_link_rcv
     main-91      ...2    84.4854:       tipc_link_input <-tipc_link_rcv
     main-91      ...2    84.4854:         tipc_buf_append <-tipc_link_input
     main-91      ...2    84.4854:           skb_pull <-tipc_buf_append
     main-91      ...2    84.4854:           skb_try_coalesce <-tipc_buf_append
     main-91      ...2    84.4854:         tipc_data_input <-tipc_link_input
     main-91      ...2    84.4854:       skb_queue_tail <-tipc_data_input
     main-91      ....    84.4854:     tipc_sk_rcv <-tipc_rcv
     main-91      ....    84.4854:       tipc_sk_lookup <-tipc_sk_rcv
     main-91      ...1    84.4854:       tipc_sk_filter_rcv <-tipc_sk_rcv
     main-91      ...1    84.4854:         sock_wfree <-tipc_sk_filter_rcv
     main-91      ...1    84.4854:           tun_sock_write_space <-sock_wfree
     main-91      ...1    84.4854:         tipc_data_ready <-tipc_sk_filter_rcv
     main-91      ....    84.4855:         tipc_node_distr_xmit <-tipc_sk_rcv
     main-91      ....    84.4855:     kfree_skb <-tipc_rcv
tipc_ping-97      ....    84.4855: tipc_poll <-sock_poll                <-- tipc_ping receives the packet
tipc_ping-97      ....    84.4859: tipc_recvmsg <-__sys_recvfrom
tipc_ping-97      ....    84.4859:   tipc_wait_for_rcvmsg <-tipc_recvmsg
tipc_ping-97      ....    84.4859:   tipc_sk_set_orig_addr <-tipc_recvmsg
                                                   skb_copy_datagram_msg
tipc_ping-97      ....    84.4859:       skb_copy_datagram_iter <-tipc_recvmsg
tipc_ping-97      ....    84.4859:   kfree_skb <-tsk_advance_rx_queue
tipc_ping-97      ....    84.4859:     skb_release_all <-kfree_skb
tipc_ping-97      ....    84.4859:       skb_release_head_state <-skb_release_all
tipc_ping-97      ....    84.4859:       skb_release_data <-skb_release_all
tipc_ping-97      ....    84.4859:         kfree_skb <-skb_release_data
tipc_ping-97      ....    84.4859:           skb_release_all <-kfree_skb
tipc_ping-97      ....    84.4859:             skb_release_head_state <-skb_release_all
tipc_ping-97      ....    84.4859:             sock_wfree <-skb_release_head_state
tipc_ping-97      ....    84.4859:               warn_slowpath_fmt <-refcount_warn_saturate  <--- first WARN_ON dump
tipc_ping-97      ....    84.4897:               __warn <-warn_slowpath_fmt
tipc_ping-97      ....    84.5227:               dump_backtrace <-show_stack

  
Best regards
Holger Brunck


From: Jon Maloy <jm...@re...>
Sent: 23 November 2020 15:25
To: Holger Brunck <hol...@hi...>; tip...@li... <tip...@li...>
Cc: CC: Tung Nguyen <tun...@de...>; CC: Xin Long <luc...@gm...>; Matteo Ghidoni <mat...@hi...>
Subject: Re: refcount warnings with TIPC in combination with a TAP device and fragmented messages 
 



Hi Holger,
We will look into this. It is a new setup for us, so it has to be
prepared first.

///jon


On 11/19/20 9:33 AM, Holger Brunck wrote:
> Hi all,
> we have currently problems with TIPC in combination with a TAP interface and fragmented
> messages. We have a Kirkwood board getting TIPC ethernet packets from a third party stack
> in user space which we fill into a TAP device to pass this packets to the kernel. On the
> remote side we have another board with a physical ethernet device. After migrating to
> kernel 5.4 we saw kernel crashes as soon as we send fragmented TIPC messages via
> this link. Before the kernel crash we see "refcount_t: underflow; use-after-free"
> warnings coming either from the TIPC or the driver/net/tun.
> For smaller packets everything work fine. To track this down we wrote a small
> test program which receives raw ethernet packets in user space and send them back to
> kernel space via a TAP interface, to be able to reproduce it with a mainline kernel only.
> So instead of a link e.g. eth0:eth0 we have a link from eth0:tap0.
>
> I now tested this on latest mainline kernel and still see warnings coming from the kernel, but
> not the crash.
> So in the testcase we have TIPC server running on a remote board having a ethernet bearer
> on a interface. The ethernet interface is connected to the ethernet interface of the
> kirkwood board. The incoming messages are passed to user space and filled into the TAP device.
> So we have a TIPC link between tap0 and eth0. If I send now back and forth  TIPC connectionless
> messages to the server via the TAP device the kernel print the following messages as soon as
> the packets are larger then the MTU and TIPC need to reassemble the fragmented messages coming
> from the TAP driver:
>
> root@km_kirkwood:~#
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 94 at lib/refcount.c:28 sock_wfree+0xd8/0xe0
> refcount_t: underflow; use-after-free.
> Modules linked in:
> CPU: 0 PID: 94 Comm: tipc_ping Tainted: G        W         5.10.0-rc4-17824-g0fa8ee0d9ab9 #112
> Hardware name: Marvell Kirkwood (Flattened Device Tree)
> [<c000f924>] (unwind_backtrace) from [<c000c980>] (show_stack+0x10/0x14)
> [<c000c980>] (show_stack) from [<c001c27c>] (__warn+0xe8/0xfc)
> [<c001c27c>] (__warn) from [<c001c2fc>] (warn_slowpath_fmt+0x6c/0xb8)
> [<c001c2fc>] (warn_slowpath_fmt) from [<c0564b78>] (sock_wfree+0xd8/0xe0)
> [<c0564b78>] (sock_wfree) from [<c056b730>] (skb_release_all+0xc/0x28)
> [<c056b730>] (skb_release_all) from [<c056b784>] (kfree_skb+0x20/0x6c)
> [<c056b784>] (kfree_skb) from [<c056be44>] (skb_release_data+0xc0/0x140)
> [<c056be44>] (skb_release_data) from [<c056b784>] (kfree_skb+0x20/0x6c)
> [<c056b784>] (kfree_skb) from [<c0720ea4>] (tipc_recvmsg+0x270/0x3c4)
> [<c0720ea4>] (tipc_recvmsg) from [<c0560428>] (__sys_recvfrom+0xb0/0x128)
> [<c0560428>] (__sys_recvfrom) from [<c05604bc>] (sys_recv+0x18/0x20)
> [<c05604bc>] (sys_recv) from [<c0008240>] (ret_fast_syscall+0x0/0x50)
> Exception stack(0xc1615fa8 to 0xc1615ff0)
> 5fa0:                   00000001 00000000 00000003 00025010 0000fa00 00000000
> 5fc0: 00000001 00000000 00010888 00000123 00000000 00000000 49cb0000 be939bec
> 5fe0: 00000000 be93997c 00011114 49d9666c
> ---[ end trace e7c3a1366c544d79 ]---
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 74 at lib/refcount.c:22 sock_alloc_send_pskb+0x240/0x258
> refcount_t: saturated; leaking memory.
> Modules linked in:
> CPU: 0 PID: 74 Comm: main Tainted: G        W         5.10.0-rc4-17824-g0fa8ee0d9ab9 #112
> Hardware name: Marvell Kirkwood (Flattened Device Tree)
> [<c000f924>] (unwind_backtrace) from [<c000c980>] (show_stack+0x10/0x14)
> [<c000c980>] (show_stack) from [<c001c27c>] (__warn+0xe8/0xfc)
> [<c001c27c>] (__warn) from [<c001c2fc>] (warn_slowpath_fmt+0x6c/0xb8)
> [<c001c2fc>] (warn_slowpath_fmt) from [<c0563b40>] (sock_alloc_send_pskb+0x240/0x258)
> [<c0563b40>] (sock_alloc_send_pskb) from [<c043d500>] (tun_get_user+0x224/0x13f8)
> [<c043d500>] (tun_get_user) from [<c043e718>] (tun_chr_write_iter+0x44/0x78)
> [<c043e718>] (tun_chr_write_iter) from [<c012f1bc>] (vfs_write+0x1fc/0x448)
> [<c012f1bc>] (vfs_write) from [<c012f52c>] (ksys_write+0x54/0xcc)
> [<c012f52c>] (ksys_write) from [<c0008240>] (ret_fast_syscall+0x0/0x50)
> Exception stack(0xc158bfa8 to 0xc158bff0)
> bfa0:                   00011244 00000000 00000004 00022098 000005c2 00000004
> bfc0: 00011244 00000000 000108bc 00000004 00000000 00000000 49cb0000 becf3c94
> bfe0: 00000000 becf3bb0 49e21ce0 49e223e4
> ---[ end trace e7c3a1366c544d7a ]---
>
> Kernel version:
> root@km_kirkwood:~# cat /proc/version
> Linux version 5.10.0-rc4-17824-g0fa8ee0d9ab9
>
> If I simply use ethernet bearer on both sides everything works fine, so I need
> the TAP interface to be involved to see the problem.
>
> It seems that we have a problem in the skb_buf allocation or handling between
> the TAP driver and the TIPC layer as soon it comes to fragmentation.
> Does anyone have an idea what could cause the problem and how to track this
> down? Any help is appreciated.
>
> Best regards
> Holger Brunck

Re: [tipc-discussion] refcount warnings with TIPC in combination with a TAP device and fragmented messages

[Holger B. <hol...@hi...>]

Hi Jon,
thanks.  To add some more information I was now using ftrace to see the call stack within TIPC/TAP and the related
skb operations to narrow this down. Maybe someone more familiar with the receive flow in the TIPC stack sees already something suspicious. 

Here is the trace starting from our program (main) sending the first fragment to the TAP device until the first WARN_ON sequence
is written out when our client (tipc_ping) is receiving the reassembled packet:

     main-91      ....    84.4851: tun_chr_write_iter <-vfs_write              <-- first fragment arrives
     main-91      ....    84.4851:   tun_get_user <-tun_chr_write_iter
     main-91      ....    84.4851:     sock_alloc_send_pskb <-tun_get_user
     main-91      ....    84.4851:       skb_set_owner_w <-sock_alloc_send_pskb     <-- refcount add
     main-91      ....    84.4851:     skb_put <-tun_get_user
     main-91      ....    84.4852:     skb_copy_datagram_from_iter <-tun_get_user  <-- copy data
     main-91      ....    84.4852: tipc_l2_rcv_msg <-__netif_receive_skb_one_core
     main-91      ....    84.4852:   tipc_rcv <-tipc_l2_rcv_msg
     main-91      ...2    84.4852:     tipc_link_rcv <-tipc_rcv
     main-91      ...2    84.4852:       tipc_link_advance_transmq <-tipc_link_rcv
     main-91      ...2    84.4852:         kfree_skb <-tipc_link_advance_transmq
     main-91      ...2    84.4852:           skb_release_all <-kfree_skb
     main-91      ...2    84.4852:             skb_release_head_state <-skb_release_all
     main-91      ...2    84.4852:             skb_release_data <-skb_release_all
     main-91      ...2    84.4852:               skb_free_head <-skb_release_data
     main-91      ...2    84.4852:           kfree_skbmem <-kfree_skb
     main-91      ...2    84.4852:         kfree_skb <-tipc_link_advance_transmq
     main-91      ...2    84.4852:           skb_release_all <-kfree_skb
     main-91      ...2    84.4852:             skb_release_head_state <-skb_release_all
     main-91      ...2    84.4852:             skb_release_data <-skb_release_all
     main-91      ...2    84.4852:               skb_free_head <-skb_release_data
     main-91      ...2    84.4852:           kfree_skbmem <-kfree_skb
     main-91      ...2    84.4852:       tipc_data_input <-tipc_link_rcv
     main-91      ...2    84.4852:       tipc_link_input <-tipc_link_rcv
     main-91      ...2    84.4852:         tipc_buf_append <-tipc_link_input
     main-91      ...2    84.4852:           skb_pull <-tipc_buf_append
     main-91      ...2    84.4853:       tipc_link_advance_backlog <-tipc_link_rcv
     main-91      ....    84.4853:     kfree_skb <-tipc_rcv
     main-91      ....    84.4854: tun_chr_write_iter <-vfs_write       <--- second fragment
     main-91      ....    84.4854:   tun_get_user <-tun_chr_write_iter
     main-91      ....    84.4854:     sock_alloc_send_pskb <-tun_get_user
     main-91      ....    84.4854:       skb_set_owner_w <-sock_alloc_send_pskb
     main-91      ....    84.4854:     skb_put <-tun_get_user
     main-91      ....    84.4854:     skb_copy_datagram_from_iter <-tun_get_user
     main-91      ....    84.4854: tipc_l2_rcv_msg <-__netif_receive_skb_one_core
     main-91      ....    84.4854:   tipc_rcv <-tipc_l2_rcv_msg
     main-91      ...2    84.4854:     tipc_link_advance_transmq <-tipc_link_rcv
     main-91      ...2    84.4854:     tipc_data_input <-tipc_link_rcv
     main-91      ...2    84.4854:       tipc_link_input <-tipc_link_rcv
     main-91      ...2    84.4854:         tipc_buf_append <-tipc_link_input
     main-91      ...2    84.4854:           skb_pull <-tipc_buf_append
     main-91      ...2    84.4854:           skb_try_coalesce <-tipc_buf_append
     main-91      ...2    84.4854:         tipc_data_input <-tipc_link_input
     main-91      ...2    84.4854:       skb_queue_tail <-tipc_data_input
     main-91      ....    84.4854:     tipc_sk_rcv <-tipc_rcv
     main-91      ....    84.4854:       tipc_sk_lookup <-tipc_sk_rcv
     main-91      ...1    84.4854:       tipc_sk_filter_rcv <-tipc_sk_rcv
     main-91      ...1    84.4854:         sock_wfree <-tipc_sk_filter_rcv
     main-91      ...1    84.4854:           tun_sock_write_space <-sock_wfree
     main-91      ...1    84.4854:         tipc_data_ready <-tipc_sk_filter_rcv
     main-91      ....    84.4855:         tipc_node_distr_xmit <-tipc_sk_rcv
     main-91      ....    84.4855:     kfree_skb <-tipc_rcv
tipc_ping-97      ....    84.4855: tipc_poll <-sock_poll                <-- tipc_ping receives the packet
tipc_ping-97      ....    84.4859: tipc_recvmsg <-__sys_recvfrom
tipc_ping-97      ....    84.4859:   tipc_wait_for_rcvmsg <-tipc_recvmsg
tipc_ping-97      ....    84.4859:   tipc_sk_set_orig_addr <-tipc_recvmsg
                                                   skb_copy_datagram_msg
tipc_ping-97      ....    84.4859:       skb_copy_datagram_iter <-tipc_recvmsg
tipc_ping-97      ....    84.4859:   kfree_skb <-tsk_advance_rx_queue
tipc_ping-97      ....    84.4859:     skb_release_all <-kfree_skb
tipc_ping-97      ....    84.4859:       skb_release_head_state <-skb_release_all
tipc_ping-97      ....    84.4859:       skb_release_data <-skb_release_all
tipc_ping-97      ....    84.4859:         kfree_skb <-skb_release_data
tipc_ping-97      ....    84.4859:           skb_release_all <-kfree_skb
tipc_ping-97      ....    84.4859:             skb_release_head_state <-skb_release_all
tipc_ping-97      ....    84.4859:             sock_wfree <-skb_release_head_state
tipc_ping-97      ....    84.4859:               warn_slowpath_fmt <-refcount_warn_saturate  <--- first WARN_ON dump
tipc_ping-97      ....    84.4897:               __warn <-warn_slowpath_fmt
tipc_ping-97      ....    84.5227:               dump_backtrace <-show_stack

  
Best regards
Holger Brunck


From: Jon Maloy <jm...@re...>
Sent: 23 November 2020 15:25
To: Holger Brunck <hol...@hi...>; tip...@li... <tip...@li...>
Cc: CC: Tung Nguyen <tun...@de...>; CC: Xin Long <luc...@gm...>; Matteo Ghidoni <mat...@hi...>
Subject: Re: refcount warnings with TIPC in combination with a TAP device and fragmented messages 
 



Hi Holger,
We will look into this. It is a new setup for us, so it has to be
prepared first.

///jon


On 11/19/20 9:33 AM, Holger Brunck wrote:
> Hi all,
> we have currently problems with TIPC in combination with a TAP interface and fragmented
> messages. We have a Kirkwood board getting TIPC ethernet packets from a third party stack
> in user space which we fill into a TAP device to pass this packets to the kernel. On the
> remote side we have another board with a physical ethernet device. After migrating to
> kernel 5.4 we saw kernel crashes as soon as we send fragmented TIPC messages via
> this link. Before the kernel crash we see "refcount_t: underflow; use-after-free"
> warnings coming either from the TIPC or the driver/net/tun.
> For smaller packets everything work fine. To track this down we wrote a small
> test program which receives raw ethernet packets in user space and send them back to
> kernel space via a TAP interface, to be able to reproduce it with a mainline kernel only.
> So instead of a link e.g. eth0:eth0 we have a link from eth0:tap0.
>
> I now tested this on latest mainline kernel and still see warnings coming from the kernel, but
> not the crash.
> So in the testcase we have TIPC server running on a remote board having a ethernet bearer
> on a interface. The ethernet interface is connected to the ethernet interface of the
> kirkwood board. The incoming messages are passed to user space and filled into the TAP device.
> So we have a TIPC link between tap0 and eth0. If I send now back and forth  TIPC connectionless
> messages to the server via the TAP device the kernel print the following messages as soon as
> the packets are larger then the MTU and TIPC need to reassemble the fragmented messages coming
> from the TAP driver:
>
> root@km_kirkwood:~#
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 94 at lib/refcount.c:28 sock_wfree+0xd8/0xe0
> refcount_t: underflow; use-after-free.
> Modules linked in:
> CPU: 0 PID: 94 Comm: tipc_ping Tainted: G        W         5.10.0-rc4-17824-g0fa8ee0d9ab9 #112
> Hardware name: Marvell Kirkwood (Flattened Device Tree)
> [<c000f924>] (unwind_backtrace) from [<c000c980>] (show_stack+0x10/0x14)
> [<c000c980>] (show_stack) from [<c001c27c>] (__warn+0xe8/0xfc)
> [<c001c27c>] (__warn) from [<c001c2fc>] (warn_slowpath_fmt+0x6c/0xb8)
> [<c001c2fc>] (warn_slowpath_fmt) from [<c0564b78>] (sock_wfree+0xd8/0xe0)
> [<c0564b78>] (sock_wfree) from [<c056b730>] (skb_release_all+0xc/0x28)
> [<c056b730>] (skb_release_all) from [<c056b784>] (kfree_skb+0x20/0x6c)
> [<c056b784>] (kfree_skb) from [<c056be44>] (skb_release_data+0xc0/0x140)
> [<c056be44>] (skb_release_data) from [<c056b784>] (kfree_skb+0x20/0x6c)
> [<c056b784>] (kfree_skb) from [<c0720ea4>] (tipc_recvmsg+0x270/0x3c4)
> [<c0720ea4>] (tipc_recvmsg) from [<c0560428>] (__sys_recvfrom+0xb0/0x128)
> [<c0560428>] (__sys_recvfrom) from [<c05604bc>] (sys_recv+0x18/0x20)
> [<c05604bc>] (sys_recv) from [<c0008240>] (ret_fast_syscall+0x0/0x50)
> Exception stack(0xc1615fa8 to 0xc1615ff0)
> 5fa0:                   00000001 00000000 00000003 00025010 0000fa00 00000000
> 5fc0: 00000001 00000000 00010888 00000123 00000000 00000000 49cb0000 be939bec
> 5fe0: 00000000 be93997c 00011114 49d9666c
> ---[ end trace e7c3a1366c544d79 ]---
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 74 at lib/refcount.c:22 sock_alloc_send_pskb+0x240/0x258
> refcount_t: saturated; leaking memory.
> Modules linked in:
> CPU: 0 PID: 74 Comm: main Tainted: G        W         5.10.0-rc4-17824-g0fa8ee0d9ab9 #112
> Hardware name: Marvell Kirkwood (Flattened Device Tree)
> [<c000f924>] (unwind_backtrace) from [<c000c980>] (show_stack+0x10/0x14)
> [<c000c980>] (show_stack) from [<c001c27c>] (__warn+0xe8/0xfc)
> [<c001c27c>] (__warn) from [<c001c2fc>] (warn_slowpath_fmt+0x6c/0xb8)
> [<c001c2fc>] (warn_slowpath_fmt) from [<c0563b40>] (sock_alloc_send_pskb+0x240/0x258)
> [<c0563b40>] (sock_alloc_send_pskb) from [<c043d500>] (tun_get_user+0x224/0x13f8)
> [<c043d500>] (tun_get_user) from [<c043e718>] (tun_chr_write_iter+0x44/0x78)
> [<c043e718>] (tun_chr_write_iter) from [<c012f1bc>] (vfs_write+0x1fc/0x448)
> [<c012f1bc>] (vfs_write) from [<c012f52c>] (ksys_write+0x54/0xcc)
> [<c012f52c>] (ksys_write) from [<c0008240>] (ret_fast_syscall+0x0/0x50)
> Exception stack(0xc158bfa8 to 0xc158bff0)
> bfa0:                   00011244 00000000 00000004 00022098 000005c2 00000004
> bfc0: 00011244 00000000 000108bc 00000004 00000000 00000000 49cb0000 becf3c94
> bfe0: 00000000 becf3bb0 49e21ce0 49e223e4
> ---[ end trace e7c3a1366c544d7a ]---
>
> Kernel version:
> root@km_kirkwood:~# cat /proc/version
> Linux version 5.10.0-rc4-17824-g0fa8ee0d9ab9
>
> If I simply use ethernet bearer on both sides everything works fine, so I need
> the TAP interface to be involved to see the problem.
>
> It seems that we have a problem in the skb_buf allocation or handling between
> the TAP driver and the TIPC layer as soon it comes to fragmentation.
> Does anyone have an idea what could cause the problem and how to track this
> down? Any help is appreciated.
>
> Best regards
> Holger Brunck

Re: [tipc-discussion] refcount warnings with TIPC in combination with a TAP device and fragmented messages

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

Hi Holger,
We will look into this. It is a new setup for us, so it has to be 
prepared first.

///jon


On 11/19/20 9:33 AM, Holger Brunck wrote:
> Hi all,
> we have currently problems with TIPC in combination with a TAP interface and fragmented
> messages. We have a Kirkwood board getting TIPC ethernet packets from a third party stack
> in user space which we fill into a TAP device to pass this packets to the kernel. On the
> remote side we have another board with a physical ethernet device. After migrating to
> kernel 5.4 we saw kernel crashes as soon as we send fragmented TIPC messages via
> this link. Before the kernel crash we see "refcount_t: underflow; use-after-free"
> warnings coming either from the TIPC or the driver/net/tun.
> For smaller packets everything work fine. To track this down we wrote a small
> test program which receives raw ethernet packets in user space and send them back to
> kernel space via a TAP interface, to be able to reproduce it with a mainline kernel only.
> So instead of a link e.g. eth0:eth0 we have a link from eth0:tap0.
>
> I now tested this on latest mainline kernel and still see warnings coming from the kernel, but
> not the crash.
> So in the testcase we have TIPC server running on a remote board having a ethernet bearer
> on a interface. The ethernet interface is connected to the ethernet interface of the
> kirkwood board. The incoming messages are passed to user space and filled into the TAP device.
> So we have a TIPC link between tap0 and eth0. If I send now back and forth  TIPC connectionless
> messages to the server via the TAP device the kernel print the following messages as soon as
> the packets are larger then the MTU and TIPC need to reassemble the fragmented messages coming
> from the TAP driver:
>
> root@km_kirkwood:~#
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 94 at lib/refcount.c:28 sock_wfree+0xd8/0xe0
> refcount_t: underflow; use-after-free.
> Modules linked in:
> CPU: 0 PID: 94 Comm: tipc_ping Tainted: G        W         5.10.0-rc4-17824-g0fa8ee0d9ab9 #112
> Hardware name: Marvell Kirkwood (Flattened Device Tree)
> [<c000f924>] (unwind_backtrace) from [<c000c980>] (show_stack+0x10/0x14)
> [<c000c980>] (show_stack) from [<c001c27c>] (__warn+0xe8/0xfc)
> [<c001c27c>] (__warn) from [<c001c2fc>] (warn_slowpath_fmt+0x6c/0xb8)
> [<c001c2fc>] (warn_slowpath_fmt) from [<c0564b78>] (sock_wfree+0xd8/0xe0)
> [<c0564b78>] (sock_wfree) from [<c056b730>] (skb_release_all+0xc/0x28)
> [<c056b730>] (skb_release_all) from [<c056b784>] (kfree_skb+0x20/0x6c)
> [<c056b784>] (kfree_skb) from [<c056be44>] (skb_release_data+0xc0/0x140)
> [<c056be44>] (skb_release_data) from [<c056b784>] (kfree_skb+0x20/0x6c)
> [<c056b784>] (kfree_skb) from [<c0720ea4>] (tipc_recvmsg+0x270/0x3c4)
> [<c0720ea4>] (tipc_recvmsg) from [<c0560428>] (__sys_recvfrom+0xb0/0x128)
> [<c0560428>] (__sys_recvfrom) from [<c05604bc>] (sys_recv+0x18/0x20)
> [<c05604bc>] (sys_recv) from [<c0008240>] (ret_fast_syscall+0x0/0x50)
> Exception stack(0xc1615fa8 to 0xc1615ff0)
> 5fa0:                   00000001 00000000 00000003 00025010 0000fa00 00000000
> 5fc0: 00000001 00000000 00010888 00000123 00000000 00000000 49cb0000 be939bec
> 5fe0: 00000000 be93997c 00011114 49d9666c
> ---[ end trace e7c3a1366c544d79 ]---
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 74 at lib/refcount.c:22 sock_alloc_send_pskb+0x240/0x258
> refcount_t: saturated; leaking memory.
> Modules linked in:
> CPU: 0 PID: 74 Comm: main Tainted: G        W         5.10.0-rc4-17824-g0fa8ee0d9ab9 #112
> Hardware name: Marvell Kirkwood (Flattened Device Tree)
> [<c000f924>] (unwind_backtrace) from [<c000c980>] (show_stack+0x10/0x14)
> [<c000c980>] (show_stack) from [<c001c27c>] (__warn+0xe8/0xfc)
> [<c001c27c>] (__warn) from [<c001c2fc>] (warn_slowpath_fmt+0x6c/0xb8)
> [<c001c2fc>] (warn_slowpath_fmt) from [<c0563b40>] (sock_alloc_send_pskb+0x240/0x258)
> [<c0563b40>] (sock_alloc_send_pskb) from [<c043d500>] (tun_get_user+0x224/0x13f8)
> [<c043d500>] (tun_get_user) from [<c043e718>] (tun_chr_write_iter+0x44/0x78)
> [<c043e718>] (tun_chr_write_iter) from [<c012f1bc>] (vfs_write+0x1fc/0x448)
> [<c012f1bc>] (vfs_write) from [<c012f52c>] (ksys_write+0x54/0xcc)
> [<c012f52c>] (ksys_write) from [<c0008240>] (ret_fast_syscall+0x0/0x50)
> Exception stack(0xc158bfa8 to 0xc158bff0)
> bfa0:                   00011244 00000000 00000004 00022098 000005c2 00000004
> bfc0: 00011244 00000000 000108bc 00000004 00000000 00000000 49cb0000 becf3c94
> bfe0: 00000000 becf3bb0 49e21ce0 49e223e4
> ---[ end trace e7c3a1366c544d7a ]---
>
> Kernel version:
> root@km_kirkwood:~# cat /proc/version
> Linux version 5.10.0-rc4-17824-g0fa8ee0d9ab9
>
> If I simply use ethernet bearer on both sides everything works fine, so I need
> the TAP interface to be involved to see the problem.
>
> It seems that we have a problem in the skb_buf allocation or handling between
> the TAP driver and the TIPC layer as soon it comes to fragmentation.
> Does anyone have an idea what could cause the problem and how to track this
> down? Any help is appreciated.
>
> Best regards
> Holger Brunck

Re: [tipc-discussion] tipc over an active/backup bond device

[Howard F. <ho...@th...>]

Hi Jon,

Thanks for the response.  I was continuing to debug the situation today and I agree that the issue is with the bond device.    When using active/backup and AP monitoring, it is issuing a NETDEV_CHANGE event that arguably it should not be doing.  I am already in the process of filing a bug against the bond driver.
Essentially what happens is that the bond driver sees the slave down and stops using it. It doesn't immediately switch to the backup though because it is technically 'down'. However, it does send a NETDEV_CHANGE here.  it then immediately brings up the backup slave and sends another NETDEV_CHANGE. By then however the bearer has been reset.  The concept of the bond driver means it should switch the slave without notifying the upper layers anything happened.  But that is not the case here, and hence the bond device error.  If MII monitoring is used instead of ARP, it switches the slave immediately and TIPC is unaware.  However MII monitoring does not make much sense when going across a network involving switches, etc.

Dual TIPC links is an interesting suggestion.  However the functionality where I see this issue is on a product that is 10+ years old, and we use the bond device for redundancy.  It is not a throughput/performance issue.  

Thanks again for the quick response.

Howard

-----Original Message-----
From: Jon Maloy <jm...@re...> 
Sent: Friday, November 20, 2020 12:25 PM
To: Howard Finer <ho...@th...>; tip...@li...
Subject: Re: [tipc-discussion] tipc over an active/backup bond device

Hi Howard,
This is the code executed when TIPC receives a NETDEV_CHANGE event:

switch (evt) {
|       case NETDEV_CHANGE:
|       |       if (netif_carrier_ok(dev) && netif_oper_up(dev)) {
|       |       |       test_and_set_bit_lock(0, &b->up);
|       |       |       break;
|       |       }
|       |       fallthrough;
|       case NETDEV_GOING_DOWN:
|       |       clear_bit_unlock(0, &b->up);
|       |       tipc_reset_bearer(net, b);
|       |       break;
|       case NETDEV_UP:
|       |       test_and_set_bit_lock(0, &b->up);
|       |       break;
|       case NETDEV_CHANGEMTU:

So, unless the bond interface really reports that it is going down TIPC 
doesn't reset any links. And if it *does* report that it is going down, 
what else can we do?
To me this looks more like a problem with the bond device rather than 
with TIPC, but we might of course have misunderstood its expected behavior.
We will look into this.
On a different note, you could instead omit the bond interface and try 
using dual TIPC links, which work in active-active mode and give better 
performance.
Is that an option for you?

BR
Jon Maloy


On 11/19/20 11:36 PM, Howard Finer wrote:
> I am trying to use TIPC (kernel version 4.19) over a bond device that is
> configured for active-backup and arp monitoring for the slaves. If a slave
> goes down, TIPC is receiving a netdev_change during the timeframe that the
> bond device is working towards brining up the new slave.  This causes TIPC
> to disable the bearer, which in turn causes a temporary loss of
> communication between the nodes.
>
>   
>
> Instrumentation of the bond and tipc drivers shows the following:
>
> <6> 1 2020-11-19T23:58:33.111549+01:00 LABNBS5A kernel - - - [  153.655776]
> Enabled bearer <eth:bond0>, priority 10
>
> <6> 1 2020-11-20T00:07:58.544040+01:00 LABNBS5A kernel - - - [  718.799259]
> bond0: bond_ab_arp_commit: BOND_LINK_DOWN: link status definitely down for
> interface eth1, disabling it
>
> <6> 1 2020-11-20T00:07:58.544063+01:00 LABNBS5A kernel - - - [  718.799261]
> bond0: bond_ab_arp_commit: do_failover, block netpoll_tx and call
> select_active_slave
>
> <6> 1 2020-11-20T00:07:58.544069+01:00 LABNBS5A kernel - - - [  718.799263]
> bond0: bond_select_active_slave: bond_find_best_slave returned NULL
>
> <6> 1 2020-11-20T00:07:58.544072+01:00 LABNBS5A kernel - - - [  718.799347]
> bond0: bond_select_active_slave: now running without any active interface!
>
> <6> 1 2020-11-20T00:07:58.544080+01:00 LABNBS5A kernel - - - [  718.799349]
> bond0: bond_ab_arp_commit: do_failover, returned from select_active_slave
> and unblock netpoll tx
>
> <6> 1 2020-11-20T00:07:58.544081+01:00 LABNBS5A kernel - - - [  718.799611]
> Resetting bearer <eth:bond0>
>
> <6> 1 2020-11-20T00:07:58.655535+01:00 LABNBS5A kernel - - - [  718.907245]
> bond0: bond_ab_arp_commit: BOND_LINK_UP: link status definitely up for
> interface eth0
>
> <6> 1 2020-11-20T00:07:58.655545+01:00 LABNBS5A kernel - - - [  718.907247]
> bond0: bond_ab_arp_commit: do_failover, block netpoll_tx and call
> select_active_slave
>
> <6> 1 2020-11-20T00:07:58.655548+01:00 LABNBS5A kernel - - - [  718.907248]
> bond0: bond_select_active_slave: bond_find_best_slave returned slave eth0
>
> <6> 1 2020-11-20T00:07:58.655559+01:00 LABNBS5A kernel - - - [  718.907249]
> bond0: making interface eth0 the new active one
>
> <6> 1 2020-11-20T00:07:58.655562+01:00 LABNBS5A kernel - - - [  718.907560]
> bond0: bond_select_active_slave: first active interface up!
>
>   
>
> With arp based monitoring only 1 slave will be 'up'.  When the active slave
> goes down, the other slave needs to be brought up.  During that timeframe we
> see TIPC is resetting the bearer.    That defeats the entire purpose of
> using the bond device.
>
> It seems that the handling of the netdev_change event for a active/backup
> bond device is not correct.  It needs to leave the bearer intact so that
> when the backup slave is brought up the communication is properly restored
> without any upper layer applications being aware that something happened at
> the lower level.
>
>   
>
> Thanks,
>
> Howard
>
>
> _______________________________________________
> tipc-discussion mailing list
> tip...@li...
> https://lists.sourceforge.net/lists/listinfo/tipc-discussion
>