Re: [PATCH nf] netfilter: nf_tables: fix inconsistent element expiration calculation

["Anders K. Pedersen | Cohaesio" <akp@cohaesio.com>]

Hi Liping,

On man, 2016-11-21 at 09:48 +0800, Liping Zhang wrote:
> 2016-11-21 0:38 GMT+08:00 Anders K. Pedersen | Cohaesio <akp@cohaesio
> .com>:

> Acked-by: Liping Zhang <zlpnobody@gmail.com>
> 
> But there's some small indent issues, see below.

> diff --git a/net/netfilter/nf_tables_api.c
> > b/net/netfilter/nf_tables_api.c
> > index 026581b..e5194f6f 100644
> > --- a/net/netfilter/nf_tables_api.c
> > +++ b/net/netfilter/nf_tables_api.c
> > @@ -2570,7 +2570,8 @@ static int nf_tables_fill_set(struct sk_buff *skb, const struct nft_ctx *ctx,
> >         }
> > 
> >         if (set->timeout &&
> > -           nla_put_be64(skb, NFTA_SET_TIMEOUT, cpu_to_be64(set->timeout),
> > +           nla_put_be64(skb, NFTA_SET_TIMEOUT,
> > +                        cpu_to_be64(jiffies_to_msecs(set->timeout)),
> >                          NFTA_SET_PAD))
> >                 goto nla_put_failure;
> >         if (set->gc_int &&
> > @@ -2859,7 +2860,8 @@ static int nf_tables_newset(struct net *net, struct sock *nlsk,
> >         if (nla[NFTA_SET_TIMEOUT] != NULL) {
> >                 if (!(flags & NFT_SET_TIMEOUT))
> >                         return -EINVAL;
> > -               timeout = be64_to_cpu(nla_get_be64(nla[NFTA_SET_TIMEOUT]));
> > +               timeout = msecs_to_jiffies(be64_to_cpu(nla_get_be64(
> > +                                               nla[NFTA_SET_TIMEOUT])));
> 
> nla[NFTA_SET_TIMEOUT] should be kept indent consistent with
> be64_to_cpu.
> You can add some spaces after tab.

The indentation is deliberate, because I don't want to give the
impression that nla[] is an argument to the be64_to_cpu() function.

I looked through the file to see how similar indentation issues was
handled elsewhere. There seems to be a preference for just exceeding
the 80 character per line limit (I found lines up to 107 characters),
but I did find one place (case NFT_GOTO in nf_tables_check_loops())
that use the same style as I do here.

So I could switch to just using long lines, if that is preferred, and
just ignore the checkpatch.pl warning about it. The longest new line
would be 99 characters, which is less than what already exists in the
file.

Regards,
Anders