From: Ben Pfaff <blp@nicira.com>
Date: Fri, 4 Oct 2013 15:48:48 +0000 (-0700)
Subject: ofproto-dpif-xlate: Suppress oversize datapath actions.
X-Git-Tag: sliver-openvswitch-2.0.90-1~11^2~3
X-Git-Url: http://git.onelab.eu/?a=commitdiff_plain;h=542024c4c3d36b2d303efbe6c0da469c8e446476;p=sliver-openvswitch.git

ofproto-dpif-xlate: Suppress oversize datapath actions.

If we allow oversize datapath actions to make it out of translation, then
we will assert-fail later when we try to put those actions into a Netlink
attribute.

Bug #19277.
Reported-by: Paul ingram <paul@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Justin Pettit <jpettit@nicira.com>
---

diff --git a/lib/netlink.c b/lib/netlink.c
index 50444abd9..40477eaec 100644
--- a/lib/netlink.c
+++ b/lib/netlink.c
@@ -322,7 +322,7 @@ nl_msg_push_unspec_uninit(struct ofpbuf *msg, uint16_t type, size_t size)
 {
     size_t total_size = NLA_HDRLEN + size;
     struct nlattr* nla = nl_msg_push_uninit(msg, total_size);
-    ovs_assert(NLA_ALIGN(total_size) <= UINT16_MAX);
+    ovs_assert(!nl_attr_oversized(size));
     nla->nla_len = total_size;
     nla->nla_type = type;
     return nla + 1;
@@ -468,6 +468,16 @@ nl_msg_next(struct ofpbuf *buffer, struct ofpbuf *msg)
     msg->size = 0;
     return NULL;
 }
+
+/* Returns true if a Netlink attribute with a payload that is 'payload_size'
+ * bytes long would be oversized, that is, if it's not possible to create an
+ * nlattr of that size because its size wouldn't fit in the 16-bit nla_len
+ * field. */
+bool
+nl_attr_oversized(size_t payload_size)
+{
+    return NL_ATTR_SIZE(payload_size) > UINT16_MAX;
+}
 
 /* Attributes. */
 
diff --git a/lib/netlink.h b/lib/netlink.h
index afe2277ec..21d49d38f 100644
--- a/lib/netlink.h
+++ b/lib/netlink.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, 2009, 2010, 2011 Nicira, Inc.
+ * Copyright (c) 2008, 2009, 2010, 2011, 2013 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -103,6 +103,8 @@ struct nlmsghdr *nl_msg_next(struct ofpbuf *buffer, struct ofpbuf *msg);
 #define NL_A_BE32_SIZE NL_ATTR_SIZE(sizeof(ovs_be32))
 #define NL_A_BE64_SIZE NL_ATTR_SIZE(sizeof(ovs_be64))
 #define NL_A_FLAG_SIZE NL_ATTR_SIZE(0)
+
+bool nl_attr_oversized(size_t payload_size);
 
 /* Netlink attribute types. */
 enum nl_attr_type
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 9090b0546..930abc31a 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -2808,6 +2808,15 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
         }
     }
 
+    if (nl_attr_oversized(ctx.xout->odp_actions.size)) {
+        /* These datapath actions are too big for a Netlink attribute, so we
+         * can't execute them. */
+        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
+
+        VLOG_ERR_RL(&rl, "discarding oversize datapath actions");
+        ofpbuf_clear(&ctx.xout->odp_actions);
+    }
+
     ofpbuf_uninit(&ctx.stack);
 
     /* Clear the metadata and register wildcard masks, because we won't
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index bfb4bb3a0..b70637326 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -2914,11 +2914,14 @@ ADD_OF_PORTS([br0], 1)
      echo "in_port=$i, actions=resubmit:$j, resubmit:$j, local"
  done
  echo "in_port=13, actions=local,local,local,local,local,local,local,local") > flows
- AT_CHECK([ovs-ofctl add-flows br0 flows])
+AT_CHECK([ovs-ofctl add-flows br0 flows])
 AT_CHECK([ovs-appctl ofproto/trace br0 'in_port=1'], [0], [stdout])
 AT_CHECK([grep -c 'resubmits yielded over 64 kB of actions' ovs-vswitchd.log], [0], [1
 ])
-OVS_VSWITCHD_STOP(["/resubmits yielded over 64 kB of actions/d"])
+AT_CHECK([grep -c 'discarding oversize datapath actions' ovs-vswitchd.log], [0], [1
+])
+OVS_VSWITCHD_STOP(["/resubmits yielded over 64 kB of actions/d
+/discarding oversize datapath actions/d"])
 AT_CLEANUP
 
 AT_SETUP([ofproto-dpif - stack too deep])