Re: [PATCH net] ipv6: gro: flush instead of assuming different flows on hop_limit mismatch

From: kernel test robot <lkp@intel.com>
To: Jakub Kicinski <kuba@kernel.org>,
	davem@davemloft.net, edumazet@google.com
Cc: kbuild-all@lists.01.org, dsahern@gmail.com, pabeni@redhat.com,
	herbert@gondor.apana.org.au, netdev@vger.kernel.org,
	Jakub Kicinski <kuba@kernel.org>
Subject: Re: [PATCH net] ipv6: gro: flush instead of assuming different flows on hop_limit mismatch
Date: Tue, 25 Jan 2022 03:23:55 +0800	[thread overview]
Message-ID: <202201250210.roaIok2H-lkp@intel.com> (raw)
In-Reply-To: <20220121011941.1123392-1-kuba@kernel.org>

Hi Jakub,

I love your patch! Perhaps something to improve:

[auto build test WARNING on net/master]

url:    https://github.com/0day-ci/linux/commits/Jakub-Kicinski/ipv6-gro-flush-instead-of-assuming-different-flows-on-hop_limit-mismatch/20220121-092033
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git 57afdc0aab094b4c811b3fe030b2567812a495f3
config: x86_64-randconfig-s022 (https://download.01.org/0day-ci/archive/20220125/202201250210.roaIok2H-lkp@intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.4-dirty
        # https://github.com/0day-ci/linux/commit/6f8f3e541288381a67df8b670068d5add231d082
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Jakub-Kicinski/ipv6-gro-flush-instead-of-assuming-different-flows-on-hop_limit-mismatch/20220121-092033
        git checkout 6f8f3e541288381a67df8b670068d5add231d082
        # save the config file to linux build tree
        mkdir build_dir
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=x86_64 SHELL=/bin/bash net/ipv6/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

sparse warnings: (new ones prefixed by >>)
>> net/ipv6/ip6_offload.c:264:57: sparse: sparse: restricted __be32 degrades to integer
>> net/ipv6/ip6_offload.c:263:48: sparse: sparse: dubious: x | !y

vim +264 net/ipv6/ip6_offload.c

   182	
   183	INDIRECT_CALLABLE_SCOPE struct sk_buff *ipv6_gro_receive(struct list_head *head,
   184								 struct sk_buff *skb)
   185	{
   186		const struct net_offload *ops;
   187		struct sk_buff *pp = NULL;
   188		struct sk_buff *p;
   189		struct ipv6hdr *iph;
   190		unsigned int nlen;
   191		unsigned int hlen;
   192		unsigned int off;
   193		u16 flush = 1;
   194		int proto;
   195	
   196		off = skb_gro_offset(skb);
   197		hlen = off + sizeof(*iph);
   198		iph = skb_gro_header_fast(skb, off);
   199		if (skb_gro_header_hard(skb, hlen)) {
   200			iph = skb_gro_header_slow(skb, hlen, off);
   201			if (unlikely(!iph))
   202				goto out;
   203		}
   204	
   205		skb_set_network_header(skb, off);
   206		skb_gro_pull(skb, sizeof(*iph));
   207		skb_set_transport_header(skb, skb_gro_offset(skb));
   208	
   209		flush += ntohs(iph->payload_len) != skb_gro_len(skb);
   210	
   211		proto = iph->nexthdr;
   212		ops = rcu_dereference(inet6_offloads[proto]);
   213		if (!ops || !ops->callbacks.gro_receive) {
   214			__pskb_pull(skb, skb_gro_offset(skb));
   215			skb_gro_frag0_invalidate(skb);
   216			proto = ipv6_gso_pull_exthdrs(skb, proto);
   217			skb_gro_pull(skb, -skb_transport_offset(skb));
   218			skb_reset_transport_header(skb);
   219			__skb_push(skb, skb_gro_offset(skb));
   220	
   221			ops = rcu_dereference(inet6_offloads[proto]);
   222			if (!ops || !ops->callbacks.gro_receive)
   223				goto out;
   224	
   225			iph = ipv6_hdr(skb);
   226		}
   227	
   228		NAPI_GRO_CB(skb)->proto = proto;
   229	
   230		flush--;
   231		nlen = skb_network_header_len(skb);
   232	
   233		list_for_each_entry(p, head, list) {
   234			const struct ipv6hdr *iph2;
   235			__be32 first_word; /* <Version:4><Traffic_Class:8><Flow_Label:20> */
   236	
   237			if (!NAPI_GRO_CB(p)->same_flow)
   238				continue;
   239	
   240			iph2 = (struct ipv6hdr *)(p->data + off);
   241			first_word = *(__be32 *)iph ^ *(__be32 *)iph2;
   242	
   243			/* All fields must match except length and Traffic Class.
   244			 * XXX skbs on the gro_list have all been parsed and pulled
   245			 * already so we don't need to compare nlen
   246			 * (nlen != (sizeof(*iph2) + ipv6_exthdrs_len(iph2, &ops)))
   247			 * memcmp() alone below is sufficient, right?
   248			 */
   249			 if ((first_word & htonl(0xF00FFFFF)) ||
   250			     !ipv6_addr_equal(&iph->saddr, &iph2->saddr) ||
   251			     !ipv6_addr_equal(&iph->daddr, &iph2->daddr) ||
   252			     iph->nexthdr != iph2->nexthdr) {
   253	not_same_flow:
   254				NAPI_GRO_CB(p)->same_flow = 0;
   255				continue;
   256			}
   257			if (unlikely(nlen > sizeof(struct ipv6hdr))) {
   258				if (memcmp(iph + 1, iph2 + 1,
   259					   nlen - sizeof(struct ipv6hdr)))
   260					goto not_same_flow;
   261			}
   262			/* flush if Traffic Class fields are different */
 > 263			NAPI_GRO_CB(p)->flush |= flush |
 > 264						 !!((first_word & htonl(0x0FF00000)) |
   265						    (iph->hop_limit ^ iph2->hop_limit));
   266	
   267			/* If the previous IP ID value was based on an atomic
   268			 * datagram we can overwrite the value and ignore it.
   269			 */
   270			if (NAPI_GRO_CB(skb)->is_atomic)
   271				NAPI_GRO_CB(p)->flush_id = 0;
   272		}
   273	
   274		NAPI_GRO_CB(skb)->is_atomic = true;
   275		NAPI_GRO_CB(skb)->flush |= flush;
   276	
   277		skb_gro_postpull_rcsum(skb, iph, nlen);
   278	
   279		pp = indirect_call_gro_receive_l4(tcp6_gro_receive, udp6_gro_receive,
   280						 ops->callbacks.gro_receive, head, skb);
   281	
   282	out:
   283		skb_gro_flush_final(skb, pp, flush);
   284	
   285		return pp;
   286	}
   287	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org