Re: [PATCH] Handle routing changes for the MASQUERADE target

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Florian,

On Fri, 30 Nov 2012, Florian Westphal wrote:

> Jozsef Kadlecsik <kadlec@xxxxxxxxxxxxxxxxx> wrote:
> 
> > index bd8eea7..fc8f1b8 100644
> > --- a/include/net/netfilter/nf_nat.h
> > +++ b/include/net/netfilter/nf_nat.h
> > @@ -68,4 +68,22 @@ static inline struct nf_conn_nat *nfct_nat(const struct nf_conn *ct)
> >  
> > +static inline bool nf_nat_oif_changed(const struct sk_buff *skb,
> > +				      unsigned int hooknum,
> [..]
> > +{
> 
> If you put the #if IS_ENABLED here, it would no longer be necessary
> in the .c files.  nf_nat_oif_changed() would always return false
> in the 'no MASQERADE' case.

Yes, you're right - and a few lines are spared again :-).
 
> > +	if (nat->masq_index && hooknum == NF_INET_POST_ROUTING &&
> > +	    CTINFO2DIR(ctinfo) == IP_CT_DIR_ORIGINAL &&
> > +	    nat->masq_index != out->ifindex) {
> > +		/* Outgoing interface changed, destroy conntrack. */
> > +		nf_ct_kill_acct(cf, ctinfo, skb);
> > +		nf_ct_put(ct);
> 
> Hmm. Is the nf_ct_put() correct?
> nf_ct_kill invokes death_by_timeout(), which also puts ct.

nf_nat_ipv[46]_fn starts with "nf_ct_get", so that must be released. 
Technically the last nf_ct_put destroys the entry.
 
> > diff --git a/net/ipv4/netfilter/iptable_nat.c b/net/ipv4/netfilter/iptable_nat.c
> > index ac635a7..4ed34ab 100644
> > --- a/net/ipv4/netfilter/iptable_nat.c
> > +++ b/net/ipv4/netfilter/iptable_nat.c
> > @@ -134,6 +134,10 @@ nf_nat_ipv4_fn(unsigned int hooknum,
> >  		/* ESTABLISHED */
> >  		NF_CT_ASSERT(ctinfo == IP_CT_ESTABLISHED ||
> >  			     ctinfo == IP_CT_ESTABLISHED_REPLY);
> > +#if IS_ENABLED(CONFIG_IP_NF_TARGET_MASQUERADE)
> > +		if (nf_nat_oif_changed(skb, hooknum, ct, ctinfo, nat, out))
> > +			return NF_DROP;
> > +#endif
> >  	}
> 
> I wonder if this:
> 
> 	if (nf_nat_oif_changed(skb, hooknum, ctinfo, nat, out)) {
> 		nf_ct_kill_acct(ct, ctinfo, skb);
> 		return NF_DROP;
> 	}
> 
> Would be clearer (the name 'nf_nat_oif_changed' sounds like it only
> performs a test).

OK, I'll send an updated version.

Best regards,
Jozsef
-
E-mail  : kadlec@xxxxxxxxxxxxxxxxx, kadlecsik.jozsef@xxxxxxxxxxxxx
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
          H-1525 Budapest 114, POB. 49, Hungary
--
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


[Index of Archives]     [Netfitler Users]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux