* [net] tipc: fix uninitialized value for broadcast retransmission
@ 2018-12-19 2:52 Hoang Le
2018-12-19 3:58 ` kbuild test robot
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Hoang Le @ 2018-12-19 2:52 UTC (permalink / raw)
To: tipc-discussion, jon.maloy, maloy, ying.xue, netdev
When sending broadcast message on high load system, there are a lot of
unnecessary packets restranmission. That issue was caused by missing in
initial criteria for retransmission.
To prevent this happen, just initialize this criteria for retransmission
in next 10 milliseconds.
Fixes: 31c4f4cc32f7 tipc: improve broadcast retransmission algorithm
Acked-by: Ying Xue <ying.xue@windriver.com>
Acked-by: Jon Maloy <jon.maloy@ericsson.com>
Signed-off-by: Hoang Le <hoang.h.le@dektech.com.au>
---
net/tipc/link.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/net/tipc/link.c b/net/tipc/link.c
index 9e265eb89726..d6a4b6ae5504 100644
--- a/net/tipc/link.c
+++ b/net/tipc/link.c
@@ -945,6 +945,10 @@ int tipc_link_xmit(struct tipc_link *l, struct sk_buff_head *list,
}
__skb_dequeue(list);
__skb_queue_tail(transmq, skb);
+ /* next retransmit attempt */
+ if (link_is_bc_sndlink(l))
+ TIPC_SKB_CB(skb)->nxt_retr =
+ jiffies + TIPC_BC_RETR_LIM;
__skb_queue_tail(xmitq, _skb);
TIPC_SKB_CB(skb)->ackers = l->ackers;
l->rcv_unacked = 0;
@@ -992,6 +996,10 @@ static void tipc_link_advance_backlog(struct tipc_link *l,
hdr = buf_msg(skb);
l->backlog[msg_importance(hdr)].len--;
__skb_queue_tail(&l->transmq, skb);
+ /* next retransmit attempt */
+ if (link_is_bc_sndlink(l))
+ TIPC_SKB_CB(skb)->nxt_retr = jiffies + TIPC_BC_RETR_LIM;
+
__skb_queue_tail(xmitq, _skb);
TIPC_SKB_CB(skb)->ackers = l->ackers;
msg_set_seqno(hdr, seqno);
--
2.17.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [net] tipc: fix uninitialized value for broadcast retransmission
2018-12-19 2:52 [net] tipc: fix uninitialized value for broadcast retransmission Hoang Le
@ 2018-12-19 3:58 ` kbuild test robot
2018-12-19 4:35 ` kbuild test robot
2018-12-19 19:51 ` David Miller
2 siblings, 0 replies; 4+ messages in thread
From: kbuild test robot @ 2018-12-19 3:58 UTC (permalink / raw)
To: Hoang Le; +Cc: kbuild-all, tipc-discussion, jon.maloy, maloy, ying.xue, netdev
[-- Attachment #1: Type: text/plain, Size: 5032 bytes --]
Hi Hoang,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on net/master]
url: https://github.com/0day-ci/linux/commits/Hoang-Le/tipc-fix-uninitialized-value-for-broadcast-retransmission/20181219-112414
config: i386-randconfig-x002-201850 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
net//tipc/link.c: In function 'tipc_link_xmit':
>> net//tipc/link.c:953:21: error: 'struct tipc_skb_cb' has no member named 'nxt_retr'
TIPC_SKB_CB(skb)->nxt_retr =
^~
>> net//tipc/link.c:954:16: error: 'TIPC_BC_RETR_LIM' undeclared (first use in this function); did you mean 'TIPC_BC_RETR_LIMIT'?
jiffies + TIPC_BC_RETR_LIM;
^~~~~~~~~~~~~~~~
TIPC_BC_RETR_LIMIT
net//tipc/link.c:954:16: note: each undeclared identifier is reported only once for each function it appears in
net//tipc/link.c: In function 'tipc_link_advance_backlog':
net//tipc/link.c:1004:20: error: 'struct tipc_skb_cb' has no member named 'nxt_retr'
TIPC_SKB_CB(skb)->nxt_retr = jiffies + TIPC_BC_RETR_LIM;
^~
net//tipc/link.c:1004:43: error: 'TIPC_BC_RETR_LIM' undeclared (first use in this function); did you mean 'TIPC_BC_RETR_LIMIT'?
TIPC_SKB_CB(skb)->nxt_retr = jiffies + TIPC_BC_RETR_LIM;
^~~~~~~~~~~~~~~~
TIPC_BC_RETR_LIMIT
vim +953 net//tipc/link.c
889
890 /**
891 * tipc_link_xmit(): enqueue buffer list according to queue situation
892 * @link: link to use
893 * @list: chain of buffers containing message
894 * @xmitq: returned list of packets to be sent by caller
895 *
896 * Consumes the buffer chain.
897 * Returns 0 if success, or errno: -ELINKCONG, -EMSGSIZE or -ENOBUFS
898 * Messages at TIPC_SYSTEM_IMPORTANCE are always accepted
899 */
900 int tipc_link_xmit(struct tipc_link *l, struct sk_buff_head *list,
901 struct sk_buff_head *xmitq)
902 {
903 struct tipc_msg *hdr = buf_msg(skb_peek(list));
904 unsigned int maxwin = l->window;
905 int imp = msg_importance(hdr);
906 unsigned int mtu = l->mtu;
907 u16 ack = l->rcv_nxt - 1;
908 u16 seqno = l->snd_nxt;
909 u16 bc_ack = l->bc_rcvlink->rcv_nxt - 1;
910 struct sk_buff_head *transmq = &l->transmq;
911 struct sk_buff_head *backlogq = &l->backlogq;
912 struct sk_buff *skb, *_skb, *bskb;
913 int pkt_cnt = skb_queue_len(list);
914 int rc = 0;
915
916 if (unlikely(msg_size(hdr) > mtu)) {
917 skb_queue_purge(list);
918 return -EMSGSIZE;
919 }
920
921 /* Allow oversubscription of one data msg per source at congestion */
922 if (unlikely(l->backlog[imp].len >= l->backlog[imp].limit)) {
923 if (imp == TIPC_SYSTEM_IMPORTANCE) {
924 pr_warn("%s<%s>, link overflow", link_rst_msg, l->name);
925 return -ENOBUFS;
926 }
927 rc = link_schedule_user(l, hdr);
928 }
929
930 if (pkt_cnt > 1) {
931 l->stats.sent_fragmented++;
932 l->stats.sent_fragments += pkt_cnt;
933 }
934
935 /* Prepare each packet for sending, and add to relevant queue: */
936 while (skb_queue_len(list)) {
937 skb = skb_peek(list);
938 hdr = buf_msg(skb);
939 msg_set_seqno(hdr, seqno);
940 msg_set_ack(hdr, ack);
941 msg_set_bcast_ack(hdr, bc_ack);
942
943 if (likely(skb_queue_len(transmq) < maxwin)) {
944 _skb = skb_clone(skb, GFP_ATOMIC);
945 if (!_skb) {
946 skb_queue_purge(list);
947 return -ENOBUFS;
948 }
949 __skb_dequeue(list);
950 __skb_queue_tail(transmq, skb);
951 /* next retransmit attempt */
952 if (link_is_bc_sndlink(l))
> 953 TIPC_SKB_CB(skb)->nxt_retr =
> 954 jiffies + TIPC_BC_RETR_LIM;
955 __skb_queue_tail(xmitq, _skb);
956 TIPC_SKB_CB(skb)->ackers = l->ackers;
957 l->rcv_unacked = 0;
958 l->stats.sent_pkts++;
959 seqno++;
960 continue;
961 }
962 if (tipc_msg_bundle(skb_peek_tail(backlogq), hdr, mtu)) {
963 kfree_skb(__skb_dequeue(list));
964 l->stats.sent_bundled++;
965 continue;
966 }
967 if (tipc_msg_make_bundle(&bskb, hdr, mtu, l->addr)) {
968 kfree_skb(__skb_dequeue(list));
969 __skb_queue_tail(backlogq, bskb);
970 l->backlog[msg_importance(buf_msg(bskb))].len++;
971 l->stats.sent_bundled++;
972 l->stats.sent_bundles++;
973 continue;
974 }
975 l->backlog[imp].len += skb_queue_len(list);
976 skb_queue_splice_tail_init(list, backlogq);
977 }
978 l->snd_nxt = seqno;
979 return rc;
980 }
981
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28258 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [net] tipc: fix uninitialized value for broadcast retransmission
2018-12-19 2:52 [net] tipc: fix uninitialized value for broadcast retransmission Hoang Le
2018-12-19 3:58 ` kbuild test robot
@ 2018-12-19 4:35 ` kbuild test robot
2018-12-19 19:51 ` David Miller
2 siblings, 0 replies; 4+ messages in thread
From: kbuild test robot @ 2018-12-19 4:35 UTC (permalink / raw)
To: Hoang Le; +Cc: kbuild-all, tipc-discussion, jon.maloy, maloy, ying.xue, netdev
[-- Attachment #1: Type: text/plain, Size: 5025 bytes --]
Hi Hoang,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on net/master]
url: https://github.com/0day-ci/linux/commits/Hoang-Le/tipc-fix-uninitialized-value-for-broadcast-retransmission/20181219-112414
config: nds32-allmodconfig (attached as .config)
compiler: nds32le-linux-gcc (GCC) 6.4.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=6.4.0 make.cross ARCH=nds32
All errors (new ones prefixed by >>):
net/tipc/link.c: In function 'tipc_link_xmit':
net/tipc/link.c:953:21: error: 'struct tipc_skb_cb' has no member named 'nxt_retr'
TIPC_SKB_CB(skb)->nxt_retr =
^~
>> net/tipc/link.c:954:16: error: 'TIPC_BC_RETR_LIM' undeclared (first use in this function)
jiffies + TIPC_BC_RETR_LIM;
^~~~~~~~~~~~~~~~
net/tipc/link.c:954:16: note: each undeclared identifier is reported only once for each function it appears in
net/tipc/link.c: In function 'tipc_link_advance_backlog':
net/tipc/link.c:1004:20: error: 'struct tipc_skb_cb' has no member named 'nxt_retr'
TIPC_SKB_CB(skb)->nxt_retr = jiffies + TIPC_BC_RETR_LIM;
^~
net/tipc/link.c:1004:43: error: 'TIPC_BC_RETR_LIM' undeclared (first use in this function)
TIPC_SKB_CB(skb)->nxt_retr = jiffies + TIPC_BC_RETR_LIM;
^~~~~~~~~~~~~~~~
vim +/TIPC_BC_RETR_LIM +954 net/tipc/link.c
889
890 /**
891 * tipc_link_xmit(): enqueue buffer list according to queue situation
892 * @link: link to use
893 * @list: chain of buffers containing message
894 * @xmitq: returned list of packets to be sent by caller
895 *
896 * Consumes the buffer chain.
897 * Returns 0 if success, or errno: -ELINKCONG, -EMSGSIZE or -ENOBUFS
898 * Messages at TIPC_SYSTEM_IMPORTANCE are always accepted
899 */
900 int tipc_link_xmit(struct tipc_link *l, struct sk_buff_head *list,
901 struct sk_buff_head *xmitq)
902 {
903 struct tipc_msg *hdr = buf_msg(skb_peek(list));
904 unsigned int maxwin = l->window;
905 int imp = msg_importance(hdr);
906 unsigned int mtu = l->mtu;
907 u16 ack = l->rcv_nxt - 1;
908 u16 seqno = l->snd_nxt;
909 u16 bc_ack = l->bc_rcvlink->rcv_nxt - 1;
910 struct sk_buff_head *transmq = &l->transmq;
911 struct sk_buff_head *backlogq = &l->backlogq;
912 struct sk_buff *skb, *_skb, *bskb;
913 int pkt_cnt = skb_queue_len(list);
914 int rc = 0;
915
916 if (unlikely(msg_size(hdr) > mtu)) {
917 skb_queue_purge(list);
918 return -EMSGSIZE;
919 }
920
921 /* Allow oversubscription of one data msg per source at congestion */
922 if (unlikely(l->backlog[imp].len >= l->backlog[imp].limit)) {
923 if (imp == TIPC_SYSTEM_IMPORTANCE) {
924 pr_warn("%s<%s>, link overflow", link_rst_msg, l->name);
925 return -ENOBUFS;
926 }
927 rc = link_schedule_user(l, hdr);
928 }
929
930 if (pkt_cnt > 1) {
931 l->stats.sent_fragmented++;
932 l->stats.sent_fragments += pkt_cnt;
933 }
934
935 /* Prepare each packet for sending, and add to relevant queue: */
936 while (skb_queue_len(list)) {
937 skb = skb_peek(list);
938 hdr = buf_msg(skb);
939 msg_set_seqno(hdr, seqno);
940 msg_set_ack(hdr, ack);
941 msg_set_bcast_ack(hdr, bc_ack);
942
943 if (likely(skb_queue_len(transmq) < maxwin)) {
944 _skb = skb_clone(skb, GFP_ATOMIC);
945 if (!_skb) {
946 skb_queue_purge(list);
947 return -ENOBUFS;
948 }
949 __skb_dequeue(list);
950 __skb_queue_tail(transmq, skb);
951 /* next retransmit attempt */
952 if (link_is_bc_sndlink(l))
> 953 TIPC_SKB_CB(skb)->nxt_retr =
> 954 jiffies + TIPC_BC_RETR_LIM;
955 __skb_queue_tail(xmitq, _skb);
956 TIPC_SKB_CB(skb)->ackers = l->ackers;
957 l->rcv_unacked = 0;
958 l->stats.sent_pkts++;
959 seqno++;
960 continue;
961 }
962 if (tipc_msg_bundle(skb_peek_tail(backlogq), hdr, mtu)) {
963 kfree_skb(__skb_dequeue(list));
964 l->stats.sent_bundled++;
965 continue;
966 }
967 if (tipc_msg_make_bundle(&bskb, hdr, mtu, l->addr)) {
968 kfree_skb(__skb_dequeue(list));
969 __skb_queue_tail(backlogq, bskb);
970 l->backlog[msg_importance(buf_msg(bskb))].len++;
971 l->stats.sent_bundled++;
972 l->stats.sent_bundles++;
973 continue;
974 }
975 l->backlog[imp].len += skb_queue_len(list);
976 skb_queue_splice_tail_init(list, backlogq);
977 }
978 l->snd_nxt = seqno;
979 return rc;
980 }
981
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 48501 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [net] tipc: fix uninitialized value for broadcast retransmission
2018-12-19 2:52 [net] tipc: fix uninitialized value for broadcast retransmission Hoang Le
2018-12-19 3:58 ` kbuild test robot
2018-12-19 4:35 ` kbuild test robot
@ 2018-12-19 19:51 ` David Miller
2 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2018-12-19 19:51 UTC (permalink / raw)
To: hoang.h.le; +Cc: tipc-discussion, jon.maloy, maloy, ying.xue, netdev
From: Hoang Le <hoang.h.le@dektech.com.au>
Date: Wed, 19 Dec 2018 09:52:52 +0700
> When sending broadcast message on high load system, there are a lot of
> unnecessary packets restranmission. That issue was caused by missing in
> initial criteria for retransmission.
>
> To prevent this happen, just initialize this criteria for retransmission
> in next 10 milliseconds.
>
> Fixes: 31c4f4cc32f7 tipc: improve broadcast retransmission algorithm
> Acked-by: Ying Xue <ying.xue@windriver.com>
> Acked-by: Jon Maloy <jon.maloy@ericsson.com>
> Signed-off-by: Hoang Le <hoang.h.le@dektech.com.au>
Applied, thank you.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-12-19 19:51 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-19 2:52 [net] tipc: fix uninitialized value for broadcast retransmission Hoang Le
2018-12-19 3:58 ` kbuild test robot
2018-12-19 4:35 ` kbuild test robot
2018-12-19 19:51 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).