Hi Tuong,
Sorry, I have to use outlook client to reply to your email, which will make the
email messed a bit.
Please see my following comments:
==
[Ying] 1. Did you ever conduct comprehensive verification about this proposal?
What kinds of test environment did you use in your testing? For example, how
many TIPC physical nodes were gotten involved into your testing? Did the NICs
used during your testing support multiqueue feature? How many cores were there
on one your used physical TIPC machine?
In addition, if possible, I suggest you could try to enable RT_PREEMPT kernel
to measure what throughput results we would get.
==
In some environment, broadcast traffic is suppressed at high rate (i.e.
a kind of bandwidth limit setting). When it is applied, TIPC broadcast
can still run successfully. However, when it comes to a high load, some
packets will be dropped first and TIPC tries to retransmit them but the
packet retransmission is intentionally broadcast too, so making things
worse and not helpful at all.
This commit enables the broadcast retransmission via unicast which only
retransmits packets to the specific peer that has really reported a gap
i.e. not broadcasting to all nodes in the cluster, so will prevent from
being suppressed, and also reduce some overheads on the other peers due
to duplicates, finally improve the overall TIPC broadcast performance.
Note: the functionality can be turned on/off via the sysctl file:
echo 1 > /proc/sys/net/tipc/bc_retruni
echo 0 > /proc/sys/net/tipc/bc_retruni
Default is '0', i.e. the broadcast retransmission still works as usual.
==
[Ying] 2. Actually I had a similar idea before, so I also think the broadcast
performance might be significantly improved through this proposal, but we act
as TIPC developers, we should explicitly tell users what condition they should
enable this option and what condition they should disable it, otherwise, users
have no idea at all about when to enable this option or when to disable this
option.
So, please give more performance data obtained in different test conditions.
If this patch can give broadcast performance a clear benefit under any test
condition, ideally we completely remove this option. Otherwise, at least we can
tell users when to enable this option.
==
Signed-off-by: Tuong Lien <tuong.t.l...@dektech.com.au>
int tipc_link_bc_ack_rcv(struct tipc_link *r, u16 acked, u16 gap,
struct tipc_gap_ack_blks *ga,
- struct sk_buff_head *xmitq)
+ struct sk_buff_head *xmitq,
+ struct sk_buff_head *retrq)
{
struct tipc_link *l = r->bc_sndlink;
bool unused = false;
==
3. [Ying] Sorry, I felt a bit confused. One new "retrq" parameter was
introduced, but I didn't find where it was used in this function.
Can you please explain how the new parameter works?
==
Thanks,
Ying
@@ -2460,7 +2461,8 @@ int tipc_link_bc_nack_rcv(struct tipc_link *l, struct
sk_buff *skb,
return 0;
if (dnode == tipc_own_addr(l->net)) {
- rc = tipc_link_bc_ack_rcv(l, acked, to - acked, NULL, xmitq);
+ rc = tipc_link_bc_ack_rcv(l, acked, to - acked, NULL, xmitq,
+ xmitq);
l->stats.recv_nacks++;
return rc;
}
diff --git a/net/tipc/link.h b/net/tipc/link.h
index 0a0fa7350722..4d0768cf91d5 100644
--- a/net/tipc/link.h
+++ b/net/tipc/link.h
@@ -147,7 +147,8 @@ u16 tipc_get_gap_ack_blks(struct tipc_gap_ack_blks **ga,
struct tipc_link *l,
struct tipc_msg *hdr, bool uc);
int tipc_link_bc_ack_rcv(struct tipc_link *l, u16 acked, u16 gap,
struct tipc_gap_ack_blks *ga,
- struct sk_buff_head *xmitq);
+ struct sk_buff_head *xmitq,
+ struct sk_buff_head *retrq);
void tipc_link_build_bc_sync_msg(struct tipc_link *l,
struct sk_buff_head *xmitq);
void tipc_link_bc_init_rcv(struct tipc_link *l, struct tipc_msg *hdr);
diff --git a/net/tipc/node.c b/net/tipc/node.c
index eb6b62de81a7..917ad3920fac 100644
--- a/net/tipc/node.c
+++ b/net/tipc/node.c
@@ -1771,7 +1771,7 @@ static void tipc_node_bc_sync_rcv(struct tipc_node *n,
struct tipc_msg *hdr,
struct tipc_link *ucl;
int rc;
- rc = tipc_bcast_sync_rcv(n->net, n->bc_entry.link, hdr);
+ rc = tipc_bcast_sync_rcv(n->net, n->bc_entry.link, hdr, xmitq);
if (rc & TIPC_LINK_DOWN_EVT) {
tipc_node_reset_links(n);
diff --git a/net/tipc/sysctl.c b/net/tipc/sysctl.c
index 58ab3d6dcdce..97a6264a2993 100644
--- a/net/tipc/sysctl.c
+++ b/net/tipc/sysctl.c
@@ -36,7 +36,7 @@
#include "core.h"
#include "trace.h"
#include "crypto.h"
-
+#include "bcast.h"
#include <linux/sysctl.h>
static struct ctl_table_header *tipc_ctl_hdr;
@@ -75,6 +75,13 @@ static struct ctl_table tipc_table[] = {
.extra1 = SYSCTL_ONE,
},
#endif
+ {
+ .procname = "bc_retruni",
+ .data = &sysctl_tipc_bc_retruni,
+ .maxlen = sizeof(sysctl_tipc_bc_retruni),
+ .mode = 0644,
+ .proc_handler = proc_doulongvec_minmax,
+ },
{}
};
--
2.13.7
_______________________________________________
tipc-discussion mailing list
tipc-discussion@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tipc-discussion
