diff mbox

[ovs-dev,v12,rebased,1/3] userspace: add support for pop_eth and push_eth actions

Message ID b603f83906d4e29afd3714966e7a79a2ec2755fa.1476896419.git.jbenc@redhat.com
State RFC
Headers show

Commit Message

Jiri Benc Oct. 19, 2016, 5:21 p.m. UTC
From: Lorand Jakab <lojakab@cisco.com>

These actions will allow L2->L3 and L3->L2 switching, and are supposed
to be added to flows installed in the datapath transparently by
ovs-vswitchd.

Signed-off-by: Lorand Jakab <lojakab@cisco.com>
Signed-off-by: Simon Horman <simon.horman@netronome.com>
Signed-off-by: Jiri Benc <jbenc@redhat.com>

---
v12 [Simon Horman]
* Rebase
* Provide commit_ether_action()

v11 [Simon Horman]
* Omit type field from push_eth action, it is not needed
  as the type of the packet is already known

v10 [Simon Horman]
* No Change

v9 [Simon Horman]
* Rebased

v1 - v8 [Lorand Jakub]

clanup: commit push/pop eth
---
 datapath/linux/compat/include/linux/openvswitch.h | 14 ++++
 lib/dpif-netdev.c                                 |  2 +
 lib/dpif.c                                        |  2 +
 lib/odp-execute.c                                 | 18 +++++
 lib/odp-util.c                                    | 83 ++++++++++++++++++++++-
 lib/packets.c                                     | 24 +++++++
 lib/packets.h                                     |  4 ++
 ofproto/ofproto-dpif-sflow.c                      |  7 ++
 8 files changed, 151 insertions(+), 3 deletions(-)

Comments

Ben Pfaff Nov. 29, 2016, 12:56 a.m. UTC | #1
On Wed, Oct 19, 2016 at 07:21:17PM +0200, Jiri Benc wrote:
> From: Lorand Jakab <lojakab@cisco.com>
> 
> These actions will allow L2->L3 and L3->L2 switching, and are supposed
> to be added to flows installed in the datapath transparently by
> ovs-vswitchd.
> 
> Signed-off-by: Lorand Jakab <lojakab@cisco.com>
> Signed-off-by: Simon Horman <simon.horman@netronome.com>
> Signed-off-by: Jiri Benc <jbenc@redhat.com>

I know that this isn't intended to be applied yet, according to the
cover letter, but it doesn't build:

    ../lib/odp-util.c:5504:15: error: no member named 'base_layer' in 'struct flow'
    ../lib/odp-util.c:5504:40: error: no member named 'base_layer' in 'struct flow'
    ../lib/odp-util.c:5505:19: error: no member named 'base_layer' in 'struct flow'
    ../lib/odp-util.c:5505:33: error: use of undeclared identifier 'LAYER_2'
    ../lib/odp-util.c:5512:20: error: no member named 'base_layer' in 'struct flow'
    ../lib/odp-util.c:5512:40: error: no member named 'base_layer' in 'struct flow'

and furthermore I can't any mention of base_layer in the OVS history so
I'm not sure what's going on here.
Jiri Benc Nov. 29, 2016, 8:18 a.m. UTC | #2
On Mon, 28 Nov 2016 16:56:27 -0800, Ben Pfaff wrote:
> I know that this isn't intended to be applied yet, according to the
> cover letter, but it doesn't build:
> 
>     ../lib/odp-util.c:5504:15: error: no member named 'base_layer' in 'struct flow'
>     ../lib/odp-util.c:5504:40: error: no member named 'base_layer' in 'struct flow'
>     ../lib/odp-util.c:5505:19: error: no member named 'base_layer' in 'struct flow'
>     ../lib/odp-util.c:5505:33: error: use of undeclared identifier 'LAYER_2'
>     ../lib/odp-util.c:5512:20: error: no member named 'base_layer' in 'struct flow'
>     ../lib/odp-util.c:5512:40: error: no member named 'base_layer' in 'struct flow'
> 
> and furthermore I can't any mention of base_layer in the OVS history so
> I'm not sure what's going on here.

It's in patch 2, seems that the order is mixed up. Sorry for that.
Please apply all three patches together for testing.

We'll be taking a different approach with this patchset, implementing
EXT-382 instead.

Thanks,

 Jiri
diff mbox

Patch

diff --git a/datapath/linux/compat/include/linux/openvswitch.h b/datapath/linux/compat/include/linux/openvswitch.h
index 12260d8bfd64..67004caaf8b5 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -749,6 +749,15 @@  enum ovs_nat_attr {
 #define OVS_NAT_ATTR_MAX (__OVS_NAT_ATTR_MAX - 1)
 
 /**
+ * struct ovs_action_push_eth - %OVS_ACTION_ATTR_PUSH_ETH action argument.
+ * @addresses: Source and destination MAC addresses.
+ * @eth_type: Ethernet type
+ */
+struct ovs_action_push_eth {
+	struct ovs_key_ethernet addresses;
+};
+
+/**
  * enum ovs_action_attr - Action types.
  *
  * @OVS_ACTION_ATTR_OUTPUT: Output packet to port.
@@ -782,6 +791,9 @@  enum ovs_nat_attr {
  * is no MPLS label stack, as determined by ethertype, no action is taken.
  * @OVS_ACTION_ATTR_CT: Track the connection. Populate the conntrack-related
  * entries in the flow key.
+ * @OVS_ACTION_ATTR_PUSH_ETH: Push a new outermost Ethernet header onto the
+ * packet.
+ * @OVS_ACTION_ATTR_POP_ETH: Pop the outermost Ethernet header off the packet.
  *
  * Only a single header can be set with a single %OVS_ACTION_ATTR_SET.  Not all
  * fields within a header are modifiable, e.g. the IPv4 protocol and fragment
@@ -814,6 +826,8 @@  enum ovs_action_attr {
 				       * bits. */
 	OVS_ACTION_ATTR_CT,           /* Nested OVS_CT_ATTR_* . */
 	OVS_ACTION_ATTR_TRUNC,        /* u32 struct ovs_action_trunc. */
+	OVS_ACTION_ATTR_PUSH_ETH,     /* struct ovs_action_push_eth. */
+	OVS_ACTION_ATTR_POP_ETH,      /* No argument. */
 
 #ifndef __KERNEL__
 	OVS_ACTION_ATTR_TUNNEL_PUSH,   /* struct ovs_action_push_tnl*/
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 8fd98baeb135..8e590c5d2f5f 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -4577,6 +4577,8 @@  dp_execute_cb(void *aux_, struct dp_packet_batch *packets_,
     case OVS_ACTION_ATTR_HASH:
     case OVS_ACTION_ATTR_UNSPEC:
     case OVS_ACTION_ATTR_TRUNC:
+    case OVS_ACTION_ATTR_PUSH_ETH:
+    case OVS_ACTION_ATTR_POP_ETH:
     case __OVS_ACTION_ATTR_MAX:
         OVS_NOT_REACHED();
     }
diff --git a/lib/dpif.c b/lib/dpif.c
index 53958c559aa6..9634d44e0085 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -1186,6 +1186,8 @@  dpif_execute_helper_cb(void *aux_, struct dp_packet_batch *packets_,
     case OVS_ACTION_ATTR_SET_MASKED:
     case OVS_ACTION_ATTR_SAMPLE:
     case OVS_ACTION_ATTR_TRUNC:
+    case OVS_ACTION_ATTR_PUSH_ETH:
+    case OVS_ACTION_ATTR_POP_ETH:
     case OVS_ACTION_ATTR_UNSPEC:
     case __OVS_ACTION_ATTR_MAX:
         OVS_NOT_REACHED();
diff --git a/lib/odp-execute.c b/lib/odp-execute.c
index 65a6fcde8eb5..5fe6a867d5b8 100644
--- a/lib/odp-execute.c
+++ b/lib/odp-execute.c
@@ -504,6 +504,8 @@  requires_datapath_assistance(const struct nlattr *a)
     case OVS_ACTION_ATTR_PUSH_MPLS:
     case OVS_ACTION_ATTR_POP_MPLS:
     case OVS_ACTION_ATTR_TRUNC:
+    case OVS_ACTION_ATTR_POP_ETH:
+    case OVS_ACTION_ATTR_PUSH_ETH:
         return false;
 
     case OVS_ACTION_ATTR_UNSPEC:
@@ -637,6 +639,22 @@  odp_execute_actions(void *dp, struct dp_packet_batch *batch, bool steal,
             break;
         }
 
+        case OVS_ACTION_ATTR_PUSH_ETH: {
+            const struct ovs_action_push_eth *eth = nl_attr_get(a);
+
+            for (i = 0; i < cnt; i++) {
+                push_eth(packets[i], &eth->addresses.eth_dst,
+                         &eth->addresses.eth_src);
+            }
+            break;
+        }
+
+        case OVS_ACTION_ATTR_POP_ETH:
+            for (i = 0; i < cnt; i++) {
+                pop_eth(packets[i]);
+            }
+            break;
+
         case OVS_ACTION_ATTR_OUTPUT:
         case OVS_ACTION_ATTR_TUNNEL_PUSH:
         case OVS_ACTION_ATTR_TUNNEL_POP:
diff --git a/lib/odp-util.c b/lib/odp-util.c
index 332698bfed64..91c44fba0088 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -121,6 +121,8 @@  odp_action_len(uint16_t type)
     case OVS_ACTION_ATTR_SET_MASKED: return ATTR_LEN_VARIABLE;
     case OVS_ACTION_ATTR_SAMPLE: return ATTR_LEN_VARIABLE;
     case OVS_ACTION_ATTR_CT: return ATTR_LEN_VARIABLE;
+    case OVS_ACTION_ATTR_PUSH_ETH: return sizeof(struct ovs_action_push_eth);
+    case OVS_ACTION_ATTR_POP_ETH: return 0;
 
     case OVS_ACTION_ATTR_UNSPEC:
     case __OVS_ACTION_ATTR_MAX:
@@ -828,6 +830,16 @@  format_odp_action(struct ds *ds, const struct nlattr *a)
         format_odp_key_attr(nl_attr_get(a), NULL, NULL, ds, true);
         ds_put_cstr(ds, ")");
         break;
+    case OVS_ACTION_ATTR_PUSH_ETH: {
+        const struct ovs_action_push_eth *eth = nl_attr_get(a);
+        ds_put_format(ds, "push_eth(src="ETH_ADDR_FMT",dst="ETH_ADDR_FMT")",
+                      ETH_ADDR_ARGS(eth->addresses.eth_src),
+                      ETH_ADDR_ARGS(eth->addresses.eth_dst));
+        break;
+    }
+    case OVS_ACTION_ATTR_POP_ETH:
+        ds_put_cstr(ds, "pop_eth");
+        break;
     case OVS_ACTION_ATTR_PUSH_VLAN: {
         const struct ovs_action_push_vlan *vlan = nl_attr_get(a);
         ds_put_cstr(ds, "push_vlan(");
@@ -1015,14 +1027,39 @@  parse_odp_userspace_action(const char *s, struct ofpbuf *actions)
             odp_put_userspace_action(pid, user_data, user_data_size,
                                      tunnel_out_port, include_actions, actions);
             res = n + n1;
+            goto out;
         } else if (s[n] == ')') {
             odp_put_userspace_action(pid, user_data, user_data_size,
                                      ODPP_NONE, include_actions, actions);
             res = n + 1;
-        } else {
-            res = -EINVAL;
+            goto out;
+        }
+    }
+
+    {
+        struct ovs_action_push_eth push;
+        int n1 = -1;
+
+        if (ovs_scan(&s[n], "push_eth(src="ETH_ADDR_SCAN_FMT","
+                     "dst="ETH_ADDR_SCAN_FMT")%n",
+                     ETH_ADDR_SCAN_ARGS(push.addresses.eth_src),
+                     ETH_ADDR_SCAN_ARGS(push.addresses.eth_dst), &n1)) {
+
+            nl_msg_put_unspec(actions, OVS_ACTION_ATTR_PUSH_ETH,
+                              &push, sizeof push);
+
+            res = n + n1;
+            goto out;
         }
     }
+
+    if (!strncmp(&s[n], "pop_eth", 7)) {
+        nl_msg_put_flag(actions, OVS_ACTION_ATTR_POP_ETH);
+        res = 7;
+        goto out;
+    }
+
+    res = -EINVAL;
 out:
     ofpbuf_uninit(&buf);
     return res;
@@ -5307,6 +5344,26 @@  odp_put_userspace_action(uint32_t pid,
     return userdata_ofs;
 }
 
+static void
+odp_put_pop_eth_action(struct ofpbuf *odp_actions)
+{
+    nl_msg_put_flag(odp_actions, OVS_ACTION_ATTR_POP_ETH);
+}
+
+static void
+odp_put_push_eth_action(struct ofpbuf *odp_actions,
+                        const struct eth_addr *eth_src,
+                        const struct eth_addr *eth_dst)
+{
+    struct ovs_action_push_eth eth;
+
+    eth.addresses.eth_src = *eth_src;
+    eth.addresses.eth_dst = *eth_dst;
+
+    nl_msg_put_unspec(odp_actions, OVS_ACTION_ATTR_PUSH_ETH,
+                      &eth, sizeof eth);
+}
+
 void
 odp_put_tunnel_action(const struct flow_tnl *tunnel,
                       struct ofpbuf *odp_actions)
@@ -5440,6 +5497,26 @@  commit_set_ether_addr_action(const struct flow *flow, struct flow *base_flow,
 }
 
 static void
+commit_ether_action(const struct flow *flow, struct flow *base_flow,
+                    struct ofpbuf *odp_actions, struct flow_wildcards *wc,
+                    bool use_masked)
+{
+    if (flow->base_layer != base_flow->base_layer) {
+        if (flow->base_layer == LAYER_2) {
+            odp_put_push_eth_action(odp_actions, &flow->dl_src, &flow->dl_dst);
+            base_flow->dl_src = flow->dl_src;
+            base_flow->dl_dst = flow->dl_dst;
+        } else {
+            odp_put_pop_eth_action(odp_actions);
+        }
+        base_flow->base_layer =  flow->base_layer;
+    }
+
+    commit_set_ether_addr_action(flow, base_flow, odp_actions, wc,
+                                 use_masked);
+}
+
+static void
 pop_vlan(struct flow *base,
          struct ofpbuf *odp_actions, struct flow_wildcards *wc)
 {
@@ -5896,7 +5973,7 @@  commit_odp_actions(const struct flow *flow, struct flow *base,
 {
     enum slow_path_reason slow1, slow2;
 
-    commit_set_ether_addr_action(flow, base, odp_actions, wc, use_masked);
+    commit_ether_action(flow, base, odp_actions, wc, use_masked);
     slow1 = commit_set_nw_action(flow, base, odp_actions, wc, use_masked);
     commit_set_port_action(flow, base, odp_actions, wc, use_masked);
     slow2 = commit_set_icmp_action(flow, base, odp_actions, wc);
diff --git a/lib/packets.c b/lib/packets.c
index 990c407c51f9..cffb3db93abb 100644
--- a/lib/packets.c
+++ b/lib/packets.c
@@ -225,6 +225,30 @@  eth_pop_vlan(struct dp_packet *packet)
     }
 }
 
+/* Push Ethernet header onto 'packet' assuming it is layer 3 */
+void
+push_eth(struct dp_packet *packet, const struct eth_addr *dst,
+         const struct eth_addr *src)
+{
+    struct eth_header *eh;
+
+    eh = dp_packet_resize_l2(packet, ETH_HEADER_LEN);
+    eh->eth_dst = *dst;
+    eh->eth_src = *src;
+}
+
+/* Removes Ethernet header, including all VLAN and MPLS headers, from 'packet'.
+ *
+ * Previous to calling this function, 'ofpbuf_l3(packet)' must not be NULL */
+void
+pop_eth(struct dp_packet *packet)
+{
+    ovs_assert(dp_packet_l3(packet) != NULL);
+
+    dp_packet_resize_l2_5(packet, -packet->l3_ofs);
+    dp_packet_set_l2_5(packet, NULL);
+}
+
 /* Set ethertype of the packet. */
 static void
 set_ethertype(struct dp_packet *packet, ovs_be16 eth_type)
diff --git a/lib/packets.h b/lib/packets.h
index 21bd35c09dcd..e12a46a14bc5 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -366,6 +366,10 @@  struct eth_header {
 });
 BUILD_ASSERT_DECL(ETH_HEADER_LEN == sizeof(struct eth_header));
 
+void push_eth(struct dp_packet *packet, const struct eth_addr *dst,
+              const struct eth_addr *src);
+void pop_eth(struct dp_packet *packet);
+
 #define LLC_DSAP_SNAP 0xaa
 #define LLC_SSAP_SNAP 0xaa
 #define LLC_CNTL_SNAP 3
diff --git a/ofproto/ofproto-dpif-sflow.c b/ofproto/ofproto-dpif-sflow.c
index 9ea8851419b3..5ba5ec933554 100644
--- a/ofproto/ofproto-dpif-sflow.c
+++ b/ofproto/ofproto-dpif-sflow.c
@@ -1161,6 +1161,13 @@  dpif_sflow_read_actions(const struct flow *flow,
 	    dpif_sflow_pop_mpls_lse(sflow_actions);
 	    break;
 	}
+	case OVS_ACTION_ATTR_PUSH_ETH:
+	case OVS_ACTION_ATTR_POP_ETH:
+	    /* TODO: SFlow does not currently define a MAC-in-MAC
+	     * encapsulation structure.  We could use an extension
+	     * structure to report this.
+	     */
+	    break;
 	case OVS_ACTION_ATTR_SAMPLE:
 	case OVS_ACTION_ATTR_UNSPEC:
 	case __OVS_ACTION_ATTR_MAX: