[ovs-dev] [PATCH 1/2] netdev-dpdk: Drop offload API for vhost ports.

[Stokes, Ian]

> On Thu, Nov 08, 2018 at 02:40:12PM +0000, Stokes, Ian wrote:
> > > On Thu, Nov 08, 2018 at 02:16:15PM +0000, Stokes, Ian wrote:
> > > > > On 06.11.2018 17:31, Stokes, Ian wrote:
> > > > > >> On 18.10.2018 16:29, Ilya Maximets wrote:
> > > > > >>> vhost ports are not DPDK eth ports and has no rte_flow API.
> > > > > >>> Stop calling this API with DPDK_ETH_PORT_ID_INVALID to avoid
> > > > > >>> time wasting and errors in log.
> > > > > >>>
> > > > > >>> Additionally, DPDK_FLOW_OFFLOAD_API definition moved to .c
> > > > > >>> file, because there is no need to expose it in header.
> > > > > >>>
> > > > > >>> CC: Finn Christensen <fc at napatech.com>
> > > > > >>> Fixes: e8a2b5bf92bb ("netdev-dpdk: implement flow offload
> > > > > >>> with rte
> > > > > >>> flow")
> > > > > >>> Signed-off-by: Ilya Maximets <i.maximets at samsung.com>
> > > > > >>> ---
> > > > > >>
> > > > > >> Hi Ian,
> > > > > >> You didn't backport this patch to 2.10. Do you think that
> > > > > >> it's not needed or you just missed it while preparing the pull
> request?
> > > > > >>
> > > > > >> Periodic errors in log are a bit annoying.
> > > > > >
> > > > > > Hi Ilya,
> > > > > >
> > > > > > The patch above assumes that a previous commit 89c09c1cd1f0
> > > ("netdev:
> > > > > Clean up class initialization.") is in place, however
> > > > > 89c09c1cd1f0
> > > > > ("netdev: Clean up class initialization.") was never backported
> > > > > to branch
> > > > > 2.10 I had discussed this with Ben but we didn’t see the need.
> > > > > >
> > > > >
> > > > > OK. I see.
> > > > >
> > > > > > As such the patch does not apply as the netdev dpdk class
> > > > > > layout
> > > > > differs. You could submit a specific patch for branch 2.10 with
> > > > > an amended commit message.
> > > > > >
> > > > > > Alternatively I'm thinking it might make sense to backport
> > > > > > 89c09c1cd1f0
> > > > > as well as the patch above in order to remove the periodic log
> > > requests?
> > > > >
> > > > > In this case you'll need to backport also commit
> > > > > 72713c651 ("netdev-bsd: Fix build failure because of undefined
> > > > > NO_OFFLOAD_API.").
> > > > >
> > > > > Maybe we can backport only changes related to netdev-dpdk like
> this:
> > > > >     git cherry-pick -n 89c09c1cd1f0 c0af6425d \
> > > > >         && git reset HEAD \
> > > > >         && git add lib/netdev-dpdk.c \
> > > > >         && git checkout . \
> > > > >         && git commit -sv
> > > > >
> > > > > What do you think?
> > > >
> > > > Sure, that would work, I'd like to ask Bens opinion here also.
> > > >
> > > > Ben would above be ok or is it preferred to backport commits
> > > > separately to 2.10 as
> > > >
> > > > 89c09c1cd1f0 ("netdev: Clean up class initialization.")
> > > > 72713c651 ("netdev-bsd: Fix build failure because of undefined
> > > NO_OFFLOAD_API.").
> > > > c0af6425d7ed ("netdev-dpdk: Drop offload API for vhost ports.")
> > > >
> > > > Is there a preference as regards keeping the original commits and
> > > > sign
> > > off tags for the code changes when backporting rather than creating
> > > a new commit that combines code changes.
> > >
> > > Usually we backport commit individually because it makes it easier
> > > to figure out what was backported.  Individual commits are easy to
> > > see at a glance in the history, squashed commits take a closer look.
> >
> > Thanks for clarifying, in that case I'll backport the three commits
> separately.
> >
> > A quick follow up, commit 89c09c1cd1f0 isn't specific to netdev-dpdk and
> also is not a bug fix but it does help avoid periodic errors in the logs
> for HWOL. As it doesn't add new functionality I would assume it's ok to
> backport?
> 
> Seems fine to me.

Thanks for the clarification, I've backported the following to branch 2.10

89c09c1cd1f0 ("netdev: Clean up class initialization.")
72713c651 ("netdev-bsd: Fix build failure because of undefined NO_OFFLOAD_API.").
c0af6425d7ed ("netdev-dpdk: Drop offload API for vhost ports.")

Thanks
Ian