[PATCH 2/2] fix NF_QUEUE_NR() macro

[PATCH 2/2] fix NF_QUEUE_NR() macro

[Harald Welte]

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

Hi Dave, please apply!

-- 
- Harald Welte <laforge@gnumonks.org>          	        http://gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
                                                  (ETSI EN 300 175-7 Ch. A6)

[-- Attachment #1.2: 44-nf_queue_nr.patch --]
[-- Type: text/plain, Size: 1001 bytes --]

[NETFILTER] Fix NF_QUEUE_NR() macro

I obviously wanted to use bitwise-or, not logical or.

Signed-off-by: Harald Welte <laforge@netfilter.org>

---
commit 359a4ba102b661dbd8da28b9dcb3330656a7a963
tree 3d1549cbff8af472bf761322b468ccd0d5be55c2
parent 5d6a0527049e43780fdaef065d0240a3338f92fd
author Harald Welte <laforge@netfilter.org> Di, 09 Aug 2005 09:32:02 +0200
committer Harald Welte <laforge@netfilter.org> Di, 09 Aug 2005 09:32:02 +0200

 include/linux/netfilter.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/linux/netfilter.h b/include/linux/netfilter.h
--- a/include/linux/netfilter.h
+++ b/include/linux/netfilter.h
@@ -29,7 +29,7 @@
 #define NF_VERDICT_QMASK 0xffff0000
 #define NF_VERDICT_QBITS 16
 
-#define NF_QUEUE_NR(x) ((x << NF_VERDICT_QBITS) & NF_VERDICT_QMASK || NF_QUEUE)
+#define NF_QUEUE_NR(x) ((x << NF_VERDICT_QBITS) & NF_VERDICT_QMASK | NF_QUEUE)
 
 /* only for userspace compatibility */
 #ifndef __KERNEL__

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

Re: [PATCH 2/2] fix NF_QUEUE_NR() macro

[Ingo Oeser]

Harald Welte wrote:
> Hi Dave, please apply!

What about using a brace more:

>diff --git a/include/linux/netfilter.h b/include/linux/netfilter.h
>--- a/include/linux/netfilter.h
>+++ b/include/linux/netfilter.h
>@@ -29,7 +29,7 @@
> #define NF_VERDICT_QMASK 0xffff0000
> #define NF_VERDICT_QBITS 16
> 
>-#define NF_QUEUE_NR(x) ((x << NF_VERDICT_QBITS) & NF_VERDICT_QMASK || NF_QUEUE)
>+#define NF_QUEUE_NR(x) ((x << NF_VERDICT_QBITS) & NF_VERDICT_QMASK | NF_QUEUE)

#define NF_QUEUE_NR(x) ((x << NF_VERDICT_QBITS) & (NF_VERDICT_QMASK | NF_QUEUE))

I know we are cool and know operator precedence even after 10 beer, but some people
consider it difficult to understand without :-)


Regards

Ingo Oeser

Re: [PATCH 2/2] fix NF_QUEUE_NR() macro

[Harald Welte]

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

On Thu, Aug 11, 2005 at 04:31:24PM +0200, Ingo Oeser wrote:
> Harald Welte wrote:
> > Hi Dave, please apply!
> 
> What about using a brace more:

I wouldn't mind the extra brace, but I don't want to make this more
difficult for Dave.  So it's his call whether he would manually edit the
line before applying the patch ;)

-- 
- 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: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH 2/2] fix NF_QUEUE_NR() macro

[Herbert Xu]

Ingo Oeser <netdev@axxeo.de> wrote:
> 
> What about using a brace more:
> 
>>diff --git a/include/linux/netfilter.h b/include/linux/netfilter.h
>>--- a/include/linux/netfilter.h
>>+++ b/include/linux/netfilter.h
>>@@ -29,7 +29,7 @@
>> #define NF_VERDICT_QMASK 0xffff0000
>> #define NF_VERDICT_QBITS 16
>> 
>>-#define NF_QUEUE_NR(x) ((x << NF_VERDICT_QBITS) & NF_VERDICT_QMASK || NF_QUEUE)
>>+#define NF_QUEUE_NR(x) ((x << NF_VERDICT_QBITS) & NF_VERDICT_QMASK | NF_QUEUE)
> 
> #define NF_QUEUE_NR(x) ((x << NF_VERDICT_QBITS) & (NF_VERDICT_QMASK | NF_QUEUE))

Actually, these two versions are different.  & is higher than | so
Harald's version is actually 

#define NF_QUEUE_NR(x) (((x << NF_VERDICT_QBITS) & NF_VERDICT_QMASK) | NF_QUEUE)

Which is it?

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH 2/2] fix NF_QUEUE_NR() macro

[Harald Welte]

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

On Fri, Aug 12, 2005 at 07:46:55AM +1000, Herbert Xu wrote:
> Ingo Oeser <netdev@axxeo.de> wrote:
> > 
> > What about using a brace more:
> > 
> >>diff --git a/include/linux/netfilter.h b/include/linux/netfilter.h
> >>--- a/include/linux/netfilter.h
> >>+++ b/include/linux/netfilter.h
> >>@@ -29,7 +29,7 @@
> >> #define NF_VERDICT_QMASK 0xffff0000
> >> #define NF_VERDICT_QBITS 16
> >> 
> >>-#define NF_QUEUE_NR(x) ((x << NF_VERDICT_QBITS) & NF_VERDICT_QMASK || NF_QUEUE)
> >>+#define NF_QUEUE_NR(x) ((x << NF_VERDICT_QBITS) & NF_VERDICT_QMASK | NF_QUEUE)
> > 
> > #define NF_QUEUE_NR(x) ((x << NF_VERDICT_QBITS) & (NF_VERDICT_QMASK | NF_QUEUE))
> 
> Actually, these two versions are different.  & is higher than | so
> Harald's version is actually 
> 
> #define NF_QUEUE_NR(x) (((x << NF_VERDICT_QBITS) & NF_VERDICT_QMASK) | NF_QUEUE)
> 
> Which is it?

the latter.  So my original patch is right (and is also confirmed to
work in tests by some users and myself), I 'just' didn't review Ingo's
alternative line (embarrassing).

We have that saying in german: "I didn't see the wood because of all the
trees".  Somehow I feel like that now...

-- 
- 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: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH 2/2] fix NF_QUEUE_NR() macro

[David S. Miller]

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Fri, 12 Aug 2005 07:46:55 +1000

> Actually, these two versions are different.  & is higher than | so
> Harald's version is actually 
> 
> #define NF_QUEUE_NR(x) (((x << NF_VERDICT_QBITS) & NF_VERDICT_QMASK) | NF_QUEUE)
> 
> Which is it?

It's the former that is the intended version.

I put Harald's patch in as-is, if people want to pretty it
up please do so with a follow-on patch but I really don't
think it matters :-)

Re: [PATCH 2/2] fix NF_QUEUE_NR() macro

[David S. Miller]

From: Harald Welte <laforge@gnumonks.org>
Date: Thu, 11 Aug 2005 09:13:21 +0200

> [NETFILTER] Fix NF_QUEUE_NR() macro
> 
> I obviously wanted to use bitwise-or, not logical or.
> 
> Signed-off-by: Harald Welte <laforge@netfilter.org>

Applied.