Velocity Reviews

Velocity Reviews (http://www.velocityreviews.com/forums/index.php)
-   C Programming (http://www.velocityreviews.com/forums/f42-c-programming.html)
-   -   Could someone please explain this code mystery (http://www.velocityreviews.com/forums/t805619-could-someone-please-explain-this-code-mystery.html)

wkevin 11-06-2011 08:06 PM

Could someone please explain this code mystery
 
Hi all,
I delved into code of net/xfrm/xfrm_policy.c and there is something
which seems quite mysterious to
me . I would appreciate if someone could explain.
I am looking in kernel 2.6.32.11, but this code seems to be from long
ago with no change.

we have, in __xfrm_policy_check(), this msytery:
{
....
int reverse;
....
reverse = dir & ~XFRM_POLICY_MASK;
dir &= XFRM_POLICY_MASK;
....

Now , we have this enum:

enum
{
XFRM_POLICY_IN = 0,
XFRM_POLICY_IN = 1,
XFRM_POLICY_FWD = 2,
XFRM_POLICY_MASK = 3,
XFRM_POLICY_MAX = 3
};


and dir can be XFRM_POLICY_IN or XFRM_POLICY_OUT or XFRM_POLICY_FW.


Now , isn't reverse value is always 0 ?

I tried this short problem covering all three cases:
int main()
{
int reverse;
reverse = XFRM_POLICY_IN & ~XFRM_POLICY_MASK;
printf("XFRM_POLICY_IN & ~XFRM_POLICY_MASK = %d in %s
\n",reverse,__func__);
reverse = XFRM_POLICY_OUT & ~XFRM_POLICY_MASK;
printf("XFRM_POLICY_OUT & ~XFRM_POLICY_MASK = %d in %s
\n",reverse,__func__);

reverse = XFRM_POLICY_FWD & ~XFRM_POLICY_MASK;
printf("XFRM_POLICY_FWD & ~XFRM_POLICY_MASK = %d in %s
\n",reverse,__func__);


}
and it prints 0 in all three cases:

XFRM_POLICY_IN & ~XFRM_POLICY_MASK = 0 in main
XFRM_POLICY_OUT & ~XFRM_POLICY_MASK = 0 in main
XFRM_POLICY_FWD & ~XFRM_POLICY_MASK = 0 in main


so what is the purpose of
reverse = dir & ~XFRM_POLICY_MASK;
why not simply: reverse = XFRM_POLICY_IN
if in all cases it sets reverse to 0 ?

rgs,
Kevin

Hans Vlems 11-06-2011 08:17 PM

Re: Could someone please explain this code mystery
 
On Nov 6, 9:06*pm, wkevin <wkev...@gmail.com> wrote:
> Hi all,
> I delved into code of net/xfrm/xfrm_policy.c and there is something
> which seems quite mysterious to
> me . I would appreciate if someone could explain.
> I am looking in kernel 2.6.32.11, but this code seems to be from long
> ago with no change.
>
> we have, in __xfrm_policy_check(), this msytery:
> {
> ...
> * * * *int reverse;
> ...
> * * * *reverse = dir & ~XFRM_POLICY_MASK;
> * * * *dir &= XFRM_POLICY_MASK;
> ...
>
> Now , we have this enum:
>
> enum
> {
> * * * *XFRM_POLICY_IN *= 0,
> * * * *XFRM_POLICY_IN *= 1,
> * * * *XFRM_POLICY_FWD = 2,
> * * * *XFRM_POLICY_MASK = 3,
> * * * *XFRM_POLICY_MAX = 3
>
> };
>
> and dir can be XFRM_POLICY_IN or XFRM_POLICY_OUT or XFRM_POLICY_FW.
>
> Now , isn't reverse value is always 0 ?
>
> I tried this short problem covering all three cases:
> int main()
> {
> * * * *int reverse;
> * * * *reverse = XFRM_POLICY_IN & ~XFRM_POLICY_MASK;
> * * * *printf("XFRM_POLICY_IN & ~XFRM_POLICY_MASK = %d in %s
> \n",reverse,__func__);
> * * * *reverse = XFRM_POLICY_OUT & ~XFRM_POLICY_MASK;
> * * * *printf("XFRM_POLICY_OUT & ~XFRM_POLICY_MASK = %d in %s
> \n",reverse,__func__);
>
> * * * *reverse = XFRM_POLICY_FWD & ~XFRM_POLICY_MASK;
> * * * *printf("XFRM_POLICY_FWD & ~XFRM_POLICY_MASK = %d in %s
> \n",reverse,__func__);
>
> }
>
> and it prints 0 in all three cases:
>
> XFRM_POLICY_IN & ~XFRM_POLICY_MASK = 0 in main
> XFRM_POLICY_OUT & ~XFRM_POLICY_MASK = 0 in main
> XFRM_POLICY_FWD & ~XFRM_POLICY_MASK = 0 in main
>
> so what is the purpose of
> reverse = dir & ~XFRM_POLICY_MASK;
> why not simply: reverse = XFRM_POLICY_IN
> if in all cases it sets reverse to 0 ?
>
> rgs,
> Kevin


I'm guessing here, but I think you've spent so much time trying to
figure out
what the boolean expressions do that you've forgotten to note where
(in what
variables) the two results are stored.
Hans

Eric Sosman 11-06-2011 09:26 PM

Re: Could someone please explain this code mystery
 
On 11/6/2011 3:06 PM, wkevin wrote:
> Hi all,
> I delved into code of net/xfrm/xfrm_policy.c and there is something
> which seems quite mysterious to
> me . I would appreciate if someone could explain.
> I am looking in kernel 2.6.32.11, but this code seems to be from long
> ago with no change.
>
> we have, in __xfrm_policy_check(), this msytery:
> {
> ...
> int reverse;
> ...
> reverse = dir& ~XFRM_POLICY_MASK;
> dir&= XFRM_POLICY_MASK;
> ...
>
> Now , we have this enum:
>
> enum
> {
> XFRM_POLICY_IN = 0,
> XFRM_POLICY_IN = 1,
> XFRM_POLICY_FWD = 2,
> XFRM_POLICY_MASK = 3,
> XFRM_POLICY_MAX = 3
> };
>
>
> and dir can be XFRM_POLICY_IN or XFRM_POLICY_OUT or XFRM_POLICY_FW.
>
>
> Now , isn't reverse value is always 0 ?


If `dir' has one of the three values you say, then yes: `reverse'
will be zero. Also, the second assignment will leave `dir' unchanged.

My guess is that `dir' is not always as you say, but sometimes
can have bits other than the low-order pair set. Or, perhaps, `dir'
always has one of the three stated values today, but in days gone by
it used to have additional bits. Or, perhaps, `dir' is as you say
but the code author thinks additional bits may be used in the future.

--
Eric Sosman
esosman@ieee-dot-org.invalid

Alan Curry 11-06-2011 10:20 PM

Re: Could someone please explain this code mystery
 
In article <cd6295cd-3530-4ba2-ae5d-5a68f10d6ece@hc5g2000vbb.googlegroups.com>,
wkevin <wkevils@gmail.com> wrote:
>Hi all,
>I delved into code of net/xfrm/xfrm_policy.c and there is something
>which seems quite mysterious to
>me . I would appreciate if someone could explain.
>I am looking in kernel 2.6.32.11, but this code seems to be from long
>ago with no change.
>
>we have, in __xfrm_policy_check(), this msytery:
>{
>...
> int reverse;
>...
> reverse = dir & ~XFRM_POLICY_MASK;
> dir &= XFRM_POLICY_MASK;
>...


$ git blame net/xfrm/xfrm_policy.c | fgrep 'reverse ='
d5422efe (Herbert Xu 2007-12-12 10:44:16 -0800 2044) reverse = dir & ~XFRM_POLICY_MASK;
$ git show d5422efe
commit d5422efe680fc55010c6ddca2370ca9548a96355
Author: Herbert Xu <herbert@gondor.apana.org.au>
Date: Wed Dec 12 10:44:16 2007 -0800

[IPSEC]: Added xfrm_decode_session_reverse and xfrmX_policy_check_reverse

RFC 4301 requires us to relookup ICMP traffic that does not match any
policies using the reverse of its payload. This patch adds the functions
xfrm_decode_session_reverse and xfrmX_policy_check_reverse so we can get
the reverse flow to perform such a lookup.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: David S. Miller <davem@davemloft.net>
[snip]
+ int ndir = dir | (reverse ? XFRM_POLICY_MASK + 1 : 0);
+
if (sk && sk->sk_policy[XFRM_POLICY_IN])
- return __xfrm_policy_check(sk, dir, skb, family);
+ return __xfrm_policy_check(sk, ndir, skb, family);

return (!xfrm_policy_count[dir] && !skb->sp) ||
(skb->dst->flags & DST_NOPOLICY) ||
- __xfrm_policy_check(sk, dir, skb, family);
+ __xfrm_policy_check(sk, ndir, skb, family);
[snip]

So that's where dir can get a higher bit set. I think it would look better if
that bit was given a name in the enum instead of using XFRM_POLICY_MASK+1

--
Alan Curry

Edward A. Falk 11-07-2011 01:26 AM

Re: Could someone please explain this code mystery
 
In article <cd6295cd-3530-4ba2-ae5d-5a68f10d6ece@hc5g2000vbb.googlegroups.com>,
wkevin <wkevils@gmail.com> wrote:
>Hi all,
>I delved into code of net/xfrm/xfrm_policy.c and there is something
>which seems quite mysterious to
>me . I would appreciate if someone could explain.
>I am looking in kernel 2.6.32.11, but this code seems to be from long
>ago with no change.
>
>we have, in __xfrm_policy_check(), this msytery:
>{
>...
> int reverse;
>...
> reverse = dir & ~XFRM_POLICY_MASK;
> dir &= XFRM_POLICY_MASK;
>...


>Now , we have this enum:
>
>enum
>{
> XFRM_POLICY_IN = 0,
> XFRM_POLICY_IN = 1,
> XFRM_POLICY_FWD = 2,
> XFRM_POLICY_MASK = 3,
>};


OK, to start with, this won't compile, and there's no XFRM_POLICY_OUT declaration,
so I'm going to guess that you typed it in wrong and the actual values are:

XFRM_POLICY_IN = 0,
XFRM_POLICY_OUT = 1,
XFRM_POLICY_FWD = 2,
XFRM_POLICY_MAX = 3
XFRM_POLICY_MASK = 3,

What's going on is that the value "dir" is several different values
packed into one integer. The low-order two bits are the "policy",
which can be IN, OUT, FWD, or MAX. The mask XFRM_POLICY_MASK can be
used to isolate the policy from "dir", and inverting the mask will
allow you to isolate all the bits *except* the policy value.

This is a standard C programming technique for unpacking multiple
values which were stored in a single word.

For clarity, the code could have been better written this way:

int reverse, policy;

policy = dir & XFRM_POLICY_MASK;
reverse = dir & ~XFRM_POLICY_MASK;

It looks like the original author was trying to be clever and re-purpose the
"dir" variable to hold the policy value after the reverse value had been
extracted. However, any decent compiler would have generated the same code
either way, so the author sacrificed clarity for nothing.

As for the rest of your question, I think you mis-read the code. This
is understandable, since I mis-read it the same way at first glance.

You're right that the code

dir &= XFRM_POLICY_MASK;
dir &= ~XFRM_POLICY_MASK;

would have resulted in zero, but go look at the code a second time.

--
-Ed Falk, falk@despams.r.us.com
http://thespamdiaries.blogspot.com/


All times are GMT. The time now is 12:16 PM.

Powered by vBulletin®. Copyright ©2000 - 2014, vBulletin Solutions, Inc.
SEO by vBSEO ©2010, Crawlability, Inc.