tunneling: Rely on protocol handles to parse ToS.

Currently the generic tunneling code assumes that on receive the
outer IP header is present on the packet and the protocol pointers
are initialized to it.  This knowledge is used in only one place,
which is to copy ECN bits from outer to inner IP packets.  These
assumptions are difficult to keep track of because the receive code
generally uses the protocol pointers for the inner packet.  A
number of bugs have resulted from incorrect assumptions, including
one fixed here about the location of a vlan header.  This drops the
convention and instead uses the protocol handles to supply the ToS,
which is less error-prone.

Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>

diff --git a/datapath/tunnel.c b/datapath/tunnel.c
index 98af303..a7d4943 100644 (file)
--- a/datapath/tunnel.c
+++ b/datapath/tunnel.c
@@ -379,18 +379,14 @@ found:
        return tnl_vport_to_vport(tnl_vport_table_cast(tbl_node));
 }
 
-static inline void ecn_decapsulate(struct sk_buff *skb)
+static void ecn_decapsulate(struct sk_buff *skb, u8 tos)
 {
-       /* This is accessing the outer IP header of the tunnel, which we've
-        * already validated to be OK.  skb->data is currently set to the start
-        * of the inner Ethernet header, and we've validated ETH_HLEN.
-        */
-       if (unlikely(INET_ECN_is_ce(ip_hdr(skb)->tos))) {
+       if (unlikely(INET_ECN_is_ce(tos))) {
                __be16 protocol = skb->protocol;
 
                skb_set_network_header(skb, ETH_HLEN);
 
-               if (skb->protocol == htons(ETH_P_8021Q)) {
+               if (protocol == htons(ETH_P_8021Q)) {
                        if (unlikely(!pskb_may_pull(skb, VLAN_ETH_HLEN)))
                                return;
 
@@ -417,17 +413,27 @@ static inline void ecn_decapsulate(struct sk_buff *skb)
        }
 }
 
-/* Called with rcu_read_lock. */
-void tnl_rcv(struct vport *vport, struct sk_buff *skb)
+/**
+ *     tnl_rcv - ingress point for generic tunnel code
+ *
+ * @vport: port this packet was received on
+ * @skb: received packet
+ * @tos: ToS from encapsulating IP packet, used to copy ECN bits
+ *
+ * Must be called with rcu_read_lock.
+ *
+ * Packets received by this function are in the following state:
+ * - skb->data points to the inner Ethernet header.
+ * - The inner Ethernet header is in the linear data area.
+ * - skb->csum does not include the inner Ethernet header.
+ * - The layer pointers are undefined.
+ */
+void tnl_rcv(struct vport *vport, struct sk_buff *skb, u8 tos)
 {
-       /* Packets received by this function are in the following state:
-        * - skb->data points to the inner Ethernet header.
-        * - The inner Ethernet header is in the linear data area.
-        * - skb->csum does not include the inner Ethernet header.
-        * - The layer pointers point at the outer headers.
-        */
+       struct ethhdr *eh;
 
-       struct ethhdr *eh = (struct ethhdr *)skb->data;
+       skb_reset_mac_header(skb);
+       eh = eth_hdr(skb);
 
        if (likely(ntohs(eh->h_proto) >= 1536))
                skb->protocol = eh->h_proto;
@@ -439,7 +445,7 @@ void tnl_rcv(struct vport *vport, struct sk_buff *skb)
        skb_clear_rxhash(skb);
        secpath_reset(skb);
 
-       ecn_decapsulate(skb);
+       ecn_decapsulate(skb, tos);
        compute_ip_summed(skb, false);
        vlan_set_tci(skb, 0);
 

diff --git a/datapath/tunnel.h b/datapath/tunnel.h
index 6ce9c46..0fd6a69 100644 (file)
--- a/datapath/tunnel.h
+++ b/datapath/tunnel.h
@@ -225,7 +225,7 @@ int tnl_set_addr(struct vport *vport, const unsigned char *addr);
 const char *tnl_get_name(const struct vport *vport);
 const unsigned char *tnl_get_addr(const struct vport *vport);
 int tnl_send(struct vport *vport, struct sk_buff *skb);
-void tnl_rcv(struct vport *vport, struct sk_buff *skb);
+void tnl_rcv(struct vport *vport, struct sk_buff *skb, u8 tos);
 
 struct vport *tnl_find_port(__be32 saddr, __be32 daddr, __be64 key,
                            int tunnel_type,

diff --git a/datapath/vport-capwap.c b/datapath/vport-capwap.c
index 65f1f1b..e2cf400 100644 (file)
--- a/datapath/vport-capwap.c
+++ b/datapath/vport-capwap.c
@@ -206,7 +206,7 @@ static int capwap_rcv(struct sock *sk, struct sk_buff *skb)
                goto error;
        }
 
-       tnl_rcv(vport, skb);
+       tnl_rcv(vport, skb, iph->tos);
        goto out;
 
 error:

diff --git a/datapath/vport-gre.c b/datapath/vport-gre.c
index 0f45f8f..9c1c1cc 100644 (file)
--- a/datapath/vport-gre.c
+++ b/datapath/vport-gre.c
@@ -342,7 +342,7 @@ static int gre_rcv(struct sk_buff *skb)
        __skb_pull(skb, hdr_len);
        skb_postpull_rcsum(skb, skb_transport_header(skb), hdr_len + ETH_HLEN);
 
-       tnl_rcv(vport, skb);
+       tnl_rcv(vport, skb, iph->tos);
        return 0;
 
 error: