| Message ID | e6915f30aeebeb064f076ea798940a565ac09ebc.1613557616.git.grive@u256.net |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | conntrack: improve multithread scalability | expand |
On Wed, Feb 17, 2021 at 8:34 AM Gaetan Rivet <grive@u256.net> wrote: > > The 'hash_basis' field is used sometimes during sub-systems init > routine. It will be 0 by default before randomization. Sub-systems would > then init some nodes with incorrect hash values. > > The timeout policies module is affected, making the default policy being > referenced using an incorrect hash value. > > Fixes: 2078901a4c14 ("userspace: Add conntrack timeout policy support.") > Signed-off-by: Gaetan Rivet <grive@u256.net> > Reviewed-by: Eli Britstein <elibr@nvidia.com> > --- LGTM, thanks for the fix. Acked-by: William Tu <u9012063@gmail.com> > lib/conntrack.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/lib/conntrack.c b/lib/conntrack.c > index c218200dc..8062dcd6b 100644 > --- a/lib/conntrack.c > +++ b/lib/conntrack.c > @@ -291,6 +291,11 @@ conntrack_init(void) > static struct ovsthread_once setup_l4_once = OVSTHREAD_ONCE_INITIALIZER; > struct conntrack *ct = xzalloc(sizeof *ct); > > + /* This value can be used during init (e.g. timeout_policy_init()), > + * set it first to ensure it is available. > + */ > + ct->hash_basis = random_uint32(); > + > ovs_rwlock_init(&ct->resources_lock); > ovs_rwlock_wrlock(&ct->resources_lock); > hmap_init(&ct->alg_expectations); > @@ -308,7 +313,6 @@ conntrack_init(void) > timeout_policy_init(ct); > ovs_mutex_unlock(&ct->ct_lock); > > - ct->hash_basis = random_uint32(); > atomic_count_init(&ct->n_conn, 0); > atomic_init(&ct->n_conn_limit, DEFAULT_N_CONN_LIMIT); > atomic_init(&ct->tcp_seq_chk, true); > -- > 2.30.0 >
diff --git a/lib/conntrack.c b/lib/conntrack.c index c218200dc..8062dcd6b 100644 --- a/lib/conntrack.c +++ b/lib/conntrack.c @@ -291,6 +291,11 @@ conntrack_init(void) static struct ovsthread_once setup_l4_once = OVSTHREAD_ONCE_INITIALIZER; struct conntrack *ct = xzalloc(sizeof *ct); + /* This value can be used during init (e.g. timeout_policy_init()), + * set it first to ensure it is available. + */ + ct->hash_basis = random_uint32(); + ovs_rwlock_init(&ct->resources_lock); ovs_rwlock_wrlock(&ct->resources_lock); hmap_init(&ct->alg_expectations); @@ -308,7 +313,6 @@ conntrack_init(void) timeout_policy_init(ct); ovs_mutex_unlock(&ct->ct_lock); - ct->hash_basis = random_uint32(); atomic_count_init(&ct->n_conn, 0); atomic_init(&ct->n_conn_limit, DEFAULT_N_CONN_LIMIT); atomic_init(&ct->tcp_seq_chk, true);
The 'hash_basis' field is used sometimes during sub-systems init routine. It will be 0 by default before randomization. Sub-systems would then init some nodes with incorrect hash values. The timeout policies module is affected, making the default policy being referenced using an incorrect hash value. Fixes: 2078901a4c14 ("userspace: Add conntrack timeout policy support.") Signed-off-by: Gaetan Rivet <grive@u256.net> Reviewed-by: Eli Britstein <elibr@nvidia.com> --- lib/conntrack.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)