diff mbox series

[net-next,v1] flow_offload: improve extack msg for user when adding invalid filter

Message ID 1646045055-3784-1-git-send-email-baowen.zheng@corigine.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [net-next,v1] flow_offload: improve extack msg for user when adding invalid filter | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 2 this patch: 2
netdev/cc_maintainers warning 1 maintainers not CCed: jiri@resnulli.us
netdev/build_clang success Errors and warnings before: 18 this patch: 18
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 7 this patch: 7
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 8 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Baowen Zheng Feb. 28, 2022, 10:44 a.m. UTC
Add extack message to return exact message to user when adding invalid
filter with conflict flags for TC action.

In previous implement we just return EINVAL which is confusing for user.

Signed-off-by: Baowen Zheng <baowen.zheng@corigine.com>
---
 net/sched/act_api.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Roi Dayan Feb. 28, 2022, 1:27 p.m. UTC | #1
On 2022-02-28 12:44 PM, Baowen Zheng wrote:
> Add extack message to return exact message to user when adding invalid
> filter with conflict flags for TC action.
> 
> In previous implement we just return EINVAL which is confusing for user.
> 
> Signed-off-by: Baowen Zheng <baowen.zheng@corigine.com>
> ---
>   net/sched/act_api.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/net/sched/act_api.c b/net/sched/act_api.c
> index ca03e72..eb0d7bd 100644
> --- a/net/sched/act_api.c
> +++ b/net/sched/act_api.c
> @@ -1446,6 +1446,8 @@ int tcf_action_init(struct net *net, struct tcf_proto *tp, struct nlattr *nla,
>   				continue;
>   			if (skip_sw != tc_act_skip_sw(act->tcfa_flags) ||
>   			    skip_hw != tc_act_skip_hw(act->tcfa_flags)) {
> +				NL_SET_ERR_MSG(extack,
> +					       "Conflict occurs for TC action and filter flags");
>   				err = -EINVAL;
>   				goto err;
>   			}

Reviewed-by: Roi Dayan <roid@nvidia.com>
Jakub Kicinski March 2, 2022, 1:55 a.m. UTC | #2
On Mon, 28 Feb 2022 18:44:15 +0800 Baowen Zheng wrote:
> Add extack message to return exact message to user when adding invalid
> filter with conflict flags for TC action.
> 
> In previous implement we just return EINVAL which is confusing for user.
> 
> Signed-off-by: Baowen Zheng <baowen.zheng@corigine.com>
> ---
>  net/sched/act_api.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/net/sched/act_api.c b/net/sched/act_api.c
> index ca03e72..eb0d7bd 100644
> --- a/net/sched/act_api.c
> +++ b/net/sched/act_api.c
> @@ -1446,6 +1446,8 @@ int tcf_action_init(struct net *net, struct tcf_proto *tp, struct nlattr *nla,
>  				continue;
>  			if (skip_sw != tc_act_skip_sw(act->tcfa_flags) ||
>  			    skip_hw != tc_act_skip_hw(act->tcfa_flags)) {
> +				NL_SET_ERR_MSG(extack,
> +					       "Conflict occurs for TC action and filter flags");

Good improvement but I think we can reword a little, how about:

"Mismatch between action and filter offload flags" ?

>  				err = -EINVAL;
>  				goto err;
>  			}
Baowen Zheng March 2, 2022, 2:08 a.m. UTC | #3
On Wednesday, March 2, 2022 9:56 AM, Jakub wrote:
>On Mon, 28 Feb 2022 18:44:15 +0800 Baowen Zheng wrote:
>> Add extack message to return exact message to user when adding invalid
>> filter with conflict flags for TC action.
>>
>> In previous implement we just return EINVAL which is confusing for user.
>>
>> Signed-off-by: Baowen Zheng <baowen.zheng@corigine.com>
>> ---
>>  net/sched/act_api.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/net/sched/act_api.c b/net/sched/act_api.c index
>> ca03e72..eb0d7bd 100644
>> --- a/net/sched/act_api.c
>> +++ b/net/sched/act_api.c
>> @@ -1446,6 +1446,8 @@ int tcf_action_init(struct net *net, struct tcf_proto
>*tp, struct nlattr *nla,
>>  				continue;
>>  			if (skip_sw != tc_act_skip_sw(act->tcfa_flags) ||
>>  			    skip_hw != tc_act_skip_hw(act->tcfa_flags)) {
>> +				NL_SET_ERR_MSG(extack,
>> +					       "Conflict occurs for TC action and
>filter flags");
>
>Good improvement but I think we can reword a little, how about:
>
>"Mismatch between action and filter offload flags" ?
Thanks Jakub for your suggestion, to be honest I did not find a best way to make the msg more understandable. 
I think your description is more clear than mine. I will change as your advice.
>
>>  				err = -EINVAL;
>>  				goto err;
>>  			}
diff mbox series

Patch

diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index ca03e72..eb0d7bd 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -1446,6 +1446,8 @@  int tcf_action_init(struct net *net, struct tcf_proto *tp, struct nlattr *nla,
 				continue;
 			if (skip_sw != tc_act_skip_sw(act->tcfa_flags) ||
 			    skip_hw != tc_act_skip_hw(act->tcfa_flags)) {
+				NL_SET_ERR_MSG(extack,
+					       "Conflict occurs for TC action and filter flags");
 				err = -EINVAL;
 				goto err;
 			}