[tipc-discussion] [PATCH RFC 2/4] tipc: add back link trace events
In the previous commit ("tipc: add Gap ACK blocks support for broadcast link"), we have removed the following link trace events due to the code changes: - tipc_link_bc_ack - tipc_link_retrans This commit adds them back along with some minor changes to adapt to the new code. Signed-off-by: Tuong Lien --- net/tipc/link.c | 3 +++ net/tipc/trace.h | 13 - 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/net/tipc/link.c b/net/tipc/link.c index 1b60ba665504..405ccf597e59 100644 --- a/net/tipc/link.c +++ b/net/tipc/link.c @@ -1517,6 +1517,8 @@ static int tipc_link_advance_transmq(struct tipc_link *l, struct tipc_link *r, bool is_uc = !link_is_bc_sndlink(l); bool bc_has_acked = false; + trace_tipc_link_retrans(r, acked + 1, acked + gap, >transmq); + /* Determine Gap ACK blocks if any for the particular link */ if (ga && is_uc) { /* Get the Gap ACKs, uc part */ @@ -2423,6 +2425,7 @@ int tipc_link_bc_ack_rcv(struct tipc_link *r, u16 acked, u16 gap, if (less(acked, r->acked) || (acked == r->acked && !gap && !ga)) return 0; + trace_tipc_link_bc_ack(r, acked, gap, >transmq); tipc_link_advance_transmq(l, r, acked, gap, ga, xmitq, , ); tipc_link_advance_backlog(l, xmitq); diff --git a/net/tipc/trace.h b/net/tipc/trace.h index 4d8e00483afc..e7535ab75255 100644 --- a/net/tipc/trace.h +++ b/net/tipc/trace.h @@ -299,8 +299,10 @@ DECLARE_EVENT_CLASS(tipc_link_transmq_class, __entry->from = f; __entry->to = t; __entry->len = skb_queue_len(tq); - __entry->fseqno = msg_seqno(buf_msg(skb_peek(tq))); - __entry->lseqno = msg_seqno(buf_msg(skb_peek_tail(tq))); + __entry->fseqno = __entry->len ? + msg_seqno(buf_msg(skb_peek(tq))) : 0; + __entry->lseqno = __entry->len ? + msg_seqno(buf_msg(skb_peek_tail(tq))) : 0; ), TP_printk("<%s> retrans req: [%u-%u] transmq: %u [%u-%u]\n", @@ -308,15 +310,16 @@ DECLARE_EVENT_CLASS(tipc_link_transmq_class, __entry->len, __entry->fseqno, __entry->lseqno) ); -DEFINE_EVENT(tipc_link_transmq_class, tipc_link_retrans, +DEFINE_EVENT_CONDITION(tipc_link_transmq_class, tipc_link_retrans, TP_PROTO(struct tipc_link *r, u16 f, u16 t, struct sk_buff_head *tq), - TP_ARGS(r, f, t, tq) + TP_ARGS(r, f, t, tq), + TP_CONDITION(less_eq(f, t)) ); DEFINE_EVENT_PRINT(tipc_link_transmq_class, tipc_link_bc_ack, TP_PROTO(struct tipc_link *r, u16 f, u16 t, struct sk_buff_head *tq), TP_ARGS(r, f, t, tq), - TP_printk("<%s> acked: [%u-%u] transmq: %u [%u-%u]\n", + TP_printk("<%s> acked: %u gap: %u transmq: %u [%u-%u]\n", __entry->name, __entry->from, __entry->to, __entry->len, __entry->fseqno, __entry->lseqno) ); -- 2.13.7 ___ tipc-discussion mailing list tipc-discussion@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tipc-discussion
[tipc-discussion] [PATCH RFC 3/4] tipc: enable broadcast retrans via unicast
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. Signed-off-by: Tuong Lien --- net/tipc/bcast.c | 11 --- net/tipc/bcast.h | 4 +++- net/tipc/link.c | 8 +--- net/tipc/link.h | 3 ++- net/tipc/node.c | 2 +- net/tipc/sysctl.c | 9 - 6 files changed, 27 insertions(+), 10 deletions(-) diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c index 3ce690a96ee9..50a16f8bebd9 100644 --- a/net/tipc/bcast.c +++ b/net/tipc/bcast.c @@ -46,6 +46,7 @@ #define BCLINK_WIN_MIN 32 /* bcast minimum link window size */ const char tipc_bclink_name[] = "broadcast-link"; +unsigned long sysctl_tipc_bc_retruni __read_mostly; /** * struct tipc_bc_base - base structure for keeping broadcast send state @@ -474,7 +475,7 @@ void tipc_bcast_ack_rcv(struct net *net, struct tipc_link *l, __skb_queue_head_init(); tipc_bcast_lock(net); - tipc_link_bc_ack_rcv(l, acked, 0, NULL, ); + tipc_link_bc_ack_rcv(l, acked, 0, NULL, , NULL); tipc_bcast_unlock(net); tipc_bcbase_xmit(net, ); @@ -489,7 +490,8 @@ void tipc_bcast_ack_rcv(struct net *net, struct tipc_link *l, * RCU is locked, no other locks set */ int tipc_bcast_sync_rcv(struct net *net, struct tipc_link *l, - struct tipc_msg *hdr) + struct tipc_msg *hdr, + struct sk_buff_head *retrq) { struct sk_buff_head *inputq = _bc_base(net)->inputq; struct tipc_gap_ack_blks *ga; @@ -503,8 +505,11 @@ int tipc_bcast_sync_rcv(struct net *net, struct tipc_link *l, tipc_link_bc_init_rcv(l, hdr); } else if (!msg_bc_ack_invalid(hdr)) { tipc_get_gap_ack_blks(, l, hdr, false); + if (!sysctl_tipc_bc_retruni) + retrq = rc = tipc_link_bc_ack_rcv(l, msg_bcast_ack(hdr), - msg_bc_gap(hdr), ga, ); + msg_bc_gap(hdr), ga, , + retrq); rc |= tipc_link_bc_sync_rcv(l, hdr, ); } tipc_bcast_unlock(net); diff --git a/net/tipc/bcast.h b/net/tipc/bcast.h index 9e847d9617d3..97d3cf9d3e4d 100644 --- a/net/tipc/bcast.h +++ b/net/tipc/bcast.h @@ -45,6 +45,7 @@ struct tipc_nl_msg; struct tipc_nlist; struct tipc_nitem; extern const char tipc_bclink_name[]; +extern unsigned long sysctl_tipc_bc_retruni; #define TIPC_METHOD_EXPIRE msecs_to_jiffies(5000) @@ -93,7 +94,8 @@ int tipc_bcast_rcv(struct net *net, struct tipc_link *l, struct sk_buff *skb); void tipc_bcast_ack_rcv(struct net *net, struct tipc_link *l, struct tipc_msg *hdr); int tipc_bcast_sync_rcv(struct net *net, struct tipc_link *l, - struct tipc_msg *hdr); + struct tipc_msg *hdr, + struct sk_buff_head *retrq); int tipc_nl_add_bc_link(struct net *net, struct tipc_nl_msg *msg); int tipc_nl_bc_link_set(struct net *net, struct nlattr *attrs[]); int tipc_bclink_reset_stats(struct net *net); diff --git a/net/tipc/link.c b/net/tipc/link.c index 405ccf597e59..3071e46f029a 100644 --- a/net/tipc/link.c +++ b/net/tipc/link.c @@ -375,7 +375,7 @@ void tipc_link_remove_bc_peer(struct tipc_link *snd_l, snd_l->ackers--; rcv_l->bc_peer_is_up = true; rcv_l->state = LINK_ESTABLISHED; - tipc_link_bc_ack_rcv(rcv_l, ack, 0, NULL, xmitq); + tipc_link_bc_ack_rcv(rcv_l, ack, 0, NULL, xmitq, NULL); trace_tipc_link_reset(rcv_l, TIPC_DUMP_ALL, "bclink removed!"); tipc_link_reset(rcv_l); rcv_l->state = LINK_RESET; @@ -2413,7 +2413,8 @@ int tipc_link_bc_sync_rcv(struct tipc_link *l, struct tipc_msg *hdr, 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 =
[tipc-discussion] [PATCH RFC 0/4] tipc: add some improvements for broadcast
Hi Jon, all, Please find the full series here, + For the 1st patch: it's really the last one I sent before, so you have ack-ed already. + For the other ones, please help take a look. Also, I will send another patch for iproute2/tipc which is user-space part of the last one in this series i.e. broadcast rcv stats dumping. Thanks alot! Tuong Lien (4): tipc: introduce Gap ACK blocks for broadcast link tipc: add back link trace events tipc: enable broadcast retrans via unicast tipc: add support for broadcast rcv stats dumping net/tipc/bcast.c | 22 ++- net/tipc/bcast.h | 9 +- net/tipc/link.c| 500 +++-- net/tipc/link.h| 11 +- net/tipc/msg.c | 9 +- net/tipc/msg.h | 16 +- net/tipc/netlink.c | 2 +- net/tipc/node.c| 75 ++-- net/tipc/sysctl.c | 9 +- net/tipc/trace.h | 17 +- 10 files changed, 424 insertions(+), 246 deletions(-) -- 2.13.7 ___ tipc-discussion mailing list tipc-discussion@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tipc-discussion
[tipc-discussion] [PATCH RFC 1/4] tipc: introduce Gap ACK blocks for broadcast link
As achieved through commit 9195948fbf34 ("tipc: improve TIPC throughput by Gap ACK blocks"), we apply the same mechanism for the broadcast link as well. The 'Gap ACK blocks' data field in a 'PROTOCOL/STATE_MSG' will consist of two parts built for both the broadcast and unicast types: 31 16 150 +-+-+-+-+ | bgack_cnt | ugack_cnt |len| +-+-+-+-+ - |gap|ack| | +-+-+-+-+> bc gacks : : : | +-+-+-+-+ - |gap|ack| | +-+-+-+-+> uc gacks : : : | +-+-+-+-+ - which is "automatically" backward-compatible. We also increase the max number of Gap ACK blocks to 128, allowing upto 64 blocks per type (total buffer size = 516 bytes). Besides, the 'tipc_link_advance_transmq()' function is refactored which is applicable for both the unicast and broadcast cases now, so some old functions can be removed and the code is optimized. With the patch, TIPC broadcast is more robust regardless of packet loss or disorder, latency, ... in the underlying network. Its performance is boost up significantly. For example, experiment with a 5% packet loss rate results: $ time tipc-pipe --mc --rdm --data_size 123 --data_num 150 real0m 42.46s user0m 1.16s sys 0m 17.67s Without the patch: $ time tipc-pipe --mc --rdm --data_size 123 --data_num 150 real8m 27.94s user0m 0.55s sys 0m 2.38s Acked-by: Jon Maloy Signed-off-by: Tuong Lien --- net/tipc/bcast.c | 9 +- net/tipc/link.c | 438 +-- net/tipc/link.h | 7 +- net/tipc/msg.h | 14 +- net/tipc/node.c | 10 +- 5 files changed, 293 insertions(+), 185 deletions(-) diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c index 4c20be08b9c4..3ce690a96ee9 100644 --- a/net/tipc/bcast.c +++ b/net/tipc/bcast.c @@ -474,7 +474,7 @@ void tipc_bcast_ack_rcv(struct net *net, struct tipc_link *l, __skb_queue_head_init(); tipc_bcast_lock(net); - tipc_link_bc_ack_rcv(l, acked, ); + tipc_link_bc_ack_rcv(l, acked, 0, NULL, ); tipc_bcast_unlock(net); tipc_bcbase_xmit(net, ); @@ -492,6 +492,7 @@ int tipc_bcast_sync_rcv(struct net *net, struct tipc_link *l, struct tipc_msg *hdr) { struct sk_buff_head *inputq = _bc_base(net)->inputq; + struct tipc_gap_ack_blks *ga; struct sk_buff_head xmitq; int rc = 0; @@ -501,8 +502,10 @@ int tipc_bcast_sync_rcv(struct net *net, struct tipc_link *l, if (msg_type(hdr) != STATE_MSG) { tipc_link_bc_init_rcv(l, hdr); } else if (!msg_bc_ack_invalid(hdr)) { - tipc_link_bc_ack_rcv(l, msg_bcast_ack(hdr), ); - rc = tipc_link_bc_sync_rcv(l, hdr, ); + tipc_get_gap_ack_blks(, l, hdr, false); + rc = tipc_link_bc_ack_rcv(l, msg_bcast_ack(hdr), + msg_bc_gap(hdr), ga, ); + rc |= tipc_link_bc_sync_rcv(l, hdr, ); } tipc_bcast_unlock(net); diff --git a/net/tipc/link.c b/net/tipc/link.c index 467c53a1fb5c..1b60ba665504 100644 --- a/net/tipc/link.c +++ b/net/tipc/link.c @@ -188,6 +188,8 @@ struct tipc_link { /* Broadcast */ u16 ackers; u16 acked; + u16 last_gap; + struct tipc_gap_ack_blks *last_ga; struct tipc_link *bc_rcvlink; struct tipc_link *bc_sndlink; u8 nack_state; @@ -249,11 +251,14 @@ static int tipc_link_build_nack_msg(struct tipc_link *l, struct sk_buff_head *xmitq); static void tipc_link_build_bc_init_msg(struct tipc_link *l, struct sk_buff_head *xmitq); -static int tipc_link_release_pkts(struct tipc_link *l, u16 to); -static u16 tipc_build_gap_ack_blks(struct tipc_link *l, void *data, u16 gap); -static int tipc_link_advance_transmq(struct tipc_link *l, u16 acked, u16 gap, +static u8 __tipc_build_gap_ack_blks(struct tipc_gap_ack_blks *ga, + struct tipc_link *l, u8 start_index); +static u16 tipc_build_gap_ack_blks(struct tipc_link *l, struct tipc_msg *hdr); +static int tipc_link_advance_transmq(struct tipc_link *l, struct tipc_link *r, +u16 acked, u16 gap, struct tipc_gap_ack_blks *ga, -struct sk_buff_head *xmitq); +struct sk_buff_head *xmitq, +
[tipc-discussion] [PATCH RFC 4/4] tipc: add support for broadcast rcv stats dumping
This commit enables dumping the statistics of a broadcast-receiver link like the traditional 'broadcast-link' one (which is for broadcast- sender). The link dumping can be triggered via netlink (e.g. the iproute2/tipc tool) by the link flag - 'TIPC_NLA_LINK_BROADCAST' as the indicator. The name of a broadcast-receiver link of a specific peer will be in the format: 'broadcast-link:'. For example: Link Window:50 packets RX packets:7841 fragments:2408/440 bundles:0/0 TX packets:0 fragments:0/0 bundles:0/0 RX naks:0 defs:124 dups:0 TX naks:21 acks:0 retrans:0 Congestion link:0 Send queue max:0 avg:0 In addition, the broadcast-receiver link statistics can be reset in the usual way via netlink by specifying that link name in command. Note: the 'tipc_link_name_ext()' is removed because the link name can now be retrieved simply via the 'l->name'. Signed-off-by: Tuong Lien --- net/tipc/bcast.c | 6 ++--- net/tipc/bcast.h | 5 +++-- net/tipc/link.c| 65 +++--- net/tipc/link.h| 3 +-- net/tipc/msg.c | 9 net/tipc/msg.h | 2 +- net/tipc/netlink.c | 2 +- net/tipc/node.c| 63 +--- net/tipc/trace.h | 4 ++-- 9 files changed, 103 insertions(+), 56 deletions(-) diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c index 50a16f8bebd9..383f87bc1061 100644 --- a/net/tipc/bcast.c +++ b/net/tipc/bcast.c @@ -563,10 +563,8 @@ void tipc_bcast_remove_peer(struct net *net, struct tipc_link *rcv_l) tipc_sk_rcv(net, inputq); } -int tipc_bclink_reset_stats(struct net *net) +int tipc_bclink_reset_stats(struct net *net, struct tipc_link *l) { - struct tipc_link *l = tipc_bc_sndlink(net); - if (!l) return -ENOPROTOOPT; @@ -694,7 +692,7 @@ int tipc_bcast_init(struct net *net) tn->bcbase = bb; spin_lock_init(_net(net)->bclock); - if (!tipc_link_bc_create(net, 0, 0, + if (!tipc_link_bc_create(net, 0, 0, NULL, FB_MTU, BCLINK_WIN_DEFAULT, BCLINK_WIN_DEFAULT, diff --git a/net/tipc/bcast.h b/net/tipc/bcast.h index 97d3cf9d3e4d..4240c95188b1 100644 --- a/net/tipc/bcast.h +++ b/net/tipc/bcast.h @@ -96,9 +96,10 @@ void tipc_bcast_ack_rcv(struct net *net, struct tipc_link *l, int tipc_bcast_sync_rcv(struct net *net, struct tipc_link *l, struct tipc_msg *hdr, struct sk_buff_head *retrq); -int tipc_nl_add_bc_link(struct net *net, struct tipc_nl_msg *msg); +int tipc_nl_add_bc_link(struct net *net, struct tipc_nl_msg *msg, + struct tipc_link *bcl); int tipc_nl_bc_link_set(struct net *net, struct nlattr *attrs[]); -int tipc_bclink_reset_stats(struct net *net); +int tipc_bclink_reset_stats(struct net *net, struct tipc_link *l); u32 tipc_bcast_get_broadcast_mode(struct net *net); u32 tipc_bcast_get_broadcast_ratio(struct net *net); diff --git a/net/tipc/link.c b/net/tipc/link.c index 3071e46f029a..808d3a76c27f 100644 --- a/net/tipc/link.c +++ b/net/tipc/link.c @@ -539,7 +539,7 @@ bool tipc_link_create(struct net *net, char *if_name, int bearer_id, * * Returns true if link was created, otherwise false */ -bool tipc_link_bc_create(struct net *net, u32 ownnode, u32 peer, +bool tipc_link_bc_create(struct net *net, u32 ownnode, u32 peer, u8 *peer_id, int mtu, u32 min_win, u32 max_win, u16 peer_caps, struct sk_buff_head *inputq, struct sk_buff_head *namedq, @@ -554,7 +554,18 @@ bool tipc_link_bc_create(struct net *net, u32 ownnode, u32 peer, return false; l = *link; - strcpy(l->name, tipc_bclink_name); + if (peer_id) { + char peer_str[NODE_ID_STR_LEN] = {0,}; + + tipc_nodeid2string(peer_str, peer_id); + if (strlen(peer_str) > 16) + sprintf(peer_str, "%x", peer); + /* Broadcast receiver link name: "broadcast-link:" */ + snprintf(l->name, sizeof(l->name), "%s:%s", tipc_bclink_name, +peer_str); + } else { + strcpy(l->name, tipc_bclink_name); + } trace_tipc_link_reset(l, TIPC_DUMP_ALL, "bclink created!"); tipc_link_reset(l); l->state = LINK_RESET; @@ -1412,11 +1423,8 @@ static u8 __tipc_build_gap_ack_blks(struct tipc_gap_ack_blks *ga, gacks[n].ack = htons(expect - 1); gacks[n].gap = htons(seqno - expect); if (++n >= MAX_GAP_ACK_BLKS / 2) { - char buf[TIPC_MAX_LINK_NAME]; - pr_info_ratelimited("Gacks on %s: %d, ql: %d!\n", - tipc_link_name_ext(l, buf), -
Re: [tipc-discussion] [PATCH RFC 2/4] tipc: add back link trace events
Yes, I got the same results. Hoang -Original Message- From: Jon Maloy Sent: Friday, March 27, 2020 8:43 PM To: Tuong Tong Lien ; ma...@donjonn.com; ying@windriver.com; tipc-discussion@lists.sourceforge.net Cc: tipc-...@dektech.com.au Subject: Re: [tipc-discussion] [PATCH RFC 2/4] tipc: add back link trace events I received [2/4], 3/4] and [4/4] of thi series but no [0/4] and [1/4]. This is the case both for my redhat account and my private account, so I assume the problem is at your end. Please re-post. ///jon On 3/27/20 7:56 AM, Tuong Lien wrote: > In the previous commit ("tipc: add Gap ACK blocks support for broadcast > link"), we have removed the following link trace events due to the code > changes: > > - tipc_link_bc_ack > - tipc_link_retrans > > This commit adds them back along with some minor changes to adapt to > the new code. > > Signed-off-by: Tuong Lien > --- > net/tipc/link.c | 3 +++ > net/tipc/trace.h | 13 - > 2 files changed, 11 insertions(+), 5 deletions(-) > > diff --git a/net/tipc/link.c b/net/tipc/link.c > index 1b60ba665504..405ccf597e59 100644 > --- a/net/tipc/link.c > +++ b/net/tipc/link.c > @@ -1517,6 +1517,8 @@ static int tipc_link_advance_transmq(struct tipc_link > *l, struct tipc_link *r, > bool is_uc = !link_is_bc_sndlink(l); > bool bc_has_acked = false; > > + trace_tipc_link_retrans(r, acked + 1, acked + gap, >transmq); > + > /* Determine Gap ACK blocks if any for the particular link */ > if (ga && is_uc) { > /* Get the Gap ACKs, uc part */ > @@ -2423,6 +2425,7 @@ int tipc_link_bc_ack_rcv(struct tipc_link *r, u16 > acked, u16 gap, > if (less(acked, r->acked) || (acked == r->acked && !gap && !ga)) > return 0; > > + trace_tipc_link_bc_ack(r, acked, gap, >transmq); > tipc_link_advance_transmq(l, r, acked, gap, ga, xmitq, , ); > > tipc_link_advance_backlog(l, xmitq); > diff --git a/net/tipc/trace.h b/net/tipc/trace.h > index 4d8e00483afc..e7535ab75255 100644 > --- a/net/tipc/trace.h > +++ b/net/tipc/trace.h > @@ -299,8 +299,10 @@ DECLARE_EVENT_CLASS(tipc_link_transmq_class, > __entry->from = f; > __entry->to = t; > __entry->len = skb_queue_len(tq); > - __entry->fseqno = msg_seqno(buf_msg(skb_peek(tq))); > - __entry->lseqno = msg_seqno(buf_msg(skb_peek_tail(tq))); > + __entry->fseqno = __entry->len ? > + msg_seqno(buf_msg(skb_peek(tq))) : 0; > + __entry->lseqno = __entry->len ? > + msg_seqno(buf_msg(skb_peek_tail(tq))) : 0; > ), > > TP_printk("<%s> retrans req: [%u-%u] transmq: %u [%u-%u]\n", > @@ -308,15 +310,16 @@ DECLARE_EVENT_CLASS(tipc_link_transmq_class, > __entry->len, __entry->fseqno, __entry->lseqno) > ); > > -DEFINE_EVENT(tipc_link_transmq_class, tipc_link_retrans, > +DEFINE_EVENT_CONDITION(tipc_link_transmq_class, tipc_link_retrans, > TP_PROTO(struct tipc_link *r, u16 f, u16 t, struct sk_buff_head *tq), > - TP_ARGS(r, f, t, tq) > + TP_ARGS(r, f, t, tq), > + TP_CONDITION(less_eq(f, t)) > ); > > DEFINE_EVENT_PRINT(tipc_link_transmq_class, tipc_link_bc_ack, > TP_PROTO(struct tipc_link *r, u16 f, u16 t, struct sk_buff_head *tq), > TP_ARGS(r, f, t, tq), > - TP_printk("<%s> acked: [%u-%u] transmq: %u [%u-%u]\n", > + TP_printk("<%s> acked: %u gap: %u transmq: %u [%u-%u]\n", > __entry->name, __entry->from, __entry->to, > __entry->len, __entry->fseqno, __entry->lseqno) > ); ___ tipc-discussion mailing list tipc-discussion@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tipc-discussion ___ tipc-discussion mailing list tipc-discussion@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tipc-discussion
Re: [tipc-discussion] [PATCH RFC 2/4] tipc: add back link trace events
I received [2/4], 3/4] and [4/4] of thi series but no [0/4] and [1/4]. This is the case both for my redhat account and my private account, so I assume the problem is at your end. Please re-post. ///jon On 3/27/20 7:56 AM, Tuong Lien wrote: In the previous commit ("tipc: add Gap ACK blocks support for broadcast link"), we have removed the following link trace events due to the code changes: - tipc_link_bc_ack - tipc_link_retrans This commit adds them back along with some minor changes to adapt to the new code. Signed-off-by: Tuong Lien --- net/tipc/link.c | 3 +++ net/tipc/trace.h | 13 - 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/net/tipc/link.c b/net/tipc/link.c index 1b60ba665504..405ccf597e59 100644 --- a/net/tipc/link.c +++ b/net/tipc/link.c @@ -1517,6 +1517,8 @@ static int tipc_link_advance_transmq(struct tipc_link *l, struct tipc_link *r, bool is_uc = !link_is_bc_sndlink(l); bool bc_has_acked = false; + trace_tipc_link_retrans(r, acked + 1, acked + gap, >transmq); + /* Determine Gap ACK blocks if any for the particular link */ if (ga && is_uc) { /* Get the Gap ACKs, uc part */ @@ -2423,6 +2425,7 @@ int tipc_link_bc_ack_rcv(struct tipc_link *r, u16 acked, u16 gap, if (less(acked, r->acked) || (acked == r->acked && !gap && !ga)) return 0; + trace_tipc_link_bc_ack(r, acked, gap, >transmq); tipc_link_advance_transmq(l, r, acked, gap, ga, xmitq, , ); tipc_link_advance_backlog(l, xmitq); diff --git a/net/tipc/trace.h b/net/tipc/trace.h index 4d8e00483afc..e7535ab75255 100644 --- a/net/tipc/trace.h +++ b/net/tipc/trace.h @@ -299,8 +299,10 @@ DECLARE_EVENT_CLASS(tipc_link_transmq_class, __entry->from = f; __entry->to = t; __entry->len = skb_queue_len(tq); - __entry->fseqno = msg_seqno(buf_msg(skb_peek(tq))); - __entry->lseqno = msg_seqno(buf_msg(skb_peek_tail(tq))); + __entry->fseqno = __entry->len ? + msg_seqno(buf_msg(skb_peek(tq))) : 0; + __entry->lseqno = __entry->len ? + msg_seqno(buf_msg(skb_peek_tail(tq))) : 0; ), TP_printk("<%s> retrans req: [%u-%u] transmq: %u [%u-%u]\n", @@ -308,15 +310,16 @@ DECLARE_EVENT_CLASS(tipc_link_transmq_class, __entry->len, __entry->fseqno, __entry->lseqno) ); -DEFINE_EVENT(tipc_link_transmq_class, tipc_link_retrans, +DEFINE_EVENT_CONDITION(tipc_link_transmq_class, tipc_link_retrans, TP_PROTO(struct tipc_link *r, u16 f, u16 t, struct sk_buff_head *tq), - TP_ARGS(r, f, t, tq) + TP_ARGS(r, f, t, tq), + TP_CONDITION(less_eq(f, t)) ); DEFINE_EVENT_PRINT(tipc_link_transmq_class, tipc_link_bc_ack, TP_PROTO(struct tipc_link *r, u16 f, u16 t, struct sk_buff_head *tq), TP_ARGS(r, f, t, tq), - TP_printk("<%s> acked: [%u-%u] transmq: %u [%u-%u]\n", + TP_printk("<%s> acked: %u gap: %u transmq: %u [%u-%u]\n", __entry->name, __entry->from, __entry->to, __entry->len, __entry->fseqno, __entry->lseqno) ); ___ tipc-discussion mailing list tipc-discussion@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tipc-discussion
[tipc-discussion] [net] tipc: fix incorrect increasing of link window
In commit 16ad3f4022bb ("tipc: introduce variable window congestion control"), we allow link window to change with the congestion avoidance algorithm. However, there is a bug that during the slow-start if packet retransmission occurs, the link will enter the fast-recovery phase, set its window to the 'ssthresh' which is never less than 300, so the link window suddenly increases to that limit instead of decreasing. Consequently, two issues have been observed: - For broadcast-link: it can leave a gap between the link queues that a new packet will be inserted and sent before the previous ones, i.e. not in-order. - For unicast: the algorithm does not work as expected, the link window jumps to the slow-start threshold whereas packet retransmission occurs. This commit fixes the issues by avoiding such the link window increase, but still decreasing if the 'ssthresh' is lowered. Signed-off-by: Tuong Lien --- net/tipc/link.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/tipc/link.c b/net/tipc/link.c index 467c53a1fb5c..d4675e922a8f 100644 --- a/net/tipc/link.c +++ b/net/tipc/link.c @@ -1065,7 +1065,7 @@ static void tipc_link_update_cwin(struct tipc_link *l, int released, /* Enter fast recovery */ if (unlikely(retransmitted)) { l->ssthresh = max_t(u16, l->window / 2, 300); - l->window = l->ssthresh; + l->window = min_t(u16, l->ssthresh, l->window); return; } /* Enter slow start */ -- 2.13.7 ___ tipc-discussion mailing list tipc-discussion@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tipc-discussion
[tipc-discussion] [PATCH RFC 2/4] tipc: add back link trace events
In the previous commit ("tipc: add Gap ACK blocks support for broadcast link"), we have removed the following link trace events due to the code changes: - tipc_link_bc_ack - tipc_link_retrans This commit adds them back along with some minor changes to adapt to the new code. Signed-off-by: Tuong Lien --- net/tipc/link.c | 3 +++ net/tipc/trace.h | 13 - 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/net/tipc/link.c b/net/tipc/link.c index 1b60ba665504..405ccf597e59 100644 --- a/net/tipc/link.c +++ b/net/tipc/link.c @@ -1517,6 +1517,8 @@ static int tipc_link_advance_transmq(struct tipc_link *l, struct tipc_link *r, bool is_uc = !link_is_bc_sndlink(l); bool bc_has_acked = false; + trace_tipc_link_retrans(r, acked + 1, acked + gap, >transmq); + /* Determine Gap ACK blocks if any for the particular link */ if (ga && is_uc) { /* Get the Gap ACKs, uc part */ @@ -2423,6 +2425,7 @@ int tipc_link_bc_ack_rcv(struct tipc_link *r, u16 acked, u16 gap, if (less(acked, r->acked) || (acked == r->acked && !gap && !ga)) return 0; + trace_tipc_link_bc_ack(r, acked, gap, >transmq); tipc_link_advance_transmq(l, r, acked, gap, ga, xmitq, , ); tipc_link_advance_backlog(l, xmitq); diff --git a/net/tipc/trace.h b/net/tipc/trace.h index 4d8e00483afc..e7535ab75255 100644 --- a/net/tipc/trace.h +++ b/net/tipc/trace.h @@ -299,8 +299,10 @@ DECLARE_EVENT_CLASS(tipc_link_transmq_class, __entry->from = f; __entry->to = t; __entry->len = skb_queue_len(tq); - __entry->fseqno = msg_seqno(buf_msg(skb_peek(tq))); - __entry->lseqno = msg_seqno(buf_msg(skb_peek_tail(tq))); + __entry->fseqno = __entry->len ? + msg_seqno(buf_msg(skb_peek(tq))) : 0; + __entry->lseqno = __entry->len ? + msg_seqno(buf_msg(skb_peek_tail(tq))) : 0; ), TP_printk("<%s> retrans req: [%u-%u] transmq: %u [%u-%u]\n", @@ -308,15 +310,16 @@ DECLARE_EVENT_CLASS(tipc_link_transmq_class, __entry->len, __entry->fseqno, __entry->lseqno) ); -DEFINE_EVENT(tipc_link_transmq_class, tipc_link_retrans, +DEFINE_EVENT_CONDITION(tipc_link_transmq_class, tipc_link_retrans, TP_PROTO(struct tipc_link *r, u16 f, u16 t, struct sk_buff_head *tq), - TP_ARGS(r, f, t, tq) + TP_ARGS(r, f, t, tq), + TP_CONDITION(less_eq(f, t)) ); DEFINE_EVENT_PRINT(tipc_link_transmq_class, tipc_link_bc_ack, TP_PROTO(struct tipc_link *r, u16 f, u16 t, struct sk_buff_head *tq), TP_ARGS(r, f, t, tq), - TP_printk("<%s> acked: [%u-%u] transmq: %u [%u-%u]\n", + TP_printk("<%s> acked: %u gap: %u transmq: %u [%u-%u]\n", __entry->name, __entry->from, __entry->to, __entry->len, __entry->fseqno, __entry->lseqno) ); -- 2.13.7 ___ tipc-discussion mailing list tipc-discussion@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tipc-discussion
[tipc-discussion] [PATCH RFC 4/4] tipc: add support for broadcast rcv stats dumping
This commit enables dumping the statistics of a broadcast-receiver link like the traditional 'broadcast-link' one (which is for broadcast- sender). The link dumping can be triggered via netlink (e.g. the iproute2/tipc tool) by the link flag - 'TIPC_NLA_LINK_BROADCAST' as the indicator. The name of a broadcast-receiver link of a specific peer will be in the format: 'broadcast-link:'. For example: Link Window:50 packets RX packets:7841 fragments:2408/440 bundles:0/0 TX packets:0 fragments:0/0 bundles:0/0 RX naks:0 defs:124 dups:0 TX naks:21 acks:0 retrans:0 Congestion link:0 Send queue max:0 avg:0 In addition, the broadcast-receiver link statistics can be reset in the usual way via netlink by specifying that link name in command. Note: the 'tipc_link_name_ext()' is removed because the link name can now be retrieved simply via the 'l->name'. Signed-off-by: Tuong Lien --- net/tipc/bcast.c | 6 ++--- net/tipc/bcast.h | 5 +++-- net/tipc/link.c| 65 +++--- net/tipc/link.h| 3 +-- net/tipc/msg.c | 9 net/tipc/msg.h | 2 +- net/tipc/netlink.c | 2 +- net/tipc/node.c| 63 +--- net/tipc/trace.h | 4 ++-- 9 files changed, 103 insertions(+), 56 deletions(-) diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c index 50a16f8bebd9..383f87bc1061 100644 --- a/net/tipc/bcast.c +++ b/net/tipc/bcast.c @@ -563,10 +563,8 @@ void tipc_bcast_remove_peer(struct net *net, struct tipc_link *rcv_l) tipc_sk_rcv(net, inputq); } -int tipc_bclink_reset_stats(struct net *net) +int tipc_bclink_reset_stats(struct net *net, struct tipc_link *l) { - struct tipc_link *l = tipc_bc_sndlink(net); - if (!l) return -ENOPROTOOPT; @@ -694,7 +692,7 @@ int tipc_bcast_init(struct net *net) tn->bcbase = bb; spin_lock_init(_net(net)->bclock); - if (!tipc_link_bc_create(net, 0, 0, + if (!tipc_link_bc_create(net, 0, 0, NULL, FB_MTU, BCLINK_WIN_DEFAULT, BCLINK_WIN_DEFAULT, diff --git a/net/tipc/bcast.h b/net/tipc/bcast.h index 97d3cf9d3e4d..4240c95188b1 100644 --- a/net/tipc/bcast.h +++ b/net/tipc/bcast.h @@ -96,9 +96,10 @@ void tipc_bcast_ack_rcv(struct net *net, struct tipc_link *l, int tipc_bcast_sync_rcv(struct net *net, struct tipc_link *l, struct tipc_msg *hdr, struct sk_buff_head *retrq); -int tipc_nl_add_bc_link(struct net *net, struct tipc_nl_msg *msg); +int tipc_nl_add_bc_link(struct net *net, struct tipc_nl_msg *msg, + struct tipc_link *bcl); int tipc_nl_bc_link_set(struct net *net, struct nlattr *attrs[]); -int tipc_bclink_reset_stats(struct net *net); +int tipc_bclink_reset_stats(struct net *net, struct tipc_link *l); u32 tipc_bcast_get_broadcast_mode(struct net *net); u32 tipc_bcast_get_broadcast_ratio(struct net *net); diff --git a/net/tipc/link.c b/net/tipc/link.c index 3071e46f029a..808d3a76c27f 100644 --- a/net/tipc/link.c +++ b/net/tipc/link.c @@ -539,7 +539,7 @@ bool tipc_link_create(struct net *net, char *if_name, int bearer_id, * * Returns true if link was created, otherwise false */ -bool tipc_link_bc_create(struct net *net, u32 ownnode, u32 peer, +bool tipc_link_bc_create(struct net *net, u32 ownnode, u32 peer, u8 *peer_id, int mtu, u32 min_win, u32 max_win, u16 peer_caps, struct sk_buff_head *inputq, struct sk_buff_head *namedq, @@ -554,7 +554,18 @@ bool tipc_link_bc_create(struct net *net, u32 ownnode, u32 peer, return false; l = *link; - strcpy(l->name, tipc_bclink_name); + if (peer_id) { + char peer_str[NODE_ID_STR_LEN] = {0,}; + + tipc_nodeid2string(peer_str, peer_id); + if (strlen(peer_str) > 16) + sprintf(peer_str, "%x", peer); + /* Broadcast receiver link name: "broadcast-link:" */ + snprintf(l->name, sizeof(l->name), "%s:%s", tipc_bclink_name, +peer_str); + } else { + strcpy(l->name, tipc_bclink_name); + } trace_tipc_link_reset(l, TIPC_DUMP_ALL, "bclink created!"); tipc_link_reset(l); l->state = LINK_RESET; @@ -1412,11 +1423,8 @@ static u8 __tipc_build_gap_ack_blks(struct tipc_gap_ack_blks *ga, gacks[n].ack = htons(expect - 1); gacks[n].gap = htons(seqno - expect); if (++n >= MAX_GAP_ACK_BLKS / 2) { - char buf[TIPC_MAX_LINK_NAME]; - pr_info_ratelimited("Gacks on %s: %d, ql: %d!\n", - tipc_link_name_ext(l, buf), -
[tipc-discussion] [PATCH RFC 3/4] tipc: enable broadcast retrans via unicast
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. Signed-off-by: Tuong Lien --- net/tipc/bcast.c | 11 --- net/tipc/bcast.h | 4 +++- net/tipc/link.c | 8 +--- net/tipc/link.h | 3 ++- net/tipc/node.c | 2 +- net/tipc/sysctl.c | 9 - 6 files changed, 27 insertions(+), 10 deletions(-) diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c index 3ce690a96ee9..50a16f8bebd9 100644 --- a/net/tipc/bcast.c +++ b/net/tipc/bcast.c @@ -46,6 +46,7 @@ #define BCLINK_WIN_MIN 32 /* bcast minimum link window size */ const char tipc_bclink_name[] = "broadcast-link"; +unsigned long sysctl_tipc_bc_retruni __read_mostly; /** * struct tipc_bc_base - base structure for keeping broadcast send state @@ -474,7 +475,7 @@ void tipc_bcast_ack_rcv(struct net *net, struct tipc_link *l, __skb_queue_head_init(); tipc_bcast_lock(net); - tipc_link_bc_ack_rcv(l, acked, 0, NULL, ); + tipc_link_bc_ack_rcv(l, acked, 0, NULL, , NULL); tipc_bcast_unlock(net); tipc_bcbase_xmit(net, ); @@ -489,7 +490,8 @@ void tipc_bcast_ack_rcv(struct net *net, struct tipc_link *l, * RCU is locked, no other locks set */ int tipc_bcast_sync_rcv(struct net *net, struct tipc_link *l, - struct tipc_msg *hdr) + struct tipc_msg *hdr, + struct sk_buff_head *retrq) { struct sk_buff_head *inputq = _bc_base(net)->inputq; struct tipc_gap_ack_blks *ga; @@ -503,8 +505,11 @@ int tipc_bcast_sync_rcv(struct net *net, struct tipc_link *l, tipc_link_bc_init_rcv(l, hdr); } else if (!msg_bc_ack_invalid(hdr)) { tipc_get_gap_ack_blks(, l, hdr, false); + if (!sysctl_tipc_bc_retruni) + retrq = rc = tipc_link_bc_ack_rcv(l, msg_bcast_ack(hdr), - msg_bc_gap(hdr), ga, ); + msg_bc_gap(hdr), ga, , + retrq); rc |= tipc_link_bc_sync_rcv(l, hdr, ); } tipc_bcast_unlock(net); diff --git a/net/tipc/bcast.h b/net/tipc/bcast.h index 9e847d9617d3..97d3cf9d3e4d 100644 --- a/net/tipc/bcast.h +++ b/net/tipc/bcast.h @@ -45,6 +45,7 @@ struct tipc_nl_msg; struct tipc_nlist; struct tipc_nitem; extern const char tipc_bclink_name[]; +extern unsigned long sysctl_tipc_bc_retruni; #define TIPC_METHOD_EXPIRE msecs_to_jiffies(5000) @@ -93,7 +94,8 @@ int tipc_bcast_rcv(struct net *net, struct tipc_link *l, struct sk_buff *skb); void tipc_bcast_ack_rcv(struct net *net, struct tipc_link *l, struct tipc_msg *hdr); int tipc_bcast_sync_rcv(struct net *net, struct tipc_link *l, - struct tipc_msg *hdr); + struct tipc_msg *hdr, + struct sk_buff_head *retrq); int tipc_nl_add_bc_link(struct net *net, struct tipc_nl_msg *msg); int tipc_nl_bc_link_set(struct net *net, struct nlattr *attrs[]); int tipc_bclink_reset_stats(struct net *net); diff --git a/net/tipc/link.c b/net/tipc/link.c index 405ccf597e59..3071e46f029a 100644 --- a/net/tipc/link.c +++ b/net/tipc/link.c @@ -375,7 +375,7 @@ void tipc_link_remove_bc_peer(struct tipc_link *snd_l, snd_l->ackers--; rcv_l->bc_peer_is_up = true; rcv_l->state = LINK_ESTABLISHED; - tipc_link_bc_ack_rcv(rcv_l, ack, 0, NULL, xmitq); + tipc_link_bc_ack_rcv(rcv_l, ack, 0, NULL, xmitq, NULL); trace_tipc_link_reset(rcv_l, TIPC_DUMP_ALL, "bclink removed!"); tipc_link_reset(rcv_l); rcv_l->state = LINK_RESET; @@ -2413,7 +2413,8 @@ int tipc_link_bc_sync_rcv(struct tipc_link *l, struct tipc_msg *hdr, 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 =