meta-flow: Remove cmap dependency.

Previous patch 04f48a68 ("ofp-actions: Fix variable length meta-flow OXMs.")
introduced dependency of an internal library (cmap.h) to ovs public
interface (meta-flow.h) that may cause potential building problem. In this
patch, we remove cmap from struct mf_field, and provide a wrapper struct
vl_mff_map that resolve the dependency problem.

Fixes: 04f48a68c428 ("ofp-actions: Fix variable length meta-flow OXMs.")
Suggested-by: Joe Stringer <joe@ovn.org>
Suggested-by: Daniele Di Proietto <diproiettod@vmware.com>
Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com>
Signed-off-by: Joe Stringer <joe@ovn.org>

diff --git a/build-aux/extract-ofp-fields b/build-aux/extract-ofp-fields
index 40f1bb2873559356c1b3200b081f3177604977bd..498b887731eba3a48aff42e16deae2b5c30cebf7 100755 (executable)
--- a/build-aux/extract-ofp-fields
+++ b/build-aux/extract-ofp-fields
@@ -386,7 +386,7 @@ def make_meta_flow(meta_flow_h):
         else:
             output += ["    -1, /* not usable for prefix lookup */"]
 
-        output += ["    {OVSRCU_INITIALIZER(NULL)},},"]
+        output += ["},"]
     for oline in output:
         print(oline)
 

diff --git a/include/openvswitch/meta-flow.h b/include/openvswitch/meta-flow.h
index d5c097179d22ad9a53a41ad45cb75ada2e4ec179..83e2599330cee4919e609050554b1e0dd8d3099e 100644 (file)
--- a/include/openvswitch/meta-flow.h
+++ b/include/openvswitch/meta-flow.h
@@ -23,11 +23,9 @@
 #include <sys/types.h>
 #include <netinet/in.h>
 #include <netinet/ip6.h>
-#include "cmap.h"
 #include "openvswitch/flow.h"
 #include "openvswitch/ofp-errors.h"
 #include "openvswitch/packets.h"
-#include "openvswitch/thread.h"
 #include "openvswitch/util.h"
 
 struct ds;
@@ -1774,9 +1772,6 @@ struct mf_field {
 
     int flow_be32ofs;  /* Field's be32 offset in "struct flow", if prefix tree
                         * lookup is supported for the field, or -1. */
-
-    /* For variable length mf_fields only. In ofproto->vl_mff_map->cmap. */
-    struct cmap_node cmap_node;
 };
 
 /* The representation of a field's value. */
@@ -1853,14 +1848,6 @@ union mf_subvalue {
 };
 BUILD_ASSERT_DECL(sizeof(union mf_value) == sizeof (union mf_subvalue));
 
-/* Variable length mf_fields mapping map. This is a single writer,
- * multiple-reader hash table that a writer must hold the following mutex
- * to access this map. */
-struct vl_mff_map {
-    struct cmap cmap;       /* Contains 'struct mf_field' */
-    struct ovs_mutex mutex;
-};
-
 bool mf_subvalue_intersect(const union mf_subvalue *a_value,
                            const union mf_subvalue *a_mask,
                            const union mf_subvalue *b_value,
@@ -1987,14 +1974,4 @@ void mf_format_subvalue(const union mf_subvalue *subvalue, struct ds *s);
 /* Field Arrays. */
 void field_array_set(enum mf_field_id id, const union mf_value *,
                      struct field_array *);
-
-/* Variable length fields. */
-void mf_vl_mff_map_clear(struct vl_mff_map *vl_mff_map)
-    OVS_REQUIRES(vl_mff_map->mutex);
-enum ofperr mf_vl_mff_map_mod_from_tun_metadata(
-    struct vl_mff_map *vl_mff_map, const struct ofputil_tlv_table_mod *)
-    OVS_REQUIRES(vl_mff_map->mutex);
-const struct mf_field * mf_get_vl_mff(const struct mf_field *,
-                                      const struct vl_mff_map *);
-bool mf_vl_mff_invalid(const struct mf_field *, const struct vl_mff_map *);
 #endif /* meta-flow.h */

diff --git a/include/openvswitch/ofp-actions.h b/include/openvswitch/ofp-actions.h
index a3783c2b8a08c51d396a3b10ec24cbdadaf8f39d..88f573dcd74eea6f228ed6704cf74eed20b5552e 100644 (file)
--- a/include/openvswitch/ofp-actions.h
+++ b/include/openvswitch/ofp-actions.h
@@ -26,6 +26,8 @@
 #include "openvswitch/ofp-errors.h"
 #include "openvswitch/types.h"
 
+struct vl_mff_map;
+
 /* List of OVS abstracted actions.
  *
  * This macro is used directly only internally by this header, but the list is

diff --git a/include/openvswitch/ofp-util.h b/include/openvswitch/ofp-util.h
index dd254e6ebd184ddf22d34844a19041a740479eaa..0c3a10aa426483b1d6634d2a6cec10e9b5657f68 100644 (file)
--- a/include/openvswitch/ofp-util.h
+++ b/include/openvswitch/ofp-util.h
@@ -36,6 +36,7 @@
 struct ofpbuf;
 union ofp_action;
 struct ofpact_set_field;
+struct vl_mff_map;
 
 /* Port numbers. */
 enum ofperr ofputil_port_from_ofp11(ovs_be32 ofp11_port,

diff --git a/lib/automake.mk b/lib/automake.mk
index abc9d0d5cc4e1bfb6cacd423b96b76e9307996ec..b266af13e4c70439d3b3f8b3d4dfae71575d9163 100644 (file)
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -284,6 +284,7 @@ lib_libopenvswitch_la_SOURCES = \
        lib/vconn-stream.c \
        lib/vconn.c \
        lib/versions.h \
+       lib/vl-mff-map.h \
        lib/vlan-bitmap.c \
        lib/vlan-bitmap.h \
        lib/vlog.c \

diff --git a/lib/meta-flow.c b/lib/meta-flow.c
index b92950bd51715dbf3de28106ddc8360971abc433..40704e628aaa02a5719d7bb3f964d3dfd1e27fbf 100644 (file)
--- a/lib/meta-flow.c
+++ b/lib/meta-flow.c
@@ -38,6 +38,7 @@
 #include "util.h"
 #include "openvswitch/ofp-errors.h"
 #include "openvswitch/vlog.h"
+#include "vl-mff-map.h"
 
 VLOG_DEFINE_THIS_MODULE(meta_flow);
 
@@ -2641,6 +2642,13 @@ field_array_set(enum mf_field_id id, const union mf_value *value,
     memcpy(fa->values + offset, value, value_size);
 }
 
+/* A wrapper for variable length mf_fields that is maintained by
+ * struct vl_mff_map.*/
+struct vl_mf_field {
+    struct mf_field mf;
+    struct cmap_node cmap_node; /* In ofproto->vl_mff_map->cmap. */
+};
+
 static inline uint32_t
 mf_field_hash(uint32_t key)
 {
@@ -2651,23 +2659,24 @@ void
 mf_vl_mff_map_clear(struct vl_mff_map *vl_mff_map)
     OVS_REQUIRES(vl_mff_map->mutex)
 {
-    struct mf_field *mf;
+    struct vl_mf_field *vmf;
 
-    CMAP_FOR_EACH (mf, cmap_node, &vl_mff_map->cmap) {
-        cmap_remove(&vl_mff_map->cmap, &mf->cmap_node, mf_field_hash(mf->id));
-        ovsrcu_postpone(free, mf);
+    CMAP_FOR_EACH (vmf, cmap_node, &vl_mff_map->cmap) {
+        cmap_remove(&vl_mff_map->cmap, &vmf->cmap_node,
+                    mf_field_hash(vmf->mf.id));
+        ovsrcu_postpone(free, vmf);
     }
 }
 
-static struct mf_field *
+static struct vl_mf_field *
 mf_get_vl_mff__(uint32_t id, const struct vl_mff_map *vl_mff_map)
 {
-    struct mf_field *field;
+    struct vl_mf_field *vmf;
 
-    CMAP_FOR_EACH_WITH_HASH (field, cmap_node, mf_field_hash(id),
+    CMAP_FOR_EACH_WITH_HASH (vmf, cmap_node, mf_field_hash(id),
                              &vl_mff_map->cmap) {
-        if (field->id == id) {
-            return field;
+        if (vmf->mf.id == id) {
+            return vmf;
         }
     }
 
@@ -2682,7 +2691,7 @@ mf_get_vl_mff(const struct mf_field *mff,
               const struct vl_mff_map *vl_mff_map)
 {
     if (mff && mff->variable_len && vl_mff_map) {
-        return mf_get_vl_mff__(mff->id, vl_mff_map);
+        return &mf_get_vl_mff__(mff->id, vl_mff_map)->mf;
     }
 
     return NULL;
@@ -2704,7 +2713,7 @@ mf_vl_mff_map_mod_from_tun_metadata(struct vl_mff_map *vl_mff_map,
 
     LIST_FOR_EACH (tlv_map, list_node, &ttm->mappings) {
         unsigned int idx = MFF_TUN_METADATA0 + tlv_map->index;
-        struct mf_field *mf;
+        struct vl_mf_field *vmf;
 
         if (idx >= MFF_TUN_METADATA0 + TUN_METADATA_NUM_OPTS) {
             return OFPERR_NXTTMFC_BAD_FIELD_IDX;
@@ -2712,21 +2721,22 @@ mf_vl_mff_map_mod_from_tun_metadata(struct vl_mff_map *vl_mff_map,
 
         switch (ttm->command) {
         case NXTTMC_ADD:
-            mf = xmalloc(sizeof *mf);
-            *mf = mf_fields[idx];
-            mf->n_bytes = tlv_map->option_len;
-            mf->n_bits = tlv_map->option_len * 8;
-            mf->mapped = true;
-
-            cmap_insert(&vl_mff_map->cmap, &mf->cmap_node, mf_field_hash(idx));
+            vmf = xmalloc(sizeof *vmf);
+            vmf->mf = mf_fields[idx];
+            vmf->mf.n_bytes = tlv_map->option_len;
+            vmf->mf.n_bits = tlv_map->option_len * 8;
+            vmf->mf.mapped = true;
+
+            cmap_insert(&vl_mff_map->cmap, &vmf->cmap_node,
+                        mf_field_hash(idx));
             break;
 
         case NXTTMC_DELETE:
-            mf = mf_get_vl_mff__(idx, vl_mff_map);
-            if (mf) {
-                cmap_remove(&vl_mff_map->cmap, &mf->cmap_node,
+            vmf = mf_get_vl_mff__(idx, vl_mff_map);
+            if (vmf) {
+                cmap_remove(&vl_mff_map->cmap, &vmf->cmap_node,
                             mf_field_hash(idx));
-                ovsrcu_postpone(free, mf);
+                ovsrcu_postpone(free, vmf);
             }
             break;
 

diff --git a/lib/nx-match.c b/lib/nx-match.c
index e9d649bf74bdbb3c023527f462fe1f43e1de7fab..91401e2201c6e53ba85b63054a9b6aa92576a0d1 100644 (file)
--- a/lib/nx-match.c
+++ b/lib/nx-match.c
@@ -36,6 +36,7 @@
 #include "tun-metadata.h"
 #include "unaligned.h"
 #include "util.h"
+#include "vl-mff-map.h"
 
 VLOG_DEFINE_THIS_MODULE(nx_match);
 

diff --git a/lib/nx-match.h b/lib/nx-match.h
index c3432d70eef4c725e2b8085c06bfac2bb661ada4..5dca24a01a4974c3e36b8f1dbeb922a28eeb0d5e 100644 (file)
--- a/lib/nx-match.h
+++ b/lib/nx-match.h
@@ -34,6 +34,7 @@ struct ofpact_stack;
 struct ofpbuf;
 struct nx_action_reg_load;
 struct nx_action_reg_move;
+struct vl_mff_map;
 
 
 /* Nicira Extended Match (NXM) flexible flow match helper functions.

diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index 11ab6921763b04ee54ed08d07b2a0a3ad3670dd5..ce80f57e8df3b4436565c1f416ea0287d5589c9c 100644 (file)
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -37,6 +37,7 @@
 #include "openvswitch/vlog.h"
 #include "unaligned.h"
 #include "util.h"
+#include "vl-mff-map.h"
 
 VLOG_DEFINE_THIS_MODULE(ofp_actions);
 

diff --git a/lib/vl-mff-map.h b/lib/vl-mff-map.h
new file mode 100644 (file)
index 0000000..1c29385
--- /dev/null
+++ b/lib/vl-mff-map.h
@@ -0,0 +1,41 @@
+/*
+ * Copyright (c) 2017 Nicira, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef VL_MFF_MAP_H
+#define VL_MFF_MAP_H 1
+
+#include "cmap.h"
+#include "openvswitch/thread.h"
+
+/* Variable length mf_fields mapping map. This is a single writer,
+ * multiple-reader hash table that a writer must hold the following mutex
+ * to access this map. */
+struct vl_mff_map {
+    struct cmap cmap;       /* Contains 'struct mf_field' */
+    struct ovs_mutex mutex;
+};
+
+/* Variable length fields. */
+void mf_vl_mff_map_clear(struct vl_mff_map *vl_mff_map)
+    OVS_REQUIRES(vl_mff_map->mutex);
+enum ofperr mf_vl_mff_map_mod_from_tun_metadata(
+    struct vl_mff_map *vl_mff_map, const struct ofputil_tlv_table_mod *)
+    OVS_REQUIRES(vl_mff_map->mutex);
+const struct mf_field * mf_get_vl_mff(const struct mf_field *,
+                                      const struct vl_mff_map *);
+bool mf_vl_mff_invalid(const struct mf_field *, const struct vl_mff_map *);
+
+#endif /* vl-mff-map.h */

diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 4808974b64f39fe0683b4f9fad6e3b4e5f00f73b..7d3e929f21e34fb4cedd79e9ac8d588e931736ee 100644 (file)
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -52,6 +52,7 @@
 #include "timeval.h"
 #include "tun-metadata.h"
 #include "versions.h"
+#include "vl-mff-map.h"
 
 struct match;
 struct ofputil_flow_mod;
@@ -59,7 +60,6 @@ struct bfd_cfg;
 struct meter;
 struct ofoperation;
 struct ofproto_packet_out;
-struct vl_mff_map;
 struct smap;
 
 extern struct ovs_mutex ofproto_mutex;