[PATCHv2 net 0/2] net/sched: fix parsing of TCA_EXT_WARN_MSG for tc action

[PATCHv2 net 0/2] net/sched: fix parsing of TCA_EXT_WARN_MSG for tc action

[Hangbin Liu]

In my previous commit 0349b8779cc9 ("sched: add new attr TCA_EXT_WARN_MSG
to report tc extact message") I didn't notice the tc action use different
enum with filter. So we can't use TCA_EXT_WARN_MSG directly for tc action.

Let's rever the previous fix 923b2e30dc9c ("net/sched: act_api: move
TCA_EXT_WARN_MSG to the correct hierarchy") and add a new
TCA_ROOT_EXT_WARN_MSG for tc action specifically.

Here is the tdc test result:

1..1119
ok 1 d959 - Add cBPF action with valid bytecode
ok 2 f84a - Add cBPF action with invalid bytecode
ok 3 e939 - Add eBPF action with valid object-file
ok 4 282d - Add eBPF action with invalid object-file
ok 5 d819 - Replace cBPF bytecode and action control
ok 6 6ae3 - Delete cBPF action
ok 7 3e0d - List cBPF actions
ok 8 55ce - Flush BPF actions
ok 9 ccc3 - Add cBPF action with duplicate index
ok 10 89c7 - Add cBPF action with invalid index
[...]
ok 1115 2348 - Show TBF class
ok 1116 84a0 - Create TEQL with default setting
ok 1117 7734 - Create TEQL with multiple device
ok 1118 34a9 - Delete TEQL with valid handle
ok 1119 6289 - Show TEQL stats

Hangbin Liu (2):
  Revert "net/sched: act_api: move TCA_EXT_WARN_MSG to the correct
    hierarchy"
  net/sched: act_api: add specific EXT_WARN_MSG for tc action

 include/uapi/linux/rtnetlink.h | 1 +
 net/sched/act_api.c            | 8 ++++----
 2 files changed, 5 insertions(+), 4 deletions(-)

-- 
2.38.1

[PATCHv2 net 1/2] Revert "net/sched: act_api: move TCA_EXT_WARN_MSG to the correct hierarchy"

[Hangbin Liu]

This reverts commit 923b2e30dc9cd05931da0f64e2e23d040865c035.

This is not a correct fix as TCA_EXT_WARN_MSG is not a hierarchy to
TCA_ACT_TAB. I didn't notice the TC actions use different enum when adding
TCA_EXT_WARN_MSG. To fix the difference I will add a new WARN enum in
TCA_ROOT_MAX as Jamal suggested.

Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 net/sched/act_api.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 34c508675041..fce522886099 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -1596,12 +1596,12 @@ static int tca_get_fill(struct sk_buff *skb, struct tc_action *actions[],
 	if (tcf_action_dump(skb, actions, bind, ref, false) < 0)
 		goto out_nlmsg_trim;
 
+	nla_nest_end(skb, nest);
+
 	if (extack && extack->_msg &&
 	    nla_put_string(skb, TCA_EXT_WARN_MSG, extack->_msg))
 		goto out_nlmsg_trim;
 
-	nla_nest_end(skb, nest);
-
 	nlh->nlmsg_len = skb_tail_pointer(skb) - b;
 
 	return skb->len;
-- 
2.38.1

[PATCHv2 net 2/2] net/sched: act_api: add specific EXT_WARN_MSG for tc action

[Hangbin Liu]

In my previous commit 0349b8779cc9 ("sched: add new attr TCA_EXT_WARN_MSG
to report tc extact message") I didn't notice the tc action use different
enum with filter. So we can't use TCA_EXT_WARN_MSG directly for tc action.
Let's add a TCA_ROOT_EXT_WARN_MSG for tc action specifically and put this
param before going to the TCA_ACT_TAB nest.

Fixes: 0349b8779cc9 ("sched: add new attr TCA_EXT_WARN_MSG to report tc extact message")
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
v2: rename TCA_ACT_EXT_WARN_MSG to TCA_ROOT_EXT_WARN_MSG
---
 include/uapi/linux/rtnetlink.h | 1 +
 net/sched/act_api.c            | 8 ++++----
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
index 25a0af57dd5e..51c13cf9c5ae 100644
--- a/include/uapi/linux/rtnetlink.h
+++ b/include/uapi/linux/rtnetlink.h
@@ -789,6 +789,7 @@ enum {
 	TCA_ROOT_FLAGS,
 	TCA_ROOT_COUNT,
 	TCA_ROOT_TIME_DELTA, /* in msecs */
+	TCA_ROOT_EXT_WARN_MSG,
 	__TCA_ROOT_MAX,
 #define	TCA_ROOT_MAX (__TCA_ROOT_MAX - 1)
 };
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index fce522886099..296fc1afedd8 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -1589,6 +1589,10 @@ static int tca_get_fill(struct sk_buff *skb, struct tc_action *actions[],
 	t->tca__pad1 = 0;
 	t->tca__pad2 = 0;
 
+	if (extack && extack->_msg &&
+	    nla_put_string(skb, TCA_ROOT_EXT_WARN_MSG, extack->_msg))
+		goto out_nlmsg_trim;
+
 	nest = nla_nest_start_noflag(skb, TCA_ACT_TAB);
 	if (!nest)
 		goto out_nlmsg_trim;
@@ -1598,10 +1602,6 @@ static int tca_get_fill(struct sk_buff *skb, struct tc_action *actions[],
 
 	nla_nest_end(skb, nest);
 
-	if (extack && extack->_msg &&
-	    nla_put_string(skb, TCA_EXT_WARN_MSG, extack->_msg))
-		goto out_nlmsg_trim;
-
 	nlh->nlmsg_len = skb_tail_pointer(skb) - b;
 
 	return skb->len;
-- 
2.38.1

Re: [PATCHv2 net 0/2] net/sched: fix parsing of TCA_EXT_WARN_MSG for tc action

[Hangbin Liu]

On Thu, Mar 16, 2023 at 11:37:51AM +0800, Hangbin Liu wrote:
> In my previous commit 0349b8779cc9 ("sched: add new attr TCA_EXT_WARN_MSG
> to report tc extact message") I didn't notice the tc action use different
> enum with filter. So we can't use TCA_EXT_WARN_MSG directly for tc action.
> 
> Let's rever the previous fix 923b2e30dc9c ("net/sched: act_api: move
> TCA_EXT_WARN_MSG to the correct hierarchy") and add a new
> TCA_ROOT_EXT_WARN_MSG for tc action specifically.

Sigh. Sorry I sent the mail too quick and forgot to add

Reported-and-tested-by: Davide Caratti <dcaratti@redhat.com>

Re: [PATCHv2 net 1/2] Revert "net/sched: act_api: move TCA_EXT_WARN_MSG to the correct hierarchy"

[Jamal Hadi Salim]

On Wed, Mar 15, 2023 at 11:38 PM Hangbin Liu <liuhangbin@gmail.com> wrote:
>
> This reverts commit 923b2e30dc9cd05931da0f64e2e23d040865c035.
>
> This is not a correct fix as TCA_EXT_WARN_MSG is not a hierarchy to
> TCA_ACT_TAB. I didn't notice the TC actions use different enum when adding
> TCA_EXT_WARN_MSG. To fix the difference I will add a new WARN enum in
> TCA_ROOT_MAX as Jamal suggested.
>
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> ---
>  net/sched/act_api.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/sched/act_api.c b/net/sched/act_api.c
> index 34c508675041..fce522886099 100644
> --- a/net/sched/act_api.c
> +++ b/net/sched/act_api.c
> @@ -1596,12 +1596,12 @@ static int tca_get_fill(struct sk_buff *skb, struct tc_action *actions[],
>         if (tcf_action_dump(skb, actions, bind, ref, false) < 0)
>                 goto out_nlmsg_trim;
>
> +       nla_nest_end(skb, nest);
> +
>         if (extack && extack->_msg &&
>             nla_put_string(skb, TCA_EXT_WARN_MSG, extack->_msg))
>                 goto out_nlmsg_trim;
>
> -       nla_nest_end(skb, nest);
> -
>         nlh->nlmsg_len = skb_tail_pointer(skb) - b;
>
>         return skb->len;

Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>

cheers,
jamal

> 2.38.1
>

Re: [PATCHv2 net 2/2] net/sched: act_api: add specific EXT_WARN_MSG for tc action

[Jamal Hadi Salim]

On Wed, Mar 15, 2023 at 11:38 PM Hangbin Liu <liuhangbin@gmail.com> wrote:
>
> In my previous commit 0349b8779cc9 ("sched: add new attr TCA_EXT_WARN_MSG
> to report tc extact message") I didn't notice the tc action use different
> enum with filter. So we can't use TCA_EXT_WARN_MSG directly for tc action.
> Let's add a TCA_ROOT_EXT_WARN_MSG for tc action specifically and put this
> param before going to the TCA_ACT_TAB nest.
>
> Fixes: 0349b8779cc9 ("sched: add new attr TCA_EXT_WARN_MSG to report tc extact message")
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>

Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>

cheers,
jamal
> ---
> v2: rename TCA_ACT_EXT_WARN_MSG to TCA_ROOT_EXT_WARN_MSG
> ---
>  include/uapi/linux/rtnetlink.h | 1 +
>  net/sched/act_api.c            | 8 ++++----
>  2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
> index 25a0af57dd5e..51c13cf9c5ae 100644
> --- a/include/uapi/linux/rtnetlink.h
> +++ b/include/uapi/linux/rtnetlink.h
> @@ -789,6 +789,7 @@ enum {
>         TCA_ROOT_FLAGS,
>         TCA_ROOT_COUNT,
>         TCA_ROOT_TIME_DELTA, /* in msecs */
> +       TCA_ROOT_EXT_WARN_MSG,
>         __TCA_ROOT_MAX,
>  #define        TCA_ROOT_MAX (__TCA_ROOT_MAX - 1)
>  };
> diff --git a/net/sched/act_api.c b/net/sched/act_api.c
> index fce522886099..296fc1afedd8 100644
> --- a/net/sched/act_api.c
> +++ b/net/sched/act_api.c
> @@ -1589,6 +1589,10 @@ static int tca_get_fill(struct sk_buff *skb, struct tc_action *actions[],
>         t->tca__pad1 = 0;
>         t->tca__pad2 = 0;
>
> +       if (extack && extack->_msg &&
> +           nla_put_string(skb, TCA_ROOT_EXT_WARN_MSG, extack->_msg))
> +               goto out_nlmsg_trim;
> +
>         nest = nla_nest_start_noflag(skb, TCA_ACT_TAB);
>         if (!nest)
>                 goto out_nlmsg_trim;
> @@ -1598,10 +1602,6 @@ static int tca_get_fill(struct sk_buff *skb, struct tc_action *actions[],
>
>         nla_nest_end(skb, nest);
>
> -       if (extack && extack->_msg &&
> -           nla_put_string(skb, TCA_EXT_WARN_MSG, extack->_msg))
> -               goto out_nlmsg_trim;
> -
>         nlh->nlmsg_len = skb_tail_pointer(skb) - b;
>
>         return skb->len;
> --
> 2.38.1
>

Re: [PATCHv2 net 0/2] net/sched: fix parsing of TCA_EXT_WARN_MSG for tc action

[Jamal Hadi Salim]

On Wed, Mar 15, 2023 at 11:55 PM Hangbin Liu <liuhangbin@gmail.com> wrote:
>
> On Thu, Mar 16, 2023 at 11:37:51AM +0800, Hangbin Liu wrote:
> > In my previous commit 0349b8779cc9 ("sched: add new attr TCA_EXT_WARN_MSG
> > to report tc extact message") I didn't notice the tc action use different
> > enum with filter. So we can't use TCA_EXT_WARN_MSG directly for tc action.
> >
> > Let's rever the previous fix 923b2e30dc9c ("net/sched: act_api: move
> > TCA_EXT_WARN_MSG to the correct hierarchy") and add a new
> > TCA_ROOT_EXT_WARN_MSG for tc action specifically.
>
> Sigh. Sorry I sent the mail too quick and forgot to add
>
> Reported-and-tested-by: Davide Caratti <dcaratti@redhat.com>

For next time: instead of saying in the commit message "suggested by
foo" specify it using "suggested-by: foo" semantics.

cheers,
jamal

Re: [PATCHv2 net 0/2] net/sched: fix parsing of TCA_EXT_WARN_MSG for tc action

[Hangbin Liu]

On Thu, Mar 16, 2023 at 05:45:28AM -0400, Jamal Hadi Salim wrote:
> On Wed, Mar 15, 2023 at 11:55 PM Hangbin Liu <liuhangbin@gmail.com> wrote:
> >
> > On Thu, Mar 16, 2023 at 11:37:51AM +0800, Hangbin Liu wrote:
> > > In my previous commit 0349b8779cc9 ("sched: add new attr TCA_EXT_WARN_MSG
> > > to report tc extact message") I didn't notice the tc action use different
> > > enum with filter. So we can't use TCA_EXT_WARN_MSG directly for tc action.
> > >
> > > Let's rever the previous fix 923b2e30dc9c ("net/sched: act_api: move
> > > TCA_EXT_WARN_MSG to the correct hierarchy") and add a new
> > > TCA_ROOT_EXT_WARN_MSG for tc action specifically.
> >
> > Sigh. Sorry I sent the mail too quick and forgot to add
> >
> > Reported-and-tested-by: Davide Caratti <dcaratti@redhat.com>
> 
> For next time: instead of saying in the commit message "suggested by
> foo" specify it using "suggested-by: foo" semantics.

Sure, I will

Thanks
Hangbin

Re: [PATCHv2 net 0/2] net/sched: fix parsing of TCA_EXT_WARN_MSG for tc action

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Thu, 16 Mar 2023 11:37:51 +0800 you wrote:
> In my previous commit 0349b8779cc9 ("sched: add new attr TCA_EXT_WARN_MSG
> to report tc extact message") I didn't notice the tc action use different
> enum with filter. So we can't use TCA_EXT_WARN_MSG directly for tc action.
> 
> Let's rever the previous fix 923b2e30dc9c ("net/sched: act_api: move
> TCA_EXT_WARN_MSG to the correct hierarchy") and add a new
> TCA_ROOT_EXT_WARN_MSG for tc action specifically.
> 
> [...]

Here is the summary with links:
  - [PATCHv2,net,1/2] Revert "net/sched: act_api: move TCA_EXT_WARN_MSG to the correct hierarchy"
    https://git.kernel.org/netdev/net/c/8de2bd02439e
  - [PATCHv2,net,2/2] net/sched: act_api: add specific EXT_WARN_MSG for tc action
    https://git.kernel.org/netdev/net/c/2f59823fe696

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html