Re: [PATCH] net: ipv4: Use BUG_ON directly instead of a if condition followed by BUG

From: kbuild test robot
Date: Mon Sep 10 2018 - 17:21:24 EST


Hi zhong,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net/master]
[also build test ERROR on v4.19-rc3 next-20180910]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/zhong-jiang/net-ipv4-Use-BUG_ON-directly-instead-of-a-if-condition-followed-by-BUG/20180911-034152
config: mips-rt305x_defconfig (attached as .config)
compiler: mipsel-linux-gnu-gcc (Debian 7.2.0-11) 7.2.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=7.2.0 make.cross ARCH=mips

All errors (new ones prefixed by >>):

net/ipv4/tcp_input.c: In function 'tcp_collapse':
>> net/ipv4/tcp_input.c:4924:5: error: too many arguments to function 'BUG'
BUG(skb_copy_bits(skb, offset,
^~~
In file included from include/linux/bug.h:5:0,
from include/linux/mmdebug.h:5,
from include/linux/mm.h:9,
from net/ipv4/tcp_input.c:67:
arch/mips/include/asm/bug.h:12:31: note: declared here
static inline void __noreturn BUG(void)
^~~
net/ipv4/tcp_input.c: In function 'tcp_urg':
net/ipv4/tcp_input.c:5318:4: error: too many arguments to function 'BUG'
BUG(skb_copy_bits(skb, ptr, &tmp, 1));
^~~
In file included from include/linux/bug.h:5:0,
from include/linux/mmdebug.h:5,
from include/linux/mm.h:9,
from net/ipv4/tcp_input.c:67:
arch/mips/include/asm/bug.h:12:31: note: declared here
static inline void __noreturn BUG(void)
^~~

vim +/BUG +4924 net/ipv4/tcp_input.c

4838
4839 /* Collapse contiguous sequence of skbs head..tail with
4840 * sequence numbers start..end.
4841 *
4842 * If tail is NULL, this means until the end of the queue.
4843 *
4844 * Segments with FIN/SYN are not collapsed (only because this
4845 * simplifies code)
4846 */
4847 static void
4848 tcp_collapse(struct sock *sk, struct sk_buff_head *list, struct rb_root *root,
4849 struct sk_buff *head, struct sk_buff *tail, u32 start, u32 end)
4850 {
4851 struct sk_buff *skb = head, *n;
4852 struct sk_buff_head tmp;
4853 bool end_of_skbs;
4854
4855 /* First, check that queue is collapsible and find
4856 * the point where collapsing can be useful.
4857 */
4858 restart:
4859 for (end_of_skbs = true; skb != NULL && skb != tail; skb = n) {
4860 n = tcp_skb_next(skb, list);
4861
4862 /* No new bits? It is possible on ofo queue. */
4863 if (!before(start, TCP_SKB_CB(skb)->end_seq)) {
4864 skb = tcp_collapse_one(sk, skb, list, root);
4865 if (!skb)
4866 break;
4867 goto restart;
4868 }
4869
4870 /* The first skb to collapse is:
4871 * - not SYN/FIN and
4872 * - bloated or contains data before "start" or
4873 * overlaps to the next one.
4874 */
4875 if (!(TCP_SKB_CB(skb)->tcp_flags & (TCPHDR_SYN | TCPHDR_FIN)) &&
4876 (tcp_win_from_space(sk, skb->truesize) > skb->len ||
4877 before(TCP_SKB_CB(skb)->seq, start))) {
4878 end_of_skbs = false;
4879 break;
4880 }
4881
4882 if (n && n != tail &&
4883 TCP_SKB_CB(skb)->end_seq != TCP_SKB_CB(n)->seq) {
4884 end_of_skbs = false;
4885 break;
4886 }
4887
4888 /* Decided to skip this, advance start seq. */
4889 start = TCP_SKB_CB(skb)->end_seq;
4890 }
4891 if (end_of_skbs ||
4892 (TCP_SKB_CB(skb)->tcp_flags & (TCPHDR_SYN | TCPHDR_FIN)))
4893 return;
4894
4895 __skb_queue_head_init(&tmp);
4896
4897 while (before(start, end)) {
4898 int copy = min_t(int, SKB_MAX_ORDER(0, 0), end - start);
4899 struct sk_buff *nskb;
4900
4901 nskb = alloc_skb(copy, GFP_ATOMIC);
4902 if (!nskb)
4903 break;
4904
4905 memcpy(nskb->cb, skb->cb, sizeof(skb->cb));
4906 #ifdef CONFIG_TLS_DEVICE
4907 nskb->decrypted = skb->decrypted;
4908 #endif
4909 TCP_SKB_CB(nskb)->seq = TCP_SKB_CB(nskb)->end_seq = start;
4910 if (list)
4911 __skb_queue_before(list, skb, nskb);
4912 else
4913 __skb_queue_tail(&tmp, nskb); /* defer rbtree insertion */
4914 skb_set_owner_r(nskb, sk);
4915
4916 /* Copy data, releasing collapsed skbs. */
4917 while (copy > 0) {
4918 int offset = start - TCP_SKB_CB(skb)->seq;
4919 int size = TCP_SKB_CB(skb)->end_seq - start;
4920
4921 BUG_ON(offset < 0);
4922 if (size > 0) {
4923 size = min(copy, size);
> 4924 BUG(skb_copy_bits(skb, offset,
4925 skb_put(nskb, size), size));
4926 TCP_SKB_CB(nskb)->end_seq += size;
4927 copy -= size;
4928 start += size;
4929 }
4930 if (!before(start, TCP_SKB_CB(skb)->end_seq)) {
4931 skb = tcp_collapse_one(sk, skb, list, root);
4932 if (!skb ||
4933 skb == tail ||
4934 (TCP_SKB_CB(skb)->tcp_flags & (TCPHDR_SYN | TCPHDR_FIN)))
4935 goto end;
4936 #ifdef CONFIG_TLS_DEVICE
4937 if (skb->decrypted != nskb->decrypted)
4938 goto end;
4939 #endif
4940 }
4941 }
4942 }
4943 end:
4944 skb_queue_walk_safe(&tmp, skb, n)
4945 tcp_rbtree_insert(root, skb);
4946 }
4947

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

Attachment: .config.gz
Description: application/gzip