Re: [dpdk-dev] 17.11.4 patches review and test

[Alejandro Lucero <alejandro.lucero@netronome.com>]

On Fri, Aug 24, 2018 at 4:31 PM, Yongseok Koh <yskoh@mellanox.com> wrote:

>
> > On Aug 24, 2018, at 1:51 AM, Alejandro Lucero <
> alejandro.lucero@netronome.com> wrote:
> >
> >
> >
> > On Thu, Aug 23, 2018 at 6:18 PM, Yongseok Koh <yskoh@mellanox.com>
> wrote:
> >
> > > On Aug 22, 2018, at 5:19 PM, Yongseok Koh <yskoh@mellanox.com> wrote:
> > >
> > > On Tue, Aug 21, 2018 at 12:07:49PM +0200, Alejandro Lucero wrote:
> > >> Hi Yonngseok,
> > >>
> > >> There is a patchset aimed at 17.11.x:
> > >>
> > >> https://emea01.safelinks.protection.outlook.com/?url=
> https%3A%2F%2Fpatches.dpdk.org%2Fcover%2F42741%2F&amp;
> data=02%7C01%7Cyskoh%40mellanox.com%7Cb1b0e3eff71c499ff3fb08d6088e1ede%
> 7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636705803846548967&amp;sdata=
> 8f12c1IuUe4mw2EaTZ18vVTuLTjXOD2cSe%2B%2B7f6OFfk%3D&amp;reserved=0
> > >>
> > >> It was not accepted for master because the memory code has changed a
> lot
> > >> since 17.11, and I'm working on another patchset for adjusting to the
> those
> > >> changes.
> > >>
> > >> I wonder if there is any issue with adding this patchset to stable
> 17.11.4.
> > >> Note that this makes unlikely a known limitation with emulated IOMMU
> inside
> > >> VMs.
> > >
> > > This patchset seems quite large for stable release and need to be well
> verified
> > > before GA. In -rc1 stage, we don't usually take such a large patchset
> as people
> > > have already started verification. And we don't usually release -rc2.
> If you're
> > > trying to solve a very critical issue with this patchset, I have to
> release -rc2
> > > and ask people to verify again. How critical is your issue?
> > >
> > > For the patchset,
> > > - "mem: add function for checking memsegs IOVAs addresses"
> > >  This is adding a new API, so I don't expect any API/ABI breakage, but
> want to
> > >  double-confirm with Thomas. Thomas?
> > >
> > > - "bus/pci: use IOVAs check when setting IOVA mode"
> > >  All the patches got ack except for this one but from looking at the
> threads in
> > >  dev mailing list, it looks okay. I have a question though.
> > >
> > >> @@ -640,13 +643,17 @@
> > >> {
> > >>        struct rte_pci_device *dev = NULL;
> > >>        struct rte_pci_driver *drv = NULL;
> > >> +       int iommu_dma_mask_check_done = 0;
> > >>
> > >>        FOREACH_DRIVER_ON_PCIBUS(drv) {
> > >>                FOREACH_DEVICE_ON_PCIBUS(dev) {
> > >>                        if (!rte_pci_match(drv, dev))
> > >>                                continue;
> > >> -                       if (!pci_one_device_iommu_support_va(dev))
> > >> -                               return false;
> > >> +                       if (!iommu_dma_mask_check_done) {
> > >> +                               if (pci_one_device_iommu_support_va(dev)
> < 0)
> > >
> > > pci_one_device_iommu_support_va() returns true/false(1/0), then why
> do you
> > > expect to see a negative return value in case of failure?
> >
> >
> > Emulated IOMMU has a 39 bits addressing limitation in some QEMU
> versions. With pci_one_device_iommu_support_va this is checked out, and
> if it does exist, IOMMU with VA is not supported.
> >
> > This patch avoids such coarse check using dma mask code added for
> allowing IOMMU with VA if allocated memory is below the addressing
> limitation. This is going to help for using IOMMU with VA is most of the
> systems out there, and even with systems with more than 512GB as long as
> the DPDK allocated memory is below that limit.
>
> I was asking about this change:
>
> from,
> > >> -                       if (!pci_one_device_iommu_support_va(dev))
>
> to,
> > >> +                               if (pci_one_device_iommu_support_va(dev)
> < 0)
>
>
> The original code checks zero but you changed it to check negative value.
> But it looks pci_one_device_iommu_support_va() doesn't return negative
> value, right?
>
> I thought this is buggy, please let me know.
>
>
Yes, you are right. I remember I initially changed
pci_one_device_iommu_support_va for returning an int instead of boolean,
but I did leave it as boolean at the end. It seems I forgot to modify the
call. I will send another version.

Is it OK if I send it just to stable@dpdk.org tagging fix for 17.11?

Thanks

Thanks,
> Yongseok
>
> >
> > Alejandro,
> >
> > As I will release -rc2, I can integrate your patchset but this should be
> > addressed. Please let me know.
> >
> > Thanks,
> > Yongseok
> >
> > >> +                                       return false;
> > >> +                               iommu_dma_mask_check_done  = 1;
> > >> +                       }
> > >>                }
> > >>        }
> > >>        return true;
> > >>
> > >>
> > >>
> > >> Thanks
> > >>
> > >> On Thu, Aug 16, 2018 at 8:18 PM, Yongseok Koh <yskoh@mellanox.com>
> wrote:
> > >>
> > >>> Hi all,
> > >>>
> > >>> Here is a list of patches targeted for LTS release 17.11.4. Please
> help
> > >>> review
> > >>> and test. The planned date for the final release is August 23.
> Before that,
> > >>> please shout if anyone has objections with these patches being
> applied.
> > >>>
> > >>> Also for the companies committed to running regression tests, please
> run
> > >>> the
> > >>> tests and report any issue before the release date.
> > >>>
> > >>> A release candidate tarball can be found at:
> > >>>
> > >>>    https://emea01.safelinks.protection.outlook.com/?url=
> https%3A%2F%2Fdpdk.org%2Fbrowse%2Fdpdk-stable%2Ftag%
> 2F%3Fid%3Dv17.11.4-rc1&amp;data=02%7C01%7Cyskoh%40mellanox.com%
> 7Cb1b0e3eff71c499ff3fb08d6088e1ede%7Ca652971c7d2e4d9ba6a4d149256f
> 461b%7C0%7C0%7C636705803846548967&amp;sdata=AMgyJMFIs512o5zfZ4aNSy1Ptp%
> 2BhEIMUCVZ6HaL2F40%3D&amp;reserved=0
> > >>>
> > >>> These patches are located at branch 17.11 of dpdk-stable repo:
> > >>>    https://emea01.safelinks.protection.outlook.com/?url=
> https%3A%2F%2Fdpdk.org%2Fbrowse%2Fdpdk-stable%2F&amp;data=02%7C01%7Cyskoh%
> 40mellanox.com%7Cb1b0e3eff71c499ff3fb08d6088e1ede%
> 7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636705803846548967&amp;sdata=
> hvOhEk502vVzboCbRbCZXqJXcsiI3DTtgQypQJi0Aro%3D&amp;reserved=0
> > >>>
> > >>> Thanks,
> > >>> Yongseok
> > >>>
> > >>> ---
> > >>> Adrien Mazarguil (2):
> > >>>      maintainers: update for Mellanox PMDs
> > >>>      net/mlx4: fix minor resource leak during init
> > >>>
> > >>> Ajit Khaparde (7):
> > >>>      net/bnxt: fix HW Tx checksum offload check
> > >>>      net/bnxt: fix set MTU
> > >>>      net/bnxt: fix Rx ring count limitation
> > >>>      net/bnxt: fix memory leaks in NVM commands
> > >>>      net/bnxt: fix lock release on NVM write failure
> > >>>      net/bnxt: check access denied for HWRM commands
> > >>>      net/bnxt: fix RETA size
> > >>>
> > >>> Alejandro Lucero (1):
> > >>>      net/nfp: fix field initialization in Tx descriptor
> > >>>
> > >>> Alok Makhariya (1):
> > >>>      bus/dpaa: fix phandle support for Linux 4.16
> > >>>
> > >>> Anatoly Burakov (8):
> > >>>      eal/linux: fix invalid syntax in interrupts
> > >>>      eal/linux: fix uninitialized value
> > >>>      test: fix EAL flags autotest on FreeBSD
> > >>>      test: fix result printing
> > >>>      test: fix code on report
> > >>>      test: make autotest runner python 2/3 compliant
> > >>>      test: print autotest categories
> > >>>      test: improve filtering
> > >>>
> > >>> Andrew Rybchenko (2):
> > >>>      net/sfc: cut non VLAN ID bits from TCI
> > >>>      net/sfc: fix assert in set multicast address list
> > >>>
> > >>> Andy Green (1):
> > >>>      ring: fix sign conversion warning
> > >>>
> > >>> Beilei Xing (3):
> > >>>      net/i40e: fix shifts of 32-bit value
> > >>>      net/i40e: fix packet type parsing with DDP
> > >>>      net/i40e: fix setting TPID with AQ command
> > >>>
> > >>> Bruce Richardson (2):
> > >>>      examples/exception_path: fix out-of-bounds read
> > >>>      mk: fix permissions when using make install
> > >>>
> > >>> Chas Williams (2):
> > >>>      net/bonding: always update bonding link status
> > >>>      net/bonding: do not clear active slave count
> > >>>
> > >>> Dan Gora (1):
> > >>>      kni: fix crash with null name
> > >>>
> > >>> Daria Kolistratova (1):
> > >>>      net/ena: fix SIGFPE with 0 Rx queue
> > >>>
> > >>> Dariusz Stojaczyk (1):
> > >>>      eal: fix return codes on thread naming failure
> > >>>
> > >>> David Marchand (1):
> > >>>      net/bnxt: add missing ids in xstats
> > >>>
> > >>> Drocula Lambda (1):
> > >>>      kni: fix build on RHEL 7.5
> > >>>
> > >>> Ferruh Yigit (2):
> > >>>      kni: fix build with gcc 8.1
> > >>>      net/thunderx: fix build with gcc optimization on
> > >>>
> > >>> Gavin Hu (3):
> > >>>      mk: fix cross build
> > >>>      net/dpaa2: remove loop for unused pool entries
> > >>>      maintainers: claim maintainership for ARM v7 and v8
> > >>>
> > >>> Haiyue Wang (1):
> > >>>      net/i40e: workaround performance degradation
> > >>>
> > >>> Harry van Haaren (1):
> > >>>      event: fix ring init failure handling
> > >>>
> > >>> Hemant Agrawal (2):
> > >>>      test/crypto: fix device id when stopping port
> > >>>      bus/dpaa: fix buffer offset setting in FMAN
> > >>>
> > >>> Hyong Youb Kim (1):
> > >>>      net/enic: do not overwrite admin Tx queue limit
> > >>>
> > >>> Ido Goshen (1):
> > >>>      net/pcap: fix multiple queues
> > >>>
> > >>> Jananee Parthasarathy (1):
> > >>>      mk: update targets for classified tests
> > >>>
> > >>> Jay Ding (1):
> > >>>      net/bnxt: check for invalid vNIC id
> > >>>
> > >>> Jerin Jacob (2):
> > >>>      ethdev: fix queue statistics mapping documentation
> > >>>      eal: fix bitmap documentation
> > >>>
> > >>> Kiran Kumar (2):
> > >>>      net/bonding: fix MAC address reset
> > >>>      net/thunderx: avoid sq door bell write on zero packet
> > >>>
> > >>> Konstantin Ananyev (3):
> > >>>      examples/ipsec-secgw: fix IPv4 checksum at Tx
> > >>>      examples/ipsec-secgw: fix bypass rule processing
> > >>>      app/testpmd: fix DCB config
> > >>>
> > >>> Maxime Coquelin (4):
> > >>>      vhost: improve dirty pages logging performance
> > >>>      vhost: fix missing increment of log cache count
> > >>>      vhost: flush IOTLB cache on new mem table handling
> > >>>      vhost: retranslate vring addr when memory table changes
> > >>>
> > >>> Moti Haimovsky (2):
> > >>>      net/mlx5: fix build with old kernels
> > >>>      net/mlx4: check RSS queues number limitation
> > >>>
> > >>> Nelio Laranjeiro (1):
> > >>>      net/mlx5: fix TCI mask filter
> > >>>
> > >>> Nikhil Rao (5):
> > >>>      eventdev: fix port in Rx adapter internal function
> > >>>      eventdev: fix missing update to Rx adaper WRR position
> > >>>      eventdev: add event buffer flush in Rx adapter
> > >>>      eventdev: fix internal port logic in Rx adapter
> > >>>      eventdev: fix Rx SW adapter stop
> > >>>
> > >>> Nithin Dabilpuram (1):
> > >>>      app/testpmd: fix buffer leak in TM command
> > >>>
> > >>> Ophir Munk (1):
> > >>>      net/mlx5: fix secondary process resource leakage
> > >>>
> > >>> Pablo de Lara (7):
> > >>>      examples/l2fwd-crypto: fix digest with AEAD algo
> > >>>      examples/l2fwd-crypto: check return value on IV size check
> > >>>      examples/l2fwd-crypto: skip device not supporting operation
> > >>>      test/hash: fix multiwriter with non consecutive cores
> > >>>      test/hash: fix potential memory leak
> > >>>      app/crypto-perf: fix auth IV offset
> > >>>      hash: fix doxygen of return values
> > >>>
> > >>> Pavan Nikhilesh (2):
> > >>>      event/octeontx: remove unnecessary port start and stop
> > >>>      net/octeontx: fix stop clearing Rx/Tx functions
> > >>>
> > >>> Qi Zhang (1):
> > >>>      vfio: fix PCI address comparison
> > >>>
> > >>> Radu Nicolau (3):
> > >>>      security: fix crash on destroy null session
> > >>>      test: fix uninitialized port configuration
> > >>>      net/bonding: fix race condition
> > >>>
> > >>> Rafal Kozik (4):
> > >>>      net/ena: fix GENMASK_ULL macro
> > >>>      net/ena: set link speed as none
> > >>>      net/ena: check pointer before memset
> > >>>      net/ena: change memory type
> > >>>
> > >>> Rahul Lakkireddy (1):
> > >>>      net/cxgbe: fix init failure due to new flash parts
> > >>>
> > >>> Rami Rosen (2):
> > >>>      examples/l3fwd: remove useless include
> > >>>      ethdev: fix a doxygen comment for port allocation
> > >>>
> > >>> Rasesh Mody (9):
> > >>>      net/qede: fix VF MTU update
> > >>>      net/qede: remove primary MAC removal
> > >>>      net/qede: fix for devargs
> > >>>      net/qede: fix default extended VLAN offload config
> > >>>      doc: update qede management firmware guide
> > >>>      net/qede/base: fix GRC attention callback
> > >>>      net/bnx2x: fix FW command timeout during stop
> > >>>      net/bnx2x: fix poll link status
> > >>>      net/qede/base: fix to clear HW indication
> > >>>
> > >>> Remy Horton (4):
> > >>>      bitrate: add sanity check on parameters
> > >>>      metrics: add check for invalid key
> > >>>      metrics: do not fail silently when uninitialised
> > >>>      metrics: disallow null as metric name
> > >>>
> > >>> Reshma Pattan (2):
> > >>>      test/flow_classify: fix return types
> > >>>      mk: remove unnecessary test rules
> > >>>
> > >>> Rosen Xu (1):
> > >>>      examples/flow_filtering: add flow director config for i40e
> > >>>
> > >>> Shahaf Shuler (1):
> > >>>      net/mlx5: fix compilation for rdma-core v19
> > >>>
> > >>> Shahed Shaikh (7):
> > >>>      net/qede: fix link change event notification
> > >>>      net/qede: fix legacy interrupt mode
> > >>>      net/qede: fix incorrect link status update
> > >>>      net/qede: fix unicast MAC address handling in VF
> > >>>      net/qede: fix interrupt handler unregister
> > >>>      net/qede: fix MAC address removal failure message
> > >>>      net/qede: fix ntuple filter configuration
> > >>>
> > >>> Shreyansh Jain (1):
> > >>>      doc: fix bonding command in testpmd
> > >>>
> > >>> Somnath Kotur (3):
> > >>>      net/bnxt: fix to move a flow to a different queue
> > >>>      net/bnxt: use correct flags during VLAN configuration
> > >>>      net/bnxt: fix filter freeing
> > >>>
> > >>> Thomas Monjalon (1):
> > >>>      bus/dpaa: fix build
> > >>>
> > >>> Tomasz Duszynski (1):
> > >>>      net/mvpp2: check pointer before using it
> > >>>
> > >>> Wei Zhao (7):
> > >>>      net/ixgbe: add support for VLAN in IP mode FDIR
> > >>>      net/ixgbe: fix tunnel id format error for FDIR
> > >>>      net/ixgbe: fix tunnel type set error for FDIR
> > >>>      net/ixgbe: fix mask bits register set error for FDIR
> > >>>      app/testpmd: fix VLAN TCI mask set error for FDIR
> > >>>      net/i40e: fix check of flow director programming status
> > >>>      net/i40e: revert fix of flow director check
> > >>>
> > >>> Xiaoyun Li (1):
> > >>>      net/i40e: fix link speed
> > >>>
> > >>> Xueming Li (1):
> > >>>      net/mlx5: fix crash in device probe
> > >>>
> > >>> Yipeng Wang (3):
> > >>>      hash: fix multiwriter lock memory allocation
> > >>>      hash: fix a multi-writer race condition
> > >>>      hash: fix key slot size accuracy
> > >>>
> > >>> Yongseok Koh (7):
> > >>>      net/mlx5: fix Rx buffer replenishment threshold
> > >>>      net/mlx5: add missing sanity checks for Tx completion queue
> > >>>      net/mlx5: fix assert for Tx completion queue count
> > >>>      net/mlx5: fix queue rollback when starting device
> > >>>      net/mlx5: fix error number handling
> > >>>      net/mlx5: preserve promiscuous flag for flow isolation mode
> > >>>      net/mlx5: preserve allmulticast flag for flow isolation mode
> > >>>
> >
> >
>
>