On Tue, 20 Nov 2018 at 06:19, Florian Westphal <fw@xxxxxxxxx> wrote:
>
Hi Florian!
Thank you for the review!
> Taehee Yoo <ap420073@xxxxxxxxx> wrote:
> > register_{netdevice/inetaddr/inet6addr}_notifier returns value that
> > could be error value. so that error handling code are needed.
>
> Nothing should break without those notifiers in place though.
>
> > /* check if the notifier was already set */
> > if (atomic_inc_return(&masquerade_notifier_refcount) > 1)
> > - return;
> > + return -EEXIST;
>
> I don't think this is an error, it should return 0.
>
> > diff --git a/net/ipv4/netfilter/nft_masq_ipv4.c b/net/ipv4/netfilter/nft_masq_ipv4.c
> > index f1193e1e928a..6847de1d1db8 100644
> > --- a/net/ipv4/netfilter/nft_masq_ipv4.c
> > +++ b/net/ipv4/netfilter/nft_masq_ipv4.c
> > @@ -69,7 +69,9 @@ static int __init nft_masq_ipv4_module_init(void)
> > if (ret < 0)
> > return ret;
> >
> > - nf_nat_masquerade_ipv4_register_notifier();
> > + ret = nf_nat_masquerade_ipv4_register_notifier();
> > + if (ret)
> > + nft_unregister_expr(&nft_masq_ipv4_type);
>
> Else this would error out in case xtables masquerade module is already
> loaded.
I have tested just now about this.
test commands:
modprobe ipt_MASQUERADE
modprobe nft_masq_ipv4 <-- FAIL
Second command fails because
nf_nat_masquerade_ipv4_register_notifier() returns -EEXIST.
Thanks a lot for finding this. I will send v2 patch!
