Pablo Neira Ayuso wrote:
This patch removes the module dependency between ctnetlink and nf_nat by means of an indirect call that is initialized when nf_nat is loaded. Now, nf_conntrack_netlink only requires nf_conntrack and nfnetlink. This patch puts nfnetlink_parse_nat_setup_hook into the nf_conntrack_core to avoid dependencies between ctnetlink, nf_conntrack_ipv4 and nf_conntrack_ipv6. This patch also introduces the function ctnetlink_change_nat that is only invoked from the creation path. Actually, the nat handling cannot be invoked from the update path since this is not allowed. By introducing this function, we remove the useless nat handling in the update path and we avoid deadlock-prone code. This patch also adds the required EAGAIN logic for nfnetlink.
This looks great, I've applied it with one minor change, thanks!
+ctnetlink_parse_nat_setup(struct nf_conn *ct, + enum nf_nat_manip_type manip, + struct nlattr *attr) +{ + typeof(nfnetlink_parse_nat_setup_hook) parse_nat_setup; + + parse_nat_setup = rcu_dereference(nfnetlink_parse_nat_setup_hook); + if (!parse_nat_setup) { +#ifdef CONFIG_KMOD + rcu_read_unlock(); + nfnl_unlock(); + if (request_module("nf-nat-ipv4") < 0) { + nfnl_lock(); + rcu_read_lock(); + return -EOPNOTSUPP; + } + nfnl_lock(); + rcu_read_lock(); + parse_nat_setup = + rcu_dereference(nfnetlink_parse_nat_setup_hook); + if (parse_nat_setup) + return -EAGAIN;
The rcu_dereference here isn't necessary because the pointer isn't actually dereferenced. It doesn't matter much, but removing it also looks slightly nicer :)
diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c index b132124..08e82d6 100644 --- a/net/netfilter/nf_conntrack_netlink.c +++ b/net/netfilter/nf_conntrack_netlink.c @@ -832,9 +832,7 @@ ctnetlink_parse_nat_setup(struct nf_conn *ct, } nfnl_lock(); rcu_read_lock(); - parse_nat_setup = - rcu_dereference(nfnetlink_parse_nat_setup_hook); - if (parse_nat_setup) + if (nfnetlink_parse_nat_setup_hook) return -EAGAIN; #endif return -EOPNOTSUPP;