[ovs-dev] [PATCH] VLAN actions should use push/pop semantics

Pravin Shelar pshelar at nicira.com
Wed Sep 7 16:06:26 PDT 2011


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 at nicira.com>
---
 datapath/actions.c                              |   62 +++++++++++-----
 datapath/datapath.c                             |    8 +-
 datapath/linux/compat/include/linux/if_ether.h  |    1 +
 datapath/linux/compat/include/linux/netdevice.h |   19 +++++
 datapath/linux/compat/netdevice.c               |   91 +++++++++++++++++++++++
 datapath/vport-netdev.c                         |   10 +--
 include/openvswitch/datapath-protocol.h         |    4 +-
 lib/bond.c                                      |    2 +-
 lib/dpif-netdev.c                               |   14 ++--
 lib/odp-util.c                                  |   12 ++--
 lib/packets.c                                   |   27 +++----
 lib/packets.h                                   |    2 +-
 ofproto/ofproto-dpif-sflow.c                    |    2 +-
 ofproto/ofproto-dpif.c                          |   17 +++--
 14 files changed, 200 insertions(+), 71 deletions(-)

diff --git a/datapath/actions.c b/datapath/actions.c
index 8aec438..0150be2 100644
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -39,20 +39,12 @@ 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)
+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,6 +53,9 @@ 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);
@@ -71,21 +66,46 @@ 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;
 
+		current_tag = vlan_tx_tag_get(skb);
+
+		if (!__vlan_put_tag(skb, current_tag))
+			return -ENOMEM;
+	}
+	__vlan_hwaccel_put_tag(skb, ntohs(new_tci));
 	return 0;
 }
 
@@ -270,12 +290,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 0b6e2e5..ab800a7 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -553,8 +553,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,
@@ -576,7 +576,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:
@@ -594,7 +594,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..75ab82c 100644
--- a/datapath/linux/compat/include/linux/if_ether.h
+++ b/datapath/linux/compat/include/linux/if_ether.h
@@ -7,6 +7,7 @@
 #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,28)
 
 #define ETH_P_TEB      0x6558          /* Trans Ether Bridging         */
+#define ETH_P_FCOE     0x8906          /* Fibre Channel over Ethernet  */
 
 #endif /* linux kernel < 2.6.28 */
 
diff --git a/datapath/linux/compat/include/linux/netdevice.h b/datapath/linux/compat/include/linux/netdevice.h
index 04ebd89..c083477 100644
--- a/datapath/linux/compat/include/linux/netdevice.h
+++ b/datapath/linux/compat/include/linux/netdevice.h
@@ -141,4 +141,23 @@ 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,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..a06da0c 100644
--- a/datapath/linux/compat/netdevice.c
+++ b/datapath/linux/compat/netdevice.c
@@ -1,5 +1,7 @@
 #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);
@@ -46,4 +48,93 @@ struct rtnl_link_stats64 *dev_get_stats(struct net_device *dev,
 
 	return storage;
 }
+
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,26)
+static int illegal_highdma(struct net_device *dev, struct sk_buff *skb)
+{
+#ifdef CONFIG_HIGHMEM
+	int i;
+	if (!(dev->features & NETIF_F_HIGHDMA)) {
+		for (i = 0; i < skb_shinfo(skb)->nr_frags; i++)
+			if (PageHighMem(skb_shinfo(skb)->frags[i].page))
+				return 1;
+	}
+#endif
+	return 0;
+}
+
+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 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;
+}
+#endif
+
+u32 rpl_netif_skb_features(struct sk_buff *skb)
+{
+#if LINUX_VERSION_CODE >= 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 &= (skb->dev->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);
+	}
+#else
+	return 0;
+#endif
+}
+
+struct sk_buff *rpl_skb_gso_segment(struct sk_buff *skb, u32 features)
+{
+	int vlan_depth = ETH_HLEN;
+	__be16 type = skb->protocol;
+
+	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->protocol = type;
+
+	return skb_gso_segment(skb, features);
+}
+
 #endif	/* kernel version < 2.6.36 */
diff --git a/datapath/vport-netdev.c b/datapath/vport-netdev.c
index f1e9b09..4163506 100644
--- a/datapath/vport-netdev.c
+++ b/datapath/vport-netdev.c
@@ -316,19 +316,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 535aab3..522c532 100644
--- a/include/openvswitch/datapath-protocol.h
+++ b/include/openvswitch/datapath-protocol.h
@@ -412,8 +412,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
--- 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 d0a50f3..2723c68 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -712,7 +712,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;
@@ -726,7 +726,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:
@@ -1134,7 +1134,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
@@ -1306,12 +1306,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 2830fe8..82d668a 100644
--- 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..da228f0 100644
--- a/lib/packets.c
+++ b/lib/packets.c
@@ -81,27 +81,22 @@ compose_benign_packet(struct ofpbuf *b, const char *tag, uint16_t snap_type,
  *
  * 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
--- 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
--- 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 f09c230..7913cd7 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -2272,11 +2272,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;
         }
@@ -2840,9 +2840,11 @@ 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;
@@ -3639,13 +3641,16 @@ 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;
         }
-- 
1.7.1




More information about the dev mailing list