[dpdk-dev] [PATCH v3] lib/librte_vhost: vhost library support to facilitate integration with DPDK accelerated vswitch.
> -Original Message- > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > Sent: Friday, August 29, 2014 4:16 AM > To: Xie, Huawei > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v3] lib/librte_vhost: vhost library support to > facilitate integration with DPDK accelerated vswitch. > > > config/common_linuxapp |7 + > > lib/Makefile |1 + > > lib/librte_vhost/Makefile| 48 ++ > > lib/librte_vhost/eventfd_link/Makefile | 39 + > > lib/librte_vhost/eventfd_link/eventfd_link.c | 194 + > > lib/librte_vhost/eventfd_link/eventfd_link.h | 40 + > > lib/librte_vhost/rte_virtio_net.h| 192 + > > lib/librte_vhost/vhost-net-cdev.c| 363 ++ > > lib/librte_vhost/vhost-net-cdev.h| 109 +++ > > lib/librte_vhost/vhost_rxtx.c| 292 > > lib/librte_vhost/virtio-net.c| 1002 > > ++ > > It would help if you made a first patch to move existing code, > another patch to convert it into a lib, and a last one for > the new example. > So it would show how you transform the old example code and > would be easier to review. > > Thanks > -- > Thomas My original order is first patch to add a new lib but not turned on, another patch to remove the old example, and the third patch to add the new example. I am preparing new patch sets, merging the mergable feature, will follow the suggested order. -huawei
[dpdk-dev] [PATCH 01/11] ixgbe: clean log messages
On Wed, Aug 27, 2014 at 8:06 PM, Stephen Hemminger < stephen at networkplumber.org> wrote: > On Wed, 27 Aug 2014 08:53:46 -0500 > Jay Rolette wrote: > > > The updated output is definitely an improvement, but if you'll go with > the > > first solution you described (adding \n in the log macro), it works much > > better for products using DPDK. For developer use, I think it ends up > being > > a wash either way. > > Also for driver consistency, all drivers must do the same thing. I.e the > PMD_INIT_LOG > macro should either add or not add newline in the same way in all drivers. > I fixed virtio and vmxnet3 by just taking extra newlines out of messages > and leaving the macro alone. > Not sure that we can talk about consistency at the moment ... (all the more so as if you only fixed two pmds). Anyway. If we go with this approach, there is still something that won't work. ixgbe shared code uses DEBUGOUT* macros which are called a lot in this shared code. The macros call sites use trailing \n. I suppose I will have a big NO from Intel guys if I remove the trailing \n, so I suppose we can use an alternate macro that calls RTE_LOG rather than PMD_DRV_LOG in ixgbe_os_dep.h. I will also remove any reference to DEBUGOUT* in non-shared code. >From my point of view, these macro are only here for compat with shared code. By the way, did you look at the other patches of this series ? I want to send a v2 later (maybe not today), so any comment on the other patches are welcome. -- David Marchand
[dpdk-dev] [PATCH] hash: added rte_hash_keys to extract all keys
Hi Tomas,
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Tomas Vestelind
> Sent: Tuesday, August 12, 2014 10:48 PM
> To: dev at dpdk.org
> Subject: [dpdk-dev] [PATCH] hash: added rte_hash_keys to extract all keys
>
> I added a function which extracts all the configured keys in a hash map.
> This is good to have when debugging and printing data store in hash
> maps.
>
> Signed-off-by: Tomas Vestelind
> ---
> lib/librte_hash/rte_hash.c | 26 ++
> lib/librte_hash/rte_hash.h | 15 +++
> 2 files changed, 41 insertions(+)
>
> diff --git a/lib/librte_hash/rte_hash.c b/lib/librte_hash/rte_hash.c
> index d02b6b4..2108c4f 100644
> --- a/lib/librte_hash/rte_hash.c
> +++ b/lib/librte_hash/rte_hash.c
> @@ -481,3 +481,29 @@ rte_hash_lookup_bulk(const struct rte_hash *h, const
> void **keys,
>
> return 0;
> }
> +
> +unsigned int
> +rte_hash_keys(const struct rte_hash *h, void *keys)
> +{
> +unsigned int found_keys = 0;
> +unsigned int bucket, entry;
> +
> +/* Go through each bucket and all its entries */
> +for (bucket = 0; bucket < h->num_buckets; bucket++) {
> +const hash_sig_t *sig = get_sig_tbl_bucket(h, bucket);
> +
> +for (entry = 0; entry < h->bucket_entries; entry++) {
> +/* If the signature is valid, find and save the corresponding
> key */
> +if (sig[entry] != NULL_SIGNATURE) {
> + uint8_t *key_bucket = get_key_tbl_bucket(h, bucket);
> + void *key = get_key_from_bucket(h, key_bucket, entry);
> + rte_memcpy(keys, key, h->key_len);
> +
> + keys = (uint8_t* )keys + h->key_len;
> + found_keys++;
> +}
> +}
> +}
I suppose we need to add one extra parameter: keys_size (size of the buffer
provided) and inside the loop check that we wouldn't go beyond it.
Also it would probably helpful to have a function that would return to the user
size of buffer needed to sore all keys from the hash.
What you probably can do: implement a function that would iterate over all
elements in the hash table and for each of them call a callback (taken as an
argument).
Then all these functions and rte_hash_clear() could be implemented on top of it.
Something like:
typedef int (*rte_hash_elem_iter_t)(const struct rte_hash*hash, const void
*key, void *user_arg);
void
rte_hash_elem_iterate(const struct rte_hash *h, rte_hash_key_elem_t iter, void
*arg)
{
...
}
About rte_hash_clear(const struct rte_hash *h) - can't we just do
memset(h->sig_tbl, NULL_SIGNATURE, sig_tbl_size);?
BTW, why it is pointer to the const strucuture if we modifying it?
> +
> +return found_keys;
> +}
> diff --git a/lib/librte_hash/rte_hash.h b/lib/librte_hash/rte_hash.h
> index 2ecaf1a..e0fb28f 100644
> --- a/lib/librte_hash/rte_hash.h
> +++ b/lib/librte_hash/rte_hash.h
> @@ -303,6 +303,21 @@ rte_hash_hash(const struct rte_hash *h, const void *key)
> int
> rte_hash_lookup_bulk(const struct rte_hash *h, const void **keys,
> uint32_t num_keys, int32_t *positions);
> +
> +/**
> + * Copy the hash table keys to the supplied list of keys.
> + * This operation is multi-thread safe.
> + *
> + * @param h
> + * Hash table to look in.
> + * @param keys
> + * A pointer to a list of where keys will be written.
> + * Must be large enough to fit a potentially full hash map.
> + * @return
> + * The number of found keys.
> + */
> +unsigned int
> +rte_hash_keys(const struct rte_hash *h, void *keys);
> #ifdef __cplusplus
> }
> #endif
> --
> 1.7.10.4
[dpdk-dev] [PATCH] eal: remove unused macros
> Clean both linux and bsd implementations from unused macros. > > Signed-off-by: David Marchand Acked-by: Thomas Monjalon Applied for version 1.7.1. Thanks -- Thomas
[dpdk-dev] [PATCH] vmxnet3: fix crash on stop
> The cmd_ring_release can be called twice if queue has already > been released. This cause crash on shutdown. > > Signed-off-by: Stephen Hemminger > > @@ -173,6 +173,7 @@ vmxnet3_cmd_ring_release(vmxnet3_cmd_rin > vmxnet3_cmd_ring_adv_next2comp(ring); > } > rte_free(ring->buf_info); > + ring->buf_info = NULL; > } Acked-by: Thomas Monjalon Applied for version 1.7.1. Thanks -- Thomas
[dpdk-dev] [PATCH] there are some memory leak about Dir operator
2014-08-19 11:51, zhang.zhangkun at huawei.com: > From: zhangkun > > > Signed-off-by: zhangkun Acked-by: Thomas Monjalon Log changed to: eal: fix memory leak in hugepage error cases The sysfs directory for hugepages parsing was not closed properly in some error cases. Applied for version 1.7.1. Thanks -- Thomas
[dpdk-dev] [PATCH] kni: fix build with Linux 3.17
> In the series of ever-lasting ugly #ifdefs trying make out-of-tree > drivers to support latest mainline kernel... > > Signed-off-by: Aaro Koskinen Acked-by: Thomas Monjalon Applied with this commit log: Since Linux commit "set name_assign_type in alloc_netdev" (c835a677331495), the function alloc_netdev takes a new parameter (name_assign_type) whose default value is NET_NAME_UNKNOWN. Thanks -- Thomas
[dpdk-dev] [PATCH 2/2] Patch to allow live migration of a VM with US-VHost.
> > > Subject: [dpdk-dev] [PATCH 2/2] Patch to allow live migration of a VM > > > with US-VHost. > > > > > > Signed-off-by: Claire Murphy > > > > +1. This patch enables live migration for us on Intel DPDK vSwitch. > > Acked-by: Mark D. Gray Nobody else reviewed this patch? Title is wrongly formatted, there is no explanation in commit log, there are some checkpatches issues, and more importantly, it doesn't build: error: ?RTE_LOGTYPE_CONFIG? undeclared -- Thomas
[dpdk-dev] [PATCH 4/6] bond: free mbufs if transmission fails in bonding tx_burst functions
Hi Declan, Your patchset is pending for an answer to this comment. 2014-08-20 15:54, Sanford, Robert: > I have a problem with the TX-burst logic of this patch. I believe that for > packets that we *don't* enqueue to the device, we should *NOT* free them. > The API expects that the caller will free them or try again to send them. > > Here is one way to accomplish selective freeing: Move mbuf pointers of > packets successfully enqueued, to the beginning of the caller's mbuf > vector; move mbuf pointers of packets not enqueued, to the end of the > caller's mbuf list. Although possibly re-ordered, the caller will be able > to free/resend all (and only) mbufs that we failed to enqueue.
[dpdk-dev] [PATCH] ixgbe_rx_scan_hw_ring: Fix initializing id and hash fields in flow director mode.
Hi, 2014-08-20 08:46, Pawel Wodkowski: > When Flow Director was used together with bulk alloc, id and hash was swapped > when packet matches flow director filter due to improper fdir field > initialization. > > Signed-off-by: Pawel Wodkowski I'd really like someone else review this touchy patch for integration in 1.7.1. Thanks -- Thomas
[dpdk-dev] [PATCH] hash: added rte_hash_keys to extract all keys
We implemented a more general hash iterator, thought the patch was already submitted.
[dpdk-dev] [PATCH v2 0/6] Support configuring hash functions
2014-08-01 05:26, Zhang, Helin: > Would you please help to apply it as soon as possible, as another > developer here who is now developing another feature (Flow Director) > which has dependency on these patches? This patchset about hash functions and the ones about flow director and VxLAN are pending for an agreement on a good design for filters in general. The target is to have these new filters in release 1.8.0. -- Thomas
[dpdk-dev] [PATCHv4] librte_acl make it build/work for 'default' target
> -Original Message-
> From: Neil Horman [mailto:nhorman at tuxdriver.com]
> Sent: Thursday, August 28, 2014 9:38 PM
> To: dev at dpdk.org
> Cc: Neil Horman; Ananyev, Konstantin; thomas.monjalon at 6wind.com
> Subject: [PATCHv4] librte_acl make it build/work for 'default' target
>
> Make ACL library to build/work on 'default' architecture:
> - make rte_acl_classify_scalar really scalar
> (make sure it wouldn't use sse4 instrincts through resolve_priority()).
> - Provide two versions of rte_acl_classify code path:
> rte_acl_classify_sse() - could be build and used only on systems with sse4.2
> and upper, return -ENOTSUP on lower arch.
> rte_acl_classify_scalar() - a slower version, but could be build and used
> on all systems.
> - keep common code shared between these two codepaths.
>
> v2 chages:
> run-time selection of most appropriate code-path for given ISA.
> By default the highest supprted one is selected.
> User can still override that selection by manually assigning new value to
> the global function pointer rte_acl_default_classify.
> rte_acl_classify() becomes a macro calling whatever rte_acl_default_classify
> points to.
>
> V3 Changes
> Updated classify pointer to be a function so as to better preserve ABI
> REmoved macro definitions for match check functions to make them static
> inline
>
> V4 Changes
> Rewrote classification selection mechanim to use a function table, so that we
> can just store the preferred alg in the rte_acl_ctx struct so that
> multiprocess
> access works. I understand that leaves us with an extra load instruction,
> but I
> think thats ok, because it also allows...
>
> Addition of a new function rte_acl_classify_alg. This function lets you
> specify an enum value to override the acl contexts default algorith when
> doing a
> classification. This allows an application to specify a classification
> algorithm without needing to pulicize each method. I know there was concern
> over keeping those methods public, but we don't have a static ABI at the
> moment,
> so this seems to me a reasonable thing to do, as it gives us less of an ABI
> surface to worry about.
Good way to overcome the problem.
>From what I am seeing it adds a tiny slowdown (as expected) ...
Though it provides a good flexibility and I don't have any better ideas.
So I'd say let stick with that approach.
Below are few technical comments.
Thanks
Konstantin
>
> Fixed misc missed static declarations
>
> Removed acl_match_check.h and moved match_check function to acl_run.h
>
> typdeffed function pointer to match check.
>
> Signed-off-by: Neil Horman
> CC: konstantin.ananyev at intel.com
> CC: thomas.monjalon at 6wind.com
> ---
> app/test-acl/main.c | 13 +-
> app/test/test_acl.c | 10 +-
> lib/librte_acl/Makefile | 5 +-
> lib/librte_acl/acl.h| 1 +
> lib/librte_acl/acl_bld.c| 5 +-
> lib/librte_acl/acl_run.c| 944
>
> lib/librte_acl/acl_run.h| 271
> lib/librte_acl/acl_run_scalar.c | 197 +
> lib/librte_acl/acl_run_sse.c| 630 +++
> lib/librte_acl/rte_acl.c| 62 +++
> lib/librte_acl/rte_acl.h| 66 ++-
> 11 files changed, 1208 insertions(+), 996 deletions(-)
> delete mode 100644 lib/librte_acl/acl_run.c
> create mode 100644 lib/librte_acl/acl_run.h
> create mode 100644 lib/librte_acl/acl_run_scalar.c
> create mode 100644 lib/librte_acl/acl_run_sse.c
>
> diff --git a/app/test/test_acl.c b/app/test/test_acl.c
> index 869f6d3..2169f59 100644
> --- a/app/test/test_acl.c
> +++ b/app/test/test_acl.c
> @@ -859,7 +859,7 @@ test_invalid_parameters(void)
> }
>
> /* cover invalid but positive categories in classify */
> - result = rte_acl_classify_scalar(acx, NULL, NULL, 0, 3);
> + result = rte_acl_classify(acx, NULL, NULL, 0, 3);
Typo, should be:
rte_acl_classify_alg(acx, RTE_ACL_CLASSIFY_SCALAR, NULL, NULL, 0, 3);
> diff --git a/lib/librte_acl/acl.h b/lib/librte_acl/acl.h
> index b9d63fd..9236b7b 100644
> --- a/lib/librte_acl/acl.h
> +++ b/lib/librte_acl/acl.h
> @@ -168,6 +168,7 @@ struct rte_acl_ctx {
> void *mem;
> size_t mem_sz;
> struct rte_acl_config config; /* copy of build config. */
> + enum rte_acl_classify_alg alg;
> };
Each rte_acl_build() will reset all fields of rte_acl_ctx starting from
num_categories and below.
So we need to move alg somewhere above num_categories:
--- a/lib/librte_acl/acl.h
+++ b/lib/librte_acl/acl.h
@@ -153,6 +153,7 @@ struct rte_acl_ctx {
/** Name of the ACL context. */
int32_t socket_id;
/** Socket ID to allocate memory from. */
+ enum rte_acl_classify_alg alg;
void *rules;
uint32_tmax_rules;
uint32_trule_sz;
@@ -168,9 +169,11 @@ struct rte_acl_ctx {
void
[dpdk-dev] DPDK and custom memory
Hello DPDK experts, Imagine a PMD for an FPGA-based NIC that is limited to accessing certain memory regions . Is there a way to make DPDK use that exact memory? Perhaps this is more of a hugetlbfs question than DPDK but I thought I'd start here. Sincerely, Artur Saygin
