On Tue, Dec 31, 2019 at 10:33 AM Martin Varghese
[off-list ref] wrote:

On Fri, Nov 29, 2019 at 01:18:36PM -0500, Willem de Bruijn wrote:

quoted

On Thu, Nov 28, 2019 at 11:25 AM Martin Varghese
[off-list ref] wrote:

quoted

On Mon, Nov 18, 2019 at 12:23:09PM -0500, Willem de Bruijn wrote:

quoted

On Sat, Nov 16, 2019 at 12:45 AM 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

quoted

quoted

+static int bareudp_fill_metadata_dst(struct net_device *dev,
+                                    struct sk_buff *skb)
+{
+       struct ip_tunnel_info *info = skb_tunnel_info(skb);
+       struct bareudp_dev *bareudp = netdev_priv(dev);
+       bool use_cache = ip_tunnel_dst_cache_usable(skb, info);
+
+       if (ip_tunnel_info_af(info) == AF_INET) {
+               struct rtable *rt;
+               struct flowi4 fl4;
+
+               rt = iptunnel_get_v4_rt(skb, dev, bareudp->net, &fl4, info,
+                                       use_cache);
+               if (IS_ERR(rt))
+                       return PTR_ERR(rt);
+
+               ip_rt_put(rt);
+               info->key.u.ipv4.src = fl4.saddr;
+#if IS_ENABLED(CONFIG_IPV6)
+       } else if (ip_tunnel_info_af(info) == AF_INET6) {
+               struct dst_entry *dst;
+               struct flowi6 fl6;
+               struct bareudp_sock *bs6 = rcu_dereference(bareudp->sock);
+
+               dst = ip6tunnel_get_dst(skb, dev, bareudp->net, bs6->sock, &fl6,
+                                       info, use_cache);
+               if (IS_ERR(dst))
+                       return PTR_ERR(dst);
+
+               dst_release(dst);
+               info->key.u.ipv6.src = fl6.saddr;
+#endif
+       } else {
+               return -EINVAL;
+       }
+
+       info->key.tp_src = udp_flow_src_port(bareudp->net, skb,
+                                            bareudp->sport_min,
+                                            USHRT_MAX, true);
+       info->key.tp_dst = bareudp->conf.port;
+       return 0;
+}

This can probably all be deduplicated with geneve_fill_metadata_dst
once both use iptunnel_get_v4_rt.

Do you have any preference of file to keep the common function

Perhaps net/ipv4/udp_tunnel.c

I was trying this change and i found i dont have a lot of generic code here.

Populating L4 ports is function of the protocol implementation
and it is differnt for geneve and for bareudp

If geneve can use iptunnel_get_v4_rt, then it seems like the only
implementation specific logic is deriving net and sk. Would it help to
just pass those explicitly to the shared helper
(udptunnel_fill_metadata_dst)?

It is hard to say without seeing the geneve conversion to
iptunnel_get_v4_rt. Maybe that throws up a constraint I'm missing.
Lacking that, I'll trust your judgment.

The one thing we could do is to write a generic API to derive the
tunnel src address.

But since it is a very small piece i dont see much value add by making it
generic and i prefer to keep the way as it is now

In that case, that sounds sensible.

But i am open for both the option. What you think ?