The controller action had a wrong wrong userdata_len due to
missing substraction of previous data len. The pointer to oc
was not moved correctly so it could in theory point to wrong
memory. In order to fix that expose encode_start_controller_op
and encode_finish_controller_op which are helper methods
to define the controller action. Use those instead of
manually creating the action. At the same time use stack
buffer for the ofpbuf which should be large enough to hold
related data without any heap allocations.

Fixes: e52c2451 ("physical.c: Fix bug of wrong use in vm migration")
Signed-off-by: Ales Musil <amu...@redhat.com>
---
 controller/physical.c | 19 ++++++-------------
 include/ovn/actions.h |  4 ++++
 lib/actions.c         |  4 ++--
 3 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/controller/physical.c b/controller/physical.c
index 552837bcd..bc8c304fe 100644
--- a/controller/physical.c
+++ b/controller/physical.c
@@ -992,8 +992,8 @@ setup_rarp_activation_strategy(const struct 
sbrec_port_binding *binding,
                                struct ovn_desired_flow_table *flow_table)
 {
     struct match match = MATCH_CATCHALL_INITIALIZER;
-    struct ofpbuf ofpacts;
-    ofpbuf_init(&ofpacts, 0);
+    uint64_t stub[1024 / 8];
+    struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(stub);
 
     /* Unblock the port on ingress RARP. */
     match_set_dl_type(&match, htons(ETH_TYPE_RARP));
@@ -1001,18 +1001,11 @@ setup_rarp_activation_strategy(const struct 
sbrec_port_binding *binding,
 
     load_logical_ingress_metadata(binding, zone_ids, &ofpacts);
 
-    struct ofpact_controller *oc = ofpact_put_CONTROLLER(&ofpacts);
-    oc->max_len = UINT16_MAX;
-    oc->reason = OFPR_ACTION;
-
-    struct action_header ah = {
-        .opcode = htonl(ACTION_OPCODE_ACTIVATION_STRATEGY_RARP)
-    };
-    ofpbuf_put(&ofpacts, &ah, sizeof ah);
+    uint32_t ofs =
+        encode_start_controller_op(ACTION_OPCODE_ACTIVATION_STRATEGY_RARP,
+                                   false, NX_CTLR_NO_METER, &ofpacts);
+    encode_finish_controller_op(ofs, &ofpacts);
 
-    ofpacts.header = oc;
-    oc->userdata_len = ofpacts.size - (sizeof *oc);
-    ofpact_finish_CONTROLLER(&ofpacts, &oc);
     put_resubmit(OFTABLE_LOG_INGRESS_PIPELINE, &ofpacts);
 
     ofctrl_add_flow(flow_table, OFTABLE_PHY_TO_LOG, 1010,
diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index 33c319f1c..cbcf4130c 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -829,4 +829,8 @@ void ovnacts_free(struct ovnact[], size_t ovnacts_len);
 char *ovnact_op_to_string(uint32_t);
 int encode_ra_dnssl_opt(char *data, char *buf, int buf_len);
 
+size_t encode_start_controller_op(enum action_opcode opcode, bool pause,
+                                  uint32_t meter_id, struct ofpbuf *ofpacts);
+void encode_finish_controller_op(size_t ofs, struct ofpbuf *ofpacts);
+
 #endif /* ovn/actions.h */
diff --git a/lib/actions.c b/lib/actions.c
index aab044306..5c70ef411 100644
--- a/lib/actions.c
+++ b/lib/actions.c
@@ -78,7 +78,7 @@ ovnact_init(struct ovnact *ovnact, enum ovnact_type type, 
size_t len)
     ovnact->len = len;
 }
 
-static size_t
+size_t
 encode_start_controller_op(enum action_opcode opcode, bool pause,
                            uint32_t meter_id, struct ofpbuf *ofpacts)
 {
@@ -99,7 +99,7 @@ encode_start_controller_op(enum action_opcode opcode, bool 
pause,
     return ofs;
 }
 
-static void
+void
 encode_finish_controller_op(size_t ofs, struct ofpbuf *ofpacts)
 {
     struct ofpact_controller *oc = ofpbuf_at_assert(ofpacts, ofs, sizeof *oc);
-- 
2.37.1

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to