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

Ben Pfaff blp at ovn.org
Thu Nov 8 14:51:24 UTC 2018


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.


More information about the dev mailing list