Re: [PATCH] ulogd2: fix build with musl libc

2018-10-29 Thread Cameron Norman
Yes, we can do that.
On Mon, Oct 29, 2018 at 2:11 PM Pablo Neira Ayuso  wrote:
>
> Hi,
>
> On Sat, Oct 27, 2018 at 01:05:45PM -0700, Cameron Norman wrote:
> > The attached patch fixes building ulogd2 with musl libc. It is being
> > used on Void Linux right now.
> >
> > https://bugzilla.netfilter.org/show_bug.cgi?id=1278
> >
> > Thanks,
> > --
> > Cameron Nemo
>
> > diff --git a/src/ulogd.c b/src/ulogd.c
> > index b8bc57c..eb119f9 100644
> > --- a/src/ulogd.c
> > +++ b/src/ulogd.c
> > @@ -65,6 +65,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
>
> musl has limits.h, couldn't we just do
>
> #include 
>
> instead?
>
> >  #include 
> >  #include 
> >  #ifdef DEBUG
>


Re: [PATCH nf 1/2] netfilter: nf_conncount: use spin_lock_bh instead of spin_lock

2018-10-29 Thread Florian Westphal
Pablo Neira Ayuso  wrote:
> On Thu, Oct 25, 2018 at 11:56:12PM +0900, Taehee Yoo wrote:
> > conn_free() holds lock with spin_lock(). and it is called by both
> > nf_conncount_lookup() and nf_conncount_gc_list().
> > nf_conncount_lookup() is bottom-half context and nf_conncount_gc_list()
> > is process context. so that spin_lock() is not safe.
> > Hence conn_free() should use spin_lock_bh() instead of spin_lock().
> > 
> > test commands:
> >%nft add table ip filter
> >%nft add chain ip filter input { type filter hook input priority 0\; }
> >%nft add rule filter input meter test { ip saddr ct count over 2 } \
> >counter
> > 
> > splat looks like:
> > [  461.996507] 
> > [  461.998999] WARNING: inconsistent lock state
> > [  461.998999] 4.19.0-rc6+ #22 Not tainted
> > [  461.998999] 
> > [  461.998999] inconsistent {IN-SOFTIRQ-W} -> {SOFTIRQ-ON-W} usage.
> > [  461.998999] kworker/0:2/134 [HC0[0]:SC0[0]:HE1:SE1] takes:
> > [  461.998999] a71a559a (&(>list_lock)->rlock){+.?.}, at: 
> > conn_free+0x69/0x2b0 [nf_conncount]
> > [  461.998999] {IN-SOFTIRQ-W} state was registered at:
> > [  461.998999]   _raw_spin_lock+0x30/0x70
> > [  461.998999]   nf_conncount_add+0x28a/0x520 [nf_conncount]
> > [  461.998999]   nft_connlimit_eval+0x401/0x580 [nft_connlimit]
> > [  461.998999]   nft_dynset_eval+0x32b/0x590 [nf_tables]
> > [  461.998999]   nft_do_chain+0x497/0x1430 [nf_tables]
> > [  461.998999]   nft_do_chain_ipv4+0x255/0x330 [nf_tables]
> > [  461.998999]   nf_hook_slow+0xb1/0x160
> > [ ... ]
> > [  461.998999] other info that might help us debug this:
> > [  461.998999]  Possible unsafe locking scenario:
> > [  461.998999]
> > [  461.998999]CPU0
> > [  461.998999]
> > [  461.998999]   lock(&(>list_lock)->rlock);
> > [  461.998999]   
> > [  461.998999] lock(&(>list_lock)->rlock);
> > [  461.998999]
> > [  461.998999]  *** DEADLOCK ***
> > [  461.998999]
> > [ ... ]
> 
> nf_conncount_add() also holds spin_lock while allocate from there is
> GFP_ATOMIC given this is called from packet path.

Good catch, yes, this needs spin_lock_bh variant too.

> tree_nodes_free() is also called from user context without _bh
> disabled.

This one is fine, both call sites hold spin_lock_bh(_conncount_locks[x]).


Re: [PATCH nf 1/2] netfilter: nf_conncount: use spin_lock_bh instead of spin_lock

2018-10-29 Thread Pablo Neira Ayuso
On Thu, Oct 25, 2018 at 11:56:12PM +0900, Taehee Yoo wrote:
> conn_free() holds lock with spin_lock(). and it is called by both
> nf_conncount_lookup() and nf_conncount_gc_list().
> nf_conncount_lookup() is bottom-half context and nf_conncount_gc_list()
> is process context. so that spin_lock() is not safe.
> Hence conn_free() should use spin_lock_bh() instead of spin_lock().
> 
> test commands:
>%nft add table ip filter
>%nft add chain ip filter input { type filter hook input priority 0\; }
>%nft add rule filter input meter test { ip saddr ct count over 2 } \
>  counter
> 
> splat looks like:
> [  461.996507] 
> [  461.998999] WARNING: inconsistent lock state
> [  461.998999] 4.19.0-rc6+ #22 Not tainted
> [  461.998999] 
> [  461.998999] inconsistent {IN-SOFTIRQ-W} -> {SOFTIRQ-ON-W} usage.
> [  461.998999] kworker/0:2/134 [HC0[0]:SC0[0]:HE1:SE1] takes:
> [  461.998999] a71a559a (&(>list_lock)->rlock){+.?.}, at: 
> conn_free+0x69/0x2b0 [nf_conncount]
> [  461.998999] {IN-SOFTIRQ-W} state was registered at:
> [  461.998999]   _raw_spin_lock+0x30/0x70
> [  461.998999]   nf_conncount_add+0x28a/0x520 [nf_conncount]
> [  461.998999]   nft_connlimit_eval+0x401/0x580 [nft_connlimit]
> [  461.998999]   nft_dynset_eval+0x32b/0x590 [nf_tables]
> [  461.998999]   nft_do_chain+0x497/0x1430 [nf_tables]
> [  461.998999]   nft_do_chain_ipv4+0x255/0x330 [nf_tables]
> [  461.998999]   nf_hook_slow+0xb1/0x160
> [ ... ]
> [  461.998999] other info that might help us debug this:
> [  461.998999]  Possible unsafe locking scenario:
> [  461.998999]
> [  461.998999]CPU0
> [  461.998999]
> [  461.998999]   lock(&(>list_lock)->rlock);
> [  461.998999]   
> [  461.998999] lock(&(>list_lock)->rlock);
> [  461.998999]
> [  461.998999]  *** DEADLOCK ***
> [  461.998999]
> [ ... ]

nf_conncount_add() also holds spin_lock while allocate from there is
GFP_ATOMIC given this is called from packet path.

tree_nodes_free() is also called from user context without _bh
disabled.

Is this fix complete?


Re: [PATCH nf] netfilter: xt_IDLETIMER: add sysfs filename checking routine

2018-10-29 Thread Pablo Neira Ayuso
On Sun, Oct 21, 2018 at 12:00:08AM +0900, Taehee Yoo wrote:
> When IDLETIMER rule is added, sysfs file is created under
> /sys/class/xt_idletimer/timers/
> But some label name shouldn't be used.
> ".", "..", "power", "uevent", "subsystem", etc...
> So that sysfs filename checking routine is needed.
> 
> test commands:
>%iptables -I INPUT -j IDLETIMER --timeout 1 --label "power"
> 
> splat looks like:
> [95765.423132] sysfs: cannot create duplicate filename 
> '/devices/virtual/xt_idletimer/timers/power'
> [95765.433418] CPU: 0 PID: 8446 Comm: iptables Not tainted 4.19.0-rc6+ #20
> [95765.449755] Call Trace:
> [95765.449755]  dump_stack+0xc9/0x16b
> [95765.449755]  ? show_regs_print_info+0x5/0x5
> [95765.449755]  sysfs_warn_dup+0x74/0x90
> [95765.449755]  sysfs_add_file_mode_ns+0x352/0x500
> [95765.449755]  sysfs_create_file_ns+0x179/0x270
> [95765.449755]  ? sysfs_add_file_mode_ns+0x500/0x500
> [95765.449755]  ? idletimer_tg_checkentry+0x3e5/0xb1b [xt_IDLETIMER]
> [95765.449755]  ? rcu_read_lock_sched_held+0x114/0x130
> [95765.449755]  ? __kmalloc_track_caller+0x211/0x2b0
> [95765.449755]  ? memcpy+0x34/0x50
> [95765.449755]  idletimer_tg_checkentry+0x4e2/0xb1b [xt_IDLETIMER]
> [ ... ]

Applied, thanks.


Re: [PATCH nf] netfilter: xt_RATEEST: remove netns exit routine

2018-10-29 Thread Pablo Neira Ayuso
On Fri, Oct 19, 2018 at 12:27:57AM +0900, Taehee Yoo wrote:
> xt_rateest_net_exit() was added to check whether rules are flushed
> successfully. but ->net_exit() callback is called earlier than
> ->destroy() callback.
> So that ->net_exit() callback can't check that.
> 
> test commands:
>%ip netns add vm1
>%ip netns exec vm1 iptables -t mangle -I PREROUTING -p udp \
>  --dport  -j RATEEST --rateest-name ap \
>  --rateest-interval 250ms --rateest-ewma 0.5s
>%ip netns del vm1

Hm, I cannot reproduce this here.

I can see iptables-tests.py with -N fails to load entries:

# ip netns exec test xtables-legacy-multi iptables -A INPUT -m rateest 
--rateest RE1 --rateest-lt --rateest-bps 8bit
iptables: No chain/target/match by that name.

but not this warning, probably I'm missing instrumention, something
not enabled here.

> splat looks like:
> [  668.813518] WARNING: CPU: 0 PID: 87 at net/netfilter/xt_RATEEST.c:210 
> xt_rateest_net_exit+0x210/0x340 [xt_RATEEST]
> [  668.813518] CPU: 0 PID: 87 Comm: kworker/u4:2 Not tainted 4.19.0-rc7+ #21
> [  668.813518] Workqueue: netns cleanup_net
> [  668.813518] RIP: 0010:xt_rateest_net_exit+0x210/0x340 [xt_RATEEST]
> [  668.813518] Code: 00 48 8b 85 30 ff ff ff 4c 8b 23 80 38 00 0f 85 24 01 00 
> 00 48 8b 85 30 ff ff ff 4d 85 e4 4c 89 a5 58 ff ff ff c6 00 f8 74 b2 <0f> 0b 
> 48 83 c3 08 4c 39 f3 75 b0 48 b8 00 00 00 00 00 fc ff df 49
> [  668.813518] RSP: 0018:8801156c73f8 EFLAGS: 00010282
> [  668.813518] RAX: ed0022ad8e85 RBX: 880118928e98 RCX: 
> 5db8012a
> [  668.813518] RDX: 8801156c7428 RSI: cb1d185f RDI: 
> 880115663b74
> [  668.813518] RBP: 8801156c74d0 R08: 8801156633c0 R09: 
> 1100236440be
> [  668.813518] R10: 0001 R11: ed002367d852 R12: 
> 880115142b08
> [  668.813518] R13: 110022ad8e81 R14: 880118928ea8 R15: 
> dc00
> [  668.813518] FS:  () GS:88011b20() 
> knlGS:
> [  668.813518] CS:  0010 DS:  ES:  CR0: 80050033
> [  668.813518] CR2: 563aa69f4f28 CR3: 000105a16000 CR4: 
> 001006f0
> [  668.813518] Call Trace:
> [  668.813518]  ? unregister_netdevice_many+0xe0/0xe0
> [  668.813518]  ? xt_rateest_net_init+0x2c0/0x2c0 [xt_RATEEST]
> [  668.813518]  ? default_device_exit+0x1ca/0x270
> [  668.813518]  ? remove_proc_entry+0x1cd/0x390
> [  668.813518]  ? dev_change_net_namespace+0xd00/0xd00
> [  668.813518]  ? __init_waitqueue_head+0x130/0x130
> [  668.813518]  ops_exit_list.isra.10+0x94/0x140
> [  668.813518]  cleanup_net+0x45b/0x900
> [  668.813518]  ? net_drop_ns+0x110/0x110
> [  668.813518]  ? swapgs_restore_regs_and_return_to_usermode+0x3c/0x80
> [  668.813518]  ? save_trace+0x300/0x300
> [  668.813518]  ? lock_acquire+0x196/0x470
> [  668.813518]  ? lock_acquire+0x196/0x470
> [  668.813518]  ? process_one_work+0xb60/0x1de0
> [  668.813518]  ? _raw_spin_unlock_irq+0x29/0x40
> [  668.813518]  ? _raw_spin_unlock_irq+0x29/0x40
> [  668.813518]  ? __lock_acquire+0x4500/0x4500
> [  668.813518]  ? __lock_is_held+0xb4/0x140
> [  668.813518]  process_one_work+0xc13/0x1de0
> [  668.813518]  ? pwq_dec_nr_in_flight+0x3c0/0x3c0
> [  668.813518]  ? set_load_weight+0x270/0x270
> [ ... ]
> 
> Fixes: 3427b2ab63fa ("netfilter: make xt_rateest hash table per net")
> Signed-off-by: Taehee Yoo 
> ---
>  net/netfilter/xt_RATEEST.c | 10 --
>  1 file changed, 10 deletions(-)
> 
> diff --git a/net/netfilter/xt_RATEEST.c b/net/netfilter/xt_RATEEST.c
> index dec843cadf46..9e05c86ba5c4 100644
> --- a/net/netfilter/xt_RATEEST.c
> +++ b/net/netfilter/xt_RATEEST.c
> @@ -201,18 +201,8 @@ static __net_init int xt_rateest_net_init(struct net 
> *net)
>   return 0;
>  }
>  
> -static void __net_exit xt_rateest_net_exit(struct net *net)
> -{
> - struct xt_rateest_net *xn = net_generic(net, xt_rateest_id);
> - int i;
> -
> - for (i = 0; i < ARRAY_SIZE(xn->hash); i++)
> - WARN_ON_ONCE(!hlist_empty(>hash[i]));
> -}
> -
>  static struct pernet_operations xt_rateest_net_ops = {
>   .init = xt_rateest_net_init,
> - .exit = xt_rateest_net_exit,
>   .id   = _rateest_id,
>   .size = sizeof(struct xt_rateest_net),
>  };
> -- 
> 2.17.1
> 


Re: [PATCH nf 2/2] netfilter: nf_conncount: fix list_del corruption in conn_free

2018-10-29 Thread Yi-Hung Wei
On Thu, Oct 25, 2018 at 7:56 AM Taehee Yoo  wrote:
>
> nf_conncount_tuple is an element of nft_connlimit and that is deleted by
> conn_free(). elements can be deleted by both GC routine and
> data path functions(nf_conncount_lookup, nf_conncount_add) and they
> calls conn_free() to free elements.
> But conn_free() only protects lists, not each element.
> So that list_del corruption could occurred.
>
> The conn_free() doesn't check whether element is already deleted.
> In order to protect elements, dead flag is added.
> If an element is deleted, dead flag is set.
> The only conn_free() can delete elements so that both list lock and
> dead flag are enough to protect it.
>
> test commands:
>%nft add table ip filter
>%nft add chain ip filter input { type filter hook input priority 0\; }
>%nft add rule filter input meter test { ip id ct count over 2 } counter
>
> [ ... ]
>
> Fixes: 5c789e131cbb ("netfilter: nf_conncount: Add list lock and gc worker, 
> and RCU for init tree search")
> Signed-off-by: Taehee Yoo 
> ---

LGTM.
Acked-by: Yi-Hung Wei 


Re: [PATCH nf 1/2] netfilter: nf_conncount: use spin_lock_bh instead of spin_lock

2018-10-29 Thread Yi-Hung Wei
On Thu, Oct 25, 2018 at 7:56 AM Taehee Yoo  wrote:
>
> conn_free() holds lock with spin_lock(). and it is called by both
> nf_conncount_lookup() and nf_conncount_gc_list().
> nf_conncount_lookup() is bottom-half context and nf_conncount_gc_list()
> is process context. so that spin_lock() is not safe.
> Hence conn_free() should use spin_lock_bh() instead of spin_lock().
>
> test commands:
>%nft add table ip filter
>%nft add chain ip filter input { type filter hook input priority 0\; }
>%nft add rule filter input meter test { ip saddr ct count over 2 } \
>counter
>
> Fixes: 5c789e131cbb ("netfilter: nf_conncount: Add list lock and gc worker, 
> and RCU for init tree search")
> Signed-off-by: Taehee Yoo 

Thanks for the fix.
Acked-by: Yi-Hung Wei 


Re: [PATCH 5/5] netfilter: ipset: Fix calling ip_set() macro at dumping

2018-10-29 Thread Pablo Neira Ayuso
Hi Jozsef,

On Sat, Oct 27, 2018 at 06:05:43PM +0200, Jozsef Kadlecsik wrote:
> The ip_set() macro is called when either ip_set_ref_lock held only
> or no lock/nfnl mutex is held at dumping. Take this into account
> properly.
> 
> Signed-off-by: Jozsef Kadlecsik 
> ---
>  net/netfilter/ipset/ip_set_core.c | 23 +++
>  1 file changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/net/netfilter/ipset/ip_set_core.c 
> b/net/netfilter/ipset/ip_set_core.c
> index 68db946..292f7c7 100644
> --- a/net/netfilter/ipset/ip_set_core.c
> +++ b/net/netfilter/ipset/ip_set_core.c
> @@ -55,12 +55,27 @@ MODULE_AUTHOR("Jozsef Kadlecsik 
> ");
>  MODULE_DESCRIPTION("core IP set support");
>  MODULE_ALIAS_NFNL_SUBSYS(NFNL_SUBSYS_IPSET);
>  
> -/* When the nfnl mutex is held: */
> +/* When the nfnl mutex or ip_set_ref_lock is held: */
>  #define ip_set_dereference(p)\
> - rcu_dereference_protected(p, lockdep_nfnl_is_held(NFNL_SUBSYS_IPSET))
> + rcu_dereference_protected(p,\
> + lockdep_nfnl_is_held(NFNL_SUBSYS_IPSET) || \
> + lockdep_is_held(_set_ref_lock))
>  #define ip_set(inst, id) \
>   ip_set_dereference((inst)->ip_set_list)[id]
>  
> +/* When the nfnl mutex is not held (dumping): */
> +static inline
> +struct ip_set * ip_set_no_mutex(struct ip_set_net *inst, ip_set_id_t id)
> +{
> + struct ip_set *set;
> +
> + rcu_read_lock();
> + set = rcu_dereference((inst)->ip_set_list)[id];
> + rcu_read_unlock();

If your goal is to calm down bogus warnings, you can use instead:

set = rcu_dereference_raw(...);

instead, in case ip_set_dump_done() is already protected under
rcu_read_lock(). Otherwise, you have to rework your rcu approach in
the dump_done path.

rcu is not useful in the way you use it above, because future
dereference of set are only valid insider the rcu read lock section.

Would you send me a follow up patch to amend this?

Thanks.


Re: [PATCH 0/5] ipset patches for nf

2018-10-29 Thread Pablo Neira Ayuso
On Sat, Oct 27, 2018 at 06:05:38PM +0200, Jozsef Kadlecsik wrote:
> Hi Pablo,
> 
> Please pull the next patches for the nf tree:
> 
> - Decrease refcount synchronously on deletion and replace by
>   Stefano Brivio, which fixes the reference counter shown in
>   userspace.
> - Allow CIDR 0 in hash:net,port,net, which is documented but
>   was unnecessarily disabled, from Eric Westbrook.
> - Fix allocation failure when memory is fragmented, from Andrey Ryabinin.
> - Correct rcu_dereference() call in ip_set_put_comment().
> - Also, correct ip_set() macro calling at dumping with respect of held
>   mutex or lock.

Pulled, thanks Jozsef!


Re: [PATCH] ulogd2: fix build with musl libc

2018-10-29 Thread Pablo Neira Ayuso
Hi,

On Sat, Oct 27, 2018 at 01:05:45PM -0700, Cameron Norman wrote:
> The attached patch fixes building ulogd2 with musl libc. It is being
> used on Void Linux right now.
> 
> https://bugzilla.netfilter.org/show_bug.cgi?id=1278
> 
> Thanks,
> --
> Cameron Nemo

> diff --git a/src/ulogd.c b/src/ulogd.c
> index b8bc57c..eb119f9 100644
> --- a/src/ulogd.c
> +++ b/src/ulogd.c
> @@ -65,6 +65,7 @@
>  #include 
>  #include 
>  #include 
> +#include 

musl has limits.h, couldn't we just do

#include 

instead?

>  #include 
>  #include 
>  #ifdef DEBUG



Re: [PATCH nft] src: get rid of nft_ctx_output_{get,set}_numeric()

2018-10-29 Thread Pablo Neira Ayuso
On Mon, Oct 29, 2018 at 06:50:13PM +0100, Phil Sutter wrote:
> On Mon, Oct 29, 2018 at 06:31:10PM +0100, Pablo Neira Ayuso wrote:
> > @Phil, thinking here we could probably get rid of
> > NFT_CTX_OUTPUT_NUMERIC_PROTO, since it is contained already in
> > NFT_CTX_OUTPUT_NUMERIC_SYMBOL.
> > 
> > There's no option for -p anymore, so we could simply things a bit
> > before.
> 
> I think it's feasible to provide cmdline options for each bit '-n'
> enables. Not only because this gives users control, it also simplifies
> documentation a bit, like:
> 
> | -a - numeric a
> | -b - numeric b
> | -n - enable all numeric options from above
> 
> :

I can just take this v1 patch, instead of v2, and reintroduce -p to
enable numerical protocol number, sending new patchset.

> [...]
> > > diff --git a/include/nftables/libnftables.h 
> > > b/include/nftables/libnftables.h
> > > index fb81edc0df07..82b4181de0cc 100644
> > > --- a/include/nftables/libnftables.h
> > > +++ b/include/nftables/libnftables.h
> [...]
> > > @@ -54,13 +47,12 @@ enum {
> > >   NFT_CTX_OUTPUT_GUID = (1 << 6),
> > >   NFT_CTX_OUTPUT_NUMERIC_PROTO= (1 << 7),
> > >   NFT_CTX_OUTPUT_NUMERIC_PRIO = (1 << 8),
> > > + NFT_CTX_OUTPUT_NUMERIC_SYMBOL   = (1 << 9),
> > >  };
> 
> + #define NFT_CTX_OUTPUT_NUMERIC_ALL (NFT_CTX_OUTPUT_NUMERIC_PROTO |
> +   NFT_CTX_OUTPUT_NUMERIC_PRIO |
> +   NFT_CTX_OUTPUT_NUMERIC_SYMBOL)

Yes, -n is exactly doing this. I'll add this definition.

> [...]
> 
> > > diff --git a/src/main.c b/src/main.c
> > > index 883261fc9d8b..952ce356490f 100644
> > > --- a/src/main.c
> > > +++ b/src/main.c
> [...]
> > > @@ -229,14 +226,10 @@ int main(int argc, char * const *argv)
> > >   }
> > >   break;
> > >   case OPT_NUMERIC:
> > > - numeric = nft_ctx_output_get_numeric(nft);
> > > - if (numeric == NFT_NUMERIC_ALL) {
> > > - fprintf(stderr, "Too many numeric options "
> > > - "used, max. %u\n",
> > > - NFT_NUMERIC_ALL);
> > > - exit(EXIT_FAILURE);
> > > - }
> > > - nft_ctx_output_set_numeric(nft, numeric + 1);
> > > + numeric = true;
> > > + output_flags |= (NFT_CTX_OUTPUT_NUMERIC_PROTO |
> > > +  NFT_CTX_OUTPUT_NUMERIC_PRIO |
> > > +  NFT_CTX_OUTPUT_NUMERIC_SYMBOL);
> 
> + output_flags |= NFT_CTX_OUTPUT_NUMERIC_ALL;
> 
> > > @@ -298,6 +291,13 @@ int main(int argc, char * const *argv)
> > >   }
> > >   }
> > >  
> > > + if (numeric &&
> > > + (output_flags &
> > > + (NFT_CTX_OUTPUT_REVERSEDNS |
> > > +  NFT_CTX_OUTPUT_SERVICE |
> > > +  NFT_CTX_OUTPUT_GUID)))
> > > + fprintf(stderr, "cannot combine `-n' with `-N', `-S', '-u'\n");
> > > +
> 
> Why not? We could allow people to call 'nft -n -N' to make all numeric
> but enable reverse DNS (IIRC).

I didn't think about this combination, sounds reasonable.

> All we need to do is document that '-n' is an alias for '-p -q ...'
> and that later flags overwrite earlier ones (as usual with
> getopt()-based tools). Would simplify code a bit, too.
> 
> > > diff --git a/src/monitor.c b/src/monitor.c
> > > index b2267e1f63e4..0e735ed5b1aa 100644
> > > --- a/src/monitor.c
> > > +++ b/src/monitor.c
> > > @@ -835,11 +835,9 @@ static int netlink_events_newgen_cb(const struct 
> > > nlmsghdr *nlh, int type,
> > >   }
> > >   if (genid >= 0) {
> > >   nft_mon_print(monh, "# new generation %d", genid);
> > > - if (pid >= 0) {
> > > - nft_mon_print(monh, " by process %d", pid);
> > > - if (!monh->ctx->nft->output.numeric)
> > > - nft_mon_print(monh, " (%s)", name);
> > > - }
> > > + if (pid >= 0)
> > > + nft_mon_print(monh, " by process %d (%s)", pid, name);
> > > +
> > >   nft_mon_print(monh, "\n");
> > >   }
> > >  
> 
> Yes, I think that's OK. People may still do 's/ (.*$//' if they don't
> want it there.

OK.


[PATCH 1/3 nft,v3] src: get rid of nft_ctx_output_{get,set}_numeric()

2018-10-29 Thread Pablo Neira Ayuso
This patch adds NFT_CTX_OUTPUT_NUMERIC_SYMBOL, which replaces the last
client of the numeric level approach.

This patch updates `-n' option semantics to display all output
numerically.

Note that monitor code was still using the -n option to skip printing
the process name, this patch updates that path too to print it
inconditionally to simplify things.

Given the numeric levels have no more clients after this patch, remove
that code.

Update several tests/shell not to use -nn.

This patch adds NFT_CTX_OUTPUT_NUMERIC_ALL which enables all flags to
provide a fully numerical output.

Signed-off-by: Pablo Neira Ayuso 
---
v3: Add NFT_CTX_OUTPUT_NUMERIC_ALL.
Allow to combine -N, -S with -n.
Leave NFT_CTX_OUTPUT_NUMERIC_PROTO in place. -Requested by Phil.

 doc/libnftables.adoc| 39 +++--
 include/nftables.h  |  6 +++-
 include/nftables/libnftables.h  | 13 +++--
 src/datatype.c  |  2 +-
 src/json.c  |  2 +-
 src/libnftables.c   | 11 ---
 src/main.c  | 14 ++---
 src/monitor.c   |  8 ++---
 tests/shell/testcases/netns/0001nft-f_0 |  2 +-
 tests/shell/testcases/netns/0002loosecommands_0 |  2 +-
 tests/shell/testcases/netns/0003many_0  |  2 +-
 11 files changed, 23 insertions(+), 78 deletions(-)

diff --git a/doc/libnftables.adoc b/doc/libnftables.adoc
index 788194396db1..007506784600 100644
--- a/doc/libnftables.adoc
+++ b/doc/libnftables.adoc
@@ -21,10 +21,6 @@ void nft_ctx_set_dry_run(struct nft_ctx* '\*ctx'*, bool* 
'dry'*);
 unsigned int nft_ctx_output_get_flags(struct nft_ctx* '\*ctx'*);
 void nft_ctx_output_set_flags(struct nft_ctx* '\*ctx'*, unsigned int* 
'flags'*);
 
-enum nft_numeric_level nft_ctx_output_get_numeric(struct nft_ctx* '\*ctx'*);
-void nft_ctx_output_set_numeric(struct nft_ctx* '\*ctx'*,
-   enum nft_numeric_level* 'level'*);
-
 unsigned int nft_ctx_output_get_debug(struct nft_ctx* '\*ctx'*);
 void nft_ctx_output_set_debug(struct nft_ctx* '\*ctx'*, unsigned int* 'mask'*);
 
@@ -125,37 +121,10 @@ NFT_CTX_OUTPUT_NUMERIC_PROTO::
Display layer 4 protocol numerically.
 NFT_CTX_OUTPUT_NUMERIC_PRIO::
Display base chain priority numerically.
-
-=== nft_ctx_output_get_numeric() and nft_ctx_output_set_numeric()
-These functions allow control over value representation in library output.
-For instance, port numbers by default are printed by their name (as listed in 
'/etc/services' file), if known.
-In libnftables, numeric output is leveled, defined as such:
-
-
-enum nft_numeric_level {
-NFT_NUMERIC_NONE,
-NFT_NUMERIC_ADDR,
-NFT_NUMERIC_PORT,
-NFT_NUMERIC_ALL,
-};
-
-
-Each numeric level includes all previous ones:
-
-NFT_NUMERIC_NONE::
-   No conversion into numeric format happens, this is the default.
-NFT_NUMERIC_ADDR::
-   Network addresses are always converted into numeric format.
-NFT_NUMERIC_PORT::
-   Network services are always converted into numeric format.
-NFT_NUMERIC_ALL::
-   Everything is converted into numeric format.
-
-The default numeric level is *NFT_NUMERIC_NONE*.
-
-The *nft_ctx_output_get_numeric*() function returns the numeric output 
setting's value contained in 'ctx'.
-
-The *nft_ctx_output_set_numeric*() function sets the numeric output setting in 
'ctx' to the value of 'level'.
+NFT_CTX_OUTPUT_NUMERIC_SYMBOL::
+   Display expression datatype as numeric value.
+NFT_CTX_OUTPUT_NUMERIC_ALL::
+   Display all numerically.
 
 === nft_ctx_output_get_debug() and nft_ctx_output_set_debug()
 Libnftables supports separate debugging of different parts of its internals.
diff --git a/include/nftables.h b/include/nftables.h
index a4d01e0cddea..5c0292615b3f 100644
--- a/include/nftables.h
+++ b/include/nftables.h
@@ -17,7 +17,6 @@ struct cookie {
 
 struct output_ctx {
unsigned int flags;
-   unsigned int numeric;
union {
FILE *output_fp;
struct cookie output_cookie;
@@ -73,6 +72,11 @@ static inline bool nft_output_numeric_prio(const struct 
output_ctx *octx)
return octx->flags & NFT_CTX_OUTPUT_NUMERIC_PRIO;
 }
 
+static inline bool nft_output_numeric_symbol(const struct output_ctx *octx)
+{
+   return octx->flags & NFT_CTX_OUTPUT_NUMERIC_SYMBOL;
+}
+
 struct nft_cache {
uint16_tgenid;
struct list_headlist;
diff --git a/include/nftables/libnftables.h b/include/nftables/libnftables.h
index fb81edc0df07..70e9d238843a 100644
--- a/include/nftables/libnftables.h
+++ b/include/nftables/libnftables.h
@@ -26,13 +26,6 @@ enum nft_debug_level {
NFT_DEBUG_SEGTREE   = 0x40,
 };
 
-enum nft_numeric_level {
-   NFT_NUMERIC_NONE,
-   NFT_NUMERIC_ADDR,
-   NFT_NUMERIC_PORT,
- 

[PATCH 3/3 nft,v2] expression: always print range expression numerically

2018-10-29 Thread Pablo Neira Ayuso
Otherwise we end up displaying things that we cannot parse as input.
Moreover, in a range, it's relevant to the user the values that are
enclosed in the range, so let's print this numerically.

Fixes: baa4e0e3fa5f ("src: add NFT_CTX_OUTPUT_NUMERIC_PROTO")
Reported-by: Phil Sutter 
Signed-off-by: Pablo Neira Ayuso 
---
v2: Cast out all literal flags in ranges.
Use NFT_CTX_OUTPUT_NUMERIC_ALL to enable all numerical flags.

 src/expression.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/expression.c b/src/expression.c
index 5ff469c561d5..62a6f890f814 100644
--- a/src/expression.c
+++ b/src/expression.c
@@ -662,8 +662,10 @@ static void range_expr_print(const struct expr *expr, 
struct output_ctx *octx)
 {
unsigned int flags = octx->flags;
 
-   octx->flags &= ~NFT_CTX_OUTPUT_SERVICE;
-   octx->flags |= NFT_CTX_OUTPUT_NUMERIC_PROTO;
+   octx->flags &= ~(NFT_CTX_OUTPUT_SERVICE |
+NFT_CTX_OUTPUT_REVERSEDNS |
+NFT_CTX_OUTPUT_GUID);
+   octx->flags |= NFT_CTX_OUTPUT_NUMERIC_ALL;
expr_print(expr->left, octx);
nft_print(octx, "-");
expr_print(expr->right, octx);
-- 
2.11.0



[PATCH 2/3 nft,v3] src: add -p to print layer 4 protocol numerically

2018-10-29 Thread Pablo Neira Ayuso
We keep printing layer 4 protocols as literals since we do not use
/etc/protocols. Add -p option to print layer 4 protocols numerically.

Signed-off-by: Pablo Neira Ayuso 
---
v3: Expose this -p option, Phil has a usecase for this.

 doc/nft.txt | 4 
 src/main.c  | 7 ++-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/doc/nft.txt b/doc/nft.txt
index 99ac0e333a93..5ee06f16f642 100644
--- a/doc/nft.txt
+++ b/doc/nft.txt
@@ -56,6 +56,10 @@ For a full summary of options, run *nft --help*.
 *--guid**::
Translate numeric UID/GID to names as defined by /etc/passwd and 
/etc/group.
 
+*-p*::
+*--numeric-protocol**::
+   Display layer 4 protocol numerically.
+
 *-y*::
 *--numeric-priority**::
Display base chain priority numerically.
diff --git a/src/main.c b/src/main.c
index 1f01a6c0b528..9a50f30f850b 100644
--- a/src/main.c
+++ b/src/main.c
@@ -41,9 +41,10 @@ enum opt_vals {
OPT_ECHO= 'e',
OPT_GUID= 'u',
OPT_NUMERIC_PRIO= 'y',
+   OPT_NUMERIC_PROTO   = 'p',
OPT_INVALID = '?',
 };
-#define OPTSTRING  "hvcf:iI:jvnsNaeSupy"
+#define OPTSTRING  "hvcf:iI:jvnsNaeSupyp"
 
 static const struct option options[] = {
{
@@ -137,6 +138,7 @@ static void show_help(const char *name)
 "  -u, --guid  Print UID/GID as defined in /etc/passwd and 
/etc/group.\n"
 "  -N  Translate IP addresses to names.\n"
 "  -S, --service   Translate ports to service names as 
described in /etc/services.\n"
+"  -p, --numeric-protocol  Print layer 4 protocols numerically.\n"
 "  -y, --numeric-priority  Print chain priority numerically.\n"
 "  -a, --handleOutput rule handle.\n"
 "  -e, --echo  Echo what has been added, inserted or 
replaced.\n"
@@ -283,6 +285,9 @@ int main(int argc, char * const *argv)
case OPT_NUMERIC_PRIO:
output_flags |= NFT_CTX_OUTPUT_NUMERIC_PRIO;
break;
+   case OPT_NUMERIC_PROTO:
+   output_flags |= NFT_CTX_OUTPUT_NUMERIC_PROTO;
+   break;
case OPT_INVALID:
exit(EXIT_FAILURE);
}
-- 
2.11.0



Re: [PATCH nft] src: get rid of nft_ctx_output_{get,set}_numeric()

2018-10-29 Thread Phil Sutter
On Mon, Oct 29, 2018 at 06:31:10PM +0100, Pablo Neira Ayuso wrote:
> @Phil, thinking here we could probably get rid of
> NFT_CTX_OUTPUT_NUMERIC_PROTO, since it is contained already in
> NFT_CTX_OUTPUT_NUMERIC_SYMBOL.
> 
> There's no option for -p anymore, so we could simply things a bit
> before.

I think it's feasible to provide cmdline options for each bit '-n'
enables. Not only because this gives users control, it also simplifies
documentation a bit, like:

| -a - numeric a
| -b - numeric b
| -n - enable all numeric options from above

:)

[...]
> > diff --git a/include/nftables/libnftables.h b/include/nftables/libnftables.h
> > index fb81edc0df07..82b4181de0cc 100644
> > --- a/include/nftables/libnftables.h
> > +++ b/include/nftables/libnftables.h
[...]
> > @@ -54,13 +47,12 @@ enum {
> > NFT_CTX_OUTPUT_GUID = (1 << 6),
> > NFT_CTX_OUTPUT_NUMERIC_PROTO= (1 << 7),
> > NFT_CTX_OUTPUT_NUMERIC_PRIO = (1 << 8),
> > +   NFT_CTX_OUTPUT_NUMERIC_SYMBOL   = (1 << 9),
> >  };

+ #define NFT_CTX_OUTPUT_NUMERIC_ALL (NFT_CTX_OUTPUT_NUMERIC_PROTO |
+ NFT_CTX_OUTPUT_NUMERIC_PRIO |
+ NFT_CTX_OUTPUT_NUMERIC_SYMBOL)

[...]

> > diff --git a/src/main.c b/src/main.c
> > index 883261fc9d8b..952ce356490f 100644
> > --- a/src/main.c
> > +++ b/src/main.c
[...]
> > @@ -229,14 +226,10 @@ int main(int argc, char * const *argv)
> > }
> > break;
> > case OPT_NUMERIC:
> > -   numeric = nft_ctx_output_get_numeric(nft);
> > -   if (numeric == NFT_NUMERIC_ALL) {
> > -   fprintf(stderr, "Too many numeric options "
> > -   "used, max. %u\n",
> > -   NFT_NUMERIC_ALL);
> > -   exit(EXIT_FAILURE);
> > -   }
> > -   nft_ctx_output_set_numeric(nft, numeric + 1);
> > +   numeric = true;
> > +   output_flags |= (NFT_CTX_OUTPUT_NUMERIC_PROTO |
> > +NFT_CTX_OUTPUT_NUMERIC_PRIO |
> > +NFT_CTX_OUTPUT_NUMERIC_SYMBOL);

+ output_flags |= NFT_CTX_OUTPUT_NUMERIC_ALL;

> > @@ -298,6 +291,13 @@ int main(int argc, char * const *argv)
> > }
> > }
> >  
> > +   if (numeric &&
> > +   (output_flags &
> > +   (NFT_CTX_OUTPUT_REVERSEDNS |
> > +NFT_CTX_OUTPUT_SERVICE |
> > +NFT_CTX_OUTPUT_GUID)))
> > +   fprintf(stderr, "cannot combine `-n' with `-N', `-S', '-u'\n");
> > +

Why not? We could allow people to call 'nft -n -N' to make all numeric
but enable reverse DNS (IIRC). All we need to do is document that '-n'
is an alias for '-p -q ...' and that later flags overwrite earlier ones
(as usual with getopt()-based tools). Would simplify code a bit, too.

> > diff --git a/src/monitor.c b/src/monitor.c
> > index b2267e1f63e4..0e735ed5b1aa 100644
> > --- a/src/monitor.c
> > +++ b/src/monitor.c
> > @@ -835,11 +835,9 @@ static int netlink_events_newgen_cb(const struct 
> > nlmsghdr *nlh, int type,
> > }
> > if (genid >= 0) {
> > nft_mon_print(monh, "# new generation %d", genid);
> > -   if (pid >= 0) {
> > -   nft_mon_print(monh, " by process %d", pid);
> > -   if (!monh->ctx->nft->output.numeric)
> > -   nft_mon_print(monh, " (%s)", name);
> > -   }
> > +   if (pid >= 0)
> > +   nft_mon_print(monh, " by process %d (%s)", pid, name);
> > +
> > nft_mon_print(monh, "\n");
> > }
> >  

Yes, I think that's OK. People may still do 's/ (.*$//' if they don't
want it there.

Cheers, Phil


[PATCH nft 1/2,v2] src: get rid of nft_ctx_output_{get,set}_numeric()

2018-10-29 Thread Pablo Neira Ayuso
This patch adds NFT_CTX_OUTPUT_NUMERIC_SYMBOL, which replaces the last
client of the numeric level approach.

This patch updates `-n' option semantics to display all output
numerically.

Note that monitor code was still using the -n option to skip printing
the process name, this patch updates that path too to print it
inconditionally to simplify things.

Given the numeric levels have no more clients after this patch, remove
that code.

Update several tests/shell not to use -nn.

This patch removes NFT_CTX_OUTPUT_NUMERIC_PROTOCOL, since it is already
contained in NFT_CTX_OUTPUT_NUMERIC_SYMBOL.

Signed-off-by: Pablo Neira Ayuso 
---
v2: merge NFT_CTX_OUTPUT_NUMERIC_PROTOCOL into NFT_CTX_OUTPUT_NUMERIC_SYMBOL
to remove extra toggle.

 doc/libnftables.adoc| 42 +++--
 include/nftables.h  |  5 ++-
 include/nftables/libnftables.h  | 11 +--
 src/datatype.c  |  4 +--
 src/expression.c|  2 +-
 src/json.c  |  6 ++--
 src/libnftables.c   | 11 ---
 src/main.c  | 25 +++
 src/monitor.c   |  8 ++---
 tests/shell/testcases/netns/0001nft-f_0 |  2 +-
 tests/shell/testcases/netns/0002loosecommands_0 |  2 +-
 tests/shell/testcases/netns/0003many_0  |  2 +-
 12 files changed, 31 insertions(+), 89 deletions(-)

diff --git a/doc/libnftables.adoc b/doc/libnftables.adoc
index 788194396db1..dd9a1c93ee27 100644
--- a/doc/libnftables.adoc
+++ b/doc/libnftables.adoc
@@ -21,10 +21,6 @@ void nft_ctx_set_dry_run(struct nft_ctx* '\*ctx'*, bool* 
'dry'*);
 unsigned int nft_ctx_output_get_flags(struct nft_ctx* '\*ctx'*);
 void nft_ctx_output_set_flags(struct nft_ctx* '\*ctx'*, unsigned int* 
'flags'*);
 
-enum nft_numeric_level nft_ctx_output_get_numeric(struct nft_ctx* '\*ctx'*);
-void nft_ctx_output_set_numeric(struct nft_ctx* '\*ctx'*,
-   enum nft_numeric_level* 'level'*);
-
 unsigned int nft_ctx_output_get_debug(struct nft_ctx* '\*ctx'*);
 void nft_ctx_output_set_debug(struct nft_ctx* '\*ctx'*, unsigned int* 'mask'*);
 
@@ -91,8 +87,9 @@ enum {
 NFT_CTX_OUTPUT_JSON= (1 << 4),
 NFT_CTX_OUTPUT_ECHO= (1 << 5),
 NFT_CTX_OUTPUT_GUID= (1 << 6),
-NFT_CTX_OUTPUT_NUMERIC_PROTO = (1 << 7),
+NFT_CTX_OUTPUT_NUMERIC_SYMBOL = (1 << 7),
 NFT_CTX_OUTPUT_NUMERIC_PRIO = (1 << 8),
+
 };
 
 
@@ -121,41 +118,10 @@ NFT_CTX_OUTPUT_GUID::
 The *nft_ctx_output_get_flags*() function returns the output flags setting's 
value in 'ctx'.
 
 The *nft_ctx_output_set_flags*() function sets the output flags setting in 
'ctx' to the value of 'val'.
-NFT_CTX_OUTPUT_NUMERIC_PROTO::
-   Display layer 4 protocol numerically.
 NFT_CTX_OUTPUT_NUMERIC_PRIO::
Display base chain priority numerically.
-
-=== nft_ctx_output_get_numeric() and nft_ctx_output_set_numeric()
-These functions allow control over value representation in library output.
-For instance, port numbers by default are printed by their name (as listed in 
'/etc/services' file), if known.
-In libnftables, numeric output is leveled, defined as such:
-
-
-enum nft_numeric_level {
-NFT_NUMERIC_NONE,
-NFT_NUMERIC_ADDR,
-NFT_NUMERIC_PORT,
-NFT_NUMERIC_ALL,
-};
-
-
-Each numeric level includes all previous ones:
-
-NFT_NUMERIC_NONE::
-   No conversion into numeric format happens, this is the default.
-NFT_NUMERIC_ADDR::
-   Network addresses are always converted into numeric format.
-NFT_NUMERIC_PORT::
-   Network services are always converted into numeric format.
-NFT_NUMERIC_ALL::
-   Everything is converted into numeric format.
-
-The default numeric level is *NFT_NUMERIC_NONE*.
-
-The *nft_ctx_output_get_numeric*() function returns the numeric output 
setting's value contained in 'ctx'.
-
-The *nft_ctx_output_set_numeric*() function sets the numeric output setting in 
'ctx' to the value of 'level'.
+NFT_CTX_OUTPUT_NUMERIC_SYMBOL::
+   Display expression datatype as numeric value.
 
 === nft_ctx_output_get_debug() and nft_ctx_output_set_debug()
 Libnftables supports separate debugging of different parts of its internals.
diff --git a/include/nftables.h b/include/nftables.h
index a4d01e0cddea..21c0221b9548 100644
--- a/include/nftables.h
+++ b/include/nftables.h
@@ -17,7 +17,6 @@ struct cookie {
 
 struct output_ctx {
unsigned int flags;
-   unsigned int numeric;
union {
FILE *output_fp;
struct cookie output_cookie;
@@ -63,9 +62,9 @@ static inline bool nft_output_guid(const struct output_ctx 
*octx)
return octx->flags & NFT_CTX_OUTPUT_GUID;
 }
 
-static inline bool nft_output_numeric_proto(const struct output_ctx *octx)
+static inline bool 

[PATCH nft 2/2] expression: always print range expression numerically

2018-10-29 Thread Pablo Neira Ayuso
Otherwise we end up displaying things that we cannot parse as input.
Moreover, in a range, it's relevant to the user the values that are
enclosed in the range, so let's print this numerically.

Fixes: baa4e0e3fa5f ("src: add NFT_CTX_OUTPUT_NUMERIC_PROTO")
Reported-by: Phil Sutter 
Signed-off-by: Pablo Neira Ayuso 
---
v2: rebase upon ("src: get rid of nft_ctx_output_{get,set}_numeric()")

 src/expression.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/expression.c b/src/expression.c
index af78c39806b2..6298d5a7bdc8 100644
--- a/src/expression.c
+++ b/src/expression.c
@@ -662,7 +662,7 @@ static void range_expr_print(const struct expr *expr, 
struct output_ctx *octx)
 {
unsigned int flags = octx->flags;
 
-   octx->flags &= ~NFT_CTX_OUTPUT_SERVICE;
+   octx->flags &= ~(NFT_CTX_OUTPUT_SERVICE | NFT_CTX_OUTPUT_REVERSEDNS);
octx->flags |= NFT_CTX_OUTPUT_NUMERIC_SYMBOL;
expr_print(expr->left, octx);
nft_print(octx, "-");
-- 
2.11.0



Re: [PATCH nft] src: get rid of nft_ctx_output_{get,set}_numeric()

2018-10-29 Thread Pablo Neira Ayuso
@Phil, thinking here we could probably get rid of
NFT_CTX_OUTPUT_NUMERIC_PROTO, since it is contained already in
NFT_CTX_OUTPUT_NUMERIC_SYMBOL.

There's no option for -p anymore, so we could simply things a bit
before.

On Mon, Oct 29, 2018 at 04:03:32PM +0100, Pablo Neira Ayuso wrote:
> This patch adds NFT_CTX_OUTPUT_NUMERIC_SYMBOL, which replaces the last
> client of the numeric level approach.
> 
> This patch updates `-n' option semantics to display all output
> numerically.
> 
> Note that monitor code was still using the -n option to skip printing
> the process name, this patch updates that path too to print it
> inconditionally to simplify things.
> 
> Given the numeric levels have no more clients after this patch, remove
> that code.
> 
> Update several tests/shell not to use -nn.
> 
> Signed-off-by: Pablo Neira Ayuso 
> ---
> @Phil: I think it should be possible to evaluate numerical through flags
>in case this interface needs to remain in place for nftables.py
>interface. I did not update tests/py/ bits, I cannot run that
>until python infrastructure gets in sync with this patchset, no rush
>just a side note.
> 
>  doc/libnftables.adoc| 35 
> -
>  include/nftables.h  |  6 -
>  include/nftables/libnftables.h  | 10 +--
>  src/datatype.c  |  2 +-
>  src/json.c  |  2 +-
>  src/libnftables.c   | 11 
>  src/main.c  | 26 +-
>  src/monitor.c   |  8 +++---
>  tests/shell/testcases/netns/0001nft-f_0 |  2 +-
>  tests/shell/testcases/netns/0002loosecommands_0 |  2 +-
>  tests/shell/testcases/netns/0003many_0  |  2 +-
>  11 files changed, 27 insertions(+), 79 deletions(-)
> 
> diff --git a/doc/libnftables.adoc b/doc/libnftables.adoc
> index 788194396db1..52b8fb0ca5a1 100644
> --- a/doc/libnftables.adoc
> +++ b/doc/libnftables.adoc
> @@ -21,10 +21,6 @@ void nft_ctx_set_dry_run(struct nft_ctx* '\*ctx'*, bool* 
> 'dry'*);
>  unsigned int nft_ctx_output_get_flags(struct nft_ctx* '\*ctx'*);
>  void nft_ctx_output_set_flags(struct nft_ctx* '\*ctx'*, unsigned int* 
> 'flags'*);
>  
> -enum nft_numeric_level nft_ctx_output_get_numeric(struct nft_ctx* '\*ctx'*);
> -void nft_ctx_output_set_numeric(struct nft_ctx* '\*ctx'*,
> - enum nft_numeric_level* 'level'*);
> -
>  unsigned int nft_ctx_output_get_debug(struct nft_ctx* '\*ctx'*);
>  void nft_ctx_output_set_debug(struct nft_ctx* '\*ctx'*, unsigned int* 
> 'mask'*);
>  
> @@ -126,37 +122,6 @@ NFT_CTX_OUTPUT_NUMERIC_PROTO::
>  NFT_CTX_OUTPUT_NUMERIC_PRIO::
>   Display base chain priority numerically.
>  
> -=== nft_ctx_output_get_numeric() and nft_ctx_output_set_numeric()
> -These functions allow control over value representation in library output.
> -For instance, port numbers by default are printed by their name (as listed 
> in '/etc/services' file), if known.
> -In libnftables, numeric output is leveled, defined as such:
> -
> -
> -enum nft_numeric_level {
> -NFT_NUMERIC_NONE,
> -NFT_NUMERIC_ADDR,
> -NFT_NUMERIC_PORT,
> -NFT_NUMERIC_ALL,
> -};
> -
> -
> -Each numeric level includes all previous ones:
> -
> -NFT_NUMERIC_NONE::
> - No conversion into numeric format happens, this is the default.
> -NFT_NUMERIC_ADDR::
> - Network addresses are always converted into numeric format.
> -NFT_NUMERIC_PORT::
> - Network services are always converted into numeric format.
> -NFT_NUMERIC_ALL::
> - Everything is converted into numeric format.
> -
> -The default numeric level is *NFT_NUMERIC_NONE*.
> -
> -The *nft_ctx_output_get_numeric*() function returns the numeric output 
> setting's value contained in 'ctx'.
> -
> -The *nft_ctx_output_set_numeric*() function sets the numeric output setting 
> in 'ctx' to the value of 'level'.
> -
>  === nft_ctx_output_get_debug() and nft_ctx_output_set_debug()
>  Libnftables supports separate debugging of different parts of its internals.
>  To facilitate this, debugging output is controlled via a bit mask.
> diff --git a/include/nftables.h b/include/nftables.h
> index a4d01e0cddea..5c0292615b3f 100644
> --- a/include/nftables.h
> +++ b/include/nftables.h
> @@ -17,7 +17,6 @@ struct cookie {
>  
>  struct output_ctx {
>   unsigned int flags;
> - unsigned int numeric;
>   union {
>   FILE *output_fp;
>   struct cookie output_cookie;
> @@ -73,6 +72,11 @@ static inline bool nft_output_numeric_prio(const struct 
> output_ctx *octx)
>   return octx->flags & NFT_CTX_OUTPUT_NUMERIC_PRIO;
>  }
>  
> +static inline bool nft_output_numeric_symbol(const struct output_ctx *octx)
> +{
> + return octx->flags & NFT_CTX_OUTPUT_NUMERIC_SYMBOL;
> +}
> +
>  struct nft_cache {
>   uint16_t 

Re: [PATCH nft] src: add -p to print layer 4 protocol numerically

2018-10-29 Thread Pablo Neira Ayuso
On Mon, Oct 29, 2018 at 05:49:56PM +0100, Pablo Neira Ayuso wrote:
> On Mon, Oct 29, 2018 at 05:46:29PM +0100, Phil Sutter wrote:
> > Hi,
> > 
> > On Mon, Oct 29, 2018 at 02:10:27PM +0100, Pablo Neira Ayuso wrote:
> > > We keep printing layer 4 protocols as literals since we do not use
> > > /etc/protocols. Add -p option to print layer 4 protocols numerically.
> > > 
> > > Signed-off-by: Pablo Neira Ayuso 
> > 
> > Acked-by: Phil Sutter 
> > 
> > One question:
> > 
> > [...]
> > > diff --git a/src/datatype.c b/src/datatype.c
> > > index 48eaca277757..2e957e60bb71 100644
> > > --- a/src/datatype.c
> > > +++ b/src/datatype.c
> > > @@ -564,7 +564,7 @@ static void inet_protocol_type_print(const struct 
> > > expr *expr,
> > >  {
> > >   struct protoent *p;
> > >  
> > > - if (octx->numeric < NFT_NUMERIC_ALL) {
> > > + if (!nft_output_numeric_protocol(octx)) {
> > >   p = getprotobynumber(mpz_get_uint8(expr->value));
> > >   if (p != NULL) {
> > >   nft_print(octx, "%s", p->p_name);
> > 
> > In range_expression_print(), we did:
> > 
> > | octx->numeric += NFT_NUMERIC_ALL + 1
> > 
> > to avoid confusion with names containing dashes. I see that now the same
> > function just removes NFT_CTX_OUTPUT_SERVICE bit instead. Is that
> > sufficient? I guess users could still turn on reverse DNS while listing
> > interval sets with IP addresses, right?
> 
> Right, reverse dns listing was broken before this patch, we should
> disable it too. Sending a patch for this.

Oh, indeed. Actually I broke it with this patch :-), since
octx->numeric < NFT_NUMERIC_ALL was covering that case. Will included
a Fixes: tag in http://patchwork.ozlabs.org/patch/990427/

Thanks!


Re: [nft PATCH] JSON: Add support for echo option

2018-10-29 Thread Pablo Neira Ayuso
On Mon, Oct 29, 2018 at 05:54:25PM +0100, Phil Sutter wrote:
> Hi,
> 
> On Mon, Oct 29, 2018 at 04:19:03PM +0100, Pablo Neira Ayuso wrote:
> > On Fri, Oct 26, 2018 at 03:01:38PM +0200, Phil Sutter wrote:
> > > The basic principle is to not return a JSON object freshly created from
> > > netlink responses, but just update the existing user-provided one to
> > > make sure callers get back exactly what they expect.
> > 
> > Applied, thanks Phil.
> > 
> > > To achieve that, keep the parsed JSON object around in a global variable
> > > ('cur_root') and provide a custom callback to insert handles into it
> > > from received netlink messages. The tricky bit here is updating rules
> > > since unique identification is problematic. Therefore drop possibly
> > > present handles from input and later assume updates are received in
> > > order so the first rule not having a handle set is the right one.
> > 
> > You can use the netlink sequence number to correlate the rule that you
> > are adding with the object that the kernel gives us. The kernel will
> > reply including the sequence number of the original request in the
> > reply you get. Like this, you will not need obj_info_matches() and
> > such I think. In case this helps you simplify this code.
> 
> That's an interesting idea, but I don't think it would fly: netlink
> seqnum is per batch and potentially there are multiple add commands
> contained in it.

netlink seqnum allow us to know what command in a batch is causing
problems, so there should be one unique sequence number per command.
Assuming a batch with two commands, each command will get a different
sequence number. This is how we can correlate what line in a file is
causing what problem when we load a batch via nft -f. The kernel
doesn't stop on the first error, so it keeps trying so users get
a listing of commands causing errors in one shot [1].

> So although I could make sure the kernel's reply matches the current
> request, I still have to figure out which JSON add command to
> update. Or am I missing something?

You need to keep a list of commands, that store json object that you
send to the kernel and its netlink sequence number, so you can use to
find a matching coming as reply. This should be similar to
nft_netlink(), we could event generalize it so it can be re-used from
the json codebase too.

Does this clarify?

[1] Note that we still have problems with runtime lookups that results
in table/chain/etc does not exist, ie. table_lookup() calls from
userspace using the cache, we could create error table/chain/set
objects so we can keep processing the batch on unexisting
tables/chains/sets and send everything to the kernel, then get all
errors in one go. This is convenient so users developing a firewall
policy don't have to hit one error at a time, basically we will get
this behaving like a real compiler which spews all errors in the code
in one go. But that is a different problem that I would like to sort
out at some point. Anyway, the kernel is not the limiting side here,
we only need to extend userspace to remove the limitation I'm
describing in this side note.


Re: [PATCH nft 1/2,v2] src: add NFT_CTX_OUTPUT_NUMERIC_PROTO

2018-10-29 Thread Phil Sutter
On Mon, Oct 29, 2018 at 03:31:37PM +0100, Pablo Neira Ayuso wrote:
> We keep printing layer 4 protocols as literals since we do not use
> /etc/protocols. This new flag allows us to print it as a number.
> 
> libnftables internally uses this to print layer 4 protocol as numbers
> when part of a range.
> 
> Signed-off-by: Pablo Neira Ayuso 

Acked-by: Phil Sutter 


Re: [PATCH nft 2/2,v2] src: add -y to priority base chain nummerically

2018-10-29 Thread Phil Sutter
On Mon, Oct 29, 2018 at 03:31:38PM +0100, Pablo Neira Ayuso wrote:
> By default base chains are printed using default hook priority
> definitions. Add -y option to print them as numbers.
> 
> Signed-off-by: Pablo Neira Ayuso 

Acked-by: Phil Sutter 


[PATCH nft] expression: always print range expression numerically

2018-10-29 Thread Pablo Neira Ayuso
Otherwise we end up displaying things that we cannot parse as input.
Moreover, in a range, it's relevant to the user the values that are
enclosed in the range, so let's print this numerically.

Reported-by: Phil Sutter 
Signed-off-by: Pablo Neira Ayuso 
---
Based on my recent patches, still not pushed out.

 src/expression.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/expression.c b/src/expression.c
index 5ff469c561d5..aadc4efcba82 100644
--- a/src/expression.c
+++ b/src/expression.c
@@ -662,8 +662,8 @@ static void range_expr_print(const struct expr *expr, 
struct output_ctx *octx)
 {
unsigned int flags = octx->flags;
 
-   octx->flags &= ~NFT_CTX_OUTPUT_SERVICE;
-   octx->flags |= NFT_CTX_OUTPUT_NUMERIC_PROTO;
+   octx->flags &= ~(NFT_CTX_OUTPUT_SERVICE | NFT_CTX_OUTPUT_REVERSEDNS);
+   octx->flags |= (NFT_CTX_OUTPUT_NUMERIC_PROTO | 
NFT_CTX_OUTPUT_NUMERIC_SYMBOL);
expr_print(expr->left, octx);
nft_print(octx, "-");
expr_print(expr->right, octx);
-- 
2.11.0



Re: [nft PATCH] JSON: Add support for echo option

2018-10-29 Thread Phil Sutter
Hi,

On Mon, Oct 29, 2018 at 04:19:03PM +0100, Pablo Neira Ayuso wrote:
> On Fri, Oct 26, 2018 at 03:01:38PM +0200, Phil Sutter wrote:
> > The basic principle is to not return a JSON object freshly created from
> > netlink responses, but just update the existing user-provided one to
> > make sure callers get back exactly what they expect.
> 
> Applied, thanks Phil.
> 
> > To achieve that, keep the parsed JSON object around in a global variable
> > ('cur_root') and provide a custom callback to insert handles into it
> > from received netlink messages. The tricky bit here is updating rules
> > since unique identification is problematic. Therefore drop possibly
> > present handles from input and later assume updates are received in
> > order so the first rule not having a handle set is the right one.
> 
> You can use the netlink sequence number to correlate the rule that you
> are adding with the object that the kernel gives us. The kernel will
> reply including the sequence number of the original request in the
> reply you get. Like this, you will not need obj_info_matches() and
> such I think. In case this helps you simplify this code.

That's an interesting idea, but I don't think it would fly: netlink
seqnum is per batch and potentially there are multiple add commands
contained in it. So although I could make sure the kernel's reply
matches the current request, I still have to figure out which JSON add
command to update. Or am I missing something?

Thanks, Phil


Re: [PATCH nft] src: add -p to print layer 4 protocol numerically

2018-10-29 Thread Pablo Neira Ayuso
On Mon, Oct 29, 2018 at 05:46:29PM +0100, Phil Sutter wrote:
> Hi,
> 
> On Mon, Oct 29, 2018 at 02:10:27PM +0100, Pablo Neira Ayuso wrote:
> > We keep printing layer 4 protocols as literals since we do not use
> > /etc/protocols. Add -p option to print layer 4 protocols numerically.
> > 
> > Signed-off-by: Pablo Neira Ayuso 
> 
> Acked-by: Phil Sutter 

BTW, this patch:

http://patchwork.ozlabs.org/patch/990273/

replaces this one, it's basically the same but no -p option.


Re: [nft PATCH] tests/shell: Add testcase for cache update problems

2018-10-29 Thread Phil Sutter
On Mon, Oct 29, 2018 at 04:20:52PM +0100, Pablo Neira Ayuso wrote:
> On Fri, Oct 26, 2018 at 11:42:05AM +0200, Phil Sutter wrote:
> > The first test in there shows how the current cache update strategy
> > causes trouble. The second test shows that proposed "locking" of cache
> > when local entries are added is flawed, too.
> 
> Applied, thanks.
> 
> BTW, is your recent patch to lock the cache solving all caching issues?

Nope, part two of the testcase I submitted here exposes why it's flawed,
too. :(



Re: [PATCH nft] src: add -p to print layer 4 protocol numerically

2018-10-29 Thread Phil Sutter
Hi,

On Mon, Oct 29, 2018 at 02:10:27PM +0100, Pablo Neira Ayuso wrote:
> We keep printing layer 4 protocols as literals since we do not use
> /etc/protocols. Add -p option to print layer 4 protocols numerically.
> 
> Signed-off-by: Pablo Neira Ayuso 

Acked-by: Phil Sutter 

One question:

[...]
> diff --git a/src/datatype.c b/src/datatype.c
> index 48eaca277757..2e957e60bb71 100644
> --- a/src/datatype.c
> +++ b/src/datatype.c
> @@ -564,7 +564,7 @@ static void inet_protocol_type_print(const struct expr 
> *expr,
>  {
>   struct protoent *p;
>  
> - if (octx->numeric < NFT_NUMERIC_ALL) {
> + if (!nft_output_numeric_protocol(octx)) {
>   p = getprotobynumber(mpz_get_uint8(expr->value));
>   if (p != NULL) {
>   nft_print(octx, "%s", p->p_name);

In range_expression_print(), we did:

| octx->numeric += NFT_NUMERIC_ALL + 1

to avoid confusion with names containing dashes. I see that now the same
function just removes NFT_CTX_OUTPUT_SERVICE bit instead. Is that
sufficient? I guess users could still turn on reverse DNS while listing
interval sets with IP addresses, right?

Cheers, Phil


Re: [nft PATCH] nft.8: Document log level audit

2018-10-29 Thread Pablo Neira Ayuso
On Sat, Oct 27, 2018 at 12:15:50PM +0200, Phil Sutter wrote:
> Since this pseudo log level fundamentally changes behaviour of log
> statement, dedicate this mode a separate paragraph.

Applied, thanks Phil.


Re: [nft PATCH] JSON: Add support for echo option

2018-10-29 Thread Pablo Neira Ayuso
On Fri, Oct 26, 2018 at 03:01:38PM +0200, Phil Sutter wrote:
> The basic principle is to not return a JSON object freshly created from
> netlink responses, but just update the existing user-provided one to
> make sure callers get back exactly what they expect.

Applied, thanks Phil.

> To achieve that, keep the parsed JSON object around in a global variable
> ('cur_root') and provide a custom callback to insert handles into it
> from received netlink messages. The tricky bit here is updating rules
> since unique identification is problematic. Therefore drop possibly
> present handles from input and later assume updates are received in
> order so the first rule not having a handle set is the right one.

You can use the netlink sequence number to correlate the rule that you
are adding with the object that the kernel gives us. The kernel will
reply including the sequence number of the original request in the
reply you get. Like this, you will not need obj_info_matches() and
such I think. In case this helps you simplify this code.


[PATCH nft] src: get rid of nft_ctx_output_{get,set}_numeric()

2018-10-29 Thread Pablo Neira Ayuso
This patch adds NFT_CTX_OUTPUT_NUMERIC_SYMBOL, which replaces the last
client of the numeric level approach.

This patch updates `-n' option semantics to display all output
numerically.

Note that monitor code was still using the -n option to skip printing
the process name, this patch updates that path too to print it
inconditionally to simplify things.

Given the numeric levels have no more clients after this patch, remove
that code.

Update several tests/shell not to use -nn.

Signed-off-by: Pablo Neira Ayuso 
---
@Phil: I think it should be possible to evaluate numerical through flags
   in case this interface needs to remain in place for nftables.py
   interface. I did not update tests/py/ bits, I cannot run that
   until python infrastructure gets in sync with this patchset, no rush
   just a side note.

 doc/libnftables.adoc| 35 -
 include/nftables.h  |  6 -
 include/nftables/libnftables.h  | 10 +--
 src/datatype.c  |  2 +-
 src/json.c  |  2 +-
 src/libnftables.c   | 11 
 src/main.c  | 26 +-
 src/monitor.c   |  8 +++---
 tests/shell/testcases/netns/0001nft-f_0 |  2 +-
 tests/shell/testcases/netns/0002loosecommands_0 |  2 +-
 tests/shell/testcases/netns/0003many_0  |  2 +-
 11 files changed, 27 insertions(+), 79 deletions(-)

diff --git a/doc/libnftables.adoc b/doc/libnftables.adoc
index 788194396db1..52b8fb0ca5a1 100644
--- a/doc/libnftables.adoc
+++ b/doc/libnftables.adoc
@@ -21,10 +21,6 @@ void nft_ctx_set_dry_run(struct nft_ctx* '\*ctx'*, bool* 
'dry'*);
 unsigned int nft_ctx_output_get_flags(struct nft_ctx* '\*ctx'*);
 void nft_ctx_output_set_flags(struct nft_ctx* '\*ctx'*, unsigned int* 
'flags'*);
 
-enum nft_numeric_level nft_ctx_output_get_numeric(struct nft_ctx* '\*ctx'*);
-void nft_ctx_output_set_numeric(struct nft_ctx* '\*ctx'*,
-   enum nft_numeric_level* 'level'*);
-
 unsigned int nft_ctx_output_get_debug(struct nft_ctx* '\*ctx'*);
 void nft_ctx_output_set_debug(struct nft_ctx* '\*ctx'*, unsigned int* 'mask'*);
 
@@ -126,37 +122,6 @@ NFT_CTX_OUTPUT_NUMERIC_PROTO::
 NFT_CTX_OUTPUT_NUMERIC_PRIO::
Display base chain priority numerically.
 
-=== nft_ctx_output_get_numeric() and nft_ctx_output_set_numeric()
-These functions allow control over value representation in library output.
-For instance, port numbers by default are printed by their name (as listed in 
'/etc/services' file), if known.
-In libnftables, numeric output is leveled, defined as such:
-
-
-enum nft_numeric_level {
-NFT_NUMERIC_NONE,
-NFT_NUMERIC_ADDR,
-NFT_NUMERIC_PORT,
-NFT_NUMERIC_ALL,
-};
-
-
-Each numeric level includes all previous ones:
-
-NFT_NUMERIC_NONE::
-   No conversion into numeric format happens, this is the default.
-NFT_NUMERIC_ADDR::
-   Network addresses are always converted into numeric format.
-NFT_NUMERIC_PORT::
-   Network services are always converted into numeric format.
-NFT_NUMERIC_ALL::
-   Everything is converted into numeric format.
-
-The default numeric level is *NFT_NUMERIC_NONE*.
-
-The *nft_ctx_output_get_numeric*() function returns the numeric output 
setting's value contained in 'ctx'.
-
-The *nft_ctx_output_set_numeric*() function sets the numeric output setting in 
'ctx' to the value of 'level'.
-
 === nft_ctx_output_get_debug() and nft_ctx_output_set_debug()
 Libnftables supports separate debugging of different parts of its internals.
 To facilitate this, debugging output is controlled via a bit mask.
diff --git a/include/nftables.h b/include/nftables.h
index a4d01e0cddea..5c0292615b3f 100644
--- a/include/nftables.h
+++ b/include/nftables.h
@@ -17,7 +17,6 @@ struct cookie {
 
 struct output_ctx {
unsigned int flags;
-   unsigned int numeric;
union {
FILE *output_fp;
struct cookie output_cookie;
@@ -73,6 +72,11 @@ static inline bool nft_output_numeric_prio(const struct 
output_ctx *octx)
return octx->flags & NFT_CTX_OUTPUT_NUMERIC_PRIO;
 }
 
+static inline bool nft_output_numeric_symbol(const struct output_ctx *octx)
+{
+   return octx->flags & NFT_CTX_OUTPUT_NUMERIC_SYMBOL;
+}
+
 struct nft_cache {
uint16_tgenid;
struct list_headlist;
diff --git a/include/nftables/libnftables.h b/include/nftables/libnftables.h
index fb81edc0df07..82b4181de0cc 100644
--- a/include/nftables/libnftables.h
+++ b/include/nftables/libnftables.h
@@ -26,13 +26,6 @@ enum nft_debug_level {
NFT_DEBUG_SEGTREE   = 0x40,
 };
 
-enum nft_numeric_level {
-   NFT_NUMERIC_NONE,
-   NFT_NUMERIC_ADDR,
-   NFT_NUMERIC_PORT,
-   NFT_NUMERIC_ALL,
-};
-
 /**
  * Possible flags to 

[PATCH nft 1/2,v2] src: add NFT_CTX_OUTPUT_NUMERIC_PROTO

2018-10-29 Thread Pablo Neira Ayuso
We keep printing layer 4 protocols as literals since we do not use
/etc/protocols. This new flag allows us to print it as a number.

libnftables internally uses this to print layer 4 protocol as numbers
when part of a range.

Signed-off-by: Pablo Neira Ayuso 
---
This patch was "src: add -p to print layer 4 protocol numerically."

 doc/libnftables.adoc   | 3 +++
 include/nftables.h | 5 +
 include/nftables/libnftables.h | 1 +
 src/datatype.c | 2 +-
 src/expression.c   | 1 +
 src/json.c | 3 ++-
 6 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/doc/libnftables.adoc b/doc/libnftables.adoc
index 67d9f261034c..dc3299f037df 100644
--- a/doc/libnftables.adoc
+++ b/doc/libnftables.adoc
@@ -91,6 +91,7 @@ enum {
 NFT_CTX_OUTPUT_JSON= (1 << 4),
 NFT_CTX_OUTPUT_ECHO= (1 << 5),
 NFT_CTX_OUTPUT_GUID= (1 << 6),
+NFT_CTX_OUTPUT_NUMERIC_PROTO = (1 << 7),
 };
 
 
@@ -119,6 +120,8 @@ NFT_CTX_OUTPUT_GUID::
 The *nft_ctx_output_get_flags*() function returns the output flags setting's 
value in 'ctx'.
 
 The *nft_ctx_output_set_flags*() function sets the output flags setting in 
'ctx' to the value of 'val'.
+NFT_CTX_OUTPUT_NUMERIC_PROTO::
+   Display layer 4 protocol numerically.
 
 === nft_ctx_output_get_numeric() and nft_ctx_output_set_numeric()
 These functions allow control over value representation in library output.
diff --git a/include/nftables.h b/include/nftables.h
index 2dff07fef599..d0031e841134 100644
--- a/include/nftables.h
+++ b/include/nftables.h
@@ -63,6 +63,11 @@ static inline bool nft_output_guid(const struct output_ctx 
*octx)
return octx->flags & NFT_CTX_OUTPUT_GUID;
 }
 
+static inline bool nft_output_numeric_proto(const struct output_ctx *octx)
+{
+   return octx->flags & NFT_CTX_OUTPUT_NUMERIC_PROTO;
+}
+
 struct nft_cache {
uint16_tgenid;
struct list_headlist;
diff --git a/include/nftables/libnftables.h b/include/nftables/libnftables.h
index ff7b47aa3160..74f2dabbd1d5 100644
--- a/include/nftables/libnftables.h
+++ b/include/nftables/libnftables.h
@@ -52,6 +52,7 @@ enum {
NFT_CTX_OUTPUT_JSON = (1 << 4),
NFT_CTX_OUTPUT_ECHO = (1 << 5),
NFT_CTX_OUTPUT_GUID = (1 << 6),
+   NFT_CTX_OUTPUT_NUMERIC_PROTO= (1 << 7),
 };
 
 unsigned int nft_ctx_output_get_flags(struct nft_ctx *ctx);
diff --git a/src/datatype.c b/src/datatype.c
index 48eaca277757..bfb70a6ebb76 100644
--- a/src/datatype.c
+++ b/src/datatype.c
@@ -564,7 +564,7 @@ static void inet_protocol_type_print(const struct expr 
*expr,
 {
struct protoent *p;
 
-   if (octx->numeric < NFT_NUMERIC_ALL) {
+   if (!nft_output_numeric_proto(octx)) {
p = getprotobynumber(mpz_get_uint8(expr->value));
if (p != NULL) {
nft_print(octx, "%s", p->p_name);
diff --git a/src/expression.c b/src/expression.c
index 25883ea771ef..5ff469c561d5 100644
--- a/src/expression.c
+++ b/src/expression.c
@@ -663,6 +663,7 @@ static void range_expr_print(const struct expr *expr, 
struct output_ctx *octx)
unsigned int flags = octx->flags;
 
octx->flags &= ~NFT_CTX_OUTPUT_SERVICE;
+   octx->flags |= NFT_CTX_OUTPUT_NUMERIC_PROTO;
expr_print(expr->left, octx);
nft_print(octx, "-");
expr_print(expr->right, octx);
diff --git a/src/json.c b/src/json.c
index e90445fc9276..8a2bcd658bd8 100644
--- a/src/json.c
+++ b/src/json.c
@@ -448,6 +448,7 @@ json_t *range_expr_json(const struct expr *expr, struct 
output_ctx *octx)
json_t *root;
 
octx->flags &= ~NFT_CTX_OUTPUT_SERVICE;
+   octx->flags |= NFT_CTX_OUTPUT_NUMERIC_PROTO;
root = json_pack("{s:[o, o]}", "range",
 expr_print_json(expr->left, octx),
 expr_print_json(expr->right, octx));
@@ -961,7 +962,7 @@ json_t *inet_protocol_type_json(const struct expr *expr,
 {
struct protoent *p;
 
-   if (octx->numeric < NFT_NUMERIC_ALL) {
+   if (!nft_output_numeric_proto(octx)) {
p = getprotobynumber(mpz_get_uint8(expr->value));
if (p != NULL)
return json_string(p->p_name);
-- 
2.11.0



[PATCH nft 2/2,v2] src: add -y to priority base chain nummerically

2018-10-29 Thread Pablo Neira Ayuso
By default base chains are printed using default hook priority
definitions. Add -y option to print them as numbers.

Signed-off-by: Pablo Neira Ayuso 
---
v2: Rebase on top of "src: add NFT_CTX_OUTPUT_NUMERIC_PROTO"

 doc/libnftables.adoc   |  3 +++
 doc/nft.txt|  4 
 include/nftables.h |  5 +
 include/nftables/libnftables.h |  1 +
 src/main.c | 12 ++--
 src/rule.c | 20 ++--
 6 files changed, 33 insertions(+), 12 deletions(-)

diff --git a/doc/libnftables.adoc b/doc/libnftables.adoc
index dc3299f037df..788194396db1 100644
--- a/doc/libnftables.adoc
+++ b/doc/libnftables.adoc
@@ -92,6 +92,7 @@ enum {
 NFT_CTX_OUTPUT_ECHO= (1 << 5),
 NFT_CTX_OUTPUT_GUID= (1 << 6),
 NFT_CTX_OUTPUT_NUMERIC_PROTO = (1 << 7),
+NFT_CTX_OUTPUT_NUMERIC_PRIO = (1 << 8),
 };
 
 
@@ -122,6 +123,8 @@ The *nft_ctx_output_get_flags*() function returns the 
output flags setting's val
 The *nft_ctx_output_set_flags*() function sets the output flags setting in 
'ctx' to the value of 'val'.
 NFT_CTX_OUTPUT_NUMERIC_PROTO::
Display layer 4 protocol numerically.
+NFT_CTX_OUTPUT_NUMERIC_PRIO::
+   Display base chain priority numerically.
 
 === nft_ctx_output_get_numeric() and nft_ctx_output_set_numeric()
 These functions allow control over value representation in library output.
diff --git a/doc/nft.txt b/doc/nft.txt
index 39527c4e8e55..99ac0e333a93 100644
--- a/doc/nft.txt
+++ b/doc/nft.txt
@@ -56,6 +56,10 @@ For a full summary of options, run *nft --help*.
 *--guid**::
Translate numeric UID/GID to names as defined by /etc/passwd and 
/etc/group.
 
+*-y*::
+*--numeric-priority**::
+   Display base chain priority numerically.
+
 *-c*::
 *--check*::
Check commands validity without actually applying the changes.
diff --git a/include/nftables.h b/include/nftables.h
index d0031e841134..51eaad3cbdf4 100644
--- a/include/nftables.h
+++ b/include/nftables.h
@@ -68,6 +68,11 @@ static inline bool nft_output_numeric_proto(const struct 
output_ctx *octx)
return octx->flags & NFT_CTX_OUTPUT_NUMERIC_PROTO;
 }
 
+static inline bool nft_output_numeric_priority(const struct output_ctx *octx)
+{
+   return octx->flags & NFT_CTX_OUTPUT_NUMERIC_PRIO;
+}
+
 struct nft_cache {
uint16_tgenid;
struct list_headlist;
diff --git a/include/nftables/libnftables.h b/include/nftables/libnftables.h
index 74f2dabbd1d5..fb81edc0df07 100644
--- a/include/nftables/libnftables.h
+++ b/include/nftables/libnftables.h
@@ -53,6 +53,7 @@ enum {
NFT_CTX_OUTPUT_ECHO = (1 << 5),
NFT_CTX_OUTPUT_GUID = (1 << 6),
NFT_CTX_OUTPUT_NUMERIC_PROTO= (1 << 7),
+   NFT_CTX_OUTPUT_NUMERIC_PRIO = (1 << 8),
 };
 
 unsigned int nft_ctx_output_get_flags(struct nft_ctx *ctx);
diff --git a/src/main.c b/src/main.c
index 0c8fa1e9a108..883261fc9d8b 100644
--- a/src/main.c
+++ b/src/main.c
@@ -40,10 +40,10 @@ enum opt_vals {
OPT_HANDLE_OUTPUT   = 'a',
OPT_ECHO= 'e',
OPT_GUID= 'u',
+   OPT_NUMERIC_PRIO= 'y',
OPT_INVALID = '?',
 };
-
-#define OPTSTRING  "hvcf:iI:jvnsNaeSu"
+#define OPTSTRING  "hvcf:iI:jvnsNaeSupy"
 
 static const struct option options[] = {
{
@@ -110,6 +110,10 @@ static const struct option options[] = {
.val= OPT_GUID,
},
{
+   .name   = "numeric-priority",
+   .val= OPT_NUMERIC_PRIO,
+   },
+   {
.name   = NULL
}
 };
@@ -135,6 +139,7 @@ static void show_help(const char *name)
 "  -u, --guid  Print UID/GID as defined in /etc/passwd and 
/etc/group.\n"
 "  -N  Translate IP addresses to names.\n"
 "  -S, --service   Translate ports to service names as 
described in /etc/services.\n"
+"  -y, --numeric-priority  Print chain priority numerically.\n"
 "  -a, --handleOutput rule handle.\n"
 "  -e, --echo  Echo what has been added, inserted or 
replaced.\n"
 "  -I, --includepathAdd  to the paths searched 
for include files. Default is: %s\n"
@@ -285,6 +290,9 @@ int main(int argc, char * const *argv)
case OPT_GUID:
output_flags |= NFT_CTX_OUTPUT_GUID;
break;
+   case OPT_NUMERIC_PRIO:
+   output_flags |= NFT_CTX_OUTPUT_NUMERIC_PRIO;
+   break;
case OPT_INVALID:
exit(EXIT_FAILURE);
}
diff --git a/src/rule.c b/src/rule.c
index 33cbf0e2e9bb..cdf204cd4d8c 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -946,8 +946,9 @@ int std_prio_lookup(const char *std_prio_name, int family, 
int hook)

Re: [PATCH 1/2 nft,v2] src: add -p to print layer 4 protocol numerically

2018-10-29 Thread Pablo Neira Ayuso
On Mon, Oct 29, 2018 at 02:55:15PM +0100, Pablo Neira Ayuso wrote:
> Hi Phil,
> 
> I would probably keep back this patch, it's too fine grain. Instead I
> would turn -n into 'display everything numerically' after this
> patchset.
> 
> Hence, we have the fine grain toggles for stateless, reverse dns,
> service, guid and chain priority. Then the global toggles to turn
> everything into numeric.
> 
> That should be enough by now I think.

Hm, it seems we need this for ranges.

So I would expose this as a flag but not as an option in nft.
Sending new patch.


Re: [PATCH 1/2 nft,v2] src: add -p to print layer 4 protocol numerically

2018-10-29 Thread Pablo Neira Ayuso
Hi Phil,

I would probably keep back this patch, it's too fine grain. Instead I
would turn -n into 'display everything numerically' after this
patchset.

Hence, we have the fine grain toggles for stateless, reverse dns,
service, guid and chain priority. Then the global toggles to turn
everything into numeric.

That should be enough by now I think.

Let me know, thanks.

On Mon, Oct 29, 2018 at 02:39:51PM +0100, Pablo Neira Ayuso wrote:
> We keep printing layer 4 protocols as literals since we do not use
> /etc/protocols. Add -p option to print layer 4 protocols numerically.
> 
> Signed-off-by: Pablo Neira Ayuso 
> ---
> v2: update documentation.
> Rename to NUMERIC_PROTO.
> 
>  doc/libnftables.adoc   |  3 +++
>  doc/nft.txt|  4 
>  include/nftables.h |  5 +
>  include/nftables/libnftables.h |  1 +
>  src/datatype.c |  2 +-
>  src/main.c | 11 ++-
>  6 files changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/doc/libnftables.adoc b/doc/libnftables.adoc
> index 67d9f261034c..dc3299f037df 100644
> --- a/doc/libnftables.adoc
> +++ b/doc/libnftables.adoc
> @@ -91,6 +91,7 @@ enum {
>  NFT_CTX_OUTPUT_JSON= (1 << 4),
>  NFT_CTX_OUTPUT_ECHO= (1 << 5),
>  NFT_CTX_OUTPUT_GUID= (1 << 6),
> +NFT_CTX_OUTPUT_NUMERIC_PROTO = (1 << 7),
>  };
>  
>  
> @@ -119,6 +120,8 @@ NFT_CTX_OUTPUT_GUID::
>  The *nft_ctx_output_get_flags*() function returns the output flags setting's 
> value in 'ctx'.
>  
>  The *nft_ctx_output_set_flags*() function sets the output flags setting in 
> 'ctx' to the value of 'val'.
> +NFT_CTX_OUTPUT_NUMERIC_PROTO::
> + Display layer 4 protocol numerically.
>  
>  === nft_ctx_output_get_numeric() and nft_ctx_output_set_numeric()
>  These functions allow control over value representation in library output.
> diff --git a/doc/nft.txt b/doc/nft.txt
> index 39527c4e8e55..4ef8c5b3506f 100644
> --- a/doc/nft.txt
> +++ b/doc/nft.txt
> @@ -56,6 +56,10 @@ For a full summary of options, run *nft --help*.
>  *--guid**::
>   Translate numeric UID/GID to names as defined by /etc/passwd and 
> /etc/group.
>  
> +*-p*::
> +*--numeric-protocol**::
> + Display layer 4 protocol numerically.
> +
>  *-c*::
>  *--check*::
>   Check commands validity without actually applying the changes.
> diff --git a/include/nftables.h b/include/nftables.h
> index 57203dfcce3f..8f483bf84ff6 100644
> --- a/include/nftables.h
> +++ b/include/nftables.h
> @@ -77,6 +77,11 @@ static inline bool nft_output_guid(const struct output_ctx 
> *octx)
>   return octx->flags & NFT_CTX_OUTPUT_GUID;
>  }
>  
> +static inline bool nft_output_numeric_proto(const struct output_ctx *octx)
> +{
> + return octx->flags & NFT_CTX_OUTPUT_NUMERIC_PROTO;
> +}
> +
>  struct nft_cache {
>   uint16_tgenid;
>   struct list_headlist;
> diff --git a/include/nftables/libnftables.h b/include/nftables/libnftables.h
> index ff7b47aa3160..74f2dabbd1d5 100644
> --- a/include/nftables/libnftables.h
> +++ b/include/nftables/libnftables.h
> @@ -52,6 +52,7 @@ enum {
>   NFT_CTX_OUTPUT_JSON = (1 << 4),
>   NFT_CTX_OUTPUT_ECHO = (1 << 5),
>   NFT_CTX_OUTPUT_GUID = (1 << 6),
> + NFT_CTX_OUTPUT_NUMERIC_PROTO= (1 << 7),
>  };
>  
>  unsigned int nft_ctx_output_get_flags(struct nft_ctx *ctx);
> diff --git a/src/datatype.c b/src/datatype.c
> index 48eaca277757..bfb70a6ebb76 100644
> --- a/src/datatype.c
> +++ b/src/datatype.c
> @@ -564,7 +564,7 @@ static void inet_protocol_type_print(const struct expr 
> *expr,
>  {
>   struct protoent *p;
>  
> - if (octx->numeric < NFT_NUMERIC_ALL) {
> + if (!nft_output_numeric_proto(octx)) {
>   p = getprotobynumber(mpz_get_uint8(expr->value));
>   if (p != NULL) {
>   nft_print(octx, "%s", p->p_name);
> diff --git a/src/main.c b/src/main.c
> index 0c8fa1e9a108..70bde253495e 100644
> --- a/src/main.c
> +++ b/src/main.c
> @@ -40,10 +40,11 @@ enum opt_vals {
>   OPT_HANDLE_OUTPUT   = 'a',
>   OPT_ECHO= 'e',
>   OPT_GUID= 'u',
> + OPT_NUMERIC_PROTO   = 'p',
>   OPT_INVALID = '?',
>  };
>  
> -#define OPTSTRING"hvcf:iI:jvnsNaeSu"
> +#define OPTSTRING"hvcf:iI:jvnsNaeSup"
>  
>  static const struct option options[] = {
>   {
> @@ -110,6 +111,10 @@ static const struct option options[] = {
>   .val= OPT_GUID,
>   },
>   {
> + .name   = "numeric-protocol",
> + .val= OPT_NUMERIC_PROTO,
> + },
> + {
>   .name   = NULL
>   }
>  };
> @@ -135,6 +140,7 @@ static void show_help(const char *name)
>  "  -u, --guidPrint UID/GID as defined in /etc/passwd 
> and /etc/group.\n"
>  "  -NTranslate IP 

Re: [PATCH nft,v3 2/5] src: add nft_ctx_output_{get,set}_stateless() to nft_ctx_output_{get,flags}_flags

2018-10-29 Thread Pablo Neira Ayuso
On Mon, Oct 29, 2018 at 02:15:34PM +0100, Phil Sutter wrote:
> Hi,
> 
> On Mon, Oct 29, 2018 at 01:31:00PM +0100, Pablo Neira Ayuso wrote:
> [...]
> > @@ -1354,10 +1354,11 @@ json_t *objref_stmt_json(const struct stmt *stmt, 
> > struct output_ctx *octx)
> >  json_t *meter_stmt_json(const struct stmt *stmt, struct output_ctx *octx)
> >  {
> > json_t *root, *tmp;
> > +   unsigned int flags;
> >  
> > -   octx->stateless++;
> > +   nft_output_save_flags(octx, , NFT_CTX_OUTPUT_STATELESS);
> > tmp = stmt_print_json(stmt->meter.stmt, octx);
> > -   octx->stateless--;
> > +   nft_output_restore_flags(octx, flags);
> >  
> > root = json_pack("{s:o, s:o, s:i}",
> >  "key", expr_print_json(stmt->meter.key, octx),
> 
> No objections, though I don't think this is any more clear than:
> 
> |  json_t *meter_stmt_json(const struct stmt *stmt, struct output_ctx *octx)
> |  {
> | json_t *root, *tmp;
> | +   unsigned int flags = octx->flags;
> |  
> | -   octx->stateless++;
> | +   octx->flags |= NFT_CTX_OUTPUT_STATELESS;
> | tmp = stmt_print_json(stmt->meter.stmt, octx);
> | -   octx->stateless--;
> | +   octx->flags = flags;

OK, will update this before pushing this out.


[PATCH 1/2 nft,v2] src: add -p to print layer 4 protocol numerically

2018-10-29 Thread Pablo Neira Ayuso
We keep printing layer 4 protocols as literals since we do not use
/etc/protocols. Add -p option to print layer 4 protocols numerically.

Signed-off-by: Pablo Neira Ayuso 
---
v2: update documentation.
Rename to NUMERIC_PROTO.

 doc/libnftables.adoc   |  3 +++
 doc/nft.txt|  4 
 include/nftables.h |  5 +
 include/nftables/libnftables.h |  1 +
 src/datatype.c |  2 +-
 src/main.c | 11 ++-
 6 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/doc/libnftables.adoc b/doc/libnftables.adoc
index 67d9f261034c..dc3299f037df 100644
--- a/doc/libnftables.adoc
+++ b/doc/libnftables.adoc
@@ -91,6 +91,7 @@ enum {
 NFT_CTX_OUTPUT_JSON= (1 << 4),
 NFT_CTX_OUTPUT_ECHO= (1 << 5),
 NFT_CTX_OUTPUT_GUID= (1 << 6),
+NFT_CTX_OUTPUT_NUMERIC_PROTO = (1 << 7),
 };
 
 
@@ -119,6 +120,8 @@ NFT_CTX_OUTPUT_GUID::
 The *nft_ctx_output_get_flags*() function returns the output flags setting's 
value in 'ctx'.
 
 The *nft_ctx_output_set_flags*() function sets the output flags setting in 
'ctx' to the value of 'val'.
+NFT_CTX_OUTPUT_NUMERIC_PROTO::
+   Display layer 4 protocol numerically.
 
 === nft_ctx_output_get_numeric() and nft_ctx_output_set_numeric()
 These functions allow control over value representation in library output.
diff --git a/doc/nft.txt b/doc/nft.txt
index 39527c4e8e55..4ef8c5b3506f 100644
--- a/doc/nft.txt
+++ b/doc/nft.txt
@@ -56,6 +56,10 @@ For a full summary of options, run *nft --help*.
 *--guid**::
Translate numeric UID/GID to names as defined by /etc/passwd and 
/etc/group.
 
+*-p*::
+*--numeric-protocol**::
+   Display layer 4 protocol numerically.
+
 *-c*::
 *--check*::
Check commands validity without actually applying the changes.
diff --git a/include/nftables.h b/include/nftables.h
index 57203dfcce3f..8f483bf84ff6 100644
--- a/include/nftables.h
+++ b/include/nftables.h
@@ -77,6 +77,11 @@ static inline bool nft_output_guid(const struct output_ctx 
*octx)
return octx->flags & NFT_CTX_OUTPUT_GUID;
 }
 
+static inline bool nft_output_numeric_proto(const struct output_ctx *octx)
+{
+   return octx->flags & NFT_CTX_OUTPUT_NUMERIC_PROTO;
+}
+
 struct nft_cache {
uint16_tgenid;
struct list_headlist;
diff --git a/include/nftables/libnftables.h b/include/nftables/libnftables.h
index ff7b47aa3160..74f2dabbd1d5 100644
--- a/include/nftables/libnftables.h
+++ b/include/nftables/libnftables.h
@@ -52,6 +52,7 @@ enum {
NFT_CTX_OUTPUT_JSON = (1 << 4),
NFT_CTX_OUTPUT_ECHO = (1 << 5),
NFT_CTX_OUTPUT_GUID = (1 << 6),
+   NFT_CTX_OUTPUT_NUMERIC_PROTO= (1 << 7),
 };
 
 unsigned int nft_ctx_output_get_flags(struct nft_ctx *ctx);
diff --git a/src/datatype.c b/src/datatype.c
index 48eaca277757..bfb70a6ebb76 100644
--- a/src/datatype.c
+++ b/src/datatype.c
@@ -564,7 +564,7 @@ static void inet_protocol_type_print(const struct expr 
*expr,
 {
struct protoent *p;
 
-   if (octx->numeric < NFT_NUMERIC_ALL) {
+   if (!nft_output_numeric_proto(octx)) {
p = getprotobynumber(mpz_get_uint8(expr->value));
if (p != NULL) {
nft_print(octx, "%s", p->p_name);
diff --git a/src/main.c b/src/main.c
index 0c8fa1e9a108..70bde253495e 100644
--- a/src/main.c
+++ b/src/main.c
@@ -40,10 +40,11 @@ enum opt_vals {
OPT_HANDLE_OUTPUT   = 'a',
OPT_ECHO= 'e',
OPT_GUID= 'u',
+   OPT_NUMERIC_PROTO   = 'p',
OPT_INVALID = '?',
 };
 
-#define OPTSTRING  "hvcf:iI:jvnsNaeSu"
+#define OPTSTRING  "hvcf:iI:jvnsNaeSup"
 
 static const struct option options[] = {
{
@@ -110,6 +111,10 @@ static const struct option options[] = {
.val= OPT_GUID,
},
{
+   .name   = "numeric-protocol",
+   .val= OPT_NUMERIC_PROTO,
+   },
+   {
.name   = NULL
}
 };
@@ -135,6 +140,7 @@ static void show_help(const char *name)
 "  -u, --guid  Print UID/GID as defined in /etc/passwd and 
/etc/group.\n"
 "  -N  Translate IP addresses to names.\n"
 "  -S, --service   Translate ports to service names as 
described in /etc/services.\n"
+"  -p, --numeric-protocol  Print layer 4 protocols numerically.\n"
 "  -a, --handleOutput rule handle.\n"
 "  -e, --echo  Echo what has been added, inserted or 
replaced.\n"
 "  -I, --includepathAdd  to the paths searched 
for include files. Default is: %s\n"
@@ -285,6 +291,9 @@ int main(int argc, char * const *argv)
case OPT_GUID:
output_flags |= NFT_CTX_OUTPUT_GUID;
break;
+   case 

[PATCH 2/2] src: add -y to priority base chain nummerically

2018-10-29 Thread Pablo Neira Ayuso
By default base chains are printed using default hook priority
definitions. Add -y option to print them as numbers.

Signed-off-by: Pablo Neira Ayuso 
---
 doc/libnftables.adoc   |  3 +++
 doc/nft.txt|  4 
 include/nftables.h |  5 +
 include/nftables/libnftables.h |  1 +
 src/main.c | 11 ++-
 src/rule.c | 11 ++-
 6 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/doc/libnftables.adoc b/doc/libnftables.adoc
index dc3299f037df..788194396db1 100644
--- a/doc/libnftables.adoc
+++ b/doc/libnftables.adoc
@@ -92,6 +92,7 @@ enum {
 NFT_CTX_OUTPUT_ECHO= (1 << 5),
 NFT_CTX_OUTPUT_GUID= (1 << 6),
 NFT_CTX_OUTPUT_NUMERIC_PROTO = (1 << 7),
+NFT_CTX_OUTPUT_NUMERIC_PRIO = (1 << 8),
 };
 
 
@@ -122,6 +123,8 @@ The *nft_ctx_output_get_flags*() function returns the 
output flags setting's val
 The *nft_ctx_output_set_flags*() function sets the output flags setting in 
'ctx' to the value of 'val'.
 NFT_CTX_OUTPUT_NUMERIC_PROTO::
Display layer 4 protocol numerically.
+NFT_CTX_OUTPUT_NUMERIC_PRIO::
+   Display base chain priority numerically.
 
 === nft_ctx_output_get_numeric() and nft_ctx_output_set_numeric()
 These functions allow control over value representation in library output.
diff --git a/doc/nft.txt b/doc/nft.txt
index 4ef8c5b3506f..5ee06f16f642 100644
--- a/doc/nft.txt
+++ b/doc/nft.txt
@@ -60,6 +60,10 @@ For a full summary of options, run *nft --help*.
 *--numeric-protocol**::
Display layer 4 protocol numerically.
 
+*-y*::
+*--numeric-priority**::
+   Display base chain priority numerically.
+
 *-c*::
 *--check*::
Check commands validity without actually applying the changes.
diff --git a/include/nftables.h b/include/nftables.h
index 8f483bf84ff6..1b31da034254 100644
--- a/include/nftables.h
+++ b/include/nftables.h
@@ -82,6 +82,11 @@ static inline bool nft_output_numeric_proto(const struct 
output_ctx *octx)
return octx->flags & NFT_CTX_OUTPUT_NUMERIC_PROTO;
 }
 
+static inline bool nft_output_numeric_priority(const struct output_ctx *octx)
+{
+   return octx->flags & NFT_CTX_OUTPUT_NUMERIC_PRIO;
+}
+
 struct nft_cache {
uint16_tgenid;
struct list_headlist;
diff --git a/include/nftables/libnftables.h b/include/nftables/libnftables.h
index 74f2dabbd1d5..fb81edc0df07 100644
--- a/include/nftables/libnftables.h
+++ b/include/nftables/libnftables.h
@@ -53,6 +53,7 @@ enum {
NFT_CTX_OUTPUT_ECHO = (1 << 5),
NFT_CTX_OUTPUT_GUID = (1 << 6),
NFT_CTX_OUTPUT_NUMERIC_PROTO= (1 << 7),
+   NFT_CTX_OUTPUT_NUMERIC_PRIO = (1 << 8),
 };
 
 unsigned int nft_ctx_output_get_flags(struct nft_ctx *ctx);
diff --git a/src/main.c b/src/main.c
index 70bde253495e..2aec3458f956 100644
--- a/src/main.c
+++ b/src/main.c
@@ -41,10 +41,11 @@ enum opt_vals {
OPT_ECHO= 'e',
OPT_GUID= 'u',
OPT_NUMERIC_PROTO   = 'p',
+   OPT_NUMERIC_PRIO= 'y',
OPT_INVALID = '?',
 };
 
-#define OPTSTRING  "hvcf:iI:jvnsNaeSup"
+#define OPTSTRING  "hvcf:iI:jvnsNaeSupy"
 
 static const struct option options[] = {
{
@@ -115,6 +116,10 @@ static const struct option options[] = {
.val= OPT_NUMERIC_PROTO,
},
{
+   .name   = "numeric-priority",
+   .val= OPT_NUMERIC_PRIO,
+   },
+   {
.name   = NULL
}
 };
@@ -141,6 +146,7 @@ static void show_help(const char *name)
 "  -N  Translate IP addresses to names.\n"
 "  -S, --service   Translate ports to service names as 
described in /etc/services.\n"
 "  -p, --numeric-protocol  Print layer 4 protocols numerically.\n"
+"  -y, --numeric-priority  Print chain priority numerically.\n"
 "  -a, --handleOutput rule handle.\n"
 "  -e, --echo  Echo what has been added, inserted or 
replaced.\n"
 "  -I, --includepathAdd  to the paths searched 
for include files. Default is: %s\n"
@@ -294,6 +300,9 @@ int main(int argc, char * const *argv)
case OPT_NUMERIC_PROTO:
output_flags |= NFT_CTX_OUTPUT_NUMERIC_PROTO;
break;
+   case OPT_NUMERIC_PRIO:
+   output_flags |= NFT_CTX_OUTPUT_NUMERIC_PRIO;
+   break;
case OPT_INVALID:
exit(EXIT_FAILURE);
}
diff --git a/src/rule.c b/src/rule.c
index 33cbf0e2e9bb..e0082c1f5a93 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -946,7 +946,8 @@ int std_prio_lookup(const char *std_prio_name, int family, 
int hook)
return NF_IP_PRI_LAST;
 }
 
-static const char *prio2str(char *buf, size_t bufsize, int family, 

Re: [PATCH nft 4/5,v3] src: add nft_ctx_output_{get,set}_json() to nft_ctx_output_{get,set}_flags

2018-10-29 Thread Phil Sutter
On Mon, Oct 29, 2018 at 01:48:49PM +0100, Pablo Neira Ayuso wrote:
> Add NFT_CTX_OUTPUT_JSON flag and display output in json format.
> 
> Signed-off-by: Pablo Neira Ayuso 

Acked-by: Phil Sutter 


Re: [PATCH nft 4/5,v2] src: add nft_ctx_output_{get,set}_json() to nft_ctx_output_{get,set}_flags

2018-10-29 Thread Phil Sutter
Hi,

On Mon, Oct 29, 2018 at 01:43:00PM +0100, Pablo Neira Ayuso wrote:
> On Mon, Oct 29, 2018 at 01:29:32PM +0100, Phil Sutter wrote:
> > On Mon, Oct 29, 2018 at 12:33:39PM +0100, Pablo Neira Ayuso wrote:
[...]
> > > diff --git a/src/libnftables.c b/src/libnftables.c
> > > index 6dc1be3d5ef8..ff7a53d22ba4 100644
> > > --- a/src/libnftables.c
> > > +++ b/src/libnftables.c
> > > @@ -352,22 +352,6 @@ void nft_ctx_output_set_echo(struct nft_ctx *ctx, 
> > > bool val)
> > >   ctx->output.echo = val;
> > >  }
> > >  
> > > -bool nft_ctx_output_get_json(struct nft_ctx *ctx)
> > > -{
> > > -#ifdef HAVE_LIBJANSSON
> > > - return ctx->output.json;
> > > -#else
> > > - return false;
> > > -#endif
> > > -}
> > > -
> > > -void nft_ctx_output_set_json(struct nft_ctx *ctx, bool val)
> > > -{
> > > -#ifdef HAVE_LIBJANSSON
> > > - ctx->output.json = val;
> > > -#endif
> > > -}
> > > -
> > 
> > In above functions, I guarded output.json setting by whether JSON
> > support was built-in.
> 
> I'm going to do the same here but...
> 
> > [...]
> > > diff --git a/src/main.c b/src/main.c
> > > index 97b8746608a7..8ea07641734d 100644
> > > --- a/src/main.c
> > > +++ b/src/main.c
> > > @@ -271,7 +271,7 @@ int main(int argc, char * const *argv)
> > >   nft_ctx_output_set_echo(nft, true);
> > >   break;
> > >   case OPT_JSON:
> > > - nft_ctx_output_set_json(nft, true);
> > > + output_flags |= NFT_CTX_OUTPUT_JSON;
> > >   break;
> > >   case OPT_INVALID:
> > >   exit(EXIT_FAILURE);
> > 
> > Maybe we should do the same here? Otherwise if JSON wasn't enabled at
> > compile-time, calling 'nft -j' leads to no output at all. (Not sure if
> > silently falling back to standard output formatting is a better choice
> > after all. :)
> 
> ... I think failing here is json support is not built-in is the way to
> go - instead of silently ignoring it.
> 
> Would you mind send a follow up patch to change this behaviour? :-)

Will do!

Thanks, Phil



Re: [PATCH nft,v3 2/5] src: add nft_ctx_output_{get,set}_stateless() to nft_ctx_output_{get,flags}_flags

2018-10-29 Thread Phil Sutter
Hi,

On Mon, Oct 29, 2018 at 01:31:00PM +0100, Pablo Neira Ayuso wrote:
[...]
> @@ -1354,10 +1354,11 @@ json_t *objref_stmt_json(const struct stmt *stmt, 
> struct output_ctx *octx)
>  json_t *meter_stmt_json(const struct stmt *stmt, struct output_ctx *octx)
>  {
>   json_t *root, *tmp;
> + unsigned int flags;
>  
> - octx->stateless++;
> + nft_output_save_flags(octx, , NFT_CTX_OUTPUT_STATELESS);
>   tmp = stmt_print_json(stmt->meter.stmt, octx);
> - octx->stateless--;
> + nft_output_restore_flags(octx, flags);
>  
>   root = json_pack("{s:o, s:o, s:i}",
>"key", expr_print_json(stmt->meter.key, octx),

No objections, though I don't think this is any more clear than:

|  json_t *meter_stmt_json(const struct stmt *stmt, struct output_ctx *octx)
|  {
|   json_t *root, *tmp;
| + unsigned int flags = octx->flags;
|  
| - octx->stateless++;
| + octx->flags |= NFT_CTX_OUTPUT_STATELESS;
|   tmp = stmt_print_json(stmt->meter.stmt, octx);
| - octx->stateless--;
| + octx->flags = flags;

Cheers, Phil


[PATCH nft] src: add -p to print layer 4 protocol numerically

2018-10-29 Thread Pablo Neira Ayuso
We keep printing layer 4 protocols as literals since we do not use
/etc/protocols. Add -p option to print layer 4 protocols numerically.

Signed-off-by: Pablo Neira Ayuso 
---
 include/nftables.h | 5 +
 include/nftables/libnftables.h | 1 +
 src/datatype.c | 2 +-
 src/main.c | 6 +-
 4 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/include/nftables.h b/include/nftables.h
index 57203dfcce3f..4301b4147dda 100644
--- a/include/nftables.h
+++ b/include/nftables.h
@@ -77,6 +77,11 @@ static inline bool nft_output_guid(const struct output_ctx 
*octx)
return octx->flags & NFT_CTX_OUTPUT_GUID;
 }
 
+static inline bool nft_output_numeric_protocol(const struct output_ctx *octx)
+{
+   return octx->flags & NFT_CTX_OUTPUT_NUMERIC_PROTOCOL;
+}
+
 struct nft_cache {
uint16_tgenid;
struct list_headlist;
diff --git a/include/nftables/libnftables.h b/include/nftables/libnftables.h
index ff7b47aa3160..bef0c83f74c3 100644
--- a/include/nftables/libnftables.h
+++ b/include/nftables/libnftables.h
@@ -52,6 +52,7 @@ enum {
NFT_CTX_OUTPUT_JSON = (1 << 4),
NFT_CTX_OUTPUT_ECHO = (1 << 5),
NFT_CTX_OUTPUT_GUID = (1 << 6),
+   NFT_CTX_OUTPUT_NUMERIC_PROTOCOL = (1 << 7),
 };
 
 unsigned int nft_ctx_output_get_flags(struct nft_ctx *ctx);
diff --git a/src/datatype.c b/src/datatype.c
index 48eaca277757..2e957e60bb71 100644
--- a/src/datatype.c
+++ b/src/datatype.c
@@ -564,7 +564,7 @@ static void inet_protocol_type_print(const struct expr 
*expr,
 {
struct protoent *p;
 
-   if (octx->numeric < NFT_NUMERIC_ALL) {
+   if (!nft_output_numeric_protocol(octx)) {
p = getprotobynumber(mpz_get_uint8(expr->value));
if (p != NULL) {
nft_print(octx, "%s", p->p_name);
diff --git a/src/main.c b/src/main.c
index 7b253dd53f93..f30490fdce8e 100644
--- a/src/main.c
+++ b/src/main.c
@@ -40,10 +40,11 @@ enum opt_vals {
OPT_HANDLE_OUTPUT   = 'a',
OPT_ECHO= 'e',
OPT_GUID= 'u',
+   OPT_NUMERIC_PROTOCOL= 'p',
OPT_INVALID = '?',
 };
 
-#define OPTSTRING  "hvcf:iI:jvnsNaeSu"
+#define OPTSTRING  "hvcf:iI:jvnsNaeSup"
 
 static const struct option options[] = {
{
@@ -285,6 +286,9 @@ int main(int argc, char * const *argv)
case OPT_GUID:
output_flags |= NFT_CTX_OUTPUT_GUID;
break;
+   case OPT_NUMERIC_PROTOCOL:
+   output_flags |= NFT_CTX_OUTPUT_NUMERIC_PROTOCOL;
+   break;
case OPT_INVALID:
exit(EXIT_FAILURE);
}
-- 
2.11.0



[PATCH nft 4/5,v3] src: add nft_ctx_output_{get,set}_json() to nft_ctx_output_{get,set}_flags

2018-10-29 Thread Pablo Neira Ayuso
Add NFT_CTX_OUTPUT_JSON flag and display output in json format.

Signed-off-by: Pablo Neira Ayuso 
---
v3: Refer to flag, not function, in the documentation.
add missing #ifdef HAVE_LIBJANSSON in src/main.c
- Both requested by Phil.

 doc/libnftables.adoc   | 18 +-
 include/nftables.h |  6 +-
 include/nftables/libnftables.h |  3 +--
 src/libnftables.c  | 24 
 src/main.c |  4 +++-
 src/monitor.c  |  2 +-
 src/rule.c |  2 +-
 7 files changed, 20 insertions(+), 39 deletions(-)

diff --git a/doc/libnftables.adoc b/doc/libnftables.adoc
index dbd38bdd2134..b385567c2f69 100644
--- a/doc/libnftables.adoc
+++ b/doc/libnftables.adoc
@@ -31,9 +31,6 @@ void nft_ctx_output_set_debug(struct nft_ctx* '\*ctx'*, 
unsigned int* 'mask'*);
 bool nft_ctx_output_get_echo(struct nft_ctx* '\*ctx'*);
 void nft_ctx_output_set_echo(struct nft_ctx* '\*ctx'*, bool* 'val'*);
 
-bool nft_ctx_output_get_json(struct nft_ctx* '\*ctx'*);
-void nft_ctx_output_set_json(struct nft_ctx* '\*ctx'*, bool* 'val'*);
-
 FILE *nft_ctx_set_output(struct nft_ctx* '\*ctx'*, FILE* '\*fp'*);
 int nft_ctx_buffer_output(struct nft_ctx* '\*ctx'*);
 int nft_ctx_unbuffer_output(struct nft_ctx* '\*ctx'*);
@@ -94,6 +91,7 @@ enum {
 NFT_CTX_OUTPUT_SERVICE = (1 << 1),
 NFT_CTX_OUTPUT_STATELESS   = (1 << 2),
 NFT_CTX_OUTPUT_HANDLE  = (1 << 3),
+NFT_CTX_OUTPUT_JSON= (1 << 4),
 };
 
 
@@ -109,6 +107,10 @@ NFT_CTX_OUTPUT_HANDLE::
 For example, when deleting a table or chain, it may be identified either by 
name or handle.
 Rules on the other hand must be deleted by handle because there is no other 
way to uniquely identify them.
 This flag makes ruleset listings include handle values.
+NFT_CTX_OUTPUT_JSON::
+   If enabled at compile-time, libnftables accepts input in JSON format 
and is able to print output in JSON format as well.
+See *libnftables-json*(5) for a description of the supported schema.
+This flag controls JSON output format, input is auto-detected.
 
 The *nft_ctx_output_get_flags*() function returns the output flags setting's 
value in 'ctx'.
 
@@ -190,16 +192,6 @@ The *nft_ctx_output_get_echo*() function returns the echo 
output setting's value
 
 The *nft_ctx_output_set_echo*() function sets the echo output setting in 'ctx' 
to the value of 'val'.
 
-=== nft_ctx_output_get_json() and nft_ctx_output_set_json()
-If enabled at compile-time, libnftables accepts input in JSON format and is 
able to print output in JSON format as well.
-See *libnftables-json*(5) for a description of the supported schema.
-These functions control JSON output format, input is auto-detected.
-The default setting is *false*.
-
-The *nft_ctx_output_get_json*() function returns the JSON output setting's 
value in 'ctx'.
-
-The *nft_ctx_output_set_json*() function sets the JSON output setting in 'ctx' 
to the value of 'val'.
-
 === Controlling library standard and error output
 By default, any output from the library (e.g., after a *list* command) is 
written to 'stdout' and any error messages are written to 'stderr'.
 To give applications control over them, there are functions to assign custom 
file pointers as well as having the library buffer what would be written for 
later retrieval in a static buffer.
diff --git a/include/nftables.h b/include/nftables.h
index f0f123586d66..3cbfe7f3a305 100644
--- a/include/nftables.h
+++ b/include/nftables.h
@@ -19,7 +19,6 @@ struct output_ctx {
unsigned int flags;
unsigned int numeric;
unsigned int echo;
-   unsigned int json;
union {
FILE *output_fp;
struct cookie output_cookie;
@@ -64,6 +63,11 @@ static inline bool nft_output_handle(const struct output_ctx 
*octx)
return octx->flags & NFT_CTX_OUTPUT_HANDLE;
 }
 
+static inline bool nft_output_json(const struct output_ctx *octx)
+{
+   return octx->flags & NFT_CTX_OUTPUT_JSON;
+}
+
 struct nft_cache {
uint16_tgenid;
struct list_headlist;
diff --git a/include/nftables/libnftables.h b/include/nftables/libnftables.h
index a6ce938305c3..35374072560e 100644
--- a/include/nftables/libnftables.h
+++ b/include/nftables/libnftables.h
@@ -49,6 +49,7 @@ enum {
NFT_CTX_OUTPUT_SERVICE  = (1 << 1),
NFT_CTX_OUTPUT_STATELESS= (1 << 2),
NFT_CTX_OUTPUT_HANDLE   = (1 << 3),
+   NFT_CTX_OUTPUT_JSON = (1 << 4),
 };
 
 unsigned int nft_ctx_output_get_flags(struct nft_ctx *ctx);
@@ -60,8 +61,6 @@ unsigned int nft_ctx_output_get_debug(struct nft_ctx *ctx);
 void nft_ctx_output_set_debug(struct nft_ctx *ctx, unsigned int mask);
 bool nft_ctx_output_get_echo(struct nft_ctx *ctx);
 void nft_ctx_output_set_echo(struct nft_ctx *ctx, bool val);
-bool nft_ctx_output_get_json(struct nft_ctx *ctx);
-void nft_ctx_output_set_json(struct nft_ctx *ctx, 

Re: [PATCH] src: default to numeric UID and GID listing

2018-10-29 Thread Pablo Neira Ayuso
On Mon, Oct 29, 2018 at 01:34:23PM +0100, Phil Sutter wrote:
> On Mon, Oct 29, 2018 at 01:07:28PM +0100, Pablo Neira Ayuso wrote:
> > Like iptables-save, print UID and GID as numeric values by default.
> > 
> > Add a new option `-u' to print the UID and GID names as defined by
> > /etc/passwd and /etc/group.
> > 
> > Note that -n is ignored after this patch, since default are numeric
> > printing for UID and GID.
> > 
> > Signed-off-by: Pablo Neira Ayuso 
> 
> Acked-by: Phil Sutter 

Thanks, someone around here mentioned you have used _GUID in your
original patch to revisit, so I'll use that instead of _UIDGID.


Re: [PATCH nft 3/5,v2] src: add nft_ctx_output_{get,set}_handle() to nft_ctx_output_{get,set}_flags

2018-10-29 Thread Pablo Neira Ayuso
On Mon, Oct 29, 2018 at 01:23:04PM +0100, Phil Sutter wrote:
> On Mon, Oct 29, 2018 at 12:33:38PM +0100, Pablo Neira Ayuso wrote:
> > Add NFT_CTX_OUTPUT_HANDLE flag and print handle that uniquely identify
> > objects from new output flags interface.
> > 
> > Signed-off-by: Pablo Neira Ayuso 
> 
> Acked-by: Phil Sutter 
> 
> One minor nit here as well:
> 
> [...]
> > diff --git a/doc/libnftables.adoc b/doc/libnftables.adoc
> > index c837c2d251bc..8b7aee9af134 100644
> > --- a/doc/libnftables.adoc
> > +++ b/doc/libnftables.adoc
> [...]
> > @@ -105,6 +103,8 @@ NFT_CTX_OUTPUT_SERVICE::
> > Print port numbers as services as described in the /etc/services file.
> >  NFT_CTX_OUTPUT_STATELESS::
> > If stateless output has been requested then stateful data is not 
> > printed. Stateful data refers to those objects that carry run-time data, 
> > eg. the *counter* statement holds packet and byte counter values, making it 
> > stateful.
> > +NFT_CTX_OUTPUT_HANDLE::
> > +   Upon insertion into the ruleset, some elements are assigned a unique 
> > handle for identification purposes. For example, when deleting a table or 
> > chain, it may be identified either by name or handle. Rules on the other 
> > hand must be deleted by handle because there is no other way to uniquely 
> > identify them. These functions allow to control whether ruleset listings 
> > should include handles or not.
> 
> s/These functions allow/This flag allows/
> 
> (Or maybe rewrite the whole last sentence to read: "This flag makes
> ruleset listings include handle values.")

I took this sentence.

> One thing to consider given the very long line above: When writing
> documentation for man pages, I've usually adhered to 80 column max
> width. Though searching for asciidoc style guides I encountered an
> interesting alternative approach, namely to keep each sentence in a
> separate line. The rationale was to clear up diffs since no reformatting
> would happen and on the other hand to easily identify overlong sentences
> one might want to break into two. I don't have a strong opinion here,
> just wanted to share an interesting idea.

I have split long line into separed lines, thanks for suggesting this.


Re: [PATCH] src: default to numeric UID and GID listing

2018-10-29 Thread Phil Sutter
On Mon, Oct 29, 2018 at 01:07:28PM +0100, Pablo Neira Ayuso wrote:
> Like iptables-save, print UID and GID as numeric values by default.
> 
> Add a new option `-u' to print the UID and GID names as defined by
> /etc/passwd and /etc/group.
> 
> Note that -n is ignored after this patch, since default are numeric
> printing for UID and GID.
> 
> Signed-off-by: Pablo Neira Ayuso 

Acked-by: Phil Sutter 


Re: [PATCH nft 5/5,v2] src: add nft_ctx_output_{get,set}_echo() to nft_ctx_output_{get,set}_flags

2018-10-29 Thread Phil Sutter
On Mon, Oct 29, 2018 at 12:33:40PM +0100, Pablo Neira Ayuso wrote:
> Add NFT_CTX_OUTPUT_ECHO flag and echo the command that has been send to
> the kernel.
> 
> Signed-off-by: Pablo Neira Ayuso 

Acked-by: Phil Sutter 


[PATCH nft,v3 2/5] src: add nft_ctx_output_{get,set}_stateless() to nft_ctx_output_{get,flags}_flags

2018-10-29 Thread Pablo Neira Ayuso
Add NFT_CTX_OUTPUT_STATELESS flag and enable stateless printing from new
output flags interface.

This patch adds nft_output_save_flags() and nft_output_restore_flags()
to temporarily disable stateful printing

Signed-off-by: Pablo Neira Ayuso 
---
v3: Use nft_output_save_flags() and nft_output_restore_flags().
Problem reported by Phil.

 doc/libnftables.adoc   | 17 +++--
 include/nftables.h | 20 +++-
 include/nftables/libnftables.h |  3 +--
 src/expression.c   |  2 +-
 src/json.c |  9 +
 src/libnftables.c  | 10 --
 src/main.c |  2 +-
 src/rule.c |  6 +++---
 src/statement.c| 22 ++
 9 files changed, 47 insertions(+), 44 deletions(-)

diff --git a/doc/libnftables.adoc b/doc/libnftables.adoc
index 9655834f6d87..896fbf608a69 100644
--- a/doc/libnftables.adoc
+++ b/doc/libnftables.adoc
@@ -25,9 +25,6 @@ enum nft_numeric_level nft_ctx_output_get_numeric(struct 
nft_ctx* '\*ctx'*);
 void nft_ctx_output_set_numeric(struct nft_ctx* '\*ctx'*,
enum nft_numeric_level* 'level'*);
 
-bool nft_ctx_output_get_stateless(struct nft_ctx* '\*ctx'*);
-void nft_ctx_output_set_stateless(struct nft_ctx* '\*ctx'*, bool* 'val'*);
-
 unsigned int nft_ctx_output_get_debug(struct nft_ctx* '\*ctx'*);
 void nft_ctx_output_set_debug(struct nft_ctx* '\*ctx'*, unsigned int* 'mask'*);
 
@@ -98,6 +95,7 @@ The flags setting controls the output format.
 enum {
 NFT_CTX_OUTPUT_REVERSEDNS  = (1 << 0),
 NFT_CTX_OUTPUT_SERVICE = (1 << 1),
+NFT_CTX_OUTPUT_STATELESS   = (1 << 2),
 };
 
 
@@ -105,6 +103,8 @@ NFT_CTX_OUTPUT_REVERSEDNS::
Reverse DNS lookups are performed for IP addresses when printing. Note 
that this may add significant delay to *list* commands depending on DNS 
resolver speed.
 NFT_CTX_OUTPUT_SERVICE::
Print port numbers as services as described in the /etc/services file.
+NFT_CTX_OUTPUT_STATELESS::
+   If stateless output has been requested then stateful data is not 
printed. Stateful data refers to those objects that carry run-time data, eg. 
the *counter* statement holds packet and byte counter values, making it 
stateful.
 
 The *nft_ctx_output_get_flags*() function returns the output flags setting's 
value in 'ctx'.
 
@@ -141,17 +141,6 @@ The *nft_ctx_output_get_numeric*() function returns the 
numeric output setting's
 
 The *nft_ctx_output_set_numeric*() function sets the numeric output setting in 
'ctx' to the value of 'level'.
 
-=== nft_ctx_output_get_stateless() and nft_ctx_output_set_stateless()
-In nftables, there are stateful objects, i.e. ruleset elements which carry 
run-time data.
-For example the *counter* statement holds packet and byte counter values, 
making it stateful.
-If stateless output has been requested, this data is omitted when printing 
ruleset elements.
-The default setting is *false*.
-
-
-The *nft_ctx_output_get_stateless*() function returns the stateless output 
setting's value in 'ctx'.
-
-The *nft_ctx_output_set_stateless*() function sets the stateless output 
setting in 'ctx' to the value of 'val'.
-
 === nft_ctx_output_get_debug() and nft_ctx_output_set_debug()
 Libnftables supports separate debugging of different parts of its internals.
 To facilitate this, debugging output is controlled via a bit mask.
diff --git a/include/nftables.h b/include/nftables.h
index 86b44f17d6e2..46a27f19e205 100644
--- a/include/nftables.h
+++ b/include/nftables.h
@@ -18,7 +18,6 @@ struct cookie {
 struct output_ctx {
unsigned int flags;
unsigned int numeric;
-   unsigned int stateless;
unsigned int handle;
unsigned int echo;
unsigned int json;
@@ -32,6 +31,20 @@ struct output_ctx {
};
 };
 
+static inline void nft_output_save_flags(struct output_ctx *octx,
+unsigned int *saved_flags,
+unsigned int flag)
+{
+   *saved_flags = octx->flags;
+   octx->flags |= flag;
+}
+
+static inline void nft_output_restore_flags(struct output_ctx *octx,
+   unsigned int restore_flags)
+{
+   octx->flags = restore_flags;
+}
+
 static inline bool nft_output_reversedns(const struct output_ctx *octx)
 {
return octx->flags & NFT_CTX_OUTPUT_REVERSEDNS;
@@ -42,6 +55,11 @@ static inline bool nft_output_service(const struct 
output_ctx *octx)
return octx->flags & NFT_CTX_OUTPUT_SERVICE;
 }
 
+static inline bool nft_output_stateless(const struct output_ctx *octx)
+{
+   return octx->flags & NFT_CTX_OUTPUT_STATELESS;
+}
+
 struct nft_cache {
uint16_tgenid;
struct list_headlist;
diff --git a/include/nftables/libnftables.h b/include/nftables/libnftables.h
index 321441b03ca8..4f1c10901b1b 100644
--- a/include/nftables/libnftables.h
+++ 

Re: [PATCH nft 4/5,v2] src: add nft_ctx_output_{get,set}_json() to nft_ctx_output_{get,set}_flags

2018-10-29 Thread Phil Sutter
Hi,

On Mon, Oct 29, 2018 at 12:33:39PM +0100, Pablo Neira Ayuso wrote:
> Add NFT_CTX_OUTPUT_JSON flag and display output in json format.
> 
> Signed-off-by: Pablo Neira Ayuso 
[...]
> diff --git a/doc/libnftables.adoc b/doc/libnftables.adoc
> index 8b7aee9af134..5a3562c3266c 100644
> --- a/doc/libnftables.adoc
> +++ b/doc/libnftables.adoc
[...]
> @@ -105,6 +103,8 @@ NFT_CTX_OUTPUT_STATELESS::
>   If stateless output has been requested then stateful data is not 
> printed. Stateful data refers to those objects that carry run-time data, eg. 
> the *counter* statement holds packet and byte counter values, making it 
> stateful.
>  NFT_CTX_OUTPUT_HANDLE::
>   Upon insertion into the ruleset, some elements are assigned a unique 
> handle for identification purposes. For example, when deleting a table or 
> chain, it may be identified either by name or handle. Rules on the other hand 
> must be deleted by handle because there is no other way to uniquely identify 
> them. These functions allow to control whether ruleset listings should 
> include handles or not.
> +NFT_CTX_OUTPUT_JSON::
> + If enabled at compile-time, libnftables accepts input in JSON format 
> and is able to print output in JSON format as well. See *libnftables-json*(5) 
> for a description of the supported schema. These functions control JSON 
> output format, input is auto-detected.

s/These functions control/This flag controls/

[...]
> diff --git a/src/libnftables.c b/src/libnftables.c
> index 6dc1be3d5ef8..ff7a53d22ba4 100644
> --- a/src/libnftables.c
> +++ b/src/libnftables.c
> @@ -352,22 +352,6 @@ void nft_ctx_output_set_echo(struct nft_ctx *ctx, bool 
> val)
>   ctx->output.echo = val;
>  }
>  
> -bool nft_ctx_output_get_json(struct nft_ctx *ctx)
> -{
> -#ifdef HAVE_LIBJANSSON
> - return ctx->output.json;
> -#else
> - return false;
> -#endif
> -}
> -
> -void nft_ctx_output_set_json(struct nft_ctx *ctx, bool val)
> -{
> -#ifdef HAVE_LIBJANSSON
> - ctx->output.json = val;
> -#endif
> -}
> -

In above functions, I guarded output.json setting by whether JSON
support was built-in.

[...]
> diff --git a/src/main.c b/src/main.c
> index 97b8746608a7..8ea07641734d 100644
> --- a/src/main.c
> +++ b/src/main.c
> @@ -271,7 +271,7 @@ int main(int argc, char * const *argv)
>   nft_ctx_output_set_echo(nft, true);
>   break;
>   case OPT_JSON:
> - nft_ctx_output_set_json(nft, true);
> + output_flags |= NFT_CTX_OUTPUT_JSON;
>   break;
>   case OPT_INVALID:
>   exit(EXIT_FAILURE);

Maybe we should do the same here? Otherwise if JSON wasn't enabled at
compile-time, calling 'nft -j' leads to no output at all. (Not sure if
silently falling back to standard output formatting is a better choice
after all. :)

Thanks, Phil


Re: [PATCH nft 3/5,v2] src: add nft_ctx_output_{get,set}_handle() to nft_ctx_output_{get,set}_flags

2018-10-29 Thread Phil Sutter
On Mon, Oct 29, 2018 at 12:33:38PM +0100, Pablo Neira Ayuso wrote:
> Add NFT_CTX_OUTPUT_HANDLE flag and print handle that uniquely identify
> objects from new output flags interface.
> 
> Signed-off-by: Pablo Neira Ayuso 

Acked-by: Phil Sutter 

One minor nit here as well:

[...]
> diff --git a/doc/libnftables.adoc b/doc/libnftables.adoc
> index c837c2d251bc..8b7aee9af134 100644
> --- a/doc/libnftables.adoc
> +++ b/doc/libnftables.adoc
[...]
> @@ -105,6 +103,8 @@ NFT_CTX_OUTPUT_SERVICE::
>   Print port numbers as services as described in the /etc/services file.
>  NFT_CTX_OUTPUT_STATELESS::
>   If stateless output has been requested then stateful data is not 
> printed. Stateful data refers to those objects that carry run-time data, eg. 
> the *counter* statement holds packet and byte counter values, making it 
> stateful.
> +NFT_CTX_OUTPUT_HANDLE::
> + Upon insertion into the ruleset, some elements are assigned a unique 
> handle for identification purposes. For example, when deleting a table or 
> chain, it may be identified either by name or handle. Rules on the other hand 
> must be deleted by handle because there is no other way to uniquely identify 
> them. These functions allow to control whether ruleset listings should 
> include handles or not.

s/These functions allow/This flag allows/

(Or maybe rewrite the whole last sentence to read: "This flag makes
ruleset listings include handle values.")

One thing to consider given the very long line above: When writing
documentation for man pages, I've usually adhered to 80 column max
width. Though searching for asciidoc style guides I encountered an
interesting alternative approach, namely to keep each sentence in a
separate line. The rationale was to clear up diffs since no reformatting
would happen and on the other hand to easily identify overlong sentences
one might want to break into two. I don't have a strong opinion here,
just wanted to share an interesting idea.

Thanks, Phil


Re: [PATCH nft 2/5,v2] src: add nft_ctx_output_{get,set}_stateless() to nft_ctx_output_{get,flags}_flags

2018-10-29 Thread Pablo Neira Ayuso
On Mon, Oct 29, 2018 at 01:09:31PM +0100, Phil Sutter wrote:
> Hi,
> 
> On Mon, Oct 29, 2018 at 12:33:37PM +0100, Pablo Neira Ayuso wrote:
> > Add NFT_CTX_OUTPUT_STATELESS flag and enable stateless printing from new
> > output flags interface.
> > 
> > Signed-off-by: Pablo Neira Ayuso 
> > ---
> > v2: Add nft_output_stateless()
> > Fix missing conversion to use NFT_CTX_OUTPUT_STATELESS.
> > Remove stateless field from struct output_ctx.
> 
> [...]
> 
> > diff --git a/src/statement.c b/src/statement.c
> > index e50ac706402d..162922108020 100644
> > --- a/src/statement.c
> > +++ b/src/statement.c
> > @@ -121,9 +121,9 @@ static void meter_stmt_print(const struct stmt *stmt, 
> > struct output_ctx *octx)
> > expr_print(stmt->meter.key, octx);
> > nft_print(octx, " ");
> >  
> > -   octx->stateless++;
> > +   octx->flags |= NFT_CTX_OUTPUT_STATELESS;
> > stmt_print(stmt->meter.stmt, octx);
> > -   octx->stateless--;
> > +   octx->flags &= ~NFT_CTX_OUTPUT_STATELESS;
> >  
> > nft_print(octx, "} ");
> >  
> 
> Are you sure this is safe? If meter_stmt_print() is called with
> stateless output enabled, it will be disabled when the function returns.
> I guess this should backup octx->flags and restore the old value before
> returning to caller.
> 
> Same goes for other places were we do 'stateless++; something();
> stateless--'.

Right, I need to annotate old flags.


Re: [PATCH nft 2/5,v2] src: add nft_ctx_output_{get,set}_stateless() to nft_ctx_output_{get,flags}_flags

2018-10-29 Thread Phil Sutter
Hi,

On Mon, Oct 29, 2018 at 12:33:37PM +0100, Pablo Neira Ayuso wrote:
> Add NFT_CTX_OUTPUT_STATELESS flag and enable stateless printing from new
> output flags interface.
> 
> Signed-off-by: Pablo Neira Ayuso 
> ---
> v2: Add nft_output_stateless()
> Fix missing conversion to use NFT_CTX_OUTPUT_STATELESS.
> Remove stateless field from struct output_ctx.

[...]

> diff --git a/src/statement.c b/src/statement.c
> index e50ac706402d..162922108020 100644
> --- a/src/statement.c
> +++ b/src/statement.c
> @@ -121,9 +121,9 @@ static void meter_stmt_print(const struct stmt *stmt, 
> struct output_ctx *octx)
>   expr_print(stmt->meter.key, octx);
>   nft_print(octx, " ");
>  
> - octx->stateless++;
> + octx->flags |= NFT_CTX_OUTPUT_STATELESS;
>   stmt_print(stmt->meter.stmt, octx);
> - octx->stateless--;
> + octx->flags &= ~NFT_CTX_OUTPUT_STATELESS;
>  
>   nft_print(octx, "} ");
>  

Are you sure this is safe? If meter_stmt_print() is called with
stateless output enabled, it will be disabled when the function returns.
I guess this should backup octx->flags and restore the old value before
returning to caller.

Same goes for other places were we do 'stateless++; something();
stateless--'.

Thanks, Phil


[PATCH] src: default to numeric UID and GID listing

2018-10-29 Thread Pablo Neira Ayuso
Like iptables-save, print UID and GID as numeric values by default.

Add a new option `-u' to print the UID and GID names as defined by
/etc/passwd and /etc/group.

Note that -n is ignored after this patch, since default are numeric
printing for UID and GID.

Signed-off-by: Pablo Neira Ayuso 
---
 doc/libnftables.adoc   |  3 +++
 include/nftables.h |  5 +
 include/nftables/libnftables.h |  1 +
 src/json.c |  4 ++--
 src/main.c | 11 ++-
 src/meta.c |  4 ++--
 6 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/doc/libnftables.adoc b/doc/libnftables.adoc
index 28a43abb4a4e..5b2605f4c02d 100644
--- a/doc/libnftables.adoc
+++ b/doc/libnftables.adoc
@@ -90,6 +90,7 @@ enum {
 NFT_CTX_OUTPUT_HANDLE  = (1 << 3),
 NFT_CTX_OUTPUT_JSON= (1 << 4),
 NFT_CTX_OUTPUT_ECHO= (1 << 5),
+NFT_CTX_OUTPUT_UIDGID  = (1 << 6),
 };
 
 
@@ -105,6 +106,8 @@ NFT_CTX_OUTPUT_JSON::
If enabled at compile-time, libnftables accepts input in JSON format 
and is able to print output in JSON format as well. See *libnftables-json*(5) 
for a description of the supported schema. These functions control JSON output 
format, input is auto-detected.
 NFT_CTX_OUTPUT_ECHO::
The echo setting makes libnftables print the changes once they are 
committed to the kernel, just like a running instance of *nft monitor* would. 
Amongst other things, this allows to retrieve an added rule's handle atomically.
+NFT_CTX_OUTPUT_UIDGID::
+   Display UID and GID as described in the /etc/passwd and /etc/group 
files.
 
 The *nft_ctx_output_get_flags*() function returns the output flags setting's 
value in 'ctx'.
 
diff --git a/include/nftables.h b/include/nftables.h
index fa6665a17a7e..f766818d2629 100644
--- a/include/nftables.h
+++ b/include/nftables.h
@@ -58,6 +58,11 @@ static inline bool nft_output_echo(const struct output_ctx 
*octx)
return octx->flags & NFT_CTX_OUTPUT_ECHO;
 }
 
+static inline bool nft_output_uidgid(const struct output_ctx *octx)
+{
+   return octx->flags & NFT_CTX_OUTPUT_UIDGID;
+}
+
 struct nft_cache {
uint16_tgenid;
struct list_headlist;
diff --git a/include/nftables/libnftables.h b/include/nftables/libnftables.h
index 4777240883f0..1b2dd19f3059 100644
--- a/include/nftables/libnftables.h
+++ b/include/nftables/libnftables.h
@@ -51,6 +51,7 @@ enum {
NFT_CTX_OUTPUT_HANDLE   = (1 << 3),
NFT_CTX_OUTPUT_JSON = (1 << 4),
NFT_CTX_OUTPUT_ECHO = (1 << 5),
+   NFT_CTX_OUTPUT_UIDGID   = (1 << 6),
 };
 
 unsigned int nft_ctx_output_get_flags(struct nft_ctx *ctx);
diff --git a/src/json.c b/src/json.c
index 5edd3a0851e4..cf6707917d02 100644
--- a/src/json.c
+++ b/src/json.c
@@ -1020,7 +1020,7 @@ json_t *uid_type_json(const struct expr *expr, struct 
output_ctx *octx)
 {
uint32_t uid = mpz_get_uint32(expr->value);
 
-   if (octx->numeric < NFT_NUMERIC_ALL) {
+   if (nft_output_uidgid(octx)) {
struct passwd *pw = getpwuid(uid);
 
if (pw)
@@ -1033,7 +1033,7 @@ json_t *gid_type_json(const struct expr *expr, struct 
output_ctx *octx)
 {
uint32_t gid = mpz_get_uint32(expr->value);
 
-   if (octx->numeric < NFT_NUMERIC_ALL) {
+   if (nft_output_uidgid(octx)) {
struct group *gr = getgrgid(gid);
 
if (gr)
diff --git a/src/main.c b/src/main.c
index eee3abc46161..155a63673b82 100644
--- a/src/main.c
+++ b/src/main.c
@@ -39,10 +39,11 @@ enum opt_vals {
OPT_DEBUG   = 'd',
OPT_HANDLE_OUTPUT   = 'a',
OPT_ECHO= 'e',
+   OPT_UIDGID  = 'u',
OPT_INVALID = '?',
 };
 
-#define OPTSTRING  "hvcf:iI:jvnsNaeS"
+#define OPTSTRING  "hvcf:iI:jvnsNaeSu"
 
 static const struct option options[] = {
{
@@ -105,6 +106,10 @@ static const struct option options[] = {
.val= OPT_JSON,
},
{
+   .name   = "uidgid",
+   .val= OPT_UIDGID,
+   },
+   {
.name   = NULL
}
 };
@@ -127,6 +132,7 @@ static void show_help(const char *name)
 "  Specify twice to also show Internet services 
(port numbers) numerically.\n"
 "  Specify three times to also show protocols, 
user IDs, and group IDs numerically.\n"
 "  -s, --stateless Omit stateful information of ruleset.\n"
+"  -u, --uidgidPrint UID/GID as defined in /etc/passwd 
and /etc/group.\n"
 "  -N  Translate IP addresses to names.\n"
 "  -S, --serviceTranslate ports to service names as described 
in /etc/services.\n"
 "  -a, --handleOutput rule handle.\n"
@@ -274,6 +280,9 @@ int 

Re: [PATCH nft 1/5,v4] src: Revert --literal, add -S/--service

2018-10-29 Thread Phil Sutter
Hi,

On Mon, Oct 29, 2018 at 12:33:36PM +0100, Pablo Neira Ayuso wrote:
> This is a partial revert of b0f6a45b25dd1 ("src: add --literal option")
> which was added during the development cycle before 0.9.1 is released.
> 
> After looking at patch: https://patchwork.ozlabs.org/patch/969864/ that
> allows to print priority, uid, gid and protocols as numerics, I decided
> to revisit this to provide individual options to turn on literal
> printing.
> 
> What I'm proposing is to provide a good default for everyone, and
> provide options to turn on literal/numeric printing.
> 
> This patch adds nft_ctx_output_{set,get}_flags() and define two flags to
> enable reverse DNS lookups and to print ports as service names.
> 
> This patch introduces -S/--services, to print service names as per
> /etc/services.
> 
> Signed-off-by: Pablo Neira Ayuso 

Acked-by: Phil Sutter 

Just one minor nit:

[...]
> diff --git a/doc/libnftables.adoc b/doc/libnftables.adoc
> index 0387652fa3c1..1c6ea0152d13 100644
> --- a/doc/libnftables.adoc
> +++ b/doc/libnftables.adoc
[...]
> @@ -91,6 +91,25 @@ The *nft_ctx_get_dry_run*() function returns the dry-run 
> setting's value contain
>  
>  The *nft_ctx_set_dry_run*() function sets the dry-run setting in 'ctx' to 
> the value of 'dry'.
>  
> +=== nft_ctx_output_get_flags() and nft_ctx_output_set_flags()
> +The flags setting controls the output format.
> +
> +
> +enum {
> +NFT_CTX_OUTPUT_REVERSEDNS  = (1 << 0),
> +NFT_CTX_OUTPUT_SERVICE = (1 << 1),
> +};
> +
> +
> +NFT_CTX_OUTPUT_REVERSEDNS::
> + Perform reverse DNS lookups are performed for IP addresses when 
> printing. Note that this may add significant delay to *list* commands 
> depending on DNS resolver speed.

s/Perform reverse/Reverse/

Thanks, Phil


[PATCH nft 4/5,v2] src: add nft_ctx_output_{get,set}_json() to nft_ctx_output_{get,set}_flags

2018-10-29 Thread Pablo Neira Ayuso
Add NFT_CTX_OUTPUT_JSON flag and display output in json format.

Signed-off-by: Pablo Neira Ayuso 
---
v2: Add nft_output_json()
Fix missing conversion to use NFT_CTX_OUTPUT_JSON.
Remove json field from struct output_ctx.

 doc/libnftables.adoc   | 16 +++-
 include/nftables.h |  6 +-
 include/nftables/libnftables.h |  3 +--
 src/libnftables.c  | 24 
 src/main.c |  2 +-
 src/monitor.c  |  2 +-
 src/rule.c |  2 +-
 7 files changed, 16 insertions(+), 39 deletions(-)

diff --git a/doc/libnftables.adoc b/doc/libnftables.adoc
index 8b7aee9af134..5a3562c3266c 100644
--- a/doc/libnftables.adoc
+++ b/doc/libnftables.adoc
@@ -31,9 +31,6 @@ void nft_ctx_output_set_debug(struct nft_ctx* '\*ctx'*, 
unsigned int* 'mask'*);
 bool nft_ctx_output_get_echo(struct nft_ctx* '\*ctx'*);
 void nft_ctx_output_set_echo(struct nft_ctx* '\*ctx'*, bool* 'val'*);
 
-bool nft_ctx_output_get_json(struct nft_ctx* '\*ctx'*);
-void nft_ctx_output_set_json(struct nft_ctx* '\*ctx'*, bool* 'val'*);
-
 FILE *nft_ctx_set_output(struct nft_ctx* '\*ctx'*, FILE* '\*fp'*);
 int nft_ctx_buffer_output(struct nft_ctx* '\*ctx'*);
 int nft_ctx_unbuffer_output(struct nft_ctx* '\*ctx'*);
@@ -94,6 +91,7 @@ enum {
 NFT_CTX_OUTPUT_SERVICE = (1 << 1),
 NFT_CTX_OUTPUT_STATELESS   = (1 << 2),
 NFT_CTX_OUTPUT_HANDLE  = (1 << 3),
+NFT_CTX_OUTPUT_JSON= (1 << 4),
 };
 
 
@@ -105,6 +103,8 @@ NFT_CTX_OUTPUT_STATELESS::
If stateless output has been requested then stateful data is not 
printed. Stateful data refers to those objects that carry run-time data, eg. 
the *counter* statement holds packet and byte counter values, making it 
stateful.
 NFT_CTX_OUTPUT_HANDLE::
Upon insertion into the ruleset, some elements are assigned a unique 
handle for identification purposes. For example, when deleting a table or 
chain, it may be identified either by name or handle. Rules on the other hand 
must be deleted by handle because there is no other way to uniquely identify 
them. These functions allow to control whether ruleset listings should include 
handles or not.
+NFT_CTX_OUTPUT_JSON::
+   If enabled at compile-time, libnftables accepts input in JSON format 
and is able to print output in JSON format as well. See *libnftables-json*(5) 
for a description of the supported schema. These functions control JSON output 
format, input is auto-detected.
 
 The *nft_ctx_output_get_flags*() function returns the output flags setting's 
value in 'ctx'.
 
@@ -186,16 +186,6 @@ The *nft_ctx_output_get_echo*() function returns the echo 
output setting's value
 
 The *nft_ctx_output_set_echo*() function sets the echo output setting in 'ctx' 
to the value of 'val'.
 
-=== nft_ctx_output_get_json() and nft_ctx_output_set_json()
-If enabled at compile-time, libnftables accepts input in JSON format and is 
able to print output in JSON format as well.
-See *libnftables-json*(5) for a description of the supported schema.
-These functions control JSON output format, input is auto-detected.
-The default setting is *false*.
-
-The *nft_ctx_output_get_json*() function returns the JSON output setting's 
value in 'ctx'.
-
-The *nft_ctx_output_set_json*() function sets the JSON output setting in 'ctx' 
to the value of 'val'.
-
 === Controlling library standard and error output
 By default, any output from the library (e.g., after a *list* command) is 
written to 'stdout' and any error messages are written to 'stderr'.
 To give applications control over them, there are functions to assign custom 
file pointers as well as having the library buffer what would be written for 
later retrieval in a static buffer.
diff --git a/include/nftables.h b/include/nftables.h
index e0e7a1135406..86788a43dd71 100644
--- a/include/nftables.h
+++ b/include/nftables.h
@@ -19,7 +19,6 @@ struct output_ctx {
unsigned int flags;
unsigned int numeric;
unsigned int echo;
-   unsigned int json;
union {
FILE *output_fp;
struct cookie output_cookie;
@@ -50,6 +49,11 @@ static inline bool nft_output_handle(const struct output_ctx 
*octx)
return octx->flags & NFT_CTX_OUTPUT_HANDLE;
 }
 
+static inline bool nft_output_json(const struct output_ctx *octx)
+{
+   return octx->flags & NFT_CTX_OUTPUT_JSON;
+}
+
 struct nft_cache {
uint16_tgenid;
struct list_headlist;
diff --git a/include/nftables/libnftables.h b/include/nftables/libnftables.h
index a6ce938305c3..35374072560e 100644
--- a/include/nftables/libnftables.h
+++ b/include/nftables/libnftables.h
@@ -49,6 +49,7 @@ enum {
NFT_CTX_OUTPUT_SERVICE  = (1 << 1),
NFT_CTX_OUTPUT_STATELESS= (1 << 2),
NFT_CTX_OUTPUT_HANDLE   = (1 << 3),
+   NFT_CTX_OUTPUT_JSON = (1 << 4),
 };
 
 unsigned int 

[PATCH nft 5/5,v2] src: add nft_ctx_output_{get,set}_echo() to nft_ctx_output_{get,set}_flags

2018-10-29 Thread Pablo Neira Ayuso
Add NFT_CTX_OUTPUT_ECHO flag and echo the command that has been send to
the kernel.

Signed-off-by: Pablo Neira Ayuso 
---
v2: Add nft_output_echo()
Fix missing conversion to use NFT_CTX_OUTPUT_ECHO.
Remove echo field from struct output_ctx.

 doc/libnftables.adoc   | 15 +++
 include/nftables.h |  6 +-
 include/nftables/libnftables.h |  1 +
 src/libnftables.c  | 18 ++
 src/main.c |  2 +-
 src/mnl.c  |  2 +-
 src/monitor.c  |  2 +-
 src/parser_json.c  |  4 ++--
 src/rule.c |  4 ++--
 9 files changed, 22 insertions(+), 32 deletions(-)

diff --git a/doc/libnftables.adoc b/doc/libnftables.adoc
index 5a3562c3266c..28a43abb4a4e 100644
--- a/doc/libnftables.adoc
+++ b/doc/libnftables.adoc
@@ -28,9 +28,6 @@ void nft_ctx_output_set_numeric(struct nft_ctx* '\*ctx'*,
 unsigned int nft_ctx_output_get_debug(struct nft_ctx* '\*ctx'*);
 void nft_ctx_output_set_debug(struct nft_ctx* '\*ctx'*, unsigned int* 'mask'*);
 
-bool nft_ctx_output_get_echo(struct nft_ctx* '\*ctx'*);
-void nft_ctx_output_set_echo(struct nft_ctx* '\*ctx'*, bool* 'val'*);
-
 FILE *nft_ctx_set_output(struct nft_ctx* '\*ctx'*, FILE* '\*fp'*);
 int nft_ctx_buffer_output(struct nft_ctx* '\*ctx'*);
 int nft_ctx_unbuffer_output(struct nft_ctx* '\*ctx'*);
@@ -92,6 +89,7 @@ enum {
 NFT_CTX_OUTPUT_STATELESS   = (1 << 2),
 NFT_CTX_OUTPUT_HANDLE  = (1 << 3),
 NFT_CTX_OUTPUT_JSON= (1 << 4),
+NFT_CTX_OUTPUT_ECHO= (1 << 5),
 };
 
 
@@ -105,6 +103,8 @@ NFT_CTX_OUTPUT_HANDLE::
Upon insertion into the ruleset, some elements are assigned a unique 
handle for identification purposes. For example, when deleting a table or 
chain, it may be identified either by name or handle. Rules on the other hand 
must be deleted by handle because there is no other way to uniquely identify 
them. These functions allow to control whether ruleset listings should include 
handles or not.
 NFT_CTX_OUTPUT_JSON::
If enabled at compile-time, libnftables accepts input in JSON format 
and is able to print output in JSON format as well. See *libnftables-json*(5) 
for a description of the supported schema. These functions control JSON output 
format, input is auto-detected.
+NFT_CTX_OUTPUT_ECHO::
+   The echo setting makes libnftables print the changes once they are 
committed to the kernel, just like a running instance of *nft monitor* would. 
Amongst other things, this allows to retrieve an added rule's handle atomically.
 
 The *nft_ctx_output_get_flags*() function returns the output flags setting's 
value in 'ctx'.
 
@@ -177,15 +177,6 @@ The *nft_ctx_output_get_debug*() function returns the 
debug output setting's val
 
 The *nft_ctx_output_set_debug*() function sets the debug output setting in 
'ctx' to the value of 'mask'.
 
-=== nft_ctx_output_get_echo() and nft_ctx_output_set_echo()
-The echo setting makes libnftables print the changes once they are committed 
to the kernel, just like a running instance of *nft monitor* would.
-Amongst other things, this allows to retrieve an added rule's handle 
atomically.
-The default setting is *false*.
-
-The *nft_ctx_output_get_echo*() function returns the echo output setting's 
value in 'ctx'.
-
-The *nft_ctx_output_set_echo*() function sets the echo output setting in 'ctx' 
to the value of 'val'.
-
 === Controlling library standard and error output
 By default, any output from the library (e.g., after a *list* command) is 
written to 'stdout' and any error messages are written to 'stderr'.
 To give applications control over them, there are functions to assign custom 
file pointers as well as having the library buffer what would be written for 
later retrieval in a static buffer.
diff --git a/include/nftables.h b/include/nftables.h
index 86788a43dd71..fa6665a17a7e 100644
--- a/include/nftables.h
+++ b/include/nftables.h
@@ -18,7 +18,6 @@ struct cookie {
 struct output_ctx {
unsigned int flags;
unsigned int numeric;
-   unsigned int echo;
union {
FILE *output_fp;
struct cookie output_cookie;
@@ -54,6 +53,11 @@ static inline bool nft_output_json(const struct output_ctx 
*octx)
return octx->flags & NFT_CTX_OUTPUT_JSON;
 }
 
+static inline bool nft_output_echo(const struct output_ctx *octx)
+{
+   return octx->flags & NFT_CTX_OUTPUT_ECHO;
+}
+
 struct nft_cache {
uint16_tgenid;
struct list_headlist;
diff --git a/include/nftables/libnftables.h b/include/nftables/libnftables.h
index 35374072560e..5ee50c689374 100644
--- a/include/nftables/libnftables.h
+++ b/include/nftables/libnftables.h
@@ -50,6 +50,7 @@ enum {
NFT_CTX_OUTPUT_STATELESS= (1 << 2),
NFT_CTX_OUTPUT_HANDLE   = (1 << 3),
NFT_CTX_OUTPUT_JSON = (1 << 4),
+   NFT_CTX_OUTPUT_ECHO = (1 

[PATCH nft 2/5,v2] src: add nft_ctx_output_{get,set}_stateless() to nft_ctx_output_{get,flags}_flags

2018-10-29 Thread Pablo Neira Ayuso
Add NFT_CTX_OUTPUT_STATELESS flag and enable stateless printing from new
output flags interface.

Signed-off-by: Pablo Neira Ayuso 
---
v2: Add nft_output_stateless()
Fix missing conversion to use NFT_CTX_OUTPUT_STATELESS.
Remove stateless field from struct output_ctx.

 doc/libnftables.adoc   | 17 +++--
 include/nftables.h |  6 +-
 include/nftables/libnftables.h |  3 +--
 src/expression.c   |  2 +-
 src/json.c |  8 
 src/libnftables.c  | 10 --
 src/main.c |  2 +-
 src/rule.c |  6 +++---
 src/statement.c| 16 
 9 files changed, 26 insertions(+), 44 deletions(-)

diff --git a/doc/libnftables.adoc b/doc/libnftables.adoc
index 1c6ea0152d13..c837c2d251bc 100644
--- a/doc/libnftables.adoc
+++ b/doc/libnftables.adoc
@@ -25,9 +25,6 @@ enum nft_numeric_level nft_ctx_output_get_numeric(struct 
nft_ctx* '\*ctx'*);
 void nft_ctx_output_set_numeric(struct nft_ctx* '\*ctx'*,
enum nft_numeric_level* 'level'*);
 
-bool nft_ctx_output_get_stateless(struct nft_ctx* '\*ctx'*);
-void nft_ctx_output_set_stateless(struct nft_ctx* '\*ctx'*, bool* 'val'*);
-
 unsigned int nft_ctx_output_get_debug(struct nft_ctx* '\*ctx'*);
 void nft_ctx_output_set_debug(struct nft_ctx* '\*ctx'*, unsigned int* 'mask'*);
 
@@ -98,6 +95,7 @@ The flags setting controls the output format.
 enum {
 NFT_CTX_OUTPUT_REVERSEDNS  = (1 << 0),
 NFT_CTX_OUTPUT_SERVICE = (1 << 1),
+NFT_CTX_OUTPUT_STATELESS   = (1 << 2),
 };
 
 
@@ -105,6 +103,8 @@ NFT_CTX_OUTPUT_REVERSEDNS::
Perform reverse DNS lookups are performed for IP addresses when 
printing. Note that this may add significant delay to *list* commands depending 
on DNS resolver speed.
 NFT_CTX_OUTPUT_SERVICE::
Print port numbers as services as described in the /etc/services file.
+NFT_CTX_OUTPUT_STATELESS::
+   If stateless output has been requested then stateful data is not 
printed. Stateful data refers to those objects that carry run-time data, eg. 
the *counter* statement holds packet and byte counter values, making it 
stateful.
 
 The *nft_ctx_output_get_flags*() function returns the output flags setting's 
value in 'ctx'.
 
@@ -141,17 +141,6 @@ The *nft_ctx_output_get_numeric*() function returns the 
numeric output setting's
 
 The *nft_ctx_output_set_numeric*() function sets the numeric output setting in 
'ctx' to the value of 'level'.
 
-=== nft_ctx_output_get_stateless() and nft_ctx_output_set_stateless()
-In nftables, there are stateful objects, i.e. ruleset elements which carry 
run-time data.
-For example the *counter* statement holds packet and byte counter values, 
making it stateful.
-If stateless output has been requested, this data is omitted when printing 
ruleset elements.
-The default setting is *false*.
-
-
-The *nft_ctx_output_get_stateless*() function returns the stateless output 
setting's value in 'ctx'.
-
-The *nft_ctx_output_set_stateless*() function sets the stateless output 
setting in 'ctx' to the value of 'val'.
-
 === nft_ctx_output_get_debug() and nft_ctx_output_set_debug()
 Libnftables supports separate debugging of different parts of its internals.
 To facilitate this, debugging output is controlled via a bit mask.
diff --git a/include/nftables.h b/include/nftables.h
index 86b44f17d6e2..cb36e06633e9 100644
--- a/include/nftables.h
+++ b/include/nftables.h
@@ -18,7 +18,6 @@ struct cookie {
 struct output_ctx {
unsigned int flags;
unsigned int numeric;
-   unsigned int stateless;
unsigned int handle;
unsigned int echo;
unsigned int json;
@@ -42,6 +41,11 @@ static inline bool nft_output_service(const struct 
output_ctx *octx)
return octx->flags & NFT_CTX_OUTPUT_SERVICE;
 }
 
+static inline bool nft_output_stateless(const struct output_ctx *octx)
+{
+   return octx->flags & NFT_CTX_OUTPUT_STATELESS;
+}
+
 struct nft_cache {
uint16_tgenid;
struct list_headlist;
diff --git a/include/nftables/libnftables.h b/include/nftables/libnftables.h
index 321441b03ca8..4f1c10901b1b 100644
--- a/include/nftables/libnftables.h
+++ b/include/nftables/libnftables.h
@@ -47,6 +47,7 @@ void nft_ctx_set_dry_run(struct nft_ctx *ctx, bool dry);
 enum {
NFT_CTX_OUTPUT_REVERSEDNS   = (1 << 0),
NFT_CTX_OUTPUT_SERVICE  = (1 << 1),
+   NFT_CTX_OUTPUT_STATELESS= (1 << 2),
 };
 
 unsigned int nft_ctx_output_get_flags(struct nft_ctx *ctx);
@@ -54,8 +55,6 @@ void nft_ctx_output_set_flags(struct nft_ctx *ctx, unsigned 
int flags);
 
 enum nft_numeric_level nft_ctx_output_get_numeric(struct nft_ctx *ctx);
 void nft_ctx_output_set_numeric(struct nft_ctx *ctx, enum nft_numeric_level 
level);
-bool nft_ctx_output_get_stateless(struct nft_ctx *ctx);
-void nft_ctx_output_set_stateless(struct nft_ctx *ctx, bool val);
 

[PATCH nft 3/5,v2] src: add nft_ctx_output_{get,set}_handle() to nft_ctx_output_{get,set}_flags

2018-10-29 Thread Pablo Neira Ayuso
Add NFT_CTX_OUTPUT_HANDLE flag and print handle that uniquely identify
objects from new output flags interface.

Signed-off-by: Pablo Neira Ayuso 
---
v2: Add nft_output_handle()
Fix missing conversion to use NFT_CTX_OUTPUT_HANDLE.
Remove handle field from struct output_ctx.

 doc/libnftables.adoc   | 17 +++--
 include/nftables.h |  6 +-
 include/nftables/libnftables.h |  3 +--
 src/libnftables.c  | 10 --
 src/main.c |  2 +-
 src/monitor.c  |  2 +-
 src/rule.c | 22 +++---
 7 files changed, 22 insertions(+), 40 deletions(-)

diff --git a/doc/libnftables.adoc b/doc/libnftables.adoc
index c837c2d251bc..8b7aee9af134 100644
--- a/doc/libnftables.adoc
+++ b/doc/libnftables.adoc
@@ -28,9 +28,6 @@ void nft_ctx_output_set_numeric(struct nft_ctx* '\*ctx'*,
 unsigned int nft_ctx_output_get_debug(struct nft_ctx* '\*ctx'*);
 void nft_ctx_output_set_debug(struct nft_ctx* '\*ctx'*, unsigned int* 'mask'*);
 
-bool nft_ctx_output_get_handle(struct nft_ctx* '\*ctx'*);
-void nft_ctx_output_set_handle(struct nft_ctx* '\*ctx'*, bool* 'val'*);
-
 bool nft_ctx_output_get_echo(struct nft_ctx* '\*ctx'*);
 void nft_ctx_output_set_echo(struct nft_ctx* '\*ctx'*, bool* 'val'*);
 
@@ -96,6 +93,7 @@ enum {
 NFT_CTX_OUTPUT_REVERSEDNS  = (1 << 0),
 NFT_CTX_OUTPUT_SERVICE = (1 << 1),
 NFT_CTX_OUTPUT_STATELESS   = (1 << 2),
+NFT_CTX_OUTPUT_HANDLE  = (1 << 3),
 };
 
 
@@ -105,6 +103,8 @@ NFT_CTX_OUTPUT_SERVICE::
Print port numbers as services as described in the /etc/services file.
 NFT_CTX_OUTPUT_STATELESS::
If stateless output has been requested then stateful data is not 
printed. Stateful data refers to those objects that carry run-time data, eg. 
the *counter* statement holds packet and byte counter values, making it 
stateful.
+NFT_CTX_OUTPUT_HANDLE::
+   Upon insertion into the ruleset, some elements are assigned a unique 
handle for identification purposes. For example, when deleting a table or 
chain, it may be identified either by name or handle. Rules on the other hand 
must be deleted by handle because there is no other way to uniquely identify 
them. These functions allow to control whether ruleset listings should include 
handles or not.
 
 The *nft_ctx_output_get_flags*() function returns the output flags setting's 
value in 'ctx'.
 
@@ -177,17 +177,6 @@ The *nft_ctx_output_get_debug*() function returns the 
debug output setting's val
 
 The *nft_ctx_output_set_debug*() function sets the debug output setting in 
'ctx' to the value of 'mask'.
 
-=== nft_ctx_output_get_handle() and nft_ctx_output_set_handle()
-Upon insertion into the ruleset, some elements are assigned a unique handle 
for identification purposes.
-For example, when deleting a table or chain, it may be identified either by 
name or handle.
-Rules on the other hand must be deleted by handle because there is no other 
way to uniquely identify them.
-These functions allow to control whether ruleset listings should include 
handles or not.
-The default setting is *false*.
-
-The *nft_ctx_output_get_handle*() function returns the handle output setting's 
value in 'ctx'.
-
-The *nft_ctx_output_set_handle*() function sets the handle output setting in 
'ctx' to the value of 'val'.
-
 === nft_ctx_output_get_echo() and nft_ctx_output_set_echo()
 The echo setting makes libnftables print the changes once they are committed 
to the kernel, just like a running instance of *nft monitor* would.
 Amongst other things, this allows to retrieve an added rule's handle 
atomically.
diff --git a/include/nftables.h b/include/nftables.h
index cb36e06633e9..e0e7a1135406 100644
--- a/include/nftables.h
+++ b/include/nftables.h
@@ -18,7 +18,6 @@ struct cookie {
 struct output_ctx {
unsigned int flags;
unsigned int numeric;
-   unsigned int handle;
unsigned int echo;
unsigned int json;
union {
@@ -46,6 +45,11 @@ static inline bool nft_output_stateless(const struct 
output_ctx *octx)
return octx->flags & NFT_CTX_OUTPUT_STATELESS;
 }
 
+static inline bool nft_output_handle(const struct output_ctx *octx)
+{
+   return octx->flags & NFT_CTX_OUTPUT_HANDLE;
+}
+
 struct nft_cache {
uint16_tgenid;
struct list_headlist;
diff --git a/include/nftables/libnftables.h b/include/nftables/libnftables.h
index 4f1c10901b1b..a6ce938305c3 100644
--- a/include/nftables/libnftables.h
+++ b/include/nftables/libnftables.h
@@ -48,6 +48,7 @@ enum {
NFT_CTX_OUTPUT_REVERSEDNS   = (1 << 0),
NFT_CTX_OUTPUT_SERVICE  = (1 << 1),
NFT_CTX_OUTPUT_STATELESS= (1 << 2),
+   NFT_CTX_OUTPUT_HANDLE   = (1 << 3),
 };
 
 unsigned int nft_ctx_output_get_flags(struct nft_ctx *ctx);
@@ -57,8 +58,6 @@ enum nft_numeric_level nft_ctx_output_get_numeric(struct 
nft_ctx *ctx);
 void 

[PATCH nft 1/5,v4] src: Revert --literal, add -S/--service

2018-10-29 Thread Pablo Neira Ayuso
This is a partial revert of b0f6a45b25dd1 ("src: add --literal option")
which was added during the development cycle before 0.9.1 is released.

After looking at patch: https://patchwork.ozlabs.org/patch/969864/ that
allows to print priority, uid, gid and protocols as numerics, I decided
to revisit this to provide individual options to turn on literal
printing.

What I'm proposing is to provide a good default for everyone, and
provide options to turn on literal/numeric printing.

This patch adds nft_ctx_output_{set,get}_flags() and define two flags to
enable reverse DNS lookups and to print ports as service names.

This patch introduces -S/--services, to print service names as per
/etc/services.

Signed-off-by: Pablo Neira Ayuso 
---
v4: add nft_output_reversedns() and nft_output_service().
Refer to /etc/services in documentation and use them.
Rename to --service as all options are in single form.

 doc/libnftables.adoc   | 34 ++
 doc/nft.txt| 17 +
 include/nftables.h | 12 +++-
 include/nftables/libnftables.h | 17 +
 src/datatype.c |  6 +++---
 src/json.c |  2 +-
 src/libnftables.c  |  8 
 src/main.c | 30 ++
 8 files changed, 69 insertions(+), 57 deletions(-)

diff --git a/doc/libnftables.adoc b/doc/libnftables.adoc
index 0387652fa3c1..1c6ea0152d13 100644
--- a/doc/libnftables.adoc
+++ b/doc/libnftables.adoc
@@ -18,6 +18,9 @@ void nft_ctx_free(struct nft_ctx* '\*ctx'*);
 bool nft_ctx_get_dry_run(struct nft_ctx* '\*ctx'*);
 void nft_ctx_set_dry_run(struct nft_ctx* '\*ctx'*, bool* 'dry'*);
 
+unsigned int nft_ctx_output_get_flags(struct nft_ctx* '\*ctx'*);
+void nft_ctx_output_set_flags(struct nft_ctx* '\*ctx'*, unsigned int* 
'flags'*);
+
 enum nft_numeric_level nft_ctx_output_get_numeric(struct nft_ctx* '\*ctx'*);
 void nft_ctx_output_set_numeric(struct nft_ctx* '\*ctx'*,
enum nft_numeric_level* 'level'*);
@@ -25,9 +28,6 @@ void nft_ctx_output_set_numeric(struct nft_ctx* '\*ctx'*,
 bool nft_ctx_output_get_stateless(struct nft_ctx* '\*ctx'*);
 void nft_ctx_output_set_stateless(struct nft_ctx* '\*ctx'*, bool* 'val'*);
 
-enum nft_literal_level nft_ctx_output_get_literal(struct nft_ctx* '\*ctx'*);
-void nft_ctx_output_set_literal(struct nft_ctx* '\*ctx'*, bool* 'val'*);
-
 unsigned int nft_ctx_output_get_debug(struct nft_ctx* '\*ctx'*);
 void nft_ctx_output_set_debug(struct nft_ctx* '\*ctx'*, unsigned int* 'mask'*);
 
@@ -91,6 +91,25 @@ The *nft_ctx_get_dry_run*() function returns the dry-run 
setting's value contain
 
 The *nft_ctx_set_dry_run*() function sets the dry-run setting in 'ctx' to the 
value of 'dry'.
 
+=== nft_ctx_output_get_flags() and nft_ctx_output_set_flags()
+The flags setting controls the output format.
+
+
+enum {
+NFT_CTX_OUTPUT_REVERSEDNS  = (1 << 0),
+NFT_CTX_OUTPUT_SERVICE = (1 << 1),
+};
+
+
+NFT_CTX_OUTPUT_REVERSEDNS::
+   Perform reverse DNS lookups are performed for IP addresses when 
printing. Note that this may add significant delay to *list* commands depending 
on DNS resolver speed.
+NFT_CTX_OUTPUT_SERVICE::
+   Print port numbers as services as described in the /etc/services file.
+
+The *nft_ctx_output_get_flags*() function returns the output flags setting's 
value in 'ctx'.
+
+The *nft_ctx_output_set_flags*() function sets the output flags setting in 
'ctx' to the value of 'val'.
+
 === nft_ctx_output_get_numeric() and nft_ctx_output_set_numeric()
 These functions allow control over value representation in library output.
 For instance, port numbers by default are printed by their name (as listed in 
'/etc/services' file), if known.
@@ -133,15 +152,6 @@ The *nft_ctx_output_get_stateless*() function returns the 
stateless output setti
 
 The *nft_ctx_output_set_stateless*() function sets the stateless output 
setting in 'ctx' to the value of 'val'.
 
-=== nft_ctx_output_get_literal() and nft_ctx_output_set_literal()
-The literal setting controls whether reverse DNS lookups are performed for IP 
addresses when printing them.
-Note that this may add significant delay to *list* commands depending on DNS 
resolver speed.
-The default setting is *NFT_LITERAL_NONE*.
-
-The *nft_ctx_output_get_literal*() function returns the literal output 
setting's value in 'ctx'.
-
-The *nft_ctx_output_set_literal*() function sets the literal output setting in 
'ctx' to the value of 'val'.
-
 === nft_ctx_output_get_debug() and nft_ctx_output_set_debug()
 Libnftables supports separate debugging of different parts of its internals.
 To facilitate this, debugging output is controlled via a bit mask.
diff --git a/doc/nft.txt b/doc/nft.txt
index 2a76a6cc63c4..1735a3577beb 100644
--- a/doc/nft.txt
+++ b/doc/nft.txt
@@ -9,7 +9,7 @@ nft - Administration tool of the nftables framework for packet 
filtering and cla