On Wed, Nov 29, 2017 at 06:35:35AM +0000, Vishal Deep Ajmera wrote:
> On Thu, Nov 16, 2017 at 06:01:54AM +0000, Vishal Deep Ajmera 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 <[email protected]>
>
> Is this a bug fix or a precaution? That is, are there actual cases in the
> code today where such a crash occurs?
>
> [Vishal] This is only a precaution. As of now, I do not see a case where a
> crash
> can happen if we continue to execute the actions on an empty batch.
> However any new actions we add in future will need to take care of empty
> batches.
OK, thanks. Will you please make that clear in the commit message? It
can be important when reading a commit message later to know whether it
fixes a bug.
> In the code, please do not add extra () where they are not needed:
> if (last_action || (batch->count == 0)) {
>
> [Vishal] Should I spin a V2 patch addressing your comment ?
Please do, with an updated commit message too.
Thanks,
Ben.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
