utils: fix arp family number

NFPROTO_ARP = 3 in kernel space.

We need the same value here in userspace in order to correctly communicate with the kernel.

The failure solved by this patch made that {XML|JSON}-parsed tables of ARP family unable to be directly injected into kernel.

To prevent future errors, this patch changes raw and AF_* values by the mathing NFPROTO_* couterpart as seen in linux/netfilter.h in both functions:
* nft_family2str()
* nft_str2family()

---
v2: rework+fix using the array-matching approach suggested by Pablo.
v3: constify both the pointer and the data, suggested by Jan.
Keep setting errno to EAFNOSUPPORT in nft_str2family().

 src/utils.c |   43 ++++++++++++++++++++-----------------------
 1 file changed, 20 insertions(+), 23 deletions(-)

index d70fbf1..9013b68 100644
@@ -20,36 +20,33 @@
 #include <linux/netfilter.h>
 #include <linux/netfilter/nf_tables.h>
 
+static const char *const nft_family_str[NFPROTO_NUMPROTO] = {
+	[NFPROTO_INET]		= "inet",
+	[NFPROTO_IPV4]		= "ip",
+	[NFPROTO_ARP]		= "arp",
+	[NFPROTO_BRIDGE]	= "bridge",
+	[NFPROTO_IPV6]		= "ip6",
+};
+
 const char *nft_family2str(uint32_t family)
 {
-	switch (family) {
-	case AF_INET:
-		return "ip";
-	case AF_INET6:
-		return "ip6";
-	case 1:
-		return "inet";
-	case AF_BRIDGE:
-		return "bridge";
-	case 3: /* NFPROTO_ARP */
-		return "arp";
-	default:
+	if (nft_family_str[family] == NULL)
 		return "unknown";
-	}
+
+	return nft_family_str[family];
 }
 
 int nft_str2family(const char *family)
 {
-	if (strcmp(family, "ip") == 0)
-		return AF_INET;
-	else if (strcmp(family, "ip6") == 0)
-		return AF_INET6;
-	else if (strcmp(family, "inet") == 0)
-		return 1;
-	else if (strcmp(family, "bridge") == 0)
-		return AF_BRIDGE;
-	else if (strcmp(family, "arp") == 0)
-		return 0;
+	int i;
+
+	for (i = 0; i < NFPROTO_NUMPROTO; i++) {
+		if (nft_family_str[i] == NULL)
+			continue;
+
+		if (strcmp(nft_family_str[i], family) == 0)
+			return i;
+	}
 
 	errno = EAFNOSUPPORT;
 	return -1;

Applied, thanks.

BTW, it would be good to see a similar refactor in nft_verdict2str().

I don't see a clean way to do it, given some verdicts are negative numbers (enum nft_verdicts in nf_tables.h).
We may end accessing a negative index, out of bounds of the array.

Arturo Borrero González
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

I see, you mean:

enum nft_verdicts {

        NFT_CONTINUE    = -1,
        NFT_BREAK       = -2,
        NFT_JUMP        = -3,
        NFT_GOTO        = -4,
        NFT_RETURN      = -5,

};

You can add some function to shift the values:

#define nft_verdict_index(base) (base + 5)

... nft_verdict_array[] = {
[nft_verdict_index(NFT_RETURN)] = "return",
...
};

BTW, instead of 5, add:

#define NFT_VERDICT_BASE NFT_RETURN

and use it.

Ok, following your idea, I end with something like this:

/*

 * NF_DROP      = 0
 * NF_ACCEPT    = 1
 * NFT_JUMP     = -3
 * NFT_GOTO     = -4
 * NFT_RETURN   = -5

*/

#define NFT_VERDICT_BASE                -NFT_RETURN
#define nft_verdict_index(base)         (base + NFT_VERDICT_BASE)
#define NFT_VERDICT_ARRAY_LEN           nft_verdict_index(NF_ACCEPT)

static const char *const nft_verdict_str[NFT_VERDICT_ARRAY_LEN + 1] = {
        [nft_verdict_index(NFT_RETURN)] =       "return",       /* 0 */
        [nft_verdict_index(NFT_GOTO)]   =       "goto",         /* 1 */
        [nft_verdict_index(NFT_JUMP)]   =       "jump",         /* 2 */
        [nft_verdict_index(NF_DROP)]    =       "drop",         /* 5 */
        [nft_verdict_index(NF_ACCEPT)]  =       "accept",       /* 6 */
};

const char *nft_verdict2str(int verdict)

{
        if (nft_verdict_str[nft_verdict_index(verdict)] == NULL)
                return "unknown";

        return nft_verdict_str[nft_verdict_index(verdict)];
}

int nft_str2verdict(const char *verdict, int *verdict_num)

{
        int i;

        for (i = 0; i < NFT_VERDICT_ARRAY_LEN; i++) {
                if (nft_verdict_str[i] == NULL)
                        continue;

                if (strcmp(nft_verdict_str[i], verdict) == 0) {
                        *verdict_num = nft_verdict_index(i);
                        return 0;
                }
        }

        return -1;
}

I think the current code (the switch based one) is better, don't you? That array seems very error prone.

Arturo Borrero González
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html