vlan: Fix the ingress VLAN_FLAG_REORDER_HDR check

Testing of VLAN_FLAG_REORDER_HDR does not belong in vlan_untag
but rather in vlan_do_receive.  Otherwise the vlan header
will not be properly put on the packet in the case of
vlan header accelleration.

As we remove the check from vlan_check_reorder_header
rename it vlan_reorder_header to keep the naming clean.

Fix up the skb->pkt_type early so we don't look at the packet
after adding the vlan tag, which guarantees we don't goof
and look at the wrong field.

Use a simple if statement instead of a complicated switch
statement to decided that we need to increment rx_stats
for a multicast packet.

Hopefully at somepoint we will just declare the case where
VLAN_FLAG_REORDER_HDR is cleared as unsupported and remove
the code.  Until then this keeps it working correctly.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
Signed-off-by: Jiri Pirko <jpirko@redhat.com>
Acked-by: Changli Gao <xiaosuo@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

4 files changed, 61 insertions, 31 deletions

diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index dc01681fbb42..affa27380b72 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -225,7 +225,7 @@ static inline int vlan_hwaccel_receive_skb(struct sk_buff *skb,
 }
 
 /**
- * __vlan_put_tag - regular VLAN tag inserting
+ * vlan_insert_tag - regular VLAN tag inserting
  * @skb: skbuff to tag
  * @vlan_tci: VLAN TCI to insert
  *
@@ -234,8 +234,10 @@ static inline int vlan_hwaccel_receive_skb(struct sk_buff *skb,
  *
  * Following the skb_unshare() example, in case of error, the calling function
  * doesn't have to worry about freeing the original skb.
+ *
+ * Does not change skb->protocol so this function can be used during receive.
  */
-static inline struct sk_buff *__vlan_put_tag(struct sk_buff *skb, u16 vlan_tci)
+static inline struct sk_buff *vlan_insert_tag(struct sk_buff *skb, u16 vlan_tci)
 {
         struct vlan_ethhdr *veth;
 
@@ -255,8 +257,25 @@ static inline struct sk_buff *__vlan_put_tag(struct sk_buff *skb, u16 vlan_tci)
         /* now, the TCI */
         veth->h_vlan_TCI = htons(vlan_tci);
 
-        skb->protocol = htons(ETH_P_8021Q);
+        return skb;
+}
 
+/**
+ * __vlan_put_tag - regular VLAN tag inserting
+ * @skb: skbuff to tag
+ * @vlan_tci: VLAN TCI to insert
+ *
+ * Inserts the VLAN tag into @skb as part of the payload
+ * Returns a VLAN tagged skb. If a new skb is created, @skb is freed.
+ *
+ * Following the skb_unshare() example, in case of error, the calling function
+ * doesn't have to worry about freeing the original skb.
+ */
+static inline struct sk_buff *__vlan_put_tag(struct sk_buff *skb, u16 vlan_tci)
+{
+        skb = vlan_insert_tag(skb, vlan_tci);
+        if (skb)
+                skb->protocol = htons(ETH_P_8021Q);
         return skb;
 }
 
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index e8b78ce14474..c0a4f3ab0cc0 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -1256,6 +1256,11 @@ static inline void skb_reserve(struct sk_buff *skb, int len)
         skb->tail += len;
 }
 
+static inline void skb_reset_mac_len(struct sk_buff *skb)
+{
+        skb->mac_len = skb->network_header - skb->mac_header;
+}
+
 #ifdef NET_SKBUFF_DATA_USES_OFFSET
 static inline unsigned char *skb_transport_header(const struct sk_buff *skb)
 {
diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
index 41495dc2a4c9..fcc684678af6 100644
--- a/net/8021q/vlan_core.c
+++ b/net/8021q/vlan_core.c
@@ -23,6 +23,31 @@ bool vlan_do_receive(struct sk_buff **skbp)
                 return false;
 
         skb->dev = vlan_dev;
+        if (skb->pkt_type == PACKET_OTHERHOST) {
+                /* Our lower layer thinks this is not local, let's make sure.
+                 * This allows the VLAN to have a different MAC than the
+                 * underlying device, and still route correctly. */
+                if (!compare_ether_addr(eth_hdr(skb)->h_dest,
+                                        vlan_dev->dev_addr))
+                        skb->pkt_type = PACKET_HOST;
+        }
+
+        if (!(vlan_dev_info(vlan_dev)->flags & VLAN_FLAG_REORDER_HDR)) {
+                unsigned int offset = skb->data - skb_mac_header(skb);
+
+                /*
+                 * vlan_insert_tag expect skb->data pointing to mac header.
+                 * So change skb->data before calling it and change back to
+                 * original position later
+                 */
+                skb_push(skb, offset);
+                skb = *skbp = vlan_insert_tag(skb, skb->vlan_tci);
+                if (!skb)
+                        return false;
+                skb_pull(skb, offset + VLAN_HLEN);
+                skb_reset_mac_len(skb);
+        }
+
         skb->priority = vlan_get_ingress_priority(vlan_dev, skb->vlan_tci);
         skb->vlan_tci = 0;
 
@@ -31,22 +56,8 @@ bool vlan_do_receive(struct sk_buff **skbp)
         u64_stats_update_begin(&rx_stats->syncp);
         rx_stats->rx_packets++;
         rx_stats->rx_bytes += skb->len;
-
-        switch (skb->pkt_type) {
-        case PACKET_BROADCAST:
-                break;
-        case PACKET_MULTICAST:
+        if (skb->pkt_type == PACKET_MULTICAST)
                 rx_stats->rx_multicast++;
-                break;
-        case PACKET_OTHERHOST:
-                /* Our lower layer thinks this is not local, let's make sure.
-                 * This allows the VLAN to have a different MAC than the
-                 * underlying device, and still route correctly. */
-                if (!compare_ether_addr(eth_hdr(skb)->h_dest,
-                                        vlan_dev->dev_addr))
-                        skb->pkt_type = PACKET_HOST;
-                break;
-        }
         u64_stats_update_end(&rx_stats->syncp);
 
         return true;
@@ -89,18 +100,13 @@ gro_result_t vlan_gro_frags(struct napi_struct *napi, struct vlan_group *grp,
 }
 EXPORT_SYMBOL(vlan_gro_frags);
 
-static struct sk_buff *vlan_check_reorder_header(struct sk_buff *skb)
+static struct sk_buff *vlan_reorder_header(struct sk_buff *skb)
 {
-        if (vlan_dev_info(skb->dev)->flags & VLAN_FLAG_REORDER_HDR) {
-                if (skb_cow(skb, skb_headroom(skb)) < 0)
-                        skb = NULL;
-                if (skb) {
-                        /* Lifted from Gleb's VLAN code... */
-                        memmove(skb->data - ETH_HLEN,
-                                skb->data - VLAN_ETH_HLEN, 12);
-                        skb->mac_header += VLAN_HLEN;
-                }
-        }
+        if (skb_cow(skb, skb_headroom(skb)) < 0)
+                return NULL;
+        memmove(skb->data - ETH_HLEN, skb->data - VLAN_ETH_HLEN, 2 * ETH_ALEN);
+        skb->mac_header += VLAN_HLEN;
+        skb_reset_mac_len(skb);
         return skb;
 }
 
@@ -161,7 +167,7 @@ struct sk_buff *vlan_untag(struct sk_buff *skb)
         skb_pull_rcsum(skb, VLAN_HLEN);
         vlan_set_encap_proto(skb, vhdr);
 
-        skb = vlan_check_reorder_header(skb);
+        skb = vlan_reorder_header(skb);
         if (unlikely(!skb))
                 goto err_free;
 
diff --git a/net/core/dev.c b/net/core/dev.c
index a54c9f87ddbb..9c58c1ec41a9 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3114,7 +3114,7 @@ static int __netif_receive_skb(struct sk_buff *skb)
 
         skb_reset_network_header(skb);
         skb_reset_transport_header(skb);
-        skb->mac_len = skb->network_header - skb->mac_header;
+        skb_reset_mac_len(skb);
 
         pt_prev = NULL;