[PATCH libnetfilter_log 0/2] utils: nfulnl_test: Test nflog_get_packet_hw

[PATCH libnetfilter_log 0/2] utils: nfulnl_test: Test nflog_get_packet_hw

[Duncan Roe]

Hi Pablo,

This patchset extends utils/nfulnl_test.c to fully exercise
nflog_get_packet_hw(). I made these changes while researching how the doxygen
for nflog_get_packet_hw should read.

I was rather surprised to find the hw_addrlen field in struct
nfulnl_msg_packet_hw is in Network Byte Order, seeing as how it doesn't come off
the wire.

Is this a bug? - it's been that way for at least 5 years (Linux 4.4.14).

I'll document on the basis this is correct behaviour for now.

Cheers ... Duncan.

 --------------
Sample output:

> 17:30:51# /home/dunc/tests/netfilter/nfulnl_test/nfulnl_test
> unbinding existing nf_log handler for AF_INET (if any)
> binding nfnetlink_log to AF_INET
> binding this socket to group 0
> binding this socket to group 100
> setting copy_packet mode
> registering callback for group 0
> going into main loop
> pkt received (len=184)
> hw_addrlen = 6 (after htons)
> HW addr: 00:00:00:00:00:00
> hw_protocol=0x86dd hook=1 mark=0 indev=1 prefix="Hi there Tiger" payload_len=52
> pkt received (len=136)
> No struct nfulnl_msg_packet_hw returned
> hw_protocol=0x86dd hook=3 mark=0 outdev=2 prefix="What's new Pussycat?" payload_len=52
> pkt received (len=196)
> hw_addrlen = 6 (after htons)
> HW addr: 18:60:24:bb:02:d6
> hw_protocol=0x86dd hook=1 mark=0 indev=2 prefix="Hello World" payload_len=52
> ^C
 "Hi there Tiger"       - locally generated UDP6 for interface lo
 "What's new Pussycat?" - locally generated UDP6 for interface eth1
 "Hello World"          - remotely generated UDP6 on interface eth1

Duncan Roe (2):
  utils: nfulnl_test: Agree with man pages
  utils: nfulnl_test: Add rather extensive test of nflog_get_packet_hw()

 utils/nfulnl_test.c | 39 ++++++++++++++++++++++++++++++++-------
 1 file changed, 32 insertions(+), 7 deletions(-)

-- 
2.17.5

[PATCH libnetfilter_log 1/2] utils: nfulnl_test: Agree with man pages

[Duncan Roe]

Use the same variable name as the man pages / html for functions in the Parsing
module (e.g. nflog_get_msg_packet_hdr(nfad)).

Rationale: make it easier for users to follow the code
Signed-off-by: Duncan Roe <duncan_roe@optusnet.com.au>
---
 utils/nfulnl_test.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/utils/nfulnl_test.c b/utils/nfulnl_test.c
index 05ddc6c..237adc3 100644
--- a/utils/nfulnl_test.c
+++ b/utils/nfulnl_test.c
@@ -6,15 +6,15 @@
 
 #include <libnetfilter_log/libnetfilter_log.h>
 
-static int print_pkt(struct nflog_data *ldata)
+static int print_pkt(struct nflog_data *nfad)
 {
-	struct nfulnl_msg_packet_hdr *ph = nflog_get_msg_packet_hdr(ldata);
-	uint32_t mark = nflog_get_nfmark(ldata);
-	uint32_t indev = nflog_get_indev(ldata);
-	uint32_t outdev = nflog_get_outdev(ldata);
-	char *prefix = nflog_get_prefix(ldata);
+	struct nfulnl_msg_packet_hdr *ph = nflog_get_msg_packet_hdr(nfad);
+	uint32_t mark = nflog_get_nfmark(nfad);
+	uint32_t indev = nflog_get_indev(nfad);
+	uint32_t outdev = nflog_get_outdev(nfad);
+	char *prefix = nflog_get_prefix(nfad);
 	char *payload;
-	int payload_len = nflog_get_payload(ldata, &payload);
+	int payload_len = nflog_get_payload(nfad, &payload);
 
 	if (ph) {
 		printf("hw_protocol=0x%04x hook=%u ",
-- 
2.17.5

[PATCH libnetfilter_log 2/2] utils: nfulnl_test: Add rather extensive test of nflog_get_packet_hw()

[Duncan Roe]

Wasn't expecting length in NBO, so code caters for it being either way

Signed-off-by: Duncan Roe <duncan_roe@optusnet.com.au>
---
 utils/nfulnl_test.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/utils/nfulnl_test.c b/utils/nfulnl_test.c
index 237adc3..a07c100 100644
--- a/utils/nfulnl_test.c
+++ b/utils/nfulnl_test.c
@@ -15,6 +15,31 @@ static int print_pkt(struct nflog_data *nfad)
 	char *prefix = nflog_get_prefix(nfad);
 	char *payload;
 	int payload_len = nflog_get_payload(nfad, &payload);
+	struct nfulnl_msg_packet_hw *hw = nflog_get_packet_hw(nfad);
+
+	if (!hw)
+		puts("No struct nfulnl_msg_packet_hw returned");
+	else {
+		char *p = "";
+		int i;
+
+		if (hw->hw_addrlen > sizeof hw->hw_addr) {
+			i = htons(hw->hw_addrlen);
+			if (i <= sizeof hw->hw_addr) {
+				hw->hw_addrlen = i;
+				p = " (after htons)";
+			}
+		}
+		printf("hw_addrlen = %d%s\n", hw->hw_addrlen, p);
+		if (i && i <= sizeof hw->hw_addr) {
+			uint8_t *u = hw->hw_addr;
+
+			fputs("HW addr: ", stdout);
+			for (i--; i >=0; i--, u++)
+				printf("%02x%s", *u, i ? ":" : "");
+			puts("");
+		}
+	}
 
 	if (ph) {
 		printf("hw_protocol=0x%04x hook=%u ",
-- 
2.17.5

Re: [PATCH libnetfilter_log 1/2] utils: nfulnl_test: Agree with man pages

[Pablo Neira Ayuso]

On Tue, Sep 21, 2021 at 06:53:14PM +1000, Duncan Roe wrote:
> Use the same variable name as the man pages / html for functions in the Parsing
> module (e.g. nflog_get_msg_packet_hdr(nfad)).

Applied, thanks.

Re: [PATCH libnetfilter_log 2/2] utils: nfulnl_test: Add rather extensive test of nflog_get_packet_hw()

[Pablo Neira Ayuso]

On Tue, Sep 21, 2021 at 06:53:15PM +1000, Duncan Roe wrote:
> Wasn't expecting length in NBO, so code caters for it being either way

For the record: I have written a different patch and push it out.