[ovs-git] [openvswitch/ovs] 3d0d5a: netdev-dpdk: Fix variables naming in set_admin_sta...

GitHub noreply at github.com
Mon Dec 11 18:32:26 UTC 2017


  Branch: refs/heads/master
  Home:   https://github.com/openvswitch/ovs
  Commit: 3d0d5ab1532f147e829983fa1f3bb427ce64cf64
      https://github.com/openvswitch/ovs/commit/3d0d5ab1532f147e829983fa1f3bb427ce64cf64
  Author: Ilya Maximets <i.maximets at samsung.com>
  Date:   2017-12-08 (Fri, 08 Dec 2017)

  Changed paths:
    M lib/netdev-dpdk.c

  Log Message:
  -----------
  netdev-dpdk: Fix variables naming in set_admin_state function.

Function 'netdev_dpdk_set_admin_state()' was missed while fixing
variables naming according to the following convention:

    'struct netdev':'netdev'
    'struct netdev_dpdk':'dev'
    'struct netdev_rxq':'rxq'
    'struct netdev_rxq_dpdk':'rx'

Fixes: d46285a2206f ("netdev-dpdk: Consistent variable naming.")
Signed-off-by: Ilya Maximets <i.maximets at samsung.com>
Signed-off-by: Ian Stokess <ian.stokes at intel.com>


  Commit: b2e72a9c9d64b975c4049a9b1a391588668f2d36
      https://github.com/openvswitch/ovs/commit/b2e72a9c9d64b975c4049a9b1a391588668f2d36
  Author: Ilya Maximets <i.maximets at samsung.com>
  Date:   2017-12-08 (Fri, 08 Dec 2017)

  Changed paths:
    M lib/netdev-dpdk.c

  Log Message:
  -----------
  netdev-dpdk: Add comment about variables naming convention.

It'll be nice to document current naming convention for variables of
the following types used in netdev-dpdk:

	* netdev
	* netdev_dpdk
	* netdev_rxq
	* netdev_rxq_dpdk

to be sure that we will not return to chaos which was before
commit d46285a2206f ("netdev-dpdk: Consistent variable naming.").

Signed-off-by: Ilya Maximets <i.maximets at samsung.com>
Signed-off-by: Ian Stokes <ian.stokes at intel.com>


  Commit: 255b7bda98974c9732c435c3d500d50e20bd728e
      https://github.com/openvswitch/ovs/commit/255b7bda98974c9732c435c3d500d50e20bd728e
  Author: Kevin Traynor <ktraynor at redhat.com>
  Date:   2017-12-08 (Fri, 08 Dec 2017)

  Changed paths:
    M lib/netdev-dpdk.c

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

The call to rte_eth_dev_count() was added as workaround
for rte_eth_dev_get_port_by_name() not handling cases
when there was no DPDK ports.

In versions of DPDK >= 17.02 rte_eth_dev_get_port_by_name()
does handle this case (DPDK commit f9ae888b1e19).
rte_eth_dev_count() is no longer needed so remove it.

Acked-by: Ciara Loftus <ciara.loftus at intel.com>
Signed-off-by: Kevin Traynor <ktraynor at redhat.com>
Signed-off-by: Ian Stokes <ian.stokes at intel.com>


  Commit: a130f1a89bd8ec1176ac8d35a98c284a5e3f5691
      https://github.com/openvswitch/ovs/commit/a130f1a89bd8ec1176ac8d35a98c284a5e3f5691
  Author: Kevin Traynor <ktraynor at redhat.com>
  Date:   2017-12-08 (Fri, 08 Dec 2017)

  Changed paths:
    M lib/dpif-netdev.c

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

rxq_cycle_sort is used to compare rx queues by their measured number
of cycles. In the event that they are equal, 0 could be returned.
However, it is observed that returning 0 results in a different sort
order on Windows/Linux. This is ok in practice but it causes a unit
test failure for
"1007: PMD - pmd-cpu-mask/distribution of rx queues" when running
on different OS's.

In order to have a consistent sort result across multiple OS's,
introduce a tiebreaker of port/queue.

Fixes: 655856ef39b9 ("dpif-netdev: Change rxq_scheduling to use rxq processing cycles.")
Reported-by: Alin Gabriel Serdean <aserdean at ovn.org>
Tested-by: Alin Gabriel Serdean <aserdean at ovn.org>
Co-authored-by: Ilya Maximets <i.maximets at samsung.com>
Signed-off-by: Ilya Maximets <i.maximets at samsung.com>
Signed-off-by: Kevin Traynor <ktraynor at redhat.com>
Signed-off-by: Ian Stokes <ian.stokes at intel.com>


  Commit: cc131ac1840ebca283e92780588208de6e28079a
      https://github.com/openvswitch/ovs/commit/cc131ac1840ebca283e92780588208de6e28079a
  Author: Kevin Traynor <ktraynor at redhat.com>
  Date:   2017-12-08 (Fri, 08 Dec 2017)

  Changed paths:
    M lib/dpif-netdev.c

  Log Message:
  -----------
  dpif-netdev: Rename rxq_cycle_sort to compare_rxq_cycles.

This function is used for comparison between queues
as part of the sort. It does not do the sort itself.
As such, give it a more appropriate name.

Suggested-by: Billy O'Mahony <billy.o.mahony at intel.com>
Signed-off-by: Kevin Traynor <ktraynor at redhat.com>
Acked-by: Billy O'Mahony
Signed-off-by: Ian Stokes <ian.stokes at intel.com>


  Commit: 8368866efbc8c0a1e5bbb7538e87128b452386cc
      https://github.com/openvswitch/ovs/commit/8368866efbc8c0a1e5bbb7538e87128b452386cc
  Author: Kevin Traynor <ktraynor at redhat.com>
  Date:   2017-12-08 (Fri, 08 Dec 2017)

  Changed paths:
    M lib/dpif-netdev.c

  Log Message:
  -----------
  dpif-netdev: Calculate rxq cycles prior to compare_rxq_cycles calls.

compare_rxq_cycles sums the latest cycles from each queue for
comparison with each other. While each comparison correctly
gets the latest cycles, the cycles could change between calls
to compare_rxq_cycle. In order to use consistent values through
each call of compare_rxq_cycles, sum the cycles before qsort is
called.

Requested-by: Ilya Maximets <i.maximets at samsung.com>
Signed-off-by: Kevin Traynor <ktraynor at redhat.com>
Signed-off-by: Ian Stokes <ian.stokes at intel.com>


  Commit: d1ce9c2033fcbd4ee73c4b22abb6cf11db968bd2
      https://github.com/openvswitch/ovs/commit/d1ce9c2033fcbd4ee73c4b22abb6cf11db968bd2
  Author: Yifeng Sun <pkusunyifeng at gmail.com>
  Date:   2017-12-08 (Fri, 08 Dec 2017)

  Changed paths:
    M lib/dpif-netdev.c

  Log Message:
  -----------
  dpif-netdev: Fix memory leak

Valgrind complains in test 1019 (dpctl - add-if set-if del-if):

4,850,896 (4,850,240 direct, 656 indirect) bytes in 1 blocks are
definitely lost in loss record 364 of 364
   by 0x517062: xcalloc (util.c:103)
   by 0x46CBBC: dp_netdev_set_nonpmd (dpif-netdev.c:4498)
   by 0x46CBBC: create_dp_netdev (dpif-netdev.c:1299)
   by 0x46CBBC: dpif_netdev_open (dpif-netdev.c:1337)
   by 0x472CB0: do_open (dpif.c:350)
   by 0x472E6F: dpif_create (dpif.c:404)
   by 0x472E6F: dpif_create_and_open (dpif.c:417)
   by 0x430EBC: open_dpif_backer (ofproto-dpif.c:727)
   by 0x430EBC: construct (ofproto-dpif.c:1411)
   by 0x41B714: ofproto_create (ofproto.c:539)
   by 0x40C84E: bridge_reconfigure (bridge.c:647)
   by 0x4104C5: bridge_run (bridge.c:2998)
   by 0x406FA4: main (ovs-vswitchd.c:119)

The reference count wasn't released at this earlier return.

This fix passes the test 'make check'.

Signed-off-by: Yifeng Sun <pkusunyifeng at gmail.com>
Tested-by: Greg Rose <gvrose8192 at gmail.com>
Reviewed-by: Greg Rose <gvrose8192 at gmail.com>
Signed-off-by: Ian Stokes <ian.stokes at intel.com>


  Commit: 5e925ccc2a6f569f1b32365e3660671b8e7d36b3
      https://github.com/openvswitch/ovs/commit/5e925ccc2a6f569f1b32365e3660671b8e7d36b3
  Author: Mark Kavanagh <mark.b.kavanagh at intel.com>
  Date:   2017-12-08 (Fri, 08 Dec 2017)

  Changed paths:
    M .travis/linux-build.sh
    M Documentation/faq/releases.rst
    M Documentation/intro/install/dpdk.rst
    M Documentation/topics/dpdk/ring.rst
    M Documentation/topics/dpdk/vhost-user.rst
    M NEWS
    M lib/netdev-dpdk.c

  Log Message:
  -----------
  netdev-dpdk: DPDK v17.11 upgrade

This commit adds support for DPDK v17.11:
- minor updates to accomodate DPDK API changes
- update references to DPDK version in Documentation
- update DPDK version in travis' linux-build script
- document DPDK v17.11 virtio driver bug

Signed-off-by: Mark Kavanagh <mark.b.kavanagh at intel.com>
Acked-by: Maxime Coquelin <maxime.coquelin at redhat.com>
Acked-by: Ciara Loftus <ciara.loftus at intel.com>
Acked-by: Jan Scheurich <jan.scheurich at ericsson.com>
Tested-by: Jan Scheurich <jan.scheurich at ericsson.com>
Tested-by: Guoshuai Li <ligs at dtdream.com>
Acked-by: Kevin Traynor <ktraynor at redhat.com>
Signed-off-by: Ian Stokes <ian.stokes at intel.com>


  Commit: a14d1cc8a74858c7488207e02b9ebdb67e50bd88
      https://github.com/openvswitch/ovs/commit/a14d1cc8a74858c7488207e02b9ebdb67e50bd88
  Author: Mark Kavanagh <mark.b.kavanagh at intel.com>
  Date:   2017-12-08 (Fri, 08 Dec 2017)

  Changed paths:
    M Documentation/topics/dpdk/vhost-user.rst
    M NEWS
    M lib/dpdk-stub.c
    M lib/dpdk.c
    M lib/dpdk.h
    M lib/netdev-dpdk.c
    M vswitchd/vswitch.xml

  Log Message:
  -----------
  netdev-dpdk: vHost IOMMU support

DPDK v17.11 introduces support for the vHost IOMMU feature.
This is a security feature, which restricts the vhost memory
that a virtio device may access.

This feature also enables the vhost REPLY_ACK protocol, the
implementation of which is known to work in newer versions of
QEMU (i.e. v2.10.0), but is buggy in older versions (v2.7.0 -
v2.9.0, inclusive). As such, the feature is disabled by default
in (and should remain so), for the aforementioned older QEMU
verions. Starting with QEMU v2.9.1, vhost-iommu-support can
safely be enabled, even without having an IOMMU device, with
no performance penalty.

This patch adds a new global config option, vhost-iommu-support,
that controls enablement of the vhost IOMMU feature:

    ovs-vsctl set Open_vSwitch . other_config:vhost-iommu-support=true

This value defaults to false; to enable IOMMU support, this field
should be set to true when setting other global parameters on init
(such as "dpdk-socket-mem", for example). Changing the value at
runtime is not supported, and requires restarting the vswitch daemon.

Signed-off-by: Mark Kavanagh <mark.b.kavanagh at intel.com>
Acked-by: Kevin Traynor <ktraynor at redhat.com>
Signed-off-by: Ian Stokes <ian.stokes at intel.com>


  Commit: d9d73f84ea2211c15ad66a2d578a6af1c7f2e26d
      https://github.com/openvswitch/ovs/commit/d9d73f84ea2211c15ad66a2d578a6af1c7f2e26d
  Author: Ilya Maximets <i.maximets at samsung.com>
  Date:   2017-12-08 (Fri, 08 Dec 2017)

  Changed paths:
    M lib/dpif-netdev.c

  Log Message:
  -----------
  Revert "dpif_netdev: Refactor dp_netdev_pmd_thread structure."

This reverts commit a807c15796ddc43ba1ffb2a6b0bd2ad4e2b73941.

Padding and aligning of dp_netdev_pmd_thread structure members
is useless, broken in a several ways and only greatly degrades
maintainability and extensibility of the structure.

Issues:

    1. It's not working because all the instances of struct
       dp_netdev_pmd_thread allocated only by usual malloc. All the
       memory is not aligned to cachelines -> structure almost never
       starts at aligned memory address. This means that any further
       paddings and alignments inside the structure are completely
       useless. Fo example:
  Breakpoint 1, pmd_thread_main
       (gdb) p pmd
       $49 = (struct dp_netdev_pmd_thread *) 0x1b1af20
       (gdb) p &pmd->cacheline1
       $51 = (OVS_CACHE_LINE_MARKER *) 0x1b1af60
       (gdb) p &pmd->cacheline0
       $52 = (OVS_CACHE_LINE_MARKER *) 0x1b1af20
       (gdb) p &pmd->flow_cache
       $53 = (struct emc_cache *) 0x1b1afe0
  All of the above addresses shifted from cacheline start by 32B.
  Can we fix it properly? NO.
       OVS currently doesn't have appropriate API to allocate aligned
       memory. The best candidate is 'xmalloc_cacheline()' but it
       clearly states that "The memory returned will not be at the
       start of a cache line, though, so don't assume such alignment".
       And also, this function will never return aligned memory on
       Windows or MacOS.

    2. CACHE_LINE_SIZE is not constant. Different architectures have
       different cache line sizes, but the code assumes that
       CACHE_LINE_SIZE is always equal to 64 bytes. All the structure
       members are grouped by 64 bytes and padded to CACHE_LINE_SIZE.
       This leads to a huge holes in a structures if CACHE_LINE_SIZE
       differs from 64. This is opposite to portability. If I want
       good performance of cmap I need to have CACHE_LINE_SIZE equal
       to the real cache line size, but I will have huge holes in the
       structures. If you'll take a look to struct rte_mbuf from DPDK
       you'll see that it uses 2 defines: RTE_CACHE_LINE_SIZE and
       RTE_CACHE_LINE_MIN_SIZE to avoid holes in mbuf structure.

    3. Sizes of system/libc defined types are not constant for all the
       systems. For example, sizeof(pthread_mutex_t) == 48 on my
       ARMv8 machine, but only 40 on x86. The difference could be
       much bigger on Windows or MacOS systems. But the code assumes
       that sizeof(struct ovs_mutex) is always 48 bytes. This may lead
       to broken alignment/big holes in case of padding/wrong comments
       about amount of free pad bytes.

    4. Sizes of the many fileds in structure depends on defines like
       DP_N_STATS, PMD_N_CYCLES, EM_FLOW_HASH_ENTRIES and so on.
       Any change in these defines or any change in any structure
       contained by thread should lead to the not so simple
       refactoring of the whole dp_netdev_pmd_thread structure. This
       greatly reduces maintainability and complicates development of
       a new features.

    5. There is no reason to align flow_cache member because it's
       too big and we usually access random entries by single thread
       only.

So, the padding/alignment only creates some visibility of performance
optimization but does nothing useful in reality. It only complicates
maintenance and adds huge holes for non-x86 architectures and non-Linux
systems. Performance improvement stated in a original commit message
should be random and not valuable. I see no performance difference.

Most of the above issues are also true for some other padded/aligned
structures like 'struct netdev_dpdk'. They will be treated separately.

CC: Bhanuprakash Bodireddy <bhanuprakash.bodireddy at intel.com>
CC: Ben Pfaff <blp at ovn.org>
Signed-off-by: Ilya Maximets <i.maximets at samsung.com>
Acked-by: Jan Scheurich <jan.scheurich at ericsson.com>
Signed-off-by: Ian Stokes <ian.stokes at intel.com>


  Commit: 3eb8d4fa0db3159a8ffc8f52223417b3417263b3
      https://github.com/openvswitch/ovs/commit/3eb8d4fa0db3159a8ffc8f52223417b3417263b3
  Author: Michal Weglicki <michalx.weglicki at intel.com>
  Date:   2017-12-08 (Fri, 08 Dec 2017)

  Changed paths:
    M lib/netdev-dpdk.c
    M vswitchd/vswitch.xml

  Log Message:
  -----------
  netdev-dpdk: extend netdev_dpdk_get_status to include if_type and if_descr

This commit extends netdev_dpdk_get_status API to include additional
driver-related information: if_type and if_descr.

v2->v3: Code rebase.
v3->v4: Minor comments applied.
v5->v6: Adds DPDK port specific description in documentation.

Co-authored-by: Michal Weglicki <michalx.weglicki at intel.com>
Signed-off-by: Michal Weglicki <michalx.weglicki at intel.com>
Signed-off-by: Przemyslaw Szczerbik <przemyslawx.szczerbik at intel.com>
Tested-by: Greg Rose <gvrose8192 at gmail.com>
Reviewed-by: Greg Rose <gvrose8192 at gmail.com>
Signed-off-by: Ian Stokes <ian.stokes at intel.com>


  Commit: 8c087cecffb0355680ffcc2ac2debc41ac163ae6
      https://github.com/openvswitch/ovs/commit/8c087cecffb0355680ffcc2ac2debc41ac163ae6
  Author: Ben Pfaff <blp at ovn.org>
  Date:   2017-12-11 (Mon, 11 Dec 2017)

  Changed paths:
    M .travis/linux-build.sh
    M Documentation/faq/releases.rst
    M Documentation/intro/install/dpdk.rst
    M Documentation/topics/dpdk/ring.rst
    M Documentation/topics/dpdk/vhost-user.rst
    M NEWS
    M lib/dpdk-stub.c
    M lib/dpdk.c
    M lib/dpdk.h
    M lib/dpif-netdev.c
    M lib/netdev-dpdk.c
    M vswitchd/vswitch.xml

  Log Message:
  -----------
  Merge branch 'dpdk_merge' of https://github.com/istokes/ovs into HEAD


Compare: https://github.com/openvswitch/ovs/compare/433695320a9e...8c087cecffb0


More information about the git mailing list