On 13 June 2017 at 11:07, Florian Westphal <f...@strlen.de> wrote: > Cong Wang <xiyou.wangc...@gmail.com> wrote: >> On Mon, Jun 12, 2017 at 11:16 PM, Florian Westphal <f...@strlen.de> wrote: >> > Cong Wang <xiyou.wangc...@gmail.com> wrote: >> >> On Thu, Jun 1, 2017 at 1:52 AM, Florian Westphal <f...@strlen.de> wrote: >> >> > Joe described it nicely, problem is that after unload we may have >> >> > conntracks that still have a nf_conn_help extension attached that >> >> > has a pointer to a structure that resided in the (unloaded) module. >> >> >> >> Why not hold a refcnt for its module? >> > >> > That would work as well. >> > >> > I'm not sure its nice to disallow rmmod of helper modules if they are >> > used by a connection however. >> >> I am _not_ suggesting to disallow rmmod. > > My point was that if you hold reference counts to the module users > will need to manually flush conntrack table (or at least manually > remove affected connections), else rmmod won't work as refcount might be > gt 0. > >> > Right now you can "rmmod nf_conntrack_foo" at any time and this should >> > work just fine without first having to flush affected conntracks >> > manually. >> >> My point is that since netns wq could invoke code of that module, >> why it doesn't hold a refcnt of that module? > > *shrug*, I did not write this stuff. > > Historically it wasn't needed because we just clear out the helper area > in the affected conntracks (i.e, future packets are not inspected by > the helper anymore). > > When conntracks were made per-netns this problem was added as we're not > guaranteed to see all net namespace because module_exit and netns cleanup > can run concurrently. > > We can still use the "old" model if we guarantee that we wait for > netns cleanup to finish (which is what this patch does). > > The alternative, as you pointed out, is to take a module reference for > each conntrack that uses the helper (and put again when connection is > destroyed). > > I don't really care that much except that if we go for the latter > solution users cannot "just rmmod" the module anymore but might have > to manually remove the affected connections first.
The barrier approach sounds less surprising from user perspective for this very reason.
