[ovs-dev] [patch v7] conntrack: Add rcu support.

Darrell Ball dlu998 at gmail.com
Wed May 8 19:07:14 UTC 2019


On Wed, May 8, 2019 at 10:56 AM Ben Pfaff <blp at ovn.org> wrote:

> On Wed, May 08, 2019 at 12:14:56AM -0700, Darrell Ball wrote:
> > On Tue, May 7, 2019 at 2:56 PM Ben Pfaff <blp at ovn.org> wrote:
> > > I am not confident about destruction ordering here.  It appears that
> > > conntrack_destroy() frees 'ct'.  I don't see anything that assures that
> > > a grace period has expired before this.  In the meantime, it seems that
> > > packet processing could access 'ct'.  I think that the same is true
> for,
> > > e.g., ct_lock.  I guess one way it could be safe if is
> > > conntrack_destroy() requires the caller to have already quiesced packet
> > > processing for a grace period, but that doesn't seem to be documented
> > > and I don't think dpif-netdev does that.
> > >
> >
> > 'dp_netdev_free' destroys all the ports and then pmds before calling
> > conntrack_destroy().
>
> OK, I see that now, thank you.
>
> So I guess conntrack_destroy() does rely on the caller having quiesced
> packet processing before destroying the conntrack.  That is fine, but I
> think that it should be documented in a comment on conntrack_destroy(),
> because naively in the presence of RCU one might expect that stray
> packets might still squeak through.
>

Done


> > I think memory barriers are already implied; meaning, I do NOT think
> > something like the following is needed, although it does no harm.
>
> I agree.
>
> > ovsrcu_synchronize() is also not needed here.
>
> Is there any chance that already-completed packet processing might have
> done ovsrcu_postpone() to RCU-free data structures, and that those might
> be deferred until after conntrack_destroy() begins, and that they might
> access freed or otherwise corrupt memory due to the conntrack
> destruction?  If that is not possible--I have not checked--then I agree.
>

No, deferred frees access 'conn' heap allocated memory with direct
references


> Thanks,
>
> Ben.
>


More information about the dev mailing list