This patch basically reverts 603f2ce04d00("dpif-netdev: Clear flow batches
before execute.")
As explained in that commit message the problem is that
packet_batch_per_flow_execute() can trigger recirculation. This means that
we will call recursively dp_netdev_input__(). Here's a stack frame:
dp_netdev_input__()
{
struct packet_batch_per_flow batches[PKT_ARRAY_SIZE];
/*...*/
packet_batch_per_flow_execute()
{
dp_execute_cb()
{
case OVS_ACTION_ATTR_RECIRC:
dp_netdev_input__() {
struct packet_batch_per_flow
batches[PKT_ARRAY_SIZE];
/*...*/
}
}
}
In the inner dp_netdev_input__() a lookup could theoretically find the same
flows that it finds in the outer.
If we don't clear _all_ the batch pointers before calling the inner
dp_netdev_input__() we could find a flow that still has a pointer to a
batch in the outer dp_netdev_input__(). Then we will append packets to the
outer batch, which is clearly wrong.
2016-10-07 14:09 GMT-07:00 Jarno Rajahalme <ja...@ovn.org>:
> Daniele had a comment on this, I believe?
>
> > On Oct 7, 2016, at 9:17 AM, Bhanuprakash Bodireddy <
> bhanuprakash.bodire...@intel.com> wrote:
> >
> > There is a slight negative performance impact, by zeroing out the flow
> > batch pointers in dp_netdev_input__ ahead of packet_batch_execute(). The
> > issue has been observed with multiple batches test scenario.
> >
> > This patch fixes the problem by removing the extra for loop and clear
> > the flow batches inside the packet_batch_per_flow_execute(). Also the
> > vtune analysis showed that the overall no. of instructions retired for
> > dp_netdev_input__ reduced by 1,273,800,000 with this patch.
> >
> > Fixes: 603f2ce04d00 ("dpif-netdev: Clear flow batches before execute.")
> > Signed-off-by: Bhanuprakash Bodireddy <bhanuprakash.bodire...@intel.com>
> > Signed-off-by: Antonio Fischetti <antonio.fische...@intel.com>
> > ---
> > lib/dpif-netdev.c | 5 +----
> > 1 file changed, 1 insertion(+), 4 deletions(-)
> >
> > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> > index c002dd3..d0bb191 100644
> > --- a/lib/dpif-netdev.c
> > +++ b/lib/dpif-netdev.c
> > @@ -3878,6 +3878,7 @@ packet_batch_per_flow_execute(struct
> packet_batch_per_flow *batch,
> > {
> > struct dp_netdev_actions *actions;
> > struct dp_netdev_flow *flow = batch->flow;
> > + flow->batch = NULL;
> >
> > dp_netdev_flow_used(flow, batch->array.count, batch->byte_count,
> > batch->tcp_flags, now);
> > @@ -4173,10 +4174,6 @@ dp_netdev_input__(struct dp_netdev_pmd_thread
> *pmd,
> > }
> >
> > for (i = 0; i < n_batches; i++) {
> > - batches[i].flow->batch = NULL;
> > - }
> > -
> > - for (i = 0; i < n_batches; i++) {
> > packet_batch_per_flow_execute(&batches[i], pmd, now);
> > }
> > }
> > --
> > 2.4.11
> >
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
