[dpdk-dev] Solarflare PMD submission question
On 10/28/2016 05:43 PM, Andrew Rybchenko wrote: > On 10/28/2016 03:33 PM, Thomas Monjalon wrote: >> 2016-10-28 13:50, Andrew Rybchenko: >>> The only thing which comes to my mind is to split libefx import on >>> subsystem >>> basis (few files per subsystem). It is artificial and added files will >>> be abandoned >>> until the patch which adds them into build. It could be something like: >>>1. External interfaces definition >>>2. Internal interfaces definition >>>3. Registers definition (hardware interface) >>>4. Management CPU interface definition (it is one file, but still >>> big >>> 650K) >>>5. Management CPU interface implementation >>> and so on for NIC global controls, interrupts, event queue, transmit, >>> receive, >>>filtering etc. >> Yes it is artificial. >> The most valuable would be a transversal logical split, kind of feature >> per feature, in order to explain how the device works. > > I'm not the main author of the libefx and personally would consider it > very useful. > From the other hand I understand that it is a huge amount of work to > make it. > >> Such commit is also the opportunity to explain acronyms and so on. > > Good. We'll go this way and 'll do my best to make it useful to > understand > overall structure of the code and how the device works. Now we have a split of the base driver import in big feature steps. The base driver is split into 28 patches. Just only 1 patch exceeds 300K boundary (which add MCDI definitions header). Before submitting 56 patches I'd like to double-check that checkpatch.pl errors (for example, because of assignments in the 'if' condition, parenthesis around return value) is not a show-stopper for base driver import. Andrew.
[dpdk-dev] [RFC PATCH 6/6] eal: removing eth_driver
sorry for delay in responding; somehow I didn't notice this email. On Thursday 17 November 2016 06:23 PM, Jan Blunck wrote: > On Thu, Nov 17, 2016 at 6:30 AM, Shreyansh Jain > wrote: >> This patch demonstrates how eth_driver can be replaced with appropriate >> changes for rte_xxx_driver from the PMD itself. It uses ixgbe_ethernet as >> an example. >> >> A large set of changes exists in the rte_ethdev.c - primarily because too >> much PCI centric code (names, assumption of rte_pci_device) still exists >> in it. Most, except symbol naming, has been changed in this patch. >> >> This proposes that: >> - PMD would declare the rte_xxx_driver. In case of ixgbe, it would be >>rte_pci_driver. >> - Probe and remove continue to exists in rte_pci_driver. But, the >>rte_driver has new hooks for init and uninit. The rationale is that >>once a ethernet or cryto device is created, the rte_driver->init would >>be responsible for initializing the device. >>-- Eth_dev -> rte_driver -> rte_pci_driver >> |`-> probe/remove >> `--> init/uninit > > Hmm, from my perspective this moves struct rte_driver a step closer to > struct rte_eth_dev instead of decoupling them. It is up to the > rte_driver->probe if it wants to allocate a struct rte_eth_dev, > rte_crypto_dev or the famous rte_foo_dev. That 'closeness' was my intention - to make rte_eth_dev an implementation of rte_device type. rte_eth_dev == rte_cryptodev == rte_anyother_functional_device - for the above context. All would include rte_device. As for rte_driver->probe(), it still comes in the rte_driver->init()'s role to initialize the 'generic' functional device associated with the driver. And, allowing bus specific driver (like PCI) for its individual initialization using rte_xxx_driver->probe. > > Instead of explicitly modelling rte_eth_dev specifics like init, unit > or dev_private_size I think we should delegate this to the > rte_driver->probe instead. Most of what is in rte_eth_dev_pci_probe() > today is anyway a rte_eth_dev_allocate_priv() anyway. I already have > some patches in this area in my patch stack. Can be done - either way rte_pci_driver->probe() ends up calling driver->init() (or erstwhile eth_driver->eth_dev_init()). But, I still think it is better to keep them separate. A PCI device is type of rte_device, physically. A ethernet device is type of rte_device, logically. They both should exist independently. It will help in splitting the functionality from physical layout in future - if need be. > > >> - necessary changes in the rte_eth_dev have also been done so that it >>refers to the rte_device and rte_driver rather than rte_xxx_*. This >>would imply, ethernet device is 'linked' to a rte_device/rte_driver >>which in turn is a rte_xxx_device/rte_xxx_driver type. >>- for all operations related to extraction relvant xxx type, >> container_of would have to be used. >> >> Signed-off-by: Shreyansh Jain >> --- >> drivers/net/ixgbe/ixgbe_ethdev.c | 49 >> +--- >> lib/librte_ether/rte_ethdev.c| 36 + >> lib/librte_ether/rte_ethdev.h| 6 ++--- >> 3 files changed, 51 insertions(+), 40 deletions(-) >> >> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c >> b/drivers/net/ixgbe/ixgbe_ethdev.c >> index edc9b22..acead31 100644 >> --- a/drivers/net/ixgbe/ixgbe_ethdev.c >> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c >> @@ -1419,7 +1419,7 @@ eth_ixgbevf_dev_init(struct rte_eth_dev *eth_dev) >> return 0; >> } >> >> - pci_dev = eth_dev->pci_dev; >> + pci_dev = container_of(eth_dev->device, struct rte_pci_device, >> device); >> >> rte_eth_copy_pci_info(eth_dev, pci_dev); >> >> @@ -1532,7 +1532,9 @@ static int >> eth_ixgbevf_dev_uninit(struct rte_eth_dev *eth_dev) >> { >> struct ixgbe_hw *hw; >> - struct rte_pci_device *pci_dev = eth_dev->pci_dev; >> + struct rte_pci_device *pci_dev; >> + >> + pci_dev = container_of(eth_dev->device, struct rte_pci_device, >> device); >> >> PMD_INIT_FUNC_TRACE(); >> >> @@ -1562,32 +1564,33 @@ eth_ixgbevf_dev_uninit(struct rte_eth_dev *eth_dev) >> return 0; >> } >> >> -static struct eth_driver rte_ixgbe_pmd = { >> - .pci_drv = { >> - .id_table = pci_id_ixgbe_map, >> - .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC >> | >> - RTE_PCI_DRV_DETACHABLE, >> - .probe = rte_eth_dev_pci_probe, >> - .remove = rte_eth_dev_pci_remove, >> +static struct rte_pci_driver rte_ixgbe_pci_driver = { >> + .id_table = pci_id_ixgbe_map, >> + .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC | >> +RTE_PCI_DRV_DETACHABLE, >> + .probe = rte_eth_dev_pci_probe, >> + .remove = rte_eth_dev_pci_remove, >> + .driver = { >> +
[dpdk-dev] [PATCH] crypto: remove unused digest-appended feature
On 17/11/16 17:33, Fiona Trahe wrote: > The cryptodev API had specified that if the digest address field was > left empty on an authentication operation, then the PMD would assume > the digest was appended to the source or destination data. > This case was not handled at all by most PMDs and incorrectly handled > by the QAT PMD. > As no bugs were raised, it is assumed to be not needed, so this patch > removes it, rather than add handling for the case on all PMDs. > The digest can still be appended to the data, but its > address must now be provided in the op. > > Signed-off-by: Fiona Trahe > --- > drivers/crypto/qat/qat_adf/qat_algs_build_desc.c | 2 ++ > drivers/crypto/qat/qat_crypto.c | 18 +- > lib/librte_cryptodev/rte_crypto_sym.h| 10 +- > 3 files changed, 4 insertions(+), 26 deletions(-) > Acked-by: John Griffin
[dpdk-dev] [PATCH] examples/ethtool: fix bug in drvinfo callback
Function pcmd_drvinfo_callback uses struct info to get the ethtool infomation of each port. Struct info will store the infomation of previous port until this infomation be updated. This patch fixes this issue. Fixes: bda68ab9d1e7 ("examples/ethtool: add user-space ethtool sample application") Signed-off-by: Qiming Yang --- examples/ethtool/ethtool-app/ethapp.c | 1 + 1 file changed, 1 insertion(+) diff --git a/examples/ethtool/ethtool-app/ethapp.c b/examples/ethtool/ethtool-app/ethapp.c index 9b77385..192d941 100644 --- a/examples/ethtool/ethtool-app/ethapp.c +++ b/examples/ethtool/ethtool-app/ethapp.c @@ -177,6 +177,7 @@ pcmd_drvinfo_callback(__rte_unused void *ptr_params, int id_port; for (id_port = 0; id_port < rte_eth_dev_count(); id_port++) { + memset(, 0, sizeof(info)); if (rte_ethtool_get_drvinfo(id_port, )) { printf("Error getting info for port %i\n", id_port); return; -- 2.7.4
[dpdk-dev] [PATCH v2] nfp: report link speed using hardware info
2016-11-18 16:06, Alejandro Lucero: > Previous reported speed was hardcoded. > > Signed-off-by: Alejandro Lucero > --- > drivers/net/nfp/nfp_net.c | 28 ++-- > drivers/net/nfp/nfp_net_ctrl.h | 13 + > 2 files changed, 39 insertions(+), 2 deletions(-) You should update the doc in the same patch: doc/guides/nics/features/nfp.ini It will be the first feature as the file appears to be empty. So you will need another patch to fill other existing features. I have an unrelated question: why nfp is disabled in the default build?
[dpdk-dev] [PATCH v1] maintainers: update lthreads maintainer
Signed-off-by: John McNamara --- MAINTAINERS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index d6bb8f8..2744212 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -677,7 +677,7 @@ F: examples/netmap_compat/ F: doc/guides/sample_app_ug/netmap_compatibility.rst L-threads - EXPERIMENTAL -M: Ian Betts +M: John McNamara F: examples/performance-thread/ F: doc/guides/sample_app_ug/performance_thread.rst -- 2.7.4
[dpdk-dev] [PATCH v1] maintainers: update procinfo maintainer
Update procinfo maintainer and name of the application. Signed-off-by: John McNamara --- MAINTAINERS | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index d6bb8f8..c40e9e0 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -618,9 +618,9 @@ M: Pablo de Lara F: app/test-pmd/ F: doc/guides/testpmd_app_ug/ -Dump tool +Procinfo tool M: Maryam Tahhan -M: John McNamara +M: Reshma Pattan F: app/proc_info/ F: doc/guides/tools/proc_info.rst -- 2.7.4
[dpdk-dev] [PATCH] net/bonding: improve non-ip packets RSS
Most ethernet not support non-ip packets RSS and only first queue can used to receive. In this scenario lacp bond can only use one queue even if multi queue configured. We use below formula to change the map between bond_qid and slave_qid to let at least slave_num queues to receive packets: slave_qid = (bond_qid + slave_id) % queue_num Signed-off-by: Haifeng Lin --- drivers/net/bonding/rte_eth_bond_pmd.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c index 09ce7bf..8ad843a 100644 --- a/drivers/net/bonding/rte_eth_bond_pmd.c +++ b/drivers/net/bonding/rte_eth_bond_pmd.c @@ -141,6 +141,8 @@ bond_ethdev_rx_burst_8023ad(void *queue, struct rte_mbuf **bufs, uint8_t collecting; /* current slave collecting status */ const uint8_t promisc = internals->promiscuous_en; uint8_t i, j, k; + int slave_qid, bond_qid = bd_rx_q->queue_id; + int queue_num = internals->nb_rx_queues; rte_eth_macaddr_get(internals->port_id, _mac); /* Copy slave list to protect against slave up/down changes during tx @@ -154,7 +156,9 @@ bond_ethdev_rx_burst_8023ad(void *queue, struct rte_mbuf **bufs, collecting = ACTOR_STATE(_8023ad_ports[slaves[i]], COLLECTING); /* Read packets from this slave */ - num_rx_total += rte_eth_rx_burst(slaves[i], bd_rx_q->queue_id, + slave_qid = queue_num ? (bond_qid + slaves[i]) % queue_num : + bond_qid; + num_rx_total += rte_eth_rx_burst(slaves[i], slave_qid, [num_rx_total], nb_pkts - num_rx_total); for (k = j; k < 2 && k < num_rx_total; k++) -- 1.8.3.1
[dpdk-dev] [PATCH v2] nfp: report link speed using hardware info
On Fri, Nov 18, 2016 at 4:29 PM, Thomas Monjalon wrote: > 2016-11-18 16:06, Alejandro Lucero: > > Previous reported speed was hardcoded. > > > > Signed-off-by: Alejandro Lucero > > --- > > drivers/net/nfp/nfp_net.c | 28 ++-- > > drivers/net/nfp/nfp_net_ctrl.h | 13 + > > 2 files changed, 39 insertions(+), 2 deletions(-) > > You should update the doc in the same patch: > doc/guides/nics/features/nfp.ini > It will be the first feature as the file appears to be empty. > So you will need another patch to fill other existing features. > Yes. I'm just working on updating that file properly. May I delay this doc change for including it with that other one? It will be a bit weird to just have one feature there. > > I have an unrelated question: why nfp is disabled in the default build? > > Because NFP PMD can just work if Netronome BSP is installed in the system. We do not support PF with the PMD, so it requires a Linux PF driver which comes with the BSP. The compilation has no dependencies, but we had our own UIO driver before (now using igb_uio). So basically, we wanted the people aware of this dependency and to specifically configure this option. I know what you are surely going to ask about DPDK in Linux distributions, and that this being a bad idea. The fact is, we have people using NFP PMD as part of a product, so installing that product implies to (automatically) install the BSP and a specific DPDK version with the NFP PMD enabled. But yes, maybe we should modify this and to add some sort of BSP check inside the PMD. So, thanks for the heads up. I will think about this.
[dpdk-dev] Fwd: |WARNING| [PATCH] nfp: report link speed using hardware info
2016-11-18 15:31, Alejandro Lucero: > On Fri, Nov 18, 2016 at 3:24 PM, Ferruh Yigit > wrote: > > > On 11/18/2016 3:10 PM, Alejandro Lucero wrote: > > > Hi Thomas, > > > > > > I got this email when sending a patch some minutes ago. > > > > > > The point is I trusted script/checkpatches.sh which did not report those > > > warnings. > > > Am I doing anything wrong when using checkpatches.sh? > > > > I am also getting same warnings as below, this can be related to the > > checkpatch.pl version. > > > > I have: Version: 0.32 > > (./scripts/checkpatch.pl --version) > > > Uhmm, I got same one. The last update of this version number is from 2011... I guess we have to live without checkpatch versioning.
[dpdk-dev] pmdinfogen issues: cross compilation for ARM fails with older host compiler
On Fri, Nov 18, 2016 at 08:50:52AM -0500, Neil Horman wrote: > On Fri, Nov 18, 2016 at 12:03:19PM +, Hemant Agrawal wrote: > > > -Original Message- > > > From: Neil Horman [mailto:nhorman at tuxdriver.com] > > > On Tue, Nov 15, 2016 at 09:34:16AM +, Hemant Agrawal wrote: > > > > > On Mon, Nov 14, 2016 at 02:29:24AM +0530, Jerin Jacob wrote: > > > > > > On Fri, Nov 11, 2016 at 10:34:39AM +, Hemant Agrawal wrote: > > > > > > > Hi Neil, > > > > > > >Pmdinfogen compiles with host compiler. It usages > > > rte_byteorder.h > > > > > of the target platform. > > > > > > > However, if the host compiler is older than 4.8, it will be an > > > > > > > issue during > > > cross > > > > > compilation for some platforms. > > > > > > > e.g. if we are compiling on x86 host for ARM, x86 host compiler > > > > > > > will not > > > > > understand the arm asm instructions. > > > > > > > > > > > > > > /* fix missing __builtin_bswap16 for gcc older then 4.8 */ #if > > > > > > > !(__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 8)) static > > > > > > > inline uint16_t rte_arch_bswap16(uint16_t _x) { > > > > > > >register uint16_t x = _x; > > > > > > >asm volatile ("rev16 %0,%1" > > > > > > > : "=r" (x) > > > > > > > : "r" (x) > > > > > > > ); > > > > > > >return x; > > > > > > > } > > > > > > > #endif > > > > > > > > > > > > > > One easy solution is that we add compiler platform check in this > > > > > > > code section of rte_byteorder.h e.g #if !(defined __arm__ || > > > > > > > defined > > > > > > > __aarch64__) static inline uint16_t rte_arch_bswap16(uint16_t _x) > > > > > > > { > > > > > > >return (_x >> 8) | ((_x << 8) & 0xff00); } #else ?. > > > > > > > > > > > > > > Is there a better way to fix it? > > > > > > > > > > > > IMO, It is a HOST build infrastructure issue. If a host app is using > > > > > > the dpdk service then it should compile and link against HOST > > > > > > target(in this specific case, build/x86_64-native-linuxapp-gcc). I > > > > > > think, introducing the HOSTTARGET kind of scheme is a clean > > > > > > solution. > > > > > > > > > > > > /Jerin > > > > > > > > > > > > > > > > > That would be accurate. That is to say, pmdinfogen is a tool that > > > > > should only > > > be > > > > > run on the host doing the build, by the host doing the build, and so > > > > > should be > > > > > compiled to run on the host, not on the target being built for. > > > > > > > > > > Yeah, so what we need is a way to get to the host version of > > > > > rte_byteorder.h > > > > > when building in a cross environment > > > > > > > > > +1 > > > > > > > > > Neil > > > > > > > > > > Give this a try, I've tested it on linux, but not BSD. From what I read > > > the > > > functions are not posix compliant, though they should exist on all BSD > > > and Linux > > > systems in recent history. There may be some fiddling needed for Net and > > > OpenBSD variants, but I think this is the right general direction. > > > > + 1 > > This patch works good for Linux. > > > Can someone test it on BSD? I'd like to ensure we don't need to modify it for > that platform > > Neil > > > > > > > > > > diff --git a/buildtools/pmdinfogen/pmdinfogen.h > > > b/buildtools/pmdinfogen/pmdinfogen.h > > > index 1da2966..c5ef89d 100644 > > > --- a/buildtools/pmdinfogen/pmdinfogen.h > > > +++ b/buildtools/pmdinfogen/pmdinfogen.h > > > @@ -21,7 +21,6 @@ > > > #include > > > #include > > > #include > > > -#include > > > > > > /* On BSD-alike OSes elf.h defines these according to host's word size */ > > > #undef ELF_ST_BIND > > > @@ -75,9 +74,9 @@ > > > #define CONVERT_NATIVE(fend, width, x) ({ \ > > > typeof(x) ___x; \ > > > if ((fend) == ELFDATA2LSB) \ > > > - ___x = rte_le_to_cpu_##width(x); \ > > > + ___x = le##width##toh(x); \ > > > else \ > > > - ___x = rte_be_to_cpu_##width(x); \ > > > + ___x = be##width##toh(x); \ > > > ___x; \ > > > }) > > > For compile on FreeBSD 10 we need "#include " and this works. /Bruce
[dpdk-dev] Fwd: |WARNING| [PATCH] nfp: report link speed using hardware info
2016-11-18 15:24, Ferruh Yigit: > On 11/18/2016 3:10 PM, Alejandro Lucero wrote: > > Hi Thomas, > > > > I got this email when sending a patch some minutes ago. > > > > The point is I trusted script/checkpatches.sh which did not report those > > warnings. > > Am I doing anything wrong when using checkpatches.sh? > > I am also getting same warnings as below, this can be related to the > checkpatch.pl version. > > I have: Version: 0.32 > (./scripts/checkpatch.pl --version) Yes checkpatch at dpdk.org uses the version 0.32. I could try to add it in the mail report.
[dpdk-dev] [PATCH] vdev: fix missing alias check on uninit
Fixes: d63eed6b2dca ("eal: add driver name alias") Signed-off-by: Anatoly Burakov --- lib/librte_eal/common/eal_common_vdev.c | 8 1 file changed, 8 insertions(+) diff --git a/lib/librte_eal/common/eal_common_vdev.c b/lib/librte_eal/common/eal_common_vdev.c index 0ff2377..7d6e54f 100644 --- a/lib/librte_eal/common/eal_common_vdev.c +++ b/lib/librte_eal/common/eal_common_vdev.c @@ -111,6 +111,14 @@ rte_eal_vdev_uninit(const char *name) return driver->remove(name); } + /* Give new names precedence over aliases. */ + TAILQ_FOREACH(driver, _driver_list, next) { + if (driver->driver.alias && + !strncmp(driver->driver.alias, name, + strlen(driver->driver.alias))) + return driver->remove(name); + } + RTE_LOG(ERR, EAL, "no driver found for %s\n", name); return -EINVAL; } -- 2.5.5
[dpdk-dev] [PATCH v2] nfp: report link speed using hardware info
Previous reported speed was hardcoded. Signed-off-by: Alejandro Lucero --- drivers/net/nfp/nfp_net.c | 28 ++-- drivers/net/nfp/nfp_net_ctrl.h | 13 + 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c index c6b1587..24f3164 100644 --- a/drivers/net/nfp/nfp_net.c +++ b/drivers/net/nfp/nfp_net.c @@ -816,6 +816,17 @@ static void nfp_net_read_mac(struct nfp_net_hw *hw) struct rte_eth_link link, old; uint32_t nn_link_status; + static const uint32_t ls_to_ethtool[] = { + [NFP_NET_CFG_STS_LINK_RATE_UNSUPPORTED] = ETH_SPEED_NUM_NONE, + [NFP_NET_CFG_STS_LINK_RATE_UNKNOWN] = ETH_SPEED_NUM_NONE, + [NFP_NET_CFG_STS_LINK_RATE_1G] = ETH_SPEED_NUM_1G, + [NFP_NET_CFG_STS_LINK_RATE_10G] = ETH_SPEED_NUM_10G, + [NFP_NET_CFG_STS_LINK_RATE_25G] = ETH_SPEED_NUM_25G, + [NFP_NET_CFG_STS_LINK_RATE_40G] = ETH_SPEED_NUM_40G, + [NFP_NET_CFG_STS_LINK_RATE_50G] = ETH_SPEED_NUM_50G, + [NFP_NET_CFG_STS_LINK_RATE_100G]= ETH_SPEED_NUM_100G, + }; + PMD_DRV_LOG(DEBUG, "Link update\n"); hw = NFP_NET_DEV_PRIVATE_TO_HW(dev->data->dev_private); @@ -831,8 +842,21 @@ static void nfp_net_read_mac(struct nfp_net_hw *hw) link.link_status = ETH_LINK_UP; link.link_duplex = ETH_LINK_FULL_DUPLEX; - /* Other cards can limit the tx and rx rate per VF */ - link.link_speed = ETH_SPEED_NUM_40G; + + nn_link_status = (nn_link_status >> NFP_NET_CFG_STS_LINK_RATE_SHIFT) & +NFP_NET_CFG_STS_LINK_RATE_MASK; + + if ((NFD_CFG_MAJOR_VERSION_of(hw->ver) < 4) || + ((NFD_CFG_MINOR_VERSION_of(hw->ver) == 4) && + (NFD_CFG_MINOR_VERSION_of(hw->ver) == 0))) + link.link_speed = ETH_SPEED_NUM_40G; + else { + if (nn_link_status == NFP_NET_CFG_STS_LINK_RATE_UNKNOWN || + nn_link_status >= RTE_DIM(ls_to_ethtool)) + link.link_speed = ETH_SPEED_NUM_NONE; + else + link.link_speed = ls_to_ethtool[nn_link_status]; + } if (old.link_status != link.link_status) { nfp_net_dev_atomic_write_link_status(dev, ); diff --git a/drivers/net/nfp/nfp_net_ctrl.h b/drivers/net/nfp/nfp_net_ctrl.h index fce8251..f9aaba3 100644 --- a/drivers/net/nfp/nfp_net_ctrl.h +++ b/drivers/net/nfp/nfp_net_ctrl.h @@ -157,6 +157,19 @@ #define NFP_NET_CFG_VERSION_MINOR(x)(((x) & 0xff) << 0) #define NFP_NET_CFG_STS 0x0034 #define NFP_NET_CFG_STS_LINK(0x1 << 0) /* Link up or down */ +/* Link rate */ +#define NFP_NET_CFG_STS_LINK_RATE_SHIFT 1 +#define NFP_NET_CFG_STS_LINK_RATE_MASK 0xF +#define NFP_NET_CFG_STS_LINK_RATE \ + (NFP_NET_CFG_STS_LINK_RATE_MASK << NFP_NET_CFG_STS_LINK_RATE_SHIFT) +#define NFP_NET_CFG_STS_LINK_RATE_UNSUPPORTED 0 +#define NFP_NET_CFG_STS_LINK_RATE_UNKNOWN 1 +#define NFP_NET_CFG_STS_LINK_RATE_1G2 +#define NFP_NET_CFG_STS_LINK_RATE_10G 3 +#define NFP_NET_CFG_STS_LINK_RATE_25G 4 +#define NFP_NET_CFG_STS_LINK_RATE_40G 5 +#define NFP_NET_CFG_STS_LINK_RATE_50G 6 +#define NFP_NET_CFG_STS_LINK_RATE_100G 7 #define NFP_NET_CFG_CAP 0x0038 #define NFP_NET_CFG_MAX_TXRINGS 0x003c #define NFP_NET_CFG_MAX_RXRINGS 0x0040 -- 1.9.1
[dpdk-dev] [PATCH 0/4] libeventdev API and northbound implementation
+Thomas On Fri, Nov 18, 2016 at 03:25:18PM +, Bruce Richardson wrote: > On Fri, Nov 18, 2016 at 11:14:58AM +0530, Jerin Jacob wrote: > > As previously discussed in RFC v1 [1], RFC v2 [2], with changes > > described in [3] (also pasted below), here is the first non-draft series > > for this new API. > > > > [1] http://dpdk.org/ml/archives/dev/2016-August/045181.html > > [2] http://dpdk.org/ml/archives/dev/2016-October/048592.html > > [3] http://dpdk.org/ml/archives/dev/2016-October/048196.html > > > > Changes since RFC v2: > > > > - Updated the documentation to define the need for this library[Jerin] > > - Added RTE_EVENT_QUEUE_CFG_*_ONLY configuration parameters in > > struct rte_event_queue_conf to enable optimized sw implementation [Bruce] > > - Introduced RTE_EVENT_OP* ops [Bruce] > > - Added nb_event_queue_flows,nb_event_port_dequeue_depth, > > nb_event_port_enqueue_depth > > in rte_event_dev_configure() like ethdev and crypto library[Jerin] > > - Removed rte_event_release() and replaced with RTE_EVENT_OP_RELEASE ops to > > reduce fast path APIs and it is redundant too[Jerin] > > - In the view of better application portability, Removed pin_event > > from rte_event_enqueue as it is just hint and Intel/NXP can not support > > it[Jerin] > > - Added rte_event_port_links_get()[Jerin] > > - Added rte_event_dev_dump[Harry] > > > > Notes: > > > > - This patch set is check-patch clean with an exception that > > 02/04 has one WARNING:MACRO_WITH_FLOW_CONTROL > > - Looking forward to getting additional maintainers for libeventdev > > > > > > Possible next steps: > > 1) Review this patch set > > 2) Integrate Intel's SW driver[http://dpdk.org/dev/patchwork/patch/17049/] > > 3) Review proposed examples/eventdev_pipeline > > application[http://dpdk.org/dev/patchwork/patch/17053/] > > 4) Review proposed functional > > tests[http://dpdk.org/dev/patchwork/patch/17051/] > > 5) Cavium's HW based eventdev driver > > > > I am planning to work on (3),(4) and (5) > > > Thanks Jerin, > > we'll review and get back to you with any comments or feedback (1), and > obviously start working on item (2) also! :-) > > I'm also wonder whether we should have a staging tree for this work to > make interaction between us easier. Although this may not be > finalised enough for 17.02 release, do you think having an > dpdk-eventdev-next tree would be a help? My thinking is that once we get > the eventdev library itself in reasonable shape following our review, we > could commit that and make any changes thereafter as new patches, rather > than constantly respinning the same set. It also gives us a clean git > tree to base the respective driver implementations on from our two sides. > > Thomas, any thoughts here on your end - or from anyone else? > > Regards, > /Bruce >
[dpdk-dev] [PATCH v5 4/4] latencystats: added new library for latency stats
Add a library designed to calculate latency statistics and report them to the application when queried. The library measures minimum, average and maximum latencies, and jitter in nano seconds. The current implementation supports global latency stats, i.e. per application stats. Added new field to mbuf struct to mark the packet arrival time on Rx. Modify testpmd code to initialize/uninitialize latency statistics calulation. Modify the dpdk-procinfo process to display the newly added metrics. Added new command line option "--metrics" to display metrics. APIs: * Added APIs to initialize and un initialize latency stats calculation. * Added API to retrieve latency stats names and values. Functionality: * The library will register ethdev Rx/Tx callbacks for each active port, queue combinations. * The library will register latency stats names with new metrics library. * Rx packets will be marked with time stamp on each sampling interval. * On Tx side, packets with time stamp will be considered for calculating the minimum, maximum, average latencies and also jitter. * Average latency is calculated using exponential weighted moving average method. * Minimum and maximum latencies will be low and high latency values observed so far. * Jitter calculation is done based on inter packet delay variation. * Measured stats are reported to the metrics library in a separate pthread. * Measured stats can be retrieved via get API of the libray (or) by calling generic get API of the new metrics library. Signed-off-by: Reshma Pattan Signed-off-by: Remy Horton --- MAINTAINERS| 4 + app/proc_info/main.c | 70 app/test-pmd/testpmd.c | 10 + config/common_base | 5 + doc/api/doxy-api-index.md | 1 + doc/api/doxy-api.conf | 1 + doc/guides/rel_notes/release_17_02.rst | 5 + lib/Makefile | 1 + lib/librte_latencystats/Makefile | 57 +++ lib/librte_latencystats/rte_latencystats.c | 389 + lib/librte_latencystats/rte_latencystats.h | 146 .../rte_latencystats_version.map | 10 + lib/librte_mbuf/rte_mbuf.h | 3 + mk/rte.app.mk | 2 +- 14 files changed, 703 insertions(+), 1 deletion(-) create mode 100644 lib/librte_latencystats/Makefile create mode 100644 lib/librte_latencystats/rte_latencystats.c create mode 100644 lib/librte_latencystats/rte_latencystats.h create mode 100644 lib/librte_latencystats/rte_latencystats_version.map diff --git a/MAINTAINERS b/MAINTAINERS index d6bbdd5..da7f9d1 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -713,3 +713,7 @@ F: examples/tep_termination/ F: examples/vmdq/ F: examples/vmdq_dcb/ F: doc/guides/sample_app_ug/vmdq_dcb_forwarding.rst + +Latency Stats +M: Reshma Pattan +F: lib/librte_latencystats/ diff --git a/app/proc_info/main.c b/app/proc_info/main.c index 2c56d10..33a4b39 100644 --- a/app/proc_info/main.c +++ b/app/proc_info/main.c @@ -57,6 +57,7 @@ #include #include #include +#include /* Maximum long option length for option parsing. */ #define MAX_LONG_OPT_SZ 64 @@ -68,6 +69,8 @@ static uint32_t enabled_port_mask; static uint32_t enable_stats; /**< Enable xstats. */ static uint32_t enable_xstats; +/**< Enable metrics. */ +static uint32_t enable_metrics; /**< Enable stats reset. */ static uint32_t reset_stats; /**< Enable xstats reset. */ @@ -85,6 +88,8 @@ proc_info_usage(const char *prgname) " --stats: to display port statistics, enabled by default\n" " --xstats: to display extended port statistics, disabled by " "default\n" + " --metrics: to display derived metrics of the ports, disabled by " + "default\n" " --stats-reset: to reset port statistics\n" " --xstats-reset: to reset port extended statistics\n", prgname); @@ -127,6 +132,7 @@ proc_info_parse_args(int argc, char **argv) {"stats", 0, NULL, 0}, {"stats-reset", 0, NULL, 0}, {"xstats", 0, NULL, 0}, + {"metrics", 0, NULL, 0}, {"xstats-reset", 0, NULL, 0}, {NULL, 0, 0, 0} }; @@ -159,6 +165,10 @@ proc_info_parse_args(int argc, char **argv) else if (!strncmp(long_option[option_index].name, "xstats", MAX_LONG_OPT_SZ)) enable_xstats = 1; + else if (!strncmp(long_option[option_index].name, + "metrics", + MAX_LONG_OPT_SZ)) + enable_metrics = 1;
[dpdk-dev] [PATCH v5 3/4] app/test-pmd: add support for bitrate statistics
Signed-off-by: Remy Horton --- app/test-pmd/testpmd.c | 36 1 file changed, 36 insertions(+) diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index a0332c2..60c635f 100644 --- a/app/test-pmd/testpmd.c +++ b/app/test-pmd/testpmd.c @@ -78,6 +78,10 @@ #ifdef RTE_LIBRTE_PDUMP #include #endif +#include +#ifdef RTE_LIBRTE_BITRATE +#include +#endif #include "testpmd.h" @@ -322,6 +326,9 @@ uint16_t nb_rx_queue_stats_mappings = 0; unsigned max_socket = 0; +/* Bitrate statistics */ +struct rte_stats_bitrates_s *bitrate_data; + /* Forward function declarations */ static void map_port_queue_stats_mapping_registers(uint8_t pi, struct rte_port *port); static void check_all_ports_link_status(uint32_t port_mask); @@ -921,12 +928,32 @@ run_pkt_fwd_on_lcore(struct fwd_lcore *fc, packet_fwd_t pkt_fwd) struct fwd_stream **fsm; streamid_t nb_fs; streamid_t sm_id; +#ifdef RTE_LIBRTE_BITRATE + uint64_t tics_per_1sec; + uint64_t tics_datum; + uint64_t tics_current; + uint8_t idx_port, cnt_ports; +#endif +#ifdef RTE_LIBRTE_BITRATE + cnt_ports = rte_eth_dev_count(); + tics_datum = rte_rdtsc(); + tics_per_1sec = rte_get_timer_hz(); +#endif fsm = _streams[fc->stream_idx]; nb_fs = fc->stream_nb; do { for (sm_id = 0; sm_id < nb_fs; sm_id++) (*pkt_fwd)(fsm[sm_id]); +#ifdef RTE_LIBRTE_BITRATE + tics_current = rte_rdtsc(); + if (tics_current - tics_datum >= tics_per_1sec) { + /* Periodic bitrate calculation */ + for (idx_port = 0; idx_port < cnt_ports; idx_port++) + rte_stats_bitrate_calc(bitrate_data, idx_port); + tics_datum = tics_current; + } +#endif } while (! fc->stopped); } @@ -2133,6 +2160,15 @@ main(int argc, char** argv) FOREACH_PORT(port_id, ports) rte_eth_promiscuous_enable(port_id); + /* Setup bitrate stats */ +#ifdef RTE_LIBRTE_BITRATE + bitrate_data = rte_stats_bitrate_create(); + if (bitrate_data == NULL) + rte_exit(EXIT_FAILURE, "Could not allocate bitrate data.\n"); + rte_stats_bitrate_reg(bitrate_data); +#endif + + #ifdef RTE_LIBRTE_CMDLINE if (interactive == 1) { if (auto_start) { -- 2.5.5
[dpdk-dev] [PATCH v5 1/4] lib: add information metrics library
This patch adds a new information metric library that allows other modules to register named metrics and update their values. It is intended to be independent of ethdev, rather than mixing ethdev and non-ethdev information in xstats. Signed-off-by: Remy Horton --- MAINTAINERS| 5 + config/common_base | 5 + doc/api/doxy-api-index.md | 1 + doc/api/doxy-api.conf | 1 + doc/guides/rel_notes/release_17_02.rst | 7 + lib/Makefile | 1 + lib/librte_metrics/Makefile| 51 + lib/librte_metrics/rte_metrics.c | 308 + lib/librte_metrics/rte_metrics.h | 190 ++ lib/librte_metrics/rte_metrics_version.map | 13 ++ mk/rte.app.mk | 2 + 11 files changed, 584 insertions(+) create mode 100644 lib/librte_metrics/Makefile create mode 100644 lib/librte_metrics/rte_metrics.c create mode 100644 lib/librte_metrics/rte_metrics.h create mode 100644 lib/librte_metrics/rte_metrics_version.map diff --git a/MAINTAINERS b/MAINTAINERS index d6bb8f8..52bd8a9 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -595,6 +595,11 @@ F: lib/librte_jobstats/ F: examples/l2fwd-jobstats/ F: doc/guides/sample_app_ug/l2_forward_job_stats.rst +Metrics +M: Remy Horton +F: lib/librte_metrics/ +F: doc/guides/sample_app_ug/keep_alive.rst + Test Applications - diff --git a/config/common_base b/config/common_base index 4bff83a..dedc4c3 100644 --- a/config/common_base +++ b/config/common_base @@ -589,3 +589,8 @@ CONFIG_RTE_APP_TEST_RESOURCE_TAR=n CONFIG_RTE_TEST_PMD=y CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES=n CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS=n + +# +# Compile the device metrics library +# +CONFIG_RTE_LIBRTE_METRICS=y diff --git a/doc/api/doxy-api-index.md b/doc/api/doxy-api-index.md index 6675f96..ca50fa6 100644 --- a/doc/api/doxy-api-index.md +++ b/doc/api/doxy-api-index.md @@ -147,4 +147,5 @@ There are many libraries, so their headers may be grouped by topics: [common] (@ref rte_common.h), [ABI compat] (@ref rte_compat.h), [keepalive] (@ref rte_keepalive.h), + [Device Metrics] (@ref rte_metrics.h), [version](@ref rte_version.h) diff --git a/doc/api/doxy-api.conf b/doc/api/doxy-api.conf index 9dc7ae5..fe830eb 100644 --- a/doc/api/doxy-api.conf +++ b/doc/api/doxy-api.conf @@ -57,6 +57,7 @@ INPUT = doc/api/doxy-api-index.md \ lib/librte_reorder \ lib/librte_ring \ lib/librte_sched \ + lib/librte_metrics \ lib/librte_table \ lib/librte_timer \ lib/librte_vhost diff --git a/doc/guides/rel_notes/release_17_02.rst b/doc/guides/rel_notes/release_17_02.rst index 3b65038..2d82dd1 100644 --- a/doc/guides/rel_notes/release_17_02.rst +++ b/doc/guides/rel_notes/release_17_02.rst @@ -34,6 +34,12 @@ New Features Refer to the previous release notes for examples. + * **Added information metric library.** + + A library that allows information metrics to be added and update. It is + intended to provide a reporting mechanism that is independent of the + ethdev library. + This section is a comment. do not overwrite or remove it. Also, make sure to start the actual text at the margin. = @@ -152,6 +158,7 @@ The libraries prepended with a plus sign were incremented in this version. librte_mbuf.so.2 librte_mempool.so.2 librte_meter.so.1 + + librte_metrics.so.1 librte_net.so.1 librte_pdump.so.1 librte_pipeline.so.3 diff --git a/lib/Makefile b/lib/Makefile index 990f23a..5d85dcf 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -58,6 +58,7 @@ DIRS-$(CONFIG_RTE_LIBRTE_TABLE) += librte_table DIRS-$(CONFIG_RTE_LIBRTE_PIPELINE) += librte_pipeline DIRS-$(CONFIG_RTE_LIBRTE_REORDER) += librte_reorder DIRS-$(CONFIG_RTE_LIBRTE_PDUMP) += librte_pdump +DIRS-$(CONFIG_RTE_LIBRTE_METRICS) += librte_metrics ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y) DIRS-$(CONFIG_RTE_LIBRTE_KNI) += librte_kni diff --git a/lib/librte_metrics/Makefile b/lib/librte_metrics/Makefile new file mode 100644 index 000..8d6e23a --- /dev/null +++ b/lib/librte_metrics/Makefile @@ -0,0 +1,51 @@ +# BSD LICENSE +# +# Copyright(c) 2016 Intel Corporation. All rights reserved. +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions +# are met: +# +# * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# * Redistributions in binary form
[dpdk-dev] [PATCH v5 0/4] Expanded statistics reporting
This patchset extends statistics reporting to include peak and average data-rate metrics. It comes in two parts: a statistics reporting library, and a bitrate calculation library that uses it. This structure is intended to seperate statistic reporting from ethdev and allow more flexible metric registration. Due to merge issues Reshma's latency statistics, which depends on the reporting library, has been merged into this patchset. -- v5 changes: * Updated Shared Library Versions in release notes * Merged in Reshma's latencystats library v4 changes: * References to 16.11 changed to 17.02 * Fetching of non-port values was broken * Added sanity checks to value fetching * rte_stat_value renamed to rte_metric_value * Corrected doxygen descriptions * Added MAINTAINERS entries * Added #ifdef directives to bitrate code in test-pmd v3 changes: * Marked rte_stats_bitrate_s as internal * Minor integer roundoff correction * Coding style corrections * Removed spurious object allocation * Changes to rte_metrics.[ch] moved from Patch 2/3 to 1/3. * Reintroduced non-port values (RTE_METRICS_NONPORT) * Added spinlocks to metric library * Removed spurious test registration/update * Added release notes entries v2 changes: * Uses a new metrics library rather than being part of ethdev Remy Horton (4): lib: add information metrics library lib: add bitrate statistics library app/test-pmd: add support for bitrate statistics latencystats: added new library for latency stats MAINTAINERS| 13 + app/proc_info/main.c | 70 app/test-pmd/testpmd.c | 46 +++ config/common_base | 15 + doc/api/doxy-api-index.md | 3 + doc/api/doxy-api.conf | 3 + doc/guides/rel_notes/release_17_02.rst | 18 + lib/Makefile | 3 + lib/librte_bitratestats/Makefile | 53 +++ lib/librte_bitratestats/rte_bitrate.c | 128 +++ lib/librte_bitratestats/rte_bitrate.h | 80 + .../rte_bitratestats_version.map | 9 + lib/librte_latencystats/Makefile | 57 +++ lib/librte_latencystats/rte_latencystats.c | 389 + lib/librte_latencystats/rte_latencystats.h | 146 .../rte_latencystats_version.map | 10 + lib/librte_mbuf/rte_mbuf.h | 3 + lib/librte_metrics/Makefile| 51 +++ lib/librte_metrics/rte_metrics.c | 308 lib/librte_metrics/rte_metrics.h | 190 ++ lib/librte_metrics/rte_metrics_version.map | 13 + mk/rte.app.mk | 3 + 22 files changed, 1611 insertions(+) create mode 100644 lib/librte_bitratestats/Makefile create mode 100644 lib/librte_bitratestats/rte_bitrate.c create mode 100644 lib/librte_bitratestats/rte_bitrate.h create mode 100644 lib/librte_bitratestats/rte_bitratestats_version.map create mode 100644 lib/librte_latencystats/Makefile create mode 100644 lib/librte_latencystats/rte_latencystats.c create mode 100644 lib/librte_latencystats/rte_latencystats.h create mode 100644 lib/librte_latencystats/rte_latencystats_version.map create mode 100644 lib/librte_metrics/Makefile create mode 100644 lib/librte_metrics/rte_metrics.c create mode 100644 lib/librte_metrics/rte_metrics.h create mode 100644 lib/librte_metrics/rte_metrics_version.map -- 2.5.5
[dpdk-dev] Fwd: |WARNING| [PATCH] nfp: report link speed using hardware info
On Fri, Nov 18, 2016 at 3:31 PM, Alejandro Lucero < alejandro.lucero at netronome.com> wrote: > > > On Fri, Nov 18, 2016 at 3:24 PM, Ferruh Yigit > wrote: > >> On 11/18/2016 3:10 PM, Alejandro Lucero wrote: >> > Hi Thomas, >> > >> > I got this email when sending a patch some minutes ago. >> > >> > The point is I trusted script/checkpatches.sh which did not report those >> > warnings. >> > Am I doing anything wrong when using checkpatches.sh? >> >> I am also getting same warnings as below, this can be related to the >> checkpatch.pl version. >> >> I have: Version: 0.32 >> (./scripts/checkpatch.pl --version) >> >> > Uhmm, I got same one. > > Ok. It seems I suffered a temporal blindness. I though the automatic report was about warnings but it is about checks. But I got just one of the checks messages. This is the output with -v and adding OPTIONS used: ### [PATCH] nfp: report link speed using hardware info OPTIONS: --no-tree --max-line-length=80 --show-types --ignore=LINUX_VERSION_CODE,FILE_PATH_CHANGES,VOLATILE,PREFER_PACKED,PREFER_ALIGNED,PREFER_PRINTF,PREFER_KERNEL_TYPES,BIT_MACRO,CONST_STRUCT,SPLIT_STRING,LINE_SPACING,PARENTHESIS_ALIGNMENT,NETWORKING_BLOCK_COMMENT_STYLE,NEW_TYPEDEFS,COMPARISON_TO_NULL CHECK:BRACES: Blank lines aren't necessary after an open brace '{' #60: FILE: drivers/net/nfp/nfp_net.c:856: + else { + total: 0 errors, 0 warnings, 1 checks, 68 lines checked 0/1 valid patch > > >> > >> > -- Forwarded message -- >> > From: >> > Date: Fri, Nov 18, 2016 at 3:04 PM >> > Subject: |WARNING| [PATCH] nfp: report link speed using hardware info >> > To: test-report at dpdk.org >> > Cc: Alejandro Lucero >> > >> > >> > Test-Label: checkpatch >> > Test-Status: WARNING >> > http://dpdk.org/patch/17091 >> > >> > _coding style issues_ >> > >> > >> > CHECK:MACRO_ARG_REUSE: Macro argument reuse 'arr' - possible >> side-effects? >> > #53: FILE: drivers/net/nfp/nfp_net.c:806: >> > +#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0])) >> > >> > CHECK:BRACES: Blank lines aren't necessary after an open brace '{' >> > #91: FILE: drivers/net/nfp/nfp_net.c:856: >> > + else { >> > + >> > >> > total: 0 errors, 0 warnings, 2 checks, 68 lines checked >> > >> >> >
[dpdk-dev] Fwd: |WARNING| [PATCH] nfp: report link speed using hardware info
On Fri, Nov 18, 2016 at 3:26 PM, Ferruh Yigit wrote: > On 11/18/2016 3:10 PM, Alejandro Lucero wrote: > > Hi Thomas, > > > > I got this email when sending a patch some minutes ago. > > > > The point is I trusted script/checkpatches.sh which did not report those > > warnings. > > Am I doing anything wrong when using checkpatches.sh? > > > > > > -- Forwarded message -- > > From: > > Date: Fri, Nov 18, 2016 at 3:04 PM > > Subject: |WARNING| [PATCH] nfp: report link speed using hardware info > > To: test-report at dpdk.org > > Cc: Alejandro Lucero > > > > > > Test-Label: checkpatch > > Test-Status: WARNING > > http://dpdk.org/patch/17091 > > > > _coding style issues_ > > > > > > CHECK:MACRO_ARG_REUSE: Macro argument reuse 'arr' - possible > side-effects? > > #53: FILE: drivers/net/nfp/nfp_net.c:806: > > +#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0])) > > btw, you can benefit from RTE_DIM: > > lib/librte_eal/common/include/rte_common.h:352: > #define RTE_DIM(a) (sizeof (a) / sizeof ((a)[0])) > > Thanks! I will use it in the next patch version. > > > > CHECK:BRACES: Blank lines aren't necessary after an open brace '{' > > #91: FILE: drivers/net/nfp/nfp_net.c:856: > > + else { > > + > > > > total: 0 errors, 0 warnings, 2 checks, 68 lines checked > > > >
[dpdk-dev] Fwd: |WARNING| [PATCH] nfp: report link speed using hardware info
On Fri, Nov 18, 2016 at 3:24 PM, Ferruh Yigit wrote: > On 11/18/2016 3:10 PM, Alejandro Lucero wrote: > > Hi Thomas, > > > > I got this email when sending a patch some minutes ago. > > > > The point is I trusted script/checkpatches.sh which did not report those > > warnings. > > Am I doing anything wrong when using checkpatches.sh? > > I am also getting same warnings as below, this can be related to the > checkpatch.pl version. > > I have: Version: 0.32 > (./scripts/checkpatch.pl --version) > > Uhmm, I got same one. > > > > > > -- Forwarded message -- > > From: > > Date: Fri, Nov 18, 2016 at 3:04 PM > > Subject: |WARNING| [PATCH] nfp: report link speed using hardware info > > To: test-report at dpdk.org > > Cc: Alejandro Lucero > > > > > > Test-Label: checkpatch > > Test-Status: WARNING > > http://dpdk.org/patch/17091 > > > > _coding style issues_ > > > > > > CHECK:MACRO_ARG_REUSE: Macro argument reuse 'arr' - possible > side-effects? > > #53: FILE: drivers/net/nfp/nfp_net.c:806: > > +#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0])) > > > > CHECK:BRACES: Blank lines aren't necessary after an open brace '{' > > #91: FILE: drivers/net/nfp/nfp_net.c:856: > > + else { > > + > > > > total: 0 errors, 0 warnings, 2 checks, 68 lines checked > > > >
[dpdk-dev] Fwd: |WARNING| [PATCH] nfp: report link speed using hardware info
On 11/18/2016 3:10 PM, Alejandro Lucero wrote: > Hi Thomas, > > I got this email when sending a patch some minutes ago. > > The point is I trusted script/checkpatches.sh which did not report those > warnings. > Am I doing anything wrong when using checkpatches.sh? > > > -- Forwarded message -- > From: > Date: Fri, Nov 18, 2016 at 3:04 PM > Subject: |WARNING| [PATCH] nfp: report link speed using hardware info > To: test-report at dpdk.org > Cc: Alejandro Lucero > > > Test-Label: checkpatch > Test-Status: WARNING > http://dpdk.org/patch/17091 > > _coding style issues_ > > > CHECK:MACRO_ARG_REUSE: Macro argument reuse 'arr' - possible side-effects? > #53: FILE: drivers/net/nfp/nfp_net.c:806: > +#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0])) btw, you can benefit from RTE_DIM: lib/librte_eal/common/include/rte_common.h:352: #define RTE_DIM(a) (sizeof (a) / sizeof ((a)[0])) > > CHECK:BRACES: Blank lines aren't necessary after an open brace '{' > #91: FILE: drivers/net/nfp/nfp_net.c:856: > + else { > + > > total: 0 errors, 0 warnings, 2 checks, 68 lines checked >
[dpdk-dev] [PATCH 0/4] libeventdev API and northbound implementation
On Fri, Nov 18, 2016 at 11:14:58AM +0530, Jerin Jacob wrote: > As previously discussed in RFC v1 [1], RFC v2 [2], with changes > described in [3] (also pasted below), here is the first non-draft series > for this new API. > > [1] http://dpdk.org/ml/archives/dev/2016-August/045181.html > [2] http://dpdk.org/ml/archives/dev/2016-October/048592.html > [3] http://dpdk.org/ml/archives/dev/2016-October/048196.html > > Changes since RFC v2: > > - Updated the documentation to define the need for this library[Jerin] > - Added RTE_EVENT_QUEUE_CFG_*_ONLY configuration parameters in > struct rte_event_queue_conf to enable optimized sw implementation [Bruce] > - Introduced RTE_EVENT_OP* ops [Bruce] > - Added nb_event_queue_flows,nb_event_port_dequeue_depth, > nb_event_port_enqueue_depth > in rte_event_dev_configure() like ethdev and crypto library[Jerin] > - Removed rte_event_release() and replaced with RTE_EVENT_OP_RELEASE ops to > reduce fast path APIs and it is redundant too[Jerin] > - In the view of better application portability, Removed pin_event > from rte_event_enqueue as it is just hint and Intel/NXP can not support > it[Jerin] > - Added rte_event_port_links_get()[Jerin] > - Added rte_event_dev_dump[Harry] > > Notes: > > - This patch set is check-patch clean with an exception that > 02/04 has one WARNING:MACRO_WITH_FLOW_CONTROL > - Looking forward to getting additional maintainers for libeventdev > > > Possible next steps: > 1) Review this patch set > 2) Integrate Intel's SW driver[http://dpdk.org/dev/patchwork/patch/17049/] > 3) Review proposed examples/eventdev_pipeline > application[http://dpdk.org/dev/patchwork/patch/17053/] > 4) Review proposed functional > tests[http://dpdk.org/dev/patchwork/patch/17051/] > 5) Cavium's HW based eventdev driver > > I am planning to work on (3),(4) and (5) > Thanks Jerin, we'll review and get back to you with any comments or feedback (1), and obviously start working on item (2) also! :-) I'm also wonder whether we should have a staging tree for this work to make interaction between us easier. Although this may not be finalised enough for 17.02 release, do you think having an dpdk-eventdev-next tree would be a help? My thinking is that once we get the eventdev library itself in reasonable shape following our review, we could commit that and make any changes thereafter as new patches, rather than constantly respinning the same set. It also gives us a clean git tree to base the respective driver implementations on from our two sides. Thomas, any thoughts here on your end - or from anyone else? Regards, /Bruce
[dpdk-dev] Fwd: |WARNING| [PATCH] nfp: report link speed using hardware info
On 11/18/2016 3:10 PM, Alejandro Lucero wrote: > Hi Thomas, > > I got this email when sending a patch some minutes ago. > > The point is I trusted script/checkpatches.sh which did not report those > warnings. > Am I doing anything wrong when using checkpatches.sh? I am also getting same warnings as below, this can be related to the checkpatch.pl version. I have: Version: 0.32 (./scripts/checkpatch.pl --version) > > > -- Forwarded message -- > From: > Date: Fri, Nov 18, 2016 at 3:04 PM > Subject: |WARNING| [PATCH] nfp: report link speed using hardware info > To: test-report at dpdk.org > Cc: Alejandro Lucero > > > Test-Label: checkpatch > Test-Status: WARNING > http://dpdk.org/patch/17091 > > _coding style issues_ > > > CHECK:MACRO_ARG_REUSE: Macro argument reuse 'arr' - possible side-effects? > #53: FILE: drivers/net/nfp/nfp_net.c:806: > +#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0])) > > CHECK:BRACES: Blank lines aren't necessary after an open brace '{' > #91: FILE: drivers/net/nfp/nfp_net.c:856: > + else { > + > > total: 0 errors, 0 warnings, 2 checks, 68 lines checked >
[dpdk-dev] [PATCH] lib/librte_mempool: a redundant word in comment
Hi Wei, 2lOn 11/15/2016 07:54 AM, Zhao1, Wei wrote: > Hi, john > >> -Original Message- >> From: Mcnamara, John >> Sent: Monday, November 14, 2016 6:30 PM >> To: Zhao1, Wei ; dev at dpdk.org >> Cc: olivier.matz at 6wind.com; Zhao1, Wei >> Subject: RE: [dpdk-dev] [PATCH] lib/librte_mempool: a redundant word in >> comment >> >> >> >>> -Original Message- >>> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Wei Zhao >>> Sent: Monday, November 14, 2016 2:47 AM >>> To: dev at dpdk.org >>> Cc: olivier.matz at 6wind.com; Zhao1, Wei >>> Subject: [dpdk-dev] [PATCH] lib/librte_mempool: a redundant word in >>> comment >>> >>> From: zhao wei >> >> I think you need to add your name to gitconfig file on the sending machine to >> avoid this "From:" >> >>> >>> There is a redundant repetition word "for" in commnet line the file >>> rte_mempool.h after the definition of RTE_MEMPOOL_OPS_NAMESIZE. >>> The word "for"appear twice in line 359 and 360.One of them is >>> redundant, so delete it. >>> >>> Fixes: 449c49b93a6b ("lib/librte_mempool: mempool: support handler >>> operations") The proper fixline should be: Fixes: 449c49b93a6b ("mempool: support handler operations") (no need to add "lib/librte_mempool:") This comment also applies to the other patch, I missed it. >>> >>> Signed-off-by: zhao wei >> >> /commnet/comment/ >> >> And same comment as before about the title. Apart from that: >> >> Acked-by: John McNamara >> >> > > Thank you for your suggestion, I will change as your comment in following > patch! > Also same comment about "mempool:" instead of "lib/librte_mempool: mempool:" Thanks, Olivier
[dpdk-dev] Fwd: |WARNING| [PATCH] nfp: report link speed using hardware info
Hi Thomas, I got this email when sending a patch some minutes ago. The point is I trusted script/checkpatches.sh which did not report those warnings. Am I doing anything wrong when using checkpatches.sh? -- Forwarded message -- From:Date: Fri, Nov 18, 2016 at 3:04 PM Subject: |WARNING| [PATCH] nfp: report link speed using hardware info To: test-report at dpdk.org Cc: Alejandro Lucero Test-Label: checkpatch Test-Status: WARNING http://dpdk.org/patch/17091 _coding style issues_ CHECK:MACRO_ARG_REUSE: Macro argument reuse 'arr' - possible side-effects? #53: FILE: drivers/net/nfp/nfp_net.c:806: +#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0])) CHECK:BRACES: Blank lines aren't necessary after an open brace '{' #91: FILE: drivers/net/nfp/nfp_net.c:856: + else { + total: 0 errors, 0 warnings, 2 checks, 68 lines checked
[dpdk-dev] [PATCH] lib/librte_mempool: a redundant of socket_id assignment
Hi Wei, On 11/14/2016 11:25 AM, Mcnamara, John wrote: > >> -Original Message- >> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Wei Zhao >> Sent: Monday, November 14, 2016 2:16 AM >> To: dev at dpdk.org >> Cc: olivier.matz at 6wind.com; Zhao1, Wei >> Subject: [dpdk-dev] [PATCH] lib/librte_mempool: a redundant of socket_id >> assignment >> >> From: zhao wei >> >> There is a redundant repetition mempool socket_id assignment in the file >> rte_mempool.c in function rte_mempool_create_empty.The statement "mp- >>> socket_id = socket_id;"appear twice in line 821 and 824.One of them is >> redundant, so delete it. >> >> Fixes: 85226f9c526b ("lib/librte_mempool: mempool:introduce a function to >> create an empty pool") >> >> Signed-off-by: zhao wei > > Titles should generally start with a verb to indicate what is being done. > Something like: > > lib/librte_mempool: remove redundant socket_id assignment > > Apart from that. > > Acked-by: John McNamara I would even say: mempool: remove redundant socket_id assignment Acked-by: Olivier Matz
[dpdk-dev] [PATCH] nfp: report link speed using hardware info
Previous reported speed was hardcoded. Signed-off-by: Alejandro Lucero --- drivers/net/nfp/nfp_net.c | 31 +-- drivers/net/nfp/nfp_net_ctrl.h | 13 + 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c index c6b1587..d5ec0ff 100644 --- a/drivers/net/nfp/nfp_net.c +++ b/drivers/net/nfp/nfp_net.c @@ -803,6 +803,7 @@ static void nfp_net_read_mac(struct nfp_net_hw *hw) hw->ctrl = new_ctrl; } +#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0])) /* * return 0 means link status changed, -1 means not changed * @@ -816,6 +817,18 @@ static void nfp_net_read_mac(struct nfp_net_hw *hw) struct rte_eth_link link, old; uint32_t nn_link_status; + static const uint32_t ls_to_ethtool[] = { + [NFP_NET_CFG_STS_LINK_RATE_UNSUPPORTED] = ETH_SPEED_NUM_NONE, + [NFP_NET_CFG_STS_LINK_RATE_UNKNOWN] = ETH_SPEED_NUM_NONE, + [NFP_NET_CFG_STS_LINK_RATE_1G] = ETH_SPEED_NUM_1G, + [NFP_NET_CFG_STS_LINK_RATE_10G] = ETH_SPEED_NUM_10G, + [NFP_NET_CFG_STS_LINK_RATE_25G] = ETH_SPEED_NUM_25G, + [NFP_NET_CFG_STS_LINK_RATE_40G] = ETH_SPEED_NUM_40G, + [NFP_NET_CFG_STS_LINK_RATE_50G] = ETH_SPEED_NUM_50G, + [NFP_NET_CFG_STS_LINK_RATE_100G]= ETH_SPEED_NUM_100G, + }; + + PMD_DRV_LOG(DEBUG, "Link update\n"); hw = NFP_NET_DEV_PRIVATE_TO_HW(dev->data->dev_private); @@ -831,8 +844,22 @@ static void nfp_net_read_mac(struct nfp_net_hw *hw) link.link_status = ETH_LINK_UP; link.link_duplex = ETH_LINK_FULL_DUPLEX; - /* Other cards can limit the tx and rx rate per VF */ - link.link_speed = ETH_SPEED_NUM_40G; + + nn_link_status = (nn_link_status >> NFP_NET_CFG_STS_LINK_RATE_SHIFT) & +NFP_NET_CFG_STS_LINK_RATE_MASK; + + if ((NFD_CFG_MAJOR_VERSION_of(hw->ver) < 4) || + ((NFD_CFG_MINOR_VERSION_of(hw->ver) == 4) && + (NFD_CFG_MINOR_VERSION_of(hw->ver) == 0))) + link.link_speed = ETH_SPEED_NUM_40G; + else { + + if (nn_link_status == NFP_NET_CFG_STS_LINK_RATE_UNKNOWN || + nn_link_status >= ARRAY_SIZE(ls_to_ethtool)) + link.link_speed = ETH_SPEED_NUM_NONE; + else + link.link_speed = ls_to_ethtool[nn_link_status]; + } if (old.link_status != link.link_status) { nfp_net_dev_atomic_write_link_status(dev, ); diff --git a/drivers/net/nfp/nfp_net_ctrl.h b/drivers/net/nfp/nfp_net_ctrl.h index fce8251..f9aaba3 100644 --- a/drivers/net/nfp/nfp_net_ctrl.h +++ b/drivers/net/nfp/nfp_net_ctrl.h @@ -157,6 +157,19 @@ #define NFP_NET_CFG_VERSION_MINOR(x)(((x) & 0xff) << 0) #define NFP_NET_CFG_STS 0x0034 #define NFP_NET_CFG_STS_LINK(0x1 << 0) /* Link up or down */ +/* Link rate */ +#define NFP_NET_CFG_STS_LINK_RATE_SHIFT 1 +#define NFP_NET_CFG_STS_LINK_RATE_MASK 0xF +#define NFP_NET_CFG_STS_LINK_RATE \ + (NFP_NET_CFG_STS_LINK_RATE_MASK << NFP_NET_CFG_STS_LINK_RATE_SHIFT) +#define NFP_NET_CFG_STS_LINK_RATE_UNSUPPORTED 0 +#define NFP_NET_CFG_STS_LINK_RATE_UNKNOWN 1 +#define NFP_NET_CFG_STS_LINK_RATE_1G2 +#define NFP_NET_CFG_STS_LINK_RATE_10G 3 +#define NFP_NET_CFG_STS_LINK_RATE_25G 4 +#define NFP_NET_CFG_STS_LINK_RATE_40G 5 +#define NFP_NET_CFG_STS_LINK_RATE_50G 6 +#define NFP_NET_CFG_STS_LINK_RATE_100G 7 #define NFP_NET_CFG_CAP 0x0038 #define NFP_NET_CFG_MAX_TXRINGS 0x003c #define NFP_NET_CFG_MAX_RXRINGS 0x0040 -- 1.9.1
[dpdk-dev] [PATCH v2] i40e: Fix eth_i40e_dev_init sequence on ThunderX
On Fri, Nov 18, 2016 at 04:52:13AM -0800, Satha Rao wrote: > i40e_asq_send_command: rd32 & wr32 under ThunderX gives unpredictable >results. To solve this include rte memory barriers > > Signed-off-by: Satha Rao > --- > 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; > +#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])) > -- Would rte_smp_*mb() functions allow you to get a similar result without the need for #ifdefs? It should be a full barrier on weakly ordered platforms which just a compiler barrier on IA. /Bruce
[dpdk-dev] Clarification for - SoC specific driver based common sub component placing
2016-11-18 14:38, Thomas Monjalon: > 2016-11-18 12:44, Hemant Agrawal: > > We like to introduce NXP's DPAA (Data Path Acceleration Architecture Gen2) > > Poll mode drivers into the DPDK. > > > > We need some clarification w.r.t the right placing of some dependent > > components, which can be common across drivers. E.g. We have hardware queue > > and buffer manager driver. This will be used by both network driver and > > crypto driver. But it is specific to NXP platform only. > > > > What is the right place for such common hardware specific components in > > DPDK? > > 1. Add a new generic Soc library structure. e.g. librte_soc/nxp/. For > > each soc configuration only the required components will be compiled-in. > > 2. Create a drivers/soc/nxp/dpaa2 structure to keep common driver libs. > > And link the network and crypto drivers to it. > > 3. Add it to main network driver and make the crypto driver dependent > > on it. > > Your question is more generic than SoC context. > You just want to share some code between drivers, right? > What about building a library located in drivers/common/nxp/ ? I'm a bit reluctant to have company name in file hierarchy, as it not something stable. And especially for NXP/Qualcomm... In this case would it be better to name the directory drivers/common/dpaa2/ ?
[dpdk-dev] Clarification for - SoC specific driver based common sub component placing
2016-11-18 12:44, Hemant Agrawal: > We like to introduce NXP's DPAA (Data Path Acceleration Architecture Gen2) > Poll mode drivers into the DPDK. > > We need some clarification w.r.t the right placing of some dependent > components, which can be common across drivers. E.g. We have hardware queue > and buffer manager driver. This will be used by both network driver and > crypto driver. But it is specific to NXP platform only. > > What is the right place for such common hardware specific components in DPDK? > 1. Add a new generic Soc library structure. e.g. librte_soc/nxp/. For > each soc configuration only the required components will be compiled-in. > 2. Create a drivers/soc/nxp/dpaa2 structure to keep common driver libs. > And link the network and crypto drivers to it. > 3. Add it to main network driver and make the crypto driver dependent > on it. Your question is more generic than SoC context. You just want to share some code between drivers, right? What about building a library located in drivers/common/nxp/ ?
[dpdk-dev] Clarification for - SoC specific driver based common sub component placing
> -Original Message- > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > Sent: Friday, November 18, 2016 7:13 PM > To: Hemant Agrawal > Cc: dev at dpdk.org > Subject: Re: Clarification for - SoC specific driver based common sub > component > placing > > 2016-11-18 14:38, Thomas Monjalon: > > 2016-11-18 12:44, Hemant Agrawal: > > > We like to introduce NXP's DPAA (Data Path Acceleration Architecture Gen2) > Poll mode drivers into the DPDK. > > > > > > We need some clarification w.r.t the right placing of some dependent > components, which can be common across drivers. E.g. We have hardware > queue and buffer manager driver. This will be used by both network driver and > crypto driver. But it is specific to NXP platform only. > > > > > > What is the right place for such common hardware specific components in > DPDK? > > > 1. Add a new generic Soc library structure. e.g. librte_soc/nxp/. For > > > each > soc configuration only the required components will be compiled-in. > > > 2. Create a drivers/soc/nxp/dpaa2 structure to keep common driver libs. > And link the network and crypto drivers to it. > > > 3. Add it to main network driver and make the crypto driver dependent > on it. > > > > Your question is more generic than SoC context. > > You just want to share some code between drivers, right? > > What about building a library located in drivers/common/nxp/ ? > > I'm a bit reluctant to have company name in file hierarchy, as it not > something > stable. And especially for NXP/Qualcomm... > > In this case would it be better to name the directory drivers/common/dpaa2/ ? [Hemant] Sounds good. Thanks!
[dpdk-dev] [PATCH v5] latencystats: added new library for latency stats
On 15/11/2016 21:37, Reshma Pattan wrote: [..] > This pacth is dependent on http://dpdk.org/dev/patchwork/patch/16927/ There are merge issues, so will combine this patch into the v6 of the above patchset. ..Remy
[dpdk-dev] [PATCH] pmdinfogen: Fix pmdinfogen to select proper endianess on cross-compile
pmdinfogen has a bug in which, during build, it pulls in rte_byteorder.h to obtain the rte macros for byteswapping between the cpu byte order and big or little endian. Unfortunately, pmdinfogen is a tool that is only meant to be run during the build of dpdk components, and so, it runs on the host. In cross compile environments however, the rte_byteorder.h is configured using a target cpu, who's endianess may differ from that of the host, leading to improper swapping. The fix is to use host system defined byte swapping routines rather than the dpdk provided routines. Note that we are using non posix compliant routines, as the posix compliant api only addresses 16 and 32 bit swaps, and we also need 64 bit swaps. Those macros exist (via endian.h), but BSD and Linux put that header in different locations so some ifdeffery is required. Tested successfully by myself on Linux and BSD systems. Signed-off-by: Neil Horman CC: Hemant Agrawal CC: Jerin Jacob CC: Bruce Richardson CC: Thomas Monjalon --- buildtools/pmdinfogen/pmdinfogen.h | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/buildtools/pmdinfogen/pmdinfogen.h b/buildtools/pmdinfogen/pmdinfogen.h index 1da2966..7c787c1 100644 --- a/buildtools/pmdinfogen/pmdinfogen.h +++ b/buildtools/pmdinfogen/pmdinfogen.h @@ -16,12 +16,16 @@ #include #include #include +#ifdef __linux__ +#include +#else +#include +#endif #include #include #include #include #include -#include /* On BSD-alike OSes elf.h defines these according to host's word size */ #undef ELF_ST_BIND @@ -75,9 +79,9 @@ #define CONVERT_NATIVE(fend, width, x) ({ \ typeof(x) ___x; \ if ((fend) == ELFDATA2LSB) \ - ___x = rte_le_to_cpu_##width(x); \ + ___x = le##width##toh(x); \ else \ - ___x = rte_be_to_cpu_##width(x); \ + ___x = be##width##toh(x); \ ___x; \ }) -- 2.7.4
[dpdk-dev] pmdinfogen issues: cross compilation for ARM fails with older host compiler
On Fri, Nov 18, 2016 at 04:37:38PM +, Bruce Richardson wrote: > On Fri, Nov 18, 2016 at 08:50:52AM -0500, Neil Horman wrote: > > On Fri, Nov 18, 2016 at 12:03:19PM +, Hemant Agrawal wrote: > > > > -Original Message- > > > > From: Neil Horman [mailto:nhorman at tuxdriver.com] > > > > On Tue, Nov 15, 2016 at 09:34:16AM +, Hemant Agrawal wrote: > > > > > > On Mon, Nov 14, 2016 at 02:29:24AM +0530, Jerin Jacob wrote: > > > > > > > On Fri, Nov 11, 2016 at 10:34:39AM +, Hemant Agrawal wrote: > > > > > > > > Hi Neil, > > > > > > > >Pmdinfogen compiles with host compiler. It usages > > > > rte_byteorder.h > > > > > > of the target platform. > > > > > > > > However, if the host compiler is older than 4.8, it will be an > > > > > > > > issue during > > > > cross > > > > > > compilation for some platforms. > > > > > > > > e.g. if we are compiling on x86 host for ARM, x86 host compiler > > > > > > > > will not > > > > > > understand the arm asm instructions. > > > > > > > > > > > > > > > > /* fix missing __builtin_bswap16 for gcc older then 4.8 */ #if > > > > > > > > !(__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 8)) static > > > > > > > > inline uint16_t rte_arch_bswap16(uint16_t _x) { > > > > > > > >register uint16_t x = _x; > > > > > > > >asm volatile ("rev16 %0,%1" > > > > > > > > : "=r" (x) > > > > > > > > : "r" (x) > > > > > > > > ); > > > > > > > >return x; > > > > > > > > } > > > > > > > > #endif > > > > > > > > > > > > > > > > One easy solution is that we add compiler platform check in this > > > > > > > > code section of rte_byteorder.h e.g #if !(defined __arm__ || > > > > > > > > defined > > > > > > > > __aarch64__) static inline uint16_t rte_arch_bswap16(uint16_t > > > > > > > > _x) { > > > > > > > >return (_x >> 8) | ((_x << 8) & 0xff00); } #else > > > > > > > > ?. > > > > > > > > > > > > > > > > Is there a better way to fix it? > > > > > > > > > > > > > > IMO, It is a HOST build infrastructure issue. If a host app is > > > > > > > using > > > > > > > the dpdk service then it should compile and link against HOST > > > > > > > target(in this specific case, build/x86_64-native-linuxapp-gcc). I > > > > > > > think, introducing the HOSTTARGET kind of scheme is a clean > > > > > > > solution. > > > > > > > > > > > > > > /Jerin > > > > > > > > > > > > > > > > > > > > That would be accurate. That is to say, pmdinfogen is a tool that > > > > > > should only > > > > be > > > > > > run on the host doing the build, by the host doing the build, and > > > > > > so should be > > > > > > compiled to run on the host, not on the target being built for. > > > > > > > > > > > > Yeah, so what we need is a way to get to the host version of > > > > > > rte_byteorder.h > > > > > > when building in a cross environment > > > > > > > > > > > +1 > > > > > > > > > > > Neil > > > > > > > > > > > > > Give this a try, I've tested it on linux, but not BSD. From what I > > > > read the > > > > functions are not posix compliant, though they should exist on all BSD > > > > and Linux > > > > systems in recent history. There may be some fiddling needed for Net > > > > and > > > > OpenBSD variants, but I think this is the right general direction. > > > > > > + 1 > > > This patch works good for Linux. > > > > > Can someone test it on BSD? I'd like to ensure we don't need to modify it > > for > > that platform > > > > Neil > > > > > > > > > > > > > > diff --git a/buildtools/pmdinfogen/pmdinfogen.h > > > > b/buildtools/pmdinfogen/pmdinfogen.h > > > > index 1da2966..c5ef89d 100644 > > > > --- a/buildtools/pmdinfogen/pmdinfogen.h > > > > +++ b/buildtools/pmdinfogen/pmdinfogen.h > > > > @@ -21,7 +21,6 @@ > > > > #include > > > > #include > > > > #include > > > > -#include > > > > > > > > /* On BSD-alike OSes elf.h defines these according to host's word size > > > > */ > > > > #undef ELF_ST_BIND > > > > @@ -75,9 +74,9 @@ > > > > #define CONVERT_NATIVE(fend, width, x) ({ \ > > > > typeof(x) ___x; \ > > > > if ((fend) == ELFDATA2LSB) \ > > > > - ___x = rte_le_to_cpu_##width(x); \ > > > > + ___x = le##width##toh(x); \ > > > > else \ > > > > - ___x = rte_be_to_cpu_##width(x); \ > > > > + ___x = be##width##toh(x); \ > > > > ___x; \ > > > > }) > > > > > > For compile on FreeBSD 10 we need "#include " and this > works. > Yeah, but that will break linux, because there endian.h is in the base include directory. I'll have to do some ifdeffing Neil > /Bruce >
[dpdk-dev] Proposal for a new Committer model
On Thu, Nov 17, 2016 at 09:20:50AM +, Mcnamara, John wrote: > Repost from the moving at dpdk.org mailing list to get a wider audience. > Original thread: http://dpdk.org/ml/archives/moving/2016-November/59.html > > > Hi, > > I'd like to propose a change to the DPDK committer model. Currently we have > one committer for the master branch of the DPDK project. > > One committer to master represents a single point of failure and at times can > be inefficient. There is also no agreed cover for times when the committer is > unavailable such as vacation, public holidays, etc. I propose that we change > to a multi-committer model for the DPDK project. We should have three > committers for each release that can commit changes to the master branch. > > There are a number of benefits: > > 1. Greater capacity to commit patches. > 2. No single points of failure - a committer should always be available if we > have three. > 3. A more timely committing of patches. More committers should equal a faster > turnaround - ideally, maintainers should also provide feedback on patches > submitted within a 2-3 day period, as much as possible, to facilitate this. > 4. It follows best practice in creating a successful multi-vendor community - > to achieve this we must ensure there is a level playing field for all > participants, no single person should be required to make all of the > decisions on patches to be included in the release. > > Having multiple committers will require some degree of co-ordination but > there are a number of other communities successfully following this model > such as Apache, OVS, FD.io, OpenStack etc. so the approach is workable. > > John I agree that the problems you are attempting to address exist and are worth finding a solution for. That said, I don't think the solution you are proposing is the ideal, or complete fix for any of the issues being addressed. If I may, I'd like to ennumerate the issues I think you are trying to address based on your comments above, then make a counter-proposal for a solution: Problems to address: 1) high-availability - There is a desire to make sure that, when patches are proposed, they are integrated in a timely fashion. 2) high-throughput - DPDK has a large volume of patches, more than one person can normally integrate. There is a desire to shard that work such that it is handled by multiple individuals 3) Multi-Vendor fairness - There is a desire for multiple vendors to feel as though the project tree maintainer isn't biased toward any individual vendor. To solve these I would propose the following solution (which is simmilar to, but not quite identical, to yours). A) Further promote subtree maintainership. This was a conversation that I proposed some time ago, but my proposed granularity was discarded in favor of something that hasn't worked as well (in my opinion). That is to say a few driver pmds (i40e and fm10k come to mind) have their own tree that send pull requests to Thomas. We should be sharding that at a much higher granularity and using it much more consistently. That is to say, that we should have a maintainer for all the ethernet pmds, and another for the crypto pmds, another for the core eal layer, another for misc libraries that have low patch volumes, etc. Each of those subdivisions should have their own list to communicate on, and each should have a tree that integrates patches for their own subsystem, and they should on a regular cycle send pull requests to Thomas. Thomas in turn should by and large, only be integrating pull requests. This should address our high- throughput issue, in that it will allow multiple maintainers to share the workload, and integration should be relatively easy. 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
[dpdk-dev] Clarification for - SoC specific driver based common sub component placing
Hi all, We like to introduce NXP's DPAA (Data Path Acceleration Architecture Gen2) Poll mode drivers into the DPDK. We need some clarification w.r.t the right placing of some dependent components, which can be common across drivers. E.g. We have hardware queue and buffer manager driver. This will be used by both network driver and crypto driver. But it is specific to NXP platform only. What is the right place for such common hardware specific components in DPDK? 1. Add a new generic Soc library structure. e.g. librte_soc/nxp/. For each soc configuration only the required components will be compiled-in. 2. Create a drivers/soc/nxp/dpaa2 structure to keep common driver libs. And link the network and crypto drivers to it. 3. Add it to main network driver and make the crypto driver dependent on it. Regards, Hemant
[dpdk-dev] pmdinfogen issues: cross compilation for ARM fails with older host compiler
> -Original Message- > From: Neil Horman [mailto:nhorman at tuxdriver.com] > On Tue, Nov 15, 2016 at 09:34:16AM +, Hemant Agrawal wrote: > > > On Mon, Nov 14, 2016 at 02:29:24AM +0530, Jerin Jacob wrote: > > > > On Fri, Nov 11, 2016 at 10:34:39AM +, Hemant Agrawal wrote: > > > > > Hi Neil, > > > > >Pmdinfogen compiles with host compiler. It usages > rte_byteorder.h > > > of the target platform. > > > > > However, if the host compiler is older than 4.8, it will be an issue > > > > > during > cross > > > compilation for some platforms. > > > > > e.g. if we are compiling on x86 host for ARM, x86 host compiler will > > > > > not > > > understand the arm asm instructions. > > > > > > > > > > /* fix missing __builtin_bswap16 for gcc older then 4.8 */ #if > > > > > !(__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 8)) static > > > > > inline uint16_t rte_arch_bswap16(uint16_t _x) { > > > > >register uint16_t x = _x; > > > > >asm volatile ("rev16 %0,%1" > > > > > : "=r" (x) > > > > > : "r" (x) > > > > > ); > > > > >return x; > > > > > } > > > > > #endif > > > > > > > > > > One easy solution is that we add compiler platform check in this > > > > > code section of rte_byteorder.h e.g #if !(defined __arm__ || defined > > > > > __aarch64__) static inline uint16_t rte_arch_bswap16(uint16_t _x) { > > > > >return (_x >> 8) | ((_x << 8) & 0xff00); } #else ?. > > > > > > > > > > Is there a better way to fix it? > > > > > > > > IMO, It is a HOST build infrastructure issue. If a host app is using > > > > the dpdk service then it should compile and link against HOST > > > > target(in this specific case, build/x86_64-native-linuxapp-gcc). I > > > > think, introducing the HOSTTARGET kind of scheme is a clean solution. > > > > > > > > /Jerin > > > > > > > > > > > That would be accurate. That is to say, pmdinfogen is a tool that should > > > only > be > > > run on the host doing the build, by the host doing the build, and so > > > should be > > > compiled to run on the host, not on the target being built for. > > > > > > Yeah, so what we need is a way to get to the host version of > > > rte_byteorder.h > > > when building in a cross environment > > > > > +1 > > > > > Neil > > > > Give this a try, I've tested it on linux, but not BSD. From what I read the > functions are not posix compliant, though they should exist on all BSD and > Linux > systems in recent history. There may be some fiddling needed for Net and > OpenBSD variants, but I think this is the right general direction. + 1 This patch works good for Linux. > > > diff --git a/buildtools/pmdinfogen/pmdinfogen.h > b/buildtools/pmdinfogen/pmdinfogen.h > index 1da2966..c5ef89d 100644 > --- a/buildtools/pmdinfogen/pmdinfogen.h > +++ b/buildtools/pmdinfogen/pmdinfogen.h > @@ -21,7 +21,6 @@ > #include > #include > #include > -#include > > /* On BSD-alike OSes elf.h defines these according to host's word size */ > #undef ELF_ST_BIND > @@ -75,9 +74,9 @@ > #define CONVERT_NATIVE(fend, width, x) ({ \ > typeof(x) ___x; \ > if ((fend) == ELFDATA2LSB) \ > - ___x = rte_le_to_cpu_##width(x); \ > + ___x = le##width##toh(x); \ > else \ > - ___x = rte_be_to_cpu_##width(x); \ > + ___x = be##width##toh(x); \ > ___x; \ > }) >
[dpdk-dev] [PATCH 01/22] ethdev: introduce generic flow API
Hi Beilei, On Fri, Nov 18, 2016 at 06:36:31AM +, Xing, Beilei wrote: > Hi Adrien, > > > -Original Message- > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Adrien Mazarguil > > Sent: Thursday, November 17, 2016 12:23 AM > > To: dev at dpdk.org > > Cc: Thomas Monjalon ; De Lara Guarch, > > Pablo ; Olivier Matz > > > > Subject: [dpdk-dev] [PATCH 01/22] ethdev: introduce generic flow API > > > > This new API supersedes all the legacy filter types described in > > rte_eth_ctrl.h. > > It is slightly higher level and as a result relies more on PMDs to process > > and > > validate flow rules. > > > > Benefits: > > > > - A unified API is easier to program for, applications do not have to be > > written for a specific filter type which may or may not be supported by > > the underlying device. > > > > - The behavior of a flow rule is the same regardless of the underlying > > device, applications do not need to be aware of hardware quirks. > > > > - Extensible by design, API/ABI breakage should rarely occur if at all. > > > > - Documentation is self-standing, no need to look up elsewhere. > > > > Existing filter types will be deprecated and removed in the near future. > > > > Signed-off-by: Adrien Mazarguil > > > > + > > +/** > > + * Opaque type returned after successfully creating a flow. > > + * > > + * This handle can be used to manage and query the related flow (e.g. > > +to > > + * destroy it or retrieve counters). > > + */ > > +struct rte_flow; > > + > > As we talked before, we use attr/pattern/actions to create and destroy a flow > in PMD, > but I don't think it's easy to clone the user-provided parameters and return > the result > to the application as a rte_flow pointer. As you suggested: > /* PMD-specific code. */ > struct rte_flow { > struct rte_flow_attr attr; > struct rte_flow_item *pattern; > struct rte_flow_action *actions; > }; Just to provide some context to the community since the above snippet comes from private exchanges, I've suggested the above structure as a mean to create and remove rules in the same fashion as FDIR, by providing the rule used for creation to the destroy callback. As an opaque type, each PMD currently needs to implement its own version of struct rte_flow. The above definition may ease transition from FDIR to rte_flow for some PMDs, however they need to clone the entire application-provided rule to do so because there is no requirement for it to be kept allocated. I've implemented such a function in testpmd (port_flow_new() in commit [1]) as an example. [1] http://dpdk.org/ml/archives/dev/2016-November/050266.html However my suggestion is for PMDs to use their own HW-specific structure that only contains relevant information instead of being forced to drag large, non-native data around, missing useful context and that requires parsing every time. This is one benefit of using an opaque type in the first place, the other being ABI breakage avoidance. > Because both pattern and actions are pointers, and there're also pointers in > structure > rte_flow_item and struct rte_flow_action. We need to iterate allocation > during clone > and iterate free during destroy, then seems that the code is something ugly, > right? Well since I wrote that code, I won't easily admit it's ugly. I think PMDs should not require the duplication of generic rules actually, which are only defined as a common language between applications and PMDs. Both are free to store rules in their own preferred and efficient format internally. > I think application saves info when creating a flow rule, so why not > application provide > attr/pattern/actions info to PMD before calling PMD API? They have to do so temporarily (e.g. allocated on the stack) while calling rte_flow_create() and rte_flow_validate(), that's it. Once a rule is created, there's no requirement for applications to keep anything around. For simple applications such as testpmd, the generic format is probably enough. More complex and existing applications such as ovs-dpdk may rather choose to keep using their internal format that already fits their needs, partially duplicating this information in rte_flow_attr and rte_flow_item/rte_flow_action lists would waste memory. The conversion in this case should only be performed when creating/validating flow rules. In short, I fail to see any downside with maintaining struct rte_flow opaque to applications. Best regards, -- Adrien Mazarguil 6WIND
[dpdk-dev] [PATCH 4/4] app/test: unit test case for eventdev APIs
This commit adds basic unit tests for the eventdev API. commands to run the test app: ./build/app/test -c 2 RTE>>eventdev_common_autotest Signed-off-by: Jerin Jacob --- MAINTAINERS | 1 + app/test/Makefile| 2 + app/test/test_eventdev.c | 776 +++ 3 files changed, 779 insertions(+) create mode 100644 app/test/test_eventdev.c diff --git a/MAINTAINERS b/MAINTAINERS index c594a23..887f133 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -252,6 +252,7 @@ F: examples/l2fwd-crypto/ Eventdev API - EXPERIMENTAL M: Jerin Jacob F: lib/librte_eventdev/ +F: app/test/test_eventdev* F: drivers/event/skeleton/ Networking Drivers diff --git a/app/test/Makefile b/app/test/Makefile index 5be023a..e28c079 100644 --- a/app/test/Makefile +++ b/app/test/Makefile @@ -197,6 +197,8 @@ SRCS-$(CONFIG_RTE_LIBRTE_CRYPTODEV) += test_cryptodev_blockcipher.c SRCS-$(CONFIG_RTE_LIBRTE_CRYPTODEV) += test_cryptodev_perf.c SRCS-$(CONFIG_RTE_LIBRTE_CRYPTODEV) += test_cryptodev.c +SRCS-$(CONFIG_RTE_LIBRTE_EVENTDEV) += test_eventdev.c + SRCS-$(CONFIG_RTE_LIBRTE_KVARGS) += test_kvargs.c CFLAGS += -O3 diff --git a/app/test/test_eventdev.c b/app/test/test_eventdev.c new file mode 100644 index 000..e876804 --- /dev/null +++ b/app/test/test_eventdev.c @@ -0,0 +1,776 @@ +/*- + * BSD LICENSE + * + * Copyright(c) 2016 Cavium networks. All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * + * * Redistributions of source code must retain the above copyright + *notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + *notice, this list of conditions and the following disclaimer in + *the documentation and/or other materials provided with the + *distribution. + * * Neither the name of Cavium networks nor the names of its + *contributors may be used to endorse or promote products derived + *from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +#include +#include +#include +#include +#include +#include +#include + +#include "test.h" + +#define TEST_DEV_NAME EVENTDEV_NAME_SKELETON_PMD + +static inline uint8_t +test_dev_id_get(void) +{ + return rte_event_dev_get_dev_id(RTE_STR(TEST_DEV_NAME)"_0"); +} + +static int +testsuite_setup(void) +{ + return rte_eal_vdev_init(RTE_STR(TEST_DEV_NAME), NULL); +} + +static void +testsuite_teardown(void) +{ +} + +static int +test_eventdev_count(void) +{ + uint8_t count; + count = rte_event_dev_count(); + TEST_ASSERT(count > 0, "Invalid eventdev count %" PRIu8, count); + return TEST_SUCCESS; +} + +static int +test_eventdev_get_dev_id(void) +{ + int ret; + ret = rte_event_dev_get_dev_id(RTE_STR(TEST_DEV_NAME)"_0"); + TEST_ASSERT(ret >= 0, "Failed to get dev_id %d", ret); + ret = rte_event_dev_get_dev_id("not_a_valid_ethdev_driver"); + TEST_ASSERT_FAIL(ret, "Expected <0 for invalid dev name ret=%d", ret); + return TEST_SUCCESS; +} + +static int +test_eventdev_socket_id(void) +{ + int ret, socket_id; + ret = rte_event_dev_get_dev_id(RTE_STR(TEST_DEV_NAME)"_0"); + socket_id = rte_event_dev_socket_id(ret); + TEST_ASSERT(socket_id != -EINVAL, "Failed to get socket_id %d", + socket_id); + socket_id = rte_event_dev_socket_id(RTE_EVENT_MAX_DEVS); + TEST_ASSERT(socket_id == -EINVAL, "Expected -EINVAL %d", socket_id); + + return TEST_SUCCESS; +} + +static int +test_eventdev_info_get(void) +{ + int ret; + struct rte_event_dev_info info; + ret = rte_event_dev_info_get(test_dev_id_get(), NULL); + TEST_ASSERT(ret == -EINVAL, "Expected -EINVAL, %d", ret); + ret = rte_event_dev_info_get(test_dev_id_get(), ); + TEST_ASSERT_SUCCESS(ret, "Failed to get event dev info"); + TEST_ASSERT(info.max_event_ports > 0, + "Not enough event ports %d",
[dpdk-dev] [PATCH 2/4] eventdev: implement the northbound APIs
This patch set defines the southbound driver interface and implements the common code required for northbound eventdev API interface. Signed-off-by: Jerin Jacob --- config/common_base |6 + lib/Makefile |1 + lib/librte_eal/common/include/rte_log.h |1 + lib/librte_eventdev/Makefile | 57 ++ lib/librte_eventdev/rte_eventdev.c | 1211 ++ lib/librte_eventdev/rte_eventdev_pmd.h | 504 +++ lib/librte_eventdev/rte_eventdev_version.map | 39 + mk/rte.app.mk|1 + 8 files changed, 1820 insertions(+) create mode 100644 lib/librte_eventdev/Makefile create mode 100644 lib/librte_eventdev/rte_eventdev.c create mode 100644 lib/librte_eventdev/rte_eventdev_pmd.h create mode 100644 lib/librte_eventdev/rte_eventdev_version.map diff --git a/config/common_base b/config/common_base index 4bff83a..7a8814e 100644 --- a/config/common_base +++ b/config/common_base @@ -411,6 +411,12 @@ CONFIG_RTE_LIBRTE_PMD_ZUC_DEBUG=n CONFIG_RTE_LIBRTE_PMD_NULL_CRYPTO=y # +# Compile generic event device library +# +CONFIG_RTE_LIBRTE_EVENTDEV=y +CONFIG_RTE_LIBRTE_EVENTDEV_DEBUG=n +CONFIG_RTE_EVENT_MAX_DEVS=16 +CONFIG_RTE_EVENT_MAX_QUEUES_PER_DEV=64 # Compile librte_ring # CONFIG_RTE_LIBRTE_RING=y diff --git a/lib/Makefile b/lib/Makefile index 990f23a..1a067bf 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -41,6 +41,7 @@ DIRS-$(CONFIG_RTE_LIBRTE_CFGFILE) += librte_cfgfile DIRS-$(CONFIG_RTE_LIBRTE_CMDLINE) += librte_cmdline DIRS-$(CONFIG_RTE_LIBRTE_ETHER) += librte_ether DIRS-$(CONFIG_RTE_LIBRTE_CRYPTODEV) += librte_cryptodev +DIRS-$(CONFIG_RTE_LIBRTE_EVENTDEV) += librte_eventdev DIRS-$(CONFIG_RTE_LIBRTE_VHOST) += librte_vhost DIRS-$(CONFIG_RTE_LIBRTE_HASH) += librte_hash DIRS-$(CONFIG_RTE_LIBRTE_LPM) += librte_lpm diff --git a/lib/librte_eal/common/include/rte_log.h b/lib/librte_eal/common/include/rte_log.h index 29f7d19..9a07d92 100644 --- a/lib/librte_eal/common/include/rte_log.h +++ b/lib/librte_eal/common/include/rte_log.h @@ -79,6 +79,7 @@ extern struct rte_logs rte_logs; #define RTE_LOGTYPE_PIPELINE 0x8000 /**< Log related to pipeline. */ #define RTE_LOGTYPE_MBUF0x0001 /**< Log related to mbuf. */ #define RTE_LOGTYPE_CRYPTODEV 0x0002 /**< Log related to cryptodev. */ +#define RTE_LOGTYPE_EVENTDEV 0x0004 /**< Log related to eventdev. */ /* these log types can be used in an application */ #define RTE_LOGTYPE_USER1 0x0100 /**< User-defined log type 1. */ diff --git a/lib/librte_eventdev/Makefile b/lib/librte_eventdev/Makefile new file mode 100644 index 000..dac0663 --- /dev/null +++ b/lib/librte_eventdev/Makefile @@ -0,0 +1,57 @@ +# BSD LICENSE +# +# Copyright(c) 2016 Cavium networks. All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions +# are met: +# +# * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# * Redistributions in binary form must reproduce the above copyright +# notice, this list of conditions and the following disclaimer in +# the documentation and/or other materials provided with the +# distribution. +# * Neither the name of Cavium networks nor the names of its +# contributors may be used to endorse or promote products derived +# from this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +include $(RTE_SDK)/mk/rte.vars.mk + +# library name +LIB = librte_eventdev.a + +# library version +LIBABIVER := 1 + +# build flags +CFLAGS += -O3 +CFLAGS += $(WERROR_FLAGS) + +# library source files +SRCS-y += rte_eventdev.c + +# export include files +SYMLINK-y-include += rte_eventdev.h +SYMLINK-y-include += rte_eventdev_pmd.h + +# versioning export map +EXPORT_MAP := rte_eventdev_version.map + +# library dependencies +DEPDIRS-y += lib/librte_eal +DEPDIRS-y += lib/librte_mbuf + +include $(RTE_SDK)/mk/rte.lib.mk diff --git a/lib/librte_eventdev/rte_eventdev.c
[dpdk-dev] [PATCH 1/4] eventdev: introduce event driven programming model
In a polling model, lcores poll ethdev ports and associated rx queues directly to look for packet. In an event driven model, by contrast, lcores call the scheduler that selects packets for them based on programmer-specified criteria. Eventdev library adds support for event driven programming model, which offer applications automatic multicore scaling, dynamic load balancing, pipelining, packet ingress order maintenance and synchronization services to simplify application packet processing. By introducing event driven programming model, DPDK can support both polling and event driven programming models for packet processing, and applications are free to choose whatever model (or combination of the two) that best suits their needs. Signed-off-by: Jerin Jacob --- MAINTAINERS|3 + doc/api/doxy-api-index.md |1 + doc/api/doxy-api.conf |1 + lib/librte_eventdev/rte_eventdev.h | 1439 4 files changed, 1444 insertions(+) create mode 100644 lib/librte_eventdev/rte_eventdev.h diff --git a/MAINTAINERS b/MAINTAINERS index d6bb8f8..e430ca7 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -249,6 +249,9 @@ F: lib/librte_cryptodev/ F: app/test/test_cryptodev* F: examples/l2fwd-crypto/ +Eventdev API - EXPERIMENTAL +M: Jerin Jacob +F: lib/librte_eventdev/ Networking Drivers -- diff --git a/doc/api/doxy-api-index.md b/doc/api/doxy-api-index.md index 6675f96..28c1329 100644 --- a/doc/api/doxy-api-index.md +++ b/doc/api/doxy-api-index.md @@ -40,6 +40,7 @@ There are many libraries, so their headers may be grouped by topics: [ethdev] (@ref rte_ethdev.h), [ethctrl](@ref rte_eth_ctrl.h), [cryptodev] (@ref rte_cryptodev.h), + [eventdev] (@ref rte_eventdev.h), [devargs](@ref rte_devargs.h), [bond] (@ref rte_eth_bond.h), [vhost] (@ref rte_virtio_net.h), diff --git a/doc/api/doxy-api.conf b/doc/api/doxy-api.conf index 9dc7ae5..9841477 100644 --- a/doc/api/doxy-api.conf +++ b/doc/api/doxy-api.conf @@ -41,6 +41,7 @@ INPUT = doc/api/doxy-api-index.md \ lib/librte_cryptodev \ lib/librte_distributor \ lib/librte_ether \ + lib/librte_eventdev \ lib/librte_hash \ lib/librte_ip_frag \ lib/librte_jobstats \ diff --git a/lib/librte_eventdev/rte_eventdev.h b/lib/librte_eventdev/rte_eventdev.h new file mode 100644 index 000..778d6dc --- /dev/null +++ b/lib/librte_eventdev/rte_eventdev.h @@ -0,0 +1,1439 @@ +/* + * BSD LICENSE + * + * Copyright 2016 Cavium. + * Copyright 2016 Intel Corporation. + * Copyright 2016 NXP. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in + * the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Cavium nor the names of its + * contributors may be used to endorse or promote products derived + * from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +#ifndef _RTE_EVENTDEV_H_ +#define _RTE_EVENTDEV_H_ + +/** + * @file + * + * RTE Event Device API + * + * In a polling model, lcores poll ethdev ports and associated rx queues + * directly to look for packet. In an event driven model, by contrast, lcores + * call the scheduler that selects packets for them based on programmer + * specified criteria. Eventdev library adds support for event driven + * programming model, which offer applications automatic multicore scaling, + * dynamic load balancing, pipelining, packet ingress order
[dpdk-dev] [PATCH 0/4] libeventdev API and northbound implementation
As previously discussed in RFC v1 [1], RFC v2 [2], with changes described in [3] (also pasted below), here is the first non-draft series for this new API. [1] http://dpdk.org/ml/archives/dev/2016-August/045181.html [2] http://dpdk.org/ml/archives/dev/2016-October/048592.html [3] http://dpdk.org/ml/archives/dev/2016-October/048196.html Changes since RFC v2: - Updated the documentation to define the need for this library[Jerin] - Added RTE_EVENT_QUEUE_CFG_*_ONLY configuration parameters in struct rte_event_queue_conf to enable optimized sw implementation [Bruce] - Introduced RTE_EVENT_OP* ops [Bruce] - Added nb_event_queue_flows,nb_event_port_dequeue_depth, nb_event_port_enqueue_depth in rte_event_dev_configure() like ethdev and crypto library[Jerin] - Removed rte_event_release() and replaced with RTE_EVENT_OP_RELEASE ops to reduce fast path APIs and it is redundant too[Jerin] - In the view of better application portability, Removed pin_event from rte_event_enqueue as it is just hint and Intel/NXP can not support it[Jerin] - Added rte_event_port_links_get()[Jerin] - Added rte_event_dev_dump[Harry] Notes: - This patch set is check-patch clean with an exception that 02/04 has one WARNING:MACRO_WITH_FLOW_CONTROL - Looking forward to getting additional maintainers for libeventdev Possible next steps: 1) Review this patch set 2) Integrate Intel's SW driver[http://dpdk.org/dev/patchwork/patch/17049/] 3) Review proposed examples/eventdev_pipeline application[http://dpdk.org/dev/patchwork/patch/17053/] 4) Review proposed functional tests[http://dpdk.org/dev/patchwork/patch/17051/] 5) Cavium's HW based eventdev driver I am planning to work on (3),(4) and (5) TODO: 1) Example applications for pipelining, packet ingress order maintenance with ORDERED type and ATOMIC synchronization services. 2) Create user guide Jerin Jacob (4): eventdev: introduce event driven programming model eventdev: implement the northbound APIs event/skeleton: add skeleton eventdev driver app/test: unit test case for eventdev APIs MAINTAINERS|5 + app/test/Makefile |2 + app/test/test_eventdev.c | 776 +++ config/common_base | 14 + doc/api/doxy-api-index.md |1 + doc/api/doxy-api.conf |1 + drivers/Makefile |1 + drivers/event/Makefile | 36 + drivers/event/skeleton/Makefile| 55 + .../skeleton/rte_pmd_skeleton_event_version.map|4 + drivers/event/skeleton/skeleton_eventdev.c | 535 drivers/event/skeleton/skeleton_eventdev.h | 72 + lib/Makefile |1 + lib/librte_eal/common/include/rte_log.h|1 + lib/librte_eventdev/Makefile | 57 + lib/librte_eventdev/rte_eventdev.c | 1211 lib/librte_eventdev/rte_eventdev.h | 1439 lib/librte_eventdev/rte_eventdev_pmd.h | 504 +++ lib/librte_eventdev/rte_eventdev_version.map | 39 + mk/rte.app.mk |5 + 20 files changed, 4759 insertions(+) create mode 100644 app/test/test_eventdev.c create mode 100644 drivers/event/Makefile create mode 100644 drivers/event/skeleton/Makefile create mode 100644 drivers/event/skeleton/rte_pmd_skeleton_event_version.map create mode 100644 drivers/event/skeleton/skeleton_eventdev.c create mode 100644 drivers/event/skeleton/skeleton_eventdev.h create mode 100644 lib/librte_eventdev/Makefile create mode 100644 lib/librte_eventdev/rte_eventdev.c create mode 100644 lib/librte_eventdev/rte_eventdev.h create mode 100644 lib/librte_eventdev/rte_eventdev_pmd.h create mode 100644 lib/librte_eventdev/rte_eventdev_version.map -- 2.5.5
[dpdk-dev] [PATCH 3/3] net/mlx5: do not invalidate title CQE
On 11/17/2016 10:38 AM, Adrien Mazarguil wrote: > On Thu, Nov 17, 2016 at 10:49:56AM +0100, Nelio Laranjeiro wrote: >> We can leave the title completion queue entry untouched since its contents >> are not modified. >> >> Reported-by: Liming Sun >> Signed-off-by: Nelio Laranjeiro <...> > Acked-by: Adrien Mazarguil Applied to dpdk-next-net/master, thanks.
[dpdk-dev] [dpdk-stable] [PATCH 2/3] net/mlx5: fix wrong htons
On 11/17/2016 10:38 AM, Adrien Mazarguil wrote: > On Thu, Nov 17, 2016 at 10:49:55AM +0100, Nelio Laranjeiro wrote: >> Completion queue entry data uses network endian, to access them we should use >> ntoh*(). >> >> Fixes: c305090bbaf8 ("net/mlx5: replace countdown with threshold for Tx >> completions") >> >> CC: stable at dpdk.org >> Reported-by: Liming Sun >> Signed-off-by: Nelio Laranjeiro <...> > Acked-by: Adrien Mazarguil Applied to dpdk-next-net/master, thanks.
[dpdk-dev] [dpdk-stable] [PATCH 1/3] net/mlx5: fix leak when starvation occurs
On 11/17/2016 10:37 AM, Adrien Mazarguil wrote: > On Thu, Nov 17, 2016 at 10:49:54AM +0100, Nelio Laranjeiro wrote: >> The list of segments to free was wrongly manipulated ending by only freeing >> the first segment instead of freeing all of them. The last one still >> belongs to the NIC and thus should not be freed. >> >> Fixes: a1bdb71a32da ("net/mlx5: fix crash in Rx") >> >> CC: stable at dpdk.org >> Reported-by: Liming Sun >> Signed-off-by: Nelio Laranjeiro <...> > Acked-by: Adrien Mazarguil Applied to dpdk-next-net/master, thanks.
[dpdk-dev] [PATCH 5/5] ethtool: dispaly bus info and firmware version
On 17/11/2016 17:42, Qiming Yang wrote: > This patch enhances the ethtool-API to support to show the > bus information and firmware version as same as kernel > version ethtool displayed. Suggest rewording as: This patch enhances the ethtool example to support to show bus information and firmware version, in the same way that the Linux kernel ethtool does.
[dpdk-dev] [PATCH 1/5] ethdev: add firmware version get
On 17/11/2016 17:42, Qiming Yang wrote: > This patch added API for 'rte_eth_dev_fwver_get' > > void rte_eth_dev_fwver_get(uint8_t port_id, > char *fw_version, int fw_length); Suggest description such as: This patch adds ethdev API for fetching firmware version. Also see Thomas's comments. Looks like some stale patches got picked up by 'git send-mail *.patch'.. ..Remy
[dpdk-dev] pmdinfogen issues: cross compilation for ARM fails with older host compiler
On Fri, Nov 18, 2016 at 12:03:19PM +, Hemant Agrawal wrote: > > -Original Message- > > From: Neil Horman [mailto:nhorman at tuxdriver.com] > > On Tue, Nov 15, 2016 at 09:34:16AM +, Hemant Agrawal wrote: > > > > On Mon, Nov 14, 2016 at 02:29:24AM +0530, Jerin Jacob wrote: > > > > > On Fri, Nov 11, 2016 at 10:34:39AM +, Hemant Agrawal wrote: > > > > > > Hi Neil, > > > > > >Pmdinfogen compiles with host compiler. It usages > > rte_byteorder.h > > > > of the target platform. > > > > > > However, if the host compiler is older than 4.8, it will be an > > > > > > issue during > > cross > > > > compilation for some platforms. > > > > > > e.g. if we are compiling on x86 host for ARM, x86 host compiler > > > > > > will not > > > > understand the arm asm instructions. > > > > > > > > > > > > /* fix missing __builtin_bswap16 for gcc older then 4.8 */ #if > > > > > > !(__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 8)) static > > > > > > inline uint16_t rte_arch_bswap16(uint16_t _x) { > > > > > >register uint16_t x = _x; > > > > > >asm volatile ("rev16 %0,%1" > > > > > > : "=r" (x) > > > > > > : "r" (x) > > > > > > ); > > > > > >return x; > > > > > > } > > > > > > #endif > > > > > > > > > > > > One easy solution is that we add compiler platform check in this > > > > > > code section of rte_byteorder.h e.g #if !(defined __arm__ || defined > > > > > > __aarch64__) static inline uint16_t rte_arch_bswap16(uint16_t _x) { > > > > > >return (_x >> 8) | ((_x << 8) & 0xff00); } #else ?. > > > > > > > > > > > > Is there a better way to fix it? > > > > > > > > > > IMO, It is a HOST build infrastructure issue. If a host app is using > > > > > the dpdk service then it should compile and link against HOST > > > > > target(in this specific case, build/x86_64-native-linuxapp-gcc). I > > > > > think, introducing the HOSTTARGET kind of scheme is a clean solution. > > > > > > > > > > /Jerin > > > > > > > > > > > > > > That would be accurate. That is to say, pmdinfogen is a tool that > > > > should only > > be > > > > run on the host doing the build, by the host doing the build, and so > > > > should be > > > > compiled to run on the host, not on the target being built for. > > > > > > > > Yeah, so what we need is a way to get to the host version of > > > > rte_byteorder.h > > > > when building in a cross environment > > > > > > > +1 > > > > > > > Neil > > > > > > > Give this a try, I've tested it on linux, but not BSD. From what I read the > > functions are not posix compliant, though they should exist on all BSD and > > Linux > > systems in recent history. There may be some fiddling needed for Net and > > OpenBSD variants, but I think this is the right general direction. > > + 1 > This patch works good for Linux. > Can someone test it on BSD? I'd like to ensure we don't need to modify it for that platform Neil > > > > > > diff --git a/buildtools/pmdinfogen/pmdinfogen.h > > b/buildtools/pmdinfogen/pmdinfogen.h > > index 1da2966..c5ef89d 100644 > > --- a/buildtools/pmdinfogen/pmdinfogen.h > > +++ b/buildtools/pmdinfogen/pmdinfogen.h > > @@ -21,7 +21,6 @@ > > #include > > #include > > #include > > -#include > > > > /* On BSD-alike OSes elf.h defines these according to host's word size */ > > #undef ELF_ST_BIND > > @@ -75,9 +74,9 @@ > > #define CONVERT_NATIVE(fend, width, x) ({ \ > > typeof(x) ___x; \ > > if ((fend) == ELFDATA2LSB) \ > > - ___x = rte_le_to_cpu_##width(x); \ > > + ___x = le##width##toh(x); \ > > else \ > > - ___x = rte_be_to_cpu_##width(x); \ > > + ___x = be##width##toh(x); \ > > ___x; \ > > }) > >
[dpdk-dev] [PATCH v2 1/1] mempool: Add sanity check when secondary link in less mempools than primary
On Fri, Nov 18, 2016 at 04:11:12PM +0100, Olivier Matz wrote: > Hi Jean, > > > Do you mind if we put back this conversation on the ML? Oh, I forgot to do it ? I intended to. Bummer. Please do so. > I think your example shows that there is no linker magic: you just > need the same linker flags for dpdk libraries than in the dpdk > framework. I suppose we need something in the build framework > to provide these flags externally, Good luck integrating that in all foreign build system (I'm looking at you, Snort). > but I don't think we need to patch mempool for that. This sanity check is just that, a sanity check. I don't understand what's bad about a sanity check, it does not change functionality, it does not fix anything and just warn users about those issues. Please look at the patch itself at face value. > Regards, > Olivier Regards, Jean
[dpdk-dev] [PATCH 01/22] ethdev: introduce generic flow API
Hi Adrien, > -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Adrien Mazarguil > Sent: Thursday, November 17, 2016 12:23 AM > To: dev at dpdk.org > Cc: Thomas Monjalon ; De Lara Guarch, > Pablo ; Olivier Matz > > Subject: [dpdk-dev] [PATCH 01/22] ethdev: introduce generic flow API > > This new API supersedes all the legacy filter types described in > rte_eth_ctrl.h. > It is slightly higher level and as a result relies more on PMDs to process and > validate flow rules. > > Benefits: > > - A unified API is easier to program for, applications do not have to be > written for a specific filter type which may or may not be supported by > the underlying device. > > - The behavior of a flow rule is the same regardless of the underlying > device, applications do not need to be aware of hardware quirks. > > - Extensible by design, API/ABI breakage should rarely occur if at all. > > - Documentation is self-standing, no need to look up elsewhere. > > Existing filter types will be deprecated and removed in the near future. > > Signed-off-by: Adrien Mazarguil > + > +/** > + * Opaque type returned after successfully creating a flow. > + * > + * This handle can be used to manage and query the related flow (e.g. > +to > + * destroy it or retrieve counters). > + */ > +struct rte_flow; > + As we talked before, we use attr/pattern/actions to create and destroy a flow in PMD, but I don't think it's easy to clone the user-provided parameters and return the result to the application as a rte_flow pointer. As you suggested: /* PMD-specific code. */ struct rte_flow { struct rte_flow_attr attr; struct rte_flow_item *pattern; struct rte_flow_action *actions; }; Because both pattern and actions are pointers, and there're also pointers in structure rte_flow_item and struct rte_flow_action. We need to iterate allocation during clone and iterate free during destroy, then seems that the code is something ugly, right? I think application saves info when creating a flow rule, so why not application provide attr/pattern/actions info to PMD before calling PMD API? Thanks, Beilei Xing
[dpdk-dev] [PATCH v2] i40e: Fix eth_i40e_dev_init sequence on ThunderX
i40e_asq_send_command: rd32 & wr32 under ThunderX gives unpredictable results. To solve this include rte memory barriers Signed-off-by: Satha Rao --- 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; +#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
[dpdk-dev] [PATCH 1/5] ethdev: add firmware version get
Yes, that's my fault. I have set their statuses to 'suspended' Qiming -Original Message- From: Horton, Remy Sent: Friday, November 18, 2016 9:10 AM To: Yang, Qiming ; dev at dpdk.org Cc: Wu, Jingjing ; Chen, Jing D Subject: Re: [PATCH 1/5] ethdev: add firmware version get On 17/11/2016 17:42, Qiming Yang wrote: > This patch added API for 'rte_eth_dev_fwver_get' > > void rte_eth_dev_fwver_get(uint8_t port_id, char *fw_version, int > fw_length); Suggest description such as: This patch adds ethdev API for fetching firmware version. Also see Thomas's comments. Looks like some stale patches got picked up by 'git send-mail *.patch'.. ..Remy
[dpdk-dev] [PATCH 1/5] ethdev: add firmware version get
Hi, Thomas -Original Message- From: Thomas Monjalon [mailto:thomas.monja...@6wind.com] Sent: Thursday, November 17, 2016 9:37 PM To: Yang, Qiming Cc: dev at dpdk.org; Horton, Remy ; Wu, Jingjing ; Chen, Jing D Subject: Re: [dpdk-dev] [PATCH 1/5] ethdev: add firmware version get 2016-11-17 17:42, Qiming Yang: > This patch added API for 'rte_eth_dev_fwver_get' > > void rte_eth_dev_fwver_get(uint8_t port_id, char *fw_version, int > fw_length); Copying some code here doesn't help really help. Could you describe what we can expect in this string? How can we compare this version number across different devices of the same driver? How does it apply to FPGA devices? What is the potential use? Qiming: I'll add more describes in the next version. [...] > /** > + * Retrieve the firmware version of an Ethernet device. We should stop talking about Ethernet device. Networking device is more appropriate. And in this case, "device" is enough. Qiming: Got it. > + * > + * @param port_id > + * The port identifier of the Ethernet device. > + * @param fw_version > + * A pointer the firmware version of an Ethernet device > + * @param fw_length > + * The size of the firmware version, which should be large enough to store > + * the firmware version of the device. How do we know that the length is too small? Qiming: The length is a number large enough to store the firmware version. Original I use a fixed number 20, but in order to be more flexible, I replace it into a variable. > + */ > +void rte_eth_dev_fwver_get(uint8_t port_id, char *fw_version, int > +fw_length); Why not returning some errors? Qiming: It's a good advice. You forgot to remove the deprecation notice in this patch. PS: the series is broken as some patches are not numbered and this one is not the first one. Please use a cover-letter to introduce such series. Qiming: It's my fault. Thank you for your reminder, I will pay attention next time.