On Mon, Feb 17, 2014 at 11:45:11AM +0100, Florian Westphal wrote: > Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote: > > > static int > > > +nfnetlink_attach_null_binding(struct nf_conn *ct, > > > + enum nf_nat_manip_type manip) > > > +{ > > > + int ret = -EAGAIN; > > > + bool can_alloc; > > > + > > > + /* This looks bogus, but its important. > > > + * > > > + * We cannot be sure that L3 NAT is available. > > > + * > > > + * If it is not, we will oops in nf_nat_setup_info when we try > > > + * to fetch the l4 nat protocol (which would be on top of the l3 one). > > > + * > > > + * Normally nf_nat_setup_info cannot be called without L3 nat > > > + * available, but this function is invoked from ctnetlink. > > > + */ > > > + rcu_read_lock(); > > > + > > > + can_alloc = !!__nf_nat_l3proto_find(nf_ct_l3num(ct)); > > > + if (can_alloc) > > > + ret = __nf_nat_alloc_null_binding(ct, manip); > > > + > > > + rcu_read_unlock(); > > > + return ret; > > > > I think we should always do the module autoloading for nf-nat and > > nf-nat-ipvX modules depending on nf_ct_l3num(ct), then return EAGAIN > > to give another retry. Now, this needs to happen in any case, not only > > when calling ctnetlink_parse_nat_setup(). > > Not sure what you mean. If we enter nf_nat_setup_info without ctnetlink > involvement the nf-nat protocol should already be there (else, how can > we end up in nf_nat_setup_info? NAT/MASQUERADE depends on nf-nat). > > What use-case did you have in mind? (or, to put it differently, where > do you think the module probing logic should be)? If __nf_nat_l3proto_find returns NULL before trying to attach the null binding, I think you should call the routine to autoload the modules before returning EAGAIN. proto = __nf_nat_l3proto_find(nf_ct_l3num(ct)); if (proto == NULL) { ... release locks request_module(...); ... acquire locks again return -EAGAIN; } -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html