Am Fri, May 29, 2026 at 12:22:58PM -0400 schrieb Aaron Conole via dev:
> Felix Huettner via dev <[email protected]> writes:
> 
> > Previously conntrack entries where just leaked when conntrack_destroy
> > was called. For whatever reason the sanitizer never caught that.
> 
> I think I've addressed this in 2/4 - this patch is probably not needed
> as it is fixing a mistake there.

Hi Aaron,

thanks for the review.

In v2 i'll include it in the 2nd patch.

> 
> > Co-Authored-by: Florian Werner <[email protected]>
> > Signed-off-by: Florian Werner <[email protected]>
> > Co-Authored-by: Sebastian Riese <[email protected]>
> > Signed-off-by: Sebastian Riese <[email protected]>
> > Signed-off-by: Felix Huettner <[email protected]>
> > ---
> >  lib/conntrack.c | 16 ++++++++++++++++
> >  1 file changed, 16 insertions(+)
> >
> > diff --git a/lib/conntrack.c b/lib/conntrack.c
> > index de386e707..7b7e8f92c 100644
> > --- a/lib/conntrack.c
> > +++ b/lib/conntrack.c
> > @@ -157,6 +157,8 @@ detect_ftp_ctl_type(const struct conn_lookup_ctx *ctx,
> >  
> >  static void
> >  expectation_clean(struct conntrack *ct, const struct conn_key *parent_key);
> > +static void
> > +expectation_flush(struct conntrack *ct) OVS_REQUIRES(ct->resources_lock);
> >  
> >  static struct ct_l4_proto *l4_protos[UINT8_MAX + 1];
> >  
> > @@ -647,6 +649,7 @@ conntrack_destroy(struct conntrack *ct)
> >  
> >      ovs_mutex_lock(&ct->ct_lock);
> >  
> > +    conntrack_flush(ct, NULL);
> 
> For the future, this should eventually call into conn_clean() which is
> declared:
> 
>   conn_clean(struct conntrack *ct, struct conn *conn)
>       OVS_EXCLUDED(conn->lock, ct->ct_lock)
> 
> Seems that this is a violation of the locking decorations by being
> inside the ovs_mutex_lock() call.  But probably this patch should
> be dropped or the defer stuff can be folded into 2/4.

I'll move the call to conntrack_flush to before the ovs_mutex_lock.

Thanks a lot,
Felix

> 
> >      for (unsigned i = 0; i < ARRAY_SIZE(ct->conns); i++) {
> >          cmap_destroy(&ct->conns[i]);
> >      }
> > @@ -657,6 +660,7 @@ conntrack_destroy(struct conntrack *ct)
> >      ovs_mutex_destroy(&ct->ct_lock);
> >  
> >      ovs_mutex_lock(&ct->resources_lock);
> > +    expectation_flush(ct);
> >      cmap_destroy(&ct->alg_expectations);
> >      hindex_destroy(&ct->alg_expectation_refs);
> >      ovs_mutex_unlock(&ct->resources_lock);
> > @@ -3170,6 +3174,18 @@ expectation_ref_create(struct hindex 
> > *alg_expectation_refs,
> >      }
> >  }
> >  
> > +static void
> > +expectation_flush(struct conntrack *ct)
> > +{
> > +    struct alg_exp_node *node;
> > +    HINDEX_FOR_EACH_SAFE (node, node_ref, &ct->alg_expectation_refs) {
> > +        expectation_remove(&ct->alg_expectations, &node->key,
> > +                           ct->hash_basis);
> > +        hindex_remove(&ct->alg_expectation_refs, &node->node_ref);
> > +        ovsrcu_postpone(free, node);
> > +    }
> > +}
> > +
> >  static void
> >  expectation_clean(struct conntrack *ct, const struct conn_key *parent_key)
> >  {
> 
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to