mbox series

[ovs-dev] OVS DPDK: dpdk_merge pull request

Message ID CD7C01071941AC429549C17338DB8A52891909A2@IRSMSX101.ger.corp.intel.com
State Accepted
Headers show
Series [ovs-dev] OVS DPDK: dpdk_merge pull request | expand

Pull-request

https://github.com/istokes/ovs dpdk_merge

Message

Stokes, Ian Dec. 8, 2017, 10:14 p.m. UTC
Hi Ben,

The following changes since commit 65d9759c4fc433dbda89ad3d7225c1f5eac274ca:

  ovsdb-data: Add OVS_WARN_UNUSED_RESULT annotations to function definitions. (2017-12-08 13:39:29 -0800)

are available in the git repository at:

  https://github.com/istokes/ovs dpdk_merge

for you to fetch changes up to 3eb8d4fa0db3159a8ffc8f52223417b3417263b3:

  netdev-dpdk: extend netdev_dpdk_get_status to include if_type and if_descr (2017-12-08 21:42:54 +0000)

----------------------------------------------------------------
Ilya Maximets (3):
      netdev-dpdk: Fix variables naming in set_admin_state function.
      netdev-dpdk: Add comment about variables naming convention.
      Revert "dpif_netdev: Refactor dp_netdev_pmd_thread structure."

Kevin Traynor (4):
      netdev-dpdk: Remove uneeded call to rte_eth_dev_count().
      dpif-netdev: Add port/queue tiebreaker to rxq_cycle_sort.
      dpif-netdev: Rename rxq_cycle_sort to compare_rxq_cycles.
      dpif-netdev: Calculate rxq cycles prior to compare_rxq_cycles calls.

Mark Kavanagh (2):
      netdev-dpdk: DPDK v17.11 upgrade
      netdev-dpdk: vHost IOMMU support

Michal Weglicki (1):
      netdev-dpdk: extend netdev_dpdk_get_status to include if_type and if_descr

Yifeng Sun (1):
      dpif-netdev: Fix memory leak

 .travis/linux-build.sh                   |   2 +-
 Documentation/faq/releases.rst           |   1 +
 Documentation/intro/install/dpdk.rst     |  10 +++++-----
 Documentation/topics/dpdk/ring.rst       |   2 +-
 Documentation/topics/dpdk/vhost-user.rst |  60 +++++++++++++++++++++++++++++++++++++++++++++++++++-----
 NEWS                                     |   3 +++
 lib/dpdk-stub.c                          |   6 ++++++
 lib/dpdk.c                               |  12 ++++++++++++
 lib/dpdk.h                               |   3 +++
 lib/dpif-netdev.c                        | 202 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--------------------------------------------------------------------------------------------------
 lib/netdev-dpdk.c                        |  70 +++++++++++++++++++++++++++++++++++++++++++++++------------------
 vswitchd/vswitch.xml                     |  79 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 12 files changed, 314 insertions(+), 136 deletions(-)

Thanks
Ian

Comments

Ben Pfaff Dec. 11, 2017, 6:35 p.m. UTC | #1
On Fri, Dec 08, 2017 at 10:14:49PM +0000, Stokes, Ian wrote:
> The following changes since commit 65d9759c4fc433dbda89ad3d7225c1f5eac274ca:
> 
>   ovsdb-data: Add OVS_WARN_UNUSED_RESULT annotations to function definitions. (2017-12-08 13:39:29 -0800)
> 
> are available in the git repository at:
> 
>   https://github.com/istokes/ovs dpdk_merge
> 
> for you to fetch changes up to 3eb8d4fa0db3159a8ffc8f52223417b3417263b3:

Thanks a lot.  I merged this to master.

This merge added one new "sparse" warning.  I sent a fix for review:
        https://patchwork.ozlabs.org/patch/847165/
Stokes, Ian Dec. 11, 2017, 6:56 p.m. UTC | #2
> On Fri, Dec 08, 2017 at 10:14:49PM +0000, Stokes, Ian wrote:
> > The following changes since commit
> 65d9759c4fc433dbda89ad3d7225c1f5eac274ca:
> >
> >   ovsdb-data: Add OVS_WARN_UNUSED_RESULT annotations to function
> definitions. (2017-12-08 13:39:29 -0800)
> >
> > are available in the git repository at:
> >
> >   https://github.com/istokes/ovs dpdk_merge
> >
> > for you to fetch changes up to 3eb8d4fa0db3159a8ffc8f52223417b3417263b3:
> 
> Thanks a lot.  I merged this to master.

Thanks for this Ben.

> 
> This merge added one new "sparse" warning.  I sent a fix for review:
>         https://patchwork.ozlabs.org/patch/847165/


I recompiled with sparse to see if I could reproduce the issue but I don't see that warning.

I'm concerned that there could be a problem with sparse on my build system at this stage as this is the second time an issue picked up by your build but not on mine.

I obviously need to look into this, out of interest what version of sparse do you use?

Thanks
Ian
Ben Pfaff Dec. 11, 2017, 9:55 p.m. UTC | #3
On Mon, Dec 11, 2017 at 06:56:12PM +0000, Stokes, Ian wrote:
> > On Fri, Dec 08, 2017 at 10:14:49PM +0000, Stokes, Ian wrote:
> > > The following changes since commit
> > 65d9759c4fc433dbda89ad3d7225c1f5eac274ca:
> > >
> > >   ovsdb-data: Add OVS_WARN_UNUSED_RESULT annotations to function
> > definitions. (2017-12-08 13:39:29 -0800)
> > >
> > > are available in the git repository at:
> > >
> > >   https://github.com/istokes/ovs dpdk_merge
> > >
> > > for you to fetch changes up to 3eb8d4fa0db3159a8ffc8f52223417b3417263b3:
> > 
> > Thanks a lot.  I merged this to master.
> 
> Thanks for this Ben.
> 
> > 
> > This merge added one new "sparse" warning.  I sent a fix for review:
> >         https://patchwork.ozlabs.org/patch/847165/
> 
> 
> I recompiled with sparse to see if I could reproduce the issue but I don't see that warning.
> 
> I'm concerned that there could be a problem with sparse on my build system at this stage as this is the second time an issue picked up by your build but not on mine.

Do you build with C=1 on the make command line?  That is required to
enable sparse, if it is installed.

This has caused surprises for others in the past.  Maybe we should
change it.

> I obviously need to look into this, out of interest what version of sparse do you use?

It looks like I have v0.5.0-44-g40791b9.  I don't think the particular
version should matter in this case.
Ilya Maximets Dec. 12, 2017, 8:41 a.m. UTC | #4
Hi Ian and Ben.

One more thing that is broken is the travis build.
I've send a patch-set to fix it here:
	https://mail.openvswitch.org/pipermail/ovs-dev/2017-December/342005.html

My general suggestion is to enable travis builds for 'dpdk_merge' branch and wait
for successful build before making the pull request. What do you think?

Best regards, Ilya Maximets.

> Hi Ben,
> 
> The following changes since commit 65d9759c4fc433dbda89ad3d7225c1f5eac274ca:
> 
>   ovsdb-data: Add OVS_WARN_UNUSED_RESULT annotations to function definitions. (2017-12-08 13:39:29 -0800)
> 
> are available in the git repository at:
> 
>   https://github.com/istokes/ovs dpdk_merge
> 
> for you to fetch changes up to 3eb8d4fa0db3159a8ffc8f52223417b3417263b3:
> 
>   netdev-dpdk: extend netdev_dpdk_get_status to include if_type and if_descr (2017-12-08 21:42:54 +0000)
> 
> ----------------------------------------------------------------
> Ilya Maximets (3):
>       netdev-dpdk: Fix variables naming in set_admin_state function.
>       netdev-dpdk: Add comment about variables naming convention.
>       Revert "dpif_netdev: Refactor dp_netdev_pmd_thread structure."
> 
> Kevin Traynor (4):
>       netdev-dpdk: Remove uneeded call to rte_eth_dev_count().
>       dpif-netdev: Add port/queue tiebreaker to rxq_cycle_sort.
>       dpif-netdev: Rename rxq_cycle_sort to compare_rxq_cycles.
>       dpif-netdev: Calculate rxq cycles prior to compare_rxq_cycles calls.
> 
> Mark Kavanagh (2):
>       netdev-dpdk: DPDK v17.11 upgrade
>       netdev-dpdk: vHost IOMMU support
> 
> Michal Weglicki (1):
>       netdev-dpdk: extend netdev_dpdk_get_status to include if_type and if_descr
> 
> Yifeng Sun (1):
>       dpif-netdev: Fix memory leak
> 
>  .travis/linux-build.sh                   |   2 +-
>  Documentation/faq/releases.rst           |   1 +
>  Documentation/intro/install/dpdk.rst     |  10 +++++-----
>  Documentation/topics/dpdk/ring.rst       |   2 +-
>  Documentation/topics/dpdk/vhost-user.rst |  60 +++++++++++++++++++++++++++++++++++++++++++++++++++-----
>  NEWS                                     |   3 +++
>  lib/dpdk-stub.c                          |   6 ++++++
>  lib/dpdk.c                               |  12 ++++++++++++
>  lib/dpdk.h                               |   3 +++
>  lib/dpif-netdev.c                        | 202 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--------------------------------------------------------------------------------------------------
>  lib/netdev-dpdk.c                        |  70 +++++++++++++++++++++++++++++++++++++++++++++++------------------
>  vswitchd/vswitch.xml                     |  79 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  12 files changed, 314 insertions(+), 136 deletions(-)
> 
> Thanks
> Ian
Stokes, Ian Dec. 12, 2017, 9:32 a.m. UTC | #5
> Hi Ian and Ben.

> 

> One more thing that is broken is the travis build.

> I've send a patch-set to fix it here:

> 	https://mail.openvswitch.org/pipermail/ovs-dev/2017-

> December/342005.html

> 

> My general suggestion is to enable travis builds for 'dpdk_merge' branch

> and wait for successful build before making the pull request. What do you

> think?


Hi Ilya,

Fully agree, I'll work this into the validation process for the dpdk_merge branch going forward.

I know when I had done the pull request in the past via the GitHub tool travis seemed to be run as part of that request but obviously this doesn’t happen when I send a pull request via mail, that’s my bad.

Ian


> 

> Best regards, Ilya Maximets.

> 

> > Hi Ben,

> >

> > The following changes since commit

> 65d9759c4fc433dbda89ad3d7225c1f5eac274ca:

> >

> >   ovsdb-data: Add OVS_WARN_UNUSED_RESULT annotations to function

> > definitions. (2017-12-08 13:39:29 -0800)

> >

> > are available in the git repository at:

> >

> >   https://github.com/istokes/ovs dpdk_merge

> >

> > for you to fetch changes up to 3eb8d4fa0db3159a8ffc8f52223417b3417263b3:

> >

> >   netdev-dpdk: extend netdev_dpdk_get_status to include if_type and

> > if_descr (2017-12-08 21:42:54 +0000)

> >

> > ----------------------------------------------------------------

> > Ilya Maximets (3):

> >       netdev-dpdk: Fix variables naming in set_admin_state function.

> >       netdev-dpdk: Add comment about variables naming convention.

> >       Revert "dpif_netdev: Refactor dp_netdev_pmd_thread structure."

> >

> > Kevin Traynor (4):

> >       netdev-dpdk: Remove uneeded call to rte_eth_dev_count().

> >       dpif-netdev: Add port/queue tiebreaker to rxq_cycle_sort.

> >       dpif-netdev: Rename rxq_cycle_sort to compare_rxq_cycles.

> >       dpif-netdev: Calculate rxq cycles prior to compare_rxq_cycles

> calls.

> >

> > Mark Kavanagh (2):

> >       netdev-dpdk: DPDK v17.11 upgrade

> >       netdev-dpdk: vHost IOMMU support

> >

> > Michal Weglicki (1):

> >       netdev-dpdk: extend netdev_dpdk_get_status to include if_type

> > and if_descr

> >

> > Yifeng Sun (1):

> >       dpif-netdev: Fix memory leak

> >

> >  .travis/linux-build.sh                   |   2 +-

> >  Documentation/faq/releases.rst           |   1 +

> >  Documentation/intro/install/dpdk.rst     |  10 +++++-----

> >  Documentation/topics/dpdk/ring.rst       |   2 +-

> >  Documentation/topics/dpdk/vhost-user.rst |  60

> +++++++++++++++++++++++++++++++++++++++++++++++++++-----

> >  NEWS                                     |   3 +++

> >  lib/dpdk-stub.c                          |   6 ++++++

> >  lib/dpdk.c                               |  12 ++++++++++++

> >  lib/dpdk.h                               |   3 +++

> >  lib/dpif-netdev.c                        | 202

> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

> ++++++++++++++++----------------------------------------------------------

> ----------------------------------------

> >  lib/netdev-dpdk.c                        |  70

> +++++++++++++++++++++++++++++++++++++++++++++++------------------

> >  vswitchd/vswitch.xml                     |  79

> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

> >  12 files changed, 314 insertions(+), 136 deletions(-)

> >

> > Thanks

> > Ian
Stokes, Ian Dec. 12, 2017, 9:38 a.m. UTC | #6
> On Mon, Dec 11, 2017 at 06:56:12PM +0000, Stokes, Ian wrote:
> > > On Fri, Dec 08, 2017 at 10:14:49PM +0000, Stokes, Ian wrote:
> > > > The following changes since commit
> > > 65d9759c4fc433dbda89ad3d7225c1f5eac274ca:
> > > >
> > > >   ovsdb-data: Add OVS_WARN_UNUSED_RESULT annotations to function
> > > definitions. (2017-12-08 13:39:29 -0800)
> > > >
> > > > are available in the git repository at:
> > > >
> > > >   https://github.com/istokes/ovs dpdk_merge
> > > >
> > > > for you to fetch changes up to
> 3eb8d4fa0db3159a8ffc8f52223417b3417263b3:
> > >
> > > Thanks a lot.  I merged this to master.
> >
> > Thanks for this Ben.
> >
> > >
> > > This merge added one new "sparse" warning.  I sent a fix for review:
> > >         https://patchwork.ozlabs.org/patch/847165/
> >
> >
> > I recompiled with sparse to see if I could reproduce the issue but I
> don't see that warning.
> >
> > I'm concerned that there could be a problem with sparse on my build
> system at this stage as this is the second time an issue picked up by your
> build but not on mine.
> 
> Do you build with C=1 on the make command line?  That is required to
> enable sparse, if it is installed.
> 
> This has caused surprises for others in the past.  Maybe we should change
> it.
> 
> > I obviously need to look into this, out of interest what version of
> sparse do you use?
> 
> It looks like I have v0.5.0-44-g40791b9.  I don't think the particular
> version should matter in this case.

Ok, on my system I have sparse-0.5.0-10.fc24.x86_64 and I run make with C=1. Hmmm, let me do some more investigating and I'll get back to you on this.

I'd be interested to hear from anyone else if they are hitting or have resolved the same issue with sparse.

Ian
Mark Kavanagh Dec. 12, 2017, 9:42 a.m. UTC | #7
>From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev-bounces@openvswitch.org]
>On Behalf Of Ilya Maximets
>Sent: Tuesday, December 12, 2017 8:41 AM
>To: ovs-dev@openvswitch.org; Stokes, Ian <ian.stokes@intel.com>; Ben Pfaff
><blp@ovn.org>
>Subject: Re: [ovs-dev] OVS DPDK: dpdk_merge pull request
>
>Hi Ian and Ben.
>
>One more thing that is broken is the travis build.
>I've send a patch-set to fix it here:
>	https://mail.openvswitch.org/pipermail/ovs-dev/2017-December/342005.html

Thanks for this Ilya.

The DPDK v17.11 upgrade patch failed to remove once instance of the word 'stable', since I simply performed a 'find-and-replace' in most instances for '17.05', removing the reference to 'stable' where the latter was found.
However, the travis script in question uses parameter substitution, hence it was missed in the 17.11 patch - apologies for this oversight on my part.

I fully agree with Ilya's proposal to enable travis on the dpdk_merge branch, which should help identify such errors prior to mainline merge in future.

Thanks and best,
Mark

>
>My general suggestion is to enable travis builds for 'dpdk_merge' branch and
>wait
>for successful build before making the pull request. What do you think?
>
>Best regards, Ilya Maximets.
>
>> Hi Ben,
>>
>> The following changes since commit 65d9759c4fc433dbda89ad3d7225c1f5eac274ca:
>>
>>   ovsdb-data: Add OVS_WARN_UNUSED_RESULT annotations to function
>definitions. (2017-12-08 13:39:29 -0800)
>>
>> are available in the git repository at:
>>
>>   https://github.com/istokes/ovs dpdk_merge
>>
>> for you to fetch changes up to 3eb8d4fa0db3159a8ffc8f52223417b3417263b3:
>>
>>   netdev-dpdk: extend netdev_dpdk_get_status to include if_type and if_descr
>(2017-12-08 21:42:54 +0000)
>>
>> ----------------------------------------------------------------
>> Ilya Maximets (3):
>>       netdev-dpdk: Fix variables naming in set_admin_state function.
>>       netdev-dpdk: Add comment about variables naming convention.
>>       Revert "dpif_netdev: Refactor dp_netdev_pmd_thread structure."
>>
>> Kevin Traynor (4):
>>       netdev-dpdk: Remove uneeded call to rte_eth_dev_count().
>>       dpif-netdev: Add port/queue tiebreaker to rxq_cycle_sort.
>>       dpif-netdev: Rename rxq_cycle_sort to compare_rxq_cycles.
>>       dpif-netdev: Calculate rxq cycles prior to compare_rxq_cycles calls.
>>
>> Mark Kavanagh (2):
>>       netdev-dpdk: DPDK v17.11 upgrade
>>       netdev-dpdk: vHost IOMMU support
>>
>> Michal Weglicki (1):
>>       netdev-dpdk: extend netdev_dpdk_get_status to include if_type and
>if_descr
>>
>> Yifeng Sun (1):
>>       dpif-netdev: Fix memory leak
>>
>>  .travis/linux-build.sh                   |   2 +-
>>  Documentation/faq/releases.rst           |   1 +
>>  Documentation/intro/install/dpdk.rst     |  10 +++++-----
>>  Documentation/topics/dpdk/ring.rst       |   2 +-
>>  Documentation/topics/dpdk/vhost-user.rst |  60
>+++++++++++++++++++++++++++++++++++++++++++++++++++-----
>>  NEWS                                     |   3 +++
>>  lib/dpdk-stub.c                          |   6 ++++++
>>  lib/dpdk.c                               |  12 ++++++++++++
>>  lib/dpdk.h                               |   3 +++
>>  lib/dpif-netdev.c                        | 202
>++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>++++++++++++------------------------------------------------------------------
>--------------------------------
>>  lib/netdev-dpdk.c                        |  70
>+++++++++++++++++++++++++++++++++++++++++++++++------------------
>>  vswitchd/vswitch.xml                     |  79
>++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  12 files changed, 314 insertions(+), 136 deletions(-)
>>
>> Thanks
>> Ian
>
>_______________________________________________
>dev mailing list
>dev@openvswitch.org
>https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Stokes, Ian Dec. 12, 2017, 11:32 a.m. UTC | #8
> > Hi Ian and Ben.

> >

> > One more thing that is broken is the travis build.

> > I've send a patch-set to fix it here:

> > 	https://mail.openvswitch.org/pipermail/ovs-dev/2017-

> > December/342005.html

> >

> > My general suggestion is to enable travis builds for 'dpdk_merge'

> > branch and wait for successful build before making the pull request.

> > What do you think?


Quick follow up, I've setup travis to run on the dpdk_merge branch now, going forward travis will be run by default before pull requests are sent.

Thanks
Ian

> 

> Hi Ilya,

> 

> Fully agree, I'll work this into the validation process for the dpdk_merge

> branch going forward.

> 

> I know when I had done the pull request in the past via the GitHub tool

> travis seemed to be run as part of that request but obviously this doesn’t

> happen when I send a pull request via mail, that’s my bad.

> 

> Ian

> 

> 

> >

> > Best regards, Ilya Maximets.

> >

> > > Hi Ben,

> > >

> > > The following changes since commit

> > 65d9759c4fc433dbda89ad3d7225c1f5eac274ca:

> > >

> > >   ovsdb-data: Add OVS_WARN_UNUSED_RESULT annotations to function

> > > definitions. (2017-12-08 13:39:29 -0800)

> > >

> > > are available in the git repository at:

> > >

> > >   https://github.com/istokes/ovs dpdk_merge

> > >

> > > for you to fetch changes up to

> 3eb8d4fa0db3159a8ffc8f52223417b3417263b3:

> > >

> > >   netdev-dpdk: extend netdev_dpdk_get_status to include if_type and

> > > if_descr (2017-12-08 21:42:54 +0000)

> > >

> > > ----------------------------------------------------------------

> > > Ilya Maximets (3):

> > >       netdev-dpdk: Fix variables naming in set_admin_state function.

> > >       netdev-dpdk: Add comment about variables naming convention.

> > >       Revert "dpif_netdev: Refactor dp_netdev_pmd_thread structure."

> > >

> > > Kevin Traynor (4):

> > >       netdev-dpdk: Remove uneeded call to rte_eth_dev_count().

> > >       dpif-netdev: Add port/queue tiebreaker to rxq_cycle_sort.

> > >       dpif-netdev: Rename rxq_cycle_sort to compare_rxq_cycles.

> > >       dpif-netdev: Calculate rxq cycles prior to compare_rxq_cycles

> > calls.

> > >

> > > Mark Kavanagh (2):

> > >       netdev-dpdk: DPDK v17.11 upgrade

> > >       netdev-dpdk: vHost IOMMU support

> > >

> > > Michal Weglicki (1):

> > >       netdev-dpdk: extend netdev_dpdk_get_status to include if_type

> > > and if_descr

> > >

> > > Yifeng Sun (1):

> > >       dpif-netdev: Fix memory leak

> > >

> > >  .travis/linux-build.sh                   |   2 +-

> > >  Documentation/faq/releases.rst           |   1 +

> > >  Documentation/intro/install/dpdk.rst     |  10 +++++-----

> > >  Documentation/topics/dpdk/ring.rst       |   2 +-

> > >  Documentation/topics/dpdk/vhost-user.rst |  60

> > +++++++++++++++++++++++++++++++++++++++++++++++++++-----

> > >  NEWS                                     |   3 +++

> > >  lib/dpdk-stub.c                          |   6 ++++++

> > >  lib/dpdk.c                               |  12 ++++++++++++

> > >  lib/dpdk.h                               |   3 +++

> > >  lib/dpif-netdev.c                        | 202

> >

> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

> > ++++++++++++++++------------------------------------------------------

> > ++++++++++++++++----

> > ----------------------------------------

> > >  lib/netdev-dpdk.c                        |  70

> > +++++++++++++++++++++++++++++++++++++++++++++++------------------

> > >  vswitchd/vswitch.xml                     |  79

> >

> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

> > >  12 files changed, 314 insertions(+), 136 deletions(-)

> > >

> > > Thanks

> > > Ian

> 

> _______________________________________________

> dev mailing list

> dev@openvswitch.org

> https://mail.openvswitch.org/mailman/listinfo/ovs-dev