All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ip_tunnel: Add ip tunnel tunnel_info dst_cache in ip_tunnel_xmit
@ 2019-02-21 12:08 wenxu
  2019-02-21 18:37 ` David Miller
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: wenxu @ 2019-02-21 12:08 UTC (permalink / raw)
  To: netdev, davem

From: wenxu <wenxu@ucloud.cn>

ip l add dev tun type gretap key 1000

Non-tunnel-dst ip tunnel device can send packet through lwtunnel.
This patch provide the tun_info dst cache support for this mode

Signed-off-by: wenxu <wenxu@ucloud.cn>
---
 net/ipv4/ip_tunnel.c | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
index 893f013..874ad58 100644
--- a/net/ipv4/ip_tunnel.c
+++ b/net/ipv4/ip_tunnel.c
@@ -662,6 +662,9 @@ void ip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
 	unsigned int max_headroom;	/* The extra header space needed */
 	__be32 dst;
 	bool connected;
+	bool use_cache = false;
+	bool md = false;
+	struct ip_tunnel_info *tun_info;
 
 	inner_iph = (const struct iphdr *)skb_inner_network_header(skb);
 	connected = (tunnel->parms.iph.daddr != 0);
@@ -671,7 +674,6 @@ void ip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
 	dst = tnl_params->daddr;
 	if (dst == 0) {
 		/* NBMA tunnel */
-		struct ip_tunnel_info *tun_info;
 
 		if (!skb_dst(skb)) {
 			dev->stats.tx_fifo_errors++;
@@ -681,8 +683,11 @@ void ip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
 		tun_info = skb_tunnel_info(skb);
 		if (tun_info && (tun_info->mode & IP_TUNNEL_INFO_TX) &&
 		    ip_tunnel_info_af(tun_info) == AF_INET &&
-		    tun_info->key.u.ipv4.dst)
+		    tun_info->key.u.ipv4.dst) {
 			dst = tun_info->key.u.ipv4.dst;
+			md = true;
+			connected = true;
+		}
 		else if (skb->protocol == htons(ETH_P_IP)) {
 			rt = skb_rtable(skb);
 			dst = rt_nexthop(rt, inner_iph->daddr);
@@ -721,7 +726,8 @@ void ip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
 		else
 			goto tx_error;
 
-		connected = false;
+		if (!md)
+			connected = false;
 	}
 
 	tos = tnl_params->tos;
@@ -743,8 +749,14 @@ void ip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
 	if (ip_tunnel_encap(skb, tunnel, &protocol, &fl4) < 0)
 		goto tx_error;
 
-	rt = connected ? dst_cache_get_ip4(&tunnel->dst_cache, &fl4.saddr) :
+	if (connected && md) {
+		use_cache = ip_tunnel_dst_cache_usable(skb, tun_info);
+		if (use_cache)
+			rt = dst_cache_get_ip4(&tun_info->dst_cache, &fl4.saddr);
+	} else {
+		rt = connected ? dst_cache_get_ip4(&tunnel->dst_cache, &fl4.saddr) :
 			 NULL;
+	}
 
 	if (!rt) {
 		rt = ip_route_output_key(tunnel->net, &fl4);
@@ -753,7 +765,10 @@ void ip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
 			dev->stats.tx_carrier_errors++;
 			goto tx_error;
 		}
-		if (connected)
+		if (use_cache)
+			dst_cache_set_ip4(&tun_info->dst_cache, &rt->dst,
+					  fl4.saddr);
+		else if (!md && connected)
 			dst_cache_set_ip4(&tunnel->dst_cache, &rt->dst,
 					  fl4.saddr);
 	}
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH] ip_tunnel: Add ip tunnel tunnel_info dst_cache in ip_tunnel_xmit
  2019-02-21 12:08 [PATCH] ip_tunnel: Add ip tunnel tunnel_info dst_cache in ip_tunnel_xmit wenxu
@ 2019-02-21 18:37 ` David Miller
  2019-02-23 23:28 ` kbuild test robot
  2019-02-25  6:27 ` Dan Carpenter
  2 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2019-02-21 18:37 UTC (permalink / raw)
  To: wenxu; +Cc: netdev

From: wenxu@ucloud.cn
Date: Thu, 21 Feb 2019 20:08:50 +0800

> From: wenxu <wenxu@ucloud.cn>
> 
> ip l add dev tun type gretap key 1000
> 
> Non-tunnel-dst ip tunnel device can send packet through lwtunnel.
> This patch provide the tun_info dst cache support for this mode
> 
> Signed-off-by: wenxu <wenxu@ucloud.cn>
> ---
>  net/ipv4/ip_tunnel.c | 25 ++++++++++++++++++++-----
>  1 file changed, 20 insertions(+), 5 deletions(-)
> 
> diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
> index 893f013..874ad58 100644
> --- a/net/ipv4/ip_tunnel.c
> +++ b/net/ipv4/ip_tunnel.c
> @@ -662,6 +662,9 @@ void ip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
>  	unsigned int max_headroom;	/* The extra header space needed */
>  	__be32 dst;
>  	bool connected;
> +	bool use_cache = false;
> +	bool md = false;
> +	struct ip_tunnel_info *tun_info;

Reverse christmas tree, please.

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH] ip_tunnel: Add ip tunnel tunnel_info dst_cache in ip_tunnel_xmit
  2019-02-21 12:08 [PATCH] ip_tunnel: Add ip tunnel tunnel_info dst_cache in ip_tunnel_xmit wenxu
  2019-02-21 18:37 ` David Miller
@ 2019-02-23 23:28 ` kbuild test robot
  2019-02-25  6:27 ` Dan Carpenter
  2 siblings, 0 replies; 4+ messages in thread
From: kbuild test robot @ 2019-02-23 23:28 UTC (permalink / raw)
  To: wenxu; +Cc: kbuild-all, netdev, davem

[-- Attachment #1: Type: text/plain, Size: 3301 bytes --]

Hi wenxu,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net/master]
[also build test WARNING on v5.0-rc4 next-20190222]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/wenxu-ucloud-cn/ip_tunnel-Add-ip-tunnel-tunnel_info-dst_cache-in-ip_tunnel_xmit/20190224-065214
config: x86_64-randconfig-x016-201908 (attached as .config)
compiler: gcc-8 (Debian 8.2.0-20) 8.2.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   In file included from net/ipv4/ip_tunnel.c:48:
   net/ipv4/ip_tunnel.c: In function 'ip_tunnel_xmit':
>> include/net/ip_tunnels.h:207:15: warning: 'tun_info' may be used uninitialized in this function [-Wmaybe-uninitialized]
     if (info->key.tun_flags & TUNNEL_NOCACHE)
         ~~~~~~~~~^~~~~~~~~~
   net/ipv4/ip_tunnel.c:640:25: note: 'tun_info' was declared here
     struct ip_tunnel_info *tun_info;
                            ^~~~~~~~
   net/ipv4/ip_tunnel.c:734:5: warning: 'rt' may be used uninitialized in this function [-Wmaybe-uninitialized]
     if (!rt) {
        ^
--
   In file included from net//ipv4/ip_tunnel.c:48:
   net//ipv4/ip_tunnel.c: In function 'ip_tunnel_xmit':
>> include/net/ip_tunnels.h:207:15: warning: 'tun_info' may be used uninitialized in this function [-Wmaybe-uninitialized]
     if (info->key.tun_flags & TUNNEL_NOCACHE)
         ~~~~~~~~~^~~~~~~~~~
   net//ipv4/ip_tunnel.c:640:25: note: 'tun_info' was declared here
     struct ip_tunnel_info *tun_info;
                            ^~~~~~~~
   net//ipv4/ip_tunnel.c:734:5: warning: 'rt' may be used uninitialized in this function [-Wmaybe-uninitialized]
     if (!rt) {
        ^

vim +/tun_info +207 include/net/ip_tunnels.h

1d8fff90 Thomas Graf     2015-07-21  198  
db3c6139 Daniel Borkmann 2016-03-04  199  static inline bool
db3c6139 Daniel Borkmann 2016-03-04  200  ip_tunnel_dst_cache_usable(const struct sk_buff *skb,
db3c6139 Daniel Borkmann 2016-03-04  201  			   const struct ip_tunnel_info *info)
db3c6139 Daniel Borkmann 2016-03-04  202  {
db3c6139 Daniel Borkmann 2016-03-04  203  	if (skb->mark)
db3c6139 Daniel Borkmann 2016-03-04  204  		return false;
db3c6139 Daniel Borkmann 2016-03-04  205  	if (!info)
db3c6139 Daniel Borkmann 2016-03-04  206  		return true;
db3c6139 Daniel Borkmann 2016-03-04 @207  	if (info->key.tun_flags & TUNNEL_NOCACHE)
db3c6139 Daniel Borkmann 2016-03-04  208  		return false;
db3c6139 Daniel Borkmann 2016-03-04  209  
db3c6139 Daniel Borkmann 2016-03-04  210  	return true;
db3c6139 Daniel Borkmann 2016-03-04  211  }
db3c6139 Daniel Borkmann 2016-03-04  212  

:::::: The code at line 207 was first introduced by commit
:::::: db3c6139e6ead91b42e7c2ad044ed8beaee884e6 bpf, vxlan, geneve, gre: fix usage of dst_cache on xmit

:::::: TO: Daniel Borkmann <daniel@iogearbox.net>
:::::: CC: David S. Miller <davem@davemloft.net>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28325 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH] ip_tunnel: Add ip tunnel tunnel_info dst_cache in ip_tunnel_xmit
  2019-02-21 12:08 [PATCH] ip_tunnel: Add ip tunnel tunnel_info dst_cache in ip_tunnel_xmit wenxu
  2019-02-21 18:37 ` David Miller
  2019-02-23 23:28 ` kbuild test robot
@ 2019-02-25  6:27 ` Dan Carpenter
  2 siblings, 0 replies; 4+ messages in thread
From: Dan Carpenter @ 2019-02-25  6:27 UTC (permalink / raw)
  To: kbuild, wenxu; +Cc: kbuild-all, netdev, davem

Hi wenxu,

Thank you for the patch! Perhaps something to improve:

url:    https://github.com/0day-ci/linux/commits/wenxu-ucloud-cn/ip_tunnel-Add-ip-tunnel-tunnel_info-dst_cache-in-ip_tunnel_xmit/20190224-065214

smatch warnings:
net/ipv4/ip_tunnel.c:734 ip_tunnel_xmit() error: uninitialized symbol 'rt'.

# https://github.com/0day-ci/linux/commit/fb0c8c40052c62033bf5f92559eabe4e7a98851e
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout fb0c8c40052c62033bf5f92559eabe4e7a98851e
vim +/rt +734 net/ipv4/ip_tunnel.c

7d442fab0 Tom Herbert        2014-01-02  717  
b0066da52 Petr Machata       2018-02-27  718  	ip_tunnel_init_flow(&fl4, protocol, dst, tnl_params->saddr,
9830ad4c6 Craig Gallek       2017-04-19  719  			    tunnel->parms.o_key, RT_TOS(tos), tunnel->parms.link,
9830ad4c6 Craig Gallek       2017-04-19  720  			    tunnel->fwmark);
7d442fab0 Tom Herbert        2014-01-02  721  
563284865 Tom Herbert        2014-09-17  722  	if (ip_tunnel_encap(skb, tunnel, &protocol, &fl4) < 0)
563284865 Tom Herbert        2014-09-17  723  		goto tx_error;
563284865 Tom Herbert        2014-09-17  724  
fb0c8c400 wenxu              2019-02-21  725  	if (connected && md) {
fb0c8c400 wenxu              2019-02-21  726  		use_cache = ip_tunnel_dst_cache_usable(skb, tun_info);
fb0c8c400 wenxu              2019-02-21  727  		if (use_cache)
fb0c8c400 wenxu              2019-02-21  728  			rt = dst_cache_get_ip4(&tun_info->dst_cache, &fl4.saddr);

What about if "use_cache" is false?  "rt" is potentially not set.

fb0c8c400 wenxu              2019-02-21  729  	} else {
e09acddf8 Paolo Abeni        2016-02-12  730  		rt = connected ? dst_cache_get_ip4(&tunnel->dst_cache, &fl4.saddr) :
e09acddf8 Paolo Abeni        2016-02-12  731  			 NULL;
fb0c8c400 wenxu              2019-02-21  732  	}
7d442fab0 Tom Herbert        2014-01-02  733  
7d442fab0 Tom Herbert        2014-01-02 @734  	if (!rt) {
                                                    ^^^

7d442fab0 Tom Herbert        2014-01-02  735  		rt = ip_route_output_key(tunnel->net, &fl4);
c54419321 Pravin B Shelar    2013-03-25  736  
c54419321 Pravin B Shelar    2013-03-25  737  		if (IS_ERR(rt)) {
c54419321 Pravin B Shelar    2013-03-25  738  			dev->stats.tx_carrier_errors++;
c54419321 Pravin B Shelar    2013-03-25  739  			goto tx_error;
c54419321 Pravin B Shelar    2013-03-25  740  		}
fb0c8c400 wenxu              2019-02-21  741  		if (use_cache)

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-02-25  6:27 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-21 12:08 [PATCH] ip_tunnel: Add ip tunnel tunnel_info dst_cache in ip_tunnel_xmit wenxu
2019-02-21 18:37 ` David Miller
2019-02-23 23:28 ` kbuild test robot
2019-02-25  6:27 ` Dan Carpenter

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.