diff mbox

[ovs-dev,v10,3/5] userspace: add layer 3 support to packet metadata

Message ID 1462347265-21887-4-git-send-email-simon.horman@netronome.com
State Changes Requested
Headers show

Commit Message

Simon Horman May 4, 2016, 7:34 a.m. UTC 
From: Lorand Jakab <lojakab@cisco.com>

This is needed for sending a packet back to the datapath after a miss
upcall was processed.  The presence of a layer 2 packet is signaled by
adding OVS_KEY_ATTR_ETHERNET to the packet metadata sent with the
ovs_packet netlink message.  Layer 3 packets need to be accompanied by
OVS_KEY_ATTR_PACKET_ETHERTYPE to indicate network protocol.

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

---
v10 [Simon Horman]
* Update BUILD_BUG_ON() call in ovs_key_attr_size()
* Set packet_ethtype in set_ethertype()
* Do not include zero-value OVS_KEY_ATTR_PACKET_ETHERTYPE attribute in flow key
  of layer 2 packets as this creates an unnecessary incompatibility with
  kernel datapaths that are unaware of that attribute.

v9 [Simon Horman]
* Rebase

v8 - v8 [Lorand Jakab]

v7 [Lorand Jakab]
* New patch
---
 datapath/flow_netlink.c                           |  2 +-
 datapath/linux/compat/include/linux/openvswitch.h |  2 ++
 include/openvswitch/match.h                       |  1 +
 lib/flow.c                                        |  4 ++++
 lib/flow.h                                        |  2 ++
 lib/match.c                                       |  7 +++++++
 lib/odp-execute.c                                 |  9 ++++++++
 lib/odp-util.c                                    | 25 ++++++++++++++++++++++-
 lib/packets.c                                     |  1 +
 lib/packets.h                                     |  2 ++
 ofproto/ofproto-dpif-sflow.c                      |  1 +
 11 files changed, 54 insertions(+), 2 deletions(-)

Comments

Ben Pfaff May 31, 2016, 8:52 p.m. UTC | #1 
On Wed, May 04, 2016 at 04:34:23PM +0900, Simon Horman wrote:
> From: Lorand Jakab <lojakab@cisco.com>
> 
> This is needed for sending a packet back to the datapath after a miss
> upcall was processed.  The presence of a layer 2 packet is signaled by
> adding OVS_KEY_ATTR_ETHERNET to the packet metadata sent with the
> ovs_packet netlink message.  Layer 3 packets need to be accompanied by
> OVS_KEY_ATTR_PACKET_ETHERTYPE to indicate network protocol.
> 
> Signed-off-by: Lorand Jakab <lojakab@cisco.com>
> Signed-off-by: Simon Horman <simon.horman@netronome.com>

I am not sure it makes sense to do a masked set of a packet Ethertype
(in odp_execute_masked_set_action()).

Acked-by: Ben Pfaff <blp@ovn.org>
Simon Horman June 1, 2016, 2:19 a.m. UTC | #2 
On Tue, May 31, 2016 at 01:52:55PM -0700, Ben Pfaff wrote:
> On Wed, May 04, 2016 at 04:34:23PM +0900, Simon Horman wrote:
> > From: Lorand Jakab <lojakab@cisco.com>
> > 
> > This is needed for sending a packet back to the datapath after a miss
> > upcall was processed.  The presence of a layer 2 packet is signaled by
> > adding OVS_KEY_ATTR_ETHERNET to the packet metadata sent with the
> > ovs_packet netlink message.  Layer 3 packets need to be accompanied by
> > OVS_KEY_ATTR_PACKET_ETHERTYPE to indicate network protocol.
> > 
> > Signed-off-by: Lorand Jakab <lojakab@cisco.com>
> > Signed-off-by: Simon Horman <simon.horman@netronome.com>
> 
> I am not sure it makes sense to do a masked set of a packet Ethertype
> (in odp_execute_masked_set_action()).

Thanks. I'm not sure that we need to allow set of packet Ethernet at all.
It does seem that its not necessary to do something useful with l3 tunnels
at this time. So I've removed it for now.

> Acked-by: Ben Pfaff <blp@ovn.org>

Thanks!
diff mbox

Patch

diff --git a/datapath/flow_netlink.c b/datapath/flow_netlink.c
index 6ffcc53ea1ca..c7dfb677fb86 100644
--- a/datapath/flow_netlink.c
+++ b/datapath/flow_netlink.c
@@ -284,7 +284,7 @@  size_t ovs_key_attr_size(void)
 	/* Whenever adding new OVS_KEY_ FIELDS, we should consider
 	 * updating this function.
 	 */
-	BUILD_BUG_ON(OVS_KEY_ATTR_TUNNEL_INFO != 26);
+	BUILD_BUG_ON(OVS_KEY_ATTR_TUNNEL_INFO != 27);
 
 	return    nla_total_size(4)   /* OVS_KEY_ATTR_PRIORITY */
 		+ nla_total_size(0)   /* OVS_KEY_ATTR_TUNNEL */
diff --git a/datapath/linux/compat/include/linux/openvswitch.h b/datapath/linux/compat/include/linux/openvswitch.h
index 8548d3b8b86b..502e8f1aca66 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -352,6 +352,8 @@  enum ovs_key_attr {
 	OVS_KEY_ATTR_CT_ZONE,	/* u16 connection tracking zone. */
 	OVS_KEY_ATTR_CT_MARK,	/* u32 connection tracking mark */
 	OVS_KEY_ATTR_CT_LABELS,	/* 16-octet connection tracking labels */
+	OVS_KEY_ATTR_PACKET_ETHERTYPE, /* be16 Ethernet type for packet
+					* execution. */
 
 #ifdef __KERNEL__
 	/* Only used within kernel data path. */
diff --git a/include/openvswitch/match.h b/include/openvswitch/match.h
index c955753d70a9..60da412eb695 100644
--- a/include/openvswitch/match.h
+++ b/include/openvswitch/match.h
@@ -96,6 +96,7 @@  void match_set_ct_mark(struct match *, uint32_t ct_mark);
 void match_set_ct_mark_masked(struct match *, uint32_t ct_mark, uint32_t mask);
 void match_set_ct_label(struct match *, ovs_u128 ct_label);
 void match_set_ct_label_masked(struct match *, ovs_u128 ct_label, ovs_u128 mask);
+void match_set_base_layer(struct match *, uint8_t base_layer);
 void match_set_skb_priority(struct match *, uint32_t skb_priority);
 void match_set_dl_type(struct match *, ovs_be16);
 void match_set_dl_src(struct match *, const struct eth_addr );
diff --git a/lib/flow.c b/lib/flow.c
index 566dbd0feb06..3896531c0936 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -933,6 +933,10 @@  flow_get_metadata(const struct flow *flow, struct match *flow_metadata)
     if (!ovs_u128_is_zero(&flow->ct_label)) {
         match_set_ct_label(flow_metadata, flow->ct_label);
     }
+
+    if (flow->base_layer != LAYER_2) {
+        match_set_base_layer(flow_metadata, flow->base_layer);
+    }
 }
 
 const char *ct_state_to_string(uint32_t state)
diff --git a/lib/flow.h b/lib/flow.h
index 68810b9a35cb..95a8a5a1b198 100644
--- a/lib/flow.h
+++ b/lib/flow.h
@@ -828,6 +828,8 @@  pkt_metadata_from_flow(struct pkt_metadata *md, const struct flow *flow)
     md->ct_zone = flow->ct_zone;
     md->ct_mark = flow->ct_mark;
     md->ct_label = flow->ct_label;
+    md->base_layer = flow->base_layer;
+    md->packet_ethertype = flow->dl_type;
 }
 
 static inline bool is_ip_any(const struct flow *flow)
diff --git a/lib/match.c b/lib/match.c
index fd22c1962aac..27fdd0071846 100644
--- a/lib/match.c
+++ b/lib/match.c
@@ -369,6 +369,13 @@  match_set_ct_label_masked(struct match *match, ovs_u128 value, ovs_u128 mask)
 }
 
 void
+match_set_base_layer(struct match *match, uint8_t base_layer)
+{
+    match->flow.base_layer = base_layer;
+    match->wc.masks.base_layer = UINT8_MAX;
+}
+
+void
 match_set_dl_type(struct match *match, ovs_be16 dl_type)
 {
     match->wc.masks.dl_type = OVS_BE16_MAX;
diff --git a/lib/odp-execute.c b/lib/odp-execute.c
index 4befde2427fc..579f323a1e82 100644
--- a/lib/odp-execute.c
+++ b/lib/odp-execute.c
@@ -326,6 +326,10 @@  odp_execute_set_action(struct dp_packet *packet, const struct nlattr *a)
         md->recirc_id = nl_attr_get_u32(a);
         break;
 
+    case OVS_KEY_ATTR_PACKET_ETHERTYPE:
+        md->packet_ethertype = nl_attr_get_be16(a);
+        break;
+
     case OVS_KEY_ATTR_UNSPEC:
     case OVS_KEY_ATTR_ENCAP:
     case OVS_KEY_ATTR_ETHERTYPE:
@@ -422,6 +426,11 @@  odp_execute_masked_set_action(struct dp_packet *packet,
             | (md->recirc_id & ~*get_mask(a, uint32_t));
         break;
 
+    case OVS_KEY_ATTR_PACKET_ETHERTYPE:
+        md->packet_ethertype = nl_attr_get_be16(a)
+            | (md->packet_ethertype & ~*get_mask(a, ovs_be16));
+        break;
+
     case OVS_KEY_ATTR_TUNNEL:    /* Masked data not supported for tunnel. */
     case OVS_KEY_ATTR_UNSPEC:
     case OVS_KEY_ATTR_CT_STATE:
diff --git a/lib/odp-util.c b/lib/odp-util.c
index bc54496b04c4..0345963a5f17 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -165,6 +165,7 @@  ovs_key_attr_to_string(enum ovs_key_attr attr, char *namebuf, size_t bufsize)
     case OVS_KEY_ATTR_MPLS: return "mpls";
     case OVS_KEY_ATTR_DP_HASH: return "dp_hash";
     case OVS_KEY_ATTR_RECIRC_ID: return "recirc_id";
+    case OVS_KEY_ATTR_PACKET_ETHERTYPE: return "pkt_eth";
 
     case __OVS_KEY_ATTR_MAX:
     default:
@@ -1819,6 +1820,7 @@  static const struct attr_len_tbl ovs_flow_key_attr_lens[OVS_KEY_ATTR_MAX + 1] =
     [OVS_KEY_ATTR_CT_ZONE]   = { .len = 2 },
     [OVS_KEY_ATTR_CT_MARK]   = { .len = 4 },
     [OVS_KEY_ATTR_CT_LABELS] = { .len = sizeof(struct ovs_key_ct_labels) },
+    [OVS_KEY_ATTR_PACKET_ETHERTYPE] = { .len = 2 },
 };
 
 /* Returns the correct length of the payload for a flow key attribute of the
@@ -2839,6 +2841,7 @@  format_odp_key_attr(const struct nlattr *a, const struct nlattr *ma,
         break;
     }
     case OVS_KEY_ATTR_ETHERTYPE:
+    case OVS_KEY_ATTR_PACKET_ETHERTYPE:
         ds_put_format(ds, "0x%04"PRIx16, ntohs(nl_attr_get_be16(a)));
         if (!is_exact) {
             ds_put_format(ds, "/0x%04"PRIx16, ntohs(nl_attr_get_be16(ma)));
@@ -4520,6 +4523,11 @@  odp_key_from_pkt_metadata(struct ofpbuf *buf, const struct pkt_metadata *md)
     if (md->in_port.odp_port != ODPP_NONE) {
         nl_msg_put_odp_port(buf, OVS_KEY_ATTR_IN_PORT, md->in_port.odp_port);
     }
+
+    if (md->base_layer == LAYER_3) {
+        nl_msg_put_be16(buf, OVS_KEY_ATTR_PACKET_ETHERTYPE,
+                        md->packet_ethertype);
+    }
 }
 
 /* Generate packet metadata from the given ODP flow key. */
@@ -4531,10 +4539,13 @@  odp_key_to_pkt_metadata(const struct nlattr *key, size_t key_len,
     size_t left;
     uint32_t wanted_attrs = 1u << OVS_KEY_ATTR_PRIORITY |
         1u << OVS_KEY_ATTR_SKB_MARK | 1u << OVS_KEY_ATTR_TUNNEL |
-        1u << OVS_KEY_ATTR_IN_PORT;
+        1u << OVS_KEY_ATTR_IN_PORT | 1u << OVS_KEY_ATTR_ETHERNET |
+        1u << OVS_KEY_ATTR_IPV4 | 1u << OVS_KEY_ATTR_IPV6;
 
     pkt_metadata_init(md, ODPP_NONE);
 
+    md->base_layer = LAYER_3;
+
     NL_ATTR_FOR_EACH (nla, left, key, key_len) {
         uint16_t type = nl_attr_type(nla);
         size_t len = nl_attr_get_size(nla);
@@ -4596,6 +4607,18 @@  odp_key_to_pkt_metadata(const struct nlattr *key, size_t key_len,
             md->in_port.odp_port = nl_attr_get_odp_port(nla);
             wanted_attrs &= ~(1u << OVS_KEY_ATTR_IN_PORT);
             break;
+        case OVS_KEY_ATTR_ETHERNET:
+            md->base_layer = LAYER_2;
+            wanted_attrs &= ~(1u << OVS_KEY_ATTR_ETHERNET);
+            break;
+        case OVS_KEY_ATTR_IPV4:
+            md->packet_ethertype = htons(ETH_TYPE_IP);
+            wanted_attrs &= ~(1u << OVS_KEY_ATTR_IPV4);
+            break;
+        case OVS_KEY_ATTR_IPV6:
+            md->packet_ethertype = htons(ETH_TYPE_IPV6);
+            wanted_attrs &= ~(1u << OVS_KEY_ATTR_IPV6);
+            break;
         default:
             break;
         }
diff --git a/lib/packets.c b/lib/packets.c
index 277b2445f2a0..b9e2182827cb 100644
--- a/lib/packets.c
+++ b/lib/packets.c
@@ -252,6 +252,7 @@  set_ethertype(struct dp_packet *packet, ovs_be16 eth_type)
     struct eth_header *eh = dp_packet_l2(packet);
 
     if (!eh) {
+        packet->md.packet_ethertype = eth_type;
         return;
     }
 
diff --git a/lib/packets.h b/lib/packets.h
index 97528f931b2c..560eaa729fcb 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -104,6 +104,8 @@  struct pkt_metadata {
     uint32_t ct_mark;           /* Connection mark. */
     ovs_u128 ct_label;          /* Connection label. */
     union flow_in_port in_port; /* Input port. */
+    ovs_be16 packet_ethertype;  /* Ethertype of the packet */
+    uint8_t base_layer;         /* Packet starts at this layer */
     struct flow_tnl tunnel;     /* Encapsulating tunnel parameters. Note that
                                  * if 'ip_dst' == 0, the rest of the fields may
                                  * be uninitialized. */
diff --git a/ofproto/ofproto-dpif-sflow.c b/ofproto/ofproto-dpif-sflow.c
index 80cb37aa0104..29808517aeb3 100644
--- a/ofproto/ofproto-dpif-sflow.c
+++ b/ofproto/ofproto-dpif-sflow.c
@@ -976,6 +976,7 @@  sflow_read_set_action(const struct nlattr *attr,
     case OVS_KEY_ATTR_IN_PORT:
     case OVS_KEY_ATTR_ETHERNET:
     case OVS_KEY_ATTR_VLAN:
+    case OVS_KEY_ATTR_PACKET_ETHERTYPE:
         break;
 
     case OVS_KEY_ATTR_MPLS: {