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

route/act: allow full set of actions on gact,skbedit,mirred #319

Closed
wants to merge 1 commit into from

Conversation

voldymyr
Copy link
Contributor

Signed-off-by: Volodymyr Bendiuga volodymyr.bendiuga@westermo.com

@thom311
Copy link
Owner

thom311 commented May 27, 2022

This patch goes to the right direction. There is no need for the setter to do such error checking. The user anyway needs to take care to configure correct values, and whether a value is correct does not only depend on the hard-coded integer range but on things that the simple setter couldn't even handling. So, there is little point in rejecting some configuration, while other (invalid) configurations cannot be prevented. Worse, hardcoding the range means that libnl needs patches to support newer kernel features.

But there is no need to even check for if ((action < TC_ACT_UNSPEC) || (action > TC_ACT_VALUE_MAX)). We set internally an "int", just allow any value and let kernel tell us whether the value is correct.

Signed-off-by: Volodymyr Bendiuga <volodymyr.bendiuga@westermo.com>
@voldymyr
Copy link
Contributor Author

Please have a look, I've made changes and force-pushed. Thanks!

thom311 pushed a commit that referenced this pull request Jul 6, 2022
Signed-off-by: Volodymyr Bendiuga <volodymyr.bendiuga@westermo.com>

#319
@thom311
Copy link
Owner

thom311 commented Jul 6, 2022

merged. Thank you for the patch!!

@thom311 thom311 closed this Jul 6, 2022
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.

None yet

2 participants