Skip to content

[PATCH CATERPILLAR v3] pktio: honor config.h settings#394

Closed
joseppc wants to merge 1 commit intoOpenDataPlane:caterpillarfrom
joseppc:check-macro-value
Closed

[PATCH CATERPILLAR v3] pktio: honor config.h settings#394
joseppc wants to merge 1 commit intoOpenDataPlane:caterpillarfrom
joseppc:check-macro-value

Conversation

@joseppc
Copy link

@joseppc joseppc commented Jan 16, 2018

It's not enough to chech if a macro is defined to enable or disable a
pktio, we also need to check if it is set to 1 as per its description.

Signed-off-by: Josep Puigdemont josep.puigdemont@linaro.org

@joseppc joseppc requested a review from heyi-arm January 16, 2018 12:17
@muvarov muvarov changed the title pktio: honor config.h settings [PATCH CATERPILLAR v1] pktio: honor config.h settings Jan 16, 2018
@heyi-arm
Copy link

@Bill-Fischofer-Linaro do you like to have a review on this PR?

Copy link
Contributor

@Bill-Fischofer-Linaro Bill-Fischofer-Linaro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed-by: Bill Fischofer bill.fischofer@linaro.org

@heyi-arm
Copy link

@joseppc this PR needs rebase after Mykyta's driver PRs.

@muvarov muvarov changed the title [PATCH CATERPILLAR v1] pktio: honor config.h settings [PATCH CATERPILLAR v2] pktio: honor config.h settings Jan 18, 2018
@joseppc
Copy link
Author

joseppc commented Jan 18, 2018

I rebased the PR and updated mdev pktio files too.

@heyi-arm
Copy link

@joseppc sorry Josep after I merged Mykyta's Chelsio driver this PR needs a rebase again, sorry about this inconvenience can you rebase again and I'll merge it right after.

It's not enough to chech if a macro is defined to enable or disable a
pktio, we also need to check if it is set to 1 to match the description
in config.h

Signed-off-by: Josep Puigdemont <josep.puigdemont@linaro.org>
@muvarov muvarov changed the title [PATCH CATERPILLAR v2] pktio: honor config.h settings [PATCH CATERPILLAR v3] pktio: honor config.h settings Jan 19, 2018
@joseppc
Copy link
Author

joseppc commented Jan 19, 2018

Rebased.
Instead of using this macros, I think we should use automake conditionals to include or exclude the files from the build, although I admit that it is very convenient to enable or disable a pktio just by redefining a macro in config.h instead of running the configure script again.

@heyi-arm
Copy link

Great, merged and closed.

@heyi-arm heyi-arm closed this Jan 19, 2018
@joseppc joseppc deleted the check-macro-value branch January 19, 2018 08:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants