Skip to contentSkip to conversation

libnetfilter_queue nfattr[] abstraction

PATCH In review Eric Leblond 2 participants 3 messages Nov 4, 2005 – Nov 5, 2005 netfilter

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 libnetfilter_queue nfattr[] abstraction

Hi,

This patch is a proposal to suppress the use of struct nfattr* nfa[] like params in user level function.

This just hide the low-level structure behind a dummy structure. But from a user point of view, this structure should remain inchanged even if the kernel internal are changing.

I've checked the provided test util and NuFW against the modified library.

BR,

Signature · 1 line
Eric Leblond <[email protected]>

==== Patch <netfilter_queue_datas> level 1
Source: [No source]
Target: f6f8485a-58f1-0310-9e83-9cad51fe5e11:/netfilter/libnetfilter_queue:1217 Log:
introduce nfqnl_q_datas struct to hide struct nfattr * nfa[]
=== include/libnetfilter_queue/libnetfilter_queue.h
==================================================================

libnetfilter_queue/libnetfilter_queue.h +14 -10
@@ -18,6 +18,10 @@
 #include <linux/netfilter/nfnetlink.h>
 #include <linux/netfilter/nfnetlink_queue.h>
 
+struct nfqnl_q_datas {
+	struct nfattr ** data;
+};
+
 struct nfqnl_handle;
 struct nfqnl_q_handle;
 
@@ -27,7 +31,7 @@
 extern int nfqnl_fd(struct nfqnl_handle *h);
 
 typedef int  nfqnl_callback(struct nfqnl_q_handle *gh, struct nfgenmsg *nfmsg,
-		       struct nfattr *nfa[], void *data);
+		       struct nfqnl_q_datas* nfa, void *data);
 
 
 extern struct nfqnl_handle *nfqnl_open(void);
@@ -63,23 +67,23 @@
 /* message parsing function */
 
 extern struct nfqnl_msg_packet_hdr *
-				nfqnl_get_msg_packet_hdr(struct nfattr *nfa[]);
+				nfqnl_get_msg_packet_hdr(struct nfqnl_q_datas* nfa);
 
-extern u_int32_t nfqnl_get_nfmark(struct nfattr *nfa[]);
+extern u_int32_t nfqnl_get_nfmark(struct nfqnl_q_datas* nfa);
 
 extern struct nfqnl_msg_packet_timestamp *
-				nfqnl_get_timestamp(struct nfattr *nfa[]);
+				nfqnl_get_timestamp(struct nfqnl_q_datas* nfa);
 
 /* return 0 if not set */
-extern u_int32_t nfqnl_get_indev(struct nfattr *nfa[]);
-extern u_int32_t nfqnl_get_physindev(struct nfattr *nfa[]);
-extern u_int32_t nfqnl_get_outdev(struct nfattr *nfa[]);
-extern u_int32_t nfqnl_get_physoutdev(struct nfattr *nfa[]);
+extern u_int32_t nfqnl_get_indev(struct nfqnl_q_datas* nfa);
+extern u_int32_t nfqnl_get_physindev(struct nfqnl_q_datas* nfa);
+extern u_int32_t nfqnl_get_outdev(struct nfqnl_q_datas* nfa);
+extern u_int32_t nfqnl_get_physoutdev(struct nfqnl_q_datas* nfa);
 
-extern struct nfqnl_msg_packet_hw *nfqnl_get_packet_hw(struct nfattr *nfa[]);
+extern struct nfqnl_msg_packet_hw *nfqnl_get_packet_hw(struct nfqnl_q_datas* nfa);
 
 /* return 0 if problem */
-extern int nfqnl_get_payload(struct nfattr *nfa[],
+extern int nfqnl_get_payload(struct nfqnl_q_datas* nfa,
 			     char ** data, unsigned int* datalen);
 
 

=== src/libnetfilter_queue.c
==================================================================

libnetfilter_queue.c +23 -20

93 lines not on this page · raw message

==== BEGIN SVK PATCH BLOCK ====
Version: svk 1.04 (linux)

eJy9lt9rG0cQx++tlZ4L7UvhApdWUqX49n5LJkIG2zQklZ1EdhuKOVa3c9aR0510t+fEzQXkxpS8 tdDQPhf6TxQKoSX07+ru6odlVZYdJ1QP4nb3O5+Zm92Z2+2ks95CebOp5gpS84f7dxuNXUy93k02 UqwcSEDjRDHzEI4gVPQ8jA8VI49wH9gqxckh0GYTMWNnbLwlDKYQgzO6mMZRqtQFzKUJgIJyq2Xn LS1v6cw0R2wxHkDkJnFMGR5pyGaLLg/JC+MUXG7KtC2TyzUFGWM9CRLwmL9jxc6DyAszAgI3QQi1 uUSt52nizSt17sycODvTMQUPUnD0JRzNmrpdC4NuBNQPQgqJO8wgAxHpZXxtxjemSWAMUAx7BXnJ 1K2eCPEyd/rYnVjFg0F47FJ4SgmEFIsQdC1HCLoIEUBdB1TPxnWfP3WJgQEjzSeKpdtsr9uS9OqD V+Gnpx3p9HPpd+n7j08/+Ul66fzQGO1Iuz/aoxf3pQc/m0OXYIpT+VmxkNIk86gc+ZjSRK5UZL6y Xny+PoxCd6Kr8OXFcXm9WGRRQhLJmRtEVNdcjuEydvzcyO/j5HFphr8Kbq0iJ0AzRlTlwJejmMop ULmytspREBE4utgP464wHvSO03cCxBl9R/dXIEwR5zT99NAdYO8xw/SeyJU56HRyNXMh2YMk7obQ n0s2C3c+VHwcxphczGwZ861BlMvk7AL4huFbvo59XDeAEKJiy/K9um7b0LXqs2KzzhWbZvB+sKyq PNFALqsqc2VVWaKqbNNWrbrq65pheJh4xLdMy1M9HzDGOmsuyDHGZfXm4J8Pv3gZSL8MpdFJUfot //Xea+ngj+9GJ19LJ+GfH41e7Enw143XUudvZ3Riqm9qhWWp4iNWXsUCS3npxrBXLhYKk22obbV3 Nrf25xZrTa+7XMAptzjwNss8n5hIxjalYa/KHLETUpU/Y9LFrSLl4rOZSeRPdzhmO85yTGMhLXFh rckeV5vTuBeWZpDzlrgqt7fvb7hfbTy4Wz2rgDI7f8+LxYuOMw36kFLcH8yf6tnkxSfwf3+tO9t3 2ptb37j8f/+/73fNhnP9OHa/fPTwLWK5rPNcP5Cdvc5bZeT9RfK+z0RV9nqYfxfFW7JalMUvi9Lg MAIimqRYCyESrsRAvn0FX9Ok7W48urezsTn2VGGJEuUvOGX+hS5M+Yza3p7pz0BLO6/obzoBB7yu 4eCuzn6eBZ7tIwDT8Ekd++NrmaWySw1NYpJ5sNCsJhtCY7kXEJAX7gr84dsDRUP5QoMeWzebWq5o 2vgG2hGX0kZjLwqOIElxeNPKWQcfYNpj974EjtggywKiaPV8bYZbdn8bt31Ft3L2SXEMx8Q10/FR TdWRWquDo9fqrJObyAcTEPoXMkHhlg==
==== END SVK PATCH BLOCK ====

#1 lore raw
Quoted 4 lines · Eric Leblond

Hi,

This patch is a proposal to suppress the use of struct nfattr* nfa[] like params in user level function.

looks fine to me. I'll rename nfnl_q_datas into nfnl_q_data, though.

Quoted 3 lines

This just hide the low-level structure behind a dummy structure. But from a user point of view, this structure should remain inchanged even if the kernel internal are changing.

ACK. this can easily be enforced if we put the structure definition into the C file, and just have a dummy 'struct nfnl_q_data' declaration in the header.

Quoted 2 lines

I've checked the provided test util and NuFW against the modified library.

Please re-check after my changes went in (maybe in 30mins).

Signature · 5 lines
- Harald Welte <[email protected]>                 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
#2 lore raw
Quoted 6 lines · Harald Welte
Quoted 3 lines · Eric Leblond

Hi,
I've checked the provided test util and NuFW against the modified library.

Please re-check after my changes went in (maybe in 30mins).

Done. I've found a problem with timestamp related function. timestamp structure can not be renamed as it is defined in kernel.
I attach a patch that correct this.

For timestamp, we could use the glibc "struct timeval" but the user will have to free the structure after use as it is not a pointer to the message from kernel.

BR,

Signature · 1 line
Eric Leblond <[email protected]>

=== include/libnetfilter_queue/libnetfilter_queue.h
==================================================================

libnetfilter_queue/libnetfilter_queue.h +1 -1
@@ -68,7 +68,7 @@
 
 extern u_int32_t nfq_get_nfmark(struct nfq_data *nfad);
 
-extern struct nfq_msg_packet_timestamp *
+extern struct nfqnl_msg_packet_timestamp *
 				nfq_get_timestamp(struct nfq_data *nfad);
 
 /* return 0 if not set */

=== src/libnetfilter_queue.c
==================================================================

libnetfilter_queue.c +1 -1
@@ -336,7 +335,7 @@
 	return ntohl(nfnl_get_data(nfad->data, NFQA_MARK, u_int32_t));
 }
 
-struct nfq_msg_packet_timestamp *nfq_get_timestamp(struct nfq_data *nfad)
+struct nfqnl_msg_packet_timestamp *nfq_get_timestamp(struct nfq_data *nfad)
 {
 	return nfnl_get_pointer_to_data(nfad->data, NFQA_TIMESTAMP,
 					struct nfq_msg_packet_timestamp);