On Wed, Sep 30, 2015 at 11:34 AM, John W. Linville
[off-list ref] wrote:

I wasn't sure why we needed this flag. Can't we just look at the
remote address family?

-static void geneve_sock_release(struct geneve_sock *gs)
+static void __geneve_sock_release(struct geneve_sock *gs)
 {
        if (--gs->refcnt)
                return;

Do we need a check for NULL first here?

+#if IS_ENABLED(CONFIG_IPV6)
+static int geneve6_build_skb(struct dst_entry *dst, struct sk_buff *skb,
+                            __be16 tun_flags, u8 vni[3], u8 opt_len, u8 *opt,
+                            bool csum, bool xnet)
+{
+       struct genevehdr *gnvh;
+       int min_headroom;
+       int err;
+
+       skb_scrub_packet(skb, xnet);

Is there a reason why this applies to only IPv6? It seems like it
would be common

+static struct dst_entry *geneve_get_dst(struct sk_buff *skb,

It might be worth clarifying this name - it wasn't immediately obvious
to me the difference between geneve_get_rt() and geneve_get_dst() is
IPv4 vs. IPv6.

+#if IS_ENABLED(CONFIG_IPV6)
+static netdev_tx_t geneve6_xmit_skb(struct sk_buff *skb, struct net_device *dev,
+                                   struct ip_tunnel_info *info)

[...]

+       dst = geneve_get_dst(skb, dev, &fl6, info);
+       if (IS_ERR(dst)) {
+               netdev_dbg(dev, "no route to %pI6\n", &fl6.daddr);
+               dev->stats.tx_carrier_errors++;
+               goto tx_error;
+       }

It looks like we double log/count this error (although this also
appears to be a problem for IPv4).

+       err = udp_tunnel6_xmit_skb(dst, gs6->sock->sk, skb, dev,
+                                  &fl6.saddr, &fl6.daddr, 0, ttl,
+                                  sport, geneve->dst_port, !udp_csum);

It seems like TOS is not handled here?

I think this will fail in the non-compat metadata case. The remote
that is passed in will be a zeroed copy on the stack, so the address
won't match the static version. I believe the check should be for
AF_UNSPEC instead.