Thread (10 messages) 10 messages, 4 authors, 2020-01-27

Re: [PATCH net-next v5 1/2] net: UDP tunnel encapsulation module for tunnelling different protocols like MPLS,IP,NSH etc.

From: Martin Varghese <hidden>
Date: 2020-01-24 14:47:23

On Thu, Jan 23, 2020 at 05:42:25PM -0500, Willem de Bruijn wrote:
On Thu, Jan 23, 2020 at 1:04 PM Martin Varghese
[off-list ref] wrote:
quoted
From: Martin Varghese <redacted>

The Bareudp tunnel module provides a generic L3 encapsulation
tunnelling module for tunnelling different protocols like MPLS,
IP,NSH etc inside a UDP tunnel.

Signed-off-by: Martin Varghese <redacted>
quoted
diff --git a/include/net/ip6_tunnel.h b/include/net/ip6_tunnel.h
index 028eaea..8215d1b 100644
--- a/include/net/ip6_tunnel.h
+++ b/include/net/ip6_tunnel.h
@@ -165,5 +165,55 @@ static inline void ip6tunnel_xmit(struct sock *sk, struct sk_buff *skb,
                iptunnel_xmit_stats(dev, pkt_len);
        }
 }
+
+static inline struct dst_entry *ip6tunnel_get_dst(struct sk_buff *skb,
+                                                 struct net_device *dev,
+                                                 struct net *net,
+                                                 struct socket *sock,
+                                                 struct flowi6 *fl6,
+                                                 const struct ip_tunnel_info *info,
+                                                 bool use_cache)
+{
+       struct dst_entry *dst = NULL;
+#ifdef CONFIG_DST_CACHE
+       struct dst_cache *dst_cache;
+#endif
I just noticed these ifdefs are absent in Geneve. On closer look,
CONFIG_NET_UDP_TUNNEL selects CONFIG_NET_IP_TUNNEL selects
CONFIG_DST_CACHE. So they are indeed not needed.

Sorry, should have noticed that in v4. It could conceivably be fixed
up later, but seems worth one more round to get it right from the
start.
But unlike geneve i have placed this definition in ip_tunnels.h &
ip6_tunnels.h which doesnt come under NET_IP_TUNNEL.Hence build 
will fail in cases where NET_UDP_TUNNEL is disabled
Kbuild robot has shown that in v3.

Even with  #ifdef CONFIG_DST_CACHE Kbuild robot reported another issue.
when ip6_tunnel.h included in ip4_tunnel_core.c.
dst_cache_get_ipv6 comes under ipv6 flag  and hence the compilation of 
ip4_tunnel_core.c fails when IPV6 is disabled.

Ideally this functions should be defined in ip_tunnel.c & ip6_tunnel.c
as these function has no significance if IP Tunnel is disabled.

Glad you found the previous reviews helpful. I will also miss a lot.
For more assurance and also as regression test, it might be
worth looking into adding a bareudp mode to
tools/testing/selftests/net/pmtu.sh. That looks like it exercises a
variety of tunnel types already. Extending it might be little work
(once ip supports bareudp).

To be clear, not for this patch set. Let's not delay that further.
Just a thought.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help