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

async: fixed coredump when enable dpdk. #12854

Merged
merged 1 commit into from Jan 10, 2017

Conversation

Projects
None yet
4 participants
@liupan1111
Contributor

liupan1111 commented Jan 10, 2017

Signed-off-by: Pan Liu pan.liu@istuary.com

async: fixed coredump when enable dpdk.
Signed-off-by: Pan Liu <pan.liu@istuary.com>
@liupan1111

This comment has been minimized.

Show comment
Hide comment
@liupan1111

liupan1111 Jan 10, 2017

Contributor

After upgrade to dpdk v16.11, async will coredump when enable dpdk

Contributor

liupan1111 commented Jan 10, 2017

After upgrade to dpdk v16.11, async will coredump when enable dpdk

@liupan1111

This comment has been minimized.

Show comment
Hide comment
@liupan1111

liupan1111 Jan 10, 2017

Contributor

@yuyuyu101 @optimistyzy , please help take look.

Contributor

liupan1111 commented Jan 10, 2017

@yuyuyu101 @optimistyzy , please help take look.

@optimistyzy

This comment has been minimized.

Show comment
Hide comment
@optimistyzy

optimistyzy Jan 10, 2017

Contributor

Since DPDK is upgrade, so I do not think that we need to consider the versioning.

Contributor

optimistyzy commented Jan 10, 2017

Since DPDK is upgrade, so I do not think that we need to consider the versioning.

@yuyuyu101 yuyuyu101 merged commit 0bf9c62 into ceph:master Jan 10, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
default Build finished.
Details
@tchaikov

This comment has been minimized.

Show comment
Hide comment
@tchaikov

tchaikov Jan 10, 2017

Contributor

@liupan1111 and @yuyuyu101 please note imcasts and ibadcrc are marked deprecated and removed in latest DPDK. see 09b4978. @liupan1111 could you share more details on the coredump?

Contributor

tchaikov commented on src/msg/async/dpdk/DPDK.h in 96046f5 Jan 10, 2017

@liupan1111 and @yuyuyu101 please note imcasts and ibadcrc are marked deprecated and removed in latest DPDK. see 09b4978. @liupan1111 could you share more details on the coredump?

This comment has been minimized.

Show comment
Hide comment
@yuyuyu101

yuyuyu101 Jan 10, 2017

Member

oh, yes. we need to remote these

Member

yuyuyu101 replied Jan 10, 2017

oh, yes. we need to remote these

This comment has been minimized.

Show comment
Hide comment
@tchaikov
Contributor

tchaikov replied Jan 10, 2017

@tchaikov

This comment has been minimized.

Show comment
Hide comment
@tchaikov

tchaikov Jan 10, 2017

Contributor

Since DPDK is upgrade, so I do not think that we need to consider the versioning.

we detect DPDK in Finddpdk.cmake which checks the dpdk in the default paths, i don't see why we don't need to take the backward compatibility into consideration.

Contributor

tchaikov commented Jan 10, 2017

Since DPDK is upgrade, so I do not think that we need to consider the versioning.

we detect DPDK in Finddpdk.cmake which checks the dpdk in the default paths, i don't see why we don't need to take the backward compatibility into consideration.

@liupan1111

This comment has been minimized.

Show comment
Hide comment
@liupan1111

liupan1111 Jan 10, 2017

Contributor

@tchaikov In my opinion, we don't need to specify the dpdk version in code, dpdk is already a submodule in ceph code, the user should use conrresponding submodule to build. What do you think?

Contributor

liupan1111 commented Jan 10, 2017

@tchaikov In my opinion, we don't need to specify the dpdk version in code, dpdk is already a submodule in ceph code, the user should use conrresponding submodule to build. What do you think?

@optimistyzy

This comment has been minimized.

Show comment
Hide comment
@optimistyzy

optimistyzy Jan 10, 2017

Contributor

I have the same opinion,each ceph version should uses the corresonding dpdk,which shown in submodulesit can avoid the compatibility issue

Contributor

optimistyzy commented Jan 10, 2017

I have the same opinion,each ceph version should uses the corresonding dpdk,which shown in submodulesit can avoid the compatibility issue

@tchaikov

This comment has been minimized.

Show comment
Hide comment
@tchaikov

tchaikov Jan 10, 2017

Contributor

the user should use conrresponding submodule to build.

i doubt it. at least, we are not forcing our user to do so. if we really want to enforce the dpdk version to 16.11 or up used in ceph, we can bump the minimum required version in #12857. but anyway, maybe we can let @yuyuyu101 to make the call.

Contributor

tchaikov commented Jan 10, 2017

the user should use conrresponding submodule to build.

i doubt it. at least, we are not forcing our user to do so. if we really want to enforce the dpdk version to 16.11 or up used in ceph, we can bump the minimum required version in #12857. but anyway, maybe we can let @yuyuyu101 to make the call.

@liupan1111

This comment has been minimized.

Show comment
Hide comment
@liupan1111

liupan1111 Jan 10, 2017

Contributor

@tchaikov @yuyuyu101 , sorry for lack of info, let me say more about the coredump.
In the function PerfCountersBuilder::create_perf_counters, it will loop each perf counter, and check whether it none: assert(d->type != PERFCOUNTER_NONE);
If we don't remove #if RTE_VERSION < RTE_VERSION_NUM(16,7,0,0), and use dpdk 16.11, assertion fail will happen.

Contributor

liupan1111 commented Jan 10, 2017

@tchaikov @yuyuyu101 , sorry for lack of info, let me say more about the coredump.
In the function PerfCountersBuilder::create_perf_counters, it will loop each perf counter, and check whether it none: assert(d->type != PERFCOUNTER_NONE);
If we don't remove #if RTE_VERSION < RTE_VERSION_NUM(16,7,0,0), and use dpdk 16.11, assertion fail will happen.

@tchaikov

This comment has been minimized.

Show comment
Hide comment
@tchaikov

tchaikov Jan 10, 2017

Contributor

@liupan1111 got it, then we might need to conditionalize the

enum {
  l_dpdk_dev_first = 58800,
  l_dpdk_dev_rx_mcast,
  l_dpdk_dev_rx_total_errors,
  l_dpdk_dev_tx_total_errors,
  l_dpdk_dev_rx_badcrc_errors,
  l_dpdk_dev_rx_dropped_errors,
  l_dpdk_dev_rx_nombuf_errors,
  l_dpdk_dev_last
};

block also.

Contributor

tchaikov commented Jan 10, 2017

@liupan1111 got it, then we might need to conditionalize the

enum {
  l_dpdk_dev_first = 58800,
  l_dpdk_dev_rx_mcast,
  l_dpdk_dev_rx_total_errors,
  l_dpdk_dev_tx_total_errors,
  l_dpdk_dev_rx_badcrc_errors,
  l_dpdk_dev_rx_dropped_errors,
  l_dpdk_dev_rx_nombuf_errors,
  l_dpdk_dev_last
};

block also.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment