[ovs-dev] [v3 dpdk-latest 2/3] netdev-dpdk: Replace rte_eth_dev_attach/detach

[Ophir Munk]

> -----Original Message-----
> From: Ilya Maximets <i.maximets at samsung.com>
> Sent: Friday, November 30, 2018 1:29 PM
> To: Stokes, Ian <ian.stokes at intel.com>; dev at openvswitch.org; Ophir Munk
> <ophirmu at mellanox.com>; Kevin Traynor <ktraynor at redhat.com>
> Cc: Asaf Penso <asafp at mellanox.com>; Olga Shern <olgas at mellanox.com>;
> Thomas Monjalon <thomasm at mellanox.com>; Shahaf Shuler
> <shahafs at mellanox.com>
> Subject: Re: [v3 dpdk-latest 2/3] netdev-dpdk: Replace
> rte_eth_dev_attach/detach
> 
> Hi.
> 
> On 29.11.2018 19:31, Stokes, Ian wrote:
> >> Hi Ian, Kevin,
> >> dpdk 18.11 was officially released. Can you please inform what are the
> plans for merging these series into dpdk-latest and dpdk-hwol branches?
> >> Once merged - I would appreciate having the representor patches
> reviewed (after being rebased).
> >>
> >
> > Thanks for Raising this Ophir,
> >
> > I would think the steps would be as follows:
> >
> > 1. Merge latest changes from OVS master to dpdk-latest. (Ian) 2. Create
> 18.11 support patch based on dpdk-latest. (Kevin) 3. Apply DPDK 18.11 patch
> to to support 18.11 to dpdk-latest(Ian).
> >
> > I intended to merge master to dpdk-latest tomorrow, if people have signed
> off on Kevins patch then I can also apply that to dpdk-latest.
> 
> Beside the other talks, is it possible to always use 'git pull --rebase' policy for
> branches ? It's much easier to track the changes / diff with the master if the
> history is plain.
> 
> >
> > As regards moving merging 18.11 support to master I think we need to
> discuss the logistics of this a little more. Discussing this with Kevin I didn’t see
> a clear path so we have a few options.
> >
> > We could generate a patch on the diff between master and dpdk-latest
> with 18.11 support.
> >
> > Upside is we move in one patch to 18.11. However this could be messy
> though in terms of bisecting issues in the future, it would also be nice to give
> proper credit to the authors for the work done on individual patches.

The commit message should include Co-authored-by in order to give credit to other authors.

> >
> > Another choice is to break the move into 2 patches. Patch 1 could move to
> 18.08 on master first (squashed commits based on the DPDK 18.08 work you
> submitted), then patch 2 would follow up immediately with a move to 18.11
> based on what Kevin has submitted.
> >
> > This would at least avoid 1 large patch, would be easier to bisect in the
> future also if any issues occur.
> >
> > The downside here is that the DPDK compilation will fail in travis for DPDK
> 18.08 due to the deprecated API for hotplug attach/detach. I'd like to avoid
> introducing warnings into the OVS build if possible.
> >
> > What are peoples thoughts?
> 
> I'd like the idea to squash everything in a single patch.
> You may squash in my patch too:
> bef2cdc8f412 ("netdev-dpdk: Fix returning the field of malloced struct.") This
> will slightly decrease the size of resulted change.
> 
> Kevin's patch is relatively small. It has only 24 lines in code diff.
> Upgrade to 18.08 is already big and contains a lot of not really connected
> changes. So, additional 24 lines will not make it worse.
> IMHO, bisecting will not suffer from squashing. And more, it'll be easier to
> bisect, because you will not have to build completely different DPDK version
> for a single commit in the middle. And, as I already said, upgrade to 18.11 is a
> small patch.
> 

Squashing is an option. There is no point in having only one commit which refers to dpdk 18.08.
However, the commit message should be detailed and include all the information regarding what was done for 18.08 and 18.11 separately.

> Another issue is that without the IOPL fix [1], unit tests will fail if linked with
> DPDK 18.08. This could also make bisecting harder.
> 18.11.0 already has the fix on DPDK level.
> 
> [1]
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fma
> il.openvswitch.org%2Fpipermail%2Fovs-dev%2F2018-
> November%2F354114.html&amp;data=02%7C01%7Cophirmu%40mellanox.c
> om%7C212ec50e3ddf4bb7aee408d656b70893%7Ca652971c7d2e4d9ba6a4d14
> 9256f461b%7C0%7C0%7C636791741471242674&amp;sdata=Tt5JpKGrez2TRUt
> iJ9ucWpx1l5mRqfS7s6rxRGK3en4%3D&amp;reserved=0
> 
> Best regards, Ilya Maximets.
> 
> > Ian

Regards,
Ophir