Thanks Ben for reviewing the patch. My comments inline below.

Warm Regards,
Vishal

-----Original Message-----
From: Ben Pfaff [mailto:[email protected]] 
Sent: Wednesday, November 29, 2017 12:17 AM
To: Vishal Deep Ajmera <[email protected]>
Cc: [email protected]
Subject: Re: [ovs-dev] [PATCH v2] odp-execute: Skip processing actions when 
batch is emptied

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.

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 ?

Thanks,

Ben.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to