[ovs-dev] [PATCH v9 1/2] dpif-netdev: Refactor PMD performance into dpif-netdev-perf

[Stokes, Ian]

> -----Original Message-----
> From: Jan Scheurich [mailto:jan.scheurich at ericsson.com]
> Sent: Friday, January 12, 2018 4:25 PM
> To: Ilya Maximets <i.maximets at samsung.com>; dev at openvswitch.org
> Cc: ktraynor at redhat.com; Stokes, Ian <ian.stokes at intel.com>; Darrell Ball
> <dlu998 at gmail.com>; Aaron Conole <aconole at redhat.com>
> Subject: RE: [PATCH v9 1/2] dpif-netdev: Refactor PMD performance into
> dpif-netdev-perf
> 
> > -----Original Message-----
> > From: Ilya Maximets [mailto:i.maximets at samsung.com]
> > Sent: Friday, 12 January, 2018 16:57
> > To: Jan Scheurich <jan.scheurich at ericsson.com>; dev at openvswitch.org
> > Cc: ktraynor at redhat.com; ian.stokes at intel.com; Darrell Ball
> > <dlu998 at gmail.com>; Aaron Conole <aconole at redhat.com>
> > Subject: Re: [PATCH v9 1/2] dpif-netdev: Refactor PMD performance into
> > dpif-netdev-perf
> >
> > >      /* The Netlink encoding of datapath flow keys cannot express @@
> > > -5137,6 +5066,9 @@ handle_packet_upcall(struct dp_netdev_pmd_thread
> *pmd,
> > >          ovs_mutex_unlock(&pmd->flow_mutex);
> > >          emc_probabilistic_insert(pmd, key, netdev_flow);
> > >      }
> > > +    /* Only error ENOSPC can reach here. We process the packet but do
> not
> > > +     * install a datapath flow. Treat as successful. */
> > > +    return 0;
> >
> > This change looks strange. You're returning 0 (successful) here, but
> > patch #2 removes the comment and returns error instead.
> > IMHO, we need to choose one of these solutions and implement it in patch
> #1.
> 
> That was not my intention. It is a leftover. Will fix in next version.
> 

Hi Jan,

I think this series is almost ready to merge, do you intent to submit a v10 to address above? I'm not aware of any other outstanding issues blocking it from merge to the dpdk_merge branch at this point.

Thanks
Ian

> > I'm not sure what was the result of discussion with Aaron and Darrell
> about this?
> > What should we return?
> 
> We agreed to not agree and I therefore proposed to keep the current
> implementation to count upcalls that processed the packet but failed to
> install a datapath flow in because of ENOSPC and return "error" here. It
> can be addressed in a new patch if so wanted.
> 
> BR, Jan