Could someone please explain this code mystery

W

wkevin

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
 
H

Hans Vlems

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
 
E

Eric Sosman

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.
 
A

Alan Curry

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 <[email protected]>
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 <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
[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
 
E

Edward A. Falk

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.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,744
Messages
2,569,484
Members
44,904
Latest member
HealthyVisionsCBDPrice

Latest Threads

Top