Hi Hoang.
See below.
On 6/1/20 5:33 AM, Hoang Huu Le wrote:
Hi Jon,
There is a concern in function tipc_node_broadcast.
See my comment inline.
Regards,
Hoang
-----Original Message-----
From: jma...@redhat.com <jma...@redhat.com>
Sent: Friday, May 29, 2020 10:34 PM
To: tipc-discussion@lists.sourceforge.net
Cc: Tung Quang Nguyen <tung.q.ngu...@dektech.com.au>; Hoang Huu Le
<hoang.h...@dektech.com.au>; Tuong Tong Lien <tuong.t.l...@dektech.com.au>;
jma...@redhat.com; ma...@donjonn.com; x...@redhat.com; ying....@windriver.com;
parthasarathy.bhuvara...@gmail.com
Subject: [ ] tipc: update a binding service via broadcast
From: Hoang Huu Le <hoang.h...@dektech.com.au>
Currently, updating binding table (add service binding to
name table/withdraw a service binding) is being sent over replicast.
However, if we are scaling up clusters to > 100 nodes/containers this
method is less affection because of looping through nodes in a cluster one
by one.
[...]
+ /* Use broadcast if all nodes support it */
+ if (!rc_dests) {
+ __skb_queue_head_init(&xmitq);
+ __skb_queue_tail(&xmitq, skb);
+ tipc_bcast_xmit(net, &xmitq, &dummy);
+ return;
+ }
+
[Hoang]
We could not use 'rc_dests' to send as broadcast mode because of it is not
fully broadcast supporting in the cluster.
As stated, if there is a node in the cluster not supporting broadcast mode, we
need to use replicast instead.
That's why we introduced the feature "Smooth switch between replicast/broadcast in
bcast.c" and a new command line to configure the broadcast link.
If we assume base on 'rc_dests' (i.e capability flags TIPC_NAMED_BCAST),
probably 'forced replicast' configuration on broadcast link becomes useless.
Then, destination nodes will be missed the publication item.
You misunderstand this function. rc_dests is a *counter*, not a flag. It
counts the number of peer nodes that don't support TIPC_NAMED_BCAST, and
if this is non-zero, we use replicast. So this is safe.
///jon
+ /* Otherwise use legacy replicast method */
rcu_read_lock();
list_for_each_entry_rcu(n, tipc_nodes(net), list) {
dst = n->addr;
@@ -1749,7 +1762,6 @@ void tipc_node_broadcast(struct net *net, struct sk_buff
*skb)
tipc_node_xmit_skb(net, txskb, dst, 0);
}
rcu_read_unlock();
-
kfree_skb(skb);
}
@@ -1844,7 +1856,9 @@ static void tipc_node_bc_rcv(struct net *net, struct sk_buff *skb, int bearer_id
/* Handle NAME_DISTRIBUTOR messages sent from 1.7 nodes */
if (!skb_queue_empty(&n->bc_entry.namedq))
- tipc_named_rcv(net, &n->bc_entry.namedq);
+ tipc_named_rcv(net, &n->bc_entry.namedq,
+ &n->bc_entry.named_rcv_nxt,
+ &n->bc_entry.named_open);
/* If reassembly or retransmission failure => reset all links to peer */
if (rc & TIPC_LINK_DOWN_EVT)
@@ -2109,7 +2123,9 @@ void tipc_rcv(struct net *net, struct sk_buff *skb,
struct tipc_bearer *b)
tipc_node_link_down(n, bearer_id, false);
if (unlikely(!skb_queue_empty(&n->bc_entry.namedq)))
- tipc_named_rcv(net, &n->bc_entry.namedq);
+ tipc_named_rcv(net, &n->bc_entry.namedq,
+ &n->bc_entry.named_rcv_nxt,
+ &n->bc_entry.named_open);
if (unlikely(!skb_queue_empty(&n->bc_entry.inputq1)))
tipc_node_mcast_rcv(n);
diff --git a/net/tipc/node.h b/net/tipc/node.h
index a6803b449a2c..9f6f13f1604f 100644
--- a/net/tipc/node.h
+++ b/net/tipc/node.h
@@ -55,7 +55,8 @@ enum {
TIPC_MCAST_RBCTL = (1 << 7),
TIPC_GAP_ACK_BLOCK = (1 << 8),
TIPC_TUNNEL_ENHANCED = (1 << 9),
- TIPC_NAGLE = (1 << 10)
+ TIPC_NAGLE = (1 << 10),
+ TIPC_NAMED_BCAST = (1 << 11)
};
#define TIPC_NODE_CAPABILITIES (TIPC_SYN_BIT | \
@@ -68,7 +69,8 @@ enum {
TIPC_MCAST_RBCTL | \
TIPC_GAP_ACK_BLOCK | \
TIPC_TUNNEL_ENHANCED | \
- TIPC_NAGLE)
+ TIPC_NAGLE | \
+ TIPC_NAMED_BCAST)
#define INVALID_BEARER_ID -1
@@ -101,7 +103,7 @@ int tipc_node_xmit_skb(struct net *net, struct sk_buff *skb, u32 dest,
u32 selector);
void tipc_node_subscribe(struct net *net, struct list_head *subscr, u32 addr);
void tipc_node_unsubscribe(struct net *net, struct list_head *subscr, u32
addr);
-void tipc_node_broadcast(struct net *net, struct sk_buff *skb);
+void tipc_node_broadcast(struct net *net, struct sk_buff *skb, int rc_dests);
int tipc_node_add_conn(struct net *net, u32 dnode, u32 port, u32 peer_port);
void tipc_node_remove_conn(struct net *net, u32 dnode, u32 port);
int tipc_node_get_mtu(struct net *net, u32 addr, u32 sel, bool connected);
_______________________________________________
tipc-discussion mailing list
tipc-discussion@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tipc-discussion
