| Message ID | 1386838432-18889-1-git-send-email-timo.teras@iki.fi |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Timo Teräs <timo.teras@iki.fi> Date: Thu, 12 Dec 2013 10:53:52 +0200 > ipgre_header_parse() needs to parse the tunnel's ip header for getting > the link-layer addresses, and uses mac_header to get the header. This > fixes setting mac_header on the receive path to original behaviour. > > Bug added in commit c54419321455 (GRE: Refactor GRE tunneling code.) > > Cc: Pravin B Shelar <pshelar@nicira.com> > Signed-off-by: Timo Teräs <timo.teras@iki.fi> > --- > Alternate would be to do skb_reset_inner_headers() in ip_tunnel_rcv() and > update ipgre_header_parse() to use skb_inner_network_header(). Though, then > inner would then be referring to the "outer" headers. > > If applied as-is, should go to -stable too. Would skb_reset_mac_header() work just as equally? I'd prefer that over direct modification of skb->*_header values. Thanks. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 12 Dec 2013 15:34:29 -0500 (EST) David Miller <davem@davemloft.net> wrote: > From: Timo Teräs <timo.teras@iki.fi> > Date: Thu, 12 Dec 2013 10:53:52 +0200 > > > ipgre_header_parse() needs to parse the tunnel's ip header for > > getting the link-layer addresses, and uses mac_header to get the > > header. This fixes setting mac_header on the receive path to > > original behaviour. > > > > Bug added in commit c54419321455 (GRE: Refactor GRE tunneling code.) > > > > Cc: Pravin B Shelar <pshelar@nicira.com> > > Signed-off-by: Timo Teräs <timo.teras@iki.fi> > > --- > > Alternate would be to do skb_reset_inner_headers() in > > ip_tunnel_rcv() and update ipgre_header_parse() to use > > skb_inner_network_header(). Though, then inner would then be > > referring to the "outer" headers. > > > > If applied as-is, should go to -stable too. > > Would skb_reset_mac_header() work just as equally? I'd prefer that > over direct modification of skb->*_header values. No. It was actually converted originally to skb_reset_mac_header() but that led to problems. See commit 1d0691674764098304ae4c63c715f588. When ipgre_rcv is entered, the outer iphdr has been already pulled, thus head points to the GRE header. So skb_reset_mac_header() would make it point at the GRE header instead of the desired outer IP header. But I agree that it'd be better to use helpers. That's why I suggested the skb_reset_inner_headers() as an alternative. But as it's slightly more intrusive approach, I opted to just restore the original assignment as first suggestion. - Timo -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Timo Teras <timo.teras@iki.fi> Date: Fri, 13 Dec 2013 08:08:44 +0200 > On Thu, 12 Dec 2013 15:34:29 -0500 (EST) > David Miller <davem@davemloft.net> wrote: > >> From: Timo Teräs <timo.teras@iki.fi> >> Date: Thu, 12 Dec 2013 10:53:52 +0200 >> >> > ipgre_header_parse() needs to parse the tunnel's ip header for >> > getting the link-layer addresses, and uses mac_header to get the >> > header. This fixes setting mac_header on the receive path to >> > original behaviour. >> > >> > Bug added in commit c54419321455 (GRE: Refactor GRE tunneling code.) >> > >> > Cc: Pravin B Shelar <pshelar@nicira.com> >> > Signed-off-by: Timo Teräs <timo.teras@iki.fi> >> > --- >> > Alternate would be to do skb_reset_inner_headers() in >> > ip_tunnel_rcv() and update ipgre_header_parse() to use >> > skb_inner_network_header(). Though, then inner would then be >> > referring to the "outer" headers. >> > >> > If applied as-is, should go to -stable too. >> >> Would skb_reset_mac_header() work just as equally? I'd prefer that >> over direct modification of skb->*_header values. > > No. It was actually converted originally to skb_reset_mac_header() but > that led to problems. See commit 1d0691674764098304ae4c63c715f588. > > When ipgre_rcv is entered, the outer iphdr has been already pulled, > thus head points to the GRE header. So skb_reset_mac_header() would > make it point at the GRE header instead of the desired outer IP header. > > But I agree that it'd be better to use helpers. That's why I suggested > the skb_reset_inner_headers() as an alternative. But as it's slightly > more intrusive approach, I opted to just restore the original assignment > as first suggestion. Just make a helper that does this mac_header = network_header thing, call it something like "skb_pop_mac_header(skb)" and the make the ip_gre code use it. Thanks. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c index d7aea4c..cc2c7f1 100644 --- a/net/ipv4/ip_gre.c +++ b/net/ipv4/ip_gre.c @@ -217,6 +217,7 @@ static int ipgre_rcv(struct sk_buff *skb, const struct tnl_ptk_info *tpi) iph->saddr, iph->daddr, tpi->key); if (tunnel) { + skb->mac_header = skb->network_header; ip_tunnel_rcv(tunnel, skb, tpi, log_ecn_error); return PACKET_RCVD; }
ipgre_header_parse() needs to parse the tunnel's ip header for getting the link-layer addresses, and uses mac_header to get the header. This fixes setting mac_header on the receive path to original behaviour. Bug added in commit c54419321455 (GRE: Refactor GRE tunneling code.) Cc: Pravin B Shelar <pshelar@nicira.com> Signed-off-by: Timo Teräs <timo.teras@iki.fi> --- Alternate would be to do skb_reset_inner_headers() in ip_tunnel_rcv() and update ipgre_header_parse() to use skb_inner_network_header(). Though, then inner would then be referring to the "outer" headers. If applied as-is, should go to -stable too. net/ipv4/ip_gre.c | 1 + 1 file changed, 1 insertion(+)