VXLAN driver

drivers/net/Kconfig +3 -0

@@ -149,6 +149,19 @@ config MACVTAP
 	  To compile this driver as a module, choose M here: the module
 	  will be called macvtap.
 
+config VXLAN
+       tristate "Virtual eXtensible Local Area Network (VXLAN)"
+       depends on EXPERIMENTAL

drivers/net/vxlan.c new file +5 -0

+	struct vxlanhdr *vxh;
+	struct vxlan_dev *vxlan;
+	struct vxlan_stats *stats;
+	__u32 vni;
+	__be32 saddr = oip->saddr;	/* source address for learning */

drivers/net/vxlan.c new file +41 -0

+	/* Drop packets with reserved bits set */
+	vxh = (struct vxlanhdr *) skb->data;
+	if (vxh->vx_flags != htonl(VXLAN_FLAGS) ||
+	    (vxh->vx_vni & htonl(0xff)))
+		goto error;
+
+	__skb_pull(skb, sizeof(struct vxlanhdr));
+
+	/* Is this VNI defined? */
+	vni = ntohl(vxh->vx_vni) >> 8;
+	vxlan = vxlan_find_vni(sock_net(sk), vni);
+	if (!vxlan)
+		goto drop;
+
+	/* Ignore packets if device is not up */
+	if (!netif_running(vxlan->dev))
+		goto drop;
+
+	/* Re-examine inner Ethernet packet */
+	skb->protocol = eth_type_trans(skb, vxlan->dev);
+	skb->ip_summed = CHECKSUM_NONE;
+
+	/* Ignore packet loops (and multicast echo) */
+	if (compare_ether_addr(eth_hdr(skb)->h_source,
+			       vxlan->dev->dev_addr) == 0)
+		goto drop;
+
+	/* Check for multicast group configuration errors */
+	if (IN_MULTICAST(ntohl(daddr)) &&
+	    daddr != vxlan->gaddr) {
+		if (net_ratelimit())
+			netdev_notice(vxlan->dev,
+				      "group address %pI4 does not match\n",
+				      &daddr);
+		goto drop;
+	}
+
+	if (vxlan->learn)
+		vxlan_snoop(skb->dev, saddr, eth_hdr(skb)->h_source);
+
+	stats = this_cpu_ptr(vxlan->stats);

drivers/net/vxlan.c new file +27 -0

+
+/* Transmit local packets over Vxlan
+ *
+ * Outer IP header inherits ECN and DF from inner header.
+ * Outer UDP destination is the VXLAN assigned port.
+ *           source port is based on hash of flow if available
+ *                       otherwise use a random value
+ */
+static netdev_tx_t vxlan_xmit(struct sk_buff *skb, struct net_device *dev)
+{
+	struct vxlan_dev *vxlan = netdev_priv(dev);
+	struct rtable *rt;
+	const struct ethhdr *eth;
+	const struct iphdr *old_iph;
+	struct iphdr *iph;
+	struct vxlanhdr *vxh;
+	struct udphdr *uh;
+	struct flowi4 fl4;
+	struct vxlan_fdb *f;
+	unsigned int pkt_len = skb->len;
+	unsigned int mtu;
+	u32 hash;
+	__be32 dst;
+	__be16 df = 0;
+	__u8 tos, ttl;
+	int err;
+

drivers/net/vxlan.c new file +17 -0

+		u64_stats_update_end(&stats->syncp);
+	} else {
+		dev->stats.tx_errors++;
+		dev->stats.tx_aborted_errors++;
+	}
+	return NETDEV_TX_OK;
+
+drop:
+	dev->stats.tx_dropped++;
+	goto tx_free;
+
+tx_error:
+	dev->stats.tx_errors++;
+tx_free:
+	dev_kfree_skb(skb);
+	return NETDEV_TX_OK;
+}

[...]

Quoted 27 lines · Stephen Hemminger

drivers/net/vxlan.c new file +27 -0

+
+/* Transmit local packets over Vxlan
+ *
+ * Outer IP header inherits ECN and DF from inner header.
+ * Outer UDP destination is the VXLAN assigned port.
+ *           source port is based on hash of flow if available
+ *                       otherwise use a random value
+ */
+static netdev_tx_t vxlan_xmit(struct sk_buff *skb, struct net_device *dev)
+{
+	struct vxlan_dev *vxlan = netdev_priv(dev);
+	struct rtable *rt;
+	const struct ethhdr *eth;
+	const struct iphdr *old_iph;
+	struct iphdr *iph;
+	struct vxlanhdr *vxh;
+	struct udphdr *uh;
+	struct flowi4 fl4;
+	struct vxlan_fdb *f;
+	unsigned int pkt_len = skb->len;
+	unsigned int mtu;
+	u32 hash;
+	__be32 dst;
+	__be16 df = 0;
+	__u8 tos, ttl;
+	int err;
+

[...]

drivers/net/vxlan.c line 721

Quoted 7 lines

drivers/net/vxlan.c new file +7 -0

+	err = ip_local_out(skb);
+	if (likely(net_xmit_eval(err) == 0)) {
+		struct vxlan_stats *stats = this_cpu_ptr(vxlan->stats);
+
+		u64_stats_update_begin(&stats->syncp);
+		stats->tx_packets++;
+		stats->tx_bytes += pkt_len;

Should pkt_len include the outer headers?

Quoted 4 lines

[...]

Quoted 9 lines

drivers/net/vxlan.c line 721

Quoted 7 lines

drivers/net/vxlan.c new file +7 -0

+	err = ip_local_out(skb);
+	if (likely(net_xmit_eval(err) == 0)) {
+		struct vxlan_stats *stats = this_cpu_ptr(vxlan->stats);
+
+		u64_stats_update_begin(&stats->syncp);
+		stats->tx_packets++;
+		stats->tx_bytes += pkt_len;

Should pkt_len include the outer headers?

It doesn't for GRE and related tunnels.

drivers/net/vxlan.c new file +7 -0

+/* Initialize the device structure. */
+static void vxlan_setup(struct net_device *dev)
+{
+	struct vxlan_dev *vxlan = netdev_priv(dev);
+	unsigned h;
+
+	eth_hw_addr_random(dev);

Quoted 11 lines · Stephen Hemminger

drivers/net/vxlan.c new file +11 -0

+/* Initialize the device structure. */
+static void vxlan_setup(struct net_device *dev)
+{
+	struct vxlan_dev *vxlan = netdev_priv(dev);
+	unsigned h;
+
+	eth_hw_addr_random(dev);
+	ether_setup(dev);
+
+	dev->netdev_ops = &vxlan_netdev_ops;
+	dev->destructor = vxlan_free;

drivers/net/vxlan.c line 921

Quoted 6 lines

drivers/net/vxlan.c new file +6 -0

+	SET_NETDEV_DEVTYPE(dev, &vxlan_type);
+
+	dev->tx_queue_len = 0;
+	dev->features	|= NETIF_F_LLTX;
+	dev->features	|= NETIF_F_NETNS_LOCAL;
+	dev->priv_flags	&= ~IFF_XMIT_DST_RELEASE;

I am not sure why you remove IFF_XMIT_DST_RELEASE here

Quoted 2 lines · On Mon, 24 Sep 2012 22:09:50 +0200 Eric Dumazet <[email protected]> wrote:

I am not sure why you remove IFF_XMIT_DST_RELEASE here

Copied from GRE. What is impact either way?

This is an implementation of Virtual eXtensible Local Area Network as described in draft RFC:
http://tools.ietf.org/html/draft-mahalingam-dutt-dcops-vxlan-02

The driver integrates a Virtual Tunnel Endpoint (VTEP) functionality that learns MAC to IP address mapping.

This implementation has not been tested for Interoperation with other equipment.

drivers/net/Kconfig +2 -0

@@ -149,6 +149,19 @@ config MACVTAP
 	  To compile this driver as a module, choose M here: the module
 	  will be called macvtap.
 
+config VXLAN
+       tristate "Virtual eXtensible Local Area Network (VXLAN)"

drivers/net/Kconfig +9 -0

+/* Add static entry (via netlink) */
+static int vxlan_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
+			 struct net_device *dev,
+			 const unsigned char *addr, u16 flags)
+{
+	struct vxlan_dev *vxlan = netdev_priv(dev);
+	__be32 ip;
+	int err;
+

drivers/net/Kconfig +10 -0

+/* Propogate ECN from outer IP header to tunneled packet */
+static inline void vxlan_ecn_decap(const struct iphdr *iph, struct sk_buff *skb)
+{
+	if (INET_ECN_is_ce(iph->tos)) {
+		if (skb->protocol == htons(ETH_P_IP))
+			IP_ECN_set_ce(ip_hdr(skb));
+		else if (skb->protocol == htons(ETH_P_IPV6))
+			IP6_ECN_set_ce(ipv6_hdr(skb));
+	}
+}

drivers/net/Kconfig +2 -0

+	udp_sk(sk)->encap_rcv = vxlan_udp_encap_recv;
+	udp_encap_enable();

Quoted 9 lines · * Stephen Hemminger ([email protected]) wrote:

This is an implementation of Virtual eXtensible Local Area Network as described in draft RFC:
http://tools.ietf.org/html/draft-mahalingam-dutt-dcops-vxlan-02

The driver integrates a Virtual Tunnel Endpoint (VTEP) functionality that learns MAC to IP address mapping.

This implementation has not been tested for Interoperation with other equipment.

I'm working on doing some interop

Quoted 8 lines

drivers/net/Kconfig +2 -0

@@ -149,6 +149,19 @@ config MACVTAP
 	  To compile this driver as a module, choose M here: the module
 	  will be called macvtap.
 
+config VXLAN
+       tristate "Virtual eXtensible Local Area Network (VXLAN)"

drivers/net/Kconfig config MACVTAP · line 163

Quoted 10 lines

drivers/net/Kconfig +10 -0

+       depends on EXPERIMENTAL
+       ---help---
+	  This allows one to create vxlan virtual interfaces that provide
+	  Layer 2 Networks over Layer 3 Networks. VXLAN is often used
+	  to tunnel virtual network infrastructure in virtualized environments.
+	  For more information see:
+	    http://tools.ietf.org/html/draft-mahalingam-dutt-dcops-vxlan-02
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called macvlan.

                         ^^^^^^^

Cut 'n paste error, s/macvlan/vxlan/

Quoted 13 lines

drivers/net/Kconfig +13 -0

+/* Add static entry (via netlink) */
+static int vxlan_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
+			 struct net_device *dev,
+			 const unsigned char *addr, u16 flags)
+{
+	struct vxlan_dev *vxlan = netdev_priv(dev);
+	__be32 ip;
+	int err;
+
+	if (tb[NDA_DST] == NULL)
+		return -EINVAL;
+
+	if (nla_len(tb[NDA_DST]) != sizeof(__be32))

drivers/net/Kconfig

Quoted 6 lines

drivers/net/Kconfig +6 -0

+		return -EAFNOSUPPORT;
+
+	ip = nla_get_be32(tb[NDA_DST]);
+
+	spin_lock_bh(&vxlan->hash_lock);
+	err = vxlan_fdb_create(vxlan, addr, ip, VXLAN_FDB_PERM);

Any reason to force permanent when created from userspace?

drivers/net/Kconfig

Quoted 10 lines

drivers/net/Kconfig +9 -0

+static bool vxlan_group_used(struct vxlan_net *vn,
+			     const struct vxlan_dev *this)
+{
+	const struct vxlan_dev *vxlan;
+	struct hlist_node *node;
+	unsigned h;
+
+	for (h = 0; h < VNI_HASH_SIZE; ++h)
+		hlist_for_each_entry(vxlan, node, &vn->vni_list[h], hlist) {

is walking this chain only protected with rtnl?

Quoted 11 lines

drivers/net/Kconfig +10 -0

+/* Propogate ECN from outer IP header to tunneled packet */
+static inline void vxlan_ecn_decap(const struct iphdr *iph, struct sk_buff *skb)
+{
+	if (INET_ECN_is_ce(iph->tos)) {
+		if (skb->protocol == htons(ETH_P_IP))
+			IP_ECN_set_ce(ip_hdr(skb));
+		else if (skb->protocol == htons(ETH_P_IPV6))
+			IP6_ECN_set_ce(ipv6_hdr(skb));
+	}
+}

<snip>

drivers/net/Kconfig

Quoted 9 lines

drivers/net/Kconfig +9 -0

+/* Propogate ECN bits out */
+static inline u8 vxlan_ecn_encap(u8 tos,
+				 const struct iphdr *iph,
+				 const struct sk_buff *skb)
+{
+	u8 inner = vxlan_get_dsfield(iph, skb);
+
+	return INET_ECN_encapsulate(tos, inner);
+}

Goal is to be RFC 6040 compliant, and it looks like some edge cases aren't met, for example, should drop on decap when inner is not supporting ECN and outer has set CE.

<snip>

drivers/net/Kconfig

Quoted 3 lines

drivers/net/Kconfig +3 -0

+/* Callback from net/ipv4/udp.c to receive packets */
+	/* Mark socket as an encapsulation socket. */
+	udp_sk(sk)->encap_type = UDP_ENCAP_L2TPINUDP;

I don't think we need this particular encap_type value, just != 0

Quoted 3 lines

drivers/net/Kconfig +2 -0

+	udp_sk(sk)->encap_rcv = vxlan_udp_encap_recv;
+	udp_encap_enable();

Quoted 5 lines · Chris Wright

Quoted 5 lines

drivers/net/vxlan.c new file +5 -0

+/* Propogate ECN bits out */
+static inline u8 vxlan_ecn_encap(u8 tos,
+				 const struct iphdr *iph,
+				 const struct sk_buff *skb)
+{

Quoted 8 lines

drivers/net/vxlan.c line 589

Quoted 4 lines

drivers/net/vxlan.c new file +4 -0

+	u8 inner = vxlan_get_dsfield(iph, skb);
+
+	return INET_ECN_encapsulate(tos, inner);
+}

Goal is to be RFC 6040 compliant, and it looks like some edge cases aren't met, for example, should drop on decap when inner is not supporting ECN and outer has set CE.

The code was taken from existing GRE in Linux. Looks like both VXLAN and GRE need to handle that.

Quoted 5 lines

drivers/net/vxlan.c line 1111

Quoted 3 lines

drivers/net/vxlan.c new file +3 -0

+/* Callback from net/ipv4/udp.c to receive packets */
+	/* Mark socket as an encapsulation socket. */
+	udp_sk(sk)->encap_type = UDP_ENCAP_L2TPINUDP;

I don't think we need this particular encap_type value, just != 0

Is there any value in defining new value?

Linux GRE was likely written before this RFC and therefore does not conform to one of the rules in Section 4.2. Default Tunnel Egress Behaviour.

The new code addresses:
o If the inner ECN field is Not-ECT, the decapsulator MUST NOT propagate any other ECN codepoint onwards. This is because the inner Not-ECT marking is set by transports that rely on dropped packets as an indication of congestion and would not understand or respond to any other ECN codepoint [RFC4774]. Specifically:

* If the inner ECN field is Not-ECT and the outer ECN field is CE, the decapsulator MUST drop the packet.

* If the inner ECN field is Not-ECT and the outer ECN field is Not-ECT, ECT(0), or ECT(1), the decapsulator MUST forward the outgoing packet with the ECN field cleared to Not-ECT.

This was caught by Chris Wright while reviewing VXLAN. This code has not been tested with real ECN through tunnel.

Signed-off-by Stephen Hemminger <[email protected]>

Quoted 13 lines · Stephen Hemminger

Linux GRE was likely written before this RFC and therefore does not conform to one of the rules in Section 4.2. Default Tunnel Egress Behaviour.

The new code addresses:
o If the inner ECN field is Not-ECT, the decapsulator MUST NOT propagate any other ECN codepoint onwards. This is because the inner Not-ECT marking is set by transports that rely on dropped packets as an indication of congestion and would not understand or respond to any other ECN codepoint [RFC4774]. Specifically:

* If the inner ECN field is Not-ECT and the outer ECN field is CE, the decapsulator MUST drop the packet.

Quoted 8 lines

* If the inner ECN field is Not-ECT and the outer ECN field is Not-ECT, ECT(0), or ECT(1), the decapsulator MUST forward the outgoing packet with the ECN field cleared to Not-ECT.

This was caught by Chris Wright while reviewing VXLAN. This code has not been tested with real ECN through tunnel.

Signed-off-by Stephen Hemminger <[email protected]>

It seems dangerous to me without any logging ?

One could argue that the outer ECN field should not be CE if the inner was Not-ECT

It means :
1) the encapsulator set ECT(0), or ECT(1) and a congestioned hop set CE
2) the encapsulator set CE

1) or 2) while inner was not-ECT (!!!)

If a router does such a thing, we should log a message to help diagnostics.

By the way other tunnels probably have the same issues.

Logging is a bad idea in this case since the tunnel might be from a remote host/protocol and the log would be filled with crap.

The tunnels in general do need to have rx_dropped counter, but it looks like that isn't being done right either.

net/ipv4/ip_gre.c +0 -0

net/ipv4/ip_gre.c +1 -0

@@ -703,17 +704,18 @@ static int ipgre_rcv(struct sk_buff *skb
 			skb_postpull_rcsum(skb, eth_hdr(skb), ETH_HLEN);
 		}
 
+		__skb_tunnel_rx(skb, tunnel->dev);

net/ipv4/ip_gre.c +0 -5

-		__skb_tunnel_rx(skb, tunnel->dev);
-
-		skb_reset_network_header(skb);
-		ipgre_ecn_decapsulate(iph, skb);
-
 		netif_rx(skb);
 
 		rcu_read_unlock();

On Mon, 2012-09-24 at 14:44 -0700, Stephen Hemminger wrote:
[...]

Quoted 2 lines

net/ipv4/ip_gre.c +0 -0

[...]

Quoted 5 lines

net/ipv4/ip_gre.c +1 -0

@@ -703,17 +704,18 @@ static int ipgre_rcv(struct sk_buff *skb
 			skb_postpull_rcsum(skb, eth_hdr(skb), ETH_HLEN);
 		}
 
+		__skb_tunnel_rx(skb, tunnel->dev);

net/ipv4/ip_gre.c static int ipgre_rcv(struct sk_buff *skb · line 717

Quoted 10 lines

net/ipv4/ip_gre.c +5 -0

+
+		skb_reset_network_header(skb);
+		if (!ipgre_ecn_decapsulate(iph, skb))
+			goto drop;
+
 		tstats = this_cpu_ptr(tunnel->dev->tstats);
 		u64_stats_update_begin(&tstats->syncp);
 		tstats->rx_packets++;
 		tstats->rx_bytes += skb->len;
 		u64_stats_update_end(&tstats->syncp);

I don't know why you're moving this code above the stats update; rx_packets/rx_bytes should include dropped packets.

Ben.

net/ipv4/ip_gre.c static int ipgre_rcv(struct sk_buff *skb · line 721

Quoted 8 lines

net/ipv4/ip_gre.c +0 -5

-		__skb_tunnel_rx(skb, tunnel->dev);
-
-		skb_reset_network_header(skb);
-		ipgre_ecn_decapsulate(iph, skb);
-
 		netif_rx(skb);
 
 		rcu_read_unlock();

Quoted 33 lines · On Mon, 1 Oct 2012 16:55:19 +0100 Ben Hutchings <[email protected]>…

On Mon, 2012-09-24 at 14:44 -0700, Stephen Hemminger wrote:
[...]

Quoted 2 lines

net/ipv4/ip_gre.c +0 -0

[...]

Quoted 5 lines

net/ipv4/ip_gre.c +1 -0

@@ -703,17 +704,18 @@ static int ipgre_rcv(struct sk_buff *skb
 			skb_postpull_rcsum(skb, eth_hdr(skb), ETH_HLEN);
 		}
 
+		__skb_tunnel_rx(skb, tunnel->dev);

net/ipv4/ip_gre.c static int ipgre_rcv(struct sk_buff *skb · line 717

Quoted 10 lines

net/ipv4/ip_gre.c +5 -0

+
+		skb_reset_network_header(skb);
+		if (!ipgre_ecn_decapsulate(iph, skb))
+			goto drop;
+
 		tstats = this_cpu_ptr(tunnel->dev->tstats);
 		u64_stats_update_begin(&tstats->syncp);
 		tstats->rx_packets++;
 		tstats->rx_bytes += skb->len;
 		u64_stats_update_end(&tstats->syncp);

I don't know why you're moving this code above the stats update; rx_packets/rx_bytes should include dropped packets.

Ben.

Quoted 7 lines

net/ipv4/ip_gre.c +0 -5

-		__skb_tunnel_rx(skb, tunnel->dev);
-
-		skb_reset_network_header(skb);
-		ipgre_ecn_decapsulate(iph, skb);
-
 		netif_rx(skb);
 

Quoted 2 lines

net/ipv4/ip_gre.c static int ipgre_rcv(struct sk_buff *skb · line 720

Quoted 1 line

net/ipv4/ip_gre.c +0 -0

 		rcu_read_unlock();

Is that true? I thought they were accounted for by rx_dropped.

I don't think rx_dropped is appropriate for counting invalid packets, but maybe actual practice is already different.

As for whether packets counted in rx_dropped should also be counted in rx_packets/rx_bytes, I really don't know. The current comments on rtnl_link_stats (inherited from net_device_stats) are totally inadequate as a specification.

Quoted 7 lines · Ben Hutchings

I don't think rx_dropped is appropriate for counting invalid packets, but maybe actual practice is already different.

As for whether packets counted in rx_dropped should also be counted in rx_packets/rx_bytes, I really don't know. The current comments on rtnl_link_stats (inherited from net_device_stats) are totally inadequate as a specification.

rx_dropped is used by core network stack, not the devices themselves.

So a packet is first accounted in rx_bytes/rx_packets by the driver, and if net/core/dev.c drops it, rx_dropped is incremented as well.

+static netdev_tx_t vxlan_xmit(struct sk_buff *skb, struct net_device *dev)

drivers/net/vxlan.c new file +9 -0

+       /* Do PMTU */
+       if (skb->protocol == htons(ETH_P_IP)) {
+               df |= old_iph->frag_off & htons(IP_DF);
+               if (df && mtu < pkt_len) {
+                       icmp_send(skb, ICMP_DEST_UNREACH, ICMP_FRAG_NEEDED,
+                                 htonl(mtu));
+                       ip_rt_put(rt);
+                       goto tx_error;
+               }

Quoted 1 line · Stephen Hemminger

+static netdev_tx_t vxlan_xmit(struct sk_buff *skb, struct net_device *dev)

[...]

Quoted 16 lines

drivers/net/vxlan.c new file +16 -0

+       /* Do PMTU */
+       if (skb->protocol == htons(ETH_P_IP)) {
+               df |= old_iph->frag_off & htons(IP_DF);
+               if (df && mtu < pkt_len) {
+                       icmp_send(skb, ICMP_DEST_UNREACH, ICMP_FRAG_NEEDED,
+                                 htonl(mtu));
+                       ip_rt_put(rt);
+                       goto tx_error;
+               }
+       }
+#if IS_ENABLED(CONFIG_IPV6)
+       else if (skb->protocol == htons(ETH_P_IPV6)) {
+               if (mtu >= IPV6_MIN_MTU && mtu < pkt_len) {
+                       icmpv6_send(skb, ICMPV6_PKT_TOOBIG, 0, mtu);
+                       ip_rt_put(rt);
+                       goto tx_error;

drivers/net/vxlan.c line 687

Quoted 3 lines

drivers/net/vxlan.c new file +3 -0

+               }
+       }
+#endif

Won't this black hole packets if we need to generate ICMP messages? Since we're doing switching and not routing here icmp_send() doesn't necessarily have a route to the relevant endpoint. It looks like Ethernet over GRE has this issue as well.

drivers/net/vxlan.c new file +6 -0

+/* Hash Ethernet address */
+static u32 eth_hash(const unsigned char *addr)
+{
+	/* could be optimized for unaligned access */
+	u32 a = addr[5] << 8 | addr[4];
+	u32 b = addr[3] << 24 | addr[2] << 16 | addr[1] << 8 | addr[0];

From: Stephen Hemminger <[email protected]>
Date: Tue, 25 Sep 2012 15:09:57 -0700

Quoted 6 lines

drivers/net/vxlan.c new file +6 -0

+/* Hash Ethernet address */
+static u32 eth_hash(const unsigned char *addr)
+{
+	/* could be optimized for unaligned access */
+	u32 a = addr[5] << 8 | addr[4];
+	u32 b = addr[3] << 24 | addr[2] << 16 | addr[1] << 8 | addr[0];

drivers/net/vxlan.c line 235

Quoted 10 lines

drivers/net/vxlan.c new file +10 -0

+
+	return jhash_2words(a, b, vxlan_salt);
+}
+
+/* Hash chain to use given mac address */
+static inline struct hlist_head *vxlan_fdb_head(struct vxlan_dev *vxlan,
+						const u8 *mac)
+{
+	return &vxlan->fdb_head[hash_32(eth_hash(mac), FDB_HASH_BITS)];
+}

This hashing is way overkill, and in fact is a mis-use of Jenkins.

Jenkins can work fine with power-of-two hash sizes, so using hash_32() on the Jenkins hash result is nothing short of silly.

I would go one step further and change this to:

(a ^ b) * vxlan_salt

much like how we hash IP addresses in the ipv4 ARP tables. That function is a universal hash, and therefore provides whatever kind of protection you're trying to obtain using the salt.

But I wonder if this matters at all, the administrator controls the contents of this table, rather than external entitites.

Quoted 2 lines · On Thu, 27 Sep 2012 18:47:40 -0400 (EDT) David Miller <[email protected]>…

But I wonder if this matters at all, the administrator controls the contents of this table, rather than external entitites.

The table includes values learned from packets received. Like a bridge, a malicious attacker who can forge MAC sourc addresses can overload one chain by swamping the table with bogus values. Probably needs a table limit.

This is an implementation of Virtual eXtensible Local Area Network as described in draft RFC:
http://tools.ietf.org/html/draft-mahalingam-dutt-dcops-vxlan-02

The driver integrates a Virtual Tunnel Endpoint (VTEP) functionality that learns MAC to IP address mapping.

This implementation has only been tested with the user-mode TAP based version for Linux, not against other vendors (yet).

Signed-off-by Stephen Hemminger <[email protected]>

From: Stephen Hemminger <[email protected]>
Date: Mon, 1 Oct 2012 13:57:19 -0700

Quoted 11 lines

This is an implementation of Virtual eXtensible Local Area Network as described in draft RFC:
http://tools.ietf.org/html/draft-mahalingam-dutt-dcops-vxlan-02

The driver integrates a Virtual Tunnel Endpoint (VTEP) functionality that learns MAC to IP address mapping.

This implementation has only been tested with the user-mode TAP based version for Linux, not against other vendors (yet).

Signed-off-by Stephen Hemminger <[email protected]>

It doesn't build.

And I'm not telling you what the build error is, you'll have to do an allmodconfig build yourself to see it.

I want you to get into the habit of doing an allmodconfig build to validate your changes because that's the very first thing I'm going to do.

From: Stephen Hemminger <[email protected]>
Date: Mon, 1 Oct 2012 13:57:19 -0700

Quoted 11 lines

This is an implementation of Virtual eXtensible Local Area Network as described in draft RFC:
http://tools.ietf.org/html/draft-mahalingam-dutt-dcops-vxlan-02

The driver integrates a Virtual Tunnel Endpoint (VTEP) functionality that learns MAC to IP address mapping.

This implementation has only been tested with the user-mode TAP based version for Linux, not against other vendors (yet).

Signed-off-by Stephen Hemminger <[email protected]>

It doesn't build.

And I'm not telling you what the build error is, you'll have to do an allmodconfig build yourself to see it.

I want you to get into the habit of doing an allmodconfig build to validate your changes because that's the very first thing I'm going to do.

Quoted 15 lines · On Mon, 01 Oct 2012 18:07:12 -0400 (EDT) David Miller <[email protected]>…

From: Stephen Hemminger <[email protected]>
Date: Mon, 1 Oct 2012 13:57:19 -0700

Quoted 11 lines

This is an implementation of Virtual eXtensible Local Area Network as described in draft RFC:
http://tools.ietf.org/html/draft-mahalingam-dutt-dcops-vxlan-02

The driver integrates a Virtual Tunnel Endpoint (VTEP) functionality that learns MAC to IP address mapping.

This implementation has only been tested with the user-mode TAP based version for Linux, not against other vendors (yet).

Signed-off-by Stephen Hemminger <[email protected]>

Quoted 8 lines

It doesn't build.

And I'm not telling you what the build error is, you'll have to do an allmodconfig build yourself to see it.

I want you to get into the habit of doing an allmodconfig build to validate your changes because that's the very first thing I'm going to do.

Dave did you remember to include the two pre-cursor patches.

Vxlan was originally submitted as a 3 part series and only the last one ever changed.

[PATCH net-next 1/3] netlink: add attributes to fdb interface [PATCH net-next 2/3] igmp: export symbol ip_mc_leave_group

Make allmodconfig works for me (x86-64).

Quoted 1 line · Stephen Hemminger

+static netdev_tx_t vxlan_xmit(struct sk_buff *skb, struct net_device *dev)

[...]

Quoted 16 lines

drivers/net/vxlan.c new file +16 -0

+       /* Do PMTU */
+       if (skb->protocol == htons(ETH_P_IP)) {
+               df |= old_iph->frag_off & htons(IP_DF);
+               if (df && mtu < pkt_len) {
+                       icmp_send(skb, ICMP_DEST_UNREACH, ICMP_FRAG_NEEDED,
+                                 htonl(mtu));
+                       ip_rt_put(rt);
+                       goto tx_error;
+               }
+       }
+#if IS_ENABLED(CONFIG_IPV6)
+       else if (skb->protocol == htons(ETH_P_IPV6)) {
+               if (mtu >= IPV6_MIN_MTU && mtu < pkt_len) {
+                       icmpv6_send(skb, ICMPV6_PKT_TOOBIG, 0, mtu);
+                       ip_rt_put(rt);
+                       goto tx_error;

drivers/net/vxlan.c line 687

Quoted 3 lines

drivers/net/vxlan.c new file +3 -0

+               }
+       }
+#endif

Won't this black hole packets if we need to generate ICMP messages? Since we're doing switching and not routing here icmp_send() doesn't necessarily have a route to the relevant endpoint. It looks like Ethernet over GRE has this issue as well.

Quoted 20 lines · On Tue, 25 Sep 2012 14:55:13 -0700 Jesse Gross <[email protected]> wrote:

Quoted 1 line · Stephen Hemminger

+static netdev_tx_t vxlan_xmit(struct sk_buff *skb, struct net_device *dev)

[...]

Quoted 16 lines

drivers/net/vxlan.c new file +16 -0

+       /* Do PMTU */
+       if (skb->protocol == htons(ETH_P_IP)) {
+               df |= old_iph->frag_off & htons(IP_DF);
+               if (df && mtu < pkt_len) {
+                       icmp_send(skb, ICMP_DEST_UNREACH, ICMP_FRAG_NEEDED,
+                                 htonl(mtu));
+                       ip_rt_put(rt);
+                       goto tx_error;
+               }
+       }
+#if IS_ENABLED(CONFIG_IPV6)
+       else if (skb->protocol == htons(ETH_P_IPV6)) {
+               if (mtu >= IPV6_MIN_MTU && mtu < pkt_len) {
+                       icmpv6_send(skb, ICMPV6_PKT_TOOBIG, 0, mtu);
+                       ip_rt_put(rt);
+                       goto tx_error;

Quoted 8 lines

drivers/net/vxlan.c line 687

Quoted 3 lines

drivers/net/vxlan.c new file +3 -0

+               }
+       }
+#endif

Won't this black hole packets if we need to generate ICMP messages? Since we're doing switching and not routing here icmp_send() doesn't necessarily have a route to the relevant endpoint. It looks like Ethernet over GRE has this issue as well.

It is an interesting question about what is the correct way to handle packets where the inner header is IPv6 or IPv4 with Don't Fragment set. As you mention sending an ICMP response won't work because the tunnel endpoint is not part of that IP network.

The simple option is to fragment it in the tunnel and since the fragmentation is not visible to the overlay network, that is okay. But for PMTU discovery it might be better to just drop the packet and not send a fragmented payload.

Some backbone networks don't allow fragmentation at all (in a futile attempt to block DoS attacks and protect fragile Windows hosts). Fragmentation brings all sorts of evil problems like the potential of corrupted assembly because of sequence wrap; the checksum in the inner packet will defend against that but tunnels are not supposed to rely on inner protocol data protection.

Or you can just do what Cisco and Microsoft do and just tell everyone to set larger MTU on the backbone.