libnetfilter_queue: mark-value byte ordering?

libnetfilter_queue: mark-value byte ordering?

[David F]

Hi,

I am using libnetfilter_queue to set the mark on some queued packets 
[nfq_set_verdict_mark(), with verdict==NF_ACCEPT]; then in a later 
iptables chain, I matched on -m mark, looking for my previously marked 
packets, but apparently they didn't match.  So I logged packets and saw 
my packets with my mark values in the log entries, but they seemed to be 
in reverse byte-order (I'm on a little-endian machine).  I changed my 
code to use htonl() on the mark-value prior to calling 
nfq_set_verdict_mark(), and it all suddenly started working.

I had a quick look through the source code of libnetfilter_queue and 
libnfnetlink_queue and didn't see any obvious byte-order conversion 
prior to sending to the kernel, so I wonder if anyone could help me 
understand,
* Is the mark value _supposed_ to be supplied in network byte order or 
is something else going on here;
and if so,
* Since the mark never hits the wire, why would it ever be kept in 
network byte order?

Thanks in advance,
-- David F.

Re: libnetfilter_queue: mark-value byte ordering?

[Alessandro Vesely]

David F wrote:
>  I changed my 
> code to use htonl() on the mark-value prior to calling 
> nfq_set_verdict_mark(), and it all suddenly started working.

Since it is not documented, everyone rediscovers it anew. See e.g. 
http://www.gossamer-threads.com/lists/iptables/devel/62591


> I had a quick look through the source code of libnetfilter_queue and 
> libnfnetlink_queue and didn't see any obvious byte-order conversion 
> prior to sending to the kernel, so I wonder if anyone could help me 
> understand,
> * Is the mark value _supposed_ to be supplied in network byte order or 
> is something else going on here;
> and if so,
> * Since the mark never hits the wire, why would it ever be kept in 
> network byte order?

The latter is an issue in kernel programming, where all code in a 
given set assumes an established byte order. In facts, it is not 
uncommon to have the same word byteswapped multiple times on the 
same machine for the sake of obeying conventions.

HTH

Re: libnetfilter_queue: mark-value byte ordering?

[Pablo Neira Ayuso]

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

Alessandro Vesely wrote:
> David F wrote:
>>  I changed my code to use htonl() on the mark-value prior to calling
>> nfq_set_verdict_mark(), and it all suddenly started working.
> 
> Since it is not documented, everyone rediscovers it anew. See e.g.
> http://www.gossamer-threads.com/lists/iptables/devel/62591

I have applied the following patch. I think that, at least, new users
will not hit this problem again. I'm very sorry that this was not fixed
before. Let me know if you are OK with it, we're still in time to revert
the patch attached.

[-- Attachment #2: fix.patch --]
[-- Type: text/x-patch, Size: 3292 bytes --]

nfq: deprecate nfq_set_verdict_mark() in favour of nfq_set_verdict2()

From: Pablo Neira Ayuso <pablo@netfilter.org>

This patch deprecates nfq_set_verdict_mark() in favour of
nfq_set_verdict2() which does exactly the same but it also
convert the mark value from host-byte order to network-byte
order as expected by nfnetlink_queue.

I know, this is hackish, but I prefer adding new functions
instead of API versioning which is also ugly.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/libnetfilter_queue/libnetfilter_queue.h |   20 ++++++++++++++------
 src/libnetfilter_queue.c                        |   19 +++++++++++++++++++
 2 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/include/libnetfilter_queue/libnetfilter_queue.h b/include/libnetfilter_queue/libnetfilter_queue.h
index 1a72c51..88a9b8c 100644
--- a/include/libnetfilter_queue/libnetfilter_queue.h
+++ b/include/libnetfilter_queue/libnetfilter_queue.h
@@ -62,12 +62,20 @@ extern int nfq_set_verdict(struct nfq_q_handle *qh,
 			     u_int32_t data_len,
 			     unsigned char *buf);
 
-extern int nfq_set_verdict_mark(struct nfq_q_handle *qh, 
-				  u_int32_t id,
-			   	  u_int32_t verdict, 
-				  u_int32_t mark,
-			   	  u_int32_t datalen,
-				  unsigned char *buf);
+extern int nfq_set_verdict2(struct nfq_q_handle *qh,
+			    u_int32_t id,
+			    u_int32_t verdict, 
+			    u_int32_t mark,
+			    u_int32_t datalen,
+			    unsigned char *buf);
+
+extern __attribute__((deprecated))
+int nfq_set_verdict_mark(struct nfq_q_handle *qh, 
+			 u_int32_t id,
+			 u_int32_t verdict, 
+			 u_int32_t mark,
+			 u_int32_t datalen,
+			 unsigned char *buf);
 
 /* message parsing function */
 
diff --git a/src/libnetfilter_queue.c b/src/libnetfilter_queue.c
index df19519..7e62317 100644
--- a/src/libnetfilter_queue.c
+++ b/src/libnetfilter_queue.c
@@ -679,6 +679,22 @@ int nfq_set_verdict(struct nfq_q_handle *qh, u_int32_t id,
 }	
 
 /**
+ * nfq_set_verdict2 - like nfq_set_verdict, but you can set the mark.
+ * \param qh Netfilter queue handle obtained by call to nfq_create_queue().
+ * \param id	ID assigned to packet by netfilter.
+ * \param verdict verdict to return to netfilter (NF_ACCEPT, NF_DROP)
+ * \param mark mark to put on packet
+ * \param data_len number of bytes of data pointed to by #buf
+ * \param buf the buffer that contains the packet data
+ */
+int nfq_set_verdict2(struct nfq_q_handle *qh, u_int32_t id,
+		     u_int32_t verdict, u_int32_t mark,
+		     u_int32_t data_len, unsigned char *buf)
+{
+	return __set_verdict(qh, id, verdict, htonl(mark), 1, data_len, buf);
+}
+
+/**
  * nfq_set_verdict_mark - like nfq_set_verdict, but you can set the mark.
  * \param qh Netfilter queue handle obtained by call to nfq_create_queue().
  * \param id	ID assigned to packet by netfilter.
@@ -686,6 +702,9 @@ int nfq_set_verdict(struct nfq_q_handle *qh, u_int32_t id,
  * \param mark mark to put on packet
  * \param data_len number of bytes of data pointed to by #buf
  * \param buf the buffer that contains the packet data
+ *
+ * This function is deprecated since it is broken, its use is highly
+ * discouraged. Please, use nfq_set_verdict2 instead.
  */
 int nfq_set_verdict_mark(struct nfq_q_handle *qh, u_int32_t id,
 		u_int32_t verdict, u_int32_t mark,

Re: libnetfilter_queue: mark-value byte ordering?

[David F]

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

Pablo Neira Ayuso wrote:
> Alessandro Vesely wrote:
>   
>> David F wrote:
>>     
>>>  I changed my code to use htonl() on the mark-value prior to calling
>>> nfq_set_verdict_mark(), and it all suddenly started working.
>>>       
>> Since it is not documented, everyone rediscovers it anew. See e.g.
>> http://www.gossamer-threads.com/lists/iptables/devel/62591
>>     
>
> I have applied the following patch. I think that, at least, new users
> will not hit this problem again. I'm very sorry that this was not fixed
> before. Let me know if you are OK with it, we're still in time to revert
> the patch attached.
>   
For what it's worth, I had previously prepared this patch which just 
clarifies the documentation on this parameter.  I think it still has 
value since I also added some missing return-value docs and changed the 
descriptions of a few parameters that I had found to be confusing.

-- David Favro


[-- Attachment #2: 0001-Documentation-enhancements.patch --]
[-- Type: text/x-patch, Size: 5084 bytes --]

From 29f601afdd546b75f6b5d64a654b1c60780899f9 Mon Sep 17 00:00:00 2001
From: David Favro <netfilter@meta-dynamic.com>
Date: Mon, 3 May 2010 21:28:55 -0400
Subject: [PATCH] Documentation enhancements.

 * Several parameters are clarified.
 * Several previously undocumented return-values are documented.
 * nfq_set_verdict_mark() [now deprecated]: notes that mark is in network
   byte order.
---
 src/libnetfilter_queue.c |   31 +++++++++++++++++--------------
 1 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/src/libnetfilter_queue.c b/src/libnetfilter_queue.c
index 7e62317..7d0fb45 100644
--- a/src/libnetfilter_queue.c
+++ b/src/libnetfilter_queue.c
@@ -541,6 +541,8 @@ int nfq_handle_packet(struct nfq_handle *h, char *buf, int len)
  * - NFQNL_COPY_NONE - do not copy any data
  * - NFQNL_COPY_META - copy only packet metadata
  * - NFQNL_COPY_PACKET - copy entire packet
+ *
+ * \return -1 on error; >=0 otherwise.
  */
 int nfq_set_mode(struct nfq_q_handle *qh,
 		u_int8_t mode, u_int32_t range)
@@ -571,6 +573,8 @@ int nfq_set_mode(struct nfq_q_handle *qh,
  * Sets the size of the queue in kernel. This fixes the maximum number
  * of packets the kernel will store before internally before dropping
  * upcoming packets.
+ *
+ * \return -1 on error; >=0 otherwise.
  */
 int nfq_set_queue_maxlen(struct nfq_q_handle *qh,
 				u_int32_t queuelen)
@@ -670,6 +674,8 @@ static int __set_verdict(struct nfq_q_handle *qh, u_int32_t id,
  * Notifies netfilter of the userspace verdict for the given packet.  Every
  * queued packet _must_ have a verdict specified by userspace, either by
  * calling this function, or by calling the nfq_set_verdict_mark() function.
+ *
+ * \return -1 on error; >= 0 otherwise.
  */
 int nfq_set_verdict(struct nfq_q_handle *qh, u_int32_t id,
 		u_int32_t verdict, u_int32_t data_len, 
@@ -699,10 +705,12 @@ int nfq_set_verdict2(struct nfq_q_handle *qh, u_int32_t id,
  * \param qh Netfilter queue handle obtained by call to nfq_create_queue().
  * \param id	ID assigned to packet by netfilter.
  * \param verdict verdict to return to netfilter (NF_ACCEPT, NF_DROP)
- * \param mark mark to put on packet
+ * \param mark the mark to put on the packet, in network byte order.
  * \param data_len number of bytes of data pointed to by #buf
  * \param buf the buffer that contains the packet data
  *
+ * \return -1 on error; >= 0 otherwise.
+ *
  * This function is deprecated since it is broken, its use is highly
  * discouraged. Please, use nfq_set_verdict2 instead.
  */
@@ -848,11 +856,10 @@ u_int32_t nfq_get_physoutdev(struct nfq_data *nfad)
  * was received through
  * \param nlif_handle pointer to a nlif interface resolving handle
  * \param nfad Netlink packet data handle passed to callback function
- * \param name pointer that will be set to the interface name string 
+ * \param name pointer to the buffer to receive the interface name;
+ *  not more than \c IFNAMSIZ bytes will be copied to it.
  * \return -1 in case of error, >0 if it succeed. 
  *
- * The #name variable will point to the name of the input interface.
- *
  * To use a nlif_handle, You need first to call nlif_open() and to open
  * an handler. Don't forget to store the result as it will be used 
  * during all your program life:
@@ -894,10 +901,8 @@ int nfq_get_indev_name(struct nlif_handle *nlif_handle,
  * packet was received through
  * \param nlif_handle pointer to a nlif interface resolving handle
  * \param nfad Netlink packet data handle passed to callback function
- * \param name pointer that will be set to the interface name string 
- *
- * The #name variable will point to the name of the input physical
- * interface.
+ * \param name pointer to the buffer to receive the interface name;
+ *  not more than \c IFNAMSIZ bytes will be copied to it.
  *
  * See nfq_get_indev_name() documentation for nlif_handle usage.
  *
@@ -915,9 +920,8 @@ int nfq_get_physindev_name(struct nlif_handle *nlif_handle,
  * packet will be sent to
  * \param nlif_handle pointer to a nlif interface resolving handle
  * \param nfad Netlink packet data handle passed to callback function
- * \param name pointer that will be set to the interface name string 
- *
- * The #name variable will point to the name of the output interface.
+ * \param name pointer to the buffer to receive the interface name;
+ *  not more than \c IFNAMSIZ bytes will be copied to it.
  *
  * See nfq_get_indev_name() documentation for nlif_handle usage.
  *
@@ -935,9 +939,8 @@ int nfq_get_outdev_name(struct nlif_handle *nlif_handle,
  * packet will be sent to
  * \param nlif_handle pointer to a nlif interface resolving handle
  * \param nfad Netlink packet data handle passed to callback function
- * \param name pointer that will be set to the interface name string 
- * The #name variable will point to the name of the physical
- * output interface.
+ * \param name pointer to the buffer to receive the interface name;
+ *  not more than \c IFNAMSIZ bytes will be copied to it.
  *
  * See nfq_get_indev_name() documentation for nlif_handle usage.
  *
-- 
1.6.3.3

Re: libnetfilter_queue: mark-value byte ordering?

[Alessandro Vesely]

David F writes:
> Pablo Neira Ayuso wrote:
>>
>> I have applied the following patch. I think that, at least, new users
>> will not hit this problem again. I'm very sorry that this was not fixed
>> before. Let me know if you are OK with it, we're still in time to revert
>> the patch attached.

Waiting one version before deprecating might allow smoother changing.

> For what it's worth, I had previously prepared this patch which just 
> clarifies the documentation on this parameter.  I think it still has 
> value since I also added some missing return-value docs and changed the 
> descriptions of a few parameters that I had found to be confusing.

Good work. Is the (current) generated doc available? I've found an older 
version in http://www.nufw.org/doc/libnetfilter_queue/

I attach a patch aimed at fixing the example, which is confusing, since rv 
can simultaneously be != 0 and >= 0 only if it is > 0. I haven't resisted 
an attempt at enumerating verdicts, though.

Re: libnetfilter_queue: mark-value byte ordering?

[Pablo Neira Ayuso]

Alessandro Vesely wrote:
> David F writes:
>> Pablo Neira Ayuso wrote:
>>>
>>> I have applied the following patch. I think that, at least, new users
>>> will not hit this problem again. I'm very sorry that this was not fixed
>>> before. Let me know if you are OK with it, we're still in time to revert
>>> the patch attached.
> 
> Waiting one version before deprecating might allow smoother changing.

The deprecated version will wait there, I don't plan to remove it, I 
don't want to break any existing application. As for now, the 
deprecation will spot a warning during compilation and, if the developer 
reads the documentation, it will notice that is better to use the new 
version.

At least new users will take the new function instead of the deprecated 
one, I would expect.

I'll apply your patches once I arrive home, I'm planning to release 
libnetfilter_queue 1.0 after this round, this has been tagged as beta 
stage for so long. If you have more patches, let me know.

Thanks everyone!

Re: libnetfilter_queue: mark-value byte ordering? --oops, pls discard previous copy

[Alessandro Vesely]

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

David F writes:
> Pablo Neira Ayuso wrote:
>>
>> I have applied the following patch. I think that, at least, new users
>> will not hit this problem again. I'm very sorry that this was not fixed
>> before. Let me know if you are OK with it, we're still in time to revert
>> the patch attached.

Waiting one version before deprecating might allow smoother changing.

> For what it's worth, I had previously prepared this patch which just
> clarifies the documentation on this parameter.  I think it still has
> value since I also added some missing return-value docs and changed the
> descriptions of a few parameters that I had found to be confusing.

Good work. Is the (current) generated doc available? I've found an 
older version in http://www.nufw.org/doc/libnetfilter_queue/

I attach a patch aimed at fixing the example, which is confusing, 
since rv can simultaneously be != 0 and >= 0 only if it is > 0. I 
haven't resisted an attempt at enumerating verdicts, though.


[-- Attachment #2: libnetfilter_queue17.patch --]
[-- Type: text/plain, Size: 1196 bytes --]

--- libnetfilter_queue-0.0.17/src/libnetfilter_queue.original.c	2009-02-17 20:55:23.000000000 +0100
+++ libnetfilter_queue-0.0.17/src/libnetfilter_queue.c	2010-05-10 12:25:33.000000000 +0200
@@ -207,13 +207,22 @@
  * \verbatim
 	fd = nfq_fd(h);
 
-	while ((rv = recv(fd, buf, sizeof(buf), 0)) && rv >= 0) {
+	while ((rv = recv(fd, buf, sizeof(buf), 0)) >= 0) {
 		printf("pkt received\n");
 		nfq_handle_packet(h, buf, rv);
 	}
 \endverbatim
  * When the decision on a packet has been choosed, the verdict has to be given
- * by calling nfq_set_verdict() or nfq_set_verdict_mark().
+ * by calling nfq_set_verdict() or nfq_set_verdict_mark(). The verdict
+ * determines the destiny of the packet as follows:
+ *
+ *   - NF_DROP discarded the packet
+ *   - NF_ACCEPT the packet passes, continue iterations
+ *   - NF_STOLEN gone away
+ *   - NF_QUEUE inject the packet into a different queue
+ *     (the target queue number is in the high 16 bits of the verdict)
+ *   - NF_REPEAT iterate the same cycle once more
+ *   - NF_STOP accept, but don't continue iterations
  *
  * Data and information about the packet can be fetch by using message parsing
  * functions (See \link Parsing \endlink).

Re: libnetfilter_queue: mark-value byte ordering? --oops, pls discard previous copy

[Pablo Neira Ayuso]

Alessandro Vesely wrote:
> David F writes:
>> Pablo Neira Ayuso wrote:
>>>
>>> I have applied the following patch. I think that, at least, new users
>>> will not hit this problem again. I'm very sorry that this was not fixed
>>> before. Let me know if you are OK with it, we're still in time to revert
>>> the patch attached.
> 
> Waiting one version before deprecating might allow smoother changing.
> 
>> For what it's worth, I had previously prepared this patch which just
>> clarifies the documentation on this parameter.  I think it still has
>> value since I also added some missing return-value docs and changed the
>> descriptions of a few parameters that I had found to be confusing.
> 
> Good work. Is the (current) generated doc available? I've found an older
> version in http://www.nufw.org/doc/libnetfilter_queue/
> 
> I attach a patch aimed at fixing the example, which is confusing, since
> rv can simultaneously be != 0 and >= 0 only if it is > 0. I haven't
> resisted an attempt at enumerating verdicts, though.

Applied. Thanks.

Please, next time include a Signed-off-by tag, short description on the
patch and title, it makes it easier for me to apply it.

Re: libnetfilter_queue: mark-value byte ordering?

[Pablo Neira Ayuso]

David F wrote:
> Pablo Neira Ayuso wrote:
>> Alessandro Vesely wrote:
>>  
>>> David F wrote:
>>>    
>>>>  I changed my code to use htonl() on the mark-value prior to calling
>>>> nfq_set_verdict_mark(), and it all suddenly started working.
>>>>       
>>> Since it is not documented, everyone rediscovers it anew. See e.g.
>>> http://www.gossamer-threads.com/lists/iptables/devel/62591
>>>     
>>
>> I have applied the following patch. I think that, at least, new users
>> will not hit this problem again. I'm very sorry that this was not fixed
>> before. Let me know if you are OK with it, we're still in time to revert
>> the patch attached.
>>   
> For what it's worth, I had previously prepared this patch which just
> clarifies the documentation on this parameter.  I think it still has
> value since I also added some missing return-value docs and changed the
> descriptions of a few parameters that I had found to be confusing.

I have applied your patch but I have mangled this part:

@@ -699,10 +705,12 @@ int nfq_set_verdict2(struct nfq_q_handle *qh,
u_int32_t id,
  * \param qh Netfilter queue handle obtained by call to nfq_create_queue().
  * \param id	ID assigned to packet by netfilter.
  * \param verdict verdict to return to netfilter (NF_ACCEPT, NF_DROP)
- * \param mark mark to put on packet
+ * \param mark the mark to put on the packet, in network byte order.

The mark parameter in nfq_set_verdict2() is in host-byte order. It must
be in network-byte order in the deprecated nfq_set_verdict_mark().

Re: libnetfilter_queue: mark-value byte ordering?

[Pablo Neira Ayuso]

Pablo Neira Ayuso wrote:
> David F wrote:
>> Pablo Neira Ayuso wrote:
>>> Alessandro Vesely wrote:
>>>  
>>>> David F wrote:
>>>>    
>>>>>  I changed my code to use htonl() on the mark-value prior to calling
>>>>> nfq_set_verdict_mark(), and it all suddenly started working.
>>>>>       
>>>> Since it is not documented, everyone rediscovers it anew. See e.g.
>>>> http://www.gossamer-threads.com/lists/iptables/devel/62591
>>>>     
>>> I have applied the following patch. I think that, at least, new users
>>> will not hit this problem again. I'm very sorry that this was not fixed
>>> before. Let me know if you are OK with it, we're still in time to revert
>>> the patch attached.
>>>   
>> For what it's worth, I had previously prepared this patch which just
>> clarifies the documentation on this parameter.  I think it still has
>> value since I also added some missing return-value docs and changed the
>> descriptions of a few parameters that I had found to be confusing.
> 
> I have applied your patch but I have mangled this part:
> 
> @@ -699,10 +705,12 @@ int nfq_set_verdict2(struct nfq_q_handle *qh,
> u_int32_t id,
>   * \param qh Netfilter queue handle obtained by call to nfq_create_queue().
>   * \param id	ID assigned to packet by netfilter.
>   * \param verdict verdict to return to netfilter (NF_ACCEPT, NF_DROP)
> - * \param mark mark to put on packet
> + * \param mark the mark to put on the packet, in network byte order.
> 
> The mark parameter in nfq_set_verdict2() is in host-byte order. It must
> be in network-byte order in the deprecated nfq_set_verdict_mark().

Sorry, it's fine. I got confused with the patch context information.
That change applies to nfq_set_verdict_mark().

Re: libnetfilter_queue: mark-value byte ordering?

[David Favro]

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

Pablo Neira Ayuso wrote:
> Pablo Neira Ayuso wrote:
>   
>> I have applied your patch but I have mangled this part:
>>
>> @@ -699,10 +705,12 @@ int nfq_set_verdict2(struct nfq_q_handle *qh,
>> u_int32_t id,
>>   * \param qh Netfilter queue handle obtained by call to nfq_create_queue().
>>   * \param id	ID assigned to packet by netfilter.
>>   * \param verdict verdict to return to netfilter (NF_ACCEPT, NF_DROP)
>> - * \param mark mark to put on packet
>> + * \param mark the mark to put on the packet, in network byte order.
>>
>> The mark parameter in nfq_set_verdict2() is in host-byte order. It must
>> be in network-byte order in the deprecated nfq_set_verdict_mark().
>>     
>
> Sorry, it's fine. I got confused with the patch context information.
> That change applies to nfq_set_verdict_mark().
>   
I might have munged it somehow when I rebased it to follow the commit 
that created nfq_set_verdict2(), that context does look strange.  
Anyhow, it was supposed to be on nfq_set_verdict_mark().

While we're at it, here's an update to the documentation which changes 
references to nfq_set_verdict_mark() to nfq_set_verdict2().  Please 
forgive me if it seems picayune, but there's nothing wrong with having 
accurate documentation.

Thanks,
David Favro


[-- Attachment #2: documentation-verdict2.patch --]
[-- Type: text/x-patch, Size: 1453 bytes --]

Documentation update: refers to "nfq_set_verdict2()" rather than "nfq_set_verdict_mark()" which is now deprecated.

From: David Favro <netfilter@meta-dynamic.com>

Signed-off-by: David Favro <netfilter@meta-dynamic.com>
---
 src/libnetfilter_queue.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/libnetfilter_queue.c b/src/libnetfilter_queue.c
index 7d0fb45..09cde59 100644
--- a/src/libnetfilter_queue.c
+++ b/src/libnetfilter_queue.c
@@ -216,8 +216,8 @@ struct nfnl_handle *nfq_nfnlh(struct nfq_handle *h)
 		nfq_handle_packet(h, buf, rv);
 	}
 \endverbatim
- * When the decision on a packet has been choosed, the verdict has to be given
- * by calling nfq_set_verdict() or nfq_set_verdict_mark().
+ * When the decision on a packet has been chosen, the verdict has to be given
+ * by calling nfq_set_verdict() or nfq_set_verdict2().
  *
  * Data and information about the packet can be fetch by using message parsing
  * functions (See \link Parsing \endlink).
@@ -673,7 +673,7 @@ static int __set_verdict(struct nfq_q_handle *qh, u_int32_t id,
  *
  * Notifies netfilter of the userspace verdict for the given packet.  Every
  * queued packet _must_ have a verdict specified by userspace, either by
- * calling this function, or by calling the nfq_set_verdict_mark() function.
+ * calling this function, or by calling the nfq_set_verdict2() function.
  *
  * \return -1 on error; >= 0 otherwise.
  */
-- 
1.6.3.3

Re: libnetfilter_queue: mark-value byte ordering?

[Pablo Neira Ayuso]

David Favro wrote:
> Pablo Neira Ayuso wrote:
>> Pablo Neira Ayuso wrote:
>>  
>>> I have applied your patch but I have mangled this part:
>>>
>>> @@ -699,10 +705,12 @@ int nfq_set_verdict2(struct nfq_q_handle *qh,
>>> u_int32_t id,
>>>   * \param qh Netfilter queue handle obtained by call to
>>> nfq_create_queue().
>>>   * \param id    ID assigned to packet by netfilter.
>>>   * \param verdict verdict to return to netfilter (NF_ACCEPT, NF_DROP)
>>> - * \param mark mark to put on packet
>>> + * \param mark the mark to put on the packet, in network byte order.
>>>
>>> The mark parameter in nfq_set_verdict2() is in host-byte order. It must
>>> be in network-byte order in the deprecated nfq_set_verdict_mark().
>>>     
>>
>> Sorry, it's fine. I got confused with the patch context information.
>> That change applies to nfq_set_verdict_mark().
>>   
> I might have munged it somehow when I rebased it to follow the commit
> that created nfq_set_verdict2(), that context does look strange. 
> Anyhow, it was supposed to be on nfq_set_verdict_mark().
> 
> While we're at it, here's an update to the documentation which changes
> references to nfq_set_verdict_mark() to nfq_set_verdict2().  Please
> forgive me if it seems picayune, but there's nothing wrong with having
> accurate documentation.

I'm always happy to receive patches. Oh, it seems that we have clashed,
I pushed this patch a couple of hours ago:

http://git.netfilter.org/cgi-bin/gitweb.cgi?p=libnetfilter_queue.git;a=commit;h=6b4e0a01259a80d91d0eaea01281372b594f05b1