[ovs-dev] [PATCH 2/2] datapath: Better calculate max nlattr-formatted flow size.

[Justin Pettit]

Both userspace and the kernel allocate space based on the max size of a
nlattr-formatted flow.  It was easy to change the max size of a flow
definition and cause crashes by forgetting to update one or both of
those definitions.  This commit attempts to make that harder by
providing a better description of how the max size is calculated and
defining that size in datapath-protocol.h and have the others use it.
---
 datapath/flow.h                         |    6 +++---
 include/openvswitch/datapath-protocol.h |   27 +++++++++++++++++++++++++++
 lib/odp-util.h                          |    6 +++---
 3 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/datapath/flow.h b/datapath/flow.h
index c003393..26bc8cd 100644
--- a/datapath/flow.h
+++ b/datapath/flow.h
@@ -107,10 +107,10 @@ u64 flow_used_time(unsigned long flow_jiffies);
 u32 flow_hash(const struct sw_flow_key *);
 int flow_cmp(const struct tbl_node *, void *target);
 
-/* By my calculations currently the longest valid nlattr-formatted flow key is
- * 132 bytes long, so this leaves some safety margin.
+/* The longest valid nlattr-formatted flow key is ODP_KEY_MAX_LEN, so
+ * this leaves some safety margin.
  */
-#define FLOW_BUFSIZE 148
+#define FLOW_BUFSIZE (ODP_KEY_MAX_LEN + 16)
 
 int flow_to_nlattrs(const struct sw_flow_key *, struct sk_buff *);
 int flow_from_nlattrs(struct sw_flow_key *swkey, const struct nlattr *);
diff --git a/include/openvswitch/datapath-protocol.h b/include/openvswitch/datapath-protocol.h
index e5bbc6a..cb4337b 100644
--- a/include/openvswitch/datapath-protocol.h
+++ b/include/openvswitch/datapath-protocol.h
@@ -308,6 +308,33 @@ enum odp_key_type {
 
 #define ODP_KEY_ATTR_MAX (__ODP_KEY_ATTR_MAX - 1)
 
+/* Upper bound on the length of a nlattr-formatted flow key.  The longest
+ * nlattr-formatted flow key would be:
+ *
+ *                         struct  pad  nl hdr  total
+ *                         ------  ---  ------  -----
+ *  ODP_KEY_ATTR_TUN_ID        8    --     4     12 
+ *  ODP_KEY_ATTR_IN_PORT       2     2     4      8
+ *  ODP_KEY_ATTR_ETHERNET     12    --     4     16
+ *  ODP_KEY_ATTR_8021Q         4    --     4      8
+ *  ODP_KEY_ATTR_ETHERTYPE     2     2     4      8
+ *  ODP_KEY_ATTR_IPV6         34     2     4     40 
+ *  ODP_KEY_ATTR_ICMPV6        2     2     4      8
+ *  ODP_KEY_ATTR_ND           28    --     4     32
+ *  -------------------------------------------------
+ *  total                                       132
+ */
+#define ODP_KEY_MAX_LEN 132
+
+/* Both the userspace and kernel make assumptions about buffer sizes
+ * based on the maximum length of a nlattr-formatted flow key.  This is
+ * an imperfect sanity-check that ODP_KEY_MAX_LEN doesn't need to
+ * change, but will at least raise awareness when new ODP key types are
+ * added. */
+#ifdef BUILD_ASSERT_DECL
+BUILD_ASSERT_DECL(__ODP_KEY_ATTR_MAX == 14);
+#endif
+
 struct odp_key_ethernet {
 	uint8_t	 eth_src[6];
 	uint8_t	 eth_dst[6];
diff --git a/lib/odp-util.h b/lib/odp-util.h
index 452c5cf..9acf5d9 100644
--- a/lib/odp-util.h
+++ b/lib/odp-util.h
@@ -63,14 +63,14 @@ void format_odp_action(struct ds *, const struct nlattr *);
 void format_odp_actions(struct ds *, const struct nlattr *odp_actions,
                         size_t actions_len);
 
-/* By my calculations currently the longest valid nlattr-formatted flow key is
- * 132 bytes long, so this leaves some safety margin.
+/* The longest valid nlattr-formatted flow key is ODP_KEY_MAX_LEN, so
+ * this leaves some safety margin.
  *
  * We allocate temporary on-stack buffers for flow keys as arrays of uint32_t
  * to ensure proper 32-bit alignment for Netlink attributes.  (An array of
  * "struct nlattr" might not, in theory, be sufficiently aligned because it
  * only contains 16-bit types.) */
-#define ODPUTIL_FLOW_KEY_BYTES 148
+#define ODPUTIL_FLOW_KEY_BYTES (ODP_KEY_MAX_LEN + 16)
 #define ODPUTIL_FLOW_KEY_U32S DIV_ROUND_UP(ODPUTIL_FLOW_KEY_BYTES, 4)
 
 void odp_flow_key_format(const struct nlattr *, size_t, struct ds *);
-- 
1.7.1