[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20140424151647.2a917ff1@vostro>
Date: Thu, 24 Apr 2014 15:16:47 +0300
From: Timo Teras <timo.teras@....fi>
To: Tom Herbert <therbert@...gle.com>
Cc: davem@...emloft.net, netdev@...r.kernel.org, eric.dumazet@...il.com
Subject: Re: [1/2,v3] ipv4: Cache dst in tunnels
On Thu, 2 Jan 2014 11:48:26 -0800 (PST)
Tom Herbert <therbert@...gle.com> wrote:
> Avoid doing a route lookup on every packet being tunneled.
>
> In ip_tunnel.c cache the route returned from ip_route_output if
> the tunnel is "connected" so that all the rouitng parameters are
> taken from tunnel parms for a packet. Specifically, not NBMA tunnel
> and tos is from tunnel parms (not inner packet).
Sorry for late reply. The 'connected' check you do is not sufficient.
It breaks NBMA mode gre. See below.
> @@ -528,10 +574,11 @@ void ip_tunnel_xmit(struct sk_buff *skb, struct
> net_device *dev, struct flowi4 fl4;
> u8 tos, ttl;
> __be16 df;
> - struct rtable *rt; /* Route to the other host
> */
> + struct rtable *rt = NULL; /* Route to the other host
> */ unsigned int max_headroom; /* The extra header space needed
> */ __be32 dst;
> int err;
> + bool connected = true;
>
> inner_iph = (const struct iphdr
> *)skb_inner_network_header(skb);
> @@ -581,27 +628,39 @@ void ip_tunnel_xmit(struct sk_buff *skb, struct
> net_device *dev, #endif
> else
> goto tx_error;
> +
> + connected = false;
> }
The assumption in these two hunks is wrong. Even if tnl_params->daddr is
set, it might be an NBMA mode tunnel.
This happens (at least) when ipgre_header() is used and it pushes the
modified iphdr to skb. In that case ipgre_xmit just pulls it, and gives
that as the tnl_params argument.
I don't remember fully if tnl_params->daddr can be zero in
ip_tunnel_xmit. It *might* be a leftover dead-code path.
Any ideas how to fix this in a proper manner?
Thanks. Timo.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists