[libnftnl RFC] src: Do not include userdata content in debug output

[libnftnl RFC] src: Do not include userdata content in debug output

[Phil Sutter]

This storage in rules and set elements is opaque by design, neither
libnftnl nor kernel should deal with its content. Yet nftables enters data
in host byte order which will lead to changing output depending on
host's byte order. Avoid this problem for test suites checking the debug
output by simply not printing userdata content. Merely print how much
storage is used if at all.

If this is acceptable, commit f20dfa7824860 ("udata: Store u32 udata
values in Big Endian") may be reverted.

There is surprisingly little adjustment needed to this in test suites,
BTW. In nftables, there is merely tests/py/ip6/srh.t.payload which
tracks set element userdata. So while this fix is a bit clumsy, its
impact is not too big at least.

Suggested-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 src/rule.c     | 18 +++---------------
 src/set_elem.c | 17 +++--------------
 2 files changed, 6 insertions(+), 29 deletions(-)

diff --git a/src/rule.c b/src/rule.c
index cd3041e5a399a..0d5496e8ad813 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -509,8 +509,8 @@ static int nftnl_rule_snprintf_default(char *buf, size_t remain,
 				       uint32_t type, uint32_t flags)
 {
 	struct nftnl_expr *expr;
-	int ret, offset = 0, i;
 	const char *sep = "";
+	int ret, offset = 0;
 
 	if (r->flags & (1 << NFTNL_RULE_FAMILY)) {
 		ret = snprintf(buf + offset, remain, "%s%s", sep,
@@ -573,21 +573,9 @@ static int nftnl_rule_snprintf_default(char *buf, size_t remain,
 	}
 
 	if (r->user.len) {
-		ret = snprintf(buf + offset, remain, "\n  userdata = { ");
-		SNPRINTF_BUFFER_SIZE(ret, remain, offset);
-
-		for (i = 0; i < r->user.len; i++) {
-			char *c = r->user.data;
-
-			ret = snprintf(buf + offset, remain,
-				       isprint(c[i]) ? "%c" : "\\x%02hhx",
-				       c[i]);
-			SNPRINTF_BUFFER_SIZE(ret, remain, offset);
-		}
-
-		ret = snprintf(buf + offset, remain, " }");
+		ret = snprintf(buf + offset, remain,
+			       "\n  userdata len %d", r->user.len);
 		SNPRINTF_BUFFER_SIZE(ret, remain, offset);
-
 	}
 
 	return offset;
diff --git a/src/set_elem.c b/src/set_elem.c
index d22643c44dd71..68f8d4f41dac9 100644
--- a/src/set_elem.c
+++ b/src/set_elem.c
@@ -705,7 +705,7 @@ int nftnl_set_elem_parse_file(struct nftnl_set_elem *e, enum nftnl_parse_type ty
 int nftnl_set_elem_snprintf_default(char *buf, size_t remain,
 				    const struct nftnl_set_elem *e)
 {
-	int ret, dregtype = DATA_NONE, offset = 0, i;
+	int ret, dregtype = DATA_NONE, offset = 0;
 
 	ret = snprintf(buf, remain, "element ");
 	SNPRINTF_BUFFER_SIZE(ret, remain, offset);
@@ -748,19 +748,8 @@ int nftnl_set_elem_snprintf_default(char *buf, size_t remain,
 	}
 
 	if (e->user.len) {
-		ret = snprintf(buf + offset, remain, "  userdata = { ");
-		SNPRINTF_BUFFER_SIZE(ret, remain, offset);
-
-		for (i = 0; i < e->user.len; i++) {
-			char *c = e->user.data;
-
-			ret = snprintf(buf + offset, remain,
-				       isprint(c[i]) ? "%c" : "\\x%02hhx",
-				       c[i]);
-			SNPRINTF_BUFFER_SIZE(ret, remain, offset);
-		}
-
-		ret = snprintf(buf + offset, remain, " }");
+		ret = snprintf(buf + offset, remain,
+			       "  userdata len %d", e->user.len);
 		SNPRINTF_BUFFER_SIZE(ret, remain, offset);
 	}
 
-- 
2.51.0

Re: [libnftnl RFC] src: Do not include userdata content in debug output

[Florian Westphal]

Phil Sutter <phil@nwl.cc> wrote:
> This storage in rules and set elements is opaque by design, neither
> libnftnl nor kernel should deal with its content. Yet nftables enters data
> in host byte order which will lead to changing output depending on
> host's byte order. Avoid this problem for test suites checking the debug
> output by simply not printing userdata content. Merely print how much
> storage is used if at all.
> 
> If this is acceptable, commit f20dfa7824860 ("udata: Store u32 udata
> values in Big Endian") may be reverted.

Thanks Phil for following up.

> There is surprisingly little adjustment needed to this in test suites,
> BTW. In nftables, there is merely tests/py/ip6/srh.t.payload which
> tracks set element userdata. So while this fix is a bit clumsy, its
> impact is not too big at least.

The udata is used to store the exthdr flavour (here srh) for printing,
so in case this would ever be corrupted. the printed rule would fail
test validation as well.  IOW, I think we can do without libnftnl
dumping the udata stashed byte soup without compromising bigendian
tests.

My s390x vm passes both pyton and shell tests at this time, this was
never the case before.  Thanks a lot for making this happen.