Re: [PATCH v3 3/4] mptcp: build ADD_ADDR/echo-ADD_ADDR option according pm.add_signal

[kernel test robot <lkp@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 9971 bytes --]

Hi Yonglong,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on mptcp/export]
[also build test WARNING on linus/master v5.13-rc6 next-20210617]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Yonglong-Li/mptcp-fix-conflicts-when-using-pm-add_signal-in-ADD_ADDR-echo-and-RM_ADDR-process/20210617-171559
base:   https://github.com/multipath-tcp/mptcp_net-next.git export
config: x86_64-randconfig-a015-20210617 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 64720f57bea6a6bf033feef4a5751ab9c0c3b401)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # https://github.com/0day-ci/linux/commit/dcb008513c667a57c48dd885599f2d760c8cf7eb
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Yonglong-Li/mptcp-fix-conflicts-when-using-pm-add_signal-in-ADD_ADDR-echo-and-RM_ADDR-process/20210617-171559
        git checkout dcb008513c667a57c48dd885599f2d760c8cf7eb
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

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

All warnings (new ones prefixed by >>):

   net/mptcp/options.c:567:21: warning: parameter 'remaining' set but not used [-Wunused-but-set-parameter]
                                             unsigned int remaining,
                                                          ^
>> net/mptcp/options.c:698:9: warning: variable 'flags' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
           } else if (mptcp_pm_should_add_signal_addr(msk)) {
                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler.h:56:28: note: expanded from macro 'if'
   #define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler.h:58:30: note: expanded from macro '__trace_if_var'
   #define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
                                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   net/mptcp/options.c:726:34: note: uninitialized use occurs here
           WRITE_ONCE(msk->pm.addr_signal, flags | msk->pm.addr_signal);
                                           ^~~~~
   include/asm-generic/rwonce.h:61:18: note: expanded from macro 'WRITE_ONCE'
           __WRITE_ONCE(x, val);                                           \
                           ^~~
   include/asm-generic/rwonce.h:55:33: note: expanded from macro '__WRITE_ONCE'
           *(volatile typeof(x) *)&(x) = (val);                            \
                                          ^~~
   net/mptcp/options.c:698:9: note: remove the 'if' if its condition is always true
           } else if (mptcp_pm_should_add_signal_addr(msk)) {
                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler.h:56:23: note: expanded from macro 'if'
   #define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
                         ^
   net/mptcp/options.c:669:20: note: initialize the variable 'flags' to silence this warning
           u8 add_addr, flags;
                             ^
                              = '\0'
   2 warnings generated.


vim +698 net/mptcp/options.c

   563	
   564	static bool mptcp_established_options_dss(struct sock *sk, struct sk_buff *skb,
   565						  bool snd_data_fin_enable,
   566						  unsigned int *size,
 > 567						  unsigned int remaining,
   568						  struct mptcp_out_options *opts)
   569	{
   570		struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(sk);
   571		struct mptcp_sock *msk = mptcp_sk(subflow->conn);
   572		unsigned int dss_size = 0;
   573		struct mptcp_ext *mpext;
   574		unsigned int ack_size;
   575		bool ret = false;
   576		u64 ack_seq;
   577	
   578		opts->csum_reqd = READ_ONCE(msk->csum_enabled);
   579		mpext = skb ? mptcp_get_ext(skb) : NULL;
   580	
   581		if (!skb || (mpext && mpext->use_map) || snd_data_fin_enable) {
   582			unsigned int map_size = TCPOLEN_MPTCP_DSS_BASE + TCPOLEN_MPTCP_DSS_MAP64;
   583	
   584			if (mpext) {
   585				if (opts->csum_reqd)
   586					map_size += TCPOLEN_MPTCP_DSS_CHECKSUM;
   587	
   588				opts->ext_copy = *mpext;
   589			}
   590	
   591			remaining -= map_size;
   592			dss_size = map_size;
   593			if (skb && snd_data_fin_enable)
   594				mptcp_write_data_fin(subflow, skb, &opts->ext_copy);
   595			ret = true;
   596		}
   597	
   598		/* passive sockets msk will set the 'can_ack' after accept(), even
   599		 * if the first subflow may have the already the remote key handy
   600		 */
   601		opts->ext_copy.use_ack = 0;
   602		if (!READ_ONCE(msk->can_ack)) {
   603			*size = ALIGN(dss_size, 4);
   604			return ret;
   605		}
   606	
   607		ack_seq = READ_ONCE(msk->ack_seq);
   608		if (READ_ONCE(msk->use_64bit_ack)) {
   609			ack_size = TCPOLEN_MPTCP_DSS_ACK64;
   610			opts->ext_copy.data_ack = ack_seq;
   611			opts->ext_copy.ack64 = 1;
   612		} else {
   613			ack_size = TCPOLEN_MPTCP_DSS_ACK32;
   614			opts->ext_copy.data_ack32 = (uint32_t)ack_seq;
   615			opts->ext_copy.ack64 = 0;
   616		}
   617		opts->ext_copy.use_ack = 1;
   618		WRITE_ONCE(msk->old_wspace, __mptcp_space((struct sock *)msk));
   619	
   620		/* Add kind/length/subtype/flag overhead if mapping is not populated */
   621		if (dss_size == 0)
   622			ack_size += TCPOLEN_MPTCP_DSS_BASE;
   623	
   624		dss_size += ack_size;
   625	
   626		*size = ALIGN(dss_size, 4);
   627		return true;
   628	}
   629	
   630	static u64 add_addr_generate_hmac(u64 key1, u64 key2,
   631					  struct mptcp_addr_info *addr)
   632	{
   633		u16 port = ntohs(addr->port);
   634		u8 hmac[SHA256_DIGEST_SIZE];
   635		u8 msg[19];
   636		int i = 0;
   637	
   638		msg[i++] = addr->id;
   639		if (addr->family == AF_INET) {
   640			memcpy(&msg[i], &addr->addr.s_addr, 4);
   641			i += 4;
   642		}
   643	#if IS_ENABLED(CONFIG_MPTCP_IPV6)
   644		else if (addr->family == AF_INET6) {
   645			memcpy(&msg[i], &addr->addr6.s6_addr, 16);
   646			i += 16;
   647		}
   648	#endif
   649		msg[i++] = port >> 8;
   650		msg[i++] = port & 0xFF;
   651	
   652		mptcp_crypto_hmac_sha(key1, key2, msg, i, hmac);
   653	
   654		return get_unaligned_be64(&hmac[SHA256_DIGEST_SIZE - sizeof(u64)]);
   655	}
   656	
   657	static bool mptcp_established_options_add_addr(struct sock *sk, struct sk_buff *skb,
   658						       unsigned int *size,
   659						       unsigned int remaining,
   660						       struct mptcp_out_options *opts)
   661	{
   662		struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(sk);
   663		struct mptcp_sock *msk = mptcp_sk(subflow->conn);
   664		bool drop_other_suboptions = false;
   665		unsigned int opt_size = *size;
   666		struct mptcp_addr_info remote;
   667		struct mptcp_addr_info local;
   668		int ret = false;
   669		u8 add_addr, flags;
   670		int len;
   671	
   672		if (!mptcp_pm_should_add_signal(msk))
   673			goto out;
   674	
   675		*size = 0;
   676		mptcp_pm_add_addr_signal(msk, &local, &remote, &add_addr);
   677		if (mptcp_pm_should_add_signal_echo(msk)) {
   678			if (skb && skb_is_tcp_pure_ack(skb)) {
   679				pr_debug("drop other suboptions");
   680				opts->suboptions = 0;
   681				opts->ext_copy.use_ack = 0;
   682				opts->ext_copy.use_map = 0;
   683				remaining += opt_size;
   684				drop_other_suboptions = true;
   685			}
   686			len = mptcp_add_addr_len(remote.family, true, !!remote.port);
   687			if (remaining < len && mptcp_pm_should_add_signal_addr(msk))
   688				goto add_addr;
   689			else if (remaining < len)
   690				goto out;
   691			remaining -= len;
   692			*size += len;
   693			opts->remote = remote;
   694			flags = (u8)~BIT(MPTCP_ADD_ADDR_ECHO);
   695			opts->suboptions |= OPTION_MPTCP_ADD_ECHO;
   696			pr_debug("addr_id=%d, echo=1, port=%d addr_signal:%x",
   697				 opts->remote.id, ntohs(opts->remote.port), add_addr);
 > 698		} else if (mptcp_pm_should_add_signal_addr(msk)) {
   699	add_addr:
   700			if ((local.family == AF_INET6 || local.port) && skb &&
   701			    skb_is_tcp_pure_ack(skb)) {
   702				pr_debug("drop other suboptions");
   703				opts->suboptions = 0;
   704				opts->ext_copy.use_ack = 0;
   705				opts->ext_copy.use_map = 0;
   706				remaining += opt_size;
   707				drop_other_suboptions = true;
   708			}
   709			len = mptcp_add_addr_len(local.family, false, !!local.port);
   710			if (remaining < len)
   711				goto out;
   712			*size += len;
   713			opts->addr = local;
   714			opts->ahmac = add_addr_generate_hmac(msk->local_key,
   715							     msk->remote_key,
   716							     &opts->addr);
   717			opts->suboptions |= OPTION_MPTCP_ADD_ADDR;
   718			flags = (u8)~BIT(MPTCP_ADD_ADDR_SIGNAL);
   719			pr_debug("addr_id=%d, ahmac=%llu, echo=0, port=%d, addr_signal:%x",
   720				 opts->addr.id, opts->ahmac, ntohs(opts->addr.port), add_addr);
   721		}
   722	
   723		if (drop_other_suboptions)
   724			*size -= opt_size;
   725		spin_lock_bh(&msk->pm.lock);
   726		WRITE_ONCE(msk->pm.addr_signal, flags | msk->pm.addr_signal);
   727		spin_unlock_bh(&msk->pm.lock);
   728		ret = true;
   729	
   730	out:
   731		return ret;
   732	}
   733	

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 31950 bytes --]