* [iptables PATCH 1/5] doc: Clean generated *-restore-translate man pages
2019-07-31 16:39 [iptables PATCH 0/5] xtables-monitor enhancements Phil Sutter
@ 2019-07-31 16:39 ` Phil Sutter
2019-07-31 16:39 ` [iptables PATCH 2/5] doc: Fix xtables-monitor man page Phil Sutter
` (3 subsequent siblings)
4 siblings, 0 replies; 16+ messages in thread
From: Phil Sutter @ 2019-07-31 16:39 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
Since they are generated, one has to specify them in CLEANFILES. While
being at it, introduce a variable holding them to improve readability a
bit.
Fixes: 3dfb01cf14d72 ("doc: Install ip{6,}tables-restore-translate.8 man pages")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
iptables/Makefile.am | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/iptables/Makefile.am b/iptables/Makefile.am
index 21ac7f08b7c1f..da07b9a4b5a2f 100644
--- a/iptables/Makefile.am
+++ b/iptables/Makefile.am
@@ -47,6 +47,9 @@ xtables_nft_multi_SOURCES += xshared.c
xtables_nft_multi_LDADD += ../libxtables/libxtables.la -lm
endif
+XTABLES_XLATE_8_LINKS = iptables-translate.8 ip6tables-translate.8 \
+ iptables-restore-translate.8 ip6tables-restore-translate.8
+
sbin_PROGRAMS = xtables-legacy-multi
if ENABLE_NFTABLES
sbin_PROGRAMS += xtables-nft-multi
@@ -56,14 +59,13 @@ man_MANS = iptables.8 iptables-restore.8 iptables-save.8 \
ip6tables-save.8 iptables-extensions.8
if ENABLE_NFTABLES
man_MANS += xtables-nft.8 xtables-translate.8 xtables-legacy.8 \
- iptables-translate.8 ip6tables-translate.8 \
- iptables-restore-translate.8 ip6tables-restore-translate.8 \
+ ${XTABLES_XLATE_8_LINKS} \
xtables-monitor.8 \
arptables-nft.8 arptables-nft-restore.8 arptables-nft-save.8 \
ebtables-nft.8
endif
CLEANFILES = iptables.8 xtables-monitor.8 \
- iptables-translate.8 ip6tables-translate.8
+ ${XTABLES_XLATE_8_LINKS}
vx_bin_links = iptables-xml
if ENABLE_IPV4
@@ -93,7 +95,7 @@ iptables-extensions.8: iptables-extensions.8.tmpl ../extensions/matches.man ../e
-e '/@MATCH@/ r ../extensions/matches.man' \
-e '/@TARGET@/ r ../extensions/targets.man' $< >$@;
-iptables-translate.8 ip6tables-translate.8 iptables-restore-translate.8 ip6tables-restore-translate.8:
+${XTABLES_XLATE_8_LINKS}:
${AM_VERBOSE_GEN} echo '.so man8/xtables-translate.8' >$@
pkgconfig_DATA = xtables.pc
--
2.22.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [iptables PATCH 2/5] doc: Fix xtables-monitor man page
2019-07-31 16:39 [iptables PATCH 0/5] xtables-monitor enhancements Phil Sutter
2019-07-31 16:39 ` [iptables PATCH 1/5] doc: Clean generated *-restore-translate man pages Phil Sutter
@ 2019-07-31 16:39 ` Phil Sutter
2019-07-31 16:39 ` [iptables PATCH 3/5] xtables-monitor: Improve error messages Phil Sutter
` (2 subsequent siblings)
4 siblings, 0 replies; 16+ messages in thread
From: Phil Sutter @ 2019-07-31 16:39 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
Fix some syntax errors, also document -4 and -6 long options.
Fixes: d26c538b9a549 ("xtables: add xtables-monitor")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
iptables/xtables-monitor.8.in | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/iptables/xtables-monitor.8.in b/iptables/xtables-monitor.8.in
index b647a79eb64ed..19eb729c51240 100644
--- a/iptables/xtables-monitor.8.in
+++ b/iptables/xtables-monitor.8.in
@@ -2,7 +2,7 @@
.SH NAME
xtables-monitor \(em show changes to rule set and trace-events
.SH SYNOPSIS
-\fBxtables\-monitor\fP [\fB\-t\fP] [\fB\-e\fP] [\fB\-4\fP|\fB|\-6\fB]
+\fBxtables\-monitor\fP [\fB\-t\fP] [\fB\-e\fP] [\fB\-4\fP|\fB\-6\fP]
.PP
\
.SH DESCRIPTION
@@ -14,8 +14,8 @@ for packets tagged using the TRACE target.
will run until the user aborts execution, typically by using CTRL-C.
.RE
.SH OPTIONS
-\fB\-e\fP, \fB\-\-event\fP
.TP
+\fB\-e\fP, \fB\-\-event\fP
Watch for updates to the rule set.
Updates include creation of new tables, chains and rules and
the name of the program that caused the rule update.
@@ -24,10 +24,10 @@ the name of the program that caused the rule update.
Watch for trace events generated by packets that have been tagged
using the TRACE target.
.TP
-\fB\-4\fP
+\fB\-4\fP, \fB--ipv4\fP
Restrict output to IPv4.
.TP
-\fB\-6\fP
+\fB\-6\fP, \fB--ipv6\fP
Restrict output to IPv6.
.SH EXAMPLE OUTPUT
.TP
--
2.22.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [iptables PATCH 3/5] xtables-monitor: Improve error messages
2019-07-31 16:39 [iptables PATCH 0/5] xtables-monitor enhancements Phil Sutter
2019-07-31 16:39 ` [iptables PATCH 1/5] doc: Clean generated *-restore-translate man pages Phil Sutter
2019-07-31 16:39 ` [iptables PATCH 2/5] doc: Fix xtables-monitor man page Phil Sutter
@ 2019-07-31 16:39 ` Phil Sutter
2019-07-31 16:39 ` [iptables PATCH 4/5] xtables-monitor: Support ARP and bridge families Phil Sutter
2019-07-31 16:39 ` [iptables PATCH 5/5] xtables-monitor: Add family-specific aliases Phil Sutter
4 siblings, 0 replies; 16+ messages in thread
From: Phil Sutter @ 2019-07-31 16:39 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
Print a line explaining what was wrong before the general help text.
Also catch multiple family selectors, they overwrite each other and
hence could cause unexpected behaviour.
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
iptables/xtables-monitor.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/iptables/xtables-monitor.c b/iptables/xtables-monitor.c
index eb80bac81c645..02e8e446b1c8c 100644
--- a/iptables/xtables-monitor.c
+++ b/iptables/xtables-monitor.c
@@ -588,6 +588,16 @@ static void print_usage(void)
exit(EXIT_FAILURE);
}
+static void set_nfproto(struct cb_arg *arg, uint32_t val)
+{
+ if (arg->nfproto != NFPROTO_UNSPEC && arg->nfproto != val) {
+ fprintf(stderr, "Only one of '-4' or '-6' may be specified at once.\n\n");
+ print_usage();
+ exit(PARAMETER_PROBLEM);
+ }
+ arg->nfproto = val;
+}
+
int xtables_monitor_main(int argc, char *argv[])
{
struct mnl_socket *nl;
@@ -626,10 +636,10 @@ int xtables_monitor_main(int argc, char *argv[])
print_usage();
exit(0);
case '4':
- cb_arg.nfproto = NFPROTO_IPV4;
+ set_nfproto(&cb_arg, NFPROTO_IPV4);
break;
case '6':
- cb_arg.nfproto = NFPROTO_IPV6;
+ set_nfproto(&cb_arg, NFPROTO_IPV6);
break;
case 'V':
printf("xtables-monitor %s\n", PACKAGE_VERSION);
@@ -647,6 +657,7 @@ int xtables_monitor_main(int argc, char *argv[])
nfgroup |= 1 << (NFNLGRP_NFTABLES - 1);
if (nfgroup == 0) {
+ fprintf(stderr, "Missing mandatory argument, specify either '-t' or '-e' (or both).\n\n");
print_usage();
exit(EXIT_FAILURE);
}
--
2.22.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [iptables PATCH 4/5] xtables-monitor: Support ARP and bridge families
2019-07-31 16:39 [iptables PATCH 0/5] xtables-monitor enhancements Phil Sutter
` (2 preceding siblings ...)
2019-07-31 16:39 ` [iptables PATCH 3/5] xtables-monitor: Improve error messages Phil Sutter
@ 2019-07-31 16:39 ` Phil Sutter
2019-08-01 11:20 ` Pablo Neira Ayuso
2019-07-31 16:39 ` [iptables PATCH 5/5] xtables-monitor: Add family-specific aliases Phil Sutter
4 siblings, 1 reply; 16+ messages in thread
From: Phil Sutter @ 2019-07-31 16:39 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
Apart from allowing to filter by these families, add missing switch()
cases in chain and rule callbacks.
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
iptables/xtables-monitor.8.in | 12 +++++++++---
iptables/xtables-monitor.c | 23 +++++++++++++++++++++--
2 files changed, 30 insertions(+), 5 deletions(-)
diff --git a/iptables/xtables-monitor.8.in b/iptables/xtables-monitor.8.in
index 19eb729c51240..6bde54fa4a359 100644
--- a/iptables/xtables-monitor.8.in
+++ b/iptables/xtables-monitor.8.in
@@ -2,7 +2,7 @@
.SH NAME
xtables-monitor \(em show changes to rule set and trace-events
.SH SYNOPSIS
-\fBxtables\-monitor\fP [\fB\-t\fP] [\fB\-e\fP] [\fB\-4\fP|\fB\-6\fP]
+\fBxtables\-monitor\fP [\fB\-t\fP] [\fB\-e\fP] [\fB\-0\fP|\fB-1\fP|\fB\-4\fP|\fB\-6\fP]
.PP
\
.SH DESCRIPTION
@@ -24,11 +24,17 @@ the name of the program that caused the rule update.
Watch for trace events generated by packets that have been tagged
using the TRACE target.
.TP
+\fB\-0\fP, \fB--arp\fP
+Restrict output to ARP (i.e., events caused by arptables-nft).
+.TP
+\fB\-1\fP, \fB--bridge\fP
+Restrict output to bridge (i.e., events caused by ebtables-nft).
+.TP
\fB\-4\fP, \fB--ipv4\fP
-Restrict output to IPv4.
+Restrict output to IPv4 (i.e., events caused by iptables-nft).
.TP
\fB\-6\fP, \fB--ipv6\fP
-Restrict output to IPv6.
+Restrict output to IPv6 (i.e., events caused by ip6tables-nft).
.SH EXAMPLE OUTPUT
.TP
.B xtables-monitor \-\-trace
diff --git a/iptables/xtables-monitor.c b/iptables/xtables-monitor.c
index 02e8e446b1c8c..9be8ce9de6b5f 100644
--- a/iptables/xtables-monitor.c
+++ b/iptables/xtables-monitor.c
@@ -101,6 +101,9 @@ static int rule_cb(const struct nlmsghdr *nlh, void *data)
case NFPROTO_ARP:
printf("-0 ");
break;
+ case NFPROTO_BRIDGE:
+ printf("-1 ");
+ break;
default:
goto err_free;
}
@@ -139,6 +142,12 @@ static int chain_cb(const struct nlmsghdr *nlh, void *data)
printf(" EVENT: ");
switch (family) {
+ case NFPROTO_ARP:
+ family = 0;
+ break;
+ case NFPROTO_BRIDGE:
+ family = 1;
+ break;
case NFPROTO_IPV4:
family = 4;
break;
@@ -565,6 +574,8 @@ static const struct option options[] = {
{.name = "counters", .has_arg = false, .val = 'c'},
{.name = "trace", .has_arg = false, .val = 't'},
{.name = "event", .has_arg = false, .val = 'e'},
+ {.name = "arp", .has_arg = false, .val = '0'},
+ {.name = "bridge", .has_arg = false, .val = '1'},
{.name = "ipv4", .has_arg = false, .val = '4'},
{.name = "ipv6", .has_arg = false, .val = '6'},
{.name = "version", .has_arg = false, .val = 'V'},
@@ -580,6 +591,8 @@ static void print_usage(void)
" --trace -t trace ruleset traversal of packets tagged via -j TRACE rule\n"
" --event -e show events that modify the ruleset\n"
"Optional arguments:\n"
+ " --arp -0 only monitor ARP\n"
+ " --bridge -1 only monitor bridge\n"
" --ipv4 -4 only monitor IPv4\n"
" --ipv6 -6 only monitor IPv6\n"
" --counters -c show counters in rules\n"
@@ -591,7 +604,7 @@ static void print_usage(void)
static void set_nfproto(struct cb_arg *arg, uint32_t val)
{
if (arg->nfproto != NFPROTO_UNSPEC && arg->nfproto != val) {
- fprintf(stderr, "Only one of '-4' or '-6' may be specified at once.\n\n");
+ fprintf(stderr, "Only one of '-0', '-1', '-4' or '-6' may be specified at once.\n\n");
print_usage();
exit(PARAMETER_PROBLEM);
}
@@ -621,8 +634,14 @@ int xtables_monitor_main(int argc, char *argv[])
#endif
opterr = 0;
- while ((c = getopt_long(argc, argv, "ceht46V", options, NULL)) != -1) {
+ while ((c = getopt_long(argc, argv, "ceht0146V", options, NULL)) != -1) {
switch (c) {
+ case '0':
+ set_nfproto(&cb_arg, NFPROTO_ARP);
+ break;
+ case '1':
+ set_nfproto(&cb_arg, NFPROTO_BRIDGE);
+ break;
case 'c':
counters = true;
break;
--
2.22.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [iptables PATCH 4/5] xtables-monitor: Support ARP and bridge families
2019-07-31 16:39 ` [iptables PATCH 4/5] xtables-monitor: Support ARP and bridge families Phil Sutter
@ 2019-08-01 11:20 ` Pablo Neira Ayuso
2019-08-01 12:00 ` Phil Sutter
0 siblings, 1 reply; 16+ messages in thread
From: Pablo Neira Ayuso @ 2019-08-01 11:20 UTC (permalink / raw)
To: Phil Sutter; +Cc: netfilter-devel
On Wed, Jul 31, 2019 at 06:39:14PM +0200, Phil Sutter wrote:
@@ -565,6 +574,8 @@ static const struct option options[] = {
> {.name = "counters", .has_arg = false, .val = 'c'},
> {.name = "trace", .has_arg = false, .val = 't'},
> {.name = "event", .has_arg = false, .val = 'e'},
> + {.name = "arp", .has_arg = false, .val = '0'},
> + {.name = "bridge", .has_arg = false, .val = '1'},
Probably?
-A for arp.
-B for bridge.
so users don't have to remember? -4 and -6 are intuitive, I'd like
these are sort of intuitive too in its compact definition.
Apart from that, patchset looks good to me.
Thanks.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [iptables PATCH 4/5] xtables-monitor: Support ARP and bridge families
2019-08-01 11:20 ` Pablo Neira Ayuso
@ 2019-08-01 12:00 ` Phil Sutter
2019-08-01 12:30 ` Pablo Neira Ayuso
0 siblings, 1 reply; 16+ messages in thread
From: Phil Sutter @ 2019-08-01 12:00 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
On Thu, Aug 01, 2019 at 01:20:50PM +0200, Pablo Neira Ayuso wrote:
> On Wed, Jul 31, 2019 at 06:39:14PM +0200, Phil Sutter wrote:
> @@ -565,6 +574,8 @@ static const struct option options[] = {
> > {.name = "counters", .has_arg = false, .val = 'c'},
> > {.name = "trace", .has_arg = false, .val = 't'},
> > {.name = "event", .has_arg = false, .val = 'e'},
> > + {.name = "arp", .has_arg = false, .val = '0'},
> > + {.name = "bridge", .has_arg = false, .val = '1'},
>
> Probably?
>
> -A for arp.
> -B for bridge.
>
> so users don't have to remember? -4 and -6 are intuitive, I'd like
> these are sort of intuitive too in its compact definition.
>
> Apart from that, patchset looks good to me.
I had something like that (-a and -b should still be free), but then
discovered that for rules there was '-0' prefix in use when printing arp
family rules. Should I change these prefixes also or leave them as -0
and -1? I guess most importantly they must not clash with real
parameters.
Thanks, Phil
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [iptables PATCH 4/5] xtables-monitor: Support ARP and bridge families
2019-08-01 12:00 ` Phil Sutter
@ 2019-08-01 12:30 ` Pablo Neira Ayuso
2019-08-01 12:33 ` Pablo Neira Ayuso
2019-08-01 12:41 ` Phil Sutter
0 siblings, 2 replies; 16+ messages in thread
From: Pablo Neira Ayuso @ 2019-08-01 12:30 UTC (permalink / raw)
To: Phil Sutter, netfilter-devel
On Thu, Aug 01, 2019 at 02:00:48PM +0200, Phil Sutter wrote:
> On Thu, Aug 01, 2019 at 01:20:50PM +0200, Pablo Neira Ayuso wrote:
> > On Wed, Jul 31, 2019 at 06:39:14PM +0200, Phil Sutter wrote:
> > @@ -565,6 +574,8 @@ static const struct option options[] = {
> > > {.name = "counters", .has_arg = false, .val = 'c'},
> > > {.name = "trace", .has_arg = false, .val = 't'},
> > > {.name = "event", .has_arg = false, .val = 'e'},
> > > + {.name = "arp", .has_arg = false, .val = '0'},
> > > + {.name = "bridge", .has_arg = false, .val = '1'},
> >
> > Probably?
> >
> > -A for arp.
> > -B for bridge.
> >
> > so users don't have to remember? -4 and -6 are intuitive, I'd like
> > these are sort of intuitive too in its compact definition.
> >
> > Apart from that, patchset looks good to me.
>
> I had something like that (-a and -b should still be free), but then
> discovered that for rules there was '-0' prefix in use when printing arp
> family rules. Should I change these prefixes also or leave them as -0
> and -1? I guess most importantly they must not clash with real
> parameters.
You can just leave them as is if this is the way this is exposed in
rules. Not sure what the logic behing -0 and -1 is, this is not
mapping to NFPROTO_* definitions, so it looks like something it's been
pulled out of someone's hat :-)
I think users will end up using --arp and --bridge for this. I myself
will not remember this -0 and -1 thing.
Feel free to explore any possibility, probably leaving the existing -0
and -1 in place if you're afraid of breaking anything, add aliases and
only document the more intuitive one. If you think this is worth
exploring, of course.
Thanks!
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [iptables PATCH 4/5] xtables-monitor: Support ARP and bridge families
2019-08-01 12:30 ` Pablo Neira Ayuso
@ 2019-08-01 12:33 ` Pablo Neira Ayuso
2019-08-01 12:41 ` Phil Sutter
1 sibling, 0 replies; 16+ messages in thread
From: Pablo Neira Ayuso @ 2019-08-01 12:33 UTC (permalink / raw)
To: Phil Sutter, netfilter-devel
On Thu, Aug 01, 2019 at 02:30:40PM +0200, Pablo Neira Ayuso wrote:
> On Thu, Aug 01, 2019 at 02:00:48PM +0200, Phil Sutter wrote:
> > On Thu, Aug 01, 2019 at 01:20:50PM +0200, Pablo Neira Ayuso wrote:
> > > On Wed, Jul 31, 2019 at 06:39:14PM +0200, Phil Sutter wrote:
> > > @@ -565,6 +574,8 @@ static const struct option options[] = {
> > > > {.name = "counters", .has_arg = false, .val = 'c'},
> > > > {.name = "trace", .has_arg = false, .val = 't'},
> > > > {.name = "event", .has_arg = false, .val = 'e'},
> > > > + {.name = "arp", .has_arg = false, .val = '0'},
> > > > + {.name = "bridge", .has_arg = false, .val = '1'},
> > >
> > > Probably?
> > >
> > > -A for arp.
> > > -B for bridge.
> > >
> > > so users don't have to remember? -4 and -6 are intuitive, I'd like
> > > these are sort of intuitive too in its compact definition.
> > >
> > > Apart from that, patchset looks good to me.
> >
> > I had something like that (-a and -b should still be free), but then
> > discovered that for rules there was '-0' prefix in use when printing arp
> > family rules. Should I change these prefixes also or leave them as -0
> > and -1? I guess most importantly they must not clash with real
> > parameters.
>
> You can just leave them as is if this is the way this is exposed in
> rules. Not sure what the logic behing -0 and -1 is, this is not
> mapping to NFPROTO_* definitions, so it looks like something it's been
> pulled out of someone's hat :-)
>
> I think users will end up using --arp and --bridge for this. I myself
> will not remember this -0 and -1 thing.
Probably exposing:
iptables-monitor
ip6tables-monitor
arptables-monitor
ebtables-monitor
although this will not solve the problem that we are discussing here,
I think having those around would be nice.
The xtables-monitor variant still will need to sort out the -0 and -1
thing that we're discussing here.
> Feel free to explore any possibility, probably leaving the existing -0
> and -1 in place if you're afraid of breaking anything, add aliases and
> only document the more intuitive one. If you think this is worth
> exploring, of course.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [iptables PATCH 4/5] xtables-monitor: Support ARP and bridge families
2019-08-01 12:30 ` Pablo Neira Ayuso
2019-08-01 12:33 ` Pablo Neira Ayuso
@ 2019-08-01 12:41 ` Phil Sutter
2019-08-01 12:47 ` Pablo Neira Ayuso
1 sibling, 1 reply; 16+ messages in thread
From: Phil Sutter @ 2019-08-01 12:41 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
Hi,
On Thu, Aug 01, 2019 at 02:30:40PM +0200, Pablo Neira Ayuso wrote:
> On Thu, Aug 01, 2019 at 02:00:48PM +0200, Phil Sutter wrote:
> > On Thu, Aug 01, 2019 at 01:20:50PM +0200, Pablo Neira Ayuso wrote:
> > > On Wed, Jul 31, 2019 at 06:39:14PM +0200, Phil Sutter wrote:
> > > @@ -565,6 +574,8 @@ static const struct option options[] = {
> > > > {.name = "counters", .has_arg = false, .val = 'c'},
> > > > {.name = "trace", .has_arg = false, .val = 't'},
> > > > {.name = "event", .has_arg = false, .val = 'e'},
> > > > + {.name = "arp", .has_arg = false, .val = '0'},
> > > > + {.name = "bridge", .has_arg = false, .val = '1'},
> > >
> > > Probably?
> > >
> > > -A for arp.
> > > -B for bridge.
> > >
> > > so users don't have to remember? -4 and -6 are intuitive, I'd like
> > > these are sort of intuitive too in its compact definition.
> > >
> > > Apart from that, patchset looks good to me.
> >
> > I had something like that (-a and -b should still be free), but then
> > discovered that for rules there was '-0' prefix in use when printing arp
> > family rules. Should I change these prefixes also or leave them as -0
> > and -1? I guess most importantly they must not clash with real
> > parameters.
>
> You can just leave them as is if this is the way this is exposed in
> rules. Not sure what the logic behing -0 and -1 is, this is not
> mapping to NFPROTO_* definitions, so it looks like something it's been
> pulled out of someone's hat :-)
Well, the '-1' certainly was! :D
In ss tool, '-0' is used to select packet sockets. Maybe that's where it
came from.
> I think users will end up using --arp and --bridge for this. I myself
> will not remember this -0 and -1 thing.
That's correct. So I guess changing cmdline flags to -a/-b makes sense
either way.
> Feel free to explore any possibility, probably leaving the existing -0
> and -1 in place if you're afraid of breaking anything, add aliases and
> only document the more intuitive one. If you think this is worth
> exploring, of course.
I would omit the prefix from output if a family was selected. For
unfiltered xtables-monitor output, I would change the prefix to
something more readable, e.g.:
'ip: ',
'ip6: ',
'arp: ',
'eb: '
What do you think?
Thanks for the input,
Phil
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [iptables PATCH 4/5] xtables-monitor: Support ARP and bridge families
2019-08-01 12:41 ` Phil Sutter
@ 2019-08-01 12:47 ` Pablo Neira Ayuso
2019-08-01 12:58 ` Phil Sutter
0 siblings, 1 reply; 16+ messages in thread
From: Pablo Neira Ayuso @ 2019-08-01 12:47 UTC (permalink / raw)
To: Phil Sutter, netfilter-devel
On Thu, Aug 01, 2019 at 02:41:07PM +0200, Phil Sutter wrote:
> Hi,
>
> On Thu, Aug 01, 2019 at 02:30:40PM +0200, Pablo Neira Ayuso wrote:
> > On Thu, Aug 01, 2019 at 02:00:48PM +0200, Phil Sutter wrote:
[...]
> > I think users will end up using --arp and --bridge for this. I myself
> > will not remember this -0 and -1 thing.
>
> That's correct. So I guess changing cmdline flags to -a/-b makes sense
> either way.
In the rule side, getopt_long() is already pretty overloaded, just
double check these are spare.
> > Feel free to explore any possibility, probably leaving the existing -0
> > and -1 in place if you're afraid of breaking anything, add aliases and
> > only document the more intuitive one. If you think this is worth
> > exploring, of course.
>
> I would omit the prefix from output if a family was selected. For
> unfiltered xtables-monitor output, I would change the prefix to
> something more readable, e.g.:
>
> 'ip: ',
> 'ip6: ',
> 'arp: ',
> 'eb: '
>
> What do you think?
Probably use the long option name, which seems more readable to me:
EVENT: --ipv4 -t filter -A INPUT -j ACCEPT
I like that the event is printed using the {ip,...}tables syntax.
Thanks!
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [iptables PATCH 4/5] xtables-monitor: Support ARP and bridge families
2019-08-01 12:47 ` Pablo Neira Ayuso
@ 2019-08-01 12:58 ` Phil Sutter
2019-08-01 13:03 ` Pablo Neira Ayuso
0 siblings, 1 reply; 16+ messages in thread
From: Phil Sutter @ 2019-08-01 12:58 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
On Thu, Aug 01, 2019 at 02:47:38PM +0200, Pablo Neira Ayuso wrote:
> On Thu, Aug 01, 2019 at 02:41:07PM +0200, Phil Sutter wrote:
> > Hi,
> >
> > On Thu, Aug 01, 2019 at 02:30:40PM +0200, Pablo Neira Ayuso wrote:
> > > On Thu, Aug 01, 2019 at 02:00:48PM +0200, Phil Sutter wrote:
> [...]
> > > I think users will end up using --arp and --bridge for this. I myself
> > > will not remember this -0 and -1 thing.
> >
> > That's correct. So I guess changing cmdline flags to -a/-b makes sense
> > either way.
>
> In the rule side, getopt_long() is already pretty overloaded, just
> double check these are spare.
This is only about xtables-monitor cmdline, or am I missing something?
> > > Feel free to explore any possibility, probably leaving the existing -0
> > > and -1 in place if you're afraid of breaking anything, add aliases and
> > > only document the more intuitive one. If you think this is worth
> > > exploring, of course.
> >
> > I would omit the prefix from output if a family was selected. For
> > unfiltered xtables-monitor output, I would change the prefix to
> > something more readable, e.g.:
> >
> > 'ip: ',
> > 'ip6: ',
> > 'arp: ',
> > 'eb: '
> >
> > What do you think?
>
> Probably use the long option name, which seems more readable to me:
>
> EVENT: --ipv4 -t filter -A INPUT -j ACCEPT
Ah, good idea!
> I like that the event is printed using the {ip,...}tables syntax.
OK. --arp/--bridge won't work there, obviously. We could of course try
to change that, but I guess it's not feasible. Also, IIRC 'iptables -6'
was buggy in that it should fail but does not. This is a compatibility
issue I didn't get to fix yet.
Cheers, Phil
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [iptables PATCH 4/5] xtables-monitor: Support ARP and bridge families
2019-08-01 12:58 ` Phil Sutter
@ 2019-08-01 13:03 ` Pablo Neira Ayuso
2019-08-01 14:20 ` Phil Sutter
0 siblings, 1 reply; 16+ messages in thread
From: Pablo Neira Ayuso @ 2019-08-01 13:03 UTC (permalink / raw)
To: Phil Sutter, netfilter-devel
On Thu, Aug 01, 2019 at 02:58:00PM +0200, Phil Sutter wrote:
> On Thu, Aug 01, 2019 at 02:47:38PM +0200, Pablo Neira Ayuso wrote:
> > On Thu, Aug 01, 2019 at 02:41:07PM +0200, Phil Sutter wrote:
> > > Hi,
> > >
> > > On Thu, Aug 01, 2019 at 02:30:40PM +0200, Pablo Neira Ayuso wrote:
> > > > On Thu, Aug 01, 2019 at 02:00:48PM +0200, Phil Sutter wrote:
> > [...]
> > > > I think users will end up using --arp and --bridge for this. I myself
> > > > will not remember this -0 and -1 thing.
> > >
> > > That's correct. So I guess changing cmdline flags to -a/-b makes sense
> > > either way.
> >
> > In the rule side, getopt_long() is already pretty overloaded, just
> > double check these are spare.
>
> This is only about xtables-monitor cmdline, or am I missing something?
I was referring to the iptables rule command. Not sure it's worth
there the alias. I think you mentioned that there's already -0 and -1
in the rule command line, hence the -0 and -1 for xtables-monitor.
> > > > Feel free to explore any possibility, probably leaving the existing -0
> > > > and -1 in place if you're afraid of breaking anything, add aliases and
> > > > only document the more intuitive one. If you think this is worth
> > > > exploring, of course.
> > >
> > > I would omit the prefix from output if a family was selected. For
> > > unfiltered xtables-monitor output, I would change the prefix to
> > > something more readable, e.g.:
> > >
> > > 'ip: ',
> > > 'ip6: ',
> > > 'arp: ',
> > > 'eb: '
> > >
> > > What do you think?
> >
> > Probably use the long option name, which seems more readable to me:
> >
> > EVENT: --ipv4 -t filter -A INPUT -j ACCEPT
>
> Ah, good idea!
>
> > I like that the event is printed using the {ip,...}tables syntax.
>
> OK. --arp/--bridge won't work there, obviously. We could of course try
> to change that, but I guess it's not feasible.
I think we would need a common parser, and that's not feasible. Unless
there is some preparsing, just to check if the family option is in
place, ie. -4, -6, --arp and --bridge, then route the parsing to the
corresponding parser. It's a bit of extra glue code, not sure it's
worth, just an idea / future work if helping all these tooling
converge might be of interest.
> Also, IIRC 'iptables -6' was buggy in that it should fail but does
> not. This is a compatibility issue I didn't get to fix yet.
Noted. I have seen the recent patch to fix this.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [iptables PATCH 4/5] xtables-monitor: Support ARP and bridge families
2019-08-01 13:03 ` Pablo Neira Ayuso
@ 2019-08-01 14:20 ` Phil Sutter
0 siblings, 0 replies; 16+ messages in thread
From: Phil Sutter @ 2019-08-01 14:20 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
On Thu, Aug 01, 2019 at 03:03:03PM +0200, Pablo Neira Ayuso wrote:
> On Thu, Aug 01, 2019 at 02:58:00PM +0200, Phil Sutter wrote:
> > On Thu, Aug 01, 2019 at 02:47:38PM +0200, Pablo Neira Ayuso wrote:
> > > On Thu, Aug 01, 2019 at 02:41:07PM +0200, Phil Sutter wrote:
> > > > Hi,
> > > >
> > > > On Thu, Aug 01, 2019 at 02:30:40PM +0200, Pablo Neira Ayuso wrote:
> > > > > On Thu, Aug 01, 2019 at 02:00:48PM +0200, Phil Sutter wrote:
> > > [...]
> > > > > I think users will end up using --arp and --bridge for this. I myself
> > > > > will not remember this -0 and -1 thing.
> > > >
> > > > That's correct. So I guess changing cmdline flags to -a/-b makes sense
> > > > either way.
> > >
> > > In the rule side, getopt_long() is already pretty overloaded, just
> > > double check these are spare.
> >
> > This is only about xtables-monitor cmdline, or am I missing something?
>
> I was referring to the iptables rule command. Not sure it's worth
> there the alias. I think you mentioned that there's already -0 and -1
> in the rule command line, hence the -0 and -1 for xtables-monitor.
Why should xtables-monitor print something that can be used as input to
iptables?
> > > > > Feel free to explore any possibility, probably leaving the existing -0
> > > > > and -1 in place if you're afraid of breaking anything, add aliases and
> > > > > only document the more intuitive one. If you think this is worth
> > > > > exploring, of course.
> > > >
> > > > I would omit the prefix from output if a family was selected. For
> > > > unfiltered xtables-monitor output, I would change the prefix to
> > > > something more readable, e.g.:
> > > >
> > > > 'ip: ',
> > > > 'ip6: ',
> > > > 'arp: ',
> > > > 'eb: '
> > > >
> > > > What do you think?
> > >
> > > Probably use the long option name, which seems more readable to me:
> > >
> > > EVENT: --ipv4 -t filter -A INPUT -j ACCEPT
> >
> > Ah, good idea!
> >
> > > I like that the event is printed using the {ip,...}tables syntax.
> >
> > OK. --arp/--bridge won't work there, obviously. We could of course try
> > to change that, but I guess it's not feasible.
>
> I think we would need a common parser, and that's not feasible. Unless
> there is some preparsing, just to check if the family option is in
> place, ie. -4, -6, --arp and --bridge, then route the parsing to the
> corresponding parser. It's a bit of extra glue code, not sure it's
> worth, just an idea / future work if helping all these tooling
> converge might be of interest.
Given the large differences in ebtables cmdline syntax to the other
tools, I consider it a plus to have different commands (and hence
separate "main" functions).
> > Also, IIRC 'iptables -6' was buggy in that it should fail but does
> > not. This is a compatibility issue I didn't get to fix yet.
>
> Noted. I have seen the recent patch to fix this.
That was only for iptables-nft-restore. I am talking about plain
iptables:
| % iptables-legacy -6 -A FORWARD -j ACCEPT
| This is the IPv4 version of iptables.
| Try `iptables -h' or 'iptables --help' for more information.
iptables-nft accepts this but the result seems to be identical to just
omitting '-6'.
Cheers, Phil
^ permalink raw reply [flat|nested] 16+ messages in thread
* [iptables PATCH 5/5] xtables-monitor: Add family-specific aliases
2019-07-31 16:39 [iptables PATCH 0/5] xtables-monitor enhancements Phil Sutter
` (3 preceding siblings ...)
2019-07-31 16:39 ` [iptables PATCH 4/5] xtables-monitor: Support ARP and bridge families Phil Sutter
@ 2019-07-31 16:39 ` Phil Sutter
2019-07-31 17:45 ` Phil Sutter
4 siblings, 1 reply; 16+ messages in thread
From: Phil Sutter @ 2019-07-31 16:39 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
Allow for more intuitive xtables-monitor use, e.g. 'ebtables-monitor'
instead of 'xtables-monitor --bridge'. This needs separate main
functions to call from xtables-nft-multi.c and in turn allows to
properly initialize for each family. The latter is required to correctly
print e.g. rules using ebtables extensions.
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
iptables/Makefile.am | 13 ++++--
iptables/xtables-monitor.8.in | 14 ++++++
iptables/xtables-monitor.c | 88 ++++++++++++++++++++++++++++-------
iptables/xtables-multi.h | 4 ++
iptables/xtables-nft-multi.c | 4 ++
5 files changed, 104 insertions(+), 19 deletions(-)
diff --git a/iptables/Makefile.am b/iptables/Makefile.am
index da07b9a4b5a2f..0af9f8dc7738e 100644
--- a/iptables/Makefile.am
+++ b/iptables/Makefile.am
@@ -49,6 +49,8 @@ endif
XTABLES_XLATE_8_LINKS = iptables-translate.8 ip6tables-translate.8 \
iptables-restore-translate.8 ip6tables-restore-translate.8
+XTABLES_MONITOR_8_LINKS = iptables-monitor.8 ip6tables-monitor.8 \
+ arptables-monitor.8 ebtables-monitor.8
sbin_PROGRAMS = xtables-legacy-multi
if ENABLE_NFTABLES
@@ -60,12 +62,13 @@ man_MANS = iptables.8 iptables-restore.8 iptables-save.8 \
if ENABLE_NFTABLES
man_MANS += xtables-nft.8 xtables-translate.8 xtables-legacy.8 \
${XTABLES_XLATE_8_LINKS} \
- xtables-monitor.8 \
+ xtables-monitor.8 ${XTABLES_MONITOR_8_LINKS} \
arptables-nft.8 arptables-nft-restore.8 arptables-nft-save.8 \
ebtables-nft.8
endif
CLEANFILES = iptables.8 xtables-monitor.8 \
- ${XTABLES_XLATE_8_LINKS}
+ ${XTABLES_XLATE_8_LINKS} \
+ ${XTABLES_MONITOR_8_LINKS}
vx_bin_links = iptables-xml
if ENABLE_IPV4
@@ -87,7 +90,8 @@ x_sbin_links = iptables-nft iptables-nft-restore iptables-nft-save \
ebtables-nft ebtables \
ebtables-nft-restore ebtables-restore \
ebtables-nft-save ebtables-save \
- xtables-monitor
+ xtables-monitor arptables-monitor ebtables-monitor \
+ iptables-monitor ip6tables-monitor
endif
iptables-extensions.8: iptables-extensions.8.tmpl ../extensions/matches.man ../extensions/targets.man
@@ -98,6 +102,9 @@ iptables-extensions.8: iptables-extensions.8.tmpl ../extensions/matches.man ../e
${XTABLES_XLATE_8_LINKS}:
${AM_VERBOSE_GEN} echo '.so man8/xtables-translate.8' >$@
+${XTABLES_MONITOR_8_LINKS}:
+ ${AM_VERBOSE_GEN} echo '.so man8/xtables-monitor.8' >$@
+
pkgconfig_DATA = xtables.pc
# Using if..fi avoids an ugly "error (ignored)" message :)
diff --git a/iptables/xtables-monitor.8.in b/iptables/xtables-monitor.8.in
index 6bde54fa4a359..b91b81489d1b9 100644
--- a/iptables/xtables-monitor.8.in
+++ b/iptables/xtables-monitor.8.in
@@ -3,6 +3,14 @@
xtables-monitor \(em show changes to rule set and trace-events
.SH SYNOPSIS
\fBxtables\-monitor\fP [\fB\-t\fP] [\fB\-e\fP] [\fB\-0\fP|\fB-1\fP|\fB\-4\fP|\fB\-6\fP]
+.br
+\fBiptables-monitor\fP [\fB\-t\fP] [\fB\-e\fP]
+.br
+\fBip6tables-monitor\fP [\fB\-t\fP] [\fB\-e\fP]
+.br
+\fBarptables-monitor\fP [\fB\-t\fP] [\fB\-e\fP]
+.br
+\fBebtables-monitor\fP [\fB\-t\fP] [\fB\-e\fP]
.PP
\
.SH DESCRIPTION
@@ -12,6 +20,12 @@ is used to monitor changes to the ruleset or to show rule evaluation events
for packets tagged using the TRACE target.
.B xtables-monitor
will run until the user aborts execution, typically by using CTRL-C.
+.PP
+.BR iptables-monitor ", " ip6tables-monitor ", "
+.BR arptables-monitor " and " ebtables-monitor
+are aliases to calling
+.B xtables-monitor
+with a family filtering flag.
.RE
.SH OPTIONS
.TP
diff --git a/iptables/xtables-monitor.c b/iptables/xtables-monitor.c
index 9be8ce9de6b5f..71e9de45a34a3 100644
--- a/iptables/xtables-monitor.c
+++ b/iptables/xtables-monitor.c
@@ -37,6 +37,7 @@
#include "xtables-multi.h"
#include "nft.h"
#include "nft-arp.h"
+#include "nft-bridge.h"
struct cb_arg {
uint32_t nfproto;
@@ -611,28 +612,16 @@ static void set_nfproto(struct cb_arg *arg, uint32_t val)
arg->nfproto = val;
}
-int xtables_monitor_main(int argc, char *argv[])
+static int __xtables_monitor_main(uint32_t family, int argc, char *argv[])
{
struct mnl_socket *nl;
char buf[MNL_SOCKET_BUFFER_SIZE];
uint32_t nfgroup = 0;
- struct cb_arg cb_arg = {};
+ struct cb_arg cb_arg = {
+ .nfproto = family,
+ };
int ret, c;
- xtables_globals.program_name = "xtables-monitor";
- /* XXX xtables_init_all does several things we don't want */
- c = xtables_init_all(&xtables_globals, NFPROTO_IPV4);
- if (c < 0) {
- fprintf(stderr, "%s/%s Failed to initialize xtables\n",
- xtables_globals.program_name,
- xtables_globals.program_version);
- exit(1);
- }
-#if defined(ALL_INCLUSIVE) || defined(NO_SHARED_LIBS)
- init_extensions();
- init_extensions4();
-#endif
-
opterr = 0;
while ((c = getopt_long(argc, argv, "ceht0146V", options, NULL)) != -1) {
switch (c) {
@@ -708,3 +697,70 @@ int xtables_monitor_main(int argc, char *argv[])
return EXIT_SUCCESS;
}
+static void common_xtables_init(const char *argv0, uint8_t family)
+{
+ xtables_globals.program_name = basename(argv0);
+
+ /* XXX xtables_init_all does several things we don't want */
+ if (xtables_init_all(&xtables_globals, family) < 0) {
+ fprintf(stderr, "%s/%s Failed to initialize xtables\n",
+ xtables_globals.program_name,
+ xtables_globals.program_version);
+ exit(1);
+ }
+}
+
+int xtables_monitor_main(int argc, char *argv[])
+{
+ /* Can't pass NFPROTO_UNSPEC, xtables_set_nfproto() would complain. */
+ common_xtables_init(argv[0], NFPROTO_IPV4);
+
+ return __xtables_monitor_main(NFPROTO_UNSPEC, argc, argv);
+}
+
+int iptables_monitor_main(int argc, char *argv[])
+{
+ common_xtables_init(argv[0], NFPROTO_IPV4);
+#if defined(ALL_INCLUSIVE) || defined(NO_SHARED_LIBS)
+ init_extensions();
+ init_extensions4();
+#endif
+
+ return __xtables_monitor_main(NFPROTO_IPV4, argc, argv);
+}
+
+int ip6tables_monitor_main(int argc, char *argv[])
+{
+ common_xtables_init(argv[0], NFPROTO_IPV6);
+#if defined(ALL_INCLUSIVE) || defined(NO_SHARED_LIBS)
+ init_extensions();
+ init_extensions4();
+#endif
+
+ return __xtables_monitor_main(NFPROTO_IPV6, argc, argv);
+}
+
+int arptables_monitor_main(int argc, char *argv[])
+{
+ common_xtables_init(argv[0], NFPROTO_ARP);
+#if defined(ALL_INCLUSIVE) || defined(NO_SHARED_LIBS)
+ init_extensionsa();
+#endif
+
+ return __xtables_monitor_main(NFPROTO_ARP, argc, argv);
+}
+
+int ebtables_monitor_main(int argc, char *argv[])
+{
+ common_xtables_init(argv[0], NFPROTO_BRIDGE);
+#if defined(ALL_INCLUSIVE) || defined(NO_SHARED_LIBS)
+ init_extensionsb();
+#endif
+ /* manually registering ebt matches, given the original ebtables parser
+ * don't use '-m matchname' and the match can't be loaded dynamically when
+ * the user calls it.
+ */
+ ebt_load_match_extensions();
+
+ return __xtables_monitor_main(NFPROTO_BRIDGE, argc, argv);
+}
diff --git a/iptables/xtables-multi.h b/iptables/xtables-multi.h
index 0fedb430e1a28..11364ba5b731e 100644
--- a/iptables/xtables-multi.h
+++ b/iptables/xtables-multi.h
@@ -22,6 +22,10 @@ extern int xtables_eb_restore_main(int, char **);
extern int xtables_eb_save_main(int, char **);
extern int xtables_config_main(int, char **);
extern int xtables_monitor_main(int, char **);
+extern int iptables_monitor_main(int, char **);
+extern int ip6tables_monitor_main(int, char **);
+extern int arptables_monitor_main(int, char **);
+extern int ebtables_monitor_main(int, char **);
#endif
#endif /* _XTABLES_MULTI_H */
diff --git a/iptables/xtables-nft-multi.c b/iptables/xtables-nft-multi.c
index e2b7c641f85dd..3b13d89dffcb3 100644
--- a/iptables/xtables-nft-multi.c
+++ b/iptables/xtables-nft-multi.c
@@ -44,6 +44,10 @@ static const struct subcommand multi_subcommands[] = {
{"ebtables-nft-restore", xtables_eb_restore_main},
{"ebtables-nft-save", xtables_eb_save_main},
{"xtables-monitor", xtables_monitor_main},
+ {"iptables-monitor", iptables_monitor_main},
+ {"ip6tables-monitor", ip6tables_monitor_main},
+ {"arptables-monitor", arptables_monitor_main},
+ {"ebtables-monitor", ebtables_monitor_main},
{NULL},
};
--
2.22.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [iptables PATCH 5/5] xtables-monitor: Add family-specific aliases
2019-07-31 16:39 ` [iptables PATCH 5/5] xtables-monitor: Add family-specific aliases Phil Sutter
@ 2019-07-31 17:45 ` Phil Sutter
0 siblings, 0 replies; 16+ messages in thread
From: Phil Sutter @ 2019-07-31 17:45 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
On Wed, Jul 31, 2019 at 06:39:15PM +0200, Phil Sutter wrote:
> Allow for more intuitive xtables-monitor use, e.g. 'ebtables-monitor'
> instead of 'xtables-monitor --bridge'. This needs separate main
> functions to call from xtables-nft-multi.c and in turn allows to
> properly initialize for each family. The latter is required to correctly
> print e.g. rules using ebtables extensions.
>
> Signed-off-by: Phil Sutter <phil@nwl.cc>
This one needs a v2, I forgot to update iptables/.gitignore.
Sorry, Phil
^ permalink raw reply [flat|nested] 16+ messages in thread