On Thu, Aug 18, 2022 at 7:40 AM Mark Michelson <mmich...@redhat.com> wrote:
>
> Thanks for the fix, Ales.
>
> Acked-by: Mark Michelson <mmich...@redhat.com>
>
> On 8/18/22 05:41, Ales Musil wrote:
> > 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.
> >
Thanks Ales! I am wondering why not expose and call encode_controller_op()
directly?
Thanks,
Han
> > 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);
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
