public inbox for netdev@vger.kernel.org 
 help / color / mirror / Atom feed
From: jamal <hadi@cyberus•ca>
To: Steffen Klassert <steffen.klassert@secunet•com>
Cc: davem@davemloft•net, kaber@trash•net,
	herbert@gondor•apana.org.au, yoshfuji@linux-ipv6•org,
	nakam@linux-ipv6•org, eric.dumazet@gmail•com,
	netdev@vger•kernel.org
Subject: Re: [RFC PATCH]xfrm: fix perpetual bundles
Date: Thu, 25 Feb 2010 08:19:50 -0500	[thread overview]
Message-ID: <1267103990.3973.942.camel@bigi> (raw)
In-Reply-To: <20100225104029.GB3927@gauss.dd.secunet.de>

Hi Steffen,

On Thu, 2010-02-25 at 11:40 +0100, Steffen Klassert wrote:

> 
> Do you have CONFIG_XFRM_SUB_POLICY enabled?

I tried with and without. If in xfrm_bundle_create()
after the call to xfrm_fill_dst() somewhere i "fixed" the
xdst->u.rt.fl.fl4_src, as in:
---
                err = xfrm_fill_dst(xdst, dev);
                if (err)
                        goto free_dst;
 

+               if (!xdst->u.rt.fl.fl4_src) {
+                       xdst->u.rt.fl.fl4_src = fl->fl4_src;
+		}
----

Then this worked as long as i turned off CONFIG_XFRM_SUB_POLICY.
If i use the simple patch i posted - it worked with or without
CONFIG_XFRM_SUB_POLICY turned on.

> I observed the same behaviour recently when I had CONFIG_XFRM_SUB_POLICY
> enabled. The problem in my case is, that we do a route lookup based on a flow
> with a source address of 0.0.0.0 in ip_route_output_flow() if we send a ping
> packet. Then we update the flow's source address based on the routing
> informations we got from __ip_route_output_key(). Now the actual flow does
> not match the the flow information in the routing table anymore. As a result,
> we generate a new xfrm bundle entry with every ping packet, as you pointed out.
> 

nod.

> I solved this by rerunning __ip_route_output_key() if we change the source or
> destination address of the flow (patch below). I have not send the patch so
> far because I'm not that familiar with the routing code and I'm still not sure
> whether this is the right way to fix it, so I wanted to do some further
> analysis first.
> 
> Interestingly this does not happen if CONFIG_XFRM_SUB_POLICY is disabled.
>
> When ping is started, it opens a udp socket. This triggers a xfrm_lookup()
> and a xfrm bundle entry is generated. In the standard case, the flow of the
> ping packets matching the flow informations from the bundle entry generated 
> by the opening of the udp socket, because we don't care for the upper layer
> flow information here. Unlike the standard case, if CONFIG_XFRM_SUB_POLICY is
> enabled we do upper layer information matching with flow_cache_uli_match().

Precisely - i actually was failing at exactly the same spot with
CONFIG_XFRM_SUB_POLICY with the "fix" i described above. 
"Fixing it" at that path level you have below may have bigger effect - i
cant think of one right now; but that path is hit by all outgoing
packets...

Lets hear what other people have to say - but iam beginning to feel
probably the change i posted is not so unreasonable since 0.0.0.0
is INADDR_ANY.

cheers,
jamal


  reply	other threads:[~2010-02-25 13:19 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-02-24 13:19 [RFC PATCH]xfrm: fix perpetual bundles jamal
2010-02-25 10:40 ` Steffen Klassert
2010-02-25 13:19   ` jamal [this message]
2010-02-28 14:07     ` jamal
2010-03-01 15:33       ` Steffen Klassert
2010-03-02 11:27 ` Herbert Xu
2010-03-02 12:11   ` jamal
2010-03-02 12:51     ` Herbert Xu
2010-03-02 13:10       ` jamal
2010-03-02 13:46         ` Steffen Klassert
2010-03-02 13:54           ` jamal
2010-03-02 14:06             ` David Miller
2010-03-02 14:16               ` jamal
2010-03-02 14:06             ` Steffen Klassert
2010-03-03  0:47               ` jamal
2010-03-03  9:07                 ` David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1267103990.3973.942.camel@bigi \
    --to=hadi@cyberus$(echo .)ca \
    --cc=davem@davemloft$(echo .)net \
    --cc=eric.dumazet@gmail$(echo .)com \
    --cc=herbert@gondor$(echo .)apana.org.au \
    --cc=kaber@trash$(echo .)net \
    --cc=nakam@linux-ipv6$(echo .)org \
    --cc=netdev@vger$(echo .)kernel.org \
    --cc=steffen.klassert@secunet$(echo .)com \
    --cc=yoshfuji@linux-ipv6$(echo .)org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox