datapath: VLAN actions should use push/pop semantics

Currently the kernel vlan actions mirror those used by OpenFlow 1.0.
i.e. MODIFY and STRIP. More flexible approach is to have an action to
push a tag and pop a tag off, so that it can handle multiple levels of vlan
tags. Plus it aligns with newer version of OpenFlow.
        As this patch replaces MODIFY with PUSH semantic, action
mapping done in userpace is fixed accordingly.
        GSO handling for multiple levels of vlan tags is also added as
Jesse suggested before.

Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>

diff --git a/datapath/actions.c b/datapath/actions.c
index 8aec438..83b531a 100644 (file)
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -39,20 +39,13 @@ static int make_writable(struct sk_buff *skb, int write_len)
        return pskb_expand_head(skb, 0, 0, GFP_ATOMIC);
 }
 
-static int strip_vlan(struct sk_buff *skb)
+/* remove VLAN header from packet and update csum accrodingly. */
+static int __pop_vlan_tci(struct sk_buff *skb, __be16 *current_tci)
 {
        struct ethhdr *eh;
+       struct vlan_ethhdr *veth;
        int err;
 
-       if (vlan_tx_tag_present(skb)) {
-               vlan_set_tci(skb, 0);
-               return 0;
-       }
-
-       if (unlikely(skb->protocol != htons(ETH_P_8021Q) ||
-           skb->len < VLAN_ETH_HLEN))
-               return 0;
-
        err = make_writable(skb, VLAN_ETH_HLEN);
        if (unlikely(err))
                return err;
@@ -61,9 +54,12 @@ static int strip_vlan(struct sk_buff *skb)
                skb->csum = csum_sub(skb->csum, csum_partial(skb->data
                                        + ETH_HLEN, VLAN_HLEN, 0));
 
+       veth = (struct vlan_ethhdr *) skb->data;
+       *current_tci = veth->h_vlan_TCI;
+
        memmove(skb->data + VLAN_HLEN, skb->data, 2 * ETH_ALEN);
 
-       eh = (struct ethhdr *)skb_pull(skb, VLAN_HLEN);
+       eh = (struct ethhdr *)__skb_pull(skb, VLAN_HLEN);
 
        skb->protocol = eh->h_proto;
        skb->mac_header += VLAN_HLEN;
@@ -71,21 +67,52 @@ static int strip_vlan(struct sk_buff *skb)
        return 0;
 }
 
-static int modify_vlan_tci(struct sk_buff *skb, __be16 tci)
+static int pop_vlan(struct sk_buff *skb)
 {
-       if (!vlan_tx_tag_present(skb) && skb->protocol == htons(ETH_P_8021Q)) {
-               int err;
+       __be16 tci;
+       int err;
 
-               if (unlikely(skb->len < VLAN_ETH_HLEN))
+       if (likely(vlan_tx_tag_present(skb))) {
+               vlan_set_tci(skb, 0);
+       } else {
+               if (unlikely(skb->protocol != htons(ETH_P_8021Q) ||
+                   skb->len < VLAN_ETH_HLEN))
                        return 0;
 
-               err = strip_vlan(skb);
-               if (unlikely(err))
+               err = __pop_vlan_tci(skb, &tci);
+               if (err)
                        return err;
        }
+       /* move next vlan tag to hw accel tag */
+       if (likely(skb->protocol != htons(ETH_P_8021Q) ||
+           skb->len < VLAN_ETH_HLEN))
+               return 0;
+
+       err = __pop_vlan_tci(skb, &tci);
+       if (unlikely(err))
+               return err;
 
        __vlan_hwaccel_put_tag(skb, ntohs(tci));
+       return 0;
+}
+
+static int push_vlan(struct sk_buff *skb, __be16 new_tci)
+{
+       if (unlikely(vlan_tx_tag_present(skb))) {
+               u16 current_tag;
+
+               /* push down current VLAN tag */
+               current_tag = vlan_tx_tag_get(skb);
+
+               if (!__vlan_put_tag(skb, current_tag))
+                       return -ENOMEM;
 
+               if (get_ip_summed(skb) == OVS_CSUM_COMPLETE)
+                       skb->csum = csum_add(skb->csum, csum_partial(skb->data
+                                       + ETH_HLEN, VLAN_HLEN, 0));
+
+       }
+       __vlan_hwaccel_put_tag(skb, ntohs(new_tci));
        return 0;
 }
 
@@ -270,12 +297,14 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
                        OVS_CB(skb)->tun_id = nla_get_be64(a);
                        break;
 
-               case OVS_ACTION_ATTR_SET_DL_TCI:
-                       err = modify_vlan_tci(skb, nla_get_be16(a));
+               case OVS_ACTION_ATTR_PUSH_VLAN:
+                       err = push_vlan(skb, nla_get_be16(a));
+                       if (unlikely(err)) /* skb already freed */
+                               return err;
                        break;
 
-               case OVS_ACTION_ATTR_STRIP_VLAN:
-                       err = strip_vlan(skb);
+               case OVS_ACTION_ATTR_POP_VLAN:
+                       err = pop_vlan(skb);
                        break;
 
                case OVS_ACTION_ATTR_SET_DL_SRC:

diff --git a/datapath/datapath.c b/datapath/datapath.c
index c1c843d..454f96c 100644 (file)
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -545,8 +545,8 @@ static int validate_actions(const struct nlattr *attr)
                static const u32 action_lens[OVS_ACTION_ATTR_MAX + 1] = {
                        [OVS_ACTION_ATTR_OUTPUT] = 4,
                        [OVS_ACTION_ATTR_USERSPACE] = 8,
-                       [OVS_ACTION_ATTR_SET_DL_TCI] = 2,
-                       [OVS_ACTION_ATTR_STRIP_VLAN] = 0,
+                       [OVS_ACTION_ATTR_PUSH_VLAN] = 2,
+                       [OVS_ACTION_ATTR_POP_VLAN] = 0,
                        [OVS_ACTION_ATTR_SET_DL_SRC] = ETH_ALEN,
                        [OVS_ACTION_ATTR_SET_DL_DST] = ETH_ALEN,
                        [OVS_ACTION_ATTR_SET_NW_SRC] = 4,
@@ -568,7 +568,7 @@ static int validate_actions(const struct nlattr *attr)
                        return -EINVAL;
 
                case OVS_ACTION_ATTR_USERSPACE:
-               case OVS_ACTION_ATTR_STRIP_VLAN:
+               case OVS_ACTION_ATTR_POP_VLAN:
                case OVS_ACTION_ATTR_SET_DL_SRC:
                case OVS_ACTION_ATTR_SET_DL_DST:
                case OVS_ACTION_ATTR_SET_NW_SRC:
@@ -586,7 +586,7 @@ static int validate_actions(const struct nlattr *attr)
                                return -EINVAL;
                        break;
 
-               case OVS_ACTION_ATTR_SET_DL_TCI:
+               case OVS_ACTION_ATTR_PUSH_VLAN:
                        if (nla_get_be16(a) & htons(VLAN_CFI_MASK))
                                return -EINVAL;
                        break;

diff --git a/datapath/linux/compat/include/linux/if_ether.h b/datapath/linux/compat/include/linux/if_ether.h
index b8390e2..41cc053 100644 (file)
--- a/datapath/linux/compat/include/linux/if_ether.h
+++ b/datapath/linux/compat/include/linux/if_ether.h
@@ -10,4 +10,10 @@
 
 #endif /* linux kernel < 2.6.28 */
 
+#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,30)
+
+#define ETH_P_FCOE     0x8906          /* Fibre Channel over Ethernet  */
+
+#endif /* linux kernel < 2.6.30 */
+
 #endif

diff --git a/datapath/linux/compat/include/linux/netdevice.h b/datapath/linux/compat/include/linux/netdevice.h
index 04ebd89..1ac3831 100644 (file)
--- a/datapath/linux/compat/include/linux/netdevice.h
+++ b/datapath/linux/compat/include/linux/netdevice.h
@@ -141,4 +141,30 @@ static inline struct net_device *dev_get_by_index_rcu(struct net *net, int ifind
 #define NETIF_F_FSO 0
 #endif
 
+#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,30)
+#define NETIF_F_FCOE_CRC       (1 << 24) /* FCoE CRC32 */
+#endif
+
+#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,23)
+#define NETIF_F_IPV6_CSUM      16      /* Can checksum TCP/UDP over IPV6 */
+
+#define NETIF_F_V4_CSUM                (NETIF_F_GEN_CSUM | NETIF_F_IP_CSUM)
+#define NETIF_F_V6_CSUM                (NETIF_F_GEN_CSUM | NETIF_F_IPV6_CSUM)
+#endif
+
+#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,38)
+#define skb_gso_segment rpl_skb_gso_segment
+struct sk_buff *rpl_skb_gso_segment(struct sk_buff *skb, u32 features);
+
+#define netif_skb_features rpl_netif_skb_features
+u32 rpl_netif_skb_features(struct sk_buff *skb);
+
+#define netif_needs_gso rpl_netif_needs_gso
+static inline int rpl_netif_needs_gso(struct sk_buff *skb, int features)
+{
+        return skb_is_gso(skb) && (!skb_gso_ok(skb, features) ||
+                unlikely(skb->ip_summed != CHECKSUM_PARTIAL));
+}
+#endif
+
 #endif

diff --git a/datapath/linux/compat/netdevice.c b/datapath/linux/compat/netdevice.c
index 1a6f327..91855c4 100644 (file)
--- a/datapath/linux/compat/netdevice.c
+++ b/datapath/linux/compat/netdevice.c
@@ -1,5 +1,6 @@
 #include <linux/if_link.h>
 #include <linux/netdevice.h>
+#include <linux/if_vlan.h>
 
 /* Linux 2.6.28 introduced dev_get_stats():
  * const struct net_device_stats *dev_get_stats(struct net_device *dev);
@@ -47,3 +48,101 @@ struct rtnl_link_stats64 *dev_get_stats(struct net_device *dev,
        return storage;
 }
 #endif /* kernel version < 2.6.36 */
+
+#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,38)
+static bool can_checksum_protocol(unsigned long features, __be16 protocol)
+{
+       return  ((features & NETIF_F_GEN_CSUM) ||
+               ((features & NETIF_F_V4_CSUM) &&
+                               protocol == htons(ETH_P_IP)) ||
+               ((features & NETIF_F_V6_CSUM) &&
+                               protocol == htons(ETH_P_IPV6)) ||
+               ((features & NETIF_F_FCOE_CRC) &&
+                               protocol == htons(ETH_P_FCOE)));
+}
+
+static inline int illegal_highdma(struct net_device *dev, struct sk_buff *skb)
+{
+#ifdef CONFIG_HIGHMEM
+       int i;
+
+       if (dev->features & NETIF_F_HIGHDMA)
+               return 0;
+
+       for (i = 0; i < skb_shinfo(skb)->nr_frags; i++)
+               if (PageHighMem(skb_shinfo(skb)->frags[i].page))
+                       return 1;
+
+#endif
+       return 0;
+}
+
+static u32 harmonize_features(struct sk_buff *skb, __be16 protocol, u32 features)
+{
+       if (!can_checksum_protocol(features, protocol)) {
+               features &= ~NETIF_F_ALL_CSUM;
+               features &= ~NETIF_F_SG;
+       } else if (illegal_highdma(skb->dev, skb)) {
+               features &= ~NETIF_F_SG;
+       }
+
+       return features;
+}
+
+u32 rpl_netif_skb_features(struct sk_buff *skb)
+{
+#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,26)
+       unsigned long vlan_features = 0;
+#else
+       unsigned long vlan_features = skb->dev->vlan_features;
+#endif /* kernel version < 2.6.26 */
+
+       __be16 protocol = skb->protocol;
+       u32 features = skb->dev->features;
+
+       if (protocol == htons(ETH_P_8021Q)) {
+               struct vlan_ethhdr *veh = (struct vlan_ethhdr *)skb->data;
+               protocol = veh->h_vlan_encapsulated_proto;
+       } else if (!vlan_tx_tag_present(skb)) {
+               return harmonize_features(skb, protocol, features);
+       }
+
+       features &= (vlan_features | NETIF_F_HW_VLAN_TX);
+
+       if (protocol != htons(ETH_P_8021Q)) {
+               return harmonize_features(skb, protocol, features);
+       } else {
+               features &= NETIF_F_SG | NETIF_F_HIGHDMA | NETIF_F_FRAGLIST |
+                       NETIF_F_GEN_CSUM | NETIF_F_HW_VLAN_TX;
+               return harmonize_features(skb, protocol, features);
+       }
+}
+
+struct sk_buff *rpl_skb_gso_segment(struct sk_buff *skb, u32 features)
+{
+       int vlan_depth = ETH_HLEN;
+       __be16 type = skb->protocol;
+       __be16 skb_proto;
+       struct sk_buff *skb_gso;
+
+       while (type == htons(ETH_P_8021Q)) {
+               struct vlan_hdr *vh;
+
+               if (unlikely(!pskb_may_pull(skb, vlan_depth + VLAN_HLEN)))
+                       return ERR_PTR(-EINVAL);
+
+               vh = (struct vlan_hdr *)(skb->data + vlan_depth);
+               type = vh->h_vlan_encapsulated_proto;
+               vlan_depth += VLAN_HLEN;
+       }
+
+       /* this hack needed to get regular skb_gso_segment() */
+#undef skb_gso_segment
+       skb_proto = skb->protocol;
+       skb->protocol = type;
+
+       skb_gso = skb_gso_segment(skb, features);
+       skb->protocol = skb_proto;
+       return skb_gso;
+}
+#endif /* kernel version < 2.6.38 */

diff --git a/datapath/vport-netdev.c b/datapath/vport-netdev.c
index dbc8db2..ee39ee3 100644 (file)
--- a/datapath/vport-netdev.c
+++ b/datapath/vport-netdev.c
@@ -333,19 +333,15 @@ static int netdev_send(struct vport *vport, struct sk_buff *skb)
        forward_ip_summed(skb, true);
 
        if (vlan_tx_tag_present(skb) && !dev_supports_vlan_tx(skb->dev)) {
-               int features = 0;
+               int features;
 
-#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,26)
-               features = skb->dev->features & skb->dev->vlan_features;
-#endif
+               features = netif_skb_features(skb);
 
                if (!vlan_tso)
                        features &= ~(NETIF_F_TSO | NETIF_F_TSO6 |
                                      NETIF_F_UFO | NETIF_F_FSO);
 
-               if (skb_is_gso(skb) &&
-                   (!skb_gso_ok(skb, features) ||
-                    unlikely(skb->ip_summed != CHECKSUM_PARTIAL))) {
+               if (netif_needs_gso(skb, features)) {
                        struct sk_buff *nskb;
 
                        nskb = skb_gso_segment(skb, features);

diff --git a/include/openvswitch/datapath-protocol.h b/include/openvswitch/datapath-protocol.h
index 97a7c04..c1c9ef0 100644 (file)
--- a/include/openvswitch/datapath-protocol.h
+++ b/include/openvswitch/datapath-protocol.h
@@ -409,8 +409,8 @@ enum ovs_action_type {
        OVS_ACTION_ATTR_UNSPEC,
        OVS_ACTION_ATTR_OUTPUT,       /* Output to switch port. */
        OVS_ACTION_ATTR_USERSPACE,    /* Send copy to userspace. */
-       OVS_ACTION_ATTR_SET_DL_TCI,   /* Set the 802.1q TCI value. */
-       OVS_ACTION_ATTR_STRIP_VLAN,   /* Strip the 802.1q header. */
+       OVS_ACTION_ATTR_PUSH_VLAN,    /* Set the 802.1q TCI value. */
+       OVS_ACTION_ATTR_POP_VLAN,     /* Strip the 802.1q header. */
        OVS_ACTION_ATTR_SET_DL_SRC,   /* Ethernet source address. */
        OVS_ACTION_ATTR_SET_DL_DST,   /* Ethernet destination address. */
        OVS_ACTION_ATTR_SET_NW_SRC,   /* IPv4 source address. */

diff --git a/lib/bond.c b/lib/bond.c
index ae914dd..5b984fb 100644 (file)
--- a/lib/bond.c
+++ b/lib/bond.c
@@ -532,7 +532,7 @@ bond_send_learning_packet(struct bond *bond,
     compose_benign_packet(&packet, "Open vSwitch Bond Failover", 0xf177,
                           eth_src);
     if (vlan) {
-        eth_set_vlan_tci(&packet, htons(vlan));
+        eth_push_vlan(&packet, htons(vlan));
     }
     error = netdev_send(slave->netdev, &packet);
     ofpbuf_uninit(&packet);

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 5b91b7f..8aefaf9 100644 (file)
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -718,7 +718,7 @@ dpif_netdev_validate_actions(const struct nlattr *actions,
         case OVS_ACTION_ATTR_USERSPACE:
             break;
 
-        case OVS_ACTION_ATTR_SET_DL_TCI:
+        case OVS_ACTION_ATTR_PUSH_VLAN:
             *mutates = true;
             if (nl_attr_get_be16(a) & htons(VLAN_CFI)) {
                 return EINVAL;
@@ -732,7 +732,7 @@ dpif_netdev_validate_actions(const struct nlattr *actions,
             }
             break;
 
-        case OVS_ACTION_ATTR_STRIP_VLAN:
+        case OVS_ACTION_ATTR_POP_VLAN:
         case OVS_ACTION_ATTR_SET_DL_SRC:
         case OVS_ACTION_ATTR_SET_DL_DST:
         case OVS_ACTION_ATTR_SET_NW_SRC:
@@ -1142,7 +1142,7 @@ dpif_netdev_wait(struct dpif *dpif)
 }
 
 static void
-dp_netdev_strip_vlan(struct ofpbuf *packet)
+dp_netdev_pop_vlan(struct ofpbuf *packet)
 {
     struct vlan_eth_header *veh = packet->l2;
     if (packet->size >= sizeof *veh
@@ -1314,12 +1314,12 @@ dp_netdev_execute_actions(struct dp_netdev *dp,
                                      key, nl_attr_get_u64(a));
             break;
 
-        case OVS_ACTION_ATTR_SET_DL_TCI:
-            eth_set_vlan_tci(packet, nl_attr_get_be16(a));
+        case OVS_ACTION_ATTR_PUSH_VLAN:
+            eth_push_vlan(packet, nl_attr_get_be16(a));
             break;
 
-        case OVS_ACTION_ATTR_STRIP_VLAN:
-            dp_netdev_strip_vlan(packet);
+        case OVS_ACTION_ATTR_POP_VLAN:
+            dp_netdev_pop_vlan(packet);
             break;
 
         case OVS_ACTION_ATTR_SET_DL_SRC:

diff --git a/lib/odp-util.c b/lib/odp-util.c
index b42a03c..9657595 100644 (file)
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -49,8 +49,8 @@ odp_action_len(uint16_t type)
     switch ((enum ovs_action_type) type) {
     case OVS_ACTION_ATTR_OUTPUT: return 4;
     case OVS_ACTION_ATTR_USERSPACE: return 8;
-    case OVS_ACTION_ATTR_SET_DL_TCI: return 2;
-    case OVS_ACTION_ATTR_STRIP_VLAN: return 0;
+    case OVS_ACTION_ATTR_PUSH_VLAN: return 2;
+    case OVS_ACTION_ATTR_POP_VLAN: return 0;
     case OVS_ACTION_ATTR_SET_DL_SRC: return ETH_ADDR_LEN;
     case OVS_ACTION_ATTR_SET_DL_DST: return ETH_ADDR_LEN;
     case OVS_ACTION_ATTR_SET_NW_SRC: return 4;
@@ -113,13 +113,13 @@ format_odp_action(struct ds *ds, const struct nlattr *a)
         ds_put_format(ds, "set_tunnel(%#"PRIx64")",
                       ntohll(nl_attr_get_be64(a)));
         break;
-    case OVS_ACTION_ATTR_SET_DL_TCI:
-        ds_put_format(ds, "set_tci(vid=%"PRIu16",pcp=%d)",
+    case OVS_ACTION_ATTR_PUSH_VLAN:
+        ds_put_format(ds, "push_vlan(vid=%"PRIu16",pcp=%d)",
                       vlan_tci_to_vid(nl_attr_get_be16(a)),
                       vlan_tci_to_pcp(nl_attr_get_be16(a)));
         break;
-    case OVS_ACTION_ATTR_STRIP_VLAN:
-        ds_put_format(ds, "strip_vlan");
+    case OVS_ACTION_ATTR_POP_VLAN:
+        ds_put_format(ds, "pop_vlan");
         break;
     case OVS_ACTION_ATTR_SET_DL_SRC:
         eth = nl_attr_get_unspec(a, ETH_ADDR_LEN);

diff --git a/lib/packets.c b/lib/packets.c
index e05e3eb..094f553 100644 (file)
--- a/lib/packets.c
+++ b/lib/packets.c
@@ -75,33 +75,27 @@ compose_benign_packet(struct ofpbuf *b, const char *tag, uint16_t snap_type,
     memcpy(payload + tag_size, eth_src, ETH_ADDR_LEN);
 }
 
-/* Modify the TCI field of 'packet', whose data must begin with an Ethernet
- * header.  If a VLAN tag is present, its TCI field is replaced by 'tci'.  If a
- * VLAN tag is not present, one is added with the TCI field set to 'tci'.
+/* Insert VLAN header according to given TCI. Packet passed must be Ethernet
+ * packet.
  *
  * Also sets 'packet->l2' to point to the new Ethernet header. */
 void
-eth_set_vlan_tci(struct ofpbuf *packet, ovs_be16 tci)
+eth_push_vlan(struct ofpbuf *packet, ovs_be16 tci)
 {
     struct eth_header *eh = packet->data;
     struct vlan_eth_header *veh;
 
-    if (packet->size >= sizeof(struct vlan_eth_header)
-        && eh->eth_type == htons(ETH_TYPE_VLAN)) {
-        veh = packet->data;
-        veh->veth_tci = tci;
-    } else {
-        /* Insert new 802.1Q header. */
-        struct vlan_eth_header tmp;
-        memcpy(tmp.veth_dst, eh->eth_dst, ETH_ADDR_LEN);
-        memcpy(tmp.veth_src, eh->eth_src, ETH_ADDR_LEN);
-        tmp.veth_type = htons(ETH_TYPE_VLAN);
-        tmp.veth_tci = tci;
-        tmp.veth_next_type = eh->eth_type;
-
-        veh = ofpbuf_push_uninit(packet, VLAN_HEADER_LEN);
-        memcpy(veh, &tmp, sizeof tmp);
-    }
+    /* Insert new 802.1Q header. */
+    struct vlan_eth_header tmp;
+    memcpy(tmp.veth_dst, eh->eth_dst, ETH_ADDR_LEN);
+    memcpy(tmp.veth_src, eh->eth_src, ETH_ADDR_LEN);
+    tmp.veth_type = htons(ETH_TYPE_VLAN);
+    tmp.veth_tci = tci;
+    tmp.veth_next_type = eh->eth_type;
+
+    veh = ofpbuf_push_uninit(packet, VLAN_HEADER_LEN);
+    memcpy(veh, &tmp, sizeof tmp);
+
     packet->l2 = packet->data;
 }
 

diff --git a/lib/packets.h b/lib/packets.h
index a389e6a..cb12638 100644 (file)
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -131,7 +131,7 @@ void compose_benign_packet(struct ofpbuf *, const char *tag,
                            uint16_t snap_type,
                            const uint8_t eth_src[ETH_ADDR_LEN]);
 
-void eth_set_vlan_tci(struct ofpbuf *, ovs_be16 tci);
+void eth_push_vlan(struct ofpbuf *, ovs_be16 tci);
 
 /* Example:
  *

diff --git a/ofproto/ofproto-dpif-sflow.c b/ofproto/ofproto-dpif-sflow.c
index d41b7da..21ef799 100644 (file)
--- a/ofproto/ofproto-dpif-sflow.c
+++ b/ofproto/ofproto-dpif-sflow.c
@@ -538,7 +538,7 @@ dpif_sflow_received(struct dpif_sflow *ds, const struct dpif_upcall *upcall,
             n_outputs++;
             break;
 
-        case OVS_ACTION_ATTR_SET_DL_TCI:
+        case OVS_ACTION_ATTR_PUSH_VLAN:
             tci = nl_attr_get_be16(a);
             switchElem.flowType.sw.dst_vlan = vlan_tci_to_vid(tci);
             switchElem.flowType.sw.dst_priority = vlan_tci_to_pcp(tci);

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 5d93153..3074881 100644 (file)
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -2289,11 +2289,11 @@ facet_account(struct ofproto_dpif *ofproto, struct facet *facet)
             }
             break;
 
-        case OVS_ACTION_ATTR_STRIP_VLAN:
+        case OVS_ACTION_ATTR_POP_VLAN:
             vlan_tci = htons(0);
             break;
 
-        case OVS_ACTION_ATTR_SET_DL_TCI:
+        case OVS_ACTION_ATTR_PUSH_VLAN:
             vlan_tci = nl_attr_get_be16(a);
             break;
         }
@@ -2857,9 +2857,12 @@ commit_odp_actions(struct action_xlate_ctx *ctx)
 
     if (base->vlan_tci != flow->vlan_tci) {
         if (!(flow->vlan_tci & htons(VLAN_CFI))) {
-            nl_msg_put_flag(odp_actions, OVS_ACTION_ATTR_STRIP_VLAN);
+            nl_msg_put_flag(odp_actions, OVS_ACTION_ATTR_POP_VLAN);
         } else {
-            nl_msg_put_be16(odp_actions, OVS_ACTION_ATTR_SET_DL_TCI,
+            if (base->vlan_tci != OFP_VLAN_NONE) {
+                nl_msg_put_flag(odp_actions, OVS_ACTION_ATTR_POP_VLAN);
+            }
+            nl_msg_put_be16(odp_actions, OVS_ACTION_ATTR_PUSH_VLAN,
                             flow->vlan_tci & ~htons(VLAN_CFI));
         }
         base->vlan_tci = flow->vlan_tci;
@@ -3656,13 +3659,17 @@ compose_actions(struct action_xlate_ctx *ctx, uint16_t vlan,
         }
         if (dst->vlan != cur_vlan) {
             if (dst->vlan == OFP_VLAN_NONE) {
-                nl_msg_put_flag(ctx->odp_actions, OVS_ACTION_ATTR_STRIP_VLAN);
+                nl_msg_put_flag(ctx->odp_actions, OVS_ACTION_ATTR_POP_VLAN);
             } else {
                 ovs_be16 tci;
+
+                if (cur_vlan != OFP_VLAN_NONE) {
+                     nl_msg_put_flag(ctx->odp_actions, OVS_ACTION_ATTR_POP_VLAN);
+                }
                 tci = htons(dst->vlan & VLAN_VID_MASK);
                 tci |= ctx->flow.vlan_tci & htons(VLAN_PCP_MASK);
                 nl_msg_put_be16(ctx->odp_actions,
-                                OVS_ACTION_ATTR_SET_DL_TCI, tci);
+                                OVS_ACTION_ATTR_PUSH_VLAN, tci);
             }
             cur_vlan = dst->vlan;
         }