Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

osd: fix dpdk runtime issue based on spdk/dpdk libarary #19559

Merged
merged 1 commit into from Jan 5, 2018

Conversation

Projects
None yet
4 participants
@liu-chunmei
Copy link
Contributor

commented Dec 16, 2017

1.misunderstand mbuf_overhead before, so correct it.
2.spdk/dpdk library will set refcnt, so needn't set it here.
3.avoid dpdk library modification. when create memory pool set elt_size as
mbuf_overhead + mbuf_data_size to avoid dpdk library check size assert.
4. call rte_pktmbuf_prefree_seg to set mbuf->next = null avoid dpdk/lib
modification .
5. use memzon to allocate mbuf data part, so these data buf can be
processes the same way as mbuf allocated by mempool create.

Signed-off-by: chunmei Liu chunmei.liu@intel.com

@liu-chunmei

This comment has been minimized.

Copy link
Contributor Author

commented Dec 16, 2017

@tchaikov @yuyuyu101 update dpdk bug fix patch here, needn't modify dpdk library.

@liu-chunmei

This comment has been minimized.

Copy link
Contributor Author

commented Dec 16, 2017

@tchaikov any suggestion on ./make-debs.sh failed when set WITH_DPDK=ON? I will work on fix debain build error after Christmas.

@liu-chunmei liu-chunmei force-pushed the liu-chunmei:fix_dpdk_bug_base_spdk branch from e615dd5 to d6e88d6 Dec 16, 2017

@tchaikov
Copy link
Contributor

left a comment

i don't really understand the DPDK stuff. will delegate it to @yuyuyu101

assert(m);
rte_mbuf_refcnt_set(m, 1);
_rx_free_bufs.push_back(m);
int ret = snprintf(mz_name, sizeof(mz_name),

This comment has been minimized.

Copy link
@tchaikov

tchaikov Jan 4, 2018

Contributor

why not just use "rx_buffer"? IMO, no need to print a string here. also, please note, "data" is the plural of "datum".

This comment has been minimized.

Copy link
@liu-chunmei

liu-chunmei Jan 4, 2018

Author Contributor

here, allocate memory just for data part, not include rx_buffer's header. so use the rx_buffer_data as the name. correct "datas" as " data".

const struct rte_memzone *mz = rte_memzone_reserve_aligned(mz_name, mbuf_data_size*bufs_count,
_pktmbuf_pool_rx->socket_id, mz_flags, mbuf_data_size);
void* m = mz->addr;
for (int i = 0; i < bufs_count; i++) {

This comment has been minimized.

Copy link
@tchaikov

tchaikov Jan 4, 2018

Contributor

better off assert(mz) right after the rte_memzone_reserve_aligned() call.

@tchaikov tchaikov requested a review from yuyuyu101 Jan 4, 2018

@liu-chunmei liu-chunmei force-pushed the liu-chunmei:fix_dpdk_bug_base_spdk branch from d6e88d6 to 6284de2 Jan 4, 2018

@liu-chunmei

This comment has been minimized.

Copy link
Contributor Author

commented Jan 4, 2018

@tchaikov add assert(mz), updated patch. @yuyuyu101 please help review the patch.

osd: fix dpdk runtime issue based on spdk/dpdk libarary
1.misunderstand mbuf_overhead before, so correct it.
2.spdk/dpdk library will set refcnt, so needn't set it here.
3.avoid dpdk library modification. when create memory pool set elt_size as
mbuf_overhead + mbuf_data_size to avoid dpdk library check size assert.
4. call rte_pktmbuf_prefree_seg to set mbuf->next = null avoid dpdk/lib
modification .
5. use memzon to allocate mbuf data part, so these data buf can be
processes the same way as mbuf allocated by mempool create.

Signed-off-by: chunmei Liu <chunmei.liu@intel.com>

@liu-chunmei liu-chunmei force-pushed the liu-chunmei:fix_dpdk_bug_base_spdk branch from 6284de2 to c9051fb Jan 4, 2018

@yuyuyu101

This comment has been minimized.

Copy link
Member

commented Jan 5, 2018

nit: maybe revert the previous patch to make clear.

@liu-chunmei

This comment has been minimized.

Copy link
Contributor Author

commented Jan 5, 2018

@yuyuyu101 which patch do you mean? who can help revert the patch?

@yuyuyu101 yuyuyu101 merged commit e4a0cb5 into ceph:master Jan 5, 2018

5 checks passed

Docs: build check OK - docs built
Details
Signed-off-by all commits in this PR are signed
Details
Unmodified Submodules submodules for project are unmodified
Details
make check make check succeeded
Details
make check (arm64) make check succeeded
Details
rte_mbuf_refcnt_set(m, 1);
_rx_free_bufs.push_back(m);
int ret = snprintf(mz_name, sizeof(mz_name),
"%s", "rx_buffer_data" + std::to_string(_qid));

This comment has been minimized.

Copy link
@tchaikov

tchaikov Jan 5, 2018

Contributor

@liu-chunmei i don't think this compiles.
"rx_buffer_data" + std::to_string(_qid)) evaluates to a std::string,

you might want to put something like:

string mz_name = "rx_buffer_data" + std::to_string(_qid));
// ...
const struct rte_memzone *mz = rte_memzone_reserve_aligned(mz_name.c_str(),
    mbuf_data_size*bufs_count,
    _pktmbuf_pool_rx->socket_id, mz_flags, mbuf_data_size);

This comment has been minimized.

Copy link
@yuyuyu101
@tchaikov

This comment has been minimized.

Copy link
Contributor

commented Jan 5, 2018

@liu-chunmei liu-chunmei deleted the liu-chunmei:fix_dpdk_bug_base_spdk branch Jan 5, 2018

@liu-chunmei liu-chunmei restored the liu-chunmei:fix_dpdk_bug_base_spdk branch Jan 5, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.