Thank you William Tu for reviewing the patch. I have fixed the commit message & corrected subject line (by including file name) in V2 version of the patch.
Warm Regards, Vishal -----Original Message----- From: William Tu [mailto:u9012...@gmail.com] Sent: Wednesday, November 15, 2017 9:19 PM To: Vishal Deep Ajmera <vishal.deep.ajm...@ericsson.com> Cc: d...@openvswitch.org Subject: Re: [ovs-dev] [PATCH] Skip processing actions when batch is emptied. On Wed, Nov 8, 2017 at 1:44 AM, Vishal Deep Ajmera <vishal.deep.ajm...@ericsson.com> wrote: > Today in OVS, when errors are encountered during the execution of an action > the entire batch of packets may be deleted (for e.g. in processing > push_tnl_action, if the port is not found in the port_cache of PMD). The > remaining actions continue to be executed even though there are no packets to > be processed. It is assumed that the code dealing with each action checks > that the batch is not empty before executing. Crashes may occur if the > assumption is not met. > > The patch makes OVS skip processing of further actions from the action-set > once a batch is emptied. Doing so centralizes the check in one place and > avoids the possibility of crashes. > > Signed-off-by: Vishal Deep Ajmera <vishal.deep.ajm...@ericsson.com> > --- nit: please wrap your commit messages to about 75 columns otherwise looks good to me. Acked-by: William Tu <u9012...@gmail.com> > lib/odp-execute.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/lib/odp-execute.c b/lib/odp-execute.c index > 3109f39..8697727 100644 > --- a/lib/odp-execute.c > +++ b/lib/odp-execute.c > @@ -685,9 +685,12 @@ odp_execute_actions(void *dp, struct dp_packet_batch > *batch, bool steal, > dp_execute_action(dp, batch, a, may_steal); > - if (last_action) { > - /* We do not need to free the packets. > dp_execute_actions() > - * has stolen them */ > + if (last_action || (batch->count == 0)) { > + /* We do not need to free the packets. > + * Either dp_execute_actions() has stolen them > + * or the batch is freed due to errors. In either > + * case we do not need to execute further actions. > + */ > return; > } > } > _______________________________________________ > 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
