* [dpdk-announce] DPDK 16.07.2 released
From: Yuanhan Liu @ 2016-11-30 12:17 UTC (permalink / raw)
To: announce
Hi all,
Here is a new stable release for 16.07:
http://fast.dpdk.org/rel/dpdk-16.07.2.tar.xz
Thanks everyone for making it happen!
Please note that this will be the last stable release for 16.07, and
16.11 will be our first LTS, which would have longer maintenance (2
years).
--yliu
---
app/proc_info/main.c | 6 +-
app/test-pmd/cmdline.c | 70 +--
app/test-pmd/config.c | 61 ++-
app/test-pmd/testpmd.c | 50 +-
app/test-pmd/testpmd.h | 6 +
app/test/test_hash_multiwriter.c | 10 +-
doc/guides/nics/enic.rst | 53 ++-
doc/guides/nics/i40e.rst | 48 ++
doc/guides/nics/mlx5.rst | 3 +-
doc/guides/rel_notes/release_16_07.rst | 95 +++-
doc/guides/testpmd_app_ug/testpmd_funcs.rst | 3 +-
drivers/net/bnx2x/bnx2x.c | 6 +-
drivers/net/bnx2x/elink.c | 2 +-
drivers/net/bnxt/bnxt.h | 1 +
drivers/net/bnxt/bnxt_ethdev.c | 30 +-
drivers/net/bnxt/bnxt_vnic.c | 2 +-
drivers/net/bonding/rte_eth_bond_api.c | 15 -
drivers/net/bonding/rte_eth_bond_pmd.c | 10 +
drivers/net/ena/ena_ethdev.c | 10 +-
drivers/net/enic/base/vnic_rq.c | 6 +-
drivers/net/enic/base/vnic_rq.h | 1 +
drivers/net/enic/enic.h | 8 +-
drivers/net/enic/enic_clsf.c | 5 +-
drivers/net/enic/enic_ethdev.c | 10 +-
drivers/net/enic/enic_main.c | 32 +-
drivers/net/fm10k/fm10k_ethdev.c | 5 +-
drivers/net/fm10k/fm10k_rxtx.c | 10 +
drivers/net/fm10k/fm10k_rxtx_vec.c | 3 +
drivers/net/i40e/i40e_ethdev.c | 214 ++++-----
drivers/net/i40e/i40e_ethdev.h | 4 +-
drivers/net/i40e/i40e_ethdev_vf.c | 81 +---
drivers/net/i40e/i40e_pf.c | 33 +-
drivers/net/i40e/i40e_pf.h | 3 +-
drivers/net/i40e/i40e_rxtx_vec.c | 4 +-
drivers/net/ixgbe/ixgbe_fdir.c | 10 +-
drivers/net/ixgbe/ixgbe_regs.h | 40 +-
drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c | 3 +
drivers/net/mlx5/Makefile | 20 +
drivers/net/mlx5/mlx5.c | 1 +
drivers/net/mlx5/mlx5.h | 4 +
drivers/net/mlx5/mlx5_ethdev.c | 162 ++++++-
drivers/net/mlx5/mlx5_fdir.c | 270 +++++++----
drivers/net/mlx5/mlx5_prm.h | 27 ++
drivers/net/mlx5/mlx5_rxq.c | 9 +-
drivers/net/mlx5/mlx5_rxtx.c | 521 +++++----------------
drivers/net/mlx5/mlx5_rxtx.h | 7 +-
drivers/net/mlx5/mlx5_txq.c | 9 +-
drivers/net/mlx5/mlx5_vlan.c | 3 +-
drivers/net/qede/Makefile | 4 +
drivers/net/ring/rte_eth_ring.c | 2 +-
drivers/net/thunderx/nicvf_rxtx.c | 11 +-
drivers/net/virtio/virtio_ethdev.c | 14 +-
drivers/net/vmxnet3/vmxnet3_rxtx.c | 34 +-
examples/ip_pipeline/init.c | 2 +-
examples/ipsec-secgw/ipsec-secgw.c | 4 +-
examples/l2fwd-crypto/main.c | 23 +-
examples/qos_sched/app_thread.c | 22 +-
examples/tep_termination/vxlan.c | 4 +-
lib/librte_eal/common/arch/arm/rte_cpuflags.c | 1 +
lib/librte_eal/common/arch/ppc_64/rte_cpuflags.c | 1 +
lib/librte_eal/common/eal_common_pci.c | 2 +-
lib/librte_eal/common/include/rte_version.h | 2 +-
lib/librte_eal/linuxapp/kni/ethtool/igb/igb_main.c | 21 +-
lib/librte_eal/linuxapp/kni/ethtool/igb/kcompat.h | 5 +
.../linuxapp/kni/ethtool/ixgbe/ixgbe_main.c | 2 +-
lib/librte_ether/rte_ethdev.c | 15 +-
lib/librte_hash/rte_cuckoo_hash.c | 6 +-
lib/librte_hash/rte_cuckoo_hash.h | 2 +
lib/librte_hash/rte_cuckoo_hash_x86.h | 3 +-
lib/librte_lpm/rte_lpm.c | 6 +-
lib/librte_mempool/rte_mempool.c | 6 +-
lib/librte_pdump/rte_pdump.c | 4 +-
lib/librte_vhost/vhost_rxtx.c | 17 +-
pkg/dpdk.spec | 2 +-
74 files changed, 1260 insertions(+), 941 deletions(-)
Adrien Mazarguil (1):
net/mlx5: fix Rx VLAN offload capability report
Ajit Khaparde (1):
net/bnxt: fix crash when closing
Beilei Xing (1):
net/i40e: fix floating VEB
Bernard Iremonger (1):
app/testpmd: fix DCB configuration
David Marchand (1):
ethdev: fix vendor id in debug message
E. Scott Daniels (1):
ethdev: prevent duplicate event callback
Eric Kinzie (1):
net/bonding: validate speed after link up
Ferruh Yigit (4):
net/ring: fix ring device creation via devargs
net/bnx2x: fix build with icc
kni: fix build with kernel 4.8
kni: fix build with kernel 4.9
Gowrishankar Muthukrishnan (1):
examples/ip_pipeline: fix plugin loading
Igor Ryzhov (1):
pci: fix probing error if no driver found
Jasvinder Singh (1):
examples/qos_sched: fix dequeue from ring
Jeff Guo (1):
net/i40e: fix hash filter on X722
Jianbo Liu (2):
eal/arm: fix file descriptor leak when getting CPU features
eal/ppc: fix file descriptor leak when getting CPU features
Jingjing Wu (2):
net/i40e: fix DCB configuration
doc: add limitations for i40e PMD
John Daley (5):
net/enic: fix flow director
net/enic: fix crash with removed flow director filters
net/enic: fix multi-queue Rx performance
net/enic: fix crash on MTU update or Rx queue reconfigure
net/enic: fix max packet length check
John W. Linville (4):
net/ena: improve safety of string handling
net/bnxt: ensure entry length is unsigned
net/i40e: do not use VSI before NULL check
net/bnxt: fix bit shift size
Kamil Rytarowski (1):
net/thunderx: fix Tx checksum handling
Michael Qiu (2):
examples/tep_term: fix L4 length
examples/tep_term: fix packet length with multi-segments
Mohammad Abdul Awal (1):
app/testpmd: fix RSS hash key size
Nelio Laranjeiro (1):
net/mlx5: fix Rx checksum macros
Nelson Escobar (3):
net/enic: revert truncated packets counter fix
net/enic: document how to configure vNIC parameters
net/enic: fix Rx queue index when not using Rx scatter
Nipun Gupta (1):
mempool: fix leak if populate fails
Nélio Laranjeiro (7):
net/mlx5: fix Rx function selection
net/mlx5: fix hash key size retrieval
net/mlx5: support Mellanox OFED 3.4
net/mlx5: re-factorize functions
net/mlx5: fix inline logic
net/mlx5: fix link speed capability information
net/mlx5: fix support for newer link speeds
Olga Shern (1):
net/mlx5: fix link status report
Olivier Gournet (1):
net/mlx5: fix initialization in secondary process
Pablo de Lara (3):
app/test: fix hash multiwriter sequence
hash: fix unlimited cuckoo path
hash: fix bucket size usage
Piotr Azarewicz (1):
examples/l2fwd-crypto: fix verify with decrypt in chain
Qi Zhang (4):
net/ixgbe: fix out of order Rx read
net/fm10k: fix out of order Rx read
net/i40e: fix Rx hang when disable LLDP
net/i40e: fix out of order Rx read
Qiming Yang (3):
net/i40e: fix link status change interrupt
net/i40e: fix VF bonded device link down
net/i40e: fixed build error with icc
Rasesh Mody (3):
net/bnx2x: fix maximum PF queues
net/bnx2x: fix socket id for slowpath memory
net/qede/base: fix 32-bit build
Raslan Darawsheh (2):
net/mlx5: fix removing VLAN filter
net/mlx5: fix handling of small mbuf sizes
Reshma Pattan (2):
pdump: fix created directory permissions
app/procinfo: free xstats memory upon failure
Sagi Grimberg (1):
net/mlx5: fix possible NULL dereference in Rx path
Sergio Gonzalez Monroy (1):
examples/ipsec-secgw: check SP only when setup
Wei Dai (3):
lpm: fix freeing unused sub-table on rule delete
mempool: fix search of maximum contiguous pages
lpm: fix freeing memory
Wenzhuo Lu (6):
app/testpmd: fix DCB configuration
app/testpmd: fix PF/VF check of flow director
net/ixgbe: fix flow director mask
app/testpmd: fix flow director mask
app/testpmd: fix flow director endianness
net/ixgbe: fix VF registers
Xiao Wang (2):
net/fm10k: fix Rx checksum flags
net/fm10k: fix VF Tx queue initialization
Yaacov Hazan (3):
net/mlx5: fix inconsistent return value in flow director
net/mlx5: refactor allocation of flow director queues
net/mlx5: fix flow director drop mode
Yong Wang (1):
net/vmxnet3: fix mbuf release on reset/stop
Yuanhan Liu (2):
net/virtio: revert fix restart
version: 16.07.2
Zhihong Wang (1):
vhost: fix Windows VM hang
^ permalink raw reply
* IXGBE VF server crash
From: Gregory Etelson @ 2016-11-30 11:57 UTC (permalink / raw)
To: dev
Hello,
I have a server with 8 VFs over Intel 82599 10Gb adapter
A test iterates DPDK process over each VF in turn for 10 sec:
while [ 1 ]; do
for vf in $VFS; do run_dpdk $vf; done
done
After several minutes server crashes.
The crash does not trigger kdump or leaves traces on console
The server power cycles
The crash reproduced on several different servers
Does anybody familiar with this fault ?
It there a way to stop it ?
Setup:
CentOS 6.8 x86_64
ixgbe-4.3.15
DPDK-16.11
Regards,
Gregory
^ permalink raw reply
* [PATCH] net/qede: fix resource leak
From: Yong Wang @ 2016-11-30 12:32 UTC (permalink / raw)
To: harish.patil; +Cc: dev, Yong Wang
Current code does not close 'fd' on function exit, leaking resources.
Signed-off-by: Yong Wang <wang.yong19@zte.com.cn>
---
drivers/net/qede/qede_main.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/qede/qede_main.c b/drivers/net/qede/qede_main.c
index ab22409..b666e1c 100644
--- a/drivers/net/qede/qede_main.c
+++ b/drivers/net/qede/qede_main.c
@@ -137,6 +137,7 @@ static int qed_load_firmware_data(struct ecore_dev *edev)
if (fstat(fd, &st) < 0) {
DP_NOTICE(edev, false, "Can't stat firmware file\n");
+ close(fd);
return -1;
}
@@ -158,9 +159,11 @@ static int qed_load_firmware_data(struct ecore_dev *edev)
if (edev->fw_len < 104) {
DP_NOTICE(edev, false, "Invalid fw size: %" PRIu64 "\n",
edev->fw_len);
+ close(fd);
return -EINVAL;
}
+ close(fd);
return 0;
}
#endif
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH v12 0/6] add Tx preparation
From: Ananyev, Konstantin @ 2016-11-30 10:59 UTC (permalink / raw)
To: John Daley (johndale), Thomas Monjalon, dev@dpdk.org,
Rahul Lakkireddy, Stephen Hurd, Jan Medala, Jakub Palider,
Adrien Mazarguil, Alejandro Lucero, Harish Patil, Rasesh Mody,
Jerin Jacob, Yuanhan Liu, Yong Wang
Cc: Kulasek, TomaszX, olivier.matz@6wind.com
In-Reply-To: <8cd623e5433d44cb89b9599f6a60d5cd@XCH-RCD-007.cisco.com>
Hi John,
>
> Hi,
> -john
>
> > -----Original Message-----
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > Sent: Monday, November 28, 2016 3:03 AM
> > To: dev@dpdk.org; Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>;
> > Stephen Hurd <stephen.hurd@broadcom.com>; Jan Medala
> > <jan@semihalf.com>; Jakub Palider <jpa@semihalf.com>; John Daley
> > (johndale) <johndale@cisco.com>; Adrien Mazarguil
> > <adrien.mazarguil@6wind.com>; Alejandro Lucero
> > <alejandro.lucero@netronome.com>; Harish Patil
> > <harish.patil@qlogic.com>; Rasesh Mody <rasesh.mody@qlogic.com>; Jerin
> > Jacob <jerin.jacob@caviumnetworks.com>; Yuanhan Liu
> > <yuanhan.liu@linux.intel.com>; Yong Wang <yongwang@vmware.com>
> > Cc: Tomasz Kulasek <tomaszx.kulasek@intel.com>;
> > konstantin.ananyev@intel.com; olivier.matz@6wind.com
> > Subject: Re: [dpdk-dev] [PATCH v12 0/6] add Tx preparation
> >
> > We need attention of every PMD developers on this thread.
> >
> > Reminder of what Konstantin suggested:
> > "
> > - if the PMD supports TX offloads AND
> > - if to be able use any of these offloads the upper layer SW would have to:
> > * modify the contents of the packet OR
> > * obey HW specific restrictions
> > then it is a PMD developer responsibility to provide tx_prep() that would
> > implement expected modifications of the packet contents and restriction
> > checks.
> > Otherwise, tx_prep() implementation is not required and can be safely set to
> > NULL.
> > "
> >
> > I copy/paste also my previous conclusion:
> >
> > Before txprep, there is only one API: the application must prepare the
> > packets checksum itself (get_psd_sum in testpmd).
> > With txprep, the application have 2 choices: keep doing the job itself or call
> > txprep which calls a PMD-specific function.
> > The question is: does non-Intel drivers need a checksum preparation for
> > TSO?
> > Will it behave well if txprep does nothing in these drivers?
> >
> > When looking at the code, most of drivers handle the TSO flags.
> > But it is hard to know whether they rely on the pseudo checksum or not.
> >
> > git grep -l 'PKT_TX_UDP_CKSUM\|PKT_TX_TCP_CKSUM\|PKT_TX_TCP_SEG'
> > drivers/net/
> >
> > drivers/net/bnxt/bnxt_txr.c
> > drivers/net/cxgbe/sge.c
> > drivers/net/e1000/em_rxtx.c
> > drivers/net/e1000/igb_rxtx.c
> > drivers/net/ena/ena_ethdev.c
> > drivers/net/enic/enic_rxtx.c
> > drivers/net/fm10k/fm10k_rxtx.c
> > drivers/net/i40e/i40e_rxtx.c
> > drivers/net/ixgbe/ixgbe_rxtx.c
> > drivers/net/mlx4/mlx4.c
> > drivers/net/mlx5/mlx5_rxtx.c
> > drivers/net/nfp/nfp_net.c
> > drivers/net/qede/qede_rxtx.c
> > drivers/net/thunderx/nicvf_rxtx.c
> > drivers/net/virtio/virtio_rxtx.c
> > drivers/net/vmxnet3/vmxnet3_rxtx.c
> >
> > Please, we need a comment for each driver saying "it is OK, we do not need
> > any checksum preparation for TSO"
> > or
> > "yes we have to implement tx_prepare or TSO will not work in this mode"
>
> I like the idea of tx prep since it should make for cleaner apps.
>
> For enic, I believe the answer is " it is OK, we do not need any checksum preparation".
>
> Prior to now, it was necessary to set IP checksum to 0 and put in a TCP/UDP pseudo header. But there is a hardware overwrite of
> checksums option which makes preparation in software unnecessary and it is testing out well so far. I plan to enable it in 17.02. TSO is also
> being enabled for 17.02 and it does not look like any prep is required. So I'm going with " txprep NULL pointer is OK for enic", but may have
> to change my mind if something comes up in testing.
That's great thanks.
Other non-Intel PMD maintainers, please any feedback?
Konstantin
>
> -john
^ permalink raw reply
* Re: [PATCH v12 0/6] add Tx preparation
From: Ananyev, Konstantin @ 2016-11-30 10:54 UTC (permalink / raw)
To: Adrien Mazarguil, Thomas Monjalon
Cc: dev@dpdk.org, Rahul Lakkireddy, Stephen Hurd, Jan Medala,
Jakub Palider, John Daley, Alejandro Lucero, Harish Patil,
Rasesh Mody, Jerin Jacob, Yuanhan Liu, Yong Wang,
Kulasek, TomaszX, olivier.matz@6wind.com
In-Reply-To: <20161130074003.GD10340@6wind.com>
Hi Adrien,
>
> On Mon, Nov 28, 2016 at 12:03:06PM +0100, Thomas Monjalon wrote:
> > We need attention of every PMD developers on this thread.
>
> I've been following this thread from the beginning while working on rte_flow
> and wanted to see where it was headed before replying. (I know, v11 was
> submitted about 1 month ago but still.)
>
> > Reminder of what Konstantin suggested:
> > "
> > - if the PMD supports TX offloads AND
> > - if to be able use any of these offloads the upper layer SW would have to:
> > * modify the contents of the packet OR
> > * obey HW specific restrictions
> > then it is a PMD developer responsibility to provide tx_prep() that would implement
> > expected modifications of the packet contents and restriction checks.
> > Otherwise, tx_prep() implementation is not required and can be safely set to NULL.
> > "
> >
> > I copy/paste also my previous conclusion:
> >
> > Before txprep, there is only one API: the application must prepare the
> > packets checksum itself (get_psd_sum in testpmd).
> > With txprep, the application have 2 choices: keep doing the job itself
> > or call txprep which calls a PMD-specific function.
>
> Something is definitely needed here, and only PMDs can provide it. I think
> applications should not have to clear checksum fields or initialize them to
> some magic value, same goes for any other offload or hardware limitation
> that needs to be worked around.
>
> tx_prep() is one possible answer to this issue, however as mentioned in the
> original patch it can be very expensive if exposed by the PMD.
>
> Another issue I'm more concerned about is the way limitations are managed
> (struct rte_eth_desc_lim). While not officially tied to tx_prep(), this
> structure contains new fields that are only relevant to a few devices, and I
> fear it will keep growing with each new hardware quirk to manage, breaking
> ABIs in the process.
Well, if some new HW capability/limitation would arise and we'd like to support
it in DPDK, then yes we probably would need to think how to incorporate it here.
Do you have anything particular in mind here?
>
> What are applications supposed to do, check each of them regardless before
> attempting to send a burst?
>
> I understand tx_prep() automates this process, however I'm wondering why
> isn't the TX burst function doing that itself. Using nb_mtu_seg_max as an
> example, tx_prep() has an extra check in case of TSO that the TX burst
> function does not perform. This ends up being much more expensive to
> applications due to the additional loop doing redundant testing on each
> mbuf.
>
> If, say as a performance improvement, we decided to leave the validation
> part to the TX burst function; what remains in tx_prep() is basically heavy
> "preparation" requiring mbuf changes (i.e. erasing checksums, for now).
>
> Following the same logic, why can't such a thing be made part of the TX
> burst function as well (through a direct call to rte_phdr_cksum_fix()
> whenever necessary). From an application standpoint, what are the advantages
> of having to:
>
> if (tx_prep()) // iterate and update mbufs as needed
> tx_burst(); // iterate and send
>
> Compared to:
>
> tx_burst(); // iterate, update as needed and send
I think that was discussed extensively quite a lot previously here:
As Thomas already replied - main motivation is to allow user
to execute them on different stages of packet TX pipeline,
and probably on different cores.
I think that provides better flexibility to the user to when/where
do these preparations and hopefully would lead to better performance.
Though, if you or any other PMD developer/maintainer would prefer
for particular PMD to combine both functionalities into tx_burst() and
keep tx_prep() as NOP - this is still possible too.
>
> Note that PMDs could still provide different TX callbacks depending on the
> set of enabled offloads so performance is not unnecessarily impacted.
>
> In my opinion the second approach is both faster to applications and more
> friendly from a usability perspective, am I missing something obvious?
>
> > The question is: does non-Intel drivers need a checksum preparation for TSO?
> > Will it behave well if txprep does nothing in these drivers?
> >
> > When looking at the code, most of drivers handle the TSO flags.
> > But it is hard to know whether they rely on the pseudo checksum or not.
> >
> > git grep -l 'PKT_TX_UDP_CKSUM\|PKT_TX_TCP_CKSUM\|PKT_TX_TCP_SEG' drivers/net/
> >
> > drivers/net/bnxt/bnxt_txr.c
> > drivers/net/cxgbe/sge.c
> > drivers/net/e1000/em_rxtx.c
> > drivers/net/e1000/igb_rxtx.c
> > drivers/net/ena/ena_ethdev.c
> > drivers/net/enic/enic_rxtx.c
> > drivers/net/fm10k/fm10k_rxtx.c
> > drivers/net/i40e/i40e_rxtx.c
> > drivers/net/ixgbe/ixgbe_rxtx.c
> > drivers/net/mlx4/mlx4.c
> > drivers/net/mlx5/mlx5_rxtx.c
> > drivers/net/nfp/nfp_net.c
> > drivers/net/qede/qede_rxtx.c
> > drivers/net/thunderx/nicvf_rxtx.c
> > drivers/net/virtio/virtio_rxtx.c
> > drivers/net/vmxnet3/vmxnet3_rxtx.c
> >
> > Please, we need a comment for each driver saying
> > "it is OK, we do not need any checksum preparation for TSO"
> > or
> > "yes we have to implement tx_prepare or TSO will not work in this mode"
>
> For both mlx4 and mlx5 then,
> "it is OK, we do not need any checksum preparation for TSO".
>
> Actually I do not think we'll ever need tx_prep() unless we add our own
> quirks to struct rte_eth_desc_lim (and friends) which are currently quietly
> handled by TX burst functions.
Ok, so MLX PMD is not affected by these changes and tx_prep for MLX can be safely
set to NULL, correct?
Thanks
Konstantin
>
> --
> Adrien Mazarguil
> 6WIND
^ permalink raw reply
* Re: [PATCH v12 0/6] add Tx preparation
From: Kulasek, TomaszX @ 2016-11-30 10:30 UTC (permalink / raw)
To: Thomas Monjalon, Adrien Mazarguil
Cc: dev@dpdk.org, Ananyev, Konstantin, olivier.matz@6wind.com
In-Reply-To: <2285429.Z7YL4y4ekv@xps13>
Hi,
> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Wednesday, November 30, 2016 09:50
> To: Adrien Mazarguil <adrien.mazarguil@6wind.com>; Kulasek, TomaszX
> <tomaszx.kulasek@intel.com>
> Cc: dev@dpdk.org; Ananyev, Konstantin <konstantin.ananyev@intel.com>;
> olivier.matz@6wind.com
> Subject: Re: [dpdk-dev] [PATCH v12 0/6] add Tx preparation
>
> 2016-11-30 08:40, Adrien Mazarguil:
> [...]
> > I understand tx_prep() automates this process, however I'm wondering
> > why isn't the TX burst function doing that itself. Using
> > nb_mtu_seg_max as an example, tx_prep() has an extra check in case of
> > TSO that the TX burst function does not perform. This ends up being
> > much more expensive to applications due to the additional loop doing
> > redundant testing on each mbuf.
> >
> > If, say as a performance improvement, we decided to leave the
> > validation part to the TX burst function; what remains in tx_prep() is
> > basically heavy "preparation" requiring mbuf changes (i.e. erasing
> checksums, for now).
> >
> > Following the same logic, why can't such a thing be made part of the
> > TX burst function as well (through a direct call to
> > rte_phdr_cksum_fix() whenever necessary). From an application
> > standpoint, what are the advantages of having to:
> >
> > if (tx_prep()) // iterate and update mbufs as needed
> > tx_burst(); // iterate and send
> >
> > Compared to:
> >
> > tx_burst(); // iterate, update as needed and send
> >
> > Note that PMDs could still provide different TX callbacks depending on
> > the set of enabled offloads so performance is not unnecessarily
> impacted.
> >
> > In my opinion the second approach is both faster to applications and
> > more friendly from a usability perspective, am I missing something
> obvious?
>
> I think it was not clearly explained in this patchset, but this is my
> understanding:
> tx_prepare and tx_burst can be called at different stages of a pipeline,
> on different cores.
Yes, this API is intended to be used optionaly, not only just before tx_burst.
1. Separating both stages:
a) We may have a control over burst (packet content, validation) when needed.
b) For invalid packets we may restore them or do some another task if needed (even on early stage of processing).
c) Tx burst keep as simple as it should be.
2. Joining the functionality of tx_prepare and tx_burst have some disadvantages:
a) When packet is invalid it cannot be restored by application should be dropped.
b) Tx burst needs to modify the content of the packet.
c) We have no way to eliminate overhead of preparation (tx_prepare) for the application where performance is a key.
3. Using tx callbacks
a) We still need to have different implementations for different devices.
b) The overhead in performance (comparing to the pair tx_prepare/tx_burst) will not be better while both ways uses very similar mechanism.
In addition, tx_prepare mechanism can be turned off by compilation flag (as discussed with Jerin in http://dpdk.org/dev/patchwork/patch/15770/) to provide real NOOP functionality (e.g. for low-end CPUs, where even unnecessary memory dereference and check can have significant impact on performance).
Tomasz
^ permalink raw reply
* Re: [PATCH v2] virtio: tx with can_push when VERSION_1 is set
From: Yuanhan Liu @ 2016-11-30 10:19 UTC (permalink / raw)
To: Pierre Pfister (ppfister); +Cc: dev@dpdk.org, Maxime Coquelin
In-Reply-To: <C1042323-AA70-46AF-B4D2-24A470ED8DB9@cisco.com>
On Wed, Nov 30, 2016 at 09:18:42AM +0000, Pierre Pfister (ppfister) wrote:
> Current virtio driver advertises VERSION_1 support,
> but does not handle device's VERSION_1 support when
> sending packets (it looks for ANY_LAYOUT feature,
> which is absent).
>
> This patch enables 'can_push' in tx path when VERSION_1
> is advertised by the device.
>
> This significantly improves small packets forwarding rate
> towards devices advertising VERSION_1 feature.
>
> Signed-off-by: Pierre Pfister <ppfister@cisco.com>
>From the virtio spec point of view, that VERSION_1 implies ANY_LAYOUT,
this patch is right and I think we could apply this patch. No objections?
--yliu
> ---
> drivers/net/virtio/virtio_rxtx.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
> index 22d97a4..1e5a6b9 100644
> --- a/drivers/net/virtio/virtio_rxtx.c
> +++ b/drivers/net/virtio/virtio_rxtx.c
> @@ -1015,7 +1015,8 @@ virtio_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
> }
>
> /* optimize ring usage */
> - if (vtpci_with_feature(hw, VIRTIO_F_ANY_LAYOUT) &&
> + if ((vtpci_with_feature(hw, VIRTIO_F_ANY_LAYOUT) ||
> + vtpci_with_feature(hw, VIRTIO_F_VERSION_1)) &&
> rte_mbuf_refcnt_read(txm) == 1 &&
> RTE_MBUF_DIRECT(txm) &&
> txm->nb_segs == 1 &&
> --
> 2.7.4 (Apple Git-66)
^ permalink raw reply
* Re: Proposal for a new Committer model
From: Mcnamara, John @ 2016-11-30 9:58 UTC (permalink / raw)
To: Neil Horman; +Cc: dev@dpdk.org
In-Reply-To: <20161129191207.GA17132@hmsreliant.think-freely.org>
> -----Original Message-----
> From: Neil Horman [mailto:nhorman@tuxdriver.com]
> Sent: Tuesday, November 29, 2016 7:12 PM
> To: Mcnamara, John <john.mcnamara@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] Proposal for a new Committer model
>
> > ...
> >
> > B) Designate alternates to serve as backups for the maintainer when
> > they are unavailable. This provides high-availablility, and sounds
> > very much like your proposal, but in the interests of clarity, there
> > is still a single maintainer at any one time, it just may change to
> > ensure the continued merging of patches, if the primary maintainer isn't
> available.
> > Ideally however, those backup alternates arent needed, because most of
> > the primary maintainers work in merging pull requests, which are done
> > based on the trust of the submaintainer, and done during a very
> > limited window of time. This also partially addreses multi-vendor
> > fairness if your subtree maintainers come from multiple participating
> companies.
> >
> > Regards
> > Neil
> >
> >
> >
>
> Soo, I feel like we're wandering away from this thread. Are you going to
> make a change to the comitter model?
Hi,
Yes. I think we have consensus on the main parts. I'll re-draft a proposal that we can discuss and then add to the contributors guide.
John
^ permalink raw reply
* Re: [PATCH 1/1] net/i40e: enable auto link update for XXV710
From: Ferruh Yigit @ 2016-11-30 9:56 UTC (permalink / raw)
To: Zhang, Qi Z, Wu, Jingjing, Zhang, Helin; +Cc: dev@dpdk.org
In-Reply-To: <039ED4275CED7440929022BC67E7061150672A60@SHSMSX103.ccr.corp.intel.com>
On 11/30/2016 2:26 AM, Zhang, Qi Z wrote:
> Hi Ferruh:
>
>> -----Original Message-----
>> From: Yigit, Ferruh
>> Sent: Wednesday, November 30, 2016 1:46 AM
>> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>;
>> Zhang, Helin <helin.zhang@intel.com>
>> Cc: dev@dpdk.org
>> Subject: Re: [dpdk-dev] [PATCH 1/1] net/i40e: enable auto link update for
>> XXV710
>>
>> Hi Qi,
>>
>> On 11/24/2016 11:43 PM, Qi Zhang wrote:
>>> This patch remove the limitation that XXV710 device does
>>
>> XXV710 is 25G device, and support added in 16.11 (please correct me if this is
>> wrong.), but I can't find any DPDK documentation for this device.
>>
>> Can you please add some documentation, at least to http://dpdk.org/doc/nics
>> and http://dpdk.org/doc/guides/nics/i40e.html
>> in a different patch?
>
> For 16.11, since XXV710 is not officially supported, so they are missing in document.
> For 17.02 this will be updated. Thanks for remind.
If officially will be added on 17.02, can you please update release
notes too, to announce new device support?
>>
>>> not support auto link update.
>>
>> Can you please add more details that why we can remove the limitation now?
> Ok, will update in v2.
>>
>>>
>>> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
>>> ---
>>> drivers/net/i40e/i40e_ethdev.c | 8 ++++----
>>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/net/i40e/i40e_ethdev.c
>>> b/drivers/net/i40e/i40e_ethdev.c index 67778ba..b7a916d 100644
>>> --- a/drivers/net/i40e/i40e_ethdev.c
>>> +++ b/drivers/net/i40e/i40e_ethdev.c
>>> @@ -1628,6 +1628,8 @@ i40e_phy_conf_link(struct i40e_hw *hw,
>>>
>>> /* use get_phy_abilities_resp value for the rest */
>>> phy_conf.phy_type = phy_ab.phy_type;
>>> + phy_conf.phy_type_ext = phy_ab.phy_type_ext;
>>> + phy_conf.fec_config = phy_ab.mod_type_ext;
>>
>> And these changes look like called for all device types, just to double check, are
>> these 25G specific?
>
> Actually only XXV710 need this two lines, but base on firmware engineer's input,
> this could be implemented in generic way since no impact for other i40e devices.
>>
>>> phy_conf.eee_capability = phy_ab.eee_capability;
>>> phy_conf.eeer = phy_ab.eeer_val;
>>> phy_conf.low_power_ctrl = phy_ab.d3_lpan; @@ -1653,8 +1655,7 @@
>>> i40e_apply_link_speed(struct rte_eth_dev *dev)
>>> struct rte_eth_conf *conf = &dev->data->dev_conf;
>>>
>>> speed = i40e_parse_link_speeds(conf->link_speeds);
>>> - if (!I40E_PHY_TYPE_SUPPORT_25G(hw->phy.phy_types))
>>> - abilities |= I40E_AQ_PHY_ENABLE_ATOMIC_LINK;
>>> + abilities |= I40E_AQ_PHY_ENABLE_ATOMIC_LINK;
>>> if (!(conf->link_speeds & ETH_LINK_SPEED_FIXED))
>>> abilities |= I40E_AQ_PHY_AN_ENABLED;
>>> abilities |= I40E_AQ_PHY_LINK_ENABLED; @@ -1990,8 +1991,7 @@
>>> i40e_dev_set_link_down(struct rte_eth_dev *dev)
>>> uint8_t abilities = 0;
>>> struct i40e_hw *hw =
>> I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
>>>
>>> - if (!I40E_PHY_TYPE_SUPPORT_25G(hw->phy.phy_types))
>>> - abilities = I40E_AQ_PHY_ENABLE_ATOMIC_LINK;
>>> + abilities = I40E_AQ_PHY_ENABLE_ATOMIC_LINK;
>>> return i40e_phy_conf_link(hw, abilities, speed); }
>>>
>>>
> Thanks!
> Qi
>
^ permalink raw reply
* [PATCH v2] virtio: tx with can_push when VERSION_1 is set
From: Pierre Pfister (ppfister) @ 2016-11-30 9:18 UTC (permalink / raw)
To: dev@dpdk.org; +Cc: Yuanhan Liu, Maxime Coquelin
In-Reply-To: <6D3BD8A2-1567-42FA-9672-EE682D827A84@cisco.com>
Current virtio driver advertises VERSION_1 support,
but does not handle device's VERSION_1 support when
sending packets (it looks for ANY_LAYOUT feature,
which is absent).
This patch enables 'can_push' in tx path when VERSION_1
is advertised by the device.
This significantly improves small packets forwarding rate
towards devices advertising VERSION_1 feature.
Signed-off-by: Pierre Pfister <ppfister@cisco.com>
---
drivers/net/virtio/virtio_rxtx.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
index 22d97a4..1e5a6b9 100644
--- a/drivers/net/virtio/virtio_rxtx.c
+++ b/drivers/net/virtio/virtio_rxtx.c
@@ -1015,7 +1015,8 @@ virtio_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
}
/* optimize ring usage */
- if (vtpci_with_feature(hw, VIRTIO_F_ANY_LAYOUT) &&
+ if ((vtpci_with_feature(hw, VIRTIO_F_ANY_LAYOUT) ||
+ vtpci_with_feature(hw, VIRTIO_F_VERSION_1)) &&
rte_mbuf_refcnt_read(txm) == 1 &&
RTE_MBUF_DIRECT(txm) &&
txm->nb_segs == 1 &&
--
2.7.4 (Apple Git-66)
^ permalink raw reply related
* Re: [PATCH] virtio: tx with can_push when VERSION_1 is set
From: Pierre Pfister (ppfister) @ 2016-11-30 9:09 UTC (permalink / raw)
To: Maxime Coquelin; +Cc: Yuanhan Liu, dev@dpdk.org
In-Reply-To: <30923c66-8eaa-f7d4-2e6a-d65d8b254e8e@redhat.com>
> Le 22 nov. 2016 à 14:17, Maxime Coquelin <maxime.coquelin@redhat.com> a écrit :
>
> Hi Pierre,
>
> On 11/22/2016 10:54 AM, Pierre Pfister (ppfister) wrote:
>> Hello Maxime,
>>
>>> Le 9 nov. 2016 à 15:51, Maxime Coquelin <maxime.coquelin@redhat.com> a écrit :
>>>
>>> Hi Pierre,
>>>
>>> On 11/09/2016 01:42 PM, Pierre Pfister (ppfister) wrote:
>>>> Hello Maxime,
>>>>
>>>> Sorry for the late reply.
>>>>
>>>>
>>>>> Le 8 nov. 2016 à 10:44, Maxime Coquelin <maxime.coquelin@redhat.com> a écrit :
>>>>>
>>>>> Hi Pierre,
>>>>>
>>>>> On 11/08/2016 10:31 AM, Pierre Pfister (ppfister) wrote:
>>>>>> Current virtio driver advertises VERSION_1 support,
>>>>>> but does not handle device's VERSION_1 support when
>>>>>> sending packets (it looks for ANY_LAYOUT feature,
>>>>>> which is absent).
>>>>>>
>>>>>> This patch enables 'can_push' in tx path when VERSION_1
>>>>>> is advertised by the device.
>>>>>>
>>>>>> This significantly improves small packets forwarding rate
>>>>>> towards devices advertising VERSION_1 feature.
>>>>> I think it depends whether offloading is enabled or not.
>>>>> If no offloading enabled, I measured significant drop.
>>>>> Indeed, when no offloading is enabled, the Tx path in Virtio
>>>>> does not access the virtio header before your patch, as the header is memset to zero at device init time.
>>>>> With your patch, it gets memset to zero at every transmit in the hot
>>>>> path.
>>>>
>>>> Right. On the virtio side that is true, but on the device side, we have to access the header anyway.
>>> No more now, if no offload features have been negotiated.
>>> I have done a patch that landed in v16.11 to skip header parsing in
>>> this case.
>>> That said, we still have to access its descriptor.
>>>
>>>> And accessing two descriptors (with the address resolution and memory fetch which comes with it)
>>>> is a costy operation compared to a single one.
>>>> In the case indirect descriptors are used, this is 1 desc access instead or 3.
>>> I agree this is far from being optimal.
>>>
>>>> And in the case chained descriptors are used, this doubles the number of packets that you can put in your queue.
>>>>
>>>> Those are the results in my PHY -> VM (testpmd) -> PHY setup
>>>> Traffic is flowing bidirectionally. Numbers are for lossless-rates.
>>>>
>>>> When chained buffers are used for dpdk's TX: 2x2.13Mpps
>>>> When indirect descriptors are used for dpdk's TX: 2x2.38Mpps
>>>> When shallow buffers are used for dpdk's TX (with this patch): 2x2.42Mpps
>>> When I tried it, I also did PVP 0% benchmark, and I got opposite results. Chained and indirect cases were significantly better.
>>>
>>> My PVP setup was using a single NIC and single Virtio PMD, and NIC2VM
>>> forwarding was IO mode done with testpmd on host, and Rx->Tx forwarding
>>> was macswap mode on guest side.
>>>
>>> I also saw some perf regression when running simple tespmd test on both
>>> ends.
>>>
>>> Yuanhan, did you run some benchmark with your series enabling
>>> ANY_LAYOUT?
>>
>> It was enabled. But the specs specify that VERSION_1 includes ANY_LAYOUT.
>> Therefor, Qemu removes ANY_LAYOUT when VERSION_1 is set.
>>
>> We can keep arguing about which is fastest. I guess we have different setups and different results, so we probably are deadlocked here.
>> But in any case, the current code is inconsistent, as it uses single descriptor when ANY_LAYOUT is set, but not when VERSION_1 is set.
>>
>> I believe it makes sense to use single-descriptor any time it is possible, but you are free to think otherwise.
>> Please make a call and make the code consistent (removes single-descriptors all together, or use them when VERSION_1 is set too). Otherwise it just creates yet-another testing headache.
> I also think it makes sense to have a single descriptor, but I had to
> highlight that I noticed a significant performance degradation when
> using single descriptor on my setup.
>
> I'm fine we take your patch in virtio-next, so that more testing is conducted.
>
Thanks,
I just realised there was an indentation error in the patch.
Meaning that this patch didn't make it to 16.11 ...
I will send a new version.
- Pierre
> Thanks,
> Maxime
>
>>
>> Thanks,
>>
>> - Pierre
>>
>>>
>>>>
>>>> I must also note that qemu 2.5 does not seem to deal with VERSION_1 and ANY_LAYOUT correctly.
>>>> The patch I am proposing here works for qemu 2.7, but with qemu 2.5, testpmd still behaves as if ANY_LAYOUT (or VERSION_1) was not available. This is not catastrophic. But just note that you will not see performance in some cases with qemu 2.5.
>>>
>>> Thanks for the info.
>>>
>>> Regards,
>>> Maxime
>>
^ permalink raw reply
* Re: [PATCH v12 0/6] add Tx preparation
From: Thomas Monjalon @ 2016-11-30 8:50 UTC (permalink / raw)
To: Adrien Mazarguil, Tomasz Kulasek; +Cc: dev, konstantin.ananyev, olivier.matz
In-Reply-To: <20161130074003.GD10340@6wind.com>
2016-11-30 08:40, Adrien Mazarguil:
[...]
> I understand tx_prep() automates this process, however I'm wondering why
> isn't the TX burst function doing that itself. Using nb_mtu_seg_max as an
> example, tx_prep() has an extra check in case of TSO that the TX burst
> function does not perform. This ends up being much more expensive to
> applications due to the additional loop doing redundant testing on each
> mbuf.
>
> If, say as a performance improvement, we decided to leave the validation
> part to the TX burst function; what remains in tx_prep() is basically heavy
> "preparation" requiring mbuf changes (i.e. erasing checksums, for now).
>
> Following the same logic, why can't such a thing be made part of the TX
> burst function as well (through a direct call to rte_phdr_cksum_fix()
> whenever necessary). From an application standpoint, what are the advantages
> of having to:
>
> if (tx_prep()) // iterate and update mbufs as needed
> tx_burst(); // iterate and send
>
> Compared to:
>
> tx_burst(); // iterate, update as needed and send
>
> Note that PMDs could still provide different TX callbacks depending on the
> set of enabled offloads so performance is not unnecessarily impacted.
>
> In my opinion the second approach is both faster to applications and more
> friendly from a usability perspective, am I missing something obvious?
I think it was not clearly explained in this patchset, but this is
my understanding:
tx_prepare and tx_burst can be called at different stages of a pipeline,
on different cores.
^ permalink raw reply
* Re: [PATCH v12 0/6] add Tx preparation
From: Adrien Mazarguil @ 2016-11-30 7:40 UTC (permalink / raw)
To: Thomas Monjalon
Cc: dev, Rahul Lakkireddy, Stephen Hurd, Jan Medala, Jakub Palider,
John Daley, Alejandro Lucero, Harish Patil, Rasesh Mody,
Jerin Jacob, Yuanhan Liu, Yong Wang, Tomasz Kulasek,
konstantin.ananyev, olivier.matz
In-Reply-To: <8317180.L80Qf11uiu@xps13>
On Mon, Nov 28, 2016 at 12:03:06PM +0100, Thomas Monjalon wrote:
> We need attention of every PMD developers on this thread.
I've been following this thread from the beginning while working on rte_flow
and wanted to see where it was headed before replying. (I know, v11 was
submitted about 1 month ago but still.)
> Reminder of what Konstantin suggested:
> "
> - if the PMD supports TX offloads AND
> - if to be able use any of these offloads the upper layer SW would have to:
> * modify the contents of the packet OR
> * obey HW specific restrictions
> then it is a PMD developer responsibility to provide tx_prep() that would implement
> expected modifications of the packet contents and restriction checks.
> Otherwise, tx_prep() implementation is not required and can be safely set to NULL.
> "
>
> I copy/paste also my previous conclusion:
>
> Before txprep, there is only one API: the application must prepare the
> packets checksum itself (get_psd_sum in testpmd).
> With txprep, the application have 2 choices: keep doing the job itself
> or call txprep which calls a PMD-specific function.
Something is definitely needed here, and only PMDs can provide it. I think
applications should not have to clear checksum fields or initialize them to
some magic value, same goes for any other offload or hardware limitation
that needs to be worked around.
tx_prep() is one possible answer to this issue, however as mentioned in the
original patch it can be very expensive if exposed by the PMD.
Another issue I'm more concerned about is the way limitations are managed
(struct rte_eth_desc_lim). While not officially tied to tx_prep(), this
structure contains new fields that are only relevant to a few devices, and I
fear it will keep growing with each new hardware quirk to manage, breaking
ABIs in the process.
What are applications supposed to do, check each of them regardless before
attempting to send a burst?
I understand tx_prep() automates this process, however I'm wondering why
isn't the TX burst function doing that itself. Using nb_mtu_seg_max as an
example, tx_prep() has an extra check in case of TSO that the TX burst
function does not perform. This ends up being much more expensive to
applications due to the additional loop doing redundant testing on each
mbuf.
If, say as a performance improvement, we decided to leave the validation
part to the TX burst function; what remains in tx_prep() is basically heavy
"preparation" requiring mbuf changes (i.e. erasing checksums, for now).
Following the same logic, why can't such a thing be made part of the TX
burst function as well (through a direct call to rte_phdr_cksum_fix()
whenever necessary). From an application standpoint, what are the advantages
of having to:
if (tx_prep()) // iterate and update mbufs as needed
tx_burst(); // iterate and send
Compared to:
tx_burst(); // iterate, update as needed and send
Note that PMDs could still provide different TX callbacks depending on the
set of enabled offloads so performance is not unnecessarily impacted.
In my opinion the second approach is both faster to applications and more
friendly from a usability perspective, am I missing something obvious?
> The question is: does non-Intel drivers need a checksum preparation for TSO?
> Will it behave well if txprep does nothing in these drivers?
>
> When looking at the code, most of drivers handle the TSO flags.
> But it is hard to know whether they rely on the pseudo checksum or not.
>
> git grep -l 'PKT_TX_UDP_CKSUM\|PKT_TX_TCP_CKSUM\|PKT_TX_TCP_SEG' drivers/net/
>
> drivers/net/bnxt/bnxt_txr.c
> drivers/net/cxgbe/sge.c
> drivers/net/e1000/em_rxtx.c
> drivers/net/e1000/igb_rxtx.c
> drivers/net/ena/ena_ethdev.c
> drivers/net/enic/enic_rxtx.c
> drivers/net/fm10k/fm10k_rxtx.c
> drivers/net/i40e/i40e_rxtx.c
> drivers/net/ixgbe/ixgbe_rxtx.c
> drivers/net/mlx4/mlx4.c
> drivers/net/mlx5/mlx5_rxtx.c
> drivers/net/nfp/nfp_net.c
> drivers/net/qede/qede_rxtx.c
> drivers/net/thunderx/nicvf_rxtx.c
> drivers/net/virtio/virtio_rxtx.c
> drivers/net/vmxnet3/vmxnet3_rxtx.c
>
> Please, we need a comment for each driver saying
> "it is OK, we do not need any checksum preparation for TSO"
> or
> "yes we have to implement tx_prepare or TSO will not work in this mode"
For both mlx4 and mlx5 then,
"it is OK, we do not need any checksum preparation for TSO".
Actually I do not think we'll ever need tx_prep() unless we add our own
quirks to struct rte_eth_desc_lim (and friends) which are currently quietly
handled by TX burst functions.
--
Adrien Mazarguil
6WIND
^ permalink raw reply
* Re: [PATCH v12 0/6] add Tx preparation
From: John Daley (johndale) @ 2016-11-30 5:48 UTC (permalink / raw)
To: Thomas Monjalon, dev@dpdk.org, Rahul Lakkireddy, Stephen Hurd,
Jan Medala, Jakub Palider, Adrien Mazarguil, Alejandro Lucero,
Harish Patil, Rasesh Mody, Jerin Jacob, Yuanhan Liu, Yong Wang
Cc: Tomasz Kulasek, konstantin.ananyev@intel.com,
olivier.matz@6wind.com
In-Reply-To: <8317180.L80Qf11uiu@xps13>
Hi,
-john
> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Monday, November 28, 2016 3:03 AM
> To: dev@dpdk.org; Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>;
> Stephen Hurd <stephen.hurd@broadcom.com>; Jan Medala
> <jan@semihalf.com>; Jakub Palider <jpa@semihalf.com>; John Daley
> (johndale) <johndale@cisco.com>; Adrien Mazarguil
> <adrien.mazarguil@6wind.com>; Alejandro Lucero
> <alejandro.lucero@netronome.com>; Harish Patil
> <harish.patil@qlogic.com>; Rasesh Mody <rasesh.mody@qlogic.com>; Jerin
> Jacob <jerin.jacob@caviumnetworks.com>; Yuanhan Liu
> <yuanhan.liu@linux.intel.com>; Yong Wang <yongwang@vmware.com>
> Cc: Tomasz Kulasek <tomaszx.kulasek@intel.com>;
> konstantin.ananyev@intel.com; olivier.matz@6wind.com
> Subject: Re: [dpdk-dev] [PATCH v12 0/6] add Tx preparation
>
> We need attention of every PMD developers on this thread.
>
> Reminder of what Konstantin suggested:
> "
> - if the PMD supports TX offloads AND
> - if to be able use any of these offloads the upper layer SW would have to:
> * modify the contents of the packet OR
> * obey HW specific restrictions
> then it is a PMD developer responsibility to provide tx_prep() that would
> implement expected modifications of the packet contents and restriction
> checks.
> Otherwise, tx_prep() implementation is not required and can be safely set to
> NULL.
> "
>
> I copy/paste also my previous conclusion:
>
> Before txprep, there is only one API: the application must prepare the
> packets checksum itself (get_psd_sum in testpmd).
> With txprep, the application have 2 choices: keep doing the job itself or call
> txprep which calls a PMD-specific function.
> The question is: does non-Intel drivers need a checksum preparation for
> TSO?
> Will it behave well if txprep does nothing in these drivers?
>
> When looking at the code, most of drivers handle the TSO flags.
> But it is hard to know whether they rely on the pseudo checksum or not.
>
> git grep -l 'PKT_TX_UDP_CKSUM\|PKT_TX_TCP_CKSUM\|PKT_TX_TCP_SEG'
> drivers/net/
>
> drivers/net/bnxt/bnxt_txr.c
> drivers/net/cxgbe/sge.c
> drivers/net/e1000/em_rxtx.c
> drivers/net/e1000/igb_rxtx.c
> drivers/net/ena/ena_ethdev.c
> drivers/net/enic/enic_rxtx.c
> drivers/net/fm10k/fm10k_rxtx.c
> drivers/net/i40e/i40e_rxtx.c
> drivers/net/ixgbe/ixgbe_rxtx.c
> drivers/net/mlx4/mlx4.c
> drivers/net/mlx5/mlx5_rxtx.c
> drivers/net/nfp/nfp_net.c
> drivers/net/qede/qede_rxtx.c
> drivers/net/thunderx/nicvf_rxtx.c
> drivers/net/virtio/virtio_rxtx.c
> drivers/net/vmxnet3/vmxnet3_rxtx.c
>
> Please, we need a comment for each driver saying "it is OK, we do not need
> any checksum preparation for TSO"
> or
> "yes we have to implement tx_prepare or TSO will not work in this mode"
I like the idea of tx prep since it should make for cleaner apps.
For enic, I believe the answer is " it is OK, we do not need any checksum preparation".
Prior to now, it was necessary to set IP checksum to 0 and put in a TCP/UDP pseudo header. But there is a hardware overwrite of checksums option which makes preparation in software unnecessary and it is testing out well so far. I plan to enable it in 17.02. TSO is also being enabled for 17.02 and it does not look like any prep is required. So I'm going with " txprep NULL pointer is OK for enic", but may have to change my mind if something comes up in testing.
-john
^ permalink raw reply
* [PATCH v2] doc: prog_guide: fix section heading
From: Baruch Siach @ 2016-11-30 5:43 UTC (permalink / raw)
To: dev; +Cc: John McNamara, Baruch Siach
This section only deals with Tx queues configuration.
Acked-by: John McNamara <john.mcnamara@intel.com>
Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
v2: Add John's ack
---
doc/guides/prog_guide/poll_mode_drv.rst | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/doc/guides/prog_guide/poll_mode_drv.rst b/doc/guides/prog_guide/poll_mode_drv.rst
index bf3ea9fde25a..d4c92ea2cf4a 100644
--- a/doc/guides/prog_guide/poll_mode_drv.rst
+++ b/doc/guides/prog_guide/poll_mode_drv.rst
@@ -200,8 +200,8 @@ Ethernet* flow control (pause frame) can be configured on the individual port.
Refer to the testpmd source code for details.
Also, L4 (UDP/TCP/ SCTP) checksum offload by the NIC can be enabled for an individual packet as long as the packet mbuf is set up correctly. See `Hardware Offload`_ for details.
-Configuration of Transmit and Receive Queues
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+Configuration of Transmit Queues
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Each transmit queue is independently configured with the following information:
--
2.10.2
^ permalink raw reply related
* Re: [PATCH] vmxnet3: fix Rx deadlock
From: Yong Wang @ 2016-11-30 4:59 UTC (permalink / raw)
To: Stefan Puiu, dev@dpdk.org; +Cc: mac_leehk@yahoo.com.hk
In-Reply-To: <1479120376-48723-1-git-send-email-stefan.puiu@gmail.com>
> -----Original Message-----
> From: Stefan Puiu [mailto:stefan.puiu@gmail.com]
> Sent: Monday, November 14, 2016 2:46 AM
> To: dev@dpdk.org
> Cc: mac_leehk@yahoo.com.hk; Yong Wang <yongwang@vmware.com>;
> Stefan Puiu <stefan.puiu@gmail.com>
> Subject: [PATCH] vmxnet3: fix Rx deadlock
>
> Our use case is that we have an app that needs to keep mbufs around
> for a while. We've seen cases when calling vmxnet3_post_rx_bufs() from
> vmxet3_recv_pkts(), it might not succeed to add any mbufs to any RX
> descriptors (where it returns -err). Since there are no mbufs that the
> virtual hardware can use, and since nobody calls
> vmxnet3_post_rx_bufs() after that, no packets will be received after
The patch looks good overall.
I think a more accurate description is that the particular descriptor's generation bit never got flipped properly when an mbuf failed to be refilled which caused the rx stuck, rather than vmxnet3_post_rx_bufs() not being called afterwards.
> this. I call this a deadlock for lack of a better term - the virtual
> HW waits for free mbufs, while the app waits for the hardware to
> notify it for data. Note that after this, the app can't recover.
>
> This fix is a rework of this patch by Marco Lee:
> https://urldefense.proofpoint.com/v2/url?u=http-
> 3A__dpdk.org_dev_patchwork_patch_6575_&d=CwIBAg&c=Sqcl0Ez6M0X8a
> eM67LKIiDJAXVeAw-YihVMNtXt-
> uEs&r=44mSO5N5yEs4CeCdtQE0xt0F7J0p67_mApYVAzyYms0&m=g2gi3ZErdx
> AKGY8d3wbhk2D6TLUVYBs3K-
> KMdiJwuvI&s=YLz0Wsl_kQUXPWij82nnO9ROB64AK5ZtDCyUvHuU8jA&e= . I
> had to forward port it,
> address review comments and also reverted the allocation failure
> handing to the first version of the patch
s/handing/handling
> (https://urldefense.proofpoint.com/v2/url?u=http-
> 3A__dpdk.org_ml_archives_dev_2015-
> 2DJuly_022079.html&d=CwIBAg&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-
> YihVMNtXt-
> uEs&r=44mSO5N5yEs4CeCdtQE0xt0F7J0p67_mApYVAzyYms0&m=g2gi3ZErdx
> AKGY8d3wbhk2D6TLUVYBs3K-
> KMdiJwuvI&s=5HksZV8s99b3jVV7Pea60d18hKqXxp4eRpJWjz6sWLc&e= ),
> since that's
> the only approach that seems to work, and seems to be what other
> drivers are doing (I checked ixgbe and em). Reusing the mbuf that's
> getting passed to the application doesn't seem to make sense, and it
> was causing weird issues in our app. Also, reusing rxm without
> checking if it's NULL could cause the code to crash.
>
> Signed-off-by: Stefan Puiu <stefan.puiu@gmail.com>
> ---
> drivers/net/vmxnet3/vmxnet3_rxtx.c | 38
> ++++++++++++++++++++++++++++++++++++--
> 1 file changed, 36 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/vmxnet3/vmxnet3_rxtx.c
> b/drivers/net/vmxnet3/vmxnet3_rxtx.c
> index b109168..c9d2488 100644
> --- a/drivers/net/vmxnet3/vmxnet3_rxtx.c
> +++ b/drivers/net/vmxnet3/vmxnet3_rxtx.c
> @@ -518,6 +518,32 @@
> return nb_tx;
> }
>
> +static inline void
> +vmxnet3_renew_desc(vmxnet3_rx_queue_t *rxq, uint8_t ring_id,
> + struct rte_mbuf *mbuf)
Nit: align the params here to be consistent with other functions.
> +{
> + uint32_t val = 0;
Nit: extra space before "val"
> + struct vmxnet3_cmd_ring *ring = &rxq->cmd_ring[ring_id];
> + struct Vmxnet3_RxDesc *rxd =
> + (struct Vmxnet3_RxDesc *)(ring->base + ring->next2fill);
> + vmxnet3_buf_info_t *buf_info = &ring->buf_info[ring->next2fill];
> +
> + if (ring_id == 0)
> + val = VMXNET3_RXD_BTYPE_HEAD;
> + else
> + val = VMXNET3_RXD_BTYPE_BODY;
> +
> + buf_info->m = mbuf;
> + buf_info->len = (uint16_t)(mbuf->buf_len -
> RTE_PKTMBUF_HEADROOM);
> + buf_info->bufPA = rte_mbuf_data_dma_addr_default(mbuf);
> +
> + rxd->addr = buf_info->bufPA;
> + rxd->btype = val;
> + rxd->len = buf_info->len;
> + rxd->gen = ring->gen;
> +
> + vmxnet3_cmd_ring_adv_next2fill(ring);
> +}
> /*
> * Allocates mbufs and clusters. Post rx descriptors with buffer details
> * so that device can receive packets in those buffers.
> @@ -657,9 +683,17 @@
> }
>
> while (rcd->gen == rxq->comp_ring.gen) {
> + struct rte_mbuf *newm;
Nit: add a blank line here.
> if (nb_rx >= nb_pkts)
> break;
>
> + newm = rte_mbuf_raw_alloc(rxq->mp);
> + if (unlikely(newm == NULL)) {
> + PMD_RX_LOG(ERR, "Error allocating mbuf");
> + rxq->stats.rx_buf_alloc_failure++;
> + break;
> + }
> +
> idx = rcd->rxdIdx;
> ring_idx = (uint8_t)((rcd->rqID == rxq->qid1) ? 0 : 1);
> rxd = (Vmxnet3_RxDesc *)rxq->cmd_ring[ring_idx].base +
> idx;
> @@ -759,8 +793,8 @@
> VMXNET3_INC_RING_IDX_ONLY(rxq-
> >cmd_ring[ring_idx].next2comp,
> rxq->cmd_ring[ring_idx].size);
>
> - /* It's time to allocate some new buf and renew descriptors
> */
> - vmxnet3_post_rx_bufs(rxq, ring_idx);
> + /* It's time to renew descriptors */
Nit: extra space before "renew"
> + vmxnet3_renew_desc(rxq, ring_idx, newm);
> if (unlikely(rxq->shared->ctrl.updateRxProd)) {
> VMXNET3_WRITE_BAR0_REG(hw,
> rxprod_reg[ring_idx] + (rxq->queue_id * VMXNET3_REG_ALIGN),
> rxq->cmd_ring[ring_idx].next2fill);
> --
> 1.9.1
^ permalink raw reply
* [PATCH v2 1/1] net/i40e: enable auto link update for 25G
From: Qi Zhang @ 2016-11-29 20:26 UTC (permalink / raw)
To: jingjing.wu, helin.zhang; +Cc: dev, Qi Zhang
In previous patch for 25G (XXV710) enable
75d133dd329: ("net/i40e: enable 25G device"),
we intend to disable the auto linke update as a work around
for the issue that link can't be turn on when auto link update
is enabled. Now we know the root cause, there are interface
changes of AQ command "set_phy_config" and "get_phy_capabilities"
for 25G. So, this patch remove this limitation.
Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
---
v2:
- use 25G to replace XXV710 in title to consist with previous patches.
- explain more detail in commit log.
drivers/net/i40e/i40e_ethdev.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 67778ba..b7a916d 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -1628,6 +1628,8 @@ i40e_phy_conf_link(struct i40e_hw *hw,
/* use get_phy_abilities_resp value for the rest */
phy_conf.phy_type = phy_ab.phy_type;
+ phy_conf.phy_type_ext = phy_ab.phy_type_ext;
+ phy_conf.fec_config = phy_ab.mod_type_ext;
phy_conf.eee_capability = phy_ab.eee_capability;
phy_conf.eeer = phy_ab.eeer_val;
phy_conf.low_power_ctrl = phy_ab.d3_lpan;
@@ -1653,8 +1655,7 @@ i40e_apply_link_speed(struct rte_eth_dev *dev)
struct rte_eth_conf *conf = &dev->data->dev_conf;
speed = i40e_parse_link_speeds(conf->link_speeds);
- if (!I40E_PHY_TYPE_SUPPORT_25G(hw->phy.phy_types))
- abilities |= I40E_AQ_PHY_ENABLE_ATOMIC_LINK;
+ abilities |= I40E_AQ_PHY_ENABLE_ATOMIC_LINK;
if (!(conf->link_speeds & ETH_LINK_SPEED_FIXED))
abilities |= I40E_AQ_PHY_AN_ENABLED;
abilities |= I40E_AQ_PHY_LINK_ENABLED;
@@ -1990,8 +1991,7 @@ i40e_dev_set_link_down(struct rte_eth_dev *dev)
uint8_t abilities = 0;
struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
- if (!I40E_PHY_TYPE_SUPPORT_25G(hw->phy.phy_types))
- abilities = I40E_AQ_PHY_ENABLE_ATOMIC_LINK;
+ abilities = I40E_AQ_PHY_ENABLE_ATOMIC_LINK;
return i40e_phy_conf_link(hw, abilities, speed);
}
--
2.7.4
^ permalink raw reply related
* [PATCH v1] examples/ethtool: fix segfault querying non-PCI devices
From: Remy Horton @ 2016-11-30 2:47 UTC (permalink / raw)
To: dev
Doing a device information query on a non-PCI device such as
vhost was resulting in the dereferencing of a NULL pointer
(the absent PCI data), causing a segmentation fault.
Fixes: bda68ab9d1e7 ("examples/ethtool: add user-space ethtool sample application")
Signed-off-by: Remy Horton <remy.horton@intel.com>
---
doc/guides/rel_notes/release_17_02.rst | 3 +++
examples/ethtool/lib/rte_ethtool.c | 13 +++++++++----
2 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/doc/guides/rel_notes/release_17_02.rst b/doc/guides/rel_notes/release_17_02.rst
index 3b65038..f471e49 100644
--- a/doc/guides/rel_notes/release_17_02.rst
+++ b/doc/guides/rel_notes/release_17_02.rst
@@ -70,6 +70,9 @@ Libraries
Examples
~~~~~~~~
+ * **examples/ethtool Fixed crash with non-PCI devices.**
+ Querying a non-PCI device was dereferencing non-existent PCI data
+ resulting in a segmentation fault.
Other
~~~~~
diff --git a/examples/ethtool/lib/rte_ethtool.c b/examples/ethtool/lib/rte_ethtool.c
index a1f91d4..6f0ce84 100644
--- a/examples/ethtool/lib/rte_ethtool.c
+++ b/examples/ethtool/lib/rte_ethtool.c
@@ -61,10 +61,15 @@ rte_ethtool_get_drvinfo(uint8_t port_id, struct ethtool_drvinfo *drvinfo)
dev_info.driver_name);
snprintf(drvinfo->version, sizeof(drvinfo->version), "%s",
rte_version());
- snprintf(drvinfo->bus_info, sizeof(drvinfo->bus_info),
- "%04x:%02x:%02x.%x",
- dev_info.pci_dev->addr.domain, dev_info.pci_dev->addr.bus,
- dev_info.pci_dev->addr.devid, dev_info.pci_dev->addr.function);
+ if (dev_info.pci_dev)
+ snprintf(drvinfo->bus_info, sizeof(drvinfo->bus_info),
+ "%04x:%02x:%02x.%x",
+ dev_info.pci_dev->addr.domain,
+ dev_info.pci_dev->addr.bus,
+ dev_info.pci_dev->addr.devid,
+ dev_info.pci_dev->addr.function);
+ else
+ snprintf(drvinfo->bus_info, sizeof(drvinfo->bus_info), "N/A");
memset(®_info, 0, sizeof(reg_info));
rte_eth_dev_get_reg_info(port_id, ®_info);
--
2.5.5
^ permalink raw reply related
* Re: [PATCH 1/1] net/i40e: enable auto link update for XXV710
From: Zhang, Qi Z @ 2016-11-30 2:26 UTC (permalink / raw)
To: Yigit, Ferruh, Wu, Jingjing, Zhang, Helin; +Cc: dev@dpdk.org
In-Reply-To: <2219dc42-a414-0e23-6f77-b3706949af8e@intel.com>
Hi Ferruh:
> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Wednesday, November 30, 2016 1:46 AM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>;
> Zhang, Helin <helin.zhang@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 1/1] net/i40e: enable auto link update for
> XXV710
>
> Hi Qi,
>
> On 11/24/2016 11:43 PM, Qi Zhang wrote:
> > This patch remove the limitation that XXV710 device does
>
> XXV710 is 25G device, and support added in 16.11 (please correct me if this is
> wrong.), but I can't find any DPDK documentation for this device.
>
> Can you please add some documentation, at least to http://dpdk.org/doc/nics
> and http://dpdk.org/doc/guides/nics/i40e.html
> in a different patch?
For 16.11, since XXV710 is not officially supported, so they are missing in document.
For 17.02 this will be updated. Thanks for remind.
>
> > not support auto link update.
>
> Can you please add more details that why we can remove the limitation now?
Ok, will update in v2.
>
> >
> > Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> > ---
> > drivers/net/i40e/i40e_ethdev.c | 8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > b/drivers/net/i40e/i40e_ethdev.c index 67778ba..b7a916d 100644
> > --- a/drivers/net/i40e/i40e_ethdev.c
> > +++ b/drivers/net/i40e/i40e_ethdev.c
> > @@ -1628,6 +1628,8 @@ i40e_phy_conf_link(struct i40e_hw *hw,
> >
> > /* use get_phy_abilities_resp value for the rest */
> > phy_conf.phy_type = phy_ab.phy_type;
> > + phy_conf.phy_type_ext = phy_ab.phy_type_ext;
> > + phy_conf.fec_config = phy_ab.mod_type_ext;
>
> And these changes look like called for all device types, just to double check, are
> these 25G specific?
Actually only XXV710 need this two lines, but base on firmware engineer's input,
this could be implemented in generic way since no impact for other i40e devices.
>
> > phy_conf.eee_capability = phy_ab.eee_capability;
> > phy_conf.eeer = phy_ab.eeer_val;
> > phy_conf.low_power_ctrl = phy_ab.d3_lpan; @@ -1653,8 +1655,7 @@
> > i40e_apply_link_speed(struct rte_eth_dev *dev)
> > struct rte_eth_conf *conf = &dev->data->dev_conf;
> >
> > speed = i40e_parse_link_speeds(conf->link_speeds);
> > - if (!I40E_PHY_TYPE_SUPPORT_25G(hw->phy.phy_types))
> > - abilities |= I40E_AQ_PHY_ENABLE_ATOMIC_LINK;
> > + abilities |= I40E_AQ_PHY_ENABLE_ATOMIC_LINK;
> > if (!(conf->link_speeds & ETH_LINK_SPEED_FIXED))
> > abilities |= I40E_AQ_PHY_AN_ENABLED;
> > abilities |= I40E_AQ_PHY_LINK_ENABLED; @@ -1990,8 +1991,7 @@
> > i40e_dev_set_link_down(struct rte_eth_dev *dev)
> > uint8_t abilities = 0;
> > struct i40e_hw *hw =
> I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> >
> > - if (!I40E_PHY_TYPE_SUPPORT_25G(hw->phy.phy_types))
> > - abilities = I40E_AQ_PHY_ENABLE_ATOMIC_LINK;
> > + abilities = I40E_AQ_PHY_ENABLE_ATOMIC_LINK;
> > return i40e_phy_conf_link(hw, abilities, speed); }
> >
> >
Thanks!
Qi
^ permalink raw reply
* [PATCH v2] net/i40evf: fix casting between structs
From: Jingjing Wu @ 2016-11-30 2:02 UTC (permalink / raw)
To: dev; +Cc: jingjing.wu, helin.zhang
In-Reply-To: <1480239317-7827-1-git-send-email-jingjing.wu@intel.com>
Casting from structs which lay out data in typed members
to structs which have flat memory buffers, will cause
problems if the alignment of the former isn't as expected.
This patch removes the casting between structs.
Fixes: ae19955e7c86 ("i40evf: support reporting PF reset")
Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
---
v2 change:
correct the arguments in log.
add more comments.
drivers/net/i40e/i40e_ethdev_vf.c | 31 +++++++++++++++++++------------
1 file changed, 19 insertions(+), 12 deletions(-)
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index aa306d6..5ec5264 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -1336,8 +1336,9 @@ i40evf_handle_aq_msg(struct rte_eth_dev *dev)
struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
struct i40e_arq_event_info info;
- struct i40e_virtchnl_msg *v_msg;
- uint16_t pending, opcode;
+ uint16_t pending, aq_opc;
+ enum i40e_virtchnl_ops msg_opc;
+ enum i40e_status_code msg_ret;
int ret;
info.buf_len = I40E_AQ_BUF_SZ;
@@ -1346,7 +1347,6 @@ i40evf_handle_aq_msg(struct rte_eth_dev *dev)
return;
}
info.msg_buf = vf->aq_resp;
- v_msg = (struct i40e_virtchnl_msg *)&info.desc;
pending = 1;
while (pending) {
@@ -1357,32 +1357,39 @@ i40evf_handle_aq_msg(struct rte_eth_dev *dev)
"ret: %d", ret);
break;
}
- opcode = rte_le_to_cpu_16(info.desc.opcode);
-
- switch (opcode) {
+ aq_opc = rte_le_to_cpu_16(info.desc.opcode);
+ /* For the message sent from pf to vf, opcode is stored in
+ * cookie_high of struct i40e_aq_desc, while return error code
+ * are stored in cookie_low, Which is done by
+ * i40e_aq_send_msg_to_vf in PF driver.*/
+ msg_opc = (enum i40e_virtchnl_ops)rte_le_to_cpu_32(
+ info.desc.cookie_high);
+ msg_ret = (enum i40e_status_code)rte_le_to_cpu_32(
+ info.desc.cookie_low);
+ switch (aq_opc) {
case i40e_aqc_opc_send_msg_to_vf:
- if (v_msg->v_opcode == I40E_VIRTCHNL_OP_EVENT)
+ if (msg_opc == I40E_VIRTCHNL_OP_EVENT)
/* process event*/
i40evf_handle_pf_event(dev, info.msg_buf,
info.msg_len);
else {
/* read message and it's expected one */
- if (v_msg->v_opcode == vf->pend_cmd) {
- vf->cmd_retval = v_msg->v_retval;
+ if (msg_opc == vf->pend_cmd) {
+ vf->cmd_retval = msg_ret;
/* prevent compiler reordering */
rte_compiler_barrier();
_clear_cmd(vf);
} else
PMD_DRV_LOG(ERR, "command mismatch,"
"expect %u, get %u",
- vf->pend_cmd, v_msg->v_opcode);
+ vf->pend_cmd, msg_opc);
PMD_DRV_LOG(DEBUG, "adminq response is received,"
- " opcode = %d\n", v_msg->v_opcode);
+ " opcode = %d\n", msg_opc);
}
break;
default:
PMD_DRV_LOG(ERR, "Request %u is not supported yet",
- opcode);
+ aq_opc);
break;
}
}
--
2.4.11
^ permalink raw reply related
* Re: [PATCH v2] i40e: Fix eth_i40e_dev_init sequence on ThunderX
From: Asok Tiyyagura @ 2016-11-30 1:23 UTC (permalink / raw)
To: konstantin.ananyev@intel.com
Cc: skoteshwar@caviumnetworks.com, helin.zhang@intel.com,
jingjing.wu@intel.com, jerin.jacob@caviumnetworks.com,
jianbo.liu@linaro.org, dev@dpdk.org
In-Reply-To: <mailman.4.1479684107.2722.dev@dpdk.org>
I can confirm we ran into a similar issue while using ixgbe.
-Asok
Hi
>
> i40e_asq_send_command: rd32 & wr32 under ThunderX gives unpredictable
> results. To solve this include rte memory barriers
>
> Signed-off-by: Satha Rao <skoteshwar@caviumnetworks.com>
> ---
> drivers/net/i40e/base/i40e_osdep.h | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/net/i40e/base/i40e_osdep.h b/drivers/net/i40e/base/i40e_osdep.h
> index 38e7ba5..ffa3160 100644
> --- a/drivers/net/i40e/base/i40e_osdep.h
> +++ b/drivers/net/i40e/base/i40e_osdep.h
> @@ -158,7 +158,13 @@ do { \
> ((volatile uint32_t *)((char *)(a)->hw_addr + (reg)))
> static inline uint32_t i40e_read_addr(volatile void *addr)
> {
> +#if defined(RTE_ARCH_ARM64)
> + uint32_t val = rte_le_to_cpu_32(I40E_PCI_REG(addr));
> + rte_rmb();
> + return val;
If you really need an rmb/wmb with MMIO read/writes on ARM,
I think you can avoid #ifdefs here and use rte_smp_rmb/rte_smp_wmb.
BTW, I suppose if you need it for i40e, you would need it for other devices too.
Konstantin
> +#else
> return rte_le_to_cpu_32(I40E_PCI_REG(addr));
> +#endif
> }
> #define I40E_PCI_REG_WRITE(reg, value) \
> do { I40E_PCI_REG((reg)) = rte_cpu_to_le_32(value); } while (0)
> @@ -171,8 +177,16 @@ static inline uint32_t i40e_read_addr(volatile void *addr)
> I40E_PCI_REG_WRITE(I40E_PCI_REG_ADDR((hw), (reg)), (value))
>
> #define rd32(a, reg) i40e_read_addr(I40E_PCI_REG_ADDR((a), (reg)))
> +#if defined(RTE_ARCH_ARM64)
> +#define wr32(a, reg, value) \
> + do { \
> + I40E_PCI_REG_WRITE(I40E_PCI_REG_ADDR((a), (reg)), (value)); \
> + rte_wmb(); \
> + } while (0)
> +#else
> #define wr32(a, reg, value) \
> I40E_PCI_REG_WRITE(I40E_PCI_REG_ADDR((a), (reg)), (value))
> +#endif
> #define flush(a) i40e_read_addr(I40E_PCI_REG_ADDR((a), (I40E_GLGEN_STAT)))
>
> #define ARRAY_SIZE(arr) (sizeof(arr)/sizeof(arr[0]))
> --
> 2.7.4
^ permalink raw reply
* Re: [PATCH] net/i40evf: fix casting between structs
From: Wu, Jingjing @ 2016-11-30 0:35 UTC (permalink / raw)
To: Yigit, Ferruh, dev@dpdk.org; +Cc: Zhang, Helin
In-Reply-To: <2c330ced-2d11-446e-0e2b-c6dd5aa49f8b@intel.com>
> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Wednesday, November 30, 2016 12:08 AM
> To: Wu, Jingjing <jingjing.wu@intel.com>; dev@dpdk.org
> Cc: Zhang, Helin <helin.zhang@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] net/i40evf: fix casting between structs
>
> On 11/27/2016 9:35 AM, Jingjing Wu wrote:
> > Casting from structs which lay out data in typed members to structs
> > which have flat memory buffers, will cause problems if the alignment
> > of the former isn't as expected.
> > This patch removes the casting between structs.
> >
> > Fixes: ae19955e7c86 ("i40evf: support reporting PF reset")
> > Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
> > ---
> > drivers/net/i40e/i40e_ethdev_vf.c | 27 +++++++++++++++------------
> > 1 file changed, 15 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/net/i40e/i40e_ethdev_vf.c
> > b/drivers/net/i40e/i40e_ethdev_vf.c
> > index aa306d6..53d7c87 100644
> > --- a/drivers/net/i40e/i40e_ethdev_vf.c
> > +++ b/drivers/net/i40e/i40e_ethdev_vf.c
> > @@ -1336,8 +1336,9 @@ i40evf_handle_aq_msg(struct rte_eth_dev *dev)
> > struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
> > struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data-
> >dev_private);
> > struct i40e_arq_event_info info;
> > - struct i40e_virtchnl_msg *v_msg;
> > - uint16_t pending, opcode;
> > + uint16_t pending, aq_opc;
> > + enum i40e_virtchnl_ops msg_opc;
> > + enum i40e_status_code msg_ret;
> > int ret;
> >
> > info.buf_len = I40E_AQ_BUF_SZ;
> > @@ -1346,7 +1347,6 @@ i40evf_handle_aq_msg(struct rte_eth_dev *dev)
> > return;
> > }
> > info.msg_buf = vf->aq_resp;
> > - v_msg = (struct i40e_virtchnl_msg *)&info.desc;
> >
> > pending = 1;
> > while (pending) {
> > @@ -1357,32 +1357,35 @@ i40evf_handle_aq_msg(struct rte_eth_dev *dev)
> > "ret: %d", ret);
> > break;
> > }
> > - opcode = rte_le_to_cpu_16(info.desc.opcode);
> > -
> > - switch (opcode) {
> > + aq_opc = rte_le_to_cpu_16(info.desc.opcode);
> > + msg_opc = (enum i40e_virtchnl_ops)rte_le_to_cpu_32(
> > + info.desc.cookie_high);
> > + msg_ret = (enum i40e_status_code)rte_le_to_cpu_32(
> > + info.desc.cookie_low);
>
> What do you think commenting cookie_high is opcode and cookie_low is
> return_value?
>
OK. Will add some comments.
> > + switch (aq_opc) {
> > case i40e_aqc_opc_send_msg_to_vf:
> > - if (v_msg->v_opcode == I40E_VIRTCHNL_OP_EVENT)
> > + if (msg_opc == I40E_VIRTCHNL_OP_EVENT)
> > /* process event*/
> > i40evf_handle_pf_event(dev, info.msg_buf,
> > info.msg_len);
> > else {
> > /* read message and it's expected one */
> > - if (v_msg->v_opcode == vf->pend_cmd) {
> > - vf->cmd_retval = v_msg->v_retval;
> > + if (msg_opc == vf->pend_cmd) {
> > + vf->cmd_retval = msg_ret;
> > /* prevent compiler reordering */
> > rte_compiler_barrier();
> > _clear_cmd(vf);
> > } else
> > PMD_DRV_LOG(ERR, "command
> mismatch,"
> > "expect %u, get %u",
> > - vf->pend_cmd, v_msg-
> >v_opcode);
> > + vf->pend_cmd, msg_ret);
>
> s/msg_ret/msg_opc/ ?
Yes, should use msg_opc here. Thanks!
Will update!
Thanks
Jingjing
^ permalink raw reply
* Re: [PATCH v9] drivers/net:new PMD using tun/tap host interface
From: Wiles, Keith @ 2016-11-29 22:16 UTC (permalink / raw)
To: Aws Ismail; +Cc: Yigit, Ferruh, DPDK
In-Reply-To: <CAMHMu0OrRz130WSE+uVR13o8t-5xqNS9xFv3NBg98smFBbxX2g@mail.gmail.com>
> On Nov 29, 2016, at 3:36 PM, Aws Ismail <aws.ismail@gmail.com> wrote:
>
> I have verified that adding just a single tap device works with testpmd. But as soon as I try more than one tap device, I would get a coredump, e.g.:
>
> root@localhost:~# testpmd -c f -n 4 --socket-mem 512 --vdev=net_tap,iface=tap0 --vdev=net_tap,iface=tap1 -- -i
> EAL: Detected 16 lcore(s)
> EAL: Probing VFIO support...
> EAL: VFIO support initialized
> EAL: cannot open /proc/self/numa_maps, consider that all memory is in socket_id 0
> PMD: Initializing pmd_tap for net_tap as dtap0
> PMD: net_tap: Create TAP Ethernet device with 32 queues on numa 0
> PMD: Initializing pmd_tap for net_tap as dtap1
> PMD: net_tap: Create TAP Ethernet device with 32 queues on numa 0
> EAL: failed to initialize net_tap device
> PANIC in rte_eal_init():
> Cannot init pmd devices
> 6: [testpmd() [0x409149]]
> 5: [/lib/libc.so.6(__libc_start_main+0xf0) [0x7f3e65fa8740]]
> 4: [testpmd() [0x408b21]]
> 3: [/usr/lib/librte_eal.so.3.1(rte_eal_init+0xe09) [0x7f3e68ceaea9]]
> 2: [/usr/lib/librte_eal.so.3.1(__rte_panic+0xc0) [0x7f3e68ce9b5a]]
> 1: [/usr/lib/librte_eal.so.3.1(rte_dump_stack+0x18) [0x7f3e68cf2078]]
It appears the call to rte_eth_dev_allocate() in the code is wrong. I did pass in the variable called tap_name created in the function, but I was told I needed to pass in ‘name’ that was passed into the function. The rte_eth_dev_allocate() needs a unique name for each call and name is all was the same.
Need to change that line to use tap_name instead or tell me the real way to handle this problem.
If you want a new patch I can try to get it done, but I am working on something else at this time and it could be a few days before I can get the patch out.
> Aborted (core dumped)
>
> root@localhost:~#
>
>
> On Fri, Nov 25, 2016 at 2:38 PM, Aws Ismail <aws.ismail@gmail.com> wrote:
> Keith,
>
> This won't build when integrated with v16.11. The register macro
> prefix has been renamed. a v10 is needed.
>
> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> index 7f303db..297d4b6 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c
> @@ -752,5 +752,6 @@ static struct rte_vdev_driver pmd_tap_drv = {
> .remove = rte_pmd_tap_remove,
> };
>
> -DRIVER_REGISTER_VDEV(net_tap, pmd_tap_drv);
> -DRIVER_REGISTER_PARAM_STRING(net_tap, "iface=<string>,speed=N");
> +RTE_PMD_REGISTER_VDEV(net_tap, pmd_tap_drv);
> +RTE_PMD_REGISTER_ALIAS(net_tap, eth_tap);
> +RTE_PMD_REGISTER_PARAM_STRING(net_tap, "iface=<string>,speed=N");
>
> On Mon, Nov 21, 2016 at 7:56 AM, Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> > On 10/13/2016 11:03 PM, Keith Wiles wrote:
> >> The rte_eth_tap.c PMD creates a device using TUN/TAP interfaces
> >> on the local host. The PMD allows for DPDK and the host to
> >> communicate using a raw device interface on the host and in
> >> the DPDK application. The device created is a Tap device with
> >> a L2 packet header.
> >>
> >> v9 - Fix up the docs to use correct syntax
> >> v8 - Fix issue with tap_tx_queue_setup() not return zero on success.
> >> v7 - Reword the comment in common_base and fix the data->name issue
> >> v6 - fixed the checkpatch issues
> >> v5 - merge in changes from list review see related emails
> >> fixed many minor edits
> >> v4 - merge with latest driver changes
> >> v3 - fix includes by removing ifdef for other type besides Linux
> >> Fix the copyright notice in the Makefile
> >> v2 - merge all of the patches into one patch
> >> Fix a typo on naming the tap device
> >> Update the maintainers list
> >>
> >> Signed-off-by: Keith Wiles <keith.wiles@intel.com>
> >> ---
> >
> > Just a reminder, this is a new PMD and waiting for community review.
>
Regards,
Keith
^ permalink raw reply
* Re: [PATCH v9] drivers/net:new PMD using tun/tap host interface
From: Aws Ismail @ 2016-11-29 21:36 UTC (permalink / raw)
To: Keith Wiles; +Cc: Ferruh Yigit, DPDK
In-Reply-To: <CAMHMu0MzwExcOOqK51Z6kt=XJzWQ2J+jWZaJcCVb2_OZmtZhzw@mail.gmail.com>
I have verified that adding just a single tap device works with testpmd.
But as soon as I try more than one tap device, I would get a coredump, e.g.:
root@
localhost
:~# testpmd -c f -n 4 --socket-mem 512 --vdev=net_tap
,iface=tap0
--vdev=net_tap
,iface=tap1
-- -i
EAL: Detected 16 lcore(s)
EAL: Probing VFIO support...
EAL: VFIO support initialized
EAL: cannot open /proc/self/numa_maps, consider that all memory is in
socket_id 0
PMD: Initializing pmd_tap for net_tap as dtap0
PMD: net_tap: Create TAP Ethernet device with 32 queues on numa 0
PMD: Initializing pmd_tap for net_tap as dtap1
PMD: net_tap: Create TAP Ethernet device with 32 queues on numa 0
EAL: failed to initialize net_tap device
PANIC in rte_eal_init():
Cannot init pmd devices
6: [testpmd() [0x409149]]
5: [/lib/libc.so.6(__libc_start_main+0xf0) [0x7f3e65fa8740]]
4: [testpmd() [0x408b21]]
3: [/usr/lib/librte_eal.so.3.1(rte_eal_init+0xe09) [0x7f3e68ceaea9]]
2: [/usr/lib/librte_eal.so.3.1(__rte_panic+0xc0) [0x7f3e68ce9b5a]]
1: [/usr/lib/librte_eal.so.3.1(rte_dump_stack+0x18) [0x7f3e68cf2078]]
Aborted (core dumped)
root@
localhost
:~#
On Fri, Nov 25, 2016 at 2:38 PM, Aws Ismail <aws.ismail@gmail.com> wrote:
> Keith,
>
> This won't build when integrated with v16.11. The register macro
> prefix has been renamed. a v10 is needed.
>
> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> index 7f303db..297d4b6 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c
> @@ -752,5 +752,6 @@ static struct rte_vdev_driver pmd_tap_drv = {
> .remove = rte_pmd_tap_remove,
> };
>
> -DRIVER_REGISTER_VDEV(net_tap, pmd_tap_drv);
> -DRIVER_REGISTER_PARAM_STRING(net_tap, "iface=<string>,speed=N");
> +RTE_PMD_REGISTER_VDEV(net_tap, pmd_tap_drv);
> +RTE_PMD_REGISTER_ALIAS(net_tap, eth_tap);
> +RTE_PMD_REGISTER_PARAM_STRING(net_tap, "iface=<string>,speed=N");
>
> On Mon, Nov 21, 2016 at 7:56 AM, Ferruh Yigit <ferruh.yigit@intel.com>
> wrote:
> > On 10/13/2016 11:03 PM, Keith Wiles wrote:
> >> The rte_eth_tap.c PMD creates a device using TUN/TAP interfaces
> >> on the local host. The PMD allows for DPDK and the host to
> >> communicate using a raw device interface on the host and in
> >> the DPDK application. The device created is a Tap device with
> >> a L2 packet header.
> >>
> >> v9 - Fix up the docs to use correct syntax
> >> v8 - Fix issue with tap_tx_queue_setup() not return zero on success.
> >> v7 - Reword the comment in common_base and fix the data->name issue
> >> v6 - fixed the checkpatch issues
> >> v5 - merge in changes from list review see related emails
> >> fixed many minor edits
> >> v4 - merge with latest driver changes
> >> v3 - fix includes by removing ifdef for other type besides Linux
> >> Fix the copyright notice in the Makefile
> >> v2 - merge all of the patches into one patch
> >> Fix a typo on naming the tap device
> >> Update the maintainers list
> >>
> >> Signed-off-by: Keith Wiles <keith.wiles@intel.com>
> >> ---
> >
> > Just a reminder, this is a new PMD and waiting for community review.
>
^ permalink raw reply
* Re: [PATCH] doc: remove wrong document description
From: Thomas Monjalon @ 2016-11-29 20:38 UTC (permalink / raw)
To: Baruch Siach; +Cc: dev, Mcnamara, John
In-Reply-To: <20161129105421.lzcgiuygbbjabaur@tarshish>
2016-11-29 12:54, Baruch Siach:
> On Tue, Nov 29, 2016 at 09:54:58AM +0000, Mcnamara, John wrote:
> > Acked-by: John McNamara <john.mcnamara@intel.com>
>
> Thanks. For some reason patchwork didn't get your ack.
That's because you are not registered to the mailing list so your email
was in a moderation queue while John sent his ack.
You should register to avoid moderation and such issues:
http://dpdk.org/ml/listinfo/dev
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox