[dpdk-dev] [PATCH v15 2/2] i40e: add device args to enable a floating VEB
The standard Virtual Ethernet Bridge(VEB) definition in 1Qbg is a bridge which has an uplink port to the outside world (maybe another bridge), but a "floating" VEB is a special VEB without an uplink port to the outside. Instead, traffic can be sent from one VF to another using the floating VEB - even when the physical link on the NIC port is down. This patch adds floating VEB options in the devargs for i40e driver. Using these parameters, applications can decide whether to use legacy VEB/VEPA or a floating VEB. To enable this feature, the user should pass a devargs parameter to the EAL, for example "-w 84:00.0,enable_floating_veb=1", to control whether the PMD will to use the floating VEB feature or not. Once the floating VEB feature is enabled, all the VFs created by this PF device are connected to the floating VEB. NOTE: The floating VEB functionality requires a NIC firmware version of 5.0 or greater. Signed-off-by: Zhe Tao --- doc/guides/nics/i40e.rst | 45 ++ doc/guides/rel_notes/release_16_07.rst | 8 ++ drivers/net/i40e/i40e_ethdev.c | 160 + drivers/net/i40e/i40e_ethdev.h | 5 ++ 4 files changed, 218 insertions(+) diff --git a/doc/guides/nics/i40e.rst b/doc/guides/nics/i40e.rst index 934eb02..da695af 100644 --- a/doc/guides/nics/i40e.rst +++ b/doc/guides/nics/i40e.rst @@ -366,3 +366,48 @@ Delete all flow director rules on a port: testpmd> flush_flow_director 0 +Floating VEB +~ + +The Intel? Ethernet Controller X710 and XL710 Family support a feature called +"Floating VEB". + +A Virtual Ethernet Bridge (VEB) is an IEEE Edge Virtual Bridging (EVB) term +for functionality that allows local switching between virtual endpoints within +a physical endpoint and also with an external bridge/network. + +A "Floating" VEB doesn't have an uplink connection to the outside world so all +switching is done internally and remains within the host. As such, this +feature provides security benefits. + +In addition, a Floating VEB overcomes a limitation of normal VEBs where they +cannot forward packets when the physical link is down. Floating VEBs don't need +to connect to the NIC port so they can still forward traffic from VF to VF +even when the physical link is down. + +Therefore, with this feature enabled VFs can be limited to communicating with +each other but not an outside network, and they can do so even when there is +no physical uplink on the associated NIC port. + +To enable this feature, the user should pass a ``devargs`` parameter to the +EAL, for example:: + +-w 84:00.0,enable_floating_veb=1 + +In this configuration the PMD will use the floating VEB feature for all the +VFs created by this PF device. + +Alternatively, the user can specify which VFs need to connect to this floating +VEB using the ``floating_veb_list`` argument:: + +-w 84:00.0,enable_floating_veb=1,floating_veb_list=1;3-4 + +In this example ``VF1``, ``VF3`` and ``VF4`` connect to the floating VEB, +while other VFs connect to the normal VEB. + +The current implementation only supports one floating VEB and one regular +VEB. VFs can connect to a floating VEB or a regular VEB according to the +configuration passed on the EAL command line. + +The floating VEB functionality requires a NIC firmware version of 5.0 +or greater. diff --git a/doc/guides/rel_notes/release_16_07.rst b/doc/guides/rel_notes/release_16_07.rst index 54ebdc7..fc574af 100644 --- a/doc/guides/rel_notes/release_16_07.rst +++ b/doc/guides/rel_notes/release_16_07.rst @@ -132,6 +132,14 @@ New Features * A new library ``librte_pdump`` is added to provide packet capture API. * A new ``app/pdump`` tool is added to capture packets in DPDK. +* **Added floating VEB support for i40e PF driver.** + + A "floating VEB" is a special Virtual Ethernet Bridge (VEB) which does not + have an upload port, but instead is used for switching traffic between + virtual functions (VFs) on a port. + + For information on this feature, please see the "I40E Poll Mode Driver" + section of the "Network Interface Controller Drivers" document. Resolved Issues --- diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c index 5efcbc2..9ef0727 100644 --- a/drivers/net/i40e/i40e_ethdev.c +++ b/drivers/net/i40e/i40e_ethdev.c @@ -63,6 +63,9 @@ #include "i40e_pf.h" #include "i40e_regs.h" +#define ETH_I40E_FLOATING_VEB_ARG "enable_floating_veb" +#define ETH_I40E_FLOATING_VEB_LIST_ARG "floating_veb_list" + #define I40E_CLEAR_PXE_WAIT_MS 200 /* Maximun number of capability elements */ @@ -755,6 +758,161 @@ i40e_add_tx_flow_control_drop_filter(struct i40e_pf *pf) } static int +floating_veb_list_handler(__rte_unused const char *key, + const char *floating_veb_value, + void *opaque) +{ + int idx = 0; + unsigned int count = 0; + char *end = NULL; + int min, max; + bool *vf_floa
[dpdk-dev] [PATCH v15 1/2] i40e: add floating VEB support
The standard Virtual Ethernet Bridge(VEB) definition in 1Qbg is a bridge which has an uplink port to the outside world (maybe another bridge), but a "floating" VEB is a special VEB without an uplink port to the outside. Instead, traffic can be sent from one VF to another using the floating VEB - even when the physical link on the NIC port is down. VFs VSIs connect either to the standard VEB/VEPA or to the floating VEB, they cannot connect to both of them. The PF, VMDQ and FD VSIs still connect to the normal VEB/VEPA. Signed-off-by: Zhe Tao --- drivers/net/i40e/i40e_ethdev.c | 113 + drivers/net/i40e/i40e_ethdev.h | 7 +++ drivers/net/i40e/i40e_pf.c | 12 - 3 files changed, 108 insertions(+), 24 deletions(-) diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c index f94ad87..5efcbc2 100644 --- a/drivers/net/i40e/i40e_ethdev.c +++ b/drivers/net/i40e/i40e_ethdev.c @@ -3768,21 +3768,27 @@ i40e_veb_release(struct i40e_veb *veb) struct i40e_vsi *vsi; struct i40e_hw *hw; - if (veb == NULL || veb->associate_vsi == NULL) + if (veb == NULL) return -EINVAL; if (!TAILQ_EMPTY(&veb->head)) { PMD_DRV_LOG(ERR, "VEB still has VSI attached, can't remove"); return -EACCES; } + /* associate_vsi field is NULL for floating VEB */ + if (veb->associate_vsi != NULL) { + vsi = veb->associate_vsi; + hw = I40E_VSI_TO_HW(vsi); - vsi = veb->associate_vsi; - hw = I40E_VSI_TO_HW(vsi); + vsi->uplink_seid = veb->uplink_seid; + vsi->veb = NULL; + } else { + veb->associate_pf->main_vsi->floating_veb = NULL; + hw = I40E_VSI_TO_HW(veb->associate_pf->main_vsi); + } - vsi->uplink_seid = veb->uplink_seid; i40e_aq_delete_element(hw, veb->seid, NULL); rte_free(veb); - vsi->veb = NULL; return I40E_SUCCESS; } @@ -3794,9 +3800,9 @@ i40e_veb_setup(struct i40e_pf *pf, struct i40e_vsi *vsi) int ret; struct i40e_hw *hw; - if (NULL == pf || vsi == NULL) { - PMD_DRV_LOG(ERR, "veb setup failed, " - "associated VSI shouldn't null"); + if (pf == NULL) { + PMD_DRV_LOG(ERR, + "veb setup failed, associated PF shouldn't null"); return NULL; } hw = I40E_PF_TO_HW(pf); @@ -3808,11 +3814,19 @@ i40e_veb_setup(struct i40e_pf *pf, struct i40e_vsi *vsi) } veb->associate_vsi = vsi; + veb->associate_pf = pf; TAILQ_INIT(&veb->head); - veb->uplink_seid = vsi->uplink_seid; + veb->uplink_seid = vsi ? vsi->uplink_seid : 0; - ret = i40e_aq_add_veb(hw, veb->uplink_seid, vsi->seid, - I40E_DEFAULT_TCMAP, false, &veb->seid, false, NULL); + /* create floating veb if vsi is NULL */ + if (vsi != NULL) { + ret = i40e_aq_add_veb(hw, veb->uplink_seid, vsi->seid, + I40E_DEFAULT_TCMAP, false, + &veb->seid, false, NULL); + } else { + ret = i40e_aq_add_veb(hw, 0, 0, I40E_DEFAULT_TCMAP, + true, &veb->seid, false, NULL); + } if (ret != I40E_SUCCESS) { PMD_DRV_LOG(ERR, "Add veb failed, aq_err: %d", @@ -3828,10 +3842,10 @@ i40e_veb_setup(struct i40e_pf *pf, struct i40e_vsi *vsi) hw->aq.asq_last_status); goto fail; } - /* Get VEB bandwidth, to be implemented */ /* Now associated vsi binding to the VEB, set uplink to this VEB */ - vsi->uplink_seid = veb->seid; + if (vsi) + vsi->uplink_seid = veb->seid; return veb; fail: @@ -3847,6 +3861,7 @@ i40e_vsi_release(struct i40e_vsi *vsi) struct i40e_vsi_list *vsi_list; int ret; struct i40e_mac_filter *f; + uint16_t user_param = vsi->user_param; if (!vsi) return I40E_SUCCESS; @@ -3864,12 +3879,22 @@ i40e_vsi_release(struct i40e_vsi *vsi) i40e_veb_release(vsi->veb); } + if (vsi->floating_veb) { + TAILQ_FOREACH(vsi_list, &vsi->floating_veb->head, list) { + if (i40e_vsi_release(vsi_list->vsi) != I40E_SUCCESS) + return -1; + TAILQ_REMOVE(&vsi->floating_veb->head, vsi_list, list); + } + } + /* Remove all macvlan filters of the VSI */ i40e_vsi_remove_all_macvlan_filter(vsi); TAILQ_FOREACH(f, &vsi->mac_list, next) rte_free(f); - if (vsi->type != I40E_VSI_MAIN) { + if (vsi->type != I40E_VSI_MAIN && + ((vsi->type != I40E_VSI_SRIOV) || + !pf->floating_veb_list[user_param])) { /*
[dpdk-dev] [PATCH v15 0/2] i40e: add floating VEB support for i40e
This patch-set add the support for floating VEB in i40e. All the VFs VSIs can decide whether to connect to the legacy VEB/VEPA or the floating VEB. When connect to the floating VEB a new floating VEB is created. Now all the VFs need to connect to floating VEB or legacy VEB, cannot connect to both of them. The PF and VMDQ,FD VSIs connect to the old legacy VEB/VEPA. All the VEB/VEPA concepts are not specific for FVL, they are defined in the 802.1Qbg spec. This floating VEB only take effects on the specific version F/W which newer than 5.0. Zhe Tao (2): i40e: add floating VEB support i40e: add device args to enable a floating VEB doc/guides/nics/i40e.rst | 45 ++ doc/guides/rel_notes/release_16_07.rst | 8 + drivers/net/i40e/i40e_ethdev.c | 273 ++--- drivers/net/i40e/i40e_ethdev.h | 12 ++ drivers/net/i40e/i40e_pf.c | 12 +- 5 files changed, 326 insertions(+), 24 deletions(-) V2: Added the release notes and changed commit log. V3: Changed the VSI release operation. V4: Added the FW version check otherwise it will cause the segment fault. V5: Edited the code for new share code APIs V6: Changed the floating VEB configuration method V7: Added global reset for i40e V8: removed global reset and added floating VEB extension support V9: Added floating VEB related explanation into commit log V10: Changed third patch commit log V11: Fixed the issues reported by check-git-log.sh V12: Changed the floating VEB VF bitmask to VF list V13: Changed some internal functions'names to make it more clear V14: Changed some internal variable names V15: Changed the sequence of patches and add more explanation for floating VEB -- 2.1.4
[dpdk-dev] [PATCH] net/thunderx: add missing external library dependency
To prevent compile error when compiling for shared library: nicvf_hw.o: In function `nicvf_qsize_regbit': nicvf_hw.c:(.text+0xe64): undefined reference to `log2' nicvf_hw.o: In function `nicvf_rss_reta_update': nicvf_hw.c:(.text+0x19dd): undefined reference to `log2' nicvf_hw.o: In function `nicvf_rss_reta_query': nicvf_hw.c:(.text+0x1acb): undefined reference to `log2' Fixes: e438796617dc ("net/thunderx: add PMD skeleton") Signed-off-by: Ferruh Yigit --- drivers/net/thunderx/Makefile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/net/thunderx/Makefile b/drivers/net/thunderx/Makefile index 078237e..8ea6b45 100644 --- a/drivers/net/thunderx/Makefile +++ b/drivers/net/thunderx/Makefile @@ -39,6 +39,8 @@ LIB = librte_pmd_thunderx_nicvf.a CFLAGS += $(WERROR_FLAGS) +LDLIBS += -lm + EXPORT_MAP := rte_pmd_thunderx_nicvf_version.map LIBABIVER := 1 -- 2.7.4
[dpdk-dev] [PATCH] ethdev: fix extended statistics description
The old structure rte_eth_xstats contained names and values. The new structure rte_eth_xstat contains ids and values. Fixes: bd6aa172cf35 ("ethdev: fetch extended statistics with integer ids") Fixes: e2aae1c1ced9 ("ethdev: remove name from extended statistic fetch") Signed-off-by: Thomas Monjalon --- lib/librte_ether/rte_ethdev.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h index ebe6578..f86ad72 100644 --- a/lib/librte_ether/rte_ethdev.h +++ b/lib/librte_ether/rte_ethdev.h @@ -2316,8 +2316,8 @@ int rte_eth_xstats_get_names(uint8_t port_id, * @param port_id * The port identifier of the Ethernet device. * @param xstats - * A pointer to a table of structure of type *rte_eth_xstats* - * to be filled with device statistics names and values. + * A pointer to a table of structure of type *rte_eth_xstat* + * to be filled with device statistics ids and values. * This parameter can be set to NULL if n is 0. * @param n * The size of the stats table, which should be large enough to store -- 2.7.0
[dpdk-dev] xstats performance
2016-06-29 17:03, Remy Horton: > On 29/06/2016 16:38, Olivier MATZ wrote: > > And assume that the id field in rte_eth_xstat corresponds to > > the index in the rte_eth_xstat_name table? > > It was an assumption I wanted to avoid setting in stone, even though at > the moment it is actually true in implementation. > > The mappings are driver-specific but I wanted to leave open, amoung > other things, the possibility of string-id mappings being standardised > across all drivers. For that to work drivers would have to be able to > use a set of ids that may contain gaps. I don't think it is possible to standardize stats ids, for two reasons: - it is hard to maintain and avoid conflicts between drivers - the drivers would have to lookup the names which degrades performance I think the idea of Olivier would improve the performance of stats retrieval, which was the idea of this rework :) Unfortunately we need someone available to fix it quickly for RC2.
[dpdk-dev] [PATCH] mempool: rename functions with confusing names
On 06/29/2016 06:02 PM, Wiles, Keith wrote: > > On 6/29/16, 11:00 AM, "dev on behalf of Bruce Richardson" dpdk.org on behalf of bruce.richardson at intel.com> wrote: > >> On Wed, Jun 29, 2016 at 05:55:27PM +0200, Thomas Monjalon wrote: >>> 2016-06-29 14:55, Bruce Richardson: The mempool_count and mempool_free_count behaved contrary to what their names suggested. The free_count function actually returned the number of elements that were allocated from the pool, not the number unallocated as the name implied. I agree the current API is not appropriate. Fix this by introducing two new functions to replace the old ones, * rte_mempool_unallocated_count to replace rte_mempool_count * rte_mempool_allocated_count to replace rte_mempool_free_count >>> >>> What about available/used instead of unallocated/allocated? >>> >> >> I don't particularly mind what the name is, to be honest. I like "avail" >> because it is shorter, but I'm a little uncertain about "used", because it >> implies that the entries are finished with i.e. like a used match, or tissue >> :-) >> >> How about "avail/in_use"? > > +1 for those names. +1 too. rte_mempool_avail_count() rte_mempool_in_use_count() Thanks
[dpdk-dev] [PATCH] mempool: rename functions with confusing names
2016-06-29 14:55, Bruce Richardson: > The mempool_count and mempool_free_count behaved contrary to what their > names suggested. The free_count function actually returned the number of > elements that were allocated from the pool, not the number unallocated as > the name implied. > > Fix this by introducing two new functions to replace the old ones, > * rte_mempool_unallocated_count to replace rte_mempool_count > * rte_mempool_allocated_count to replace rte_mempool_free_count What about available/used instead of unallocated/allocated?
[dpdk-dev] xstats performance
Hi Remy, While adapting an application to the new xstats API, I discovered that it may not be so efficient to display the statistics and their names. I think the test-pmd code illustrates the issue pretty well: /* Display xstats */ for (idx_xstat = 0; idx_xstat < cnt_xstats; idx_xstat++) for (idx_name = 0; idx_name < cnt_xstats; idx_name++) if (xstats_names[idx_name].id == xstats[idx_xstat].id) { printf("%s: %"PRIu64"\n", xstats_names[idx_name].name, xstats[idx_xstat].value); break; } The displaying is in O(n^2). It's possible to enhance the code to have it in O(n), but it requires an intermediate table. Why not changing this: struct rte_eth_xstat { uint64_t id; uint64_t value; }; struct rte_eth_xstat_name { char name[RTE_ETH_XSTATS_NAME_SIZE]; uint64_t id; }; Into this: struct rte_eth_xstat { uint64_t id; uint64_t value; }; struct rte_eth_xstat_name { char name[RTE_ETH_XSTATS_NAME_SIZE]; /* No identifier */ }; And assume that the id field in rte_eth_xstat corresponds to the index in the rte_eth_xstat_name table? The test-pmd code would be something like this: /* Display xstats */ for (idx_xstat = 0; idx_xstat < cnt_xstats; idx_xstat++) { printf("%s: %"PRIu64"\n", xstats_names[xstats[idx_xstats].id].name, xstats[idx_xstat].value); } What do you think? Regards Olivier
[dpdk-dev] [PATCH v2] mempool: rename functions with confusing names
+ Correct email for Thomas :-( On Wed, Jun 29, 2016 at 05:27:15PM +0100, Bruce Richardson wrote: > The mempool_count and mempool_free_count behaved contrary to what their > names suggested. The free_count function actually returned the number of > elements that were allocated from the pool, not the number unallocated as > the name implied. > > Fix this by introducing two new functions to replace the old ones, > * rte_mempool_avail_count to replace rte_mempool_count > * rte_mempool_in_use_count to replace rte_mempool_free_count > > In this patch, the new functions are added, and the old ones are marked > as deprecated. All apps and examples that use the old functions are > updated to use the new functions. > > Fixes: af75078fece3 ("first public release") > > Signed-off-by: Bruce Richardson > --- > > v2 changes > * original patch used allocated/unallocated for new functions. Change > those to in_use/avail. > > --- > app/test/test_cryptodev.c | 6 ++--- > app/test/test_cryptodev_perf.c | 6 ++--- > app/test/test_mbuf.c | 4 +-- > app/test/test_mempool.c| 4 +-- > app/test/test_mempool_perf.c | 2 +- > doc/guides/rel_notes/deprecation.rst | 6 + > drivers/net/qede/qede_rxtx.c | 4 +-- > examples/multi_process/l2fwd_fork/main.c | 3 ++- > examples/qos_sched/main.c | 2 -- > lib/librte_mempool/rte_mempool.c | 10 +-- > lib/librte_mempool/rte_mempool.h | 42 > +++--- > lib/librte_mempool/rte_mempool_version.map | 1 + > 12 files changed, 69 insertions(+), 21 deletions(-) > > diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c > index 9dfe34f..fbfe1d0 100644 > --- a/app/test/test_cryptodev.c > +++ b/app/test/test_cryptodev.c > @@ -316,12 +316,12 @@ testsuite_teardown(void) > > if (ts_params->mbuf_pool != NULL) { > RTE_LOG(DEBUG, USER1, "CRYPTO_MBUFPOOL count %u\n", > - rte_mempool_count(ts_params->mbuf_pool)); > + rte_mempool_avail_count(ts_params->mbuf_pool)); > } > > if (ts_params->op_mpool != NULL) { > RTE_LOG(DEBUG, USER1, "CRYPTO_OP_POOL count %u\n", > - rte_mempool_count(ts_params->op_mpool)); > + rte_mempool_avail_count(ts_params->op_mpool)); > } > > } > @@ -412,7 +412,7 @@ ut_teardown(void) > > if (ts_params->mbuf_pool != NULL) > RTE_LOG(DEBUG, USER1, "CRYPTO_MBUFPOOL count %u\n", > - rte_mempool_count(ts_params->mbuf_pool)); > + rte_mempool_avail_count(ts_params->mbuf_pool)); > > rte_cryptodev_stats_get(ts_params->valid_devs[0], &stats); > > diff --git a/app/test/test_cryptodev_perf.c b/app/test/test_cryptodev_perf.c > index e484cbb..d728211 100644 > --- a/app/test/test_cryptodev_perf.c > +++ b/app/test/test_cryptodev_perf.c > @@ -343,10 +343,10 @@ testsuite_teardown(void) > > if (ts_params->mbuf_mp != NULL) > RTE_LOG(DEBUG, USER1, "CRYPTO_PERF_MBUFPOOL count %u\n", > - rte_mempool_count(ts_params->mbuf_mp)); > + rte_mempool_avail_count(ts_params->mbuf_mp)); > if (ts_params->op_mpool != NULL) > RTE_LOG(DEBUG, USER1, "CRYPTO_PERF_OP POOL count %u\n", > - rte_mempool_count(ts_params->op_mpool)); > + rte_mempool_avail_count(ts_params->op_mpool)); > } > > static int > @@ -395,7 +395,7 @@ ut_teardown(void) > > if (ts_params->mbuf_mp != NULL) > RTE_LOG(DEBUG, USER1, "CRYPTO_PERF_MBUFPOOL count %u\n", > - rte_mempool_count(ts_params->mbuf_mp)); > + rte_mempool_avail_count(ts_params->mbuf_mp)); > > rte_cryptodev_stats_get(ts_params->dev_id, &stats); > > diff --git a/app/test/test_mbuf.c b/app/test/test_mbuf.c > index 1835acc..8664885 100644 > --- a/app/test/test_mbuf.c > +++ b/app/test/test_mbuf.c > @@ -717,7 +717,7 @@ test_refcnt_iter(unsigned lcore, unsigned iter) >* - increment it's reference up to N+1, >* - enqueue it N times into the ring for slave cores to free. >*/ > - for (i = 0, n = rte_mempool_count(refcnt_pool); > + for (i = 0, n = rte_mempool_avail_count(refcnt_pool); > i != n && (m = rte_pktmbuf_alloc(refcnt_pool)) != NULL; > i++) { > ref = RTE_MAX(rte_rand() % REFCNT_MAX_REF, 1UL); > @@ -745,7 +745,7 @@ test_refcnt_iter(unsigned lcore, unsigned iter) > > /* check that all mbufs are back into mempool by now */ > for (wn = 0; wn != REFCNT_MAX_TIMEOUT; wn++) { > - if ((i = rte_mempool_count(refcnt_pool)) == n) { > + if ((i = rte_mempool_avail_count(refcnt_pool)) == n) { > refcnt_lcore[lcore] += tref; > printf("%s(lcore=%u, iter=%u) completed, " > "%u references processed\n",
[dpdk-dev] [PATCH v2] mempool: rename functions with confusing names
The mempool_count and mempool_free_count behaved contrary to what their names suggested. The free_count function actually returned the number of elements that were allocated from the pool, not the number unallocated as the name implied. Fix this by introducing two new functions to replace the old ones, * rte_mempool_avail_count to replace rte_mempool_count * rte_mempool_in_use_count to replace rte_mempool_free_count In this patch, the new functions are added, and the old ones are marked as deprecated. All apps and examples that use the old functions are updated to use the new functions. Fixes: af75078fece3 ("first public release") Signed-off-by: Bruce Richardson --- v2 changes * original patch used allocated/unallocated for new functions. Change those to in_use/avail. --- app/test/test_cryptodev.c | 6 ++--- app/test/test_cryptodev_perf.c | 6 ++--- app/test/test_mbuf.c | 4 +-- app/test/test_mempool.c| 4 +-- app/test/test_mempool_perf.c | 2 +- doc/guides/rel_notes/deprecation.rst | 6 + drivers/net/qede/qede_rxtx.c | 4 +-- examples/multi_process/l2fwd_fork/main.c | 3 ++- examples/qos_sched/main.c | 2 -- lib/librte_mempool/rte_mempool.c | 10 +-- lib/librte_mempool/rte_mempool.h | 42 +++--- lib/librte_mempool/rte_mempool_version.map | 1 + 12 files changed, 69 insertions(+), 21 deletions(-) diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c index 9dfe34f..fbfe1d0 100644 --- a/app/test/test_cryptodev.c +++ b/app/test/test_cryptodev.c @@ -316,12 +316,12 @@ testsuite_teardown(void) if (ts_params->mbuf_pool != NULL) { RTE_LOG(DEBUG, USER1, "CRYPTO_MBUFPOOL count %u\n", - rte_mempool_count(ts_params->mbuf_pool)); + rte_mempool_avail_count(ts_params->mbuf_pool)); } if (ts_params->op_mpool != NULL) { RTE_LOG(DEBUG, USER1, "CRYPTO_OP_POOL count %u\n", - rte_mempool_count(ts_params->op_mpool)); + rte_mempool_avail_count(ts_params->op_mpool)); } } @@ -412,7 +412,7 @@ ut_teardown(void) if (ts_params->mbuf_pool != NULL) RTE_LOG(DEBUG, USER1, "CRYPTO_MBUFPOOL count %u\n", - rte_mempool_count(ts_params->mbuf_pool)); + rte_mempool_avail_count(ts_params->mbuf_pool)); rte_cryptodev_stats_get(ts_params->valid_devs[0], &stats); diff --git a/app/test/test_cryptodev_perf.c b/app/test/test_cryptodev_perf.c index e484cbb..d728211 100644 --- a/app/test/test_cryptodev_perf.c +++ b/app/test/test_cryptodev_perf.c @@ -343,10 +343,10 @@ testsuite_teardown(void) if (ts_params->mbuf_mp != NULL) RTE_LOG(DEBUG, USER1, "CRYPTO_PERF_MBUFPOOL count %u\n", - rte_mempool_count(ts_params->mbuf_mp)); + rte_mempool_avail_count(ts_params->mbuf_mp)); if (ts_params->op_mpool != NULL) RTE_LOG(DEBUG, USER1, "CRYPTO_PERF_OP POOL count %u\n", - rte_mempool_count(ts_params->op_mpool)); + rte_mempool_avail_count(ts_params->op_mpool)); } static int @@ -395,7 +395,7 @@ ut_teardown(void) if (ts_params->mbuf_mp != NULL) RTE_LOG(DEBUG, USER1, "CRYPTO_PERF_MBUFPOOL count %u\n", - rte_mempool_count(ts_params->mbuf_mp)); + rte_mempool_avail_count(ts_params->mbuf_mp)); rte_cryptodev_stats_get(ts_params->dev_id, &stats); diff --git a/app/test/test_mbuf.c b/app/test/test_mbuf.c index 1835acc..8664885 100644 --- a/app/test/test_mbuf.c +++ b/app/test/test_mbuf.c @@ -717,7 +717,7 @@ test_refcnt_iter(unsigned lcore, unsigned iter) * - increment it's reference up to N+1, * - enqueue it N times into the ring for slave cores to free. */ - for (i = 0, n = rte_mempool_count(refcnt_pool); + for (i = 0, n = rte_mempool_avail_count(refcnt_pool); i != n && (m = rte_pktmbuf_alloc(refcnt_pool)) != NULL; i++) { ref = RTE_MAX(rte_rand() % REFCNT_MAX_REF, 1UL); @@ -745,7 +745,7 @@ test_refcnt_iter(unsigned lcore, unsigned iter) /* check that all mbufs are back into mempool by now */ for (wn = 0; wn != REFCNT_MAX_TIMEOUT; wn++) { - if ((i = rte_mempool_count(refcnt_pool)) == n) { + if ((i = rte_mempool_avail_count(refcnt_pool)) == n) { refcnt_lcore[lcore] += tref; printf("%s(lcore=%u, iter=%u) completed, " "%u references processed\n", diff --git a/app/test/test_mempool.c b/app/test/test_mempool.c index 31582d8..db159aa 100644 --- a/app/test/test_mempool.c +++ b/app/test/test_mempool.c @@ -210,7 +210,7 @@ test_mempool_basic(struct rte_mempool *mp) /* tests that improve covera
[dpdk-dev] [PATCH 2/4] virtio: introduce RTE_LIBRTE_VIRTIO_INC_VECTOR
On Wed, Jun 29, 2016 at 01:25:35PM +0200, Thomas Monjalon wrote: > 2016-06-29 16:48, Jerin Jacob: > > On Mon, Jun 27, 2016 at 04:59:42PM +0200, Thomas Monjalon wrote: > > > 2016-06-27 20:18, Jerin Jacob: > > > > On Mon, Jun 27, 2016 at 04:19:57PM +0200, Thomas Monjalon wrote: > > > > > 2016-06-27 17:24, Jerin Jacob: > > > > > > --- a/config/common_base > > > > > > +++ b/config/common_base > > > > > > @@ -267,6 +267,7 @@ CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_RX=n > > > > > > CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_TX=n > > > > > > CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_DRIVER=n > > > > > > CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_DUMP=n > > > > > > +CONFIG_RTE_LIBRTE_VIRTIO_INC_VECTOR=y > > > > > > > > > > I don't remember what means INC_VECTOR? > > > > > Why a config option is needed for vector implementations? > > > > > > > > I thought of adding additional configuration option(INC_VECTOR) _apart_ > > > > from > > > > cpu flag based scheme in the patch because even though if a given > > > > platform > > > > has cpu instruction support, in some platforms scalar version may > > > > perform well wrt vector version(based on instruction latency, emulation > > > > required or not > > > > etc). So a top level flag INC_VECTOR, can override the vector selection > > > > for a given platform if required. > > > > > > Isn't it a runtime driver option needed to disable vector virtio? > > > > > > > Regarding INC_VECTOR(INC in vector configuration name, I have no idea, I > > > > followed the existing flags) > > > > $ grep "INC_VECTOR" config/common_base > > > > CONFIG_RTE_IXGBE_INC_VECTOR=y > > > > CONFIG_RTE_LIBRTE_I40E_INC_VECTOR=y > > > > CONFIG_RTE_LIBRTE_FM10K_INC_VECTOR=y > > > > > > If the flag is really needed I would suggest VIRTIO_VECTOR. > > > > OK I will change to VIRTIO_VECTOR > > I would prefer a runtime option. OK The platform I test their was NO need for additional VIRTIO_VECTOR configuration as NEON versions outperforms than scalar version. I thought of adding this option to override for any platform if it need to accommodate such platform differences NEON vs scalar versions. I will change completely to run-time detection based on cpuflags for IA and ARM. Any objections? Jerin
[dpdk-dev] xstats performance
'noon, On 29/06/2016 16:38, Olivier MATZ wrote: [..] > And assume that the id field in rte_eth_xstat corresponds to > the index in the rte_eth_xstat_name table? It was an assumption I wanted to avoid setting in stone, even though at the moment it is actually true in implementation. The mappings are driver-specific but I wanted to leave open, amoung other things, the possibility of string-id mappings being standardised across all drivers. For that to work drivers would have to be able to use a set of ids that may contain gaps. ..Remy
[dpdk-dev] [PATCH] mempool: rename functions with confusing names
On Wed, Jun 29, 2016 at 05:55:27PM +0200, Thomas Monjalon wrote: > 2016-06-29 14:55, Bruce Richardson: > > The mempool_count and mempool_free_count behaved contrary to what their > > names suggested. The free_count function actually returned the number of > > elements that were allocated from the pool, not the number unallocated as > > the name implied. > > > > Fix this by introducing two new functions to replace the old ones, > > * rte_mempool_unallocated_count to replace rte_mempool_count > > * rte_mempool_allocated_count to replace rte_mempool_free_count > > What about available/used instead of unallocated/allocated? > I don't particularly mind what the name is, to be honest. I like "avail" because it is shorter, but I'm a little uncertain about "used", because it implies that the entries are finished with i.e. like a used match, or tissue :-) How about "avail/in_use"? /Bruce
[dpdk-dev] [PATCH 3/4] virtio: move SSE based Rx implementation to separate file
On Tue, Jun 28, 2016 at 02:17:41PM +0800, Jianbo Liu wrote: > On 27 June 2016 at 19:54, Jerin Jacob > wrote: > > split out SSE instruction based virtio simple rx > > implementation to a separate file > > > > Signed-off-by: Jerin Jacob > > --- > > drivers/net/virtio/virtio_rxtx_simple.c | 166 +--- > > drivers/net/virtio/virtio_rxtx_simple_sse.h | 225 > > > > 2 files changed, 226 insertions(+), 165 deletions(-) > > create mode 100644 drivers/net/virtio/virtio_rxtx_simple_sse.h > > > I think it's better to move sse implementation to a C file, > as Bruce pointed out at > http://www.dpdk.org/ml/archives/dev/2016-April/037937.html I can move to C file, That would call for further restructuring of the code by Introducing a new file drivers/net/virtio/virtio_rxtx_simple.h and moving all static inline functions of virtio_rxtx_simple.c so that virtio_rxtx_simple_sse.c and virtio_rxtx_simple_neon.c can include it. Huawei,Yuanhan,All, Are you OK with above restructuring? Jerin > > Jianbo
[dpdk-dev] [PATCH 2/4] virtio: introduce RTE_LIBRTE_VIRTIO_INC_VECTOR
On Mon, Jun 27, 2016 at 04:59:42PM +0200, Thomas Monjalon wrote: > 2016-06-27 20:18, Jerin Jacob: > > On Mon, Jun 27, 2016 at 04:19:57PM +0200, Thomas Monjalon wrote: > > > 2016-06-27 17:24, Jerin Jacob: > > > > --- a/config/common_base > > > > +++ b/config/common_base > > > > @@ -267,6 +267,7 @@ CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_RX=n > > > > CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_TX=n > > > > CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_DRIVER=n > > > > CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_DUMP=n > > > > +CONFIG_RTE_LIBRTE_VIRTIO_INC_VECTOR=y > > > > > > I don't remember what means INC_VECTOR? > > > Why a config option is needed for vector implementations? > > > > I thought of adding additional configuration option(INC_VECTOR) _apart_ from > > cpu flag based scheme in the patch because even though if a given platform > > has cpu instruction support, in some platforms scalar version may > > perform well wrt vector version(based on instruction latency, emulation > > required or not > > etc). So a top level flag INC_VECTOR, can override the vector selection > > for a given platform if required. > > Isn't it a runtime driver option needed to disable vector virtio? > > > Regarding INC_VECTOR(INC in vector configuration name, I have no idea, I > > followed the existing flags) > > $ grep "INC_VECTOR" config/common_base > > CONFIG_RTE_IXGBE_INC_VECTOR=y > > CONFIG_RTE_LIBRTE_I40E_INC_VECTOR=y > > CONFIG_RTE_LIBRTE_FM10K_INC_VECTOR=y > > If the flag is really needed I would suggest VIRTIO_VECTOR. OK I will change to VIRTIO_VECTOR
[dpdk-dev] [PATCH] scripts: check fixed commit branch
The commit id of a fixed commit must be in the current branch. It avoids referencing a local branch or a next-* branch when pushing in the master. Signed-off-by: Thomas Monjalon --- scripts/check-git-log.sh | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/scripts/check-git-log.sh b/scripts/check-git-log.sh index 9a0d068..7d2c7ee 100755 --- a/scripts/check-git-log.sh +++ b/scripts/check-git-log.sh @@ -140,7 +140,11 @@ IFS=' fixtags=$(echo "$tags" | grep '^Fixes: ') bad=$(for fixtag in $fixtags ; do hash=$(echo "$fixtag" | sed 's,^Fixes: \([0-9a-f]*\).*,\1,') - good="Fixes: $hash "$(git log --format='("%s")' -1 $hash 2>&-) + if git branch --contains $hash | grep -q '^\*' ; then + good="Fixes: $hash "$(git log --format='("%s")' -1 $hash 2>&-) + else + good="reference not in current branch" + fi printf "$fixtag" | grep -v "^$good$" done | sed 's,^,\t,') [ -z "$bad" ] || printf "Wrong 'Fixes' reference:\n$bad\n" -- 2.7.0
[dpdk-dev] [PATCH v2 1/3] mempool: add stack (lifo) mempool handler
Hi Dave, On 06/20/2016 02:59 PM, Hunt, David wrote: > Hi Olivier, > > On 20/6/2016 9:17 AM, Olivier Matz wrote: >> Hi David, >> >> On 06/17/2016 04:18 PM, Hunt, David wrote: After reading it, I realize that it's nearly exactly the same code than in "app/test: test external mempool handler". http://patchwork.dpdk.org/dev/patchwork/patch/12896/ We should drop one of them. If this stack handler is really useful for a performance use-case, it could go in librte_mempool. At the first read, the code looks like a demo example : it uses a simple spinlock for concurrent accesses to the common pool. Maybe the mempool cache hides this cost, in this case we could also consider removing the use of the rte_ring. >>> While I agree that the code is similar, the handler in the test is a >>> ring based handler, >>> where as this patch adds an array based handler. >> Not sure I'm getting what you are saying. Do you mean stack instead >> of array? > > Yes, apologies, stack. > >> Actually, both are stacks when talking about bulks of objects. If we >> consider each objects one by one, that's true the order will differ. >> But as discussed in [1], the cache code already reverses the order of >> objects when doing a mempool_get(). I'd say the reversing in cache code >> is not really needed (only the order of object bulks should remain the >> same). A rte_memcpy() looks to be faster, but it would require to do >> some real-life tests to validate or unvalidate this theory. >> >> So to conclude, I still think both code in app/test and lib/mempool are >> quite similar, and only one of them should be kept. >> >> [1] http://www.dpdk.org/ml/archives/dev/2016-May/039873.html > > OK, so we will probably remove the test app portion in the future is if > is not needed, > and if we apply the stack handler proposed in this patch set. Back on this thread. Maybe I misunderstood what you were saying here (because I see this comment is not addressed in v3). I don't think we should add similar code at two different places and then remove it later in another patchset. I feel it's better to have only one instance of the stack handler, either in test, or in librte_mempool. If you plan to do a v4, I think this is something that could go in 16.07-rc2. Regards, Olivier
[dpdk-dev] [PATCH] mempool: rename functions with confusing names
> -Original Message- > From: Olivier MATZ [mailto:olivier.matz at 6wind.com] > Sent: Wednesday, June 29, 2016 5:05 PM > To: Wiles, Keith ; Richardson, Bruce > ; Thomas Monjalon 6wind.com> > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH] mempool: rename functions with confusing > names > > > > On 06/29/2016 06:02 PM, Wiles, Keith wrote: > > > > On 6/29/16, 11:00 AM, "dev on behalf of Bruce Richardson" bounces at dpdk.org on behalf of bruce.richardson at intel.com> wrote: > > > >> On Wed, Jun 29, 2016 at 05:55:27PM +0200, Thomas Monjalon wrote: > >>> 2016-06-29 14:55, Bruce Richardson: > The mempool_count and mempool_free_count behaved contrary to what > their names suggested. The free_count function actually returned > the number of elements that were allocated from the pool, not the > number unallocated as the name implied. > > I agree the current API is not appropriate. > > > Fix this by introducing two new functions to replace the old ones, > * rte_mempool_unallocated_count to replace rte_mempool_count > * rte_mempool_allocated_count to replace rte_mempool_free_count > >>> > >>> What about available/used instead of unallocated/allocated? > >>> > >> > >> I don't particularly mind what the name is, to be honest. I like > "avail" > >> because it is shorter, but I'm a little uncertain about "used", > >> because it implies that the entries are finished with i.e. like a > >> used match, or tissue :-) > >> > >> How about "avail/in_use"? > > > > +1 for those names. > > +1 too. > > rte_mempool_avail_count() > rte_mempool_in_use_count() > Ok, I'll see about doing a V2 for review. /Bruce
[dpdk-dev] [PATCHv8 5/6] pmdinfo.py: Add tool to query binaries for hw and other support information
'noon, The tool does not work for static PMD libraries (e.g. librte_pmd_i40e.a) - is this an intended limitation? DPDK doesn't to my knowledge have any coding guidelines for Python, so the comments below should be considered advisory rather than merge-blocking issues. On 17/06/2016 19:46, Neil Horman wrote: [..] > +++ b/tools/pmdinfo.py > @@ -0,0 +1,629 @@ > +#!/usr/bin/python > +# - > +# scripts/pmdinfo.py > +# > +# Utility to dump PMD_INFO_STRING support from an object file > +# No licence..? > +# - > +import os > +import sys > +from optparse import OptionParser > +import string > +import json > + > +# For running from development directory. It should take precedence over the > +# installed pyelftools. > +sys.path.insert(0, '.') Aside from causing all the subsequent imports to have PEP8 errors, this does not looks like a good way of pulling in project-specific Python library installs. Usual method is either using virtualenv or the PYTHONPATH enviornment variable. > +from elftools import __version__ > +from elftools.common.exceptions import ELFError [..] > +from elftools.dwarf.constants import ( > +DW_LNS_copy, DW_LNS_set_file, DW_LNE_define_file) > +from elftools.dwarf.callframe import CIE, FDE According to PyLint, most of these imports are unused. > + > + > +class Vendor: Old style class definition. Using modern notation it is: class Vendor(object): > +def report(self): > +print "\t%s\t%s" % (self.ID, self.name) > +for subID, subdev in self.subdevices.items(): > +subdev.report() subID unused. An underscore can be used as a placeholder for these types of loops. > +optparser.add_option("-t", "--table", dest="tblout", > + help="output information on hw support as a hex > table", PEP8: pmdinfo.py:573:80: E501 line too long (80 > 79 characters)
[dpdk-dev] [PATCH 2/2] enic: fix Rx queue initialization after restarting a device
If you stop then start a port that had already received some packets, the NIC could fetch discriptors from the wrong location. This could effectivly reduce the size of the Rx queue by a random amount and cause packet drop or reduced performance. Reset the NIC fetch index to 0 when allocating and posting mbuf addresses to the NIC. Fixes: 947d860c821f ("enic: improve Rx performance") Signed-off-by: John Daley Reviewed-by: Nelson Escobar --- drivers/net/enic/enic_main.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c index dc831b4..0475cc1 100644 --- a/drivers/net/enic/enic_main.c +++ b/drivers/net/enic/enic_main.c @@ -334,6 +334,7 @@ enic_alloc_rx_queue_mbufs(struct enic *enic, struct vnic_rq *rq) dev_debug(enic, "port=%u, qidx=%u, Write %u posted idx, %u sw held\n", enic->port_id, rq->index, rq->posted_index, rq->rx_nb_hold); iowrite32(rq->posted_index, &rq->ctrl->posted_index); + iowrite32(0, &rq->ctrl->fetch_index); rte_rmb(); return 0; -- 2.7.0
[dpdk-dev] [PATCH 1/2] enic: fix segfault on Tx path after restarting a device
From: Nelson Escobar If you stop then start a port that had already sent some packets, there was a segfault due to not resetting the number of completed sends to zero. Fixes: d5d882fe1a11 ("Tx path rewrite to reduce Host CPU overhead") Signed-off-by: Nelson Escobar Reviewed-by: John Daley --- drivers/net/enic/base/vnic_wq.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/net/enic/base/vnic_wq.c b/drivers/net/enic/base/vnic_wq.c index 9b9ff4d..7c4119c 100644 --- a/drivers/net/enic/base/vnic_wq.c +++ b/drivers/net/enic/base/vnic_wq.c @@ -197,6 +197,8 @@ void vnic_wq_clean(struct vnic_wq *wq, wq->head_idx = 0; wq->tail_idx = 0; + wq->last_completed_index = 0; + *((uint32_t *)wq->cqmsg_rz->addr) = 0; iowrite32(0, &wq->ctrl->fetch_index); iowrite32(0, &wq->ctrl->posted_index); -- 2.7.0
[dpdk-dev] [PATCH] mempool: rename functions with confusing names
On 6/29/16, 11:00 AM, "dev on behalf of Bruce Richardson" wrote: >On Wed, Jun 29, 2016 at 05:55:27PM +0200, Thomas Monjalon wrote: >> 2016-06-29 14:55, Bruce Richardson: >> > The mempool_count and mempool_free_count behaved contrary to what their >> > names suggested. The free_count function actually returned the number of >> > elements that were allocated from the pool, not the number unallocated as >> > the name implied. >> > >> > Fix this by introducing two new functions to replace the old ones, >> > * rte_mempool_unallocated_count to replace rte_mempool_count >> > * rte_mempool_allocated_count to replace rte_mempool_free_count >> >> What about available/used instead of unallocated/allocated? >> > >I don't particularly mind what the name is, to be honest. I like "avail" >because it is shorter, but I'm a little uncertain about "used", because it >implies that the entries are finished with i.e. like a used match, or tissue >:-) > >How about "avail/in_use"? +1 for those names. > >/Bruce >
[dpdk-dev] dpdk config on VM
hi , The below error is because invalid coremask , your VM has only one core , but you are trying to assign eal option as -c 0xf that means you are assigning 1,2,3,4 cores to dpdk application which are not available in your VM . you can increase sufficient number of cores while u launch virtual machine . please check EAL options for dpdk before proceeding . http://dpdk.org/doc/guides-16.04/testpmd_app_ug/run_app.html On Wed, Jun 29, 2016 at 2:20 PM, Raja Jayapal wrote: > Hi All, > > I have installed dpdk(16.04) on ubuntu VM and trying to run > helloworld/testpmd application, but i am getting the below errors. > > root at host4-VirtualBox:/home/host4/dpdk/dpdk-16.04/examples/helloworld# > echo $RTE_SDK > /home/host4/dpdk/dpdk-16.04 > root at host4-VirtualBox:/home/host4/dpdk/dpdk-16.04/examples/helloworld# > echo $RTE_TARGET > x86_64-native-linuxapp-gcc > root at host4-VirtualBox:/home/host4/dpdk/dpdk-16.04/examples/helloworld# > ./build/helloworld -c f -n 4 > EAL: Detected lcore 0 as core 0 on socket 0 > EAL: Support maximum 128 logical core(s) by configuration. > EAL: Detected 1 lcore(s) > EAL: lcore 1 unavailable > EAL: invalid coremask > > Usage: ./build/helloworld [options] > > EAL common options: > -c COREMASK Hexadecimal bitmask of cores to run on > -l CORELIST List of cores to run on > The argument format is [-c2][,c3[-c4],...] > where c1, c2, etc are core indexes between 0 and 128 > --lcores COREMAPMap lcore set to physical cpu set > The argument format is > '[<,lcores[@cpus]>...]' > lcores and cpus list are grouped by '(' and ')' > > root at host4-VirtualBox:/home/host4/dpdk/dpdk-16.04/app/test-pmd# ./testpmd > -c 0xf -n 4 -- -i --portmask=0x1 --nb-cores=2 > EAL: Detected lcore 0 as core 0 on socket 0 > EAL: Support maximum 128 logical core(s) by configuration. > EAL: Detected 1 lcore(s) > EAL: lcore 1 unavailable > EAL: invalid coremask > > Usage: ./testpmd [options] > > EAL common options: > -c COREMASK Hexadecimal bitmask of cores to run on > -l CORELIST List of cores to run on > The argument format is [-c2][,c3[-c4],...] > where c1, c2, etc are core indexes between 0 and 128 > > > I tried with the options provided in below thread, but getting same error. > http://openvswitch.org/pipermail/discuss/2014-December/016007.html > > Please let me know what could be the cause? > > Thanks, > Raja > > > -Srinivasreddy R wrote: - > To: Raja Jayapal > From: Srinivasreddy R > Date: 06/27/2016 07:03PM > Cc: "dev at dpdk.org" > Subject: Re: [dpdk-dev] dpdk config on VM > > > hi, > check with the below link .With little changes you can make it work for > your use case . > > https://github.com/01org/dpdk-ovs/blob/development/docs/04_Sample_Configurations/02_Userspace-vHost.md > > > On Mon, Jun 27, 2016 at 2:50 PM, Raja Jayapal > wrote: > >> Hi All, >> >> I am tyrying to install dpdk on ubuntu and test the applications. >> I have successfully installed dpdk on Host as per the instruction guide , >> but little bit confused on how to bind it with VM. >> >> VM1(TrafficGen)--Host(DPDK)--VM2(Receiver) >> >> Do we need to install ovs to test the flows for the above scenarios?. >> I did search a lot, but didnt get clear idea like how to do config on VM >> side. >> Please point out some config/installation guide for the same. >> >> >> Thanks in Advance, >> Raja >> =-=-= >> Notice: The information contained in this e-mail >> message and/or attachments to it may contain >> confidential or privileged information. If you are >> not the intended recipient, any dissemination, use, >> review, distribution, printing or copying of the >> information contained in this e-mail message >> and/or attachments to it are strictly prohibited. If >> you have received this communication in error, >> please notify us by reply e-mail or telephone and >> immediately and permanently delete the message >> and any attachments. Thank you >> >> >> > > > > Regards, > srinivas. > -- regards srinivas.
[dpdk-dev] Can one process be primary and secondary both at the same time?
Hi, Based on 18. Multi-process Support ( http://dpdk.org/doc/guides-16.04/prog_guide/multi_proc_support.html), we cannot create one process which works as primary and secondary at the same. Is it true? For example, there are three applications. A <-> B <-> C. A : primary for B B : secondary for A B : primary for C C : secondary for B If it is impossible, what is the main reasons? Thanks, Junguk
[dpdk-dev] [PATCH v15 2/2] i40e: add device args to enable a floating VEB
On Wed, Jun 29, 2016 at 01:47:27PM +, Mcnamara, John wrote: > > -Original Message- > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Zhe Tao > > Sent: Wednesday, June 29, 2016 2:06 PM > > To: dev at dpdk.org > > Cc: Tao, Zhe ; Wu, Jingjing > > Subject: [dpdk-dev] [PATCH v15 2/2] i40e: add device args to enable a > > floating VEB > > > > The standard Virtual Ethernet Bridge(VEB) definition in 1Qbg is a bridge > > which has an uplink port to the outside world (maybe another bridge), but > > a "floating" VEB is a special VEB without an uplink port to the outside. > > ... > > > > Signed-off-by: Zhe Tao > > Documentation part: > > Acked-by: John McNamara > Patchset applied to dpdk-next-net/rel_16_07 Thanks for the work going through so many revisions. /Bruce
[dpdk-dev] [PATCH v1 1/1] examples/bond: fix unchecked return value
The example is calling rte_eal_wait_lcore without checking return value. Now it is fixed by checking the value and print proper message. Coverity issue: 37789 Coverity issue: 37790 Fixes: cc7e8ae84faa ("examples/bond: add example application for link bonding mode 6") Signed-off-by: Piotr Azarewicz --- examples/bond/main.c | 24 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/examples/bond/main.c b/examples/bond/main.c index 53bd044..776fad0 100644 --- a/examples/bond/main.c +++ b/examples/bond/main.c @@ -590,10 +590,14 @@ static void cmd_stop_parsed(__attribute__((unused)) void *parsed_result, return; } global_flag_stru_p->LcoreMainIsRunning = 0; - rte_eal_wait_lcore(global_flag_stru_p->LcoreMainCore); - cmdline_printf(cl, - "lcore_main stopped on core:%d\n", - global_flag_stru_p->LcoreMainCore); + if (rte_eal_wait_lcore(global_flag_stru_p->LcoreMainCore) < 0) + cmdline_printf(cl, + "error: lcore_main can not stop on core:%d\n", + global_flag_stru_p->LcoreMainCore); + else + cmdline_printf(cl, + "lcore_main stopped on core:%d\n", + global_flag_stru_p->LcoreMainCore); rte_spinlock_unlock(&global_flag_stru_p->lock); } @@ -628,10 +632,14 @@ static void cmd_quit_parsed(__attribute__((unused)) void *parsed_result, return; } global_flag_stru_p->LcoreMainIsRunning = 0; - rte_eal_wait_lcore(global_flag_stru_p->LcoreMainCore); - cmdline_printf(cl, - "lcore_main stopped on core:%d\n", - global_flag_stru_p->LcoreMainCore); + if (rte_eal_wait_lcore(global_flag_stru_p->LcoreMainCore) < 0) + cmdline_printf(cl, + "error: lcore_main can not stop on core:%d\n", + global_flag_stru_p->LcoreMainCore); + else + cmdline_printf(cl, + "lcore_main stopped on core:%d\n", + global_flag_stru_p->LcoreMainCore); rte_spinlock_unlock(&global_flag_stru_p->lock); cmdline_quit(cl); } -- 1.7.9.5
[dpdk-dev] [PATCH v1 28/28] ether: support SoC device/driver
Hi Jan, On Friday 06 May 2016 07:18 PM, Jan Viktorin wrote: > Signed-off-by: Jan Viktorin > --- > lib/librte_ether/rte_ethdev.c | 127 > +- > lib/librte_ether/rte_ethdev.h | 31 +++ > 2 files changed, 157 insertions(+), 1 deletion(-) > > diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c > index 4af2e5f..9259c2c 100644 > --- a/lib/librte_ether/rte_ethdev.c > +++ b/lib/librte_ether/rte_ethdev.c > @@ -320,6 +320,99 @@ rte_eth_dev_pci_remove(struct rte_pci_device *pci_dev) > } > [...] > +int > rte_eth_dev_is_valid_port(uint8_t port_id) > { > if (port_id >= RTE_MAX_ETHPORTS || > @@ -1431,7 +1524,7 @@ rte_eth_dev_info_get(uint8_t port_id, struct > rte_eth_dev_info *dev_info) > > RTE_FUNC_PTR_OR_RET(*dev->dev_ops->dev_infos_get); > (*dev->dev_ops->dev_infos_get)(dev, dev_info); > - dev_info->pci_dev = dev->pci_dev; > + dev_info->soc_dev = dev->soc_dev; I think both the members, pci_dev and soc_dev, should be updated by this call. Is there some specific reason why soc_dev is the only one which is getting updated? > dev_info->driver_name = dev->data->drv_name; > } > [...] - Shreyansh
[dpdk-dev] [PATCH] vhost: fix missing flag reset on stop
> > Commit 550c9d27d143 ("vhost: set/reset device flags internally") moves > the VIRTIO_DEV_RUNNING set/reset to vhost lib. But I missed one reset > on stop; here fixes it. > > Fixes: 550c9d27d143 ("vhost: set/reset device flags internally") > > Reported-by: Loftus Ciara > Signed-off-by: Yuanhan Liu > --- > lib/librte_vhost/vhost_user/virtio-net-user.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/lib/librte_vhost/vhost_user/virtio-net-user.c > b/lib/librte_vhost/vhost_user/virtio-net-user.c > index a6a48dc..e7c4347 100644 > --- a/lib/librte_vhost/vhost_user/virtio-net-user.c > +++ b/lib/librte_vhost/vhost_user/virtio-net-user.c > @@ -317,8 +317,10 @@ user_get_vring_base(int vid, struct > vhost_vring_state *state) > if (dev == NULL) > return -1; > /* We have to stop the queue (virtio) if it is running. */ > - if (dev->flags & VIRTIO_DEV_RUNNING) > + if (dev->flags & VIRTIO_DEV_RUNNING) { > + dev->flags &= ~VIRTIO_DEV_RUNNING; > notify_ops->destroy_device(vid); > + } > > /* Here we are safe to get the last used index */ > vhost_get_vring_base(vid, state->index, state); > -- > 1.9.0 Thanks for the patch. I've tested it and it solves the issue I was seeing where destroy_device was being called too many times. Tested-by: Ciara Loftus
[dpdk-dev] [PATCH] mempool: rename functions with confusing names
The mempool_count and mempool_free_count behaved contrary to what their names suggested. The free_count function actually returned the number of elements that were allocated from the pool, not the number unallocated as the name implied. Fix this by introducing two new functions to replace the old ones, * rte_mempool_unallocated_count to replace rte_mempool_count * rte_mempool_allocated_count to replace rte_mempool_free_count In this patch, the new functions are added, and the old ones are marked as deprecated. All apps and examples that use the old functions are updated to use the new functions. Fixes: af75078fece3 ("first public release") Signed-off-by: Bruce Richardson --- app/test/test_cryptodev.c | 6 ++--- app/test/test_cryptodev_perf.c | 6 ++--- app/test/test_mbuf.c | 4 +-- app/test/test_mempool.c| 4 +-- app/test/test_mempool_perf.c | 2 +- doc/guides/rel_notes/deprecation.rst | 6 + drivers/net/qede/qede_rxtx.c | 4 +-- examples/multi_process/l2fwd_fork/main.c | 3 ++- examples/qos_sched/main.c | 2 -- lib/librte_mempool/rte_mempool.c | 10 +-- lib/librte_mempool/rte_mempool.h | 42 +++--- lib/librte_mempool/rte_mempool_version.map | 1 + 12 files changed, 69 insertions(+), 21 deletions(-) diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c index 9dfe34f..e5dfa97 100644 --- a/app/test/test_cryptodev.c +++ b/app/test/test_cryptodev.c @@ -316,12 +316,12 @@ testsuite_teardown(void) if (ts_params->mbuf_pool != NULL) { RTE_LOG(DEBUG, USER1, "CRYPTO_MBUFPOOL count %u\n", - rte_mempool_count(ts_params->mbuf_pool)); + rte_mempool_unallocated_count(ts_params->mbuf_pool)); } if (ts_params->op_mpool != NULL) { RTE_LOG(DEBUG, USER1, "CRYPTO_OP_POOL count %u\n", - rte_mempool_count(ts_params->op_mpool)); + rte_mempool_unallocated_count(ts_params->op_mpool)); } } @@ -412,7 +412,7 @@ ut_teardown(void) if (ts_params->mbuf_pool != NULL) RTE_LOG(DEBUG, USER1, "CRYPTO_MBUFPOOL count %u\n", - rte_mempool_count(ts_params->mbuf_pool)); + rte_mempool_unallocated_count(ts_params->mbuf_pool)); rte_cryptodev_stats_get(ts_params->valid_devs[0], &stats); diff --git a/app/test/test_cryptodev_perf.c b/app/test/test_cryptodev_perf.c index e484cbb..351697d 100644 --- a/app/test/test_cryptodev_perf.c +++ b/app/test/test_cryptodev_perf.c @@ -343,10 +343,10 @@ testsuite_teardown(void) if (ts_params->mbuf_mp != NULL) RTE_LOG(DEBUG, USER1, "CRYPTO_PERF_MBUFPOOL count %u\n", - rte_mempool_count(ts_params->mbuf_mp)); + rte_mempool_unallocated_count(ts_params->mbuf_mp)); if (ts_params->op_mpool != NULL) RTE_LOG(DEBUG, USER1, "CRYPTO_PERF_OP POOL count %u\n", - rte_mempool_count(ts_params->op_mpool)); + rte_mempool_unallocated_count(ts_params->op_mpool)); } static int @@ -395,7 +395,7 @@ ut_teardown(void) if (ts_params->mbuf_mp != NULL) RTE_LOG(DEBUG, USER1, "CRYPTO_PERF_MBUFPOOL count %u\n", - rte_mempool_count(ts_params->mbuf_mp)); + rte_mempool_unallocated_count(ts_params->mbuf_mp)); rte_cryptodev_stats_get(ts_params->dev_id, &stats); diff --git a/app/test/test_mbuf.c b/app/test/test_mbuf.c index 1835acc..f2c2006 100644 --- a/app/test/test_mbuf.c +++ b/app/test/test_mbuf.c @@ -717,7 +717,7 @@ test_refcnt_iter(unsigned lcore, unsigned iter) * - increment it's reference up to N+1, * - enqueue it N times into the ring for slave cores to free. */ - for (i = 0, n = rte_mempool_count(refcnt_pool); + for (i = 0, n = rte_mempool_unallocated_count(refcnt_pool); i != n && (m = rte_pktmbuf_alloc(refcnt_pool)) != NULL; i++) { ref = RTE_MAX(rte_rand() % REFCNT_MAX_REF, 1UL); @@ -745,7 +745,7 @@ test_refcnt_iter(unsigned lcore, unsigned iter) /* check that all mbufs are back into mempool by now */ for (wn = 0; wn != REFCNT_MAX_TIMEOUT; wn++) { - if ((i = rte_mempool_count(refcnt_pool)) == n) { + if ((i = rte_mempool_unallocated_count(refcnt_pool)) == n) { refcnt_lcore[lcore] += tref; printf("%s(lcore=%u, iter=%u) completed, " "%u references processed\n", diff --git a/app/test/test_mempool.c b/app/test/test_mempool.c index 31582d8..6ea678a 100644 --- a/app/test/test_mempool.c +++ b/app/test/test_mempool.c @@ -210,7 +210,7 @@ test_mempool_basic(struct rte_mempool *mp) /* tests that improve coverage */ printf("get object count\n"); - if
[dpdk-dev] [PATCH] examples/l2fwd-crypto: out-of-bounds read
Overrunning array crypto_statistics of 32 64-byte elements at element index 63 using index cdevid. Fixed by extend crypto_statistics array. Fixes: 387259bd6c67 ("examples/l2fwd-crypto: add sample application") Coverity ID 120145 Signed-off-by: Slawomir Mrozowicz --- examples/l2fwd-crypto/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/l2fwd-crypto/main.c b/examples/l2fwd-crypto/main.c index 8dc616d..70fada5 100644 --- a/examples/l2fwd-crypto/main.c +++ b/examples/l2fwd-crypto/main.c @@ -243,7 +243,7 @@ struct l2fwd_crypto_statistics { } __rte_cache_aligned; struct l2fwd_port_statistics port_statistics[RTE_MAX_ETHPORTS]; -struct l2fwd_crypto_statistics crypto_statistics[RTE_MAX_ETHPORTS]; +struct l2fwd_crypto_statistics crypto_statistics[RTE_CRYPTO_MAX_DEVS]; /* A tsc-based timer responsible for triggering statistics printout */ #define TIMER_MILLISECOND 200ULL /* around 1ms at 2 Ghz */ -- 1.9.1
[dpdk-dev] [PATCH v5 3/3] mempool: allow for user-owned mempool caches
Hi Lazaros, On 06/29/2016 01:47 AM, Lazaros Koromilas wrote: > The mempool cache is only available to EAL threads as a per-lcore > resource. Change this so that the user can create and provide their own > cache on mempool get and put operations. This works with non-EAL threads > too. This commit introduces the new API calls: > > rte_mempool_cache_create(size, socket_id) > rte_mempool_cache_free(cache) > rte_mempool_cache_flush(cache, mp) > rte_mempool_default_cache(mp, lcore_id) > > Changes the API calls: > > rte_mempool_generic_put(mp, obj_table, n, cache, flags) > rte_mempool_generic_get(mp, obj_table, n, cache, flags) > > The cache-oblivious API calls use the per-lcore default local cache. > > Signed-off-by: Lazaros Koromilas > Acked-by: Olivier Matz > --- > app/test/test_mempool.c | 73 --- > app/test/test_mempool_perf.c| 73 +-- > doc/guides/prog_guide/env_abstraction_layer.rst | 4 +- > doc/guides/prog_guide/mempool_lib.rst | 6 +- > lib/librte_mempool/rte_mempool.c| 66 +- > lib/librte_mempool/rte_mempool.h| 164 > +--- > lib/librte_mempool/rte_mempool_version.map | 4 + > 7 files changed, 308 insertions(+), 82 deletions(-) > Thanks Lazaros for the doc update, looks good to me. Thomas, as discussed IRL, could you please remove the deprecation notice and add the following note in release_16_07.rst when applying the patches? * **Added mempool external cache for non-EAL thread.** Added new functions to create, free or flush a user-owned mempool cache for non-EAL threads. Previously, cache was always disabled on these threads. Thanks, Olivier
[dpdk-dev] [PATCH v15 2/2] i40e: add device args to enable a floating VEB
> -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Zhe Tao > Sent: Wednesday, June 29, 2016 2:06 PM > To: dev at dpdk.org > Cc: Tao, Zhe ; Wu, Jingjing > Subject: [dpdk-dev] [PATCH v15 2/2] i40e: add device args to enable a > floating VEB > > The standard Virtual Ethernet Bridge(VEB) definition in 1Qbg is a bridge > which has an uplink port to the outside world (maybe another bridge), but > a "floating" VEB is a special VEB without an uplink port to the outside. > ... > > Signed-off-by: Zhe Tao Documentation part: Acked-by: John McNamara
[dpdk-dev] [PATCH] ixgbe: fix compilation when offload flags disabled
On 06/29/2016 12:15 PM, Bruce Richardson wrote: > On Mon, Jun 27, 2016 at 07:05:45PM +0200, Thomas Monjalon wrote: >> 2016-06-27 17:44, Olivier Matz: >>> The ixgbe driver does not compile if CONFIG_RTE_IXGBE_RX_OLFLAGS_ENABLE=n >>> because the macro has not the proper number of parameters. >> >> We really need to understand why this build option is needed. >> Target: zero build option. > > Agreed. However, we are where we are right now, and this build option is > there. > > Olivier, can you do a V2 of this patch based on dpdk-next-net/rel_16_07, since > the code in question has actually moved on that branch. Sure, will do. > Out of interest, can you perhaps also give a little more detail on how to > reproduce the issue, as turning off the setting fails to show up a build > problem for me with either gcc or clang, at -O3 or -O0. That's strange, I just set CONFIG_RTE_IXGBE_RX_OLFLAGS_ENABLE=n to reproduce the issue. I'll check on dpdk-next-net/rel_16_07 and add more details, like the output of the compiler. Thanks, Olivier
[dpdk-dev] [PATCH] scripts: check for commit msgs starting with 'It'
On Wed, Jun 29, 2016 at 01:23:48PM +0200, Thomas Monjalon wrote: > > Signed-off-by: Bruce Richardson > > Applied with -i case option to catch also lowercase form. Which suggests another good check to add, would be to check that the commmit message starts with a capital letter. /Bruce
[dpdk-dev] [PATCH v2 0/6] ena: update PMD to cooperate with latest ENA firmware
Bruce, We've got a small minor fixes and we will create V3 version later this week (targeting Friday). What's final deadline for 16.07 integration? Regards, Jan 2016-06-29 13:01 GMT+02:00 Bruce Richardson : > On Fri, Jun 24, 2016 at 12:52:39PM +0100, Bruce Richardson wrote: > > On Tue, Jun 21, 2016 at 02:05:57PM +0200, Jan Medala wrote: > > > As requested, big patch splitted into logical pieces for easier review. > > > Improved style and fixed icc compiler issues. > > > > > > > Thanks for the patch split. However, many of the patches don't have a > commit > > message describing (at a high level), what the goal of that patch is and > how > > it goes about implementing the changes to achieve that goal. Can you add > a > > paragraph or two of commit message to each patch on the set to help > anyone > > looking at the code understand what is happening in each patch. > > > > Regards, > > /Bruce > > > > Any update on this set? Is a V3 coming for 16.07 timeframe? > > /Bruce > >
[dpdk-dev] [PATCH] ixgbe: fix compilation when offload flags disabled
On Wed, Jun 29, 2016 at 01:35:15PM +0200, Olivier MATZ wrote: > > > On 06/29/2016 12:15 PM, Bruce Richardson wrote: > >On Mon, Jun 27, 2016 at 07:05:45PM +0200, Thomas Monjalon wrote: > >>2016-06-27 17:44, Olivier Matz: > >>>The ixgbe driver does not compile if CONFIG_RTE_IXGBE_RX_OLFLAGS_ENABLE=n > >>>because the macro has not the proper number of parameters. > >> > >>We really need to understand why this build option is needed. > >>Target: zero build option. > > > >Agreed. However, we are where we are right now, and this build option is > >there. > > > >Olivier, can you do a V2 of this patch based on dpdk-next-net/rel_16_07, > >since > >the code in question has actually moved on that branch. > > Sure, will do. > Thanks. > > >Out of interest, can you perhaps also give a little more detail on how to > >reproduce the issue, as turning off the setting fails to show up a build > >problem for me with either gcc or clang, at -O3 or -O0. > > That's strange, I just set CONFIG_RTE_IXGBE_RX_OLFLAGS_ENABLE=n to > reproduce the issue. I'll check on dpdk-next-net/rel_16_07 and add more > details, like the output of the compiler. > Ok, no problem either way since inspection of the code shows there is a bug there anyway. I'm just curious as to why I couldn't reproduce it. /Bruce
[dpdk-dev] [PATCH v2 0/6] ena: update PMD to cooperate with latest ENA firmware
On Wed, Jun 29, 2016 at 01:31:12PM +0200, Jan M?dala wrote: > Bruce, > > We've got a small minor fixes and we will create V3 version later this week > (targeting Friday). > What's final deadline for 16.07 integration? > > Regards, > > Jan > ASAP. The integration deadline for final patches to be ready has already passed, but as RC1 has not been produced yet, you have a little time, but not much. Even if you get it out this week, the patchset will likely only make RC2. /Bruce > 2016-06-29 13:01 GMT+02:00 Bruce Richardson : > > > On Fri, Jun 24, 2016 at 12:52:39PM +0100, Bruce Richardson wrote: > > > On Tue, Jun 21, 2016 at 02:05:57PM +0200, Jan Medala wrote: > > > > As requested, big patch splitted into logical pieces for easier review. > > > > Improved style and fixed icc compiler issues. > > > > > > > > > > Thanks for the patch split. However, many of the patches don't have a > > commit > > > message describing (at a high level), what the goal of that patch is and > > how > > > it goes about implementing the changes to achieve that goal. Can you add > > a > > > paragraph or two of commit message to each patch on the set to help > > anyone > > > looking at the code understand what is happening in each patch. > > > > > > Regards, > > > /Bruce > > > > > > > Any update on this set? Is a V3 coming for 16.07 timeframe? > > > > /Bruce > > > >
[dpdk-dev] [PATCH] scripts: enable qede in build test
2016-06-29 11:30, Thomas Monjalon: > The driver qede can be automatically enabled if libz is available. > > Fixes: ec94dbc57362 ("qede: add base driver") > > Signed-off-by: Thomas Monjalon Applied
[dpdk-dev] weak functions in some drivers
>> What is not clear to me is motivation to use weak here instead of simply >> using >CONFIG_RTE_I40E_INC_VECTOR >> macro to exclude stubs in i40e_rxtx.c. It will make library smaller and >> avoid issues like this one >> which are quite hard to troubleshoot. > >Since this issue seen in fd.io, I didn't investigated more, but I don't >want to clock your valid question, this is an attempt to resurrect the >question ... Hi, We are having exactly the same problem. For us the aforementioned workaround doesn't seem to work and vector mode is always disabled with the i40e drivers. If I modify i40e_rxtx.c and exclude the stub functions using CONFIG_RTE_I40E_INC_VECTOR everything works as expected. We are building DPDK with the CONFIG_RTE_BUILD_COMBINE_LIBS option enabled and link DPDK library to our application. Any other ideas how this could be fixed? Regards, Matias
[dpdk-dev] [PATCH 2/4] virtio: introduce RTE_LIBRTE_VIRTIO_INC_VECTOR
2016-06-29 16:48, Jerin Jacob: > On Mon, Jun 27, 2016 at 04:59:42PM +0200, Thomas Monjalon wrote: > > 2016-06-27 20:18, Jerin Jacob: > > > On Mon, Jun 27, 2016 at 04:19:57PM +0200, Thomas Monjalon wrote: > > > > 2016-06-27 17:24, Jerin Jacob: > > > > > --- a/config/common_base > > > > > +++ b/config/common_base > > > > > @@ -267,6 +267,7 @@ CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_RX=n > > > > > CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_TX=n > > > > > CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_DRIVER=n > > > > > CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_DUMP=n > > > > > +CONFIG_RTE_LIBRTE_VIRTIO_INC_VECTOR=y > > > > > > > > I don't remember what means INC_VECTOR? > > > > Why a config option is needed for vector implementations? > > > > > > I thought of adding additional configuration option(INC_VECTOR) _apart_ > > > from > > > cpu flag based scheme in the patch because even though if a given platform > > > has cpu instruction support, in some platforms scalar version may > > > perform well wrt vector version(based on instruction latency, emulation > > > required or not > > > etc). So a top level flag INC_VECTOR, can override the vector selection > > > for a given platform if required. > > > > Isn't it a runtime driver option needed to disable vector virtio? > > > > > Regarding INC_VECTOR(INC in vector configuration name, I have no idea, I > > > followed the existing flags) > > > $ grep "INC_VECTOR" config/common_base > > > CONFIG_RTE_IXGBE_INC_VECTOR=y > > > CONFIG_RTE_LIBRTE_I40E_INC_VECTOR=y > > > CONFIG_RTE_LIBRTE_FM10K_INC_VECTOR=y > > > > If the flag is really needed I would suggest VIRTIO_VECTOR. > > OK I will change to VIRTIO_VECTOR I would prefer a runtime option.
[dpdk-dev] [PATCH] scripts: check for commit msgs starting with 'It'
> Signed-off-by: Bruce Richardson Applied with -i case option to catch also lowercase form.
[dpdk-dev] [PATCHv8 5/6] pmdinfo.py: Add tool to query binaries for hw and other support information
On Wed, Jun 29, 2016 at 04:12:21PM +0100, Remy Horton wrote: > 'noon, > > The tool does not work for static PMD libraries (e.g. librte_pmd_i40e.a) - > is this an intended limitation? > Yes, because .a files are archives, not ELF files. If you want to run pmdinfo on an archive, you have to unarchive it first (ar x). It would be the same if you had bound them together with tar or zip. > DPDK doesn't to my knowledge have any coding guidelines for Python, so the > comments below should be considered advisory rather than merge-blocking > issues. > > > On 17/06/2016 19:46, Neil Horman wrote: > [..] > > +++ b/tools/pmdinfo.py > > @@ -0,0 +1,629 @@ > > +#!/usr/bin/python > > +# - > > +# scripts/pmdinfo.py > > +# > > +# Utility to dump PMD_INFO_STRING support from an object file > > +# > > No licence..? > Its BSD, same as the others, I let the README cover that. We can include it if we must, though we have lots of examples where we haven't bothered > > > +# - > > +import os > > +import sys > > +from optparse import OptionParser > > +import string > > +import json > > + > > +# For running from development directory. It should take precedence over > > the > > +# installed pyelftools. > > +sys.path.insert(0, '.') > > Aside from causing all the subsequent imports to have PEP8 errors, this does > not looks like a good way of pulling in project-specific Python library > installs. Usual method is either using virtualenv or the PYTHONPATH > enviornment variable. > Nope, pep8 doesn't complain about this at all: [nhorman at hmsreliant tools]$ pep8 ./pmdinfo.py pmdinfo.py:573:80: E501 line too long (80 > 79 characters) [nhorman at hmsreliant tools]$ The line too long snuck in there recently on the last round of formatting errors > > > +from elftools import __version__ > > +from elftools.common.exceptions import ELFError > [..] > > +from elftools.dwarf.constants import ( > > +DW_LNS_copy, DW_LNS_set_file, DW_LNE_define_file) > > +from elftools.dwarf.callframe import CIE, FDE > > According to PyLint, most of these imports are unused. Some of them are, the code was borrowed from some examples. > > > > + > > + > > +class Vendor: > > Old style class definition. Using modern notation it is: > > class Vendor(object): > > > > +def report(self): > > +print "\t%s\t%s" % (self.ID, self.name) > > +for subID, subdev in self.subdevices.items(): > > +subdev.report() > > subID unused. An underscore can be used as a placeholder for these types of > loops. > > > > +optparser.add_option("-t", "--table", dest="tblout", > > + help="output information on hw support as a hex > > table", > > PEP8: pmdinfo.py:573:80: E501 line too long (80 > 79 characters) > As you note, none of these are catastrophic. I'm willing to fix them up, but given, the number of iterations I've gone through for minior nits, I would prefer to see it incorporated before I post this series again. Neil > >
[dpdk-dev] [PATCH v2 0/6] ena: update PMD to cooperate with latest ENA firmware
On Fri, Jun 24, 2016 at 12:52:39PM +0100, Bruce Richardson wrote: > On Tue, Jun 21, 2016 at 02:05:57PM +0200, Jan Medala wrote: > > As requested, big patch splitted into logical pieces for easier review. > > Improved style and fixed icc compiler issues. > > > > Thanks for the patch split. However, many of the patches don't have a commit > message describing (at a high level), what the goal of that patch is and how > it goes about implementing the changes to achieve that goal. Can you add a > paragraph or two of commit message to each patch on the set to help anyone > looking at the code understand what is happening in each patch. > > Regards, > /Bruce > Any update on this set? Is a V3 coming for 16.07 timeframe? /Bruce
[dpdk-dev] [PATCH] bnx2x: Call bnx2x_init_rte() later
On Tue, Jun 28, 2016 at 12:14:50PM -0400, Chas Williams wrote: > We need sc->igu_sb_cnt determined before calculating the number of queues > we can support. > > Fixes: a787538ee754 ("bnx2x: fix MSIX vector and VF resource counts") > > Signed-off-by: Chas Williams <3chas3 at gmail.com> > --- Hi Chas, we generally try to avoid making the commit titles too technical, so we avoid using function or variable names in the titles. Since this is a bug fix the best form of title to be used is "fix ", e.g. "fix incorrect number of supported queues". Maintainers, any comments or ack for this patch. /Bruce
[dpdk-dev] [PATCH] bnx2x: set random MAC address if one isn't assigned
On Tue, Jun 21, 2016 at 08:18:46AM -0400, Chas Williams wrote: > If the PF hasn't assigned an address, assign one randomly. While here, > convert to use DPDK's ether address utility routines. > > Fixes: 540a211084a7 ("bnx2x: driver core") > > Signed-off-by: Chas Williams <3chas3 at gmail.com> Maintainers, any comment or ack on this patch? /Bruce
[dpdk-dev] [PATCH v2] enic: fix issues when using Rx scatter with multiple RQs
On Tue, Jun 28, 2016 at 11:49:11AM -0700, Nelson Escobar wrote: > The Rx scatter patch failed to make a few changes and resulted in > problems when using multiple receive queues in dpdk (ie RSS) since the > wrong adapter resources were being used. > > - get and use the correct completion queue index associated with a > receive queue. > - set the correct receive queue index when using RSS > > Fixes: 14a261bf0520 ("enic: add scattered Rx support") > > Signed-off-by: Nelson Escobar > Reviewed-by: John Daley Applied to dpdk-next-net/rel_16_07 /Bruce
[dpdk-dev] [PATCH 2/2] net/thunderx: fix unused function nicvf_mbox_msg_str error
On Wed, Jun 29, 2016 at 11:39:51AM +0100, Bruce Richardson wrote: > On Tue, Jun 28, 2016 at 09:56:02AM +0100, Ferruh Yigit wrote: > > On 6/27/2016 5:16 PM, Jerin Jacob wrote: > > > Fix the following error with clang > > > error: unused function 'nicvf_mbox_msg_str' > > > > > > The nicvf_mbox_msg_str() maybe unused, based on DEBUG > > > compilation option selected. Mark __attribute__((unused)) on > > > this function, to inform compiler that this function may be unused > > > > > > Fixes: e93592f1 ("net/thunderx/base: add mbox APIs for PF/VF > > > communication") > > > > > > Signed-off-by: Jerin Jacob > > > Reported-by: Ferruh Yigit > > > > ... > > > > > -static inline const char * > > > +static inline const char * __attribute__((unused)) > > it is possible to use __rte_unused macro here, although not required, > > and moving it before return type can prevent checkpatch error > > > > > > > nicvf_mbox_msg_str(int msg) > > > { > > > assert(msg >= 0 && msg < NIC_MBOX_MSG_MAX); > > > > > > > Checkpatch gives following error: > > ERROR:POINTER_LOCATION: "foo * bar" should be "foo *bar" > > #39: FILE: drivers/net/thunderx/base/nicvf_mbox.c:67: > > +static inline const char * __attribute__((unused)) > > > > > > check-git-log gives following errors: > > Wrong headline format: > > net/thunderx: fix unused function nicvf_mbox_msg_str error > > Thanks for flagging these Ferruh. I'll fix them on apply, no need for V2, > unless > you particularly want to do one, Jerin. :-) > > /Bruce Applied to dpdk-next-net/rel_16_07 with the check-git-log.sh issue fixed. Checkpatch issue is ok to ignore. /Bruce
[dpdk-dev] [PATCH 1/2] net/thunderx: select -fno-prefetch-loop-arrays only for gcc
On Wed, Jun 29, 2016 at 11:39:07AM +0100, Bruce Richardson wrote: > On Tue, Jun 28, 2016 at 09:55:55AM +0100, Ferruh Yigit wrote: > > On 6/27/2016 5:16 PM, Jerin Jacob wrote: > > > 'fno-prefetch-loop-arrays' optimization flag is not supported with clang > > > > > > Fixes: dcc49041 (net/thunderx: add single and multi-segment Tx functions") > > > > > > Signed-off-by: Jerin Jacob > > > Reported-by: Ferruh Yigit > > > > check-git-log gives following errors: > > > > Wrong 'Fixes' reference: > > Fixes: dcc49041 (net/thunderx: add single and multi-segment Tx > > functions") > > > > This looks like because of missing " in the beginning > > Yes, but I can fix that on apply. > > Other very minor nit is that the standard for fixes lines is to use a 12-char > commit id, rather than 8. Per contributors guide: >git config alias.fixline "log -1 --abbrev=12 --format='Fixes: %h (\"%s\")'" > > /Bruce Applied to dpdk-next-net/rel_16_07 /Bruce
[dpdk-dev] [PATCH 2/2] net/thunderx: fix unused function nicvf_mbox_msg_str error
On Tue, Jun 28, 2016 at 09:56:02AM +0100, Ferruh Yigit wrote: > On 6/27/2016 5:16 PM, Jerin Jacob wrote: > > Fix the following error with clang > > error: unused function 'nicvf_mbox_msg_str' > > > > The nicvf_mbox_msg_str() maybe unused, based on DEBUG > > compilation option selected. Mark __attribute__((unused)) on > > this function, to inform compiler that this function may be unused > > > > Fixes: e93592f1 ("net/thunderx/base: add mbox APIs for PF/VF communication") > > > > Signed-off-by: Jerin Jacob > > Reported-by: Ferruh Yigit > > ... > > > -static inline const char * > > +static inline const char * __attribute__((unused)) > it is possible to use __rte_unused macro here, although not required, > and moving it before return type can prevent checkpatch error > > > > nicvf_mbox_msg_str(int msg) > > { > > assert(msg >= 0 && msg < NIC_MBOX_MSG_MAX); > > > > Checkpatch gives following error: > ERROR:POINTER_LOCATION: "foo * bar" should be "foo *bar" > #39: FILE: drivers/net/thunderx/base/nicvf_mbox.c:67: > +static inline const char * __attribute__((unused)) > > > check-git-log gives following errors: > Wrong headline format: > net/thunderx: fix unused function nicvf_mbox_msg_str error Thanks for flagging these Ferruh. I'll fix them on apply, no need for V2, unless you particularly want to do one, Jerin. :-) /Bruce
[dpdk-dev] [PATCH 1/2] net/thunderx: select -fno-prefetch-loop-arrays only for gcc
On Tue, Jun 28, 2016 at 09:55:55AM +0100, Ferruh Yigit wrote: > On 6/27/2016 5:16 PM, Jerin Jacob wrote: > > 'fno-prefetch-loop-arrays' optimization flag is not supported with clang > > > > Fixes: dcc49041 (net/thunderx: add single and multi-segment Tx functions") > > > > Signed-off-by: Jerin Jacob > > Reported-by: Ferruh Yigit > > check-git-log gives following errors: > > Wrong 'Fixes' reference: > Fixes: dcc49041 (net/thunderx: add single and multi-segment Tx > functions") > > This looks like because of missing " in the beginning Yes, but I can fix that on apply. Other very minor nit is that the standard for fixes lines is to use a 12-char commit id, rather than 8. Per contributors guide: git config alias.fixline "log -1 --abbrev=12 --format='Fixes: %h (\"%s\")'" /Bruce
[dpdk-dev] [PATCH] igb: support setting link up or down
On Tue, Jun 28, 2016 at 01:10:58AM +, Lu, Wenzhuo wrote: > Hi, > > > -Original Message- > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Aaron Campbell > > Sent: Monday, June 27, 2016 11:46 PM > > To: dev at dpdk.org > > Cc: Aaron Campbell > > Subject: [dpdk-dev] [PATCH] igb: support setting link up or down > > > > Add driver functions to set link state up or down. > > > > Signed-off-by: Aaron Campbell > Acked-by: Wenzhuo Lu > Applied to dpdk-next-net/rel_16_07 /Bruce
[dpdk-dev] [PATCH] scripts: enable qede in build test
The driver qede can be automatically enabled if libz is available. Fixes: ec94dbc57362 ("qede: add base driver") Signed-off-by: Thomas Monjalon --- scripts/test-build.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scripts/test-build.sh b/scripts/test-build.sh index 7a9f623..24c0202 100755 --- a/scripts/test-build.sh +++ b/scripts/test-build.sh @@ -162,6 +162,8 @@ config () # sed -ri 's,(PMD_SZEDATA2=)n,\1y,' $1/.config test "$DPDK_DEP_ZLIB" != y || \ sed -ri 's,(BNX2X_PMD=)n,\1y,' $1/.config + test "$DPDK_DEP_ZLIB" != y || \ + sed -ri 's,(QEDE_PMD=)n,\1y,' $1/.config sed -ri's,(NFP_PMD=)n,\1y,' $1/.config test "$DPDK_DEP_PCAP" != y || \ sed -ri 's,(PCAP=)n,\1y,' $1/.config -- 2.7.0
[dpdk-dev] [PATCH v2] doc: virtio pmd versions
On Tue, Jun 14, 2016 at 07:53:17PM -0400, Zhihong Wang wrote: > This patch explains all the versions of current virtio pmd implementation, > what's the difference, and how to choose the right version. > > -- > Changes in v2: > >1. Changes on format and few descriptions. Change log should go ... > > > Signed-off-by: Zhihong Wang > --- here.. > doc/guides/nics/virtio.rst | 64 > +- > 1 file changed, 58 insertions(+), 6 deletions(-) > > diff --git a/doc/guides/nics/virtio.rst b/doc/guides/nics/virtio.rst > index 06ca433..a4fef89 100644 > --- a/doc/guides/nics/virtio.rst > +++ b/doc/guides/nics/virtio.rst > @@ -73,7 +73,7 @@ In this release, the virtio PMD driver provides the basic > functionality of packe > > * It supports multicast packets and promiscuous mode. > > -* The descriptor number for the RX/TX queue is hard-coded to be 256 by > qemu. > +* The descriptor number for the Rx/Tx queue is hard-coded to be 256 by > qemu. > If given a different descriptor number by the upper application, > the virtio PMD generates a warning and fall back to the hard-coded value. > > @@ -163,8 +163,8 @@ Host2VM communication example > which means received packets come from vEth0, and transmitted packets is > sent to vEth0. > > #. In the guest, bind the virtio device to the uio_pci_generic kernel > module and start the forwarding application. > -When the virtio port in guest bursts rx, it is getting packets from the > raw socket's receive queue. > -When the virtio port bursts tx, it is sending packet to the tx_q. > +When the virtio port in guest bursts Rx, it is getting packets from the > raw socket's receive queue. > +When the virtio port bursts Tx, it is sending packet to the tx_q. > > .. code-block:: console > > @@ -183,7 +183,7 @@ Host2VM communication example > > The packet reception and transmission flow path is: > > -IXIA packet generator->82599 PF->KNI rx queue->KNI raw socket > queue->Guest VM virtio port 0 rx burst->Guest VM virtio port 0 tx burst-> KNI > tx queue->82599 PF-> IXIA packet generator > +IXIA packet generator->82599 PF->KNI Rx queue->KNI raw socket > queue->Guest VM virtio port 0 Rx burst->Guest VM virtio port 0 Tx burst-> KNI > Tx queue->82599 PF-> IXIA packet generator > > Virtio with qemu virtio Back End > > @@ -206,8 +206,60 @@ Virtio with qemu virtio Back End > > In this example, the packet reception flow path is: > > -IXIA packet generator->82599 PF->Linux Bridge->TAP0's socket queue-> > Guest VM virtio port 0 rx burst-> Guest VM 82599 VF port1 tx burst-> IXIA > packet generator > +IXIA packet generator->82599 PF->Linux Bridge->TAP0's socket queue-> > Guest VM virtio port 0 Rx burst-> Guest VM 82599 VF port1 Tx burst-> IXIA > packet generator > > The packet transmission flow is: > > -IXIA packet generator-> Guest VM 82599 VF port1 rx burst-> Guest VM > virtio port 0 tx burst-> tap -> Linux Bridge->82599 PF-> IXIA packet generator > +IXIA packet generator-> Guest VM 82599 VF port1 Rx burst-> Guest VM > virtio port 0 Tx burst-> tap -> Linux Bridge->82599 PF-> IXIA packet generator > + > + > +Virtio PMD Versions > +--- Using "versions" here is a bit confusing to me, especially virtio PMD supports spec version 0.95 and 1.0. Apparently, that's not what you are talking about, virtio pmd Rx/Tx callbacks is. So, something like "Virtio PMD Rx/Tx callbacks" is what I would suggest. > + > +Virtio driver has 3 versions of Rx functions and 2 versions of Tx functions. And I will avoid using "versions". > + > +Rx functions: > + > +#. ``virtio_recv_pkts``: > + Regular version without mergeable Rx buffer support. > + > +#. ``virtio_recv_mergeable_pkts``: > + Regular version with mergeable Rx buffer support. > + > +#. ``virtio_recv_pkts_vec``: > + Simple version without mergeable Rx buffer support, also fixes the > available ring indexes and uses vector instructions to optimize performance. Just like coding and writing commit log, don't write long lines over 80 chars here. Also I will use "vector" instead of "simple" here, as "vector" is more easier to understand. > + > +Tx functions: > + > +#. ``virtio_xmit_pkts``: > + Regular version. > + > +#. ``virtio_xmit_pkts_simple``: > + Simple version fixes the available ring indexes to optimize performance. > + > + > +By default, the non-vector versions are used: > + > +* For Rx: If mergeable Rx buffers is disabled then ``virtio_recv_pkts`` is > used; otherwise ``virtio_recv_mergeable_pkts``. > + > +* For Tx: ``virtio_xmit_pkts``. > + > + > +Setting ``txq_flags`` to ``VIRTIO_SIMPLE_FLAGS`` (0xF01) enables the simple > version of the virtio poll mode driver: > + > +* For Rx: ``virtio_recv_pkts_vec``. > + > +* For Tx: ``virtio_xmit_pkts_simple``. > + This paragraph says that vector Rx/Tx will be enabled whe
[dpdk-dev] [PATCH] ixgbe: fix compilation when offload flags disabled
On Mon, Jun 27, 2016 at 07:05:45PM +0200, Thomas Monjalon wrote: > 2016-06-27 17:44, Olivier Matz: > > The ixgbe driver does not compile if CONFIG_RTE_IXGBE_RX_OLFLAGS_ENABLE=n > > because the macro has not the proper number of parameters. > > We really need to understand why this build option is needed. > Target: zero build option. Agreed. However, we are where we are right now, and this build option is there. Olivier, can you do a V2 of this patch based on dpdk-next-net/rel_16_07, since the code in question has actually moved on that branch. Out of interest, can you perhaps also give a little more detail on how to reproduce the issue, as turning off the setting fails to show up a build problem for me with either gcc or clang, at -O3 or -O0. /Bruce
[dpdk-dev] [PATCH v2 1/2] enic: determine max egress packet size and max MTU
On Fri, Jun 24, 2016 at 03:29:27PM -0700, John Daley wrote: > Pull in common VNIC code which enables querying for max egress > packet size with newer firmware via a device command. If the > field is non-zero, it is the max egress packet size. If it is > 0, the default value (9022) can safely be assumed. The value > for 1300 series VICS using firmware versions >= 3.1.2 for blade > series and >= 2.0.13 for rack series servers is 9208. > > Tx buffers can be emitted only if they are less than the max egress > packet size regardless of the MTU setting (the MTU is advisory). > The max egress packet size can used to determine the upper limit > of the MTU since the enic can also receive packets of size greater > than max egress packet size. A max_mtu variable is added with > a value of max egress packet size minus L2 header size. > > The default MTU is set via the CIMC/UCSM management interface and > currently allows value up to 9000. If the value is changed, the > host must be reboot. To avoid the reboot and allow MTU values > up to the max capability of the NIC, MTU update capability will > be added with a max value capped by max_mtu. > > Signed-off-by: John Daley > --- > v2: Squished patch 1/4 and 2/4 into one. Tried to do a little > better explanaiton of the intent of the patch in the commit > message. > Patchset applied to dpdk-next-net/rel_16_07 /Bruce
[dpdk-dev] [PATCH v2] bnx2x: fix icc compilation error
On Mon, Jun 27, 2016 at 11:21:45PM +, Rasesh Mody wrote: > > From: Ferruh Yigit [mailto:ferruh.yigit at intel.com] > > Sent: Monday, June 27, 2016 9:03 AM > > > > Fix icc warning: > > error #188: enumerated type mixed with another type > > > > Fixed the code for simple cases caused the warning. > > Still need to disable compiler warning because of more complex case. > > > > Signed-off-by: Ferruh Yigit > > Acked-by: Rasesh Mody > > Thanks Ferruh! Applied to dpdk-next-net/rel_16_07 /Bruce
[dpdk-dev] [PATCH v14 0/2] i40e: add floating VEB support for i40e
On Mon, Jun 27, 2016 at 03:20:14PM +0800, Zhe Tao wrote: > This patch-set add the support for floating VEB in i40e. > All the VFs VSIs can decide whether to connect to the legacy VEB/VEPA or > the floating VEB. When connect to the floating VEB a new floating VEB is > created. Now all the VFs need to connect to floating VEB or legacy VEB, > cannot connect to both of them. The PF and VMDQ,FD VSIs connect to > the old legacy VEB/VEPA. > > All the VEB/VEPA concepts are not specific for FVL, they are defined in the > 802.1Qbg spec. > > This floating VEB only take effects on the specific version F/W which newer > than 5.0. > > Zhe Tao (2): > i40e: support floating VEB config > i40e: add floating VEB support in i40e > I think the order of the patches would be better reversed, with the feature coming first and the config after. The documentation would then move to the new second patch on config, which is where it better fits, since a lot of it describes how to configure the feature. Regards, /Bruce
[dpdk-dev] [PATCHv8 0/6] Implement pmd hardware support exports
Morning, Compile error with virtio_user_ethdev.c - looks like a new addition (commit ce2eabdd43ec) that also needs to be converted. Regards, ..Remy On 17/06/2016 19:46, Neil Horman wrote: > Hey all- > So heres attempt number 2 at a method for exporting PMD hardware support > information. As we discussed previously, the consensus seems to be that pmd > information should be: [..] > Signed-off-by: Neil Horman > Acked-by: Panu Matilainen > CC: Bruce Richardson > CC: Thomas Monjalon > CC: Stephen Hemminger > CC: Panu Matilainen
[dpdk-dev] [PATCH] net/virtio-user: fix O_CLOEXEC undeclared error
On Tue, Jun 28, 2016 at 10:41:24AM +0100, Ferruh Yigit wrote: > On 6/26/2016 2:49 PM, Jianfeng Tan wrote: > > On some older systems, such as SUSE 11, the compiling error shows > > as: > >.../dpdk/drivers/net/virtio/virtio_user/virtio_user_dev.c:67:22: > > error: ?O_CLOEXEC? undeclared (first use in this function) > > > > The fix is to declare _GNU_SOURCE macro before include fcntl.h. > > > > Fixes: 37a7eb2ae816 ("net/virtio-user: add device emulation layer") > > > > Signed-off-by: Jianfeng Tan > > --- > > drivers/net/virtio/virtio_user/virtio_user_dev.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c > > b/drivers/net/virtio/virtio_user/virtio_user_dev.c > > index 3d12a32..180f824 100644 > > --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c > > +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c > > @@ -31,6 +31,7 @@ > > * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. > > */ > > > > +#define _GNU_SOURCE > This flag enables glibc extensions, and kind of problem on code > portability. How big problem is it? Honestly, I don't know. But I was also thinking that define _GNU_SOURCE here is an overkill to me. For this issue, we may simply fix it by reference the O_CLOEXEC flag only when it's defined. > Is there any platform that DPDK runs on which uses a libc that isn't > compatible with what this flag provides? > > And it seems this flag already used in DPDK, including libraries. Yes, "git grep _GNU_SOURCE" shows a lot of such usage. --yliu > If we > are agree on using glibc extensions, does it make sense to move this > flag into a single common location (like mk/exec-env/) ? > > > #include > > #include > > #include > >
[dpdk-dev] [PATCH 4/4] net/virtio-user: fix string unterminated
When use strcpy() to copy string with length exceeding the last parameter of strcpy(), it may lead to the destination string unterminated. We replaced strncpy with snprintf to make sure it's NULL terminated. Coverity issue: 127476 Fixes: ce2eabdd43ec ("net/virtio-user: add virtual device") Signed-off-by: Jianfeng Tan --- drivers/net/virtio/virtio_user/virtio_user_dev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c index 1b1e5bf..376c9cf 100644 --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c @@ -181,7 +181,7 @@ int virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues, int cq, int queue_size, const char *mac) { - strncpy(dev->path, path, PATH_MAX); + snprintf(dev->path, PATH_MAX, "%s", path); dev->max_queue_pairs = queues; dev->queue_pairs = 1; /* mq disabled by default */ dev->queue_size = queue_size; -- 2.1.4
[dpdk-dev] [PATCH 3/4] net/virtio-user: fix resource leaks
The return value by rte_kvargs_parse is not free(d), which leads to memory leak. Coverity issue: 127482 Fixes: ce2eabdd43ec ("net/virtio-user: add virtual device") Signed-off-by: Jianfeng Tan --- drivers/net/virtio/virtio_user_ethdev.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c index 8429b2e..8e39adf 100644 --- a/drivers/net/virtio/virtio_user_ethdev.c +++ b/drivers/net/virtio/virtio_user_ethdev.c @@ -320,7 +320,7 @@ virtio_user_eth_dev_alloc(const char *name) static int virtio_user_pmd_devinit(const char *name, const char *params) { - struct rte_kvargs *kvlist; + struct rte_kvargs *kvlist = NULL; struct rte_eth_dev *eth_dev; struct virtio_hw *hw; uint64_t queues = VIRTIO_USER_DEF_Q_NUM; @@ -422,6 +422,8 @@ virtio_user_pmd_devinit(const char *name, const char *params) ret = 0; end: + if (kvlist) + rte_kvargs_free(kvlist); if (path) free(path); if (mac_addr) -- 2.1.4
[dpdk-dev] [PATCH 2/4] net/virtio-user: fix string overflow
When parsing /proc/self/maps to get hugepage information, the string was being copied with strcpy(), which could, theoretically but in fact not possiblly, overflow the destination buffer. Anyway, to avoid the false alarm, we replaced strncpy with snprintf for safely copying the strings. Coverity issue: 127484 Fixes: 6a84c37e3975 ("net/virtio-user: add vhost-user adapter layer") Signed-off-by: Jianfeng Tan --- drivers/net/virtio/virtio_user/vhost_user.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/virtio/virtio_user/vhost_user.c b/drivers/net/virtio/virtio_user/vhost_user.c index a159ece..082e821 100644 --- a/drivers/net/virtio/virtio_user/vhost_user.c +++ b/drivers/net/virtio/virtio_user/vhost_user.c @@ -181,7 +181,7 @@ get_hugepage_file_info(struct hugepage_file_info huges[], int max) } huges[idx].addr = v_start; huges[idx].size = v_end - v_start; - strcpy(huges[idx].path, tmp); + snprintf(huges[idx].path, PATH_MAX, "%s", tmp); idx++; } -- 2.1.4
[dpdk-dev] [PATCH 1/4] net/virtio-user: fix return value not checked
When return values of function calls are not checked, Coverity will report errors like: if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_PATH) == 1) >>> CID 127477:(CHECKED_RETURN) >>> Calling "rte_kvargs_process" without checking return value (as is done elsewhere 25 out of 30 times). rte_kvargs_process(kvlist, VIRTIO_USER_ARG_PATH, &get_string_arg, &path); Coverity issue: 127344, 127478 Fixes: ce2eabdd43ec ("net/virtio-user: add virtual device") Fixes: 6a84c37e3975 ("net/virtio-user: add vhost-user adapter layer") Signed-off-by: Jianfeng Tan --- drivers/net/virtio/virtio_user/vhost_user.c | 3 +- drivers/net/virtio/virtio_user_ethdev.c | 57 ++--- 2 files changed, 45 insertions(+), 15 deletions(-) diff --git a/drivers/net/virtio/virtio_user/vhost_user.c b/drivers/net/virtio/virtio_user/vhost_user.c index a2b0687..a159ece 100644 --- a/drivers/net/virtio/virtio_user/vhost_user.c +++ b/drivers/net/virtio/virtio_user/vhost_user.c @@ -392,7 +392,8 @@ vhost_user_setup(const char *path) } flag = fcntl(fd, F_GETFD); - fcntl(fd, F_SETFD, flag | FD_CLOEXEC); + if (fcntl(fd, F_SETFD, flag | FD_CLOEXEC) < 0) + PMD_DRV_LOG(WARNING, "fcntl failed, %s", strerror(errno)); memset(&un, 0, sizeof(un)); un.sun_family = AF_UNIX; diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c index 5ab2471..8429b2e 100644 --- a/drivers/net/virtio/virtio_user_ethdev.c +++ b/drivers/net/virtio/virtio_user_ethdev.c @@ -343,29 +343,58 @@ virtio_user_pmd_devinit(const char *name, const char *params) } if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_PATH) == 1) - rte_kvargs_process(kvlist, VIRTIO_USER_ARG_PATH, - &get_string_arg, &path); + ret = rte_kvargs_process(kvlist, VIRTIO_USER_ARG_PATH, +&get_string_arg, &path); + if (ret < 0) { + PMD_INIT_LOG(ERR, "error to parse %s", +VIRTIO_USER_ARG_PATH); + goto end; + } else { PMD_INIT_LOG(ERR, "arg %s is mandatory for virtio-user\n", VIRTIO_USER_ARG_QUEUE_SIZE); goto end; } - if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_MAC) == 1) - rte_kvargs_process(kvlist, VIRTIO_USER_ARG_MAC, - &get_string_arg, &mac_addr); + if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_MAC) == 1) { + ret = rte_kvargs_process(kvlist, VIRTIO_USER_ARG_MAC, +&get_string_arg, &mac_addr); + if (ret < 0) { + PMD_INIT_LOG(ERR, "error to parse %s", +VIRTIO_USER_ARG_MAC); + goto end; + } + } - if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_QUEUE_SIZE) == 1) - rte_kvargs_process(kvlist, VIRTIO_USER_ARG_QUEUE_SIZE, - &get_integer_arg, &queue_size); + if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_QUEUE_SIZE) == 1) { + ret = rte_kvargs_process(kvlist, VIRTIO_USER_ARG_QUEUE_SIZE, +&get_integer_arg, &queue_size); + if (ret < 0) { + PMD_INIT_LOG(ERR, "error to parse %s", +VIRTIO_USER_ARG_QUEUE_SIZE); + goto end; + } + } - if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_QUEUES_NUM) == 1) - rte_kvargs_process(kvlist, VIRTIO_USER_ARG_QUEUES_NUM, - &get_integer_arg, &queues); + if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_QUEUES_NUM) == 1) { + ret = rte_kvargs_process(kvlist, VIRTIO_USER_ARG_QUEUES_NUM, +&get_integer_arg, &queues); + if (ret < 0) { + PMD_INIT_LOG(ERR, "error to parse %s", +VIRTIO_USER_ARG_QUEUES_NUM); + goto end; + } + } - if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_CQ_NUM) == 1) - rte_kvargs_process(kvlist, VIRTIO_USER_ARG_CQ_NUM, - &get_integer_arg, &cq); + if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_CQ_NUM) == 1) { + ret = rte_kvargs_process(kvlist, VIRTIO_USER_ARG_CQ_NUM, +&get_integer_arg, &cq); + if (ret < 0) { + PMD_INIT_LOG(ERR, "error to parse %s", +VIRTIO_USER_ARG_CQ_NUM); + goto end; + } + } el
[dpdk-dev] [PATCH 0/4] net/virtio-user: fix coverity issues
Patch 1: fix return value not checked, Coverity issue: 127344, 127478 Patch 2: fix string overflow, Coverity issue: 127484 Patch 3: fix resource leaks, Coverity issue: 127482 Patch 4: fix string unterminated, Coverity issue: 127476 Jianfeng Tan (4): net/virtio-user: fix return value not checked net/virtio-user: fix string overflow net/virtio-user: fix resource leaks net/virtio-user: fix string unterminated drivers/net/virtio/virtio_user/vhost_user.c | 5 +- drivers/net/virtio/virtio_user/virtio_user_dev.c | 2 +- drivers/net/virtio/virtio_user_ethdev.c | 61 ++-- 3 files changed, 50 insertions(+), 18 deletions(-) -- 2.1.4
[dpdk-dev] [PATCHv8 0/6] Implement pmd hardware support exports
On Wed, Jun 29, 2016 at 10:18:26AM +0100, Remy Horton wrote: > Morning, > > Compile error with virtio_user_ethdev.c - looks like a new addition (commit > ce2eabdd43ec) that also needs to be converted. > > Regards, > > ..Remy > diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c index 5ab2471..aba9ebe 100644 --- a/drivers/net/virtio/virtio_user_ethdev.c +++ b/drivers/net/virtio/virtio_user_ethdev.c @@ -437,4 +437,4 @@ static struct rte_driver virtio_user_driver = { .uninit = virtio_user_pmd_devuninit, }; -PMD_REGISTER_DRIVER(virtio_user_driver); +PMD_REGISTER_DRIVER(virtio_user_driver, vio_user);
[dpdk-dev] [PATCH v2] net/virtio-user: fix build error in SUSE 11
On some older systems, such as SUSE 11, the compiling error shows as: .../dpdk/drivers/net/virtio/virtio_user/virtio_user_dev.c:67:22: error: ?O_CLOEXEC? undeclared (first use in this function) The fix is to use EFD_CLOEXEC, which is defined in sys/eventfd.h, instead of O_CLOEXEC which needs _GNU_SOURCE defined on some old systems. Fixes: 37a7eb2ae816 ("net/virtio-user: add device emulation layer") Signed-off-by: Jianfeng Tan --- v2: - Change the way to fix this issue. drivers/net/virtio/virtio_user/virtio_user_dev.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c index 3d12a32..1b1e5bf 100644 --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c @@ -63,12 +63,12 @@ virtio_user_kick_queue(struct virtio_user_dev *dev, uint32_t queue_sel) /* May use invalid flag, but some backend leverages kickfd and callfd as * criteria to judge if dev is alive. so finally we use real event_fd. */ - callfd = eventfd(0, O_CLOEXEC | O_NONBLOCK); + callfd = eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK); if (callfd < 0) { PMD_DRV_LOG(ERR, "callfd error, %s\n", strerror(errno)); return -1; } - kickfd = eventfd(0, O_CLOEXEC | O_NONBLOCK); + kickfd = eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK); if (kickfd < 0) { close(callfd); PMD_DRV_LOG(ERR, "kickfd error, %s\n", strerror(errno)); -- 2.1.4
[dpdk-dev] [PATCH] net/virtio-user: fix missing default macro
With current config structure, all configuration parameters put into common_base with a default value, and overwritten in environment file if required, CONFIG_RTE_VIRTIO_USER is missing in common_base. This fix is simple, by adding CONFIG_RTE_VIRTIO_USER=n as the default macro value. Fixes: ce2eabdd43ec ("net/virtio-user: add virtual device") Reported-by: Ferruh Yigit Signed-off-by: Jianfeng Tan --- config/common_base | 5 + 1 file changed, 5 insertions(+) diff --git a/config/common_base b/config/common_base index 5d9daf5..0368d59 100644 --- a/config/common_base +++ b/config/common_base @@ -269,6 +269,11 @@ CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_DRIVER=n CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_DUMP=n # +# Compile software VIRTIO-USER PMD driver +# +CONFIG_RTE_VIRTIO_USER=n + +# # Compile burst-oriented VMXNET3 PMD driver # CONFIG_RTE_LIBRTE_VMXNET3_PMD=y -- 2.1.4
[dpdk-dev] [PATCH v5 3/3] mempool: allow for user-owned mempool caches
The mempool cache is only available to EAL threads as a per-lcore resource. Change this so that the user can create and provide their own cache on mempool get and put operations. This works with non-EAL threads too. This commit introduces the new API calls: rte_mempool_cache_create(size, socket_id) rte_mempool_cache_free(cache) rte_mempool_cache_flush(cache, mp) rte_mempool_default_cache(mp, lcore_id) Changes the API calls: rte_mempool_generic_put(mp, obj_table, n, cache, flags) rte_mempool_generic_get(mp, obj_table, n, cache, flags) The cache-oblivious API calls use the per-lcore default local cache. Signed-off-by: Lazaros Koromilas Acked-by: Olivier Matz --- app/test/test_mempool.c | 73 --- app/test/test_mempool_perf.c| 73 +-- doc/guides/prog_guide/env_abstraction_layer.rst | 4 +- doc/guides/prog_guide/mempool_lib.rst | 6 +- lib/librte_mempool/rte_mempool.c| 66 +- lib/librte_mempool/rte_mempool.h| 164 +--- lib/librte_mempool/rte_mempool_version.map | 4 + 7 files changed, 308 insertions(+), 82 deletions(-) diff --git a/app/test/test_mempool.c b/app/test/test_mempool.c index 55c2cbc..63c61f3 100644 --- a/app/test/test_mempool.c +++ b/app/test/test_mempool.c @@ -75,10 +75,16 @@ #define MAX_KEEP 16 #define MEMPOOL_SIZE ((rte_lcore_count()*(MAX_KEEP+RTE_MEMPOOL_CACHE_MAX_SIZE))-1) +#define LOG_ERR() printf("test failed at %s():%d\n", __func__, __LINE__) #define RET_ERR() do { \ - printf("test failed at %s():%d\n", __func__, __LINE__); \ + LOG_ERR(); \ return -1; \ } while (0) +#define GOTO_ERR(var, label) do { \ + LOG_ERR(); \ + var = -1; \ + goto label; \ + } while (0) static rte_atomic32_t synchro; @@ -191,7 +197,7 @@ my_obj_init(struct rte_mempool *mp, __attribute__((unused)) void *arg, /* basic tests (done on one core) */ static int -test_mempool_basic(struct rte_mempool *mp) +test_mempool_basic(struct rte_mempool *mp, int use_external_cache) { uint32_t *objnum; void **objtable; @@ -199,47 +205,62 @@ test_mempool_basic(struct rte_mempool *mp) char *obj_data; int ret = 0; unsigned i, j; + int offset; + struct rte_mempool_cache *cache; + + if (use_external_cache) { + /* Create a user-owned mempool cache. */ + cache = rte_mempool_cache_create(RTE_MEMPOOL_CACHE_MAX_SIZE, +SOCKET_ID_ANY); + if (cache == NULL) + RET_ERR(); + } else { + /* May be NULL if cache is disabled. */ + cache = rte_mempool_default_cache(mp, rte_lcore_id()); + } /* dump the mempool status */ rte_mempool_dump(stdout, mp); printf("get an object\n"); - if (rte_mempool_get(mp, &obj) < 0) - RET_ERR(); + if (rte_mempool_generic_get(mp, &obj, 1, cache, 0) < 0) + GOTO_ERR(ret, out); rte_mempool_dump(stdout, mp); /* tests that improve coverage */ printf("get object count\n"); - if (rte_mempool_count(mp) != MEMPOOL_SIZE - 1) - RET_ERR(); + /* We have to count the extra caches, one in this case. */ + offset = use_external_cache ? 1 * cache->len : 0; + if (rte_mempool_count(mp) + offset != MEMPOOL_SIZE - 1) + GOTO_ERR(ret, out); printf("get private data\n"); if (rte_mempool_get_priv(mp) != (char *)mp + MEMPOOL_HEADER_SIZE(mp, mp->cache_size)) - RET_ERR(); + GOTO_ERR(ret, out); #ifndef RTE_EXEC_ENV_BSDAPP /* rte_mem_virt2phy() not supported on bsd */ printf("get physical address of an object\n"); if (rte_mempool_virt2phy(mp, obj) != rte_mem_virt2phy(obj)) - RET_ERR(); + GOTO_ERR(ret, out); #endif printf("put the object back\n"); - rte_mempool_put(mp, obj); + rte_mempool_generic_put(mp, &obj, 1, cache, 0); rte_mempool_dump(stdout, mp); printf("get 2 objects\n"); - if (rte_mempool_get(mp, &obj) < 0) - RET_ERR(); - if (rte_mempool_get(mp, &obj2) < 0) { - rte_mempool_put(mp, obj); - RET_ERR(); + if (rte_mempool_generic_get(mp, &obj, 1, cache, 0) < 0) + GOTO_ERR(ret, out); + if (rte_mempool_generic_get(mp, &obj2, 1, cache, 0) < 0) { + rte_mempool_generic_put(mp, &obj, 1,
[dpdk-dev] [PATCH v5 2/3] mempool: use bit flags to set multi consumers and producers
Pass the same flags as in rte_mempool_create(). Changes API calls: rte_mempool_generic_put(mp, obj_table, n, flags) rte_mempool_generic_get(mp, obj_table, n, flags) Signed-off-by: Lazaros Koromilas Acked-by: Olivier Matz --- lib/librte_mempool/rte_mempool.h | 58 +--- 1 file changed, 30 insertions(+), 28 deletions(-) diff --git a/lib/librte_mempool/rte_mempool.h b/lib/librte_mempool/rte_mempool.h index a48f46d..971b1ba 100644 --- a/lib/librte_mempool/rte_mempool.h +++ b/lib/librte_mempool/rte_mempool.h @@ -953,12 +953,13 @@ void rte_mempool_dump(FILE *f, struct rte_mempool *mp); * @param n * The number of objects to store back in the mempool, must be strictly * positive. - * @param is_mp - * Mono-producer (0) or multi-producers (1). + * @param flags + * The flags used for the mempool creation. + * Single-producer (MEMPOOL_F_SP_PUT flag) or multi-producers. */ static inline void __attribute__((always_inline)) __mempool_generic_put(struct rte_mempool *mp, void * const *obj_table, - unsigned n, int is_mp) + unsigned n, int flags) { struct rte_mempool_cache *cache; uint32_t index; @@ -971,7 +972,7 @@ __mempool_generic_put(struct rte_mempool *mp, void * const *obj_table, __MEMPOOL_STAT_ADD(mp, put, n); /* cache is not enabled or single producer or non-EAL thread */ - if (unlikely(cache_size == 0 || is_mp == 0 || + if (unlikely(cache_size == 0 || flags & MEMPOOL_F_SP_PUT || lcore_id >= RTE_MAX_LCORE)) goto ring_enqueue; @@ -1024,15 +1025,16 @@ ring_enqueue: * A pointer to a table of void * pointers (objects). * @param n * The number of objects to add in the mempool from the obj_table. - * @param is_mp - * Mono-producer (0) or multi-producers (1). + * @param flags + * The flags used for the mempool creation. + * Single-producer (MEMPOOL_F_SP_PUT flag) or multi-producers. */ static inline void __attribute__((always_inline)) rte_mempool_generic_put(struct rte_mempool *mp, void * const *obj_table, - unsigned n, int is_mp) + unsigned n, int flags) { __mempool_check_cookies(mp, obj_table, n, 0); - __mempool_generic_put(mp, obj_table, n, is_mp); + __mempool_generic_put(mp, obj_table, n, flags); } /** @@ -1050,7 +1052,7 @@ __rte_deprecated static inline void __attribute__((always_inline)) rte_mempool_mp_put_bulk(struct rte_mempool *mp, void * const *obj_table, unsigned n) { - rte_mempool_generic_put(mp, obj_table, n, 1); + rte_mempool_generic_put(mp, obj_table, n, 0); } /** @@ -1068,7 +1070,7 @@ __rte_deprecated static inline void __attribute__((always_inline)) rte_mempool_sp_put_bulk(struct rte_mempool *mp, void * const *obj_table, unsigned n) { - rte_mempool_generic_put(mp, obj_table, n, 0); + rte_mempool_generic_put(mp, obj_table, n, MEMPOOL_F_SP_PUT); } /** @@ -1089,8 +1091,7 @@ static inline void __attribute__((always_inline)) rte_mempool_put_bulk(struct rte_mempool *mp, void * const *obj_table, unsigned n) { - rte_mempool_generic_put(mp, obj_table, n, - !(mp->flags & MEMPOOL_F_SP_PUT)); + rte_mempool_generic_put(mp, obj_table, n, mp->flags); } /** @@ -1105,7 +1106,7 @@ rte_mempool_put_bulk(struct rte_mempool *mp, void * const *obj_table, __rte_deprecated static inline void __attribute__((always_inline)) rte_mempool_mp_put(struct rte_mempool *mp, void *obj) { - rte_mempool_generic_put(mp, &obj, 1, 1); + rte_mempool_generic_put(mp, &obj, 1, 0); } /** @@ -1120,7 +1121,7 @@ rte_mempool_mp_put(struct rte_mempool *mp, void *obj) __rte_deprecated static inline void __attribute__((always_inline)) rte_mempool_sp_put(struct rte_mempool *mp, void *obj) { - rte_mempool_generic_put(mp, &obj, 1, 0); + rte_mempool_generic_put(mp, &obj, 1, MEMPOOL_F_SP_PUT); } /** @@ -1149,15 +1150,16 @@ rte_mempool_put(struct rte_mempool *mp, void *obj) * A pointer to a table of void * pointers (objects). * @param n * The number of objects to get, must be strictly positive. - * @param is_mc - * Mono-consumer (0) or multi-consumers (1). + * @param flags + * The flags used for the mempool creation. + * Single-consumer (MEMPOOL_F_SC_GET flag) or multi-consumers. * @return * - >=0: Success; number of objects supplied. * - <0: Error; code of ring dequeue function. */ static inline int __attribute__((always_inline)) __mempool_generic_get(struct rte_mempool *mp, void **obj_table, - unsigned n, int is_mc) + unsigned n, int flags) { int ret; struct rte_mempool_cache *cache; @@ -1167,7 +1169,7 @@ __mempool_generic_get(struct rte_mempool *mp, void **obj_table, uint32_t cache_size = mp->cache_size;
[dpdk-dev] [PATCH v5 1/3] mempool: deprecate specific get and put functions
This commit introduces the API calls: rte_mempool_generic_put(mp, obj_table, n, is_mp) rte_mempool_generic_get(mp, obj_table, n, is_mc) Deprecates the API calls: rte_mempool_mp_put_bulk(mp, obj_table, n) rte_mempool_sp_put_bulk(mp, obj_table, n) rte_mempool_mp_put(mp, obj) rte_mempool_sp_put(mp, obj) rte_mempool_mc_get_bulk(mp, obj_table, n) rte_mempool_sc_get_bulk(mp, obj_table, n) rte_mempool_mc_get(mp, obj_p) rte_mempool_sc_get(mp, obj_p) We also check cookies in one place now. Signed-off-by: Lazaros Koromilas Acked-by: Olivier Matz --- app/test/test_mempool.c| 10 +-- lib/librte_mempool/rte_mempool.h | 115 - lib/librte_mempool/rte_mempool_version.map | 2 + 3 files changed, 87 insertions(+), 40 deletions(-) diff --git a/app/test/test_mempool.c b/app/test/test_mempool.c index 31582d8..55c2cbc 100644 --- a/app/test/test_mempool.c +++ b/app/test/test_mempool.c @@ -338,7 +338,7 @@ static int test_mempool_single_producer(void) printf("obj not owned by this mempool\n"); RET_ERR(); } - rte_mempool_sp_put(mp_spsc, obj); + rte_mempool_put(mp_spsc, obj); rte_spinlock_lock(&scsp_spinlock); scsp_obj_table[i] = NULL; rte_spinlock_unlock(&scsp_spinlock); @@ -371,7 +371,7 @@ static int test_mempool_single_consumer(void) rte_spinlock_unlock(&scsp_spinlock); if (i >= MAX_KEEP) continue; - if (rte_mempool_sc_get(mp_spsc, &obj) < 0) + if (rte_mempool_get(mp_spsc, &obj) < 0) break; rte_spinlock_lock(&scsp_spinlock); scsp_obj_table[i] = obj; @@ -477,13 +477,13 @@ test_mempool_basic_ex(struct rte_mempool *mp) } for (i = 0; i < MEMPOOL_SIZE; i ++) { - if (rte_mempool_mc_get(mp, &obj[i]) < 0) { + if (rte_mempool_get(mp, &obj[i]) < 0) { printf("test_mp_basic_ex fail to get object for [%u]\n", i); goto fail_mp_basic_ex; } } - if (rte_mempool_mc_get(mp, &err_obj) == 0) { + if (rte_mempool_get(mp, &err_obj) == 0) { printf("test_mempool_basic_ex get an impossible obj\n"); goto fail_mp_basic_ex; } @@ -494,7 +494,7 @@ test_mempool_basic_ex(struct rte_mempool *mp) } for (i = 0; i < MEMPOOL_SIZE; i++) - rte_mempool_mp_put(mp, obj[i]); + rte_mempool_put(mp, obj[i]); if (rte_mempool_full(mp) != 1) { printf("test_mempool_basic_ex the mempool should be full\n"); diff --git a/lib/librte_mempool/rte_mempool.h b/lib/librte_mempool/rte_mempool.h index 0a1777c..a48f46d 100644 --- a/lib/librte_mempool/rte_mempool.h +++ b/lib/librte_mempool/rte_mempool.h @@ -957,8 +957,8 @@ void rte_mempool_dump(FILE *f, struct rte_mempool *mp); * Mono-producer (0) or multi-producers (1). */ static inline void __attribute__((always_inline)) -__mempool_put_bulk(struct rte_mempool *mp, void * const *obj_table, - unsigned n, int is_mp) +__mempool_generic_put(struct rte_mempool *mp, void * const *obj_table, + unsigned n, int is_mp) { struct rte_mempool_cache *cache; uint32_t index; @@ -1016,7 +1016,7 @@ ring_enqueue: /** - * Put several objects back in the mempool (multi-producers safe). + * Put several objects back in the mempool. * * @param mp * A pointer to the mempool structure. @@ -1024,16 +1024,37 @@ ring_enqueue: * A pointer to a table of void * pointers (objects). * @param n * The number of objects to add in the mempool from the obj_table. + * @param is_mp + * Mono-producer (0) or multi-producers (1). */ static inline void __attribute__((always_inline)) +rte_mempool_generic_put(struct rte_mempool *mp, void * const *obj_table, + unsigned n, int is_mp) +{ + __mempool_check_cookies(mp, obj_table, n, 0); + __mempool_generic_put(mp, obj_table, n, is_mp); +} + +/** + * @deprecated + * Put several objects back in the mempool (multi-producers safe). + * + * @param mp + * A pointer to the mempool structure. + * @param obj_table + * A pointer to a table of void * pointers (objects). + * @param n + * The number of objects to add in the mempool from the obj_table. + */ +__rte_deprecated static inline void __attribute__((always_inline)) rte_mempool_mp_put_bulk(struct rte_mempool *mp, void * const *obj_table, unsigned n) { - __mempool_check_cookies(mp, obj_table, n, 0); - __mempool_put_bulk(mp, obj_table, n, 1); + rte_mempool_generic_put(mp, obj_table, n, 1); } /** + * @deprecated * Put several objects back in the mempool (NOT multi-producers safe).
[dpdk-dev] [PATCH v5 0/3] mempool: user-owned mempool caches
Updated version of the user-owned cache patchset. It applies on top of the latest v15 external mempool manager patches from David Hunt [1]. [1] http://dpdk.org/ml/archives/dev/2016-June/042004.html v5 changes: * Rework error path macros in tests. * Update documentation. * Style fixes. v4 changes: * Fix compilation with shared libraries * Add a GOTO_ERR() macro to factorize code in test_mempool.c * Change title of patch 2 to conform to check-git-log.sh v3 changes: * Deprecate specific mempool API calls instead of removing them. * Split deprecation into a separate commit to limit noise. * Fix cache flush by setting cache->len = 0 and make it inline. * Remove cache->size == 0 checks and ensure size != 0 at creation. * Fix tests to check if cache creation succeeded. * Fix tests to free allocated resources on error. The mempool cache is only available to EAL threads as a per-lcore resource. Change this so that the user can create and provide their own cache on mempool get and put operations. This works with non-EAL threads too. Also, deprecate the explicit {mp,sp}_put and {mc,sc}_get calls and re-route them through the new generic calls. Minor cleanup to pass the mempool bit flags instead of using specific is_mp and is_mc. The old cache-oblivious API calls use the per-lcore default local cache. The mempool and mempool_perf tests are also updated to handle the user-owned cache case. Introduced API calls: rte_mempool_cache_create(size, socket_id) rte_mempool_cache_free(cache) rte_mempool_cache_flush(cache, mp) rte_mempool_default_cache(mp, lcore_id) rte_mempool_generic_put(mp, obj_table, n, cache, flags) rte_mempool_generic_get(mp, obj_table, n, cache, flags) Deprecated API calls: rte_mempool_mp_put_bulk(mp, obj_table, n) rte_mempool_sp_put_bulk(mp, obj_table, n) rte_mempool_mp_put(mp, obj) rte_mempool_sp_put(mp, obj) rte_mempool_mc_get_bulk(mp, obj_table, n) rte_mempool_sc_get_bulk(mp, obj_table, n) rte_mempool_mc_get(mp, obj_p) rte_mempool_sc_get(mp, obj_p) Lazaros Koromilas (3): mempool: deprecate specific get and put functions mempool: use bit flags to set multi consumers and producers mempool: allow for user-owned mempool caches app/test/test_mempool.c | 83 +--- app/test/test_mempool_perf.c| 73 ++- doc/guides/prog_guide/env_abstraction_layer.rst | 4 +- doc/guides/prog_guide/mempool_lib.rst | 6 +- lib/librte_mempool/rte_mempool.c| 66 +- lib/librte_mempool/rte_mempool.h| 257 ++-- lib/librte_mempool/rte_mempool_version.map | 6 + 7 files changed, 385 insertions(+), 110 deletions(-) -- 1.9.1
[dpdk-dev] Fwd: dpdk ixgbe PMD lro limits
Hi Asim, > -Original Message- > From: Asim Jamshed [mailto:asim.jamshed at gmail.com] > Sent: Tuesday, June 28, 2016 5:23 PM > To: Lu, Wenzhuo > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] Fwd: dpdk ixgbe PMD lro limits > > On Tue, Jun 28, 2016 at 5:42 PM, Lu, Wenzhuo wrote: > > Hi Asim, > > > > > >> -Original Message- > >> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Asim Jamshed > >> Sent: Tuesday, June 28, 2016 2:41 PM > >> To: dev at dpdk.org > >> Subject: [dpdk-dev] Fwd: dpdk ixgbe PMD lro limits > >> > >> Hi, > >> > >> Apologies in advance if this question has been asked in the past. > >> > >> I have been performing mTCP-related tests on dpdk-16.04 with ixgbe > >> PMD. I am using 82599ES 10-Gigabit adapters for my experiments. I > >> have a few queries regarding LRO. > >> > >> 1) What is the theoretical maximum size of the Ethernet frame I can > >> get from the driver once LRO is enabled? In my experiments, I was > >> seeing packet size as high as 16KB. Can it be as high as ~2^16 bytes (iph- > >tot_len)? > > I'm not sure about if I understand your question correctly. Assume you're > talking about the TCP segment after LRO. So, it's said there are no > limitations on > the maximum packet length. > > The issue that I am facing is fixing the size of mbuf. > The default size of mbuf is (2048 + sizeof(struct rte_mbuf) + > RTE_PKTMBUF_HEADROOM) which needs to be augmented once lro is enabled > (am I right?). I tried stretching the limits of the mempool by increasing > mbuf size > to (65536 + sizeof(struct rte_mbuf) + RTE_PKTMBUF_HEADROOM), and I got the > following error: I don't think you need to change the size of mbuf. Please check this field of mbuf, "struct rte_mbuf *next;/**< Next segment of scattered packet. */" > > Initializing port 0... EAL: Error - exiting with code: 1 > Cause: rte_eth_rx_queue_setup:err=-22, port=0, queueid: 0 > > My exact calls for rte_mempool_create() and > rte_eth_rx_queue_setup() were: > > pktmbuf_pool = rte_mempool_create(name, 8192, > MBUF_SIZE, 256, > sizeof(struct rte_pktmbuf_pool_private), > rte_pktmbuf_pool_init, NULL, > rte_pktmbuf_init, NULL, > rte_socket_id(), 0); > > ret = rte_eth_rx_queue_setup(portid, rxlcore_id, 128, > rte_eth_dev_socket_id(portid), &rx_conf, > pktmbuf_pool); > > > > >> > >> 2) Since the NIC is reassembling payloads (of one flow) into a single > >> packet, what does the Ethernet controller do with the tcp checksum > >> field in the TCP header? I am observing that each LRO packet has > >> checksum value as zero? Is that normal? I could not find any relevant > documentation on the Web. > > That's expected. > > You can search 82599 datasheet. I think that's what you're looking for. > > Thanks! > > > Regards, > --Asim > > > > >> > >> Thanks in advance, > >> --Asim
[dpdk-dev] [PATCH v5 2/5] bnx2x: add xstats support
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > Sent: Tuesday, June 28, 2016 7:34 AM > > 2016-05-11 17:06, Rasesh Mody: > > Add extended stats support to provide additional statistical info. > > > > Signed-off-by: Rasesh Mody > > Signed-off-by: Harish Patil > > This bnx2x patch and a qede one cannot be merged in master tree because > of an API change: > http://dpdk.org/commit/bd6aa172 > http://dpdk.org/commit/e2aae1c1 > Please could you rework this patch for RC2? > > Sorry for the inconvenience, I should have caught it earlier. > We'll try to synchronize the branches more often in future. Will work on it and submit patches to incorporate the API change.
[dpdk-dev] [PATCH v2 6/9] qede: add support for xstats
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > Sent: Tuesday, June 28, 2016 7:32 AM > > 2016-06-15 22:47, Rasesh Mody: > > This patch adds support for extended statistics for QEDE PMD. > > > > Signed-off-by: Rasesh Mody > > This qede patch and a bnx2x one cannot be merged in master tree because > of an API change: > http://dpdk.org/commit/bd6aa172 > http://dpdk.org/commit/e2aae1c1 > Please could you rework this patch for RC2? > > Sorry for the inconvenience, I should have caught it earlier. > We'll try to synchronize the branches more often in future. Will work on it and submit patches to incorporate the API change.