Re: [PATCH lnfct 2/2] conntrack: revert getobjopt_is_nat condition

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Tue, Feb 28, 2017 at 08:44:53PM +0900, Ken-ichirou MATSUZAWA wrote:
>  Hi, Pablo
> 
> On Tue, Feb 28, 2017 at 11:47:25AM +0100, Pablo Neira Ayuso wrote:
> > > diff --git a/src/conntrack/objopt.c b/src/conntrack/objopt.c
> > > index fb43d6c..1581480 100644
> > > --- a/src/conntrack/objopt.c
> > > +++ b/src/conntrack/objopt.c
> > > @@ -144,10 +144,8 @@ int __setobjopt(struct nf_conntrack *ct, unsigned int option)
> > >  
> > >  static int getobjopt_is_snat(const struct nf_conntrack *ct)
> > >  {
> > > -	if (!(test_bit(ATTR_STATUS, ct->head.set)))
> > > -		return 0;
> > > -
> > > -	if (!(ct->status & IPS_SRC_NAT_DONE))
> > > +	if (test_bit(ATTR_STATUS, ct->head.set) &&
> > > +	    !(ct->status & IPS_SRC_NAT_DONE))
> > 
> > However, if ATTR_STATUS is not set, we keep checking ahead. What are
> > you trying to fix?
> 
> It was:
> 
> -       return ((test_bit(ATTR_STATUS, ct->head.set) ?
> -               ct->status & IPS_SRC_NAT_DONE : 1) &&
> -               ct->repl.dst.v4 !=
> -               ct->head.orig.src.v4);
> 
> I thought it keeps checking even ATTR_STATUS is not set.
> But it's ok not to apply, returning false in case of
> ATTR_STATUS is not set.

Ah, I see.

static int getobjopt_is_snat(const struct nf_conntrack *ct)
{
        if (!(test_bit(ATTR_STATUS, ct->head.set)))
                return 0;

        if (!(ct->status & IPS_SRC_NAT_DONE))
                return 0;

        switch (ct->head.orig.l3protonum) {
        case AF_INET:
                return ct->repl.dst.v4 != ct->head.orig.src.v4;
        case AF_INET6:
                if (memcmp(&ct->repl.dst.v6, &ct->head.orig.src.v6,
                           sizeof(struct in6_addr)) != 0)
                        return 1;
                else
                        return 0;
        default:
                return 0;
        }
}

So you want to check if the addresses mismatch, so we infer from there
if there is NAT or not when status bits are not available.

Are you trying to catch up some case in netlink event specifically?

Thanks for explaining.