[RFC] libnfnetlink and iface conversion to string

[RFC] libnfnetlink and iface conversion to string

[Eric Leblond]

[-- Attachment #1: Type: text/plain, Size: 757 bytes --]

Hi,

I'm currently working on NuFW where I try to do iface conversion from
integer to string to be able to filter "naturally" on interfaces.

For now, it is impossible to do this conversion from either
libnetfilter_queue or libnfnetlink. But there is two files in
libnfnetlink (iftable.c and rntl.c) which have been put here by Harald
to provide this conversion (if I remember correctly an old mail from
Harald).

It was a long time ago and I don't know if it still planned to modify
libnfnetlink to include this feature. Or, would it be a better choice to
create a new library dedicated to this conversion.

Any comments are welcome.

I will start to work on it as soon as I know where to go.

BR,
-- 
Eric Leblond <eric@inl.fr>
INL

[-- Attachment #2: Ceci est une partie de message numériquement signée --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [RFC] libnfnetlink and iface conversion to string

[Pablo Neira Ayuso]

Eric Leblond wrote:
> I'm currently working on NuFW where I try to do iface conversion from
> integer to string to be able to filter "naturally" on interfaces.
> 
> For now, it is impossible to do this conversion from either
> libnetfilter_queue or libnfnetlink. But there is two files in
> libnfnetlink (iftable.c and rntl.c) which have been put here by Harald
> to provide this conversion (if I remember correctly an old mail from
> Harald).
> 
> It was a long time ago and I don't know if it still planned to modify
> libnfnetlink to include this feature. Or, would it be a better choice to
> create a new library dedicated to this conversion.

Interesting question. Since this is related with the rtnetlink
subsystem, I'm unsure that this feature naturally belongs to
libnfnetlink nor to libnetfilter_queue.

However, IMO libnetfilter_queue should provide enough features to let
userspace apps implement filtering without requiring extra dependencies.
BTW, what are the benefits of dumping the index instead of the interface
name in this particular case?

Patrick?

-- 
The dawn of the fourth age of Linux firewalling is coming; a time of
great struggle and heroic deeds -- J.Kadlecsik got inspired by J.Morris

Re: [RFC] libnfnetlink and iface conversion to string

[Eric Leblond]

[-- Attachment #1.1: Type: text/plain, Size: 1063 bytes --]

Hi,

Le jeudi 28 décembre 2006 à 18:39 +0100, Pablo Neira Ayuso a écrit :
> Eric Leblond wrote:
> > I'm currently working on NuFW where I try to do iface conversion from
> > integer to string to be able to filter "naturally" on interfaces.
> Interesting question. Since this is related with the rtnetlink
> subsystem, I'm unsure that this feature naturally belongs to
> libnfnetlink nor to libnetfilter_queue.

I agree that this does not seem to be the correct place.

> However, IMO libnetfilter_queue should provide enough features to let
> userspace apps implement filtering without requiring extra dependencies.
> BTW, what are the benefits of dumping the index instead of the interface
> name in this particular case?

This is interesting !

I've looked at kernel code and the information was easy to add. I attach
a small patch against kernel.

It will add 4*IFNAMSIZ = 64 octets to each nfnetlink_queue message but
this is not impressive as a part of the packet payload is usually sent.

BR,
-- 
Eric Leblond <eric@inl.fr>
INL

[-- Attachment #1.2: nfnetlink_queue-iface-name.diff --]
[-- Type: text/x-patch, Size: 3609 bytes --]

diff --git a/include/linux/netfilter/nfnetlink_queue.h b/include/linux/netfilter/nfnetlink_queue.h
index 83e7896..8318105 100644
--- a/include/linux/netfilter/nfnetlink_queue.h
+++ b/include/linux/netfilter/nfnetlink_queue.h
@@ -41,6 +41,10 @@ enum nfqnl_attr_type {
 	NFQA_IFINDEX_PHYSOUTDEV,	/* u_int32_t ifindex */
 	NFQA_HWADDR,			/* nfqnl_msg_packet_hw */
 	NFQA_PAYLOAD,			/* opaque data payload */
+	NFQA_IFNAME_INDEV,		/* string name of iface */
+	NFQA_IFNAME_OUTDEV,		/*  string name of iface  */
+	NFQA_IFNAME_PHYSINDEV,		/* string name of iface */
+	NFQA_IFNAME_PHYSOUTDEV,	/* string name of iface */
 
 	__NFQA_MAX
 };
diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
index a88a017..9b86b9d 100644
--- a/net/netfilter/nfnetlink_queue.c
+++ b/net/netfilter/nfnetlink_queue.c
@@ -424,6 +424,7 @@ #endif
 		tmp_uint = htonl(indev->ifindex);
 #ifndef CONFIG_BRIDGE_NETFILTER
 		NFA_PUT(skb, NFQA_IFINDEX_INDEV, sizeof(tmp_uint), &tmp_uint);
+		NFA_PUT(skb, NFQA_IFNAME_INDEV, IFNAMSIZ, indev->name);
 #else
 		if (entinf->pf == PF_BRIDGE) {
 			/* Case 1: indev is physical input device, we need to
@@ -431,20 +432,28 @@ #else
 			 * netfilter_bridge) */
 			NFA_PUT(skb, NFQA_IFINDEX_PHYSINDEV, sizeof(tmp_uint), 
 				&tmp_uint);
+			NFA_PUT(skb, NFQA_IFNAME_PHYSINDEV, IFNAMSIZ,
+				indev->name);
 			/* this is the bridge group "brX" */
 			tmp_uint = htonl(indev->br_port->br->dev->ifindex);
 			NFA_PUT(skb, NFQA_IFINDEX_INDEV, sizeof(tmp_uint),
 				&tmp_uint);
+			NFA_PUT(skb, NFQA_IFNAME_INDEV, IFNAMSIZ,
+				indev->name);
 		} else {
 			/* Case 2: indev is bridge group, we need to look for
 			 * physical device (when called from ipv4) */
 			NFA_PUT(skb, NFQA_IFINDEX_INDEV, sizeof(tmp_uint),
 				&tmp_uint);
+			NFA_PUT(skb, NFQA_IFNAME_INDEV, IFNAMSIZ,
+				indev->name);
 			if (entskb->nf_bridge
 			    && entskb->nf_bridge->physindev) {
 				tmp_uint = htonl(entskb->nf_bridge->physindev->ifindex);
 				NFA_PUT(skb, NFQA_IFINDEX_PHYSINDEV,
 					sizeof(tmp_uint), &tmp_uint);
+				NFA_PUT(skb, NFQA_IFNAME_PHYSINDEV, IFNAMSIZ,
+					entskb->nf_bridge->physindev->name);
 			}
 		}
 #endif
@@ -454,6 +463,7 @@ #endif
 		tmp_uint = htonl(outdev->ifindex);
 #ifndef CONFIG_BRIDGE_NETFILTER
 		NFA_PUT(skb, NFQA_IFINDEX_OUTDEV, sizeof(tmp_uint), &tmp_uint);
+		NFA_PUT(skb, NFQA_IFNAME_OUTDEV, IFNAMSIZ, outdev->name);
 #else
 		if (entinf->pf == PF_BRIDGE) {
 			/* Case 1: outdev is physical output device, we need to
@@ -461,20 +471,28 @@ #else
 			 * netfilter_bridge) */
 			NFA_PUT(skb, NFQA_IFINDEX_PHYSOUTDEV, sizeof(tmp_uint),
 				&tmp_uint);
+			NFA_PUT(skb, NFQA_IFNAME_PHYSOUTDEV, IFNAMSIZ,
+				outdev->name);
 			/* this is the bridge group "brX" */
 			tmp_uint = htonl(outdev->br_port->br->dev->ifindex);
 			NFA_PUT(skb, NFQA_IFINDEX_OUTDEV, sizeof(tmp_uint),
 				&tmp_uint);
+			NFA_PUT(skb, NFQA_IFNAME_OUTDEV, IFNAMSIZ,
+				outdev->name);
 		} else {
 			/* Case 2: outdev is bridge group, we need to look for
 			 * physical output device (when called from ipv4) */
 			NFA_PUT(skb, NFQA_IFINDEX_OUTDEV, sizeof(tmp_uint),
 				&tmp_uint);
+			NFA_PUT(skb, NFQA_IFNAME_OUTDEV, IFNAMSIZ,
+				outdev->name);
 			if (entskb->nf_bridge
 			    && entskb->nf_bridge->physoutdev) {
 				tmp_uint = htonl(entskb->nf_bridge->physoutdev->ifindex);
 				NFA_PUT(skb, NFQA_IFINDEX_PHYSOUTDEV,
 					sizeof(tmp_uint), &tmp_uint);
+				NFA_PUT(skb, NFQA_IFNAME_PHYSOUTDEV, IFNAMSIZ,
+					entskb->nf_bridge->physoutdev->name);
 			}
 		}
 #endif

[-- Attachment #2: Ceci est une partie de message numériquement signée --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[Patch 1/2] Resend : sending iface name from nfnetlink_queue

[Eric Leblond]

[-- Attachment #1.1: Type: text/plain, Size: 825 bytes --]

Hi,

Le vendredi 29 décembre 2006 à 00:40 +0100, Eric Leblond a écrit :
> Hi,
> 
> Le jeudi 28 décembre 2006 à 18:39 +0100, Pablo Neira Ayuso a écrit :
> > Eric Leblond wrote:
> > > I'm currently working on NuFW where I try to do iface conversion from
> > > integer to string to be able to filter "naturally" on interfaces.
> > Interesting question. Since this is related with the rtnetlink
> > subsystem, I'm unsure that this feature naturally belongs to
> > libnfnetlink nor to libnetfilter_queue.
> 
> I've looked at kernel code and the information was easy to add. I attach
> a small patch against kernel.

I resend a modified version of my kernel patch. It fixes a bug on fields
size.

BR,
-- 
Éric Leblond, eleblond@inl.fr
Téléphone : 01 44 89 46 39, Fax : 01 44 89 45 01
INL, http://www.inl.fr

[-- Attachment #1.2: nfnetlink_queue-ifacename.diff --]
[-- Type: text/x-patch, Size: 3739 bytes --]

diff --git a/include/linux/netfilter/nfnetlink_queue.h b/include/linux/netfilter/nfnetlink_queue.h
index 83e7896..8318105 100644
--- a/include/linux/netfilter/nfnetlink_queue.h
+++ b/include/linux/netfilter/nfnetlink_queue.h
@@ -41,6 +41,10 @@ enum nfqnl_attr_type {
 	NFQA_IFINDEX_PHYSOUTDEV,	/* u_int32_t ifindex */
 	NFQA_HWADDR,			/* nfqnl_msg_packet_hw */
 	NFQA_PAYLOAD,			/* opaque data payload */
+	NFQA_IFNAME_INDEV,		/* string name of iface */
+	NFQA_IFNAME_OUTDEV,		/*  string name of iface  */
+	NFQA_IFNAME_PHYSINDEV,		/* string name of iface */
+	NFQA_IFNAME_PHYSOUTDEV,	/* string name of iface */
 
 	__NFQA_MAX
 };
diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
index a88a017..6cc301a 100644
--- a/net/netfilter/nfnetlink_queue.c
+++ b/net/netfilter/nfnetlink_queue.c
@@ -424,6 +424,7 @@ #endif
 		tmp_uint = htonl(indev->ifindex);
 #ifndef CONFIG_BRIDGE_NETFILTER
 		NFA_PUT(skb, NFQA_IFINDEX_INDEV, sizeof(tmp_uint), &tmp_uint);
+		NFA_PUT(skb, NFQA_IFNAME_INDEV, IFNAMSIZ*sizeof(char), indev->name);
 #else
 		if (entinf->pf == PF_BRIDGE) {
 			/* Case 1: indev is physical input device, we need to
@@ -431,20 +432,28 @@ #else
 			 * netfilter_bridge) */
 			NFA_PUT(skb, NFQA_IFINDEX_PHYSINDEV, sizeof(tmp_uint), 
 				&tmp_uint);
+			NFA_PUT(skb, NFQA_IFNAME_PHYSINDEV, IFNAMSIZ*sizeof(char),
+				indev->name);
 			/* this is the bridge group "brX" */
 			tmp_uint = htonl(indev->br_port->br->dev->ifindex);
 			NFA_PUT(skb, NFQA_IFINDEX_INDEV, sizeof(tmp_uint),
 				&tmp_uint);
+			NFA_PUT(skb, NFQA_IFNAME_INDEV, IFNAMSIZ*sizeof(char),
+				indev->name);
 		} else {
 			/* Case 2: indev is bridge group, we need to look for
 			 * physical device (when called from ipv4) */
 			NFA_PUT(skb, NFQA_IFINDEX_INDEV, sizeof(tmp_uint),
 				&tmp_uint);
+			NFA_PUT(skb, NFQA_IFNAME_INDEV, IFNAMSIZ*sizeof(char),
+				indev->name);
 			if (entskb->nf_bridge
 			    && entskb->nf_bridge->physindev) {
 				tmp_uint = htonl(entskb->nf_bridge->physindev->ifindex);
 				NFA_PUT(skb, NFQA_IFINDEX_PHYSINDEV,
 					sizeof(tmp_uint), &tmp_uint);
+				NFA_PUT(skb, NFQA_IFNAME_PHYSINDEV, IFNAMSIZ*sizeof(char),
+					entskb->nf_bridge->physindev->name);
 			}
 		}
 #endif
@@ -454,6 +463,7 @@ #endif
 		tmp_uint = htonl(outdev->ifindex);
 #ifndef CONFIG_BRIDGE_NETFILTER
 		NFA_PUT(skb, NFQA_IFINDEX_OUTDEV, sizeof(tmp_uint), &tmp_uint);
+		NFA_PUT(skb, NFQA_IFNAME_OUTDEV, IFNAMSIZ*sizeof(char), outdev->name);
 #else
 		if (entinf->pf == PF_BRIDGE) {
 			/* Case 1: outdev is physical output device, we need to
@@ -461,20 +471,28 @@ #else
 			 * netfilter_bridge) */
 			NFA_PUT(skb, NFQA_IFINDEX_PHYSOUTDEV, sizeof(tmp_uint),
 				&tmp_uint);
+			NFA_PUT(skb, NFQA_IFNAME_PHYSOUTDEV, IFNAMSIZ*sizeof(char),
+				outdev->name);
 			/* this is the bridge group "brX" */
 			tmp_uint = htonl(outdev->br_port->br->dev->ifindex);
 			NFA_PUT(skb, NFQA_IFINDEX_OUTDEV, sizeof(tmp_uint),
 				&tmp_uint);
+			NFA_PUT(skb, NFQA_IFNAME_OUTDEV, IFNAMSIZ*sizeof(char),
+				outdev->name);
 		} else {
 			/* Case 2: outdev is bridge group, we need to look for
 			 * physical output device (when called from ipv4) */
 			NFA_PUT(skb, NFQA_IFINDEX_OUTDEV, sizeof(tmp_uint),
 				&tmp_uint);
+			NFA_PUT(skb, NFQA_IFNAME_OUTDEV, IFNAMSIZ*sizeof(char),
+				outdev->name);
 			if (entskb->nf_bridge
 			    && entskb->nf_bridge->physoutdev) {
 				tmp_uint = htonl(entskb->nf_bridge->physoutdev->ifindex);
 				NFA_PUT(skb, NFQA_IFINDEX_PHYSOUTDEV,
 					sizeof(tmp_uint), &tmp_uint);
+				NFA_PUT(skb, NFQA_IFNAME_PHYSOUTDEV, IFNAMSIZ*sizeof(char),
+					entskb->nf_bridge->physoutdev->name);
 			}
 		}
 #endif

[-- Attachment #2: Ceci est une partie de message numériquement signée --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [Patch 1/2] Resend : sending iface name from nfnetlink_queue

[Patrick McHardy]

Eric Leblond wrote:
> Le vendredi 29 décembre 2006 à 00:40 +0100, Eric Leblond a écrit :
> 
>>>Eric Leblond wrote:
>>>
>>>>I'm currently working on NuFW where I try to do iface conversion from
>>>>integer to string to be able to filter "naturally" on interfaces.
>>>
>>>Interesting question. Since this is related with the rtnetlink
>>>subsystem, I'm unsure that this feature naturally belongs to
>>>libnfnetlink nor to libnetfilter_queue.
>>
>>I've looked at kernel code and the information was easy to add. I attach
>>a small patch against kernel.
> 
> 
> I resend a modified version of my kernel patch. It fixes a bug on fields
> size.


I think Harald already stated his dislike for this approach, just
to be explicit: I don't think we should add interface names to the
netlink protocols, userspace should resolve the ifindex.

[Patch 2/2] getting iface name from libnetfilter_queue

[Eric Leblond]

[-- Attachment #1.1: Type: text/plain, Size: 1376 bytes --]

Hi,

here's a patch against libnetfilter_queue.

It adds functions that can be used to get interface name.

BR,

Le vendredi 29 décembre 2006 à 00:40 +0100, Eric Leblond a écrit :
> Hi,
> 
> Le jeudi 28 décembre 2006 à 18:39 +0100, Pablo Neira Ayuso a écrit :
> > Eric Leblond wrote:
> > > I'm currently working on NuFW where I try to do iface conversion from
> > > integer to string to be able to filter "naturally" on interfaces.
> > Interesting question. Since this is related with the rtnetlink
> > subsystem, I'm unsure that this feature naturally belongs to
> > libnfnetlink nor to libnetfilter_queue.
> 
> I agree that this does not seem to be the correct place.
> 
> > However, IMO libnetfilter_queue should provide enough features to let
> > userspace apps implement filtering without requiring extra dependencies.
> > BTW, what are the benefits of dumping the index instead of the interface
> > name in this particular case?
> 
> This is interesting !
> 
> I've looked at kernel code and the information was easy to add. I attach
> a small patch against kernel.
> 
> It will add 4*IFNAMSIZ = 64 octets to each nfnetlink_queue message but
> this is not impressive as a part of the packet payload is usually sent.
> 
> BR,
-- 
Éric Leblond, eleblond@inl.fr
Téléphone : 01 44 89 46 39, Fax : 01 44 89 45 01
INL, http://www.inl.fr

[-- Attachment #1.2: libnetfilter_queue-ifacename.diff --]
[-- Type: text/x-patch, Size: 4472 bytes --]

Index: utils/nfqnl_test.c
===================================================================
--- utils/nfqnl_test.c	(révision 6720)
+++ utils/nfqnl_test.c	(copie de travail)
@@ -2,11 +2,14 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <unistd.h>
+#include <errno.h>
 #include <netinet/in.h>
 #include <linux/netfilter.h>		/* for NF_ACCEPT */
 
 #include <libnetfilter_queue/libnetfilter_queue.h>
 
+/* #define PRINT_INFOS  */
+
 /* returns packet id */
 static u_int32_t print_pkt (struct nfq_data *tb)
 {
@@ -16,13 +19,17 @@
 	int ret;
 	char *data;
 	
+    id = ntohl(ph->packet_id);
 	ph = nfq_get_msg_packet_hdr(tb);
 	if (ph){
 		id = ntohl(ph->packet_id);
+#ifdef PRINT_INFOS
 		printf("hw_protocol=0x%04x hook=%u id=%u ",
 			ntohs(ph->hw_protocol), ph->hook, id);
+#endif
 	}
 	
+#ifdef PRINT_INFOS
 	mark = nfq_get_nfmark(tb);
 	if (mark)
 		printf("mark=%u ", mark);
@@ -40,7 +47,7 @@
 		printf("payload_len=%d ", ret);
 
 	fputc('\n', stdout);
-
+#endif
 	return id;
 }
 	
@@ -49,7 +56,9 @@
 	      struct nfq_data *nfa, void *data)
 {
 	u_int32_t id = print_pkt(nfa);
+#ifdef PRINT_INFOS
 	printf("entering callback\n");
+#endif
 	return nfq_set_verdict(qh, id, NF_ACCEPT, 0, NULL);
 }
 
@@ -96,11 +105,22 @@
 
 	nh = nfq_nfnlh(h);
 	fd = nfnl_fd(nh);
+    while ((rv = recv(fd, buf, sizeof(buf), 0))) {
+        if (rv<0){
+            printf("got error %d during recv\n",errno);
+            if (errno != ENOBUFS){
+                break;
+            } else {
+                printf("got error %d restart recv\n",errno);
+                continue;
+            }
+        }
+#ifdef PRINT_INFOS
+        printf("pkt received\n");
+#endif
+        nfq_handle_packet(h, buf, rv);
+    }
 
-	while ((rv = recv(fd, buf, sizeof(buf), 0)) && rv >= 0) {
-		printf("pkt received\n");
-		nfq_handle_packet(h, buf, rv);
-	}
 
 	printf("unbinding from queue 0\n");
 	nfq_destroy_queue(qh);
Index: include/libnetfilter_queue/linux_nfnetlink_queue.h
===================================================================
--- include/libnetfilter_queue/linux_nfnetlink_queue.h	(révision 6720)
+++ include/libnetfilter_queue/linux_nfnetlink_queue.h	(copie de travail)
@@ -45,6 +45,10 @@
 	NFQA_IFINDEX_PHYSOUTDEV,	/* u_int32_t ifindex */
 	NFQA_HWADDR,			/* nfqnl_msg_packet_hw */
 	NFQA_PAYLOAD,			/* opaque data payload */
+	NFQA_IFNAME_INDEV,		/* string name of iface */
+	NFQA_IFNAME_OUTDEV,		/*  string name of iface  */
+	NFQA_IFNAME_PHYSINDEV,	/* string name of iface */
+	NFQA_IFNAME_PHYSOUTDEV,	/* string name of iface */
 
 	__NFQA_MAX
 };
Index: include/libnetfilter_queue/libnetfilter_queue.h
===================================================================
--- include/libnetfilter_queue/libnetfilter_queue.h	(révision 6720)
+++ include/libnetfilter_queue/libnetfilter_queue.h	(copie de travail)
@@ -80,6 +80,13 @@
 extern u_int32_t nfq_get_outdev(struct nfq_data *nfad);
 extern u_int32_t nfq_get_physoutdev(struct nfq_data *nfad);
 
+/* return NULL if not set */
+extern char* nfq_get_indev_name(struct nfq_data *nfad);
+extern char* nfq_get_physindev_name(struct nfq_data *nfad);
+extern char* nfq_get_outdev_name(struct nfq_data *nfad);
+extern char* nfq_get_physoutdev_name(struct nfq_data *nfad);
+
+
 extern struct nfqnl_msg_packet_hw *nfq_get_packet_hw(struct nfq_data *nfad);
 
 /* return -1 if problem, length otherwise */
Index: src/libnetfilter_queue.c
===================================================================
--- src/libnetfilter_queue.c	(révision 6720)
+++ src/libnetfilter_queue.c	(copie de travail)
@@ -419,6 +419,26 @@
 	return ntohl(nfnl_get_data(nfad->data, NFQA_IFINDEX_PHYSOUTDEV, u_int32_t));
 }
 
+char* nfq_get_indev_name(struct nfq_data *nfad)
+{
+	return nfnl_get_pointer_to_data(nfad->data, NFQA_IFNAME_INDEV, char);
+}
+
+char* nfq_get_physindev_name(struct nfq_data *nfad)
+{
+	return nfnl_get_pointer_to_data(nfad->data, NFQA_IFNAME_PHYSINDEV, char);
+}
+
+char* nfq_get_outdev_name(struct nfq_data *nfad)
+{
+	return nfnl_get_pointer_to_data(nfad->data, NFQA_IFNAME_OUTDEV, char);
+}
+
+char* nfq_get_physoutdev_name(struct nfq_data *nfad)
+{
+	return nfnl_get_pointer_to_data(nfad->data, NFQA_IFNAME_PHYSOUTDEV, char);
+}
+
 struct nfqnl_msg_packet_hw *nfq_get_packet_hw(struct nfq_data *nfad)
 {
 	return nfnl_get_pointer_to_data(nfad->data, NFQA_HWADDR,

[-- Attachment #2: Ceci est une partie de message numériquement signée --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [RFC] libnfnetlink and iface conversion to string

[Harald Welte]

[-- Attachment #1: Type: text/plain, Size: 1350 bytes --]

On Fri, Dec 29, 2006 at 12:40:47AM +0100, Eric Leblond wrote:
> It will add 4*IFNAMSIZ = 64 octets to each nfnetlink_queue message but
> this is not impressive as a part of the packet payload is usually sent.

No, please don't do that.  

In your application, it might not be a lot, since you always send the
packet payload.  other applications might only need a smal bit of that.

Also, I _really_ want to get rid of interface names throughout
netfilter/iptables at some point in the future.  all 'next generation'
subsystems (such as all nfnetlink based services) should just deal with
ifindexes.

all of iproute2 also uses ifindexes, and IIRC their netlink messages
also only have ifindexes. 

Please put some functions into libnfnetlink, even if it doesn't natively
belong there.   If there's more netlink unification, we can always make
those just wrappers for the 'real new unified' ifindex/name resolving
functions.

[yes, I'm back, more or less]

-- 
- Harald Welte <laforge@netfilter.org>                 http://netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [RFC] libnfnetlink and iface conversion to string

[Eric Leblond]

[-- Attachment #1: Type: text/plain, Size: 1447 bytes --]

Hi,
Le dimanche 07 janvier 2007 à 15:26 +0100, Harald Welte a écrit :
> On Fri, Dec 29, 2006 at 12:40:47AM +0100, Eric Leblond wrote:
> > It will add 4*IFNAMSIZ = 64 octets to each nfnetlink_queue message but
> > this is not impressive as a part of the packet payload is usually sent.
> 
> No, please don't do that.  
> Also, I _really_ want to get rid of interface names throughout
> netfilter/iptables at some point in the future.  all 'next generation'
> subsystems (such as all nfnetlink based services) should just deal with
> ifindexes.
> 
> Please put some functions into libnfnetlink, even if it doesn't natively
> belong there.   If there's more netlink unification, we can always make
> those just wrappers for the 'real new unified' ifindex/name resolving
> functions.

I've thought at this solution before getting interessed by pablo's
suggestion. But I've got a conception problem.

It is not correct to do a dump (and send a netlink message) for each
ifindex resolution. Thus, libnfnetlink needs to listen to netlink
interfaces message. A permanent "task" is thus needed to have the job
done : It can be a dedicated thread or a carefully hidden select.

In fact, if we omit the thread approach which is somehow intrusive, I
don't see a way to do this via a simple call to added functions.

Any idea welcome.

> [yes, I'm back, more or less]

Great news !

BR,
-- 
Eric Leblond <eric@inl.fr>
INL

[-- Attachment #2: Ceci est une partie de message numériquement signée --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [RFC] libnfnetlink and iface conversion to string

[Pablo Neira Ayuso]

Eric Leblond wrote:
> It is not correct to do a dump (and send a netlink message) for each
> ifindex resolution. Thus, libnfnetlink needs to listen to netlink
> interfaces message. A permanent "task" is thus needed to have the job
> done : It can be a dedicated thread or a carefully hidden select.
> 
> In fact, if we omit the thread approach which is somehow intrusive, I
> don't see a way to do this via a simple call to added functions.

You have to open a netlink socket for NETLINK_ROUTE, request a dump
(RTM_GETLINK) to keep a cache of current tuples interface/index
available, and subscribe to interface events (RTNLGRP_LINK) to keep the
cache up to date. You can put both the nfqueue and the rtnetlink sockets
in a poll. Something like:

/* create a table ifindex to interface */
struct nfnl_ifindex2name_table *nfnl_ifindex2name_open();
/* destroy table */
void nfnl_ifindex2name_close(struct nfnl_ifindex2name_table *);

/* returns a socket descriptor to listen to interface events */
int nfnl_ifindex2name_events_open();
/* process data available for the socket and update the table */
int nfnl_ifindex2name_process_event(int, struct nfnl_ifindex2name_table *);
/* close socket */
void nfnl_ifindex2name_events_close(int); /* close the socket */

Another alternative could be the use of libnl, although this introduces
a new library dependency for every libnetfilter_queue application.

-- 
The dawn of the fourth age of Linux firewalling is coming; a time of
great struggle and heroic deeds -- J.Kadlecsik got inspired by J.Morris

Re: [RFC] libnfnetlink and iface conversion to string

[Eric Leblond]

[-- Attachment #1: Type: text/plain, Size: 1901 bytes --]

Hi,

Le mardi 09 janvier 2007 à 01:53 +0100, Pablo Neira Ayuso a écrit :
> Eric Leblond wrote:
> > It is not correct to do a dump (and send a netlink message) for each
> > ifindex resolution. Thus, libnfnetlink needs to listen to netlink
> > interfaces message. A permanent "task" is thus needed to have the job
> > done : It can be a dedicated thread or a carefully hidden select.
> /* create a table ifindex to interface */
> struct nfnl_ifindex2name_table *nfnl_ifindex2name_open();
> /* destroy table */
> void nfnl_ifindex2name_close(struct nfnl_ifindex2name_table *);
> 
> /* returns a socket descriptor to listen to interface events */
> int nfnl_ifindex2name_events_open();

Speaking as NuFW's developper and thus libnetfilter_queue user, this
clearly mean that I need to modify select part of my code, not just add
some functions calls. It forces the developper to use directly
libnfnetlink. This goes against my way to see libnetfilter_queue and
other libnetfilter* libraries.

> /* process data available for the socket and update the table */
> int nfnl_ifindex2name_process_event(int, struct nfnl_ifindex2name_table *);
> /* close socket */
> void nfnl_ifindex2name_events_close(int); /* close the socket */
> 
> Another alternative could be the use of libnl, although this introduces
> a new library dependency for every libnetfilter_queue application.

I think that ifname resolution is necessary for an application if it is
a linked with wanted features. If it has to use it, application can link
to libnl (or something else) and have it done. If it don't need it then
libnetfilter_queue is enough.

route netlink message is not a part of Netfilter, we should'nt use it in
Netfilter code if it is not transparent :
      * It duplicates code
      * It changes the "spirit" of libnetfilter* libraries

BR,
-- 
Eric Leblond <eric@inl.fr>
INL

[-- Attachment #2: Ceci est une partie de message numériquement signée --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [RFC] libnfnetlink and iface conversion to string

[Harald Welte]

[-- Attachment #1: Type: text/plain, Size: 2372 bytes --]

On Mon, Jan 08, 2007 at 11:41:26PM +0100, Eric Leblond wrote:
> Hi,
> Le dimanche 07 janvier 2007 à 15:26 +0100, Harald Welte a écrit :
> > On Fri, Dec 29, 2006 at 12:40:47AM +0100, Eric Leblond wrote:
> > > It will add 4*IFNAMSIZ = 64 octets to each nfnetlink_queue message but
> > > this is not impressive as a part of the packet payload is usually sent.
> > 
> > No, please don't do that.  
> > Also, I _really_ want to get rid of interface names throughout
> > netfilter/iptables at some point in the future.  all 'next generation'
> > subsystems (such as all nfnetlink based services) should just deal with
> > ifindexes.
> > 
> > Please put some functions into libnfnetlink, even if it doesn't natively
> > belong there.   If there's more netlink unification, we can always make
> > those just wrappers for the 'real new unified' ifindex/name resolving
> > functions.
> 
> I've thought at this solution before getting interessed by pablo's
> suggestion. But I've got a conception problem.
> 
> It is not correct to do a dump (and send a netlink message) for each
> ifindex resolution. Thus, libnfnetlink needs to listen to netlink
> interfaces message. A permanent "task" is thus needed to have the job
> done : It can be a dedicated thread or a carefully hidden select.

the latter, a caerfully hidden select.  I actually have code for this,
which I am allowed to release under GPL (was written as part of a
non-gpl project)

I have committed that code to svn:
http://svn.netfilter.org/cgi-bin/viewcvs.cgi/trunk/libnfnetlink/src/
rtnl.c and iftable.c

However, I never found the time to integrate them intl libnfnetlink, so
they remained dead code for the last 16 months :(

> In fact, if we omit the thread approach which is somehow intrusive, I
> don't see a way to do this via a simple call to added functions.

It isn't all that difficult to keep a in-memory list of name-ifindex
mappings.  You once dump at startup, and then listen to events...

-- 
- Harald Welte <laforge@netfilter.org>                 http://netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[Patch 0/2] libnfnetlink and iface conversion to string

[Eric Leblond]

[-- Attachment #1.1: Type: text/plain, Size: 1331 bytes --]

Hi,

Le mardi 09 janvier 2007 à 12:51 +0100, Harald Welte a écrit :
> On Mon, Jan 08, 2007 at 11:41:26PM +0100, Eric Leblond wrote:
> > Hi,
> I have committed that code to svn:
> http://svn.netfilter.org/cgi-bin/viewcvs.cgi/trunk/libnfnetlink/src/
> rtnl.c and iftable.c
> 
> However, I never found the time to integrate them intl libnfnetlink, so
> they remained dead code for the last 16 months :(

Here's an attempt to add iface name resolution to libnfnetlink. It works
and it is not too difficult to use.

> > In fact, if we omit the thread approach which is somehow intrusive, I
> > don't see a way to do this via a simple call to added functions.
> 
> It isn't all that difficult to keep a in-memory list of name-ifindex
> mappings.  You once dump at startup, and then listen to events...

This can be translated like that :
     1. get netlink_fd with nlif_table_init()
     2. use nlif_treat_msg(netlink_fd) to parse initial message
     3. put netlink_fd in a select, and call nlif_treat_msg(netlink_fd)
        when data arrives
     4. cleanup with nlif_table_fini(void) when feature is not needed
        any more

I join the patch for NuFW as code example.

To follow :
      * Patch for libnfnetlink
      * Patch for libnetfilter_queue

BR,
-- 
Eric Leblond <eric@inl.fr>
INL

[-- Attachment #1.2: nufw_iface_name.diff --]
[-- Type: text/x-patch, Size: 3222 bytes --]

Index: src/nufw/packetsrv.c
===================================================================
--- src/nufw/packetsrv.c	(révision 2596)
+++ src/nufw/packetsrv.c	(copie de travail)
@@ -18,6 +18,8 @@
 
 #include "nufw.h"
 
+#include "iface.h"
+
 /** \file packetsrv.c
  *  \brief Packet server thread
  *
@@ -159,6 +161,7 @@
     return 1;
 }
 
+
 /**
  * Open a netlink connection and returns file descriptor
  */
@@ -335,6 +338,7 @@
     unsigned char buffer[BUFSIZ];
     struct timeval tv;
     int fd;
+    int if_fd;
     int rv;
     int select_result;
     fd_set wk_set;
@@ -345,6 +349,12 @@
         exit(EXIT_FAILURE);
     }
 
+    if_fd = iface_table_open();
+
+    if (if_fd < 0) {
+        exit(EXIT_FAILURE);
+    }
+
     log_area_printf (DEBUG_AREA_MAIN, DEBUG_LEVEL_WARNING,
             "[+] Packet server started");
 
@@ -360,6 +370,7 @@
         /* wait new event on socket */
         FD_ZERO(&wk_set);
         FD_SET(fd,&wk_set);
+        FD_SET(if_fd,&wk_set);
         select_result = select(fd+1,&wk_set,NULL,NULL,&tv);
         if (select_result == -1)
         {
@@ -380,6 +391,10 @@
             continue;
         }
 
+        if (FD_ISSET(if_fd,&wk_set)){
+            iface_treat_message(if_fd);
+            continue;
+        }
         /* read one packet */
         rv = recv(fd, buffer, sizeof(buffer), 0);
         if (rv < 0)
Index: src/nufw/iface.c
===================================================================
--- src/nufw/iface.c	(révision 2596)
+++ src/nufw/iface.c	(copie de travail)
@@ -42,4 +42,24 @@
 	return 1;
 }
 
+int iface_table_open()
+{
+    int iftable_fd;
+    /* opening ifname resolution handle */
+    iftable_fd = nlif_table_init();
+    if (iftable_fd <= 0) {
+        log_area_printf (DEBUG_AREA_MAIN, DEBUG_LEVEL_CRITICAL,
+                "[!] Error during nlif_table_init()");
+        return -1;
+    }
+    /* treat initial rtnetlink message */
+    nlif_treat_msg(iftable_fd);
+
+    return iftable_fd;
+}
+
+int iface_treat_message(int fd)
+{
+   return nlif_treat_msg(fd);
+}
 #endif
Index: src/nufw/Makefile.am
===================================================================
--- src/nufw/Makefile.am	(révision 2596)
+++ src/nufw/Makefile.am	(copie de travail)
@@ -4,8 +4,8 @@
 sbin_PROGRAMS = nufw
 
 # nufw
-nufw_SOURCES = authsrv.c  common.c  main.c  packetsrv.c  proto.h  structure.h\
-		tls.c audit.c conntrack.c log.c iface.c
+nufw_SOURCES = authsrv.c  common.c  main.c  iface.c packetsrv.c  proto.h  structure.h\
+		tls.c audit.c conntrack.c log.c
 if HAVE_IPQ
 nufw_LDADD = -lipq -lpthread -lgnutls
 endif
Index: ChangeLog
===================================================================
--- ChangeLog	(révision 2596)
+++ ChangeLog	(copie de travail)
@@ -1,6 +1,9 @@
 2.1.2
 	- log_mysql: add option to log by default in SQL
 	database with IPV4 schema
+	- libnuclient: add nu_get_home_dir function which is not
+	dependant of HOME environment variable
+	- nufw: fix compilation in ipq mode (workaround ipq.h problem)
 2.1.1 (2007/01/03)
 	- suppress ldap authentication code
 	- add support for log prefix

[-- Attachment #2: Ceci est une partie de message numériquement signée --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[Patch 1/2] libnfnetlink, iface conversion to string

[Eric Leblond]

[-- Attachment #1.1: Type: text/plain, Size: 92 bytes --]

Hi,

Here's the patch against libnfnetlink.

BR,
-- 
Eric Leblond <eric@inl.fr>
INL

[-- Attachment #1.2: libnfnetlink_iface_name.diff --]
[-- Type: text/x-patch, Size: 5687 bytes --]

Index: include/libnfnetlink/libnfnetlink.h
===================================================================
--- include/libnfnetlink/libnfnetlink.h	(révision 6736)
+++ include/libnfnetlink/libnfnetlink.h	(copie de travail)
@@ -176,6 +176,13 @@
 
 extern void nfnl_dump_packet(struct nlmsghdr *, int, char *);
 
+int nlif_table_init(void);
+void nlif_table_fini(void);
+
+char *nlif_index_2name(unsigned int index);
+int nlif_treat_msg(int fd);
+
+
 /* Pablo: What is the equivalence of be64_to_cpu in userspace?
  * 
  * Harald: Good question.  I don't think there's a standard way [yet?], 
Index: src/rtnl.c
===================================================================
--- src/rtnl.c	(révision 6736)
+++ src/rtnl.c	(copie de travail)
@@ -116,7 +116,7 @@
         struct sockaddr_nl nladdr;
 
         memset(&nladdr, 0, sizeof(nladdr));
-	memset(&req, 0, sizeof(req));
+        memset(&req, 0, sizeof(req));
         nladdr.nl_family = AF_NETLINK;
 
         req.nlh.nlmsg_len = sizeof(req);
@@ -127,11 +127,11 @@
         req.g.rtgen_family = AF_INET;
 
         return sendto(rtnl_fd, (void*)&req, sizeof(req), 0, 
-		      (struct sockaddr*)&nladdr, sizeof(nladdr));
+                (struct sockaddr*)&nladdr, sizeof(nladdr));
 }
 
 /* rtnl_receive - receive netlink packets from rtnetlink socket */
-int rtnl_receive()
+int rtnl_receive(int rtnl_fd)
 {
 	int status;
 	char buf[8192];
Index: src/iftable.c
===================================================================
--- src/iftable.c	(révision 6736)
+++ src/iftable.c	(copie de travail)
@@ -2,7 +2,7 @@
  *
  * (C) 2004 by Astaro AG, written by Harald Welte <hwelte@astaro.com>
  *
- * This software is Free Software and licensed under GNU GPLv2. 
+ * This software is Free Software and licensed under GNU GPLv2.
  *
  */
 
@@ -23,8 +23,8 @@
 
 #define iftb_log(x, ...)
 
-struct ifindex_map {
-	struct ifindex_map *next;
+struct ifindex_map_t {
+	struct ifindex_map_t *next;
 
 	u_int32_t	index;
 	u_int32_t	type;
@@ -34,7 +34,7 @@
 	char		name[16];
 };
 
-static struct ifindex_map *ifindex_map[16];
+static struct ifindex_map_t *ifindex_map[16];
 
 /* iftable_dump - Dump the interface table to a given file stream
  * @outfd:	file stream to which table should be dumped
@@ -44,7 +44,7 @@
 	int i;
 
 	for (i = 0; i < 16; i++) {
-		struct ifindex_map *im;
+		struct ifindex_map_t *im;
 		for (im = ifindex_map[i]; im; im = im->next) {
 			fprintf(outfd, "%u %s", im->index, im->name);
 			if (!(im->flags & IFF_UP))
@@ -67,7 +67,7 @@
 {
 	unsigned int hash;
 	struct ifinfomsg *ifi_msg = NLMSG_DATA(n);
-	struct ifindex_map *im, **imp;
+	struct ifindex_map_t *im, **imp;
 	struct rtattr *cb[IFLA_MAX+1];
 
 	if (n->nlmsg_type != RTM_NEWLINK)
@@ -108,7 +108,7 @@
 		iftb_log(LOG_DEBUG, "creating new iftable (ifindex=%u)",
 			 im->index);
 	}
-	
+
 	im->type = ifi_msg->ifi_type;
 	im->flags = ifi_msg->ifi_flags;
 	if (cb[IFLA_ADDRESS]) {
@@ -129,14 +129,13 @@
  * @n:		netlink message header of a RTM_DELLINK nlmsg
  * @arg:	not used
  *
- * Delete an entry from the interface table.  
+ * Delete an entry from the interface table.
  * Returns -1 on error, 0 if no matching entry was found or 1 on success.
  */
 int iftable_del(struct nlmsghdr *n, void *arg)
 {
 	struct ifinfomsg *ifi_msg = NLMSG_DATA(n);
 	struct rtattr *cb[IFLA_MAX+1];
-	struct ifindex_map *im;
 
 	if (n->nlmsg_type != RTM_DELLINK) {
 		iftb_log(LOG_ERROR,
@@ -157,15 +156,15 @@
 
 	return 1;
 }
-	
-/* ifindex_2name - get the name for an ifindex
+
+/* nl_ifindex_2name - get the name for an ifindex
  * @index:	ifindex to be resolved
  *
  * Return value: character string containing name of interface
  */
-char *ifindex_2name(unsigned int index)
+char *nlif_index_2name(unsigned int index)
 {
-	struct ifindex_map *im;
+	struct ifindex_map_t *im;
 
 	if (index == 0)
 		return "*";
@@ -183,7 +182,7 @@
  */
 int  iftable_up(unsigned int index)
 {
-	struct ifindex_map *im;
+	struct ifindex_map_t *im;
 
 	for (im = ifindex_map[index&0xF]; im; im = im->next) {
 		if (im->index == index) {
@@ -208,6 +207,14 @@
 	if (fini)
 		goto cleanup;
 
+	memset(ifindex_map,0,sizeof(ifindex_map));
+
+	ret = rtnl_init();
+
+	if (ret == -1) {
+		goto cleanup_none;
+	}
+
 	if (rtnl_handler_register(&handlers[0]) < 0) {
 		ret = -1;
 		goto cleanup_none;
@@ -223,7 +230,7 @@
 		goto cleanup_1;
 	}
 
-	return 0;
+	return ret;
 
 #if 0
 	if (rtnl_wilddump_requet(rtnl_fd, AF_UNSPEC, RTM_GETLINK) < 0) {
@@ -243,19 +250,30 @@
 	return ret;
 }
 
-/* iftable_init - Initialize interface table
+/** nl_iftable_init - Initialize interface table
+ *
+ * Initialize rtnl interface and interface table
+ *
+ * \return file descriptor to netlink socket
  */
-int iftable_init(void)
+int nlif_table_init(void)
 {
 	iftb_log(LOG_DEBUG, "%s", __FUNCTION__);
 	return init_or_fini(0);
 }
 
-/* iftable_fini - Destructor of interface table
+/* nl_iftable_fini - Destructor of interface table
  */
-void iftable_fini(void)
+void nlif_table_fini(void)
 {
 	init_or_fini(1);
 }
 
+/**
+ * \return 0 if OK
+ */
+int nlif_treat_msg(int fd)
+{
+	return rtnl_receive(fd);
+}
 
Index: src/Makefile.am
===================================================================
--- src/Makefile.am	(révision 6736)
+++ src/Makefile.am	(copie de travail)
@@ -8,4 +8,4 @@
 
 libnfnetlink_la_LDFLAGS = -Wc,-nostartfiles	\
 			  -version-info $(LIBVERSION)
-libnfnetlink_la_SOURCES = libnfnetlink.c 
+libnfnetlink_la_SOURCES = libnfnetlink.c iftable.c rtnl.c

[-- Attachment #2: Ceci est une partie de message numériquement signée --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [Patch 1/2] libnfnetlink, iface conversion to string

[Patrick McHardy]

Eric Leblond wrote:
> Index: src/rtnl.c
> ===================================================================
> --- src/rtnl.c	(révision 6736)
> +++ src/rtnl.c	(copie de travail)
> @@ -116,7 +116,7 @@
>          struct sockaddr_nl nladdr;
>  
>          memset(&nladdr, 0, sizeof(nladdr));
> -	memset(&req, 0, sizeof(req));
> +        memset(&req, 0, sizeof(req));

Reindenting, but fine ..

>          nladdr.nl_family = AF_NETLINK;
>  
>          req.nlh.nlmsg_len = sizeof(req);
> @@ -127,11 +127,11 @@
>          req.g.rtgen_family = AF_INET;
>  
>          return sendto(rtnl_fd, (void*)&req, sizeof(req), 0, 
> -		      (struct sockaddr*)&nladdr, sizeof(nladdr));
> +                (struct sockaddr*)&nladdr, sizeof(nladdr));

This one isn't.

>  }
>  
>  /* rtnl_receive - receive netlink packets from rtnetlink socket */
> -int rtnl_receive()
> +int rtnl_receive(int rtnl_fd)

header file needs to be fixed as well. Is the global rtnl_fd still
needed?

>  {
>  	int status;
>  	char buf[8192];
> Index: src/iftable.c
> ===================================================================
> --- src/iftable.c	(révision 6736)
> +++ src/iftable.c	(copie de travail)
> @@ -2,7 +2,7 @@
>   *
>   * (C) 2004 by Astaro AG, written by Harald Welte <hwelte@astaro.com>
>   *
> - * This software is Free Software and licensed under GNU GPLv2. 
> + * This software is Free Software and licensed under GNU GPLv2.

Please do cleanup in seperate patches.

>   *
>   */
>  
> @@ -23,8 +23,8 @@
>  
>  #define iftb_log(x, ...)
>  
> -struct ifindex_map {
> -	struct ifindex_map *next;
> +struct ifindex_map_t {
> +	struct ifindex_map_t *next;

No useless renaming without a reason please.

>  
>  	u_int32_t	index;
>  	u_int32_t	type;
> @@ -34,7 +34,7 @@
>  	char		name[16];
>  };
>  
> -static struct ifindex_map *ifindex_map[16];
> +static struct ifindex_map_t *ifindex_map[16];
>  
>  /* iftable_dump - Dump the interface table to a given file stream
>   * @outfd:	file stream to which table should be dumped
> @@ -44,7 +44,7 @@
>  	int i;
>  
>  	for (i = 0; i < 16; i++) {
> -		struct ifindex_map *im;
> +		struct ifindex_map_t *im;
>  		for (im = ifindex_map[i]; im; im = im->next) {
>  			fprintf(outfd, "%u %s", im->index, im->name);
>  			if (!(im->flags & IFF_UP))
> @@ -67,7 +67,7 @@
>  {
>  	unsigned int hash;
>  	struct ifinfomsg *ifi_msg = NLMSG_DATA(n);
> -	struct ifindex_map *im, **imp;
> +	struct ifindex_map_t *im, **imp;
>  	struct rtattr *cb[IFLA_MAX+1];
>  
>  	if (n->nlmsg_type != RTM_NEWLINK)
> @@ -108,7 +108,7 @@
>  		iftb_log(LOG_DEBUG, "creating new iftable (ifindex=%u)",
>  			 im->index);
>  	}
> -	
> +
>  	im->type = ifi_msg->ifi_type;
>  	im->flags = ifi_msg->ifi_flags;
>  	if (cb[IFLA_ADDRESS]) {
> @@ -129,14 +129,13 @@
>   * @n:		netlink message header of a RTM_DELLINK nlmsg
>   * @arg:	not used
>   *
> - * Delete an entry from the interface table.  
> + * Delete an entry from the interface table.
>   * Returns -1 on error, 0 if no matching entry was found or 1 on success.
>   */
>  int iftable_del(struct nlmsghdr *n, void *arg)
>  {
>  	struct ifinfomsg *ifi_msg = NLMSG_DATA(n);
>  	struct rtattr *cb[IFLA_MAX+1];
> -	struct ifindex_map *im;
>  
>  	if (n->nlmsg_type != RTM_DELLINK) {
>  		iftb_log(LOG_ERROR,
> @@ -157,15 +156,15 @@
>  
>  	return 1;
>  }
> -	
> -/* ifindex_2name - get the name for an ifindex
> +
> +/* nl_ifindex_2name - get the name for an ifindex
>   * @index:	ifindex to be resolved
>   *
>   * Return value: character string containing name of interface
>   */
> -char *ifindex_2name(unsigned int index)
> +char *nlif_index_2name(unsigned int index)
>  {
> -	struct ifindex_map *im;
> +	struct ifindex_map_t *im;
>  
>  	if (index == 0)
>  		return "*";
> @@ -183,7 +182,7 @@
>   */
>  int  iftable_up(unsigned int index)
>  {
> -	struct ifindex_map *im;
> +	struct ifindex_map_t *im;
>  
>  	for (im = ifindex_map[index&0xF]; im; im = im->next) {
>  		if (im->index == index) {
> @@ -208,6 +207,14 @@
>  	if (fini)
>  		goto cleanup;
>  
> +	memset(ifindex_map,0,sizeof(ifindex_map));

whitespace after comma.

> +
> +	ret = rtnl_init();
> +
> +	if (ret == -1) {
> +		goto cleanup_none;
> +	}

Please no parens around single expressions.

> +
>  	if (rtnl_handler_register(&handlers[0]) < 0) {
>  		ret = -1;
>  		goto cleanup_none;
> @@ -223,7 +230,7 @@
>  		goto cleanup_1;
>  	}
>  
> -	return 0;
> +	return ret;
>  
>  #if 0
>  	if (rtnl_wilddump_requet(rtnl_fd, AF_UNSPEC, RTM_GETLINK) < 0) {
> @@ -243,19 +250,30 @@
>  	return ret;
>  }
>  
> -/* iftable_init - Initialize interface table
> +/** nl_iftable_init - Initialize interface table
> + *
> + * Initialize rtnl interface and interface table
> + *
> + * \return file descriptor to netlink socket
>   */
> -int iftable_init(void)
> +int nlif_table_init(void)
>  {
>  	iftb_log(LOG_DEBUG, "%s", __FUNCTION__);
>  	return init_or_fini(0);
>  }
>  
> -/* iftable_fini - Destructor of interface table
> +/* nl_iftable_fini - Destructor of interface table
>   */
> -void iftable_fini(void)
> +void nlif_table_fini(void)
>  {
>  	init_or_fini(1);
>  }
>  
> +/**
> + * \return 0 if OK
> + */
> +int nlif_treat_msg(int fd)
> +{
> +	return rtnl_receive(fd);
> +}
>  
> Index: src/Makefile.am
> ===================================================================
> --- src/Makefile.am	(révision 6736)
> +++ src/Makefile.am	(copie de travail)
> @@ -8,4 +8,4 @@
>  
>  libnfnetlink_la_LDFLAGS = -Wc,-nostartfiles	\
>  			  -version-info $(LIBVERSION)
> -libnfnetlink_la_SOURCES = libnfnetlink.c 
> +libnfnetlink_la_SOURCES = libnfnetlink.c iftable.c rtnl.c

Re: [Patch 1/2] libnfnetlink, iface conversion to string

[Pablo Neira Ayuso]

Patrick McHardy wrote:
> Eric Leblond wrote:
>>  /* rtnl_receive - receive netlink packets from rtnetlink socket */
>> -int rtnl_receive()
>> +int rtnl_receive(int rtnl_fd)
> 
> header file needs to be fixed as well. Is the global rtnl_fd still
> needed?

Indeed. Eric, all this global stuff below must dissapear at all, from
rtnl.c:

static int rtnl_fd;
static int rtnl_seq = 0;
static int rtnl_dump;
static struct sockaddr_nl rtnl_local;

>From iftable.c:

static struct ifindex_map *ifindex_map[16];

-- 
The dawn of the fourth age of Linux firewalling is coming; a time of
great struggle and heroic deeds -- J.Kadlecsik got inspired by J.Morris

Re: [Patch 1/2] libnfnetlink, iface conversion to string

[Eric Leblond]

[-- Attachment #1: Type: text/plain, Size: 968 bytes --]

Le vendredi 19 janvier 2007 à 18:38 +0100, Pablo Neira Ayuso a écrit :
> Patrick McHardy wrote:
> > Eric Leblond wrote:
> >>  /* rtnl_receive - receive netlink packets from rtnetlink socket */
> >> -int rtnl_receive()
> >> +int rtnl_receive(int rtnl_fd)
> > 
> > header file needs to be fixed as well. Is the global rtnl_fd still
> > needed?
> 
> Indeed. Eric, all this global stuff below must dissapear at all, from
> rtnl.c:

I totally agree. Patch was prefixed with RFC because I wanted to see if
API was correct. As there is no problem from this point, I will do the
needed cleaning.

All my apologies to Patrick, a bad vimrc and some mistakes make from
previous patch something awful.

> static struct ifindex_map *ifindex_map[16];

To answer to Patrick, I really don't like to have variable and type with
the same name, it's quiet confusing. That's why I change it to "struct
ifindex_map_t".

BR,
-- 
Eric Leblond <eric@inl.fr>
INL

[-- Attachment #2: Ceci est une partie de message numériquement signée --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [Patch 1/2] libnfnetlink, iface conversion to string

[Harald Welte]

[-- Attachment #1: Type: text/plain, Size: 966 bytes --]

On Fri, Jan 19, 2007 at 06:38:42PM +0100, Pablo Neira Ayuso wrote:
> Patrick McHardy wrote:
> > Eric Leblond wrote:
> >>  /* rtnl_receive - receive netlink packets from rtnetlink socket */
> >> -int rtnl_receive()
> >> +int rtnl_receive(int rtnl_fd)
> > 
> > header file needs to be fixed as well. Is the global rtnl_fd still
> > needed?
> 
> Indeed. Eric, all this global stuff below must dissapear at all, from
> rtnl.c:

sorry for leaving such a mess of code for you to clean up.  This was
initially from a daemon, nothing that would ever clutter somebody elses'
namespace.

-- 
- Harald Welte <laforge@netfilter.org>                 http://netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [Patch 1/2] libnfnetlink, iface conversion to string

[Eric Leblond]

[-- Attachment #1: Type: text/plain, Size: 1327 bytes --]

Hi,

> On Fri, Jan 19, 2007 at 06:38:42PM +0100, Pablo Neira Ayuso wrote:
>> Patrick McHardy wrote:
>> Indeed. Eric, all this global stuff below must dissapear at all, from
>> rtnl.c:

I've done a big cleaning and I hope I will not annoy Patrick with my
coding style problem this time.

I've introduced two structures:

struct rtnl_inst {
       int rtnl_fd;
       int rtnl_seq;
       int rtnl_dump;
       struct sockaddr_nl rtnl_local;
       struct rtnl_handler *handlers;
};

struct nlif_inst {
       struct ifindex_map *ifindex_map[16];
       struct rtnl_inst *rtnl_inst;
       struct rtnl_handler *ifadd_handler;
       struct rtnl_handler *ifdel_handler;
};

which hide all previously defined global variables.

The list of exported functions is the following:

struct nlif_inst *nlif_table_init(void);
void nlif_table_fini(struct nlif_inst *orig);
int nlif_get_fd(struct nlif_inst *nlif_inst); /* to be used in select */
int nlif_treat_msg(struct nlif_inst *nlif_inst);

char *nlif_index_2name(struct nlif_inst *nlif_inst, unsigned int index);

I've just prefixed previously proposed functions with nlif_inst...

> sorry for leaving such a mess of code for you to clean up.  This was
> initially from a daemon, nothing that would ever clutter somebody elses'
> namespace.

No pb.

BR,
--
Eric Leblond <eric@inl.fr>

[-- Attachment #2: libnfnetlink-ifacename.diff --]
[-- Type: text/plain, Size: 14502 bytes --]

=== include/libnfnetlink/libnfnetlink.h
==================================================================
--- include/libnfnetlink/libnfnetlink.h	(revision 5686)
+++ include/libnfnetlink/libnfnetlink.h	(local)
@@ -176,6 +176,22 @@
 
 extern void nfnl_dump_packet(struct nlmsghdr *, int, char *);
 
+struct nlif_inst {
+	struct ifindex_map *ifindex_map[16];
+	struct rtnl_inst *rtnl_inst;
+	struct rtnl_handler *ifadd_handler;
+	struct rtnl_handler *ifdel_handler;
+};
+
+struct nlif_inst *nlif_table_init(void);
+void nlif_table_fini(struct nlif_inst *orig);
+int nlif_get_fd(struct nlif_inst *nlif_inst);
+
+
+char *nlif_index_2name(struct nlif_inst *nlif_inst, unsigned int index);
+int nlif_treat_msg(struct nlif_inst *nlif_inst);
+
+
 /* Pablo: What is the equivalence of be64_to_cpu in userspace?
  * 
  * Harald: Good question.  I don't think there's a standard way [yet?], 
=== src/Makefile.am
==================================================================
--- src/Makefile.am	(revision 5686)
+++ src/Makefile.am	(local)
@@ -8,4 +8,4 @@
 
 libnfnetlink_la_LDFLAGS = -Wc,-nostartfiles	\
 			  -version-info $(LIBVERSION)
-libnfnetlink_la_SOURCES = libnfnetlink.c 
+libnfnetlink_la_SOURCES = libnfnetlink.c iftable.c rtnl.c
=== src/iftable.c
==================================================================
--- src/iftable.c	(revision 5686)
+++ src/iftable.c	(local)
@@ -19,6 +19,7 @@
 
 #include <linux/netdevice.h>
 
+#include <libnfnetlink/libnfnetlink.h>
 #include "rtnl.h"
 
 #define iftb_log(x, ...)
@@ -34,28 +35,6 @@
 	char		name[16];
 };
 
-static struct ifindex_map *ifindex_map[16];
-
-/* iftable_dump - Dump the interface table to a given file stream
- * @outfd:	file stream to which table should be dumped
- */
-int iftable_dump(FILE *outfd)
-{
-	int i;
-
-	for (i = 0; i < 16; i++) {
-		struct ifindex_map *im;
-		for (im = ifindex_map[i]; im; im = im->next) {
-			fprintf(outfd, "%u %s", im->index, im->name);
-			if (!(im->flags & IFF_UP))
-				fputs(" DOWN", outfd);
-			fputc('\n', outfd);
-		}
-	}
-	fflush(outfd);
-	return 0;
-}
-
 /* iftable_add - Add/Update an entry to/in the interface table
  * @n:		netlink message header of a RTM_NEWLINK message
  * @arg:	not used
@@ -69,6 +48,7 @@
 	struct ifinfomsg *ifi_msg = NLMSG_DATA(n);
 	struct ifindex_map *im, **imp;
 	struct rtattr *cb[IFLA_MAX+1];
+	struct nlif_inst *nlif_inst = (struct nlif_inst *)arg;
 
 	if (n->nlmsg_type != RTM_NEWLINK)
 		return -1;
@@ -87,7 +67,7 @@
 	}
 
 	hash = ifi_msg->ifi_index&0xF;
-	for (imp = &ifindex_map[hash]; (im=*imp)!=NULL; imp = &im->next) {
+	for (imp = &((nlif_inst->ifindex_map)[hash]); (im=*imp)!=NULL; imp = &im->next) {
 		if (im->index == ifi_msg->ifi_index) {
 			iftb_log(LOG_DEBUG,
 				 "updating iftable (ifindex=%u)", im->index);
@@ -136,7 +116,7 @@
 {
 	struct ifinfomsg *ifi_msg = NLMSG_DATA(n);
 	struct rtattr *cb[IFLA_MAX+1];
-	struct ifindex_map *im;
+	struct nlif_inst *nlif_inst = (struct nlif_inst *)arg;
 
 	if (n->nlmsg_type != RTM_DELLINK) {
 		iftb_log(LOG_ERROR,
@@ -153,23 +133,22 @@
 	memset(&cb, 0, sizeof(cb));
 	rtnl_parse_rtattr(cb, IFLA_MAX, IFLA_RTA(ifi_msg), IFLA_PAYLOAD(n));
 
-	/* FIXME */
+	/* FIXME really suppress entry */
 
 	return 1;
 }
-	
-/* ifindex_2name - get the name for an ifindex
+/* nl_ifindex_2name - get the name for an ifindex
  * @index:	ifindex to be resolved
  *
  * Return value: character string containing name of interface
  */
-char *ifindex_2name(unsigned int index)
+char *nlif_index_2name(struct nlif_inst *nlif_inst, unsigned int index)
 {
 	struct ifindex_map *im;
 
 	if (index == 0)
 		return "*";
-	for (im = ifindex_map[index&0xF]; im; im = im->next)
+	for (im = (nlif_inst->ifindex_map)[index&0xF]; im; im = im->next)
 		if (im->index == index)
 			return im->name;
 
@@ -181,11 +160,11 @@
  *
  * Return value: -1 if interface unknown, 1 if interface up, 0 if not.
  */
-int  iftable_up(unsigned int index)
+int  iftable_up(struct nlif_inst *nlif_inst, unsigned int index)
 {
 	struct ifindex_map *im;
 
-	for (im = ifindex_map[index&0xF]; im; im = im->next) {
+	for (im = nlif_inst->ifindex_map[index&0xF]; im; im = im->next) {
 		if (im->index == index) {
 			if (im->flags & IFF_UP)
 				return 1;
@@ -196,34 +175,52 @@
 	return -1;
 }
 
-static struct rtnl_handler handlers[] = {
-	{ .nlmsg_type = RTM_NEWLINK, .handlefn = &iftable_add },
-	{ .nlmsg_type = RTM_DELLINK, .handlefn = &iftable_del },
-};
 
-static int init_or_fini(int fini)
+static struct nlif_inst *init_or_fini(struct nlif_inst *orig)
 {
+	struct nlif_inst *nlif_inst;
 	int ret = 0;
 
-	if (fini)
+	if (orig) {
+		nlif_inst = orig;
 		goto cleanup;
+	}
 
-	if (rtnl_handler_register(&handlers[0]) < 0) {
+		
+	nlif_inst = calloc(1,  sizeof(struct nlif_inst));
+	if (!nlif_inst)
+		goto cleanup_none;
+
+	nlif_inst->ifadd_handler = calloc(1, sizeof(struct rtnl_handler));
+	nlif_inst->ifadd_handler->nlmsg_type = RTM_NEWLINK;
+	nlif_inst->ifadd_handler->handlefn = &iftable_add;
+	nlif_inst->ifadd_handler->arg = nlif_inst;
+	nlif_inst->ifdel_handler = calloc(1, sizeof(struct rtnl_handler));
+	nlif_inst->ifdel_handler->nlmsg_type = RTM_DELLINK;
+	nlif_inst->ifdel_handler->handlefn = &iftable_del;
+	nlif_inst->ifdel_handler->arg = nlif_inst;
+
+	nlif_inst->rtnl_inst = rtnl_init();
+
+	if (! nlif_inst->rtnl_inst)
+		goto cleanup_none;
+
+	if (rtnl_handler_register(nlif_inst->rtnl_inst, nlif_inst->ifadd_handler) < 0) {
 		ret = -1;
 		goto cleanup_none;
 	}
 
-	if (rtnl_handler_register(&handlers[1]) < 0) {
+	if (rtnl_handler_register(nlif_inst->rtnl_inst, nlif_inst->ifdel_handler) < 0) {
 		ret = -1;
 		goto cleanup_0;
 	}
 
-	if (rtnl_dump_type(RTM_GETLINK) < 0) {
+	if (rtnl_dump_type(nlif_inst->rtnl_inst, RTM_GETLINK) < 0) {
 		ret = -1;
 		goto cleanup_1;
 	}
 
-	return 0;
+	return nlif_inst;
 
 #if 0
 	if (rtnl_wilddump_requet(rtnl_fd, AF_UNSPEC, RTM_GETLINK) < 0) {
@@ -236,26 +233,52 @@
 cleanup:
 
 cleanup_1:
-	rtnl_handler_unregister(&handlers[1]);
+	rtnl_handler_unregister(nlif_inst->rtnl_inst, nlif_inst->ifadd_handler);
+	free(nlif_inst->ifadd_handler);
 cleanup_0:
-	rtnl_handler_unregister(&handlers[0]);
+	rtnl_handler_unregister(nlif_inst->rtnl_inst, nlif_inst->ifdel_handler);
+	free(nlif_inst->ifdel_handler);
+	rtnl_fini(nlif_inst->rtnl_inst);
+	free(nlif_inst);
+
 cleanup_none:
-	return ret;
+	return nlif_inst;
 }
 
-/* iftable_init - Initialize interface table
+/** nl_iftable_init - Initialize interface table
+ *
+ * Initialize rtnl interface and interface table
+ *
+ * \return file descriptor to netlink socket
  */
-int iftable_init(void)
+struct nlif_inst *nlif_table_init(void)
 {
 	iftb_log(LOG_DEBUG, "%s", __FUNCTION__);
-	return init_or_fini(0);
+	return init_or_fini(NULL);
 }
 
-/* iftable_fini - Destructor of interface table
+/* nl_iftable_fini - Destructor of interface table
  */
-void iftable_fini(void)
+void nlif_table_fini(struct nlif_inst *nlif_inst)
 {
-	init_or_fini(1);
+	init_or_fini(nlif_inst);
 }
 
+/**
+ * \return 0 if OK
+ */
+int nlif_treat_msg(struct nlif_inst *nlif_inst)
+{
+	if (nlif_inst && nlif_inst->rtnl_inst)
+		return rtnl_receive(nlif_inst->rtnl_inst);
+	else
+		return -1;
+}
 
+int nlif_get_fd(struct nlif_inst *nlif_inst)
+{
+	if (nlif_inst && nlif_inst->rtnl_inst)
+		return nlif_inst->rtnl_inst->rtnl_fd;
+	else
+		return -1;
+}
=== src/rtnl.c
==================================================================
--- src/rtnl.c	(revision 5686)
+++ src/rtnl.c	(local)
@@ -1,6 +1,8 @@
 /* rtnl - rtnetlink utility functions
  *
  * (C) 2004 by Astaro AG, written by Harald Welte <hwelte@astaro.com>
+ * 
+ * Adapted to nfnetlink by Eric Leblond <eric@inl.fr>
  *
  * This software is free software and licensed under GNU GPLv2. 
  *
@@ -26,26 +28,21 @@
 
 #define rtnl_log(x, ...)
 
-static int rtnl_fd;
-static int rtnl_seq = 0;
-static int rtnl_dump;
-static struct sockaddr_nl rtnl_local;
-
-static struct rtnl_handler *handlers = NULL;
-
-static inline struct rtnl_handler *find_handler(u_int16_t type)
+static inline struct rtnl_handler *find_handler(struct rtnl_inst *rtnl_inst,
+							u_int16_t type)
 {
 	struct rtnl_handler *h;
-	for (h = handlers; h; h = h->next) {
+	for (h = rtnl_inst->handlers; h; h = h->next) {
 		if (h->nlmsg_type == type)
 			return h;
 	}
 	return NULL;
 }
 
-static int call_handler(u_int16_t type, struct nlmsghdr *hdr)
+static int call_handler(struct rtnl_inst *rtnl_inst, u_int16_t type,
+					struct nlmsghdr *hdr)
 {
-	struct rtnl_handler *h = find_handler(type);
+	struct rtnl_handler *h = find_handler(rtnl_inst, type);
 
 	if (!h) {
 		rtnl_log(LOG_DEBUG, "no registered handler for type %u",
@@ -59,31 +56,31 @@
 /* rtnl_handler_register - register handler for given nlmsg type
  * @hdlr:	handler structure
  */
-int rtnl_handler_register(struct rtnl_handler *hdlr)
+int rtnl_handler_register(struct rtnl_inst *rtnl_inst, struct rtnl_handler *hdlr)
 {
 	rtnl_log(LOG_DEBUG, "registering handler for type %u",
 		 hdlr->nlmsg_type);
-	hdlr->next = handlers;
-	handlers = hdlr;
+	hdlr->next = rtnl_inst->handlers;
+	rtnl_inst->handlers = hdlr;
 	return 1;
 }
 
 /* rtnl_handler_unregister - unregister handler for given nlmst type
  * @hdlr:	handler structure
  */
-int rtnl_handler_unregister(struct rtnl_handler *hdlr)
+int rtnl_handler_unregister(struct rtnl_inst *rtnl_inst, struct rtnl_handler *hdlr)
 {
 	struct rtnl_handler *h, *prev = NULL;
 
 	rtnl_log(LOG_DEBUG, "unregistering handler for type %u",
 		 hdlr->nlmsg_type);
 
-	for (h = handlers; h; h = h->next) {
+	for (h = rtnl_inst->handlers; h; h = h->next) {
 		if (h == hdlr) {
 			if (prev)
 				prev->next = h->next;
 			else
-				handlers = h->next;
+				rtnl_inst->handlers = h->next;
 			return 1;
 		}
 		prev = h;
@@ -107,7 +104,7 @@
 /* rtnl_dump_type - ask rtnetlink to dump a specific table
  * @type:	type of table to be dumped
  */
-int rtnl_dump_type(unsigned int type)
+int rtnl_dump_type(struct rtnl_inst *rtnl_inst, unsigned int type)
 {
         struct {
                 struct nlmsghdr nlh;
@@ -123,15 +120,15 @@
         req.nlh.nlmsg_type = type;
         req.nlh.nlmsg_flags = NLM_F_ROOT|NLM_F_MATCH|NLM_F_REQUEST;
         req.nlh.nlmsg_pid = 0;
-        req.nlh.nlmsg_seq = rtnl_dump = ++rtnl_seq;
+        req.nlh.nlmsg_seq = rtnl_inst->rtnl_dump = ++(rtnl_inst->rtnl_seq);
         req.g.rtgen_family = AF_INET;
 
-        return sendto(rtnl_fd, (void*)&req, sizeof(req), 0, 
+        return sendto(rtnl_inst->rtnl_fd, (void*)&req, sizeof(req), 0, 
 		      (struct sockaddr*)&nladdr, sizeof(nladdr));
 }
 
 /* rtnl_receive - receive netlink packets from rtnetlink socket */
-int rtnl_receive()
+int rtnl_receive(struct rtnl_inst *rtnl_inst)
 {
 	int status;
 	char buf[8192];
@@ -146,7 +143,7 @@
 		0
 	};
 
-	status = recvmsg(rtnl_fd, &msg, 0);
+	status = recvmsg(rtnl_inst->rtnl_fd, &msg, 0);
 	if (status < 0) {
 		if (errno == EINTR)
 			return 0;
@@ -184,7 +181,7 @@
 			return -1;
 		}
 
-		if (call_handler(h->nlmsg_type, h) == 0) 
+		if (call_handler(rtnl_inst, h->nlmsg_type, h) == 0) 
 			rtnl_log(LOG_NOTICE, "unhandled nlmsg_type %u",
 				 h->nlmsg_type);
 		h = NLMSG_NEXT(h, status);
@@ -193,52 +190,59 @@
 }
 
 /* rtnl_init - constructor of rtnetlink module */
-int rtnl_init(void)
+struct rtnl_inst *rtnl_init(void)
 {
 	int addr_len;
+	struct rtnl_inst *rtnl_inst = calloc(1, sizeof(struct rtnl_inst));
 
-	rtnl_local.nl_pid = getpid();
-	rtnl_fd = socket(AF_NETLINK, SOCK_RAW, NETLINK_ROUTE);
-	if (rtnl_fd < 0) {
+	if (! rtnl_inst)
+		return NULL;
+
+	rtnl_inst->rtnl_seq = 0;
+	rtnl_inst->handlers = NULL;
+
+	rtnl_inst->rtnl_local.nl_pid = getpid();
+	rtnl_inst->rtnl_fd = socket(AF_NETLINK, SOCK_RAW, NETLINK_ROUTE);
+	if (rtnl_inst->rtnl_fd < 0) {
 		rtnl_log(LOG_ERROR, "unable to create rtnetlink socket");
 		return -1;
 	}
 
-	memset(&rtnl_local, 0, sizeof(rtnl_local));
-	rtnl_local.nl_family = AF_NETLINK;
-	rtnl_local.nl_groups = RTMGRP_IPV4_ROUTE|RTMGRP_IPV4_IFADDR|RTMGRP_LINK;
-
-	if (bind(rtnl_fd, (struct sockaddr *)&rtnl_local, sizeof(rtnl_local)) < 0) {
+	memset(&(rtnl_inst->rtnl_local), 0, sizeof(rtnl_inst->rtnl_local));
+	rtnl_inst->rtnl_local.nl_family = AF_NETLINK;
+	rtnl_inst->rtnl_local.nl_groups = RTMGRP_IPV4_ROUTE|RTMGRP_IPV4_IFADDR|RTMGRP_LINK;
+	if (bind(rtnl_inst->rtnl_fd, (struct sockaddr *)&(rtnl_inst->rtnl_local), sizeof(rtnl_inst->rtnl_local)) < 0) {
 		rtnl_log(LOG_ERROR, "unable to bind rtnetlink socket");
 		return -1;
 	}
 
-	addr_len = sizeof(rtnl_local);
-	if (getsockname(rtnl_fd, (struct sockaddr *)&rtnl_local, 
+	addr_len = sizeof(rtnl_inst->rtnl_local);
+	if (getsockname(rtnl_inst->rtnl_fd, (struct sockaddr *)&(rtnl_inst->rtnl_local), 
 			&addr_len) < 0) {
 		rtnl_log(LOG_ERROR, "cannot gescockname(rtnl_socket)");
 		return -1;
 	}
 
-	if (addr_len != sizeof(rtnl_local)) {
+	if (addr_len != sizeof(rtnl_inst->rtnl_local)) {
 		rtnl_log(LOG_ERROR, "invalid address size %u", addr_len);
 		return -1;
 	}
 
-	if (rtnl_local.nl_family != AF_NETLINK) {
-		rtnl_log(LOG_ERROR, "invalid AF %u", rtnl_local.nl_family);
+	if (rtnl_inst->rtnl_local.nl_family != AF_NETLINK) {
+		rtnl_log(LOG_ERROR, "invalid AF %u", rtnl_inst->rtnl_local.nl_family);
 		return -1;
 	}
 
-	rtnl_seq = time(NULL);
+	rtnl_inst->rtnl_seq = time(NULL);
 
-	return rtnl_fd;
+	return rtnl_inst;
 }
 
 /* rtnl_fini - destructor of rtnetlink module */
-void rtnl_fini(void)
+void rtnl_fini(struct rtnl_inst *rtnl_inst)
 {
-	close(rtnl_fd);
+	close(rtnl_inst->rtnl_fd);
+	free(rtnl_inst);
 	return;
 }
 
=== src/rtnl.h
==================================================================
--- src/rtnl.h	(revision 5686)
+++ src/rtnl.h	(local)
@@ -12,17 +12,27 @@
 	void		*arg;
 };
 
+struct rtnl_inst {
+	int rtnl_fd;
+	int rtnl_seq;
+	int rtnl_dump;
+	struct sockaddr_nl rtnl_local;
+	struct rtnl_handler *handlers;
+};
 
+
+
+
 /* api for handler plugins */
-int rtnl_handler_register(struct rtnl_handler *hdlr);
-int rtnl_handler_unregister(struct rtnl_handler *hdlr);
+int rtnl_handler_register(struct rtnl_inst *rtnl_inst,struct rtnl_handler *hdlr);
+int rtnl_handler_unregister(struct rtnl_inst *rtnl_inst, struct rtnl_handler *hdlr);
 int rtnl_parse_rtattr(struct rtattr *tb[], int max, struct rtattr *rta, int len);
-int rtnl_dump_type(unsigned int type);
+int rtnl_dump_type(struct rtnl_inst *rtnl_inst, unsigned int type);
 
 /* api for core program */
-int rtnl_init(void);
-void rtnl_fini(void);
-int rtnl_receive();
+struct rtnl_inst *rtnl_init(void);
+void rtnl_fini(struct rtnl_inst *rtnl_inst);
+int rtnl_receive(struct rtnl_inst *rtnl_inst);
   
 
 #endif

Re: [Patch 1/2] libnfnetlink, iface conversion to string

[Patrick McHardy]

Eric Leblond wrote:
> Hi,
> 
> 
>>On Fri, Jan 19, 2007 at 06:38:42PM +0100, Pablo Neira Ayuso wrote:
>>
>>>Patrick McHardy wrote:
>>>Indeed. Eric, all this global stuff below must dissapear at all, from
>>>rtnl.c:
> 
> 
> I've done a big cleaning and I hope I will not annoy Patrick with my
> coding style problem this time.

Thanks Eric, no further objections from my side. Pablo is more familiar
with this code than myself, so I'll leave it to him to apply it.

Re: [Patch 1/2] libnfnetlink, iface conversion to string

[Pablo Neira Ayuso]

Hi Eric,

Eric Leblond wrote:
>> On Fri, Jan 19, 2007 at 06:38:42PM +0100, Pablo Neira Ayuso wrote:
>>> Patrick McHardy wrote:
>>> Indeed. Eric, all this global stuff below must dissapear at all, from
>>> rtnl.c:
> 
> I've done a big cleaning and I hope I will not annoy Patrick with my
> coding style problem this time.
> 
> I've introduced two structures:
> 
> struct rtnl_inst {
              ^^^^
please, rename this to _handle

>        int rtnl_fd;
>        int rtnl_seq;
>        int rtnl_dump;
>        struct sockaddr_nl rtnl_local;
>        struct rtnl_handler *handlers;
> };
> 
> struct nlif_inst {
              ^^^^
same thing here

>        struct ifindex_map *ifindex_map[16];
>        struct rtnl_inst *rtnl_inst;
>        struct rtnl_handler *ifadd_handler;
>        struct rtnl_handler *ifdel_handler;
> };
> 
> which hide all previously defined global variables.
> 
> The list of exported functions is the following:
> 
> struct nlif_inst *nlif_table_init(void);
> void nlif_table_fini(struct nlif_inst *orig);
> int nlif_get_fd(struct nlif_inst *nlif_inst); /* to be used in select */
> int nlif_treat_msg(struct nlif_inst *nlif_inst);

I prefer keeping it homogeneous with libnfnetlink, I would rename the
functions above to:

struct nlif_handle *nlif_open(void);
void nlif_close(struct nlif_handle *h);
int nlif_fd(struct nlif_handle *h);
int nlif_catch(struct nlif_handle *h);
char *nlif_index2name(struct nlif_handle *h, unsigned int index);

BTW, could you spend some time on documenting the API in docbook format?
I know that this is not available in all netfilter libraries but I'd
appreciate it, we should start getting use to do these things ;)

> === include/libnfnetlink/libnfnetlink.h
> ==================================================================
> --- include/libnfnetlink/libnfnetlink.h	(revision 5686)
> +++ include/libnfnetlink/libnfnetlink.h	(local)
> @@ -176,6 +176,22 @@
>  
>  extern void nfnl_dump_packet(struct nlmsghdr *, int, char *);
>  
> +struct nlif_inst {
> +	struct ifindex_map *ifindex_map[16];
> +	struct rtnl_inst *rtnl_inst;
> +	struct rtnl_handler *ifadd_handler;
> +	struct rtnl_handler *ifdel_handler;
> +};

move this definition to an internal header file and just put an empty
definition inside libnfnetlink.h like:

struct nlif_handle;

This is better for encapsulation, if we have to modify the layout later
for whatever reason, we will not break backward binary compatibility. We
just give a pointer to an object with private attributes that are only
accesible through the appropiate functions like the nlif_fd() getter.

> === src/iftable.c
> ==================================================================
> --- src/iftable.c	(revision 5686)
> +++ src/iftable.c	(local)
> @@ -19,6 +19,7 @@
>  
>  #include <linux/netdevice.h>
>  
> +#include <libnfnetlink/libnfnetlink.h>
>  #include "rtnl.h"
>  
>  #define iftb_log(x, ...)
> @@ -34,28 +35,6 @@
>  	char		name[16];
>  };
>  
> -static struct ifindex_map *ifindex_map[16];
> -
> -/* iftable_dump - Dump the interface table to a given file stream
> - * @outfd:	file stream to which table should be dumped
> - */
> -int iftable_dump(FILE *outfd)
> -{
> -	int i;
> -
> -	for (i = 0; i < 16; i++) {
> -		struct ifindex_map *im;
> -		for (im = ifindex_map[i]; im; im = im->next) {
> -			fprintf(outfd, "%u %s", im->index, im->name);
> -			if (!(im->flags & IFF_UP))
> -				fputs(" DOWN", outfd);
> -			fputc('\n', outfd);
> -		}
> -	}
> -	fflush(outfd);
> -	return 0;
> -}

We can introduce some kind of iterator, later:
int nlif_iterate(struct nlif_handle *h, int (*iterate)(void *data));

That's enough for now, we can revisit this later and clean it up a bit
more. Thanks.

-- 
The dawn of the fourth age of Linux firewalling is coming; a time of
great struggle and heroic deeds -- J.Kadlecsik got inspired by J.Morris

Re: [Patch 1/2] libnfnetlink, iface conversion to string

[Eric Leblond]

[-- Attachment #1: Type: text/plain, Size: 4052 bytes --]

Hi Pablo,

Excellent remarks as usual...

Here's the modified patch.

> Hi Eric,
>
> Eric Leblond wrote:
>>> On Fri, Jan 19, 2007 at 06:38:42PM +0100, Pablo Neira Ayuso wrote:
>>>> Patrick McHardy wrote:
>>>> Indeed. Eric, all this global stuff below must dissapear at all, from
>>>> rtnl.c:
>>
>> I've done a big cleaning and I hope I will not annoy Patrick with my
>> coding style problem this time.
>>
>> I've introduced two structures:
>>
>> struct rtnl_inst {
>               ^^^^
> please, rename this to _handle
>
>>        int rtnl_fd;
>>        int rtnl_seq;
>>        int rtnl_dump;
>>        struct sockaddr_nl rtnl_local;
>>        struct rtnl_handler *handlers;
>> };
>>
>> struct nlif_inst {
>               ^^^^
> same thing here
>
>>        struct ifindex_map *ifindex_map[16];
>>        struct rtnl_inst *rtnl_inst;
>>        struct rtnl_handler *ifadd_handler;
>>        struct rtnl_handler *ifdel_handler;
>> };
>>
>> which hide all previously defined global variables.
>>
>> The list of exported functions is the following:
>>
>> struct nlif_inst *nlif_table_init(void);
>> void nlif_table_fini(struct nlif_inst *orig);
>> int nlif_get_fd(struct nlif_inst *nlif_inst); /* to be used in select */
>> int nlif_treat_msg(struct nlif_inst *nlif_inst);
>
> I prefer keeping it homogeneous with libnfnetlink, I would rename the
> functions above to:
>
> struct nlif_handle *nlif_open(void);
> void nlif_close(struct nlif_handle *h);
> int nlif_fd(struct nlif_handle *h);
> int nlif_catch(struct nlif_handle *h);
> char *nlif_index2name(struct nlif_handle *h, unsigned int index);
>
> BTW, could you spend some time on documenting the API in docbook format?
> I know that this is not available in all netfilter libraries but I'd
> appreciate it, we should start getting use to do these things ;)
>
>> === include/libnfnetlink/libnfnetlink.h
>> ==================================================================
>> --- include/libnfnetlink/libnfnetlink.h	(revision 5686)
>> +++ include/libnfnetlink/libnfnetlink.h	(local)
>> @@ -176,6 +176,22 @@
>>
>>  extern void nfnl_dump_packet(struct nlmsghdr *, int, char *);
>>
>> +struct nlif_inst {
>> +	struct ifindex_map *ifindex_map[16];
>> +	struct rtnl_inst *rtnl_inst;
>> +	struct rtnl_handler *ifadd_handler;
>> +	struct rtnl_handler *ifdel_handler;
>> +};
>
> move this definition to an internal header file and just put an empty
> definition inside libnfnetlink.h like:
>
> struct nlif_handle;
>
> This is better for encapsulation, if we have to modify the layout later
> for whatever reason, we will not break backward binary compatibility. We
> just give a pointer to an object with private attributes that are only
> accesible through the appropiate functions like the nlif_fd() getter.
>
>> === src/iftable.c
>> ==================================================================
>> --- src/iftable.c	(revision 5686)
>> +++ src/iftable.c	(local)
>> @@ -19,6 +19,7 @@
>>
>>  #include <linux/netdevice.h>
>>
>> +#include <libnfnetlink/libnfnetlink.h>
>>  #include "rtnl.h"
>>
>>  #define iftb_log(x, ...)
>> @@ -34,28 +35,6 @@
>>  	char		name[16];
>>  };
>>
>> -static struct ifindex_map *ifindex_map[16];
>> -
>> -/* iftable_dump - Dump the interface table to a given file stream
>> - * @outfd:	file stream to which table should be dumped
>> - */
>> -int iftable_dump(FILE *outfd)
>> -{
>> -	int i;
>> -
>> -	for (i = 0; i < 16; i++) {
>> -		struct ifindex_map *im;
>> -		for (im = ifindex_map[i]; im; im = im->next) {
>> -			fprintf(outfd, "%u %s", im->index, im->name);
>> -			if (!(im->flags & IFF_UP))
>> -				fputs(" DOWN", outfd);
>> -			fputc('\n', outfd);
>> -		}
>> -	}
>> -	fflush(outfd);
>> -	return 0;
>> -}
>
> We can introduce some kind of iterator, later:
> int nlif_iterate(struct nlif_handle *h, int (*iterate)(void *data));
>
> That's enough for now, we can revisit this later and clean it up a bit
> more. Thanks.
>
> --
> The dawn of the fourth age of Linux firewalling is coming; a time of
> great struggle and heroic deeds -- J.Kadlecsik got inspired by J.Morris
>

[-- Attachment #2: libnfnetlink-ifacename.diff --]
[-- Type: text/plain, Size: 16011 bytes --]

=== include/libnfnetlink/libnfnetlink.h
==================================================================
--- include/libnfnetlink/libnfnetlink.h	(revision 5686)
+++ include/libnfnetlink/libnfnetlink.h	(local)
@@ -176,6 +176,17 @@
 
 extern void nfnl_dump_packet(struct nlmsghdr *, int, char *);
 
+struct nlif_handle;
+
+struct nlif_handle *nlif_open(void);
+void nlif_close(struct nlif_handle *orig);
+int nlif_fd(struct nlif_handle *nlif_handle);
+
+
+char *nlif_index2name(struct nlif_handle *nlif_handle, unsigned int index);
+int nlif_catch(struct nlif_handle *nlif_handle);
+
+
 /* Pablo: What is the equivalence of be64_to_cpu in userspace?
  * 
  * Harald: Good question.  I don't think there's a standard way [yet?], 
=== src/Makefile.am
==================================================================
--- src/Makefile.am	(revision 5686)
+++ src/Makefile.am	(local)
@@ -8,4 +8,4 @@
 
 libnfnetlink_la_LDFLAGS = -Wc,-nostartfiles	\
 			  -version-info $(LIBVERSION)
-libnfnetlink_la_SOURCES = libnfnetlink.c 
+libnfnetlink_la_SOURCES = libnfnetlink.c iftable.c rtnl.c
=== src/iftable.c
==================================================================
--- src/iftable.c	(revision 5686)
+++ src/iftable.c	(local)
@@ -19,6 +19,7 @@
 
 #include <linux/netdevice.h>
 
+#include <libnfnetlink/libnfnetlink.h>
 #include "rtnl.h"
 
 #define iftb_log(x, ...)
@@ -34,27 +35,14 @@
 	char		name[16];
 };
 
-static struct ifindex_map *ifindex_map[16];
+struct nlif_handle {
+	struct ifindex_map *ifindex_map[16];
+	struct rtnl_handle *rtnl_handle;
+	struct rtnl_handler *ifadd_handler;
+	struct rtnl_handler *ifdel_handler;
+};
 
-/* iftable_dump - Dump the interface table to a given file stream
- * @outfd:	file stream to which table should be dumped
- */
-int iftable_dump(FILE *outfd)
-{
-	int i;
 
-	for (i = 0; i < 16; i++) {
-		struct ifindex_map *im;
-		for (im = ifindex_map[i]; im; im = im->next) {
-			fprintf(outfd, "%u %s", im->index, im->name);
-			if (!(im->flags & IFF_UP))
-				fputs(" DOWN", outfd);
-			fputc('\n', outfd);
-		}
-	}
-	fflush(outfd);
-	return 0;
-}
 
 /* iftable_add - Add/Update an entry to/in the interface table
  * @n:		netlink message header of a RTM_NEWLINK message
@@ -69,6 +57,7 @@
 	struct ifinfomsg *ifi_msg = NLMSG_DATA(n);
 	struct ifindex_map *im, **imp;
 	struct rtattr *cb[IFLA_MAX+1];
+	struct nlif_handle *nlif_handle = (struct nlif_handle *)arg;
 
 	if (n->nlmsg_type != RTM_NEWLINK)
 		return -1;
@@ -87,7 +76,7 @@
 	}
 
 	hash = ifi_msg->ifi_index&0xF;
-	for (imp = &ifindex_map[hash]; (im=*imp)!=NULL; imp = &im->next) {
+	for (imp = &((nlif_handle->ifindex_map)[hash]); (im=*imp)!=NULL; imp = &im->next) {
 		if (im->index == ifi_msg->ifi_index) {
 			iftb_log(LOG_DEBUG,
 				 "updating iftable (ifindex=%u)", im->index);
@@ -136,7 +125,9 @@
 {
 	struct ifinfomsg *ifi_msg = NLMSG_DATA(n);
 	struct rtattr *cb[IFLA_MAX+1];
-	struct ifindex_map *im;
+	struct nlif_handle *nlif_handle = (struct nlif_handle *)arg;
+	struct ifindex_map *im, *ima, **imp;
+	unsigned int hash;
 
 	if (n->nlmsg_type != RTM_DELLINK) {
 		iftb_log(LOG_ERROR,
@@ -153,23 +144,43 @@
 	memset(&cb, 0, sizeof(cb));
 	rtnl_parse_rtattr(cb, IFLA_MAX, IFLA_RTA(ifi_msg), IFLA_PAYLOAD(n));
 
-	/* FIXME */
+	/* \todo Really suppress entry */
+	hash = ifi_msg->ifi_index&0xF;
+	for (ima = NULL, imp = &((nlif_handle->ifindex_map)[hash]); (im=*imp)!=NULL; imp = &im->next, ima=im) {
+		if (im->index == ifi_msg->ifi_index) {
+			iftb_log(LOG_DEBUG,
+				 "deleting iftable (ifindex=%u)", im->index);
+			break;
+		}
+	}
 
+	if (!im)
+		return 0;
+
+	if (ima)
+		ima->next = imp;
+	else
+		(nlif_handle->ifindex_map)[hash] = imp;
+	free(im);
+
+
 	return 1;
 }
-	
-/* ifindex_2name - get the name for an ifindex
- * @index:	ifindex to be resolved
+
+/** Get the name for an ifindex
  *
- * Return value: character string containing name of interface
+ * \param nlif_handle A pointer to a ::nlif_handle created
+ * \param index ifindex to be resolved
+ * \return character string containing name of interface, it needs to be duplicated
+ * for permanent use.
  */
-char *ifindex_2name(unsigned int index)
+char *nlif_index2name(struct nlif_handle *nlif_handle, unsigned int index)
 {
 	struct ifindex_map *im;
 
 	if (index == 0)
 		return "*";
-	for (im = ifindex_map[index&0xF]; im; im = im->next)
+	for (im = (nlif_handle->ifindex_map)[index&0xF]; im; im = im->next)
 		if (im->index == index)
 			return im->name;
 
@@ -181,11 +192,11 @@
  *
  * Return value: -1 if interface unknown, 1 if interface up, 0 if not.
  */
-int  iftable_up(unsigned int index)
+int  iftable_up(struct nlif_handle *nlif_handle, unsigned int index)
 {
 	struct ifindex_map *im;
 
-	for (im = ifindex_map[index&0xF]; im; im = im->next) {
+	for (im = nlif_handle->ifindex_map[index&0xF]; im; im = im->next) {
 		if (im->index == index) {
 			if (im->flags & IFF_UP)
 				return 1;
@@ -196,34 +207,52 @@
 	return -1;
 }
 
-static struct rtnl_handler handlers[] = {
-	{ .nlmsg_type = RTM_NEWLINK, .handlefn = &iftable_add },
-	{ .nlmsg_type = RTM_DELLINK, .handlefn = &iftable_del },
-};
 
-static int init_or_fini(int fini)
+static struct nlif_handle *init_or_fini(struct nlif_handle *orig)
 {
+	struct nlif_handle *nlif_handle;
 	int ret = 0;
 
-	if (fini)
+	if (orig) {
+		nlif_handle = orig;
 		goto cleanup;
+	}
 
-	if (rtnl_handler_register(&handlers[0]) < 0) {
+		
+	nlif_handle = calloc(1,  sizeof(struct nlif_handle));
+	if (!nlif_handle)
+		goto cleanup_none;
+
+	nlif_handle->ifadd_handler = calloc(1, sizeof(struct rtnl_handler));
+	nlif_handle->ifadd_handler->nlmsg_type = RTM_NEWLINK;
+	nlif_handle->ifadd_handler->handlefn = &iftable_add;
+	nlif_handle->ifadd_handler->arg = nlif_handle;
+	nlif_handle->ifdel_handler = calloc(1, sizeof(struct rtnl_handler));
+	nlif_handle->ifdel_handler->nlmsg_type = RTM_DELLINK;
+	nlif_handle->ifdel_handler->handlefn = &iftable_del;
+	nlif_handle->ifdel_handler->arg = nlif_handle;
+
+	nlif_handle->rtnl_handle = rtnl_init();
+
+	if (! nlif_handle->rtnl_handle)
+		goto cleanup_none;
+
+	if (rtnl_handler_register(nlif_handle->rtnl_handle, nlif_handle->ifadd_handler) < 0) {
 		ret = -1;
 		goto cleanup_none;
 	}
 
-	if (rtnl_handler_register(&handlers[1]) < 0) {
+	if (rtnl_handler_register(nlif_handle->rtnl_handle, nlif_handle->ifdel_handler) < 0) {
 		ret = -1;
 		goto cleanup_0;
 	}
 
-	if (rtnl_dump_type(RTM_GETLINK) < 0) {
+	if (rtnl_dump_type(nlif_handle->rtnl_handle, RTM_GETLINK) < 0) {
 		ret = -1;
 		goto cleanup_1;
 	}
 
-	return 0;
+	return nlif_handle;
 
 #if 0
 	if (rtnl_wilddump_requet(rtnl_fd, AF_UNSPEC, RTM_GETLINK) < 0) {
@@ -236,26 +265,63 @@
 cleanup:
 
 cleanup_1:
-	rtnl_handler_unregister(&handlers[1]);
+	rtnl_handler_unregister(nlif_handle->rtnl_handle, nlif_handle->ifadd_handler);
+	free(nlif_handle->ifadd_handler);
 cleanup_0:
-	rtnl_handler_unregister(&handlers[0]);
+	rtnl_handler_unregister(nlif_handle->rtnl_handle, nlif_handle->ifdel_handler);
+	free(nlif_handle->ifdel_handler);
+	rtnl_fini(nlif_handle->rtnl_handle);
+	free(nlif_handle);
+
 cleanup_none:
-	return ret;
+	return nlif_handle;
 }
 
-/* iftable_init - Initialize interface table
+/** Initialize interface table
+ *
+ * Initialize rtnl interface and interface table
+ * Call this before any nlif_* function
+ *
+ * \return file descriptor to netlink socket
  */
-int iftable_init(void)
+struct nlif_handle *nlif_open(void)
 {
 	iftb_log(LOG_DEBUG, "%s", __FUNCTION__);
-	return init_or_fini(0);
+	return init_or_fini(NULL);
 }
 
-/* iftable_fini - Destructor of interface table
+/** Destructor of interface table
+ *
+ * \param nlif_handle A pointer to a ::nlif_handle created 
+ * via nlif_open()
  */
-void iftable_fini(void)
+void nlif_close(struct nlif_handle *nlif_handle)
 {
-	init_or_fini(1);
+	init_or_fini(nlif_handle);
 }
 
+/** Treat message from netlink to modify interfaces table
+ *
+ * \param nlif_handle A pointer to a ::nlif_handle created
+ * \return 0 if OK
+ */
+int nlif_catch(struct nlif_handle *nlif_handle)
+{
+	if (nlif_handle && nlif_handle->rtnl_handle)
+		return rtnl_receive(nlif_handle->rtnl_handle);
+	else
+		return -1;
+}
 
+/** Treat message from netlink to modify interfaces table
+ *
+ * \param nlif_handle A pointer to a ::nlif_handle created
+ * \return The fd or -1 if there's an error
+ */
+int nlif_fd(struct nlif_handle *nlif_handle)
+{
+	if (nlif_handle && nlif_handle->rtnl_handle)
+		return nlif_handle->rtnl_handle->rtnl_fd;
+	else
+		return -1;
+}
=== src/rtnl.c
==================================================================
--- src/rtnl.c	(revision 5686)
+++ src/rtnl.c	(local)
@@ -1,6 +1,8 @@
 /* rtnl - rtnetlink utility functions
  *
  * (C) 2004 by Astaro AG, written by Harald Welte <hwelte@astaro.com>
+ * 
+ * Adapted to nfnetlink by Eric Leblond <eric@inl.fr>
  *
  * This software is free software and licensed under GNU GPLv2. 
  *
@@ -26,26 +28,21 @@
 
 #define rtnl_log(x, ...)
 
-static int rtnl_fd;
-static int rtnl_seq = 0;
-static int rtnl_dump;
-static struct sockaddr_nl rtnl_local;
-
-static struct rtnl_handler *handlers = NULL;
-
-static inline struct rtnl_handler *find_handler(u_int16_t type)
+static inline struct rtnl_handler *find_handler(struct rtnl_handle *rtnl_handle,
+							u_int16_t type)
 {
 	struct rtnl_handler *h;
-	for (h = handlers; h; h = h->next) {
+	for (h = rtnl_handle->handlers; h; h = h->next) {
 		if (h->nlmsg_type == type)
 			return h;
 	}
 	return NULL;
 }
 
-static int call_handler(u_int16_t type, struct nlmsghdr *hdr)
+static int call_handler(struct rtnl_handle *rtnl_handle, u_int16_t type,
+					struct nlmsghdr *hdr)
 {
-	struct rtnl_handler *h = find_handler(type);
+	struct rtnl_handler *h = find_handler(rtnl_handle, type);
 
 	if (!h) {
 		rtnl_log(LOG_DEBUG, "no registered handler for type %u",
@@ -59,31 +56,31 @@
 /* rtnl_handler_register - register handler for given nlmsg type
  * @hdlr:	handler structure
  */
-int rtnl_handler_register(struct rtnl_handler *hdlr)
+int rtnl_handler_register(struct rtnl_handle *rtnl_handle, struct rtnl_handler *hdlr)
 {
 	rtnl_log(LOG_DEBUG, "registering handler for type %u",
 		 hdlr->nlmsg_type);
-	hdlr->next = handlers;
-	handlers = hdlr;
+	hdlr->next = rtnl_handle->handlers;
+	rtnl_handle->handlers = hdlr;
 	return 1;
 }
 
 /* rtnl_handler_unregister - unregister handler for given nlmst type
  * @hdlr:	handler structure
  */
-int rtnl_handler_unregister(struct rtnl_handler *hdlr)
+int rtnl_handler_unregister(struct rtnl_handle *rtnl_handle, struct rtnl_handler *hdlr)
 {
 	struct rtnl_handler *h, *prev = NULL;
 
 	rtnl_log(LOG_DEBUG, "unregistering handler for type %u",
 		 hdlr->nlmsg_type);
 
-	for (h = handlers; h; h = h->next) {
+	for (h = rtnl_handle->handlers; h; h = h->next) {
 		if (h == hdlr) {
 			if (prev)
 				prev->next = h->next;
 			else
-				handlers = h->next;
+				rtnl_handle->handlers = h->next;
 			return 1;
 		}
 		prev = h;
@@ -107,7 +104,7 @@
 /* rtnl_dump_type - ask rtnetlink to dump a specific table
  * @type:	type of table to be dumped
  */
-int rtnl_dump_type(unsigned int type)
+int rtnl_dump_type(struct rtnl_handle *rtnl_handle, unsigned int type)
 {
         struct {
                 struct nlmsghdr nlh;
@@ -123,15 +120,15 @@
         req.nlh.nlmsg_type = type;
         req.nlh.nlmsg_flags = NLM_F_ROOT|NLM_F_MATCH|NLM_F_REQUEST;
         req.nlh.nlmsg_pid = 0;
-        req.nlh.nlmsg_seq = rtnl_dump = ++rtnl_seq;
+        req.nlh.nlmsg_seq = rtnl_handle->rtnl_dump = ++(rtnl_handle->rtnl_seq);
         req.g.rtgen_family = AF_INET;
 
-        return sendto(rtnl_fd, (void*)&req, sizeof(req), 0, 
+        return sendto(rtnl_handle->rtnl_fd, (void*)&req, sizeof(req), 0, 
 		      (struct sockaddr*)&nladdr, sizeof(nladdr));
 }
 
 /* rtnl_receive - receive netlink packets from rtnetlink socket */
-int rtnl_receive()
+int rtnl_receive(struct rtnl_handle *rtnl_handle)
 {
 	int status;
 	char buf[8192];
@@ -146,7 +143,7 @@
 		0
 	};
 
-	status = recvmsg(rtnl_fd, &msg, 0);
+	status = recvmsg(rtnl_handle->rtnl_fd, &msg, 0);
 	if (status < 0) {
 		if (errno == EINTR)
 			return 0;
@@ -184,7 +181,7 @@
 			return -1;
 		}
 
-		if (call_handler(h->nlmsg_type, h) == 0) 
+		if (call_handler(rtnl_handle, h->nlmsg_type, h) == 0) 
 			rtnl_log(LOG_NOTICE, "unhandled nlmsg_type %u",
 				 h->nlmsg_type);
 		h = NLMSG_NEXT(h, status);
@@ -193,52 +190,64 @@
 }
 
 /* rtnl_init - constructor of rtnetlink module */
-int rtnl_init(void)
+struct rtnl_handle *rtnl_init(void)
 {
-	int addr_len;
+	unsigned int addr_len;
+	struct rtnl_handle *rtnl_handle = calloc(1, sizeof(struct rtnl_handle));
 
-	rtnl_local.nl_pid = getpid();
-	rtnl_fd = socket(AF_NETLINK, SOCK_RAW, NETLINK_ROUTE);
-	if (rtnl_fd < 0) {
+	if (! rtnl_handle)
+		return NULL;
+
+	rtnl_handle->rtnl_seq = 0;
+	rtnl_handle->handlers = NULL;
+
+	rtnl_handle->rtnl_local.nl_pid = getpid();
+	rtnl_handle->rtnl_fd = socket(AF_NETLINK, SOCK_RAW, NETLINK_ROUTE);
+	if (rtnl_handle->rtnl_fd < 0) {
 		rtnl_log(LOG_ERROR, "unable to create rtnetlink socket");
-		return -1;
+		free(rtnl_handle);
+		return NULL;
 	}
 
-	memset(&rtnl_local, 0, sizeof(rtnl_local));
-	rtnl_local.nl_family = AF_NETLINK;
-	rtnl_local.nl_groups = RTMGRP_IPV4_ROUTE|RTMGRP_IPV4_IFADDR|RTMGRP_LINK;
-
-	if (bind(rtnl_fd, (struct sockaddr *)&rtnl_local, sizeof(rtnl_local)) < 0) {
+	memset(&(rtnl_handle->rtnl_local), 0, sizeof(rtnl_handle->rtnl_local));
+	rtnl_handle->rtnl_local.nl_family = AF_NETLINK;
+	rtnl_handle->rtnl_local.nl_groups = RTMGRP_IPV4_ROUTE|RTMGRP_IPV4_IFADDR|RTMGRP_LINK;
+	if (bind(rtnl_handle->rtnl_fd, (struct sockaddr *)&(rtnl_handle->rtnl_local), sizeof(rtnl_handle->rtnl_local)) < 0) {
 		rtnl_log(LOG_ERROR, "unable to bind rtnetlink socket");
-		return -1;
+		free(rtnl_handle);
+		return NULL;
 	}
 
-	addr_len = sizeof(rtnl_local);
-	if (getsockname(rtnl_fd, (struct sockaddr *)&rtnl_local, 
+	addr_len = sizeof(rtnl_handle->rtnl_local);
+	if (getsockname(rtnl_handle->rtnl_fd, (struct sockaddr *)&(rtnl_handle->rtnl_local), 
 			&addr_len) < 0) {
 		rtnl_log(LOG_ERROR, "cannot gescockname(rtnl_socket)");
-		return -1;
+		free(rtnl_handle);
+		return NULL;
 	}
 
-	if (addr_len != sizeof(rtnl_local)) {
+	if (addr_len != sizeof(rtnl_handle->rtnl_local)) {
 		rtnl_log(LOG_ERROR, "invalid address size %u", addr_len);
-		return -1;
+		free(rtnl_handle);
+		return NULL;
 	}
 
-	if (rtnl_local.nl_family != AF_NETLINK) {
-		rtnl_log(LOG_ERROR, "invalid AF %u", rtnl_local.nl_family);
-		return -1;
+	if (rtnl_handle->rtnl_local.nl_family != AF_NETLINK) {
+		rtnl_log(LOG_ERROR, "invalid AF %u", rtnl_handle->rtnl_local.nl_family);
+		free(rtnl_handle);
+		return NULL;
 	}
 
-	rtnl_seq = time(NULL);
+	rtnl_handle->rtnl_seq = time(NULL);
 
-	return rtnl_fd;
+	return rtnl_handle;
 }
 
 /* rtnl_fini - destructor of rtnetlink module */
-void rtnl_fini(void)
+void rtnl_fini(struct rtnl_handle *rtnl_handle)
 {
-	close(rtnl_fd);
+	close(rtnl_handle->rtnl_fd);
+	free(rtnl_handle);
 	return;
 }
 
=== src/rtnl.h
==================================================================
--- src/rtnl.h	(revision 5686)
+++ src/rtnl.h	(local)
@@ -12,17 +12,27 @@
 	void		*arg;
 };
 
+struct rtnl_handle {
+	int rtnl_fd;
+	int rtnl_seq;
+	int rtnl_dump;
+	struct sockaddr_nl rtnl_local;
+	struct rtnl_handler *handlers;
+};
 
+
+
+
 /* api for handler plugins */
-int rtnl_handler_register(struct rtnl_handler *hdlr);
-int rtnl_handler_unregister(struct rtnl_handler *hdlr);
+int rtnl_handler_register(struct rtnl_handle *rtnl_handle,struct rtnl_handler *hdlr);
+int rtnl_handler_unregister(struct rtnl_handle *rtnl_handle, struct rtnl_handler *hdlr);
 int rtnl_parse_rtattr(struct rtattr *tb[], int max, struct rtattr *rta, int len);
-int rtnl_dump_type(unsigned int type);
+int rtnl_dump_type(struct rtnl_handle *rtnl_handle, unsigned int type);
 
 /* api for core program */
-int rtnl_init(void);
-void rtnl_fini(void);
-int rtnl_receive();
+struct rtnl_handle *rtnl_init(void);
+void rtnl_fini(struct rtnl_handle *rtnl_handle);
+int rtnl_receive(struct rtnl_handle *rtnl_handle);
   
 
 #endif

Re: [Patch 1/2] libnfnetlink, iface conversion to string

[Harald Welte]

[-- Attachment #1: Type: text/plain, Size: 798 bytes --]

On Thu, Jan 25, 2007 at 01:11:21PM +0100, Eric Leblond wrote:
> Hi Pablo,
> 
> Excellent remarks as usual...
> 
> Here's the modified patch.

There's only some really cosmetic point that I wnat to make: I believe
it's not always within 80 characters wide lines (or is that just in the
patch due to different tab expansion?).  But we can do that even after
the commit ;)

Technically, I'm happy with the changes.

-- 
- Harald Welte <laforge@netfilter.org>                 http://netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [Patch 1/2] libnfnetlink, iface conversion to string

[Pablo Neira Ayuso]

Harald Welte wrote:
> There's only some really cosmetic point that I wnat to make: I believe
> it's not always within 80 characters wide lines (or is that just in the
> patch due to different tab expansion?).  But we can do that even after
> the commit ;)

Indeed, I just splitted some line to fit into 80 chars and remove some
blank lines, then I applied the patch to SVN.

Eric, have a look at Documentation/CodingStyle inside the kernel tree,
that is basically the same policy that the netfilter project follows for
everything inside and outside kernel, everyone will appreciate it ;)

BTW, I have inserted a test file under utils/ called iftable.c. I
introduced again two minor changes in the API since the last discussion
to keep it homogeneous with libnfnetlink:

int nlif_index2name(handler, index, name): Now you have to pass a buffer
of IFNAMSIZ where the interface name will be copied, better than the
previous that returns a pointer that needs to be "duplicated".

int nlif_query(handler): Call this function to request a whole dump of
the interface table, you have to call this after nlif_open, but someone
could call it elsewhere to request a whole resync with the interface table.

int nlif_catch(handle): Call this to update the table in the event of
changes

As Harald said, there are more cleanups still pending, like setting
errno to report errors, some possible const'ification, assertions,
probably move to linux_list interface instead of the array of pointers
to interface structures... Once we've solved this stuff I'll prepare
another libraries release.

-- 
The dawn of the fourth age of Linux firewalling is coming; a time of
great struggle and heroic deeds -- J.Kadlecsik got inspired by J.Morris

[Patch 2/2] libnetfilter_queue, iface conversion to string

[Eric Leblond]

[-- Attachment #1: Type: text/plain, Size: 87 bytes --]

Hi,

Here's the updated patch for libnetfilter_queue.

BR
--
Eric Leblond <eric@inl.fr>

[-- Attachment #2: libnetfilter_queue-ifacename.diff --]
[-- Type: text/plain, Size: 2044 bytes --]

=== include/libnetfilter_queue/libnetfilter_queue.h
==================================================================
--- include/libnetfilter_queue/libnetfilter_queue.h	(revision 5515)
+++ include/libnetfilter_queue/libnetfilter_queue.h	(local)
@@ -80,6 +80,11 @@
 extern u_int32_t nfq_get_outdev(struct nfq_data *nfad);
 extern u_int32_t nfq_get_physoutdev(struct nfq_data *nfad);
 
+extern char *nfq_get_indev_name(struct nlif_handle *nlif_handle, struct nfq_data *nfad);
+extern char *nfq_get_physindev_name(struct nlif_handle *nlif_handle, struct nfq_data *nfad);
+extern char *nfq_get_outdev_name(struct nlif_handle *nlif_handle, struct nfq_data *nfad);
+extern char *nfq_get_physoutdev_name(struct nlif_handle *nlif_handle, struct nfq_data *nfad);
+
 extern struct nfqnl_msg_packet_hw *nfq_get_packet_hw(struct nfq_data *nfad);
 
 /* return -1 if problem, length otherwise */
=== src/libnetfilter_queue.c
==================================================================
--- src/libnetfilter_queue.c	(revision 5515)
+++ src/libnetfilter_queue.c	(local)
@@ -419,6 +419,31 @@
 	return ntohl(nfnl_get_data(nfad->data, NFQA_IFINDEX_PHYSOUTDEV, u_int32_t));
 }
 
+char *nfq_get_indev_name(struct nlif_handle *nlif_handle, struct nfq_data *nfad)
+{
+	u_int32_t ifindex = nfq_get_indev(nfad);
+	return nlif_index2name(nlif_handle, ifindex);
+}
+
+char *nfq_get_physindev_name(struct nlif_handle *nlif_handle, struct nfq_data *nfad)
+{
+	u_int32_t ifindex = nfq_get_physindev(nfad);
+	return nlif_index2name(nlif_handle, ifindex);
+}
+
+char *nfq_get_outdev_name(struct nlif_handle *nlif_handle, struct nfq_data *nfad)
+{
+	u_int32_t ifindex = nfq_get_outdev(nfad);
+	return nlif_index2name(nlif_handle, ifindex);
+}
+
+char *nfq_get_physoutdev_name(struct nlif_handle *nlif_handle, struct nfq_data *nfad)
+{
+	u_int32_t ifindex = nfq_get_physoutdev(nfad);
+	return nlif_index2name(nlif_handle, ifindex);
+}
+
+
 struct nfqnl_msg_packet_hw *nfq_get_packet_hw(struct nfq_data *nfad)
 {
 	return nfnl_get_pointer_to_data(nfad->data, NFQA_HWADDR,

Re: [Patch 2/2] libnetfilter_queue, iface conversion to string

[Pablo Neira Ayuso]

Eric,

Eric Leblond wrote:
> Here's the updated patch for libnetfilter_queue.

Could you resend a patch that fits the change that I introduced in
nlif_index2name that requires a buffer to be passed? Thanks.

-- 
The dawn of the fourth age of Linux firewalling is coming; a time of
great struggle and heroic deeds -- J.Kadlecsik got inspired by J.Morris

Re: [Patch 2/2] libnetfilter_queue, iface conversion to string

[Eric Leblond]

[-- Attachment #1.1: Type: text/plain, Size: 381 bytes --]

Hi,

Le vendredi 26 janvier 2007 à 03:26 +0100, Pablo Neira Ayuso a écrit :
> Eric,
> 
> Eric Leblond wrote:
> > Here's the updated patch for libnetfilter_queue.
> 
> Could you resend a patch that fits the change that I introduced in
> nlif_index2name that requires a buffer to be passed? Thanks.

Here's the related patch.

BR,
-- 
Eric Leblond <eric@inl.fr>
INL

[-- Attachment #1.2: libnetfilter_queue-ifacename.diff --]
[-- Type: text/x-patch, Size: 2307 bytes --]

Index: include/libnetfilter_queue/libnetfilter_queue.h
===================================================================
--- include/libnetfilter_queue/libnetfilter_queue.h	(révision 6746)
+++ include/libnetfilter_queue/libnetfilter_queue.h	(copie de travail)
@@ -80,6 +80,15 @@
 extern u_int32_t nfq_get_outdev(struct nfq_data *nfad);
 extern u_int32_t nfq_get_physoutdev(struct nfq_data *nfad);
 
+extern int nfq_get_indev_name(struct nlif_handle *nlif_handle,
+			      struct nfq_data *nfad, char *name);
+extern int nfq_get_physindev_name(struct nlif_handle *nlif_handle,
+			          struct nfq_data *nfad, char *name);
+extern int nfq_get_outdev_name(struct nlif_handle *nlif_handle,
+			       struct nfq_data *nfad, char *name);
+extern int nfq_get_physoutdev_name(struct nlif_handle *nlif_handle,
+				   struct nfq_data *nfad, char *name);
+
 extern struct nfqnl_msg_packet_hw *nfq_get_packet_hw(struct nfq_data *nfad);
 
 /* return -1 if problem, length otherwise */
Index: src/libnetfilter_queue.c
===================================================================
--- src/libnetfilter_queue.c	(révision 6746)
+++ src/libnetfilter_queue.c	(copie de travail)
@@ -419,6 +419,35 @@
 	return ntohl(nfnl_get_data(nfad->data, NFQA_IFINDEX_PHYSOUTDEV, u_int32_t));
 }
 
+int nfq_get_indev_name(struct nlif_handle *nlif_handle,
+			struct nfq_data *nfad, char *name)
+{
+	u_int32_t ifindex = nfq_get_indev(nfad);
+	return nlif_index2name(nlif_handle, ifindex, name);
+}
+
+int nfq_get_physindev_name(struct nlif_handle *nlif_handle,
+			   struct nfq_data *nfad, char *name)
+{
+	u_int32_t ifindex = nfq_get_physindev(nfad);
+	return nlif_index2name(nlif_handle, ifindex, name);
+}
+
+int nfq_get_outdev_name(struct nlif_handle *nlif_handle,
+			struct nfq_data *nfad, char *name)
+{
+	u_int32_t ifindex = nfq_get_outdev(nfad);
+	return nlif_index2name(nlif_handle, ifindex, name);
+}
+
+int nfq_get_physoutdev_name(struct nlif_handle *nlif_handle,
+			    struct nfq_data *nfad, char *name)
+{
+	u_int32_t ifindex = nfq_get_physoutdev(nfad);
+	return nlif_index2name(nlif_handle, ifindex, name);
+}
+
+
 struct nfqnl_msg_packet_hw *nfq_get_packet_hw(struct nfq_data *nfad)
 {
 	return nfnl_get_pointer_to_data(nfad->data, NFQA_HWADDR,

[-- Attachment #2: Ceci est une partie de message numériquement signée --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [Patch 2/2] libnetfilter_queue, iface conversion to string

[Pablo Neira Ayuso]

Eric Leblond wrote:
> Here's the related patch.

Applied. Merci Eric.

-- 
The dawn of the fourth age of Linux firewalling is coming; a time of
great struggle and heroic deeds -- J.Kadlecsik got inspired by J.Morris

[Patch 2/2] libnetfilter_queue and iface conversion to string

[Eric Leblond]

[-- Attachment #1.1: Type: text/plain, Size: 98 bytes --]

Hi,

Here's the patch against libnetfilter_queue.

BR,
-- 
Eric Leblond <eric@inl.fr>
INL

[-- Attachment #1.2: libnetfilter_queue_iface_name.diff --]
[-- Type: text/x-patch, Size: 1817 bytes --]

Index: include/libnetfilter_queue/libnetfilter_queue.h
===================================================================
--- include/libnetfilter_queue/libnetfilter_queue.h	(révision 6737)
+++ include/libnetfilter_queue/libnetfilter_queue.h	(copie de travail)
@@ -80,6 +80,11 @@
 extern u_int32_t nfq_get_outdev(struct nfq_data *nfad);
 extern u_int32_t nfq_get_physoutdev(struct nfq_data *nfad);
 
+extern char* nfq_get_indev_name(struct nfq_data *nfad);
+extern char* nfq_get_physindev_name(struct nfq_data *nfad);
+extern char* nfq_get_outdev_name(struct nfq_data *nfad);
+extern char* nfq_get_physoutdev_name(struct nfq_data *nfad);
+
 extern struct nfqnl_msg_packet_hw *nfq_get_packet_hw(struct nfq_data *nfad);
 
 /* return -1 if problem, length otherwise */
Index: src/libnetfilter_queue.c
===================================================================
--- src/libnetfilter_queue.c	(révision 6737)
+++ src/libnetfilter_queue.c	(copie de travail)
@@ -419,6 +419,31 @@
 	return ntohl(nfnl_get_data(nfad->data, NFQA_IFINDEX_PHYSOUTDEV, u_int32_t));
 }
 
+char* nfq_get_indev_name(struct nfq_data *nfad)
+{
+	u_int32_t ifindex = nfq_get_indev(nfad);
+	return nlif_index_2name(ifindex);
+}
+
+char* nfq_get_physindev_name(struct nfq_data *nfad)
+{
+	u_int32_t ifindex = nfq_get_physindev(nfad);
+	return nlif_index_2name(ifindex);
+}
+
+char* nfq_get_outdev_name(struct nfq_data *nfad)
+{
+	u_int32_t ifindex = nfq_get_outdev(nfad);
+	return nlif_index_2name(ifindex);
+}
+
+char* nfq_get_physoutdev_name(struct nfq_data *nfad)
+{
+	u_int32_t ifindex = nfq_get_physoutdev(nfad);
+	return nlif_index_2name(ifindex);
+}
+
+
 struct nfqnl_msg_packet_hw *nfq_get_packet_hw(struct nfq_data *nfad)
 {
 	return nfnl_get_pointer_to_data(nfad->data, NFQA_HWADDR,

[-- Attachment #2: Ceci est une partie de message numériquement signée --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [Patch 2/2] libnetfilter_queue and iface conversion to string

[Patrick McHardy]

Eric Leblond wrote:
> Index: src/libnetfilter_queue.c
> ===================================================================
> --- src/libnetfilter_queue.c	(révision 6737)
> +++ src/libnetfilter_queue.c	(copie de travail)
> @@ -419,6 +419,31 @@
>  	return ntohl(nfnl_get_data(nfad->data, NFQA_IFINDEX_PHYSOUTDEV, u_int32_t));
>  }
>  
> +char* nfq_get_indev_name(struct nfq_data *nfad)
> +{
> +	u_int32_t ifindex = nfq_get_indev(nfad);
> +	return nlif_index_2name(ifindex);
> +}
> +

This might appear annoying, but I don't like inconsistency since
it distracts me. So char * please, as in the parameters or in
the other functions in this file.

Resend: [Patch 2/2] libnetfilter_queue and iface conversion to string

[Eric Leblond]

[-- Attachment #1.1: Type: text/plain, Size: 495 bytes --]

Hi,

Le vendredi 19 janvier 2007 à 16:25 +0100, Patrick McHardy a écrit :
> Eric Leblond wrote:
> > Index: src/libnetfilter_queue.c
> > +char* nfq_get_indev_name(struct nfq_data *nfad)
> > +{
> This might appear annoying, but I don't like inconsistency since
> it distracts me. So char * please, as in the parameters or in
> the other functions in this file.

Ok, not a problem. I'm sorry that I've missed this coding style point
too :(

BR,
-- 
Eric Leblond <eric@inl.fr>
INL

[-- Attachment #1.2: libnetfilter_queue-ifacename.diff --]
[-- Type: text/x-patch, Size: 1817 bytes --]

Index: include/libnetfilter_queue/libnetfilter_queue.h
===================================================================
--- include/libnetfilter_queue/libnetfilter_queue.h	(révision 6737)
+++ include/libnetfilter_queue/libnetfilter_queue.h	(copie de travail)
@@ -80,6 +80,11 @@
 extern u_int32_t nfq_get_outdev(struct nfq_data *nfad);
 extern u_int32_t nfq_get_physoutdev(struct nfq_data *nfad);
 
+extern char *nfq_get_indev_name(struct nfq_data *nfad);
+extern char *nfq_get_physindev_name(struct nfq_data *nfad);
+extern char *nfq_get_outdev_name(struct nfq_data *nfad);
+extern char *nfq_get_physoutdev_name(struct nfq_data *nfad);
+
 extern struct nfqnl_msg_packet_hw *nfq_get_packet_hw(struct nfq_data *nfad);
 
 /* return -1 if problem, length otherwise */
Index: src/libnetfilter_queue.c
===================================================================
--- src/libnetfilter_queue.c	(révision 6737)
+++ src/libnetfilter_queue.c	(copie de travail)
@@ -419,6 +419,31 @@
 	return ntohl(nfnl_get_data(nfad->data, NFQA_IFINDEX_PHYSOUTDEV, u_int32_t));
 }
 
+char *nfq_get_indev_name(struct nfq_data *nfad)
+{
+	u_int32_t ifindex = nfq_get_indev(nfad);
+	return nlif_index_2name(ifindex);
+}
+
+char *nfq_get_physindev_name(struct nfq_data *nfad)
+{
+	u_int32_t ifindex = nfq_get_physindev(nfad);
+	return nlif_index_2name(ifindex);
+}
+
+char *nfq_get_outdev_name(struct nfq_data *nfad)
+{
+	u_int32_t ifindex = nfq_get_outdev(nfad);
+	return nlif_index_2name(ifindex);
+}
+
+char *nfq_get_physoutdev_name(struct nfq_data *nfad)
+{
+	u_int32_t ifindex = nfq_get_physoutdev(nfad);
+	return nlif_index_2name(ifindex);
+}
+
+
 struct nfqnl_msg_packet_hw *nfq_get_packet_hw(struct nfq_data *nfad)
 {
 	return nfnl_get_pointer_to_data(nfad->data, NFQA_HWADDR,

[-- Attachment #2: Ceci est une partie de message numériquement signée --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Resend: [Patch 2/2] libnetfilter_queue and iface conversion to string

[Eric Leblond]

[-- Attachment #1: Type: text/plain, Size: 86 bytes --]

Hi,

Here's the patch which adapts libnetfilter_queue to last sent patch.

BR,
--
Eric

[-- Attachment #2: libnetfilter_queue-ifacename.diff --]
[-- Type: text/plain, Size: 2008 bytes --]

=== include/libnetfilter_queue/libnetfilter_queue.h
==================================================================
--- include/libnetfilter_queue/libnetfilter_queue.h	(revision 5515)
+++ include/libnetfilter_queue/libnetfilter_queue.h	(local)
@@ -80,6 +80,11 @@
 extern u_int32_t nfq_get_outdev(struct nfq_data *nfad);
 extern u_int32_t nfq_get_physoutdev(struct nfq_data *nfad);
 
+extern char *nfq_get_indev_name(struct nlif_inst *nlif_inst, struct nfq_data *nfad);
+extern char *nfq_get_physindev_name(struct nlif_inst *nlif_inst, struct nfq_data *nfad);
+extern char *nfq_get_outdev_name(struct nlif_inst *nlif_inst, struct nfq_data *nfad);
+extern char *nfq_get_physoutdev_name(struct nlif_inst *nlif_inst, struct nfq_data *nfad);
+
 extern struct nfqnl_msg_packet_hw *nfq_get_packet_hw(struct nfq_data *nfad);
 
 /* return -1 if problem, length otherwise */
=== src/libnetfilter_queue.c
==================================================================
--- src/libnetfilter_queue.c	(revision 5515)
+++ src/libnetfilter_queue.c	(local)
@@ -419,6 +419,31 @@
 	return ntohl(nfnl_get_data(nfad->data, NFQA_IFINDEX_PHYSOUTDEV, u_int32_t));
 }
 
+char *nfq_get_indev_name(struct nlif_inst *nlif_inst, struct nfq_data *nfad)
+{
+	u_int32_t ifindex = nfq_get_indev(nfad);
+	return nlif_index_2name(nlif_inst, ifindex);
+}
+
+char *nfq_get_physindev_name(struct nlif_inst *nlif_inst, struct nfq_data *nfad)
+{
+	u_int32_t ifindex = nfq_get_physindev(nfad);
+	return nlif_index_2name(nlif_inst, ifindex);
+}
+
+char *nfq_get_outdev_name(struct nlif_inst *nlif_inst, struct nfq_data *nfad)
+{
+	u_int32_t ifindex = nfq_get_outdev(nfad);
+	return nlif_index_2name(nlif_inst, ifindex);
+}
+
+char *nfq_get_physoutdev_name(struct nlif_inst *nlif_inst, struct nfq_data *nfad)
+{
+	u_int32_t ifindex = nfq_get_physoutdev(nfad);
+	return nlif_index_2name(nlif_inst, ifindex);
+}
+
+
 struct nfqnl_msg_packet_hw *nfq_get_packet_hw(struct nfq_data *nfad)
 {
 	return nfnl_get_pointer_to_data(nfad->data, NFQA_HWADDR,

Re: [RFC] libnfnetlink and iface conversion to string

[Patrick McHardy]

Pablo Neira Ayuso wrote:
> Eric Leblond wrote:
> 
>>I'm currently working on NuFW where I try to do iface conversion from
>>integer to string to be able to filter "naturally" on interfaces.
>>
>>For now, it is impossible to do this conversion from either
>>libnetfilter_queue or libnfnetlink. But there is two files in
>>libnfnetlink (iftable.c and rntl.c) which have been put here by Harald
>>to provide this conversion (if I remember correctly an old mail from
>>Harald).
>>
>>It was a long time ago and I don't know if it still planned to modify
>>libnfnetlink to include this feature. Or, would it be a better choice to
>>create a new library dedicated to this conversion.

There already is libnl .. it may be a bit overkill if all you
need is ifindex<->ifname conversion though.

> Interesting question. Since this is related with the rtnetlink
> subsystem, I'm unsure that this feature naturally belongs to
> libnfnetlink nor to libnetfilter_queue.
> 
> However, IMO libnetfilter_queue should provide enough features to let
> userspace apps implement filtering without requiring extra dependencies.

Agreed, this would be a good addition in my opinion. Probably
better to keep in it libnfnetlink since that doesn't require
people to unnecessarily link libnfnetlink_queue just for this.