All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ulrich Weber <ulrich.weber@sophos.com>
To: Julian Anastasov <ja@ssi.bg>
Cc: "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"davem@davemloft.net" <davem@davemloft.net>
Subject: Re: [PATCH 2/3] route: set iif and oif information in flowi struct
Date: Wed, 30 Nov 2011 18:21:52 +0100	[thread overview]
Message-ID: <4ED66630.8030308@sophos.com> (raw)
In-Reply-To: <alpine.LFD.2.00.1111290051240.2497@ja.ssi.bg>

On 29.11.2011 00:53, Julian Anastasov wrote:
>
> 	May be setting flowi4_oif unconditionally here is more
> correct because ip_route_output_slow fills flowi4_oif with
> the selected oif, it can even change the provided original
> oif in flowi4_oif. What about this?:
>
> 			flp4->flowi4_oif = rth->dst.dev->ifindex;
>
> 	OTOH, rt_iif has some complex semantic: original oif
> or the selected oif. May be you prefer flowi4_oif to hold
> the selected oif, right?
I wasn't aware the ip_route_output_slow() might change the original oif.
You know why this might happen? Shouldn't fib_lookup only return
a route matching the given oif?

Anyway, if thats the case your code above is more correct. The packet
should always match the xfrm policy where it was originally routed.

> 	I see one dangerous place that must be checked:
> icmp_route_lookup. Before now __ip_route_output_key was
> called after xfrm_decode_session_reverse with 0 in
> flowi4_oif, i.e. no oif binding was used. But now when
> decode_session sets flowi4_oif we will restrict the route
> via this interface?
Thanks for the hint! Yes the current patch will force the ICMP packet
over the received interface.

Will add "fl4_dec.flowi4_oif = 0;" in case the saddr is local, so the
behavior will be the same. fl4_dec.flowi4_oif will then be set in
_ip_route_output_key() again.

Cheers
Ulrich

-- 
Ulrich Weber | ulrich.weber@sophos.com | Senior Software Engineer
Astaro - a Sophos company | Amalienbadstr 41 | 76227 Karlsruhe | Germany
Phone +49-721-25516-0 | Fax –200 | www.astaro.com

  reply	other threads:[~2011-11-30 17:21 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-11-28 20:14 [PATCH 0/3] xfrm: add incoming interface to selector Ulrich Weber
2011-11-28 20:14 ` [PATCH 1/3] " Ulrich Weber
2011-11-30  0:00   ` David Miller
2011-11-30 17:33     ` Ulrich Weber
2011-11-30 17:47       ` David Miller
2011-11-28 20:14 ` [PATCH 2/3] route: set iif and oif information in flowi struct Ulrich Weber
2011-11-28 23:53   ` Julian Anastasov
2011-11-30 17:21     ` Ulrich Weber [this message]
2011-11-30 22:37       ` Julian Anastasov
2011-11-30  0:01   ` David Miller
2011-11-28 20:14 ` [PATCH 3/3] xfrm: allow to overwrite incoming dev after decryption Ulrich Weber

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=4ED66630.8030308@sophos.com \
    --to=ulrich.weber@sophos.com \
    --cc=davem@davemloft.net \
    --cc=ja@ssi.bg \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.