* [PATCH xtables-nft 0/3] xt-monitor fixes
@ 2020-12-12 15:15 Florian Westphal
2020-12-12 15:15 ` [PATCH xtables-nft 1/3] xtables-monitor: fix rule printing Florian Westphal
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Florian Westphal @ 2020-12-12 15:15 UTC (permalink / raw)
To: netfilter-devel; +Cc: Florian Westphal
rule tracing via xt-monitor has various bugs:
1. It prints unrelated rules because the function supposed
to print the traced rule does a dump instead of a handle
lookup. This prints all rules in the chain instead of just one.
2. Print the table family, not whatever family user provided on command line.
3. The packet shoud be printed first, instead of after the first
trace event.
4. also make sure to flush stdout after each event so stdout redirect
to files/pipes etc. works.
After this the output is much more similar to nft monitor, just in
xt rule format.
Florian Westphal (3):
xtables-monitor: fix rule printing
xtables-monitor: fix packet family protocol
xtables-monitor: print packet first
iptables/xtables-monitor.c | 70 ++++++++++++++++++++++----------------
1 file changed, 40 insertions(+), 30 deletions(-)
--
2.28.0
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH xtables-nft 1/3] xtables-monitor: fix rule printing
2020-12-12 15:15 [PATCH xtables-nft 0/3] xt-monitor fixes Florian Westphal
@ 2020-12-12 15:15 ` Florian Westphal
2020-12-14 14:19 ` Phil Sutter
2020-12-12 15:15 ` [PATCH xtables-nft 2/3] xtables-monitor: fix packet family protocol Florian Westphal
2020-12-12 15:15 ` [PATCH xtables-nft 3/3] xtables-monitor: print packet first Florian Westphal
2 siblings, 1 reply; 7+ messages in thread
From: Florian Westphal @ 2020-12-12 15:15 UTC (permalink / raw)
To: netfilter-devel; +Cc: Florian Westphal
trace_print_rule does a rule dump. This prints unrelated rules
in the same chain. Instead the function should only request the
specific handle.
Furthermore, flush output buffer afterwards so this plays nice when
output isn't a terminal.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
iptables/xtables-monitor.c | 32 +++++++++++++++-----------------
1 file changed, 15 insertions(+), 17 deletions(-)
diff --git a/iptables/xtables-monitor.c b/iptables/xtables-monitor.c
index 4008cc00d469..364e600e1b38 100644
--- a/iptables/xtables-monitor.c
+++ b/iptables/xtables-monitor.c
@@ -227,12 +227,12 @@ static void trace_print_rule(const struct nftnl_trace *nlt, struct cb_arg *args)
exit(EXIT_FAILURE);
}
- nlh = nftnl_chain_nlmsg_build_hdr(buf, NFT_MSG_GETRULE, family, NLM_F_DUMP, 0);
+ nlh = nftnl_chain_nlmsg_build_hdr(buf, NFT_MSG_GETRULE, family, 0, 0);
nftnl_rule_set_u32(r, NFTNL_RULE_FAMILY, family);
nftnl_rule_set_str(r, NFTNL_RULE_CHAIN, chain);
nftnl_rule_set_str(r, NFTNL_RULE_TABLE, table);
- nftnl_rule_set_u64(r, NFTNL_RULE_POSITION, handle);
+ nftnl_rule_set_u64(r, NFTNL_RULE_HANDLE, handle);
nftnl_rule_nlmsg_build_payload(nlh, r);
nftnl_rule_free(r);
@@ -248,24 +248,21 @@ static void trace_print_rule(const struct nftnl_trace *nlt, struct cb_arg *args)
}
portid = mnl_socket_get_portid(nl);
- if (mnl_socket_sendto(nl, nlh, nlh->nlmsg_len) < 0) {
- perror("mnl_socket_send");
- exit(EXIT_FAILURE);
- }
+ if (mnl_socket_sendto(nl, nlh, nlh->nlmsg_len) < 0) {
+ perror("mnl_socket_send");
+ exit(EXIT_FAILURE);
+ }
ret = mnl_socket_recvfrom(nl, buf, sizeof(buf));
- while (ret > 0) {
+ if (ret > 0) {
args->is_event = false;
- ret = mnl_cb_run(buf, ret, 0, portid, rule_cb, args);
- if (ret <= 0)
- break;
- ret = mnl_socket_recvfrom(nl, buf, sizeof(buf));
- }
- if (ret == -1) {
- perror("error");
- exit(EXIT_FAILURE);
- }
- mnl_socket_close(nl);
+ ret = mnl_cb_run(buf, ret, 0, portid, rule_cb, args);
+ }
+ if (ret == -1) {
+ perror("error");
+ exit(EXIT_FAILURE);
+ }
+ mnl_socket_close(nl);
}
static void trace_print_packet(const struct nftnl_trace *nlt, struct cb_arg *args)
@@ -531,6 +528,7 @@ static int trace_cb(const struct nlmsghdr *nlh, struct cb_arg *arg)
err_free:
nftnl_trace_free(nlt);
err:
+ fflush(stdout);
return MNL_CB_OK;
}
--
2.28.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH xtables-nft 2/3] xtables-monitor: fix packet family protocol
2020-12-12 15:15 [PATCH xtables-nft 0/3] xt-monitor fixes Florian Westphal
2020-12-12 15:15 ` [PATCH xtables-nft 1/3] xtables-monitor: fix rule printing Florian Westphal
@ 2020-12-12 15:15 ` Florian Westphal
2020-12-12 15:15 ` [PATCH xtables-nft 3/3] xtables-monitor: print packet first Florian Westphal
2 siblings, 0 replies; 7+ messages in thread
From: Florian Westphal @ 2020-12-12 15:15 UTC (permalink / raw)
To: netfilter-devel; +Cc: Florian Westphal
This prints the family passed on the command line (which might be 0).
Print the table family instead.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
iptables/xtables-monitor.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/iptables/xtables-monitor.c b/iptables/xtables-monitor.c
index 364e600e1b38..8850a12032d2 100644
--- a/iptables/xtables-monitor.c
+++ b/iptables/xtables-monitor.c
@@ -273,14 +273,14 @@ static void trace_print_packet(const struct nftnl_trace *nlt, struct cb_arg *arg
uint32_t mark;
char name[IFNAMSIZ];
- printf("PACKET: %d %08x ", args->nfproto, nftnl_trace_get_u32(nlt, NFTNL_TRACE_ID));
+ family = nftnl_trace_get_u32(nlt, NFTNL_TRACE_FAMILY);
+ printf("PACKET: %d %08x ", family, nftnl_trace_get_u32(nlt, NFTNL_TRACE_ID));
if (nftnl_trace_is_set(nlt, NFTNL_TRACE_IIF))
printf("IN=%s ", if_indextoname(nftnl_trace_get_u32(nlt, NFTNL_TRACE_IIF), name));
if (nftnl_trace_is_set(nlt, NFTNL_TRACE_OIF))
printf("OUT=%s ", if_indextoname(nftnl_trace_get_u32(nlt, NFTNL_TRACE_OIF), name));
- family = nftnl_trace_get_u32(nlt, NFTNL_TRACE_FAMILY);
nfproto = family;
if (nftnl_trace_is_set(nlt, NFTNL_TRACE_NFPROTO)) {
nfproto = nftnl_trace_get_u32(nlt, NFTNL_TRACE_NFPROTO);
--
2.28.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH xtables-nft 3/3] xtables-monitor: print packet first
2020-12-12 15:15 [PATCH xtables-nft 0/3] xt-monitor fixes Florian Westphal
2020-12-12 15:15 ` [PATCH xtables-nft 1/3] xtables-monitor: fix rule printing Florian Westphal
2020-12-12 15:15 ` [PATCH xtables-nft 2/3] xtables-monitor: fix packet family protocol Florian Westphal
@ 2020-12-12 15:15 ` Florian Westphal
2020-12-14 14:14 ` Phil Sutter
2 siblings, 1 reply; 7+ messages in thread
From: Florian Westphal @ 2020-12-12 15:15 UTC (permalink / raw)
To: netfilter-devel; +Cc: Florian Westphal
The trace mode should first print the packet that was received and
then the rule/verdict.
Furthermore, the monitor did sometimes print an extra newline.
After this patch, output is more consistent with nft monitor.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
iptables/xtables-monitor.c | 34 +++++++++++++++++++++++-----------
1 file changed, 23 insertions(+), 11 deletions(-)
diff --git a/iptables/xtables-monitor.c b/iptables/xtables-monitor.c
index 8850a12032d2..45a0d6bf1343 100644
--- a/iptables/xtables-monitor.c
+++ b/iptables/xtables-monitor.c
@@ -106,6 +106,7 @@ static int rule_cb(const struct nlmsghdr *nlh, void *data)
printf("-0 ");
break;
default:
+ puts("");
goto err_free;
}
@@ -433,9 +434,18 @@ static void trace_print_packet(const struct nftnl_trace *nlt, struct cb_arg *arg
mark = nftnl_trace_get_u32(nlt, NFTNL_TRACE_MARK);
if (mark)
printf("MARK=0x%x ", mark);
+ puts("");
+}
+
+static void trace_print_hdr(const struct nftnl_trace *nlt)
+{
+ printf(" TRACE: %d %08x %s:%s", nftnl_trace_get_u32(nlt, NFTNL_TABLE_FAMILY),
+ nftnl_trace_get_u32(nlt, NFTNL_TRACE_ID),
+ nftnl_trace_get_str(nlt, NFTNL_TRACE_TABLE),
+ nftnl_trace_get_str(nlt, NFTNL_TRACE_CHAIN));
}
-static void print_verdict(struct nftnl_trace *nlt, uint32_t verdict)
+static void print_verdict(const struct nftnl_trace *nlt, uint32_t verdict)
{
const char *chain;
@@ -496,35 +506,37 @@ static int trace_cb(const struct nlmsghdr *nlh, struct cb_arg *arg)
arg->nfproto != nftnl_trace_get_u32(nlt, NFTNL_TABLE_FAMILY))
goto err_free;
- printf(" TRACE: %d %08x %s:%s", nftnl_trace_get_u32(nlt, NFTNL_TABLE_FAMILY),
- nftnl_trace_get_u32(nlt, NFTNL_TRACE_ID),
- nftnl_trace_get_str(nlt, NFTNL_TRACE_TABLE),
- nftnl_trace_get_str(nlt, NFTNL_TRACE_CHAIN));
-
switch (nftnl_trace_get_u32(nlt, NFTNL_TRACE_TYPE)) {
case NFT_TRACETYPE_RULE:
verdict = nftnl_trace_get_u32(nlt, NFTNL_TRACE_VERDICT);
- printf(":rule:0x%llx:", (unsigned long long)nftnl_trace_get_u64(nlt, NFTNL_TRACE_RULE_HANDLE));
- print_verdict(nlt, verdict);
- if (nftnl_trace_is_set(nlt, NFTNL_TRACE_RULE_HANDLE))
- trace_print_rule(nlt, arg);
if (nftnl_trace_is_set(nlt, NFTNL_TRACE_LL_HEADER) ||
nftnl_trace_is_set(nlt, NFTNL_TRACE_NETWORK_HEADER))
trace_print_packet(nlt, arg);
+
+ if (nftnl_trace_is_set(nlt, NFTNL_TRACE_RULE_HANDLE)) {
+ trace_print_hdr(nlt);
+ printf(":rule:0x%llx:", (unsigned long long)nftnl_trace_get_u64(nlt, NFTNL_TRACE_RULE_HANDLE));
+ print_verdict(nlt, verdict);
+ printf(" ");
+ trace_print_rule(nlt, arg);
+ }
break;
case NFT_TRACETYPE_POLICY:
+ trace_print_hdr(nlt);
printf(":policy:");
verdict = nftnl_trace_get_u32(nlt, NFTNL_TRACE_POLICY);
print_verdict(nlt, verdict);
+ puts("");
break;
case NFT_TRACETYPE_RETURN:
+ trace_print_hdr(nlt);
printf(":return:");
trace_print_return(nlt);
+ puts("");
break;
}
- puts("");
err_free:
nftnl_trace_free(nlt);
err:
--
2.28.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH xtables-nft 3/3] xtables-monitor: print packet first
2020-12-12 15:15 ` [PATCH xtables-nft 3/3] xtables-monitor: print packet first Florian Westphal
@ 2020-12-14 14:14 ` Phil Sutter
2020-12-14 16:06 ` Florian Westphal
0 siblings, 1 reply; 7+ messages in thread
From: Phil Sutter @ 2020-12-14 14:14 UTC (permalink / raw)
To: Florian Westphal; +Cc: netfilter-devel
On Sat, Dec 12, 2020 at 04:15:34PM +0100, Florian Westphal wrote:
> The trace mode should first print the packet that was received and
> then the rule/verdict.
>
> Furthermore, the monitor did sometimes print an extra newline.
>
> After this patch, output is more consistent with nft monitor.
>
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
> iptables/xtables-monitor.c | 34 +++++++++++++++++++++++-----------
> 1 file changed, 23 insertions(+), 11 deletions(-)
>
> diff --git a/iptables/xtables-monitor.c b/iptables/xtables-monitor.c
> index 8850a12032d2..45a0d6bf1343 100644
> --- a/iptables/xtables-monitor.c
> +++ b/iptables/xtables-monitor.c
> @@ -106,6 +106,7 @@ static int rule_cb(const struct nlmsghdr *nlh, void *data)
> printf("-0 ");
> break;
> default:
> + puts("");
> goto err_free;
> }
>
> @@ -433,9 +434,18 @@ static void trace_print_packet(const struct nftnl_trace *nlt, struct cb_arg *arg
> mark = nftnl_trace_get_u32(nlt, NFTNL_TRACE_MARK);
> if (mark)
> printf("MARK=0x%x ", mark);
> + puts("");
> +}
> +
> +static void trace_print_hdr(const struct nftnl_trace *nlt)
> +{
> + printf(" TRACE: %d %08x %s:%s", nftnl_trace_get_u32(nlt, NFTNL_TABLE_FAMILY),
> + nftnl_trace_get_u32(nlt, NFTNL_TRACE_ID),
> + nftnl_trace_get_str(nlt, NFTNL_TRACE_TABLE),
> + nftnl_trace_get_str(nlt, NFTNL_TRACE_CHAIN));
> }
>
> -static void print_verdict(struct nftnl_trace *nlt, uint32_t verdict)
> +static void print_verdict(const struct nftnl_trace *nlt, uint32_t verdict)
> {
> const char *chain;
>
> @@ -496,35 +506,37 @@ static int trace_cb(const struct nlmsghdr *nlh, struct cb_arg *arg)
> arg->nfproto != nftnl_trace_get_u32(nlt, NFTNL_TABLE_FAMILY))
> goto err_free;
>
> - printf(" TRACE: %d %08x %s:%s", nftnl_trace_get_u32(nlt, NFTNL_TABLE_FAMILY),
> - nftnl_trace_get_u32(nlt, NFTNL_TRACE_ID),
> - nftnl_trace_get_str(nlt, NFTNL_TRACE_TABLE),
> - nftnl_trace_get_str(nlt, NFTNL_TRACE_CHAIN));
> -
> switch (nftnl_trace_get_u32(nlt, NFTNL_TRACE_TYPE)) {
> case NFT_TRACETYPE_RULE:
> verdict = nftnl_trace_get_u32(nlt, NFTNL_TRACE_VERDICT);
> - printf(":rule:0x%llx:", (unsigned long long)nftnl_trace_get_u64(nlt, NFTNL_TRACE_RULE_HANDLE));
Quite long long line here. ;)
How about using PRIx64 in the format string to avoid the cast?
> - print_verdict(nlt, verdict);
>
> - if (nftnl_trace_is_set(nlt, NFTNL_TRACE_RULE_HANDLE))
> - trace_print_rule(nlt, arg);
> if (nftnl_trace_is_set(nlt, NFTNL_TRACE_LL_HEADER) ||
> nftnl_trace_is_set(nlt, NFTNL_TRACE_NETWORK_HEADER))
> trace_print_packet(nlt, arg);
> +
> + if (nftnl_trace_is_set(nlt, NFTNL_TRACE_RULE_HANDLE)) {
> + trace_print_hdr(nlt);
> + printf(":rule:0x%llx:", (unsigned long long)nftnl_trace_get_u64(nlt, NFTNL_TRACE_RULE_HANDLE));
Same here.
Cheers, Phil
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH xtables-nft 1/3] xtables-monitor: fix rule printing
2020-12-12 15:15 ` [PATCH xtables-nft 1/3] xtables-monitor: fix rule printing Florian Westphal
@ 2020-12-14 14:19 ` Phil Sutter
0 siblings, 0 replies; 7+ messages in thread
From: Phil Sutter @ 2020-12-14 14:19 UTC (permalink / raw)
To: Florian Westphal; +Cc: netfilter-devel
On Sat, Dec 12, 2020 at 04:15:32PM +0100, Florian Westphal wrote:
> trace_print_rule does a rule dump. This prints unrelated rules
> in the same chain. Instead the function should only request the
> specific handle.
>
> Furthermore, flush output buffer afterwards so this plays nice when
> output isn't a terminal.
>
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
> iptables/xtables-monitor.c | 32 +++++++++++++++-----------------
> 1 file changed, 15 insertions(+), 17 deletions(-)
>
> diff --git a/iptables/xtables-monitor.c b/iptables/xtables-monitor.c
> index 4008cc00d469..364e600e1b38 100644
> --- a/iptables/xtables-monitor.c
> +++ b/iptables/xtables-monitor.c
> @@ -227,12 +227,12 @@ static void trace_print_rule(const struct nftnl_trace *nlt, struct cb_arg *args)
> exit(EXIT_FAILURE);
> }
>
> - nlh = nftnl_chain_nlmsg_build_hdr(buf, NFT_MSG_GETRULE, family, NLM_F_DUMP, 0);
> + nlh = nftnl_chain_nlmsg_build_hdr(buf, NFT_MSG_GETRULE, family, 0, 0);
>
> nftnl_rule_set_u32(r, NFTNL_RULE_FAMILY, family);
> nftnl_rule_set_str(r, NFTNL_RULE_CHAIN, chain);
> nftnl_rule_set_str(r, NFTNL_RULE_TABLE, table);
> - nftnl_rule_set_u64(r, NFTNL_RULE_POSITION, handle);
> + nftnl_rule_set_u64(r, NFTNL_RULE_HANDLE, handle);
> nftnl_rule_nlmsg_build_payload(nlh, r);
> nftnl_rule_free(r);
>
> @@ -248,24 +248,21 @@ static void trace_print_rule(const struct nftnl_trace *nlt, struct cb_arg *args)
> }
>
> portid = mnl_socket_get_portid(nl);
> - if (mnl_socket_sendto(nl, nlh, nlh->nlmsg_len) < 0) {
> - perror("mnl_socket_send");
> - exit(EXIT_FAILURE);
> - }
> + if (mnl_socket_sendto(nl, nlh, nlh->nlmsg_len) < 0) {
> + perror("mnl_socket_send");
> + exit(EXIT_FAILURE);
> + }
Just in case someone else wonders as well: This does a whitespace
cleanup, replacing spaces by tabs. Later changes contain the same
cleanup, too.
Cheers, Phil
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH xtables-nft 3/3] xtables-monitor: print packet first
2020-12-14 14:14 ` Phil Sutter
@ 2020-12-14 16:06 ` Florian Westphal
0 siblings, 0 replies; 7+ messages in thread
From: Florian Westphal @ 2020-12-14 16:06 UTC (permalink / raw)
To: Phil Sutter, Florian Westphal, netfilter-devel
Phil Sutter <phil@nwl.cc> wrote:
> > switch (nftnl_trace_get_u32(nlt, NFTNL_TRACE_TYPE)) {
> > case NFT_TRACETYPE_RULE:
> > verdict = nftnl_trace_get_u32(nlt, NFTNL_TRACE_VERDICT);
> > - printf(":rule:0x%llx:", (unsigned long long)nftnl_trace_get_u64(nlt, NFTNL_TRACE_RULE_HANDLE));
>
> Quite long long line here. ;)
> How about using PRIx64 in the format string to avoid the cast?
Its just being moved from here...
> > - print_verdict(nlt, verdict);
> >
> > - if (nftnl_trace_is_set(nlt, NFTNL_TRACE_RULE_HANDLE))
> > - trace_print_rule(nlt, arg);
> > if (nftnl_trace_is_set(nlt, NFTNL_TRACE_LL_HEADER) ||
> > nftnl_trace_is_set(nlt, NFTNL_TRACE_NETWORK_HEADER))
> > trace_print_packet(nlt, arg);
> > +
> > + if (nftnl_trace_is_set(nlt, NFTNL_TRACE_RULE_HANDLE)) {
> > + trace_print_hdr(nlt);
> > + printf(":rule:0x%llx:", (unsigned long long)nftnl_trace_get_u64(nlt, NFTNL_TRACE_RULE_HANDLE));
To this location. But sure, I can change it.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-12-14 16:07 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-12 15:15 [PATCH xtables-nft 0/3] xt-monitor fixes Florian Westphal
2020-12-12 15:15 ` [PATCH xtables-nft 1/3] xtables-monitor: fix rule printing Florian Westphal
2020-12-14 14:19 ` Phil Sutter
2020-12-12 15:15 ` [PATCH xtables-nft 2/3] xtables-monitor: fix packet family protocol Florian Westphal
2020-12-12 15:15 ` [PATCH xtables-nft 3/3] xtables-monitor: print packet first Florian Westphal
2020-12-14 14:14 ` Phil Sutter
2020-12-14 16:06 ` Florian Westphal
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.