[nft PATCH] datatype: rt_symbol_table_init() to search for iproute2 configs

[nft PATCH] datatype: rt_symbol_table_init() to search for iproute2 configs

[Phil Sutter]

There is an ongoing effort among various distributions to tidy up in
/etc. The idea is to reduce contents to just what the admin manually
inserted to customize the system, anything else shall move out to /usr
(or so). The various files in /etc/iproute2 fall in that category as
they are seldomly modified.

The crux is though that iproute2 project seems not quite sure yet where
the files should go. While v6.6.0 installs them into /usr/lib/iproute2,
current mast^Wmain branch uses /usr/share/iproute2. Assume this is going
to stay as /(usr/)lib does not seem right for such files.

Note that rt_symbol_table_init() is not just used for
iproute2-maintained configs but also for connlabel.conf - so retain the
old behaviour when passed an absolute path.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 src/datatype.c | 38 ++++++++++++++++++++++++++++++++++----
 src/meta.c     |  2 +-
 src/rt.c       |  2 +-
 3 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/src/datatype.c b/src/datatype.c
index 86d55a5242694..9ca0516700f81 100644
--- a/src/datatype.c
+++ b/src/datatype.c
@@ -855,19 +855,47 @@ const struct datatype inet_service_type = {
 
 #define RT_SYM_TAB_INITIAL_SIZE		16
 
+static FILE *open_iproute2_db(const char *filename, char **path)
+{
+	FILE *ret;
+
+	if (filename[0] == '/')
+		return fopen(filename, "r");
+
+	if (asprintf(path, "/etc/iproute2/%s", filename) == -1)
+		goto fail;
+
+	ret = fopen(*path, "r");
+	if (ret)
+		return ret;
+
+	free(*path);
+	if (asprintf(path, "/usr/share/iproute2/%s", filename) == -1)
+		goto fail;
+
+	ret = fopen(*path, "r");
+	if (ret)
+		return ret;
+
+	free(*path);
+fail:
+	*path = NULL;
+	return NULL;
+}
+
 struct symbol_table *rt_symbol_table_init(const char *filename)
 {
+	char buf[512], namebuf[512], *p, *path = NULL;
 	struct symbolic_constant s;
 	struct symbol_table *tbl;
 	unsigned int size, nelems, val;
-	char buf[512], namebuf[512], *p;
 	FILE *f;
 
 	size = RT_SYM_TAB_INITIAL_SIZE;
 	tbl = xmalloc(sizeof(*tbl) + size * sizeof(s));
 	nelems = 0;
 
-	f = fopen(filename, "r");
+	f = open_iproute2_db(filename, &path);
 	if (f == NULL)
 		goto out;
 
@@ -882,7 +910,7 @@ struct symbol_table *rt_symbol_table_init(const char *filename)
 		    sscanf(p, "%u %511s\n", &val, namebuf) != 2 &&
 		    sscanf(p, "%u %511s #", &val, namebuf) != 2) {
 			fprintf(stderr, "iproute database '%s' corrupted\n",
-				filename);
+				path ?: filename);
 			break;
 		}
 
@@ -899,6 +927,8 @@ struct symbol_table *rt_symbol_table_init(const char *filename)
 
 	fclose(f);
 out:
+	if (path)
+		free(path);
 	tbl->symbols[nelems] = SYMBOL_LIST_END;
 	return tbl;
 }
@@ -914,7 +944,7 @@ void rt_symbol_table_free(const struct symbol_table *tbl)
 
 void mark_table_init(struct nft_ctx *ctx)
 {
-	ctx->output.tbl.mark = rt_symbol_table_init("/etc/iproute2/rt_marks");
+	ctx->output.tbl.mark = rt_symbol_table_init("rt_marks");
 }
 
 void mark_table_exit(struct nft_ctx *ctx)
diff --git a/src/meta.c b/src/meta.c
index 8d0b7aae96292..6f76f0033a630 100644
--- a/src/meta.c
+++ b/src/meta.c
@@ -325,7 +325,7 @@ const struct datatype pkttype_type = {
 
 void devgroup_table_init(struct nft_ctx *ctx)
 {
-	ctx->output.tbl.devgroup = rt_symbol_table_init("/etc/iproute2/group");
+	ctx->output.tbl.devgroup = rt_symbol_table_init("group");
 }
 
 void devgroup_table_exit(struct nft_ctx *ctx)
diff --git a/src/rt.c b/src/rt.c
index f5c80559ffeef..3ee710ddc05b5 100644
--- a/src/rt.c
+++ b/src/rt.c
@@ -25,7 +25,7 @@
 
 void realm_table_rt_init(struct nft_ctx *ctx)
 {
-	ctx->output.tbl.realm = rt_symbol_table_init("/etc/iproute2/rt_realms");
+	ctx->output.tbl.realm = rt_symbol_table_init("rt_realms");
 }
 
 void realm_table_rt_exit(struct nft_ctx *ctx)
-- 
2.43.0

Re: [nft PATCH] datatype: rt_symbol_table_init() to search for iproute2 configs

[Pablo Neira Ayuso]

Hi Phil,

On Fri, Dec 15, 2023 at 10:19:33PM +0100, Phil Sutter wrote:
> There is an ongoing effort among various distributions to tidy up in
> /etc. The idea is to reduce contents to just what the admin manually
> inserted to customize the system, anything else shall move out to /usr
> (or so). The various files in /etc/iproute2 fall in that category as
> they are seldomly modified.
> 
> The crux is though that iproute2 project seems not quite sure yet where
> the files should go. While v6.6.0 installs them into /usr/lib/iproute2,
> current mast^Wmain branch uses /usr/share/iproute2. Assume this is going
> to stay as /(usr/)lib does not seem right for such files.
> 
> Note that rt_symbol_table_init() is not just used for
> iproute2-maintained configs but also for connlabel.conf - so retain the
> old behaviour when passed an absolute path.

Fine with me. This defines a fallback which is backward compatible.

Re: [nft PATCH] datatype: rt_symbol_table_init() to search for iproute2 configs

[Pablo Neira Ayuso]

On Fri, Dec 22, 2023 at 01:04:35PM +0100, Pablo Neira Ayuso wrote:
> Hi Phil,
> 
> On Fri, Dec 15, 2023 at 10:19:33PM +0100, Phil Sutter wrote:
> > There is an ongoing effort among various distributions to tidy up in
> > /etc. The idea is to reduce contents to just what the admin manually
> > inserted to customize the system, anything else shall move out to /usr
> > (or so). The various files in /etc/iproute2 fall in that category as
> > they are seldomly modified.
> > 
> > The crux is though that iproute2 project seems not quite sure yet where
> > the files should go. While v6.6.0 installs them into /usr/lib/iproute2,
> > current mast^Wmain branch uses /usr/share/iproute2. Assume this is going
> > to stay as /(usr/)lib does not seem right for such files.
> > 
> > Note that rt_symbol_table_init() is not just used for
> > iproute2-maintained configs but also for connlabel.conf - so retain the
> > old behaviour when passed an absolute path.
> 
> Fine with me. This defines a fallback which is backward compatible.

As an addedum, probably expose these definitions in nft describe? So
users don't have to do strace to guess this. Also display from what
file this is taken:

# nft describe rt classid
rt expression, datatype realm (routing realm) (basetype integer), 32 bits

shows nothing.

[nft PATCH 1/2] datatype: Initialize rt_symbol_tables' base field

[Phil Sutter]

It is unconditionally accessed in symbol_table_print() so make sure it
is initialized to either BASE_DECIMAL (arbitrary) for empty or
non-existent source files or a proper value depending on entry number
format.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 src/datatype.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/src/datatype.c b/src/datatype.c
index 9ca0516700f81..4d867798222be 100644
--- a/src/datatype.c
+++ b/src/datatype.c
@@ -893,6 +893,7 @@ struct symbol_table *rt_symbol_table_init(const char *filename)
 
 	size = RT_SYM_TAB_INITIAL_SIZE;
 	tbl = xmalloc(sizeof(*tbl) + size * sizeof(s));
+	tbl->base = BASE_DECIMAL;
 	nelems = 0;
 
 	f = open_iproute2_db(filename, &path);
@@ -905,10 +906,13 @@ struct symbol_table *rt_symbol_table_init(const char *filename)
 			p++;
 		if (*p == '#' || *p == '\n' || *p == '\0')
 			continue;
-		if (sscanf(p, "0x%x %511s\n", &val, namebuf) != 2 &&
-		    sscanf(p, "0x%x %511s #", &val, namebuf) != 2 &&
-		    sscanf(p, "%u %511s\n", &val, namebuf) != 2 &&
-		    sscanf(p, "%u %511s #", &val, namebuf) != 2) {
+		if (sscanf(p, "0x%x %511s\n", &val, namebuf) == 2 ||
+		    sscanf(p, "0x%x %511s #", &val, namebuf) == 2) {
+			tbl->base = BASE_HEXADECIMAL;
+		} else if (sscanf(p, "%u %511s\n", &val, namebuf) == 2 ||
+			   sscanf(p, "%u %511s #", &val, namebuf) == 2) {
+			tbl->base = BASE_DECIMAL;
+		} else {
 			fprintf(stderr, "iproute database '%s' corrupted\n",
 				path ?: filename);
 			break;
-- 
2.43.0

[nft PATCH 2/2] datatype: Describe rt symbol tables

[Phil Sutter]

Implement a symbol_table_print() wrapper for the run-time populated
rt_symbol_tables which formats output similar to expr_describe() and
includes the data source.

Since these tables reside in struct output_ctx there is no implicit
connection between data type and therefore providing callbacks for
relevant datat types which feed the data into said wrapper is a simpler
solution than extending expr_describe() itself.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 include/datatype.h |  3 +++
 src/ct.c           |  7 +++++++
 src/datatype.c     | 34 ++++++++++++++++++++++++++++++++++
 src/meta.c         |  7 +++++++
 src/rt.c           |  7 +++++++
 5 files changed, 58 insertions(+)

diff --git a/include/datatype.h b/include/datatype.h
index 09a7894567e4d..c4d6282d6f591 100644
--- a/include/datatype.h
+++ b/include/datatype.h
@@ -252,6 +252,9 @@ extern void symbol_table_print(const struct symbol_table *tbl,
 
 extern struct symbol_table *rt_symbol_table_init(const char *filename);
 extern void rt_symbol_table_free(const struct symbol_table *tbl);
+extern void rt_symbol_table_describe(struct output_ctx *octx, const char *name,
+				     const struct symbol_table *tbl,
+				     const struct datatype *type);
 
 extern const struct datatype invalid_type;
 extern const struct datatype verdict_type;
diff --git a/src/ct.c b/src/ct.c
index ebfd90a1ab0d3..6793464859cad 100644
--- a/src/ct.c
+++ b/src/ct.c
@@ -216,10 +216,17 @@ static struct error_record *ct_label_type_parse(struct parse_ctx *ctx,
 	return NULL;
 }
 
+static void ct_label_type_describe(struct output_ctx *octx)
+{
+	rt_symbol_table_describe(octx, CONNLABEL_CONF,
+				 octx->tbl.ct_label, &ct_label_type);
+}
+
 const struct datatype ct_label_type = {
 	.type		= TYPE_CT_LABEL,
 	.name		= "ct_label",
 	.desc		= "conntrack label",
+	.describe	= ct_label_type_describe,
 	.byteorder	= BYTEORDER_HOST_ENDIAN,
 	.size		= CT_LABEL_BIT_SIZE,
 	.basetype	= &bitmask_type,
diff --git a/src/datatype.c b/src/datatype.c
index 4d867798222be..3b19ae8ef52d8 100644
--- a/src/datatype.c
+++ b/src/datatype.c
@@ -946,6 +946,33 @@ void rt_symbol_table_free(const struct symbol_table *tbl)
 	free_const(tbl);
 }
 
+void rt_symbol_table_describe(struct output_ctx *octx, const char *name,
+			      const struct symbol_table *tbl,
+			      const struct datatype *type)
+{
+	char *path = NULL;
+	FILE *f;
+
+	if (!tbl || !tbl->symbols[0].identifier)
+		return;
+
+	f = open_iproute2_db(name, &path);
+	if (f)
+		fclose(f);
+	if (!path && asprintf(&path, "%s%s",
+			      name[0] == '/' ? "" : "unknown location of ",
+			      name) < 0)
+		return;
+
+	nft_print(octx, "\npre-defined symbolic constants from %s ", path);
+	if (tbl->base == BASE_DECIMAL)
+		nft_print(octx, "(in decimal):\n");
+	else
+		nft_print(octx, "(in hexadecimal):\n");
+	symbol_table_print(tbl, type, type->byteorder, octx);
+	free(path);
+}
+
 void mark_table_init(struct nft_ctx *ctx)
 {
 	ctx->output.tbl.mark = rt_symbol_table_init("rt_marks");
@@ -968,10 +995,17 @@ static struct error_record *mark_type_parse(struct parse_ctx *ctx,
 	return symbolic_constant_parse(ctx, sym, ctx->tbl->mark, res);
 }
 
+static void mark_type_describe(struct output_ctx *octx)
+{
+	rt_symbol_table_describe(octx, "rt_marks",
+				 octx->tbl.mark, &mark_type);
+}
+
 const struct datatype mark_type = {
 	.type		= TYPE_MARK,
 	.name		= "mark",
 	.desc		= "packet mark",
+	.describe	= mark_type_describe,
 	.size		= 4 * BITS_PER_BYTE,
 	.byteorder	= BYTEORDER_HOST_ENDIAN,
 	.basetype	= &integer_type,
diff --git a/src/meta.c b/src/meta.c
index 6f76f0033a630..eca8dac72098a 100644
--- a/src/meta.c
+++ b/src/meta.c
@@ -346,10 +346,17 @@ static struct error_record *devgroup_type_parse(struct parse_ctx *ctx,
 	return symbolic_constant_parse(ctx, sym, ctx->tbl->devgroup, res);
 }
 
+static void devgroup_type_describe(struct output_ctx *octx)
+{
+	rt_symbol_table_describe(octx, "group",
+				 octx->tbl.devgroup, &devgroup_type);
+}
+
 const struct datatype devgroup_type = {
 	.type		= TYPE_DEVGROUP,
 	.name		= "devgroup",
 	.desc		= "devgroup name",
+	.describe	= devgroup_type_describe,
 	.byteorder	= BYTEORDER_HOST_ENDIAN,
 	.size		= 4 * BITS_PER_BYTE,
 	.basetype	= &integer_type,
diff --git a/src/rt.c b/src/rt.c
index 3ee710ddc05b5..d8f3352f4b4a7 100644
--- a/src/rt.c
+++ b/src/rt.c
@@ -45,10 +45,17 @@ static struct error_record *realm_type_parse(struct parse_ctx *ctx,
 	return symbolic_constant_parse(ctx, sym, ctx->tbl->realm, res);
 }
 
+static void realm_type_describe(struct output_ctx *octx)
+{
+	rt_symbol_table_describe(octx, "rt_realms",
+				 octx->tbl.realm, &realm_type);
+}
+
 const struct datatype realm_type = {
 	.type		= TYPE_REALM,
 	.name		= "realm",
 	.desc		= "routing realm",
+	.describe	= realm_type_describe,
 	.byteorder	= BYTEORDER_HOST_ENDIAN,
 	.size		= 4 * BITS_PER_BYTE,
 	.basetype	= &integer_type,
-- 
2.43.0

Re: [nft PATCH] datatype: rt_symbol_table_init() to search for iproute2 configs

[Phil Sutter]

On Fri, Dec 15, 2023 at 10:19:33PM +0100, Phil Sutter wrote:
> There is an ongoing effort among various distributions to tidy up in
> /etc. The idea is to reduce contents to just what the admin manually
> inserted to customize the system, anything else shall move out to /usr
> (or so). The various files in /etc/iproute2 fall in that category as
> they are seldomly modified.
> 
> The crux is though that iproute2 project seems not quite sure yet where
> the files should go. While v6.6.0 installs them into /usr/lib/iproute2,
> current mast^Wmain branch uses /usr/share/iproute2. Assume this is going
> to stay as /(usr/)lib does not seem right for such files.
> 
> Note that rt_symbol_table_init() is not just used for
> iproute2-maintained configs but also for connlabel.conf - so retain the
> old behaviour when passed an absolute path.
> 
> Signed-off-by: Phil Sutter <phil@nwl.cc>

Patch applied along with the two-patch follow-up series.