Netfilter-Devel Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH net v2] netfilter: Use consistent ct id hash calculation
@ 2019-08-08 18:30 Dirk Morris
  2019-08-08 20:28 ` Florian Westphal
  0 siblings, 1 reply; 3+ messages in thread
From: Dirk Morris @ 2019-08-08 18:30 UTC (permalink / raw)
  To: netfilter-devel

Change ct id hash calculation to only use invariants.

Currently the ct id hash calculation is based on some fields that can
change in the lifetime on a conntrack entry in some corner cases. This
results on the ct id change after the conntrack has been confirmed.
This changes the hash to be based on attributes which should never
change. Now the ct id hash is also consistent from initialization to
conntrack confirmation either even though it is unconfirmed.

Signed-off-by: Dirk Morris <dmorris@metaloft.com>
---
  nf_conntrack_core.c |    7 ++++---
  1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index ab73c5f..dad6868 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -318,9 +318,10 @@ u32 nf_ct_get_id(const struct nf_conn *ct)
  	net_get_random_once(&ct_id_seed, sizeof(ct_id_seed));
  
  	a = (unsigned long)ct;
-	b = (unsigned long)ct->master ^ net_hash_mix(nf_ct_net(ct));
-	c = (unsigned long)ct->ext;
-	d = (unsigned long)siphash(&ct->tuplehash, sizeof(ct->tuplehash),
+	b = (unsigned long)ct->master;
+	c = (unsigned long)nf_ct_net(ct);
+	d = (unsigned long)siphash(&ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple,
+				   sizeof(ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple),
  				   &ct_id_seed);
  #ifdef CONFIG_64BIT
  	return siphash_4u64((u64)a, (u64)b, (u64)c, (u64)d, &ct_id_seed);

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net v2] netfilter: Use consistent ct id hash calculation
  2019-08-08 18:30 [PATCH net v2] netfilter: Use consistent ct id hash calculation Dirk Morris
@ 2019-08-08 20:28 ` Florian Westphal
  2019-08-08 20:39   ` Dirk Morris
  0 siblings, 1 reply; 3+ messages in thread
From: Florian Westphal @ 2019-08-08 20:28 UTC (permalink / raw)
  To: Dirk Morris; +Cc: netfilter-devel

Dirk Morris <dmorris@metaloft.com> wrote:
> Change ct id hash calculation to only use invariants.
> 
> Currently the ct id hash calculation is based on some fields that can
> change in the lifetime on a conntrack entry in some corner cases. This
> results on the ct id change after the conntrack has been confirmed.
> This changes the hash to be based on attributes which should never
> change. Now the ct id hash is also consistent from initialization to
> conntrack confirmation either even though it is unconfirmed.

Looks good, but can you also fix up the comment at the top of this
function?  (Alternatively, delete those things that are not relevant
anymore).

Also, please add following Tag:

Fixes: 3c79107631db1f7 ("netfilter: ctnetlink: don't use conntrack/expect object addresses as id")

perhaps also mention that hashing the full tuplehash includes
the hlist pointer address, which will change when a conntrack is placed
on the dying list -- this affects reliable delete event delivery --
on redelivery, the id will be different, so your commit is not
just an improvement, it also fixes a bug.

Thanks!

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net v2] netfilter: Use consistent ct id hash calculation
  2019-08-08 20:28 ` Florian Westphal
@ 2019-08-08 20:39   ` Dirk Morris
  0 siblings, 0 replies; 3+ messages in thread
From: Dirk Morris @ 2019-08-08 20:39 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netfilter-devel



On 8/8/19 1:28 PM, Florian Westphal wrote:


> Looks good, but can you also fix up the comment at the top of this
> function?  (Alternatively, delete those things that are not relevant
> anymore).

doh! Sorry should have thought of that.
Yes will also make the other changes and submit v3.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-08 18:30 [PATCH net v2] netfilter: Use consistent ct id hash calculation Dirk Morris
2019-08-08 20:28 ` Florian Westphal
2019-08-08 20:39   ` Dirk Morris

Netfilter-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/netfilter-devel/0 netfilter-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 netfilter-devel netfilter-devel/ https://lore.kernel.org/netfilter-devel \
		netfilter-devel@vger.kernel.org netfilter-devel@archiver.kernel.org
	public-inbox-index netfilter-devel


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.netfilter-devel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox