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>

5 files changed, 29 insertions, 35 deletions

diff --git a/lib/bond.c b/lib/bond.c
index ae914dd9a..5b984fb55 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 5b91b7f57..8aefaf9b0 100644
--- 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 b42a03c39..965759508 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 e05e3ebca..094f55366 100644
--- 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 a389e6af7..cb1263809 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:
  *