Re: BUG: Kernel panic at masquerade

[Patrick McHardy <kaber@trash.net>]

On 14.01, Pablo Neira Ayuso wrote:
> On Wed, Jan 14, 2015 at 06:49:10PM +0000, Patrick McHardy wrote:
> > On 14.01, Pablo Neira Ayuso wrote:
> > > I think that if we control the NAT hook registration from a module,
> > > then NAT chains will have to become built-in again, since we need to
> > > tie the NAT chain and its rules from the hook to perform the NAT
> > > handling.
> > 
> > No, I think the NAT runtime should be standalone and the NAT tables
> > should simply be there to set up mappings. Its quite easy, they
> > only receive packets in state NEW and we remove the chain invocation
> > from the NAT core.
> 
> OK, so the NAT standalone module performs the NAT for state
> ESTABLISHED packets. The mapping will be still controlled by the
> chain. This will also work for dynamic NAT set via ctnetlink, so users
> will not need to even have NAT chains to run this.

Exactly.

> I think we'll need two hooks though. And we would still have the
> incompatibility that we have with iptable_nat and nf_tables, only the
> first one in place will be considered. We'll also have to
> inconditionally register the input and output NAT hooks.

Yes, it requires an extra hook. Its not really a difference to the
current situation though, for any working setup where you have both
NAT and the ability to set up NAT mappings, you have to callbacks,
even though once of them is within NAT and not the hooks.

Regarding the input/output hooks, as a small optimization we could
only register those pairs of hooks that are actually needed, meaning
once iptable_nat is loaded we register all of them, same for ctnetlink,
for nftables we only register the pairs we have chains for. Once
registered they have to stay registered though, unless we want to
tracking the active mappings, which we most likely don't.

Regarding having both iptable_nat and nftables, I'd propose to not
only check for state NEW but also configured mappings. That way
both can set up mappings, but only if a mapping is not set up
already. That seems to be the best we can do and doesn't seem to
unreasonable.