-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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
cmake: remove counters not used #12857
Conversation
* also refactor it a little bit * support versions before 16.11 Signed-off-by: Kefu Chai <kchai@redhat.com>
@@ -219,7 +219,7 @@ endif(${WITH_XFS}) | |||
|
|||
option(WITH_SPDK "Enable SPDK" OFF) | |||
if(WITH_SPDK) | |||
find_package(dpdk REQUIRED) | |||
find_package(dpdk 16.7 REQUIRED) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need to specify version? let user must use submodule to build?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yuyuyu101 it's a follow up of 96046f5#commitcomment-20417415 . otherwise i will revert #12854.
actually, i tend to put find_package(dpdk 2.0 REQUIRED)
instead, to be consistent with 1844b72#diff-3b85bae027325c1a3d65ac2b5d316239L32 .
currently dpdk is updated to 16.11, why still 16.07? |
@optimistyzy again, please note that user could install older version of dpdk and |
Signed-off-by: Kefu Chai <kchai@redhat.com>
PerfCountersBuilder::create_perf_counters() requires that the added counter ids are consecutive without counters with PERFCOUNTER_NONE type in it, so remove them from the `enum` definition if not used. this change reverts 96046f5 Signed-off-by: Kefu Chai <kchai@redhat.com>
@yuyuyu101 comments addressed and repushed. @liupan1111 does this address the crash you ran into before? |
briefly it's ok to me, although I know little in cmake... @liupan1111 does this solve problem? |
@liupan1111 ping? |
@tchaikov @yuyuyu101 , one question, with these changes in this PR, will the core dump in 12854 happen again when we use dpdk 16.11? |
@tchaikov hmm, I think we don't need to have version condition since we have fixed submodule. we only care the submodule version compatible with current dpdk codes |
i don't think so, that's why i was asking you to confirm. |
@tchaikov , ok, let me confirm and let you know |
@tchaikov, I tried your changes, and it works, no assertion failure. Thanks. But I still think it is better to remove version condition. I could give another pr to do some cleanup later. |
No description provided.