Could someone please explain this code mystery

Discussion in 'C Programming' started by wkevin, Nov 6, 2011.

  1. wkevin

    wkevin Guest

    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
     
    wkevin, Nov 6, 2011
    #1
    1. Advertising

  2. wkevin

    Hans Vlems Guest

    On Nov 6, 9: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 ?
    >
    > 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
     
    Hans Vlems, Nov 6, 2011
    #2
    1. Advertising

  3. wkevin

    Eric Sosman Guest

    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
    d
     
    Eric Sosman, Nov 6, 2011
    #3
  4. wkevin

    Alan Curry Guest

    In article <>,
    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;
    >...


    $ 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 <>
    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 <>
    Signed-off-by: David S. Miller <>
    [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
     
    Alan Curry, Nov 6, 2011
    #4
  5. In article <>,
    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,
    >};


    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,
    http://thespamdiaries.blogspot.com/
     
    Edward A. Falk, Nov 7, 2011
    #5
    1. Advertising

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

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Gactimus
    Replies:
    22
    Views:
    749
    jeffc
    Dec 15, 2004
  2. Replies:
    5
    Views:
    309
    Bo Persson
    Jan 10, 2006
  3. Alexander Mahone

    Could someone please explain me this error?

    Alexander Mahone, May 2, 2008, in forum: C Programming
    Replies:
    5
    Views:
    286
    Igmar Palsenberg
    May 5, 2008
  4. Rubyist
    Replies:
    1
    Views:
    118
    Ross Bamford
    Feb 2, 2006
  5. John Small
    Replies:
    4
    Views:
    128
    John Small
    Feb 2, 2009
Loading...

Share This Page