diff mbox series

[net-next,2/3] tipc: make node number calculation reproducible

Message ID 20201124172834.317966-3-jmaloy@redhat.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series tipc: some minor improvements | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for net-next
netdev/subject_prefix success Link
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit fail Errors and warnings before: 263 this patch: 275
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 53 lines checked
netdev/build_allmodconfig_warn fail Errors and warnings before: 263 this patch: 275
netdev/header_inline success Link
netdev/stable success Stable not CCed

Commit Message

Jon Maloy Nov. 24, 2020, 5:28 p.m. UTC 
From: Jon Maloy <jmaloy@redhat.com>

The 32-bit node number, aka node hash or node address, is calculated
based on the 128-bit node identity when it is not set explicitly by
the user. In future commits we will need to perform this hash operation
on peer nodes while feeling safe that we obtain the same result.

We do this by interpreting the initial hash as a network byte order
number. Whenever we need to use the number locally on a node
we must therefore translate it to host byte order to obtain an
architecure independent result.

Furthermore, given the context where we use this number, we must not
allow it to be zero unless the node identity also is zero. Hence, in
the rare cases when the xor-ed hash value may end up as zero we replace
it with a fix number, knowing that the code anyway is capable of
handling hash collisions.

Acked-by: Ying Xue <ying.xue@windriver.com>
Signed-off-by: Jon Maloy <jmaloy@redhat.com>
---
 net/tipc/addr.c |  7 +++----
 net/tipc/addr.h |  1 +
 net/tipc/core.h | 12 ++++++++++++
 3 files changed, 16 insertions(+), 4 deletions(-)

Comments

Jakub Kicinski Nov. 25, 2020, 12:22 a.m. UTC | #1 
On Tue, 24 Nov 2020 12:28:33 -0500 jmaloy@redhat.com wrote:
> +static inline u32 hash128to32(char *bytes)
> +{
> +	u32 res, *tmp = (u32 *)bytes;
> +
> +	res = ntohl(tmp[0] ^ tmp[1] ^ tmp[2] ^ tmp[3]);
> +	if (likely(res))
> +		return res;
> +	res = tmp[0] | tmp[1] | tmp[2] | tmp[3];
> +	return !res ? 0 : ntohl(18140715);
> +}

This needs to use correct types otherwise sparse gets upset:

net/tipc/addr.c: note: in included file (through ../net/tipc/addr.h):
net/tipc/core.h:218:15: warning: cast to restricted __be32
net/tipc/core.h:218:15: warning: cast to restricted __be32
net/tipc/core.h:218:15: warning: cast to restricted __be32
net/tipc/core.h:218:15: warning: cast to restricted __be32
net/tipc/core.h:218:15: warning: cast to restricted __be32
net/tipc/core.h:218:15: warning: cast to restricted __be32
net/tipc/core.h:222:27: warning: cast to restricted __be32
net/tipc/core.h:222:27: warning: cast to restricted __be32
net/tipc/core.h:222:27: warning: cast to restricted __be32
net/tipc/core.h:222:27: warning: cast to restricted __be32
net/tipc/core.h:222:27: warning: cast to restricted __be32
net/tipc/core.h:222:27: warning: cast to restricted __be32
kernel test robot Nov. 25, 2020, 3:40 a.m. UTC | #2 
Hi,

I love your patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/jmaloy-redhat-com/tipc-some-minor-improvements/20201125-013225
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git d5a05e69ac6e4c431c380ced2b534c91f7bc3280
config: i386-randconfig-s002-20201125 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.3-151-g540c2c4b-dirty
        # https://github.com/0day-ci/linux/commit/417ce7b0ea8ed268ee0302eb02ee6eaa55f22f9b
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review jmaloy-redhat-com/tipc-some-minor-improvements/20201125-013225
        git checkout 417ce7b0ea8ed268ee0302eb02ee6eaa55f22f9b
        # save the attached .config to linux build tree
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=i386 

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/tipc/addr.c: note: in included file (through net/tipc/addr.h):
>> net/tipc/core.h:218:15: sparse: sparse: cast to restricted __be32
>> net/tipc/core.h:218:15: sparse: sparse: cast to restricted __be32
>> net/tipc/core.h:218:15: sparse: sparse: cast to restricted __be32
>> net/tipc/core.h:218:15: sparse: sparse: cast to restricted __be32
>> net/tipc/core.h:218:15: sparse: sparse: cast to restricted __be32
>> net/tipc/core.h:218:15: sparse: sparse: cast to restricted __be32
   net/tipc/core.h:222:27: sparse: sparse: cast to restricted __be32
   net/tipc/core.h:222:27: sparse: sparse: cast to restricted __be32
   net/tipc/core.h:222:27: sparse: sparse: cast to restricted __be32
   net/tipc/core.h:222:27: sparse: sparse: cast to restricted __be32
   net/tipc/core.h:222:27: sparse: sparse: cast to restricted __be32
   net/tipc/core.h:222:27: sparse: sparse: cast to restricted __be32

vim +218 net/tipc/core.h

   213	
   214	static inline u32 hash128to32(char *bytes)
   215	{
   216		u32 res, *tmp = (u32 *)bytes;
   217	
 > 218		res = ntohl(tmp[0] ^ tmp[1] ^ tmp[2] ^ tmp[3]);
   219		if (likely(res))
   220			return res;
   221		res = tmp[0] | tmp[1] | tmp[2] | tmp[3];
   222		return !res ? 0 : ntohl(18140715);
   223	}
   224	

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

Patch

diff --git a/net/tipc/addr.c b/net/tipc/addr.c
index 0f1eaed1bd1b..abe29d1aa23a 100644
--- a/net/tipc/addr.c
+++ b/net/tipc/addr.c
@@ -55,12 +55,11 @@  bool tipc_in_scope(bool legacy_format, u32 domain, u32 addr)
 void tipc_set_node_id(struct net *net, u8 *id)
 {
 	struct tipc_net *tn = tipc_net(net);
-	u32 *tmp = (u32 *)id;
 
 	memcpy(tn->node_id, id, NODE_ID_LEN);
 	tipc_nodeid2string(tn->node_id_string, id);
-	tn->trial_addr = tmp[0] ^ tmp[1] ^ tmp[2] ^ tmp[3];
-	pr_info("Own node identity %s, cluster identity %u\n",
+	tn->trial_addr = hash128to32(id);
+	pr_info("Node identity %s, cluster identity %u\n",
 		tipc_own_id_string(net), tn->net_id);
 }
 
@@ -76,7 +75,7 @@  void tipc_set_node_addr(struct net *net, u32 addr)
 	}
 	tn->trial_addr = addr;
 	tn->addr_trial_end = jiffies;
-	pr_info("32-bit node address hash set to %x\n", addr);
+	pr_info("Node number set to %u\n", addr);
 }
 
 char *tipc_nodeid2string(char *str, u8 *id)
diff --git a/net/tipc/addr.h b/net/tipc/addr.h
index 31bee0ea7b3e..1a11831bef62 100644
--- a/net/tipc/addr.h
+++ b/net/tipc/addr.h
@@ -3,6 +3,7 @@ 
  *
  * Copyright (c) 2000-2006, 2018, Ericsson AB
  * Copyright (c) 2004-2005, Wind River Systems
+ * Copyright (c) 2020, Red Hat Inc
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
diff --git a/net/tipc/core.h b/net/tipc/core.h
index df34dcdd0607..e6657cf97000 100644
--- a/net/tipc/core.h
+++ b/net/tipc/core.h
@@ -3,6 +3,7 @@ 
  *
  * Copyright (c) 2005-2006, 2013-2018 Ericsson AB
  * Copyright (c) 2005-2007, 2010-2013, Wind River Systems
+ * Copyright (c) 2020, Red Hat Inc
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -210,6 +211,17 @@  static inline u32 tipc_net_hash_mixes(struct net *net, int tn_rand)
 	return net_hash_mix(&init_net) ^ net_hash_mix(net) ^ tn_rand;
 }
 
+static inline u32 hash128to32(char *bytes)
+{
+	u32 res, *tmp = (u32 *)bytes;
+
+	res = ntohl(tmp[0] ^ tmp[1] ^ tmp[2] ^ tmp[3]);
+	if (likely(res))
+		return res;
+	res = tmp[0] | tmp[1] | tmp[2] | tmp[3];
+	return !res ? 0 : ntohl(18140715);
+}
+
 #ifdef CONFIG_SYSCTL
 int tipc_register_sysctl(void);
 void tipc_unregister_sysctl(void);