Pablo Neira Ayuso [off-list ref] writes:

Hi!

On Tue, Jun 13, 2017 at 09:35:20AM -0700, Cong Wang wrote:

quoted

On Mon, Jun 12, 2017 at 11:16 PM, Florian Westphal [off-list ref] wrote:

quoted

Cong Wang [off-list ref] wrote:

quoted

On Thu, Jun 1, 2017 at 1:52 AM, Florian Westphal [off-list ref] wrote:

quoted

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.

quoted

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?

I am not familiar with netfilter code base so not sure if that is
hard to do or not, but it looks more elegant than this barrier.

Florian has added a new native interface to integrate helpers into
nftables in a much better way than we do now, that allows much more
fine grain configuration. This new interface bumps refcounts on
helpers as you suggest.

However, we still have to sort of keep the existing behaviour around,
people has been relying on this rmmod feature to globally disable
helpers. It's very old thing indeed and as you can see, very sparse
grain for the netns era... But still I think we need this.

So I'm inclined to take this, and keep an eye to deprecate this
behaviour in a several years ahead once. Probably we can get rid of
this barrier at some point.

Acked-by: "Eric W. Biederman" <redacted>

If it works I don't have any problems with the code and it sounds like
it works.

My apologies for the delay.  There is an email black hole between Forian
and myself and I missed his replies.  Which gave me a very distored
picture of the conversation.

Eric