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

[PATCH 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_ACT_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

[PATCH 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

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

[Jamal Hadi Salim]

On Tue, Mar 14, 2023 at 2:58 AM 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>

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

cheers,
jamal


> ---
>  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
>

[PATCH 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_ACT_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>
---
 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..5ad3448a1fa7 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_ACT_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..f960cb534ca0 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_ACT_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: [PATCH net 2/2] net/sched: act_api: add specific EXT_WARN_MSG for tc action

[Jamal Hadi Salim]

On Tue, Mar 14, 2023 at 2:58 AM 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_ACT_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>
> ---
>  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..5ad3448a1fa7 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_ACT_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..f960cb534ca0 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_ACT_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;

Sorry, only thing i should have mentioned earlier - not clear from here:
Do you get two ext warns now in the same netlink message? One for the
action and one for the cls?
Something to check:
on terminal1 > tc monitor
on terminal2 > run a command which will get the offload to fail and
see what response you get

My concern is you may be getting two warnings in one message.

cheers,
jamal
> --
> 2.38.1
>

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

[Hangbin Liu]

On Tue, Mar 14, 2023 at 06:35:29PM -0400, Jamal Hadi Salim wrote:
> Sorry, only thing i should have mentioned earlier - not clear from here:
> Do you get two ext warns now in the same netlink message? One for the
> action and one for the cls?
> Something to check:
> on terminal1 > tc monitor
> on terminal2 > run a command which will get the offload to fail and
> see what response you get
> 
> My concern is you may be getting two warnings in one message.

From the result we only got 1 warning message.

# tc qdisc add dev enp4s0f0np0 ingress
# tc filter add dev enp4s0f0np0 ingress flower verbose ct_state +trk+new action drop
Warning: mlx5_core: matching on ct_state +new isn't supported.

# tc monitor
qdisc ingress ffff: dev enp4s0f0np0 parent ffff:fff1 ----------------
added chain dev enp4s0f0np0 parent ffff: chain 0
added filter dev enp4s0f0np0 ingress protocol all pref 49152 flower chain 0 handle 0x1
  ct_state +trk+new
  not_in_hw
        action order 1: gact action drop
         random type none pass val 0
         index 1 ref 1 bind 1

mlx5_core: matching on ct_state +new isn't supported
^C

Thanks
Hangbin

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

[Jamal Hadi Salim]

On Wed, Mar 15, 2023 at 5:47 AM Hangbin Liu <liuhangbin@gmail.com> wrote:
>
> On Tue, Mar 14, 2023 at 06:35:29PM -0400, Jamal Hadi Salim wrote:
> > Sorry, only thing i should have mentioned earlier - not clear from here:
> > Do you get two ext warns now in the same netlink message? One for the
> > action and one for the cls?
> > Something to check:
> > on terminal1 > tc monitor
> > on terminal2 > run a command which will get the offload to fail and
> > see what response you get
> >
> > My concern is you may be getting two warnings in one message.
>
> From the result we only got 1 warning message.
>
> # tc qdisc add dev enp4s0f0np0 ingress
> # tc filter add dev enp4s0f0np0 ingress flower verbose ct_state +trk+new action drop
> Warning: mlx5_core: matching on ct_state +new isn't supported.
>
> # tc monitor
> qdisc ingress ffff: dev enp4s0f0np0 parent ffff:fff1 ----------------
> added chain dev enp4s0f0np0 parent ffff: chain 0
> added filter dev enp4s0f0np0 ingress protocol all pref 49152 flower chain 0 handle 0x1
>   ct_state +trk+new
>   not_in_hw
>         action order 1: gact action drop
>          random type none pass val 0
>          index 1 ref 1 bind 1
>
> mlx5_core: matching on ct_state +new isn't supported
> ^C

Thanks for checking. I was worried from the quick glance that you will
end up calling the action code with extack from cls and that the
warning will be duplicated.

cheers,
jamal

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

[Hangbin Liu]

On Wed, Mar 15, 2023 at 02:49:43PM -0400, Jamal Hadi Salim wrote:
> On Wed, Mar 15, 2023 at 5:47 AM Hangbin Liu <liuhangbin@gmail.com> wrote:
> >
> > On Tue, Mar 14, 2023 at 06:35:29PM -0400, Jamal Hadi Salim wrote:
> > > Sorry, only thing i should have mentioned earlier - not clear from here:
> > > Do you get two ext warns now in the same netlink message? One for the
> > > action and one for the cls?
> > > Something to check:
> > > on terminal1 > tc monitor
> > > on terminal2 > run a command which will get the offload to fail and
> > > see what response you get
> > >
> > > My concern is you may be getting two warnings in one message.
> >
> > From the result we only got 1 warning message.
> >
> > # tc qdisc add dev enp4s0f0np0 ingress
> > # tc filter add dev enp4s0f0np0 ingress flower verbose ct_state +trk+new action drop
> > Warning: mlx5_core: matching on ct_state +new isn't supported.
> >
> > # tc monitor
> > qdisc ingress ffff: dev enp4s0f0np0 parent ffff:fff1 ----------------
> > added chain dev enp4s0f0np0 parent ffff: chain 0
> > added filter dev enp4s0f0np0 ingress protocol all pref 49152 flower chain 0 handle 0x1
> >   ct_state +trk+new
> >   not_in_hw
> >         action order 1: gact action drop
> >          random type none pass val 0
> >          index 1 ref 1 bind 1
> >
> > mlx5_core: matching on ct_state +new isn't supported
> > ^C
> 
> Thanks for checking. I was worried from the quick glance that you will
> end up calling the action code with extack from cls and that the
> warning will be duplicated.

The action info should be filled via dump function, which will not call
tca_get_fill(). So I think it should be safe. Please correct me if I missed
anything.

Thanks
Hangbin

Re: [PATCH 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:33 PM Hangbin Liu <liuhangbin@gmail.com> wrote:
>
> On Wed, Mar 15, 2023 at 02:49:43PM -0400, Jamal Hadi Salim wrote:
> > On Wed, Mar 15, 2023 at 5:47 AM Hangbin Liu <liuhangbin@gmail.com> wrote:
> > >
> > > On Tue, Mar 14, 2023 at 06:35:29PM -0400, Jamal Hadi Salim wrote:
> > > > Sorry, only thing i should have mentioned earlier - not clear from here:
> > > > Do you get two ext warns now in the same netlink message? One for the
> > > > action and one for the cls?
> > > > Something to check:
> > > > on terminal1 > tc monitor
> > > > on terminal2 > run a command which will get the offload to fail and
> > > > see what response you get
> > > >
> > > > My concern is you may be getting two warnings in one message.
> > >
> > > From the result we only got 1 warning message.
> > >
> > > # tc qdisc add dev enp4s0f0np0 ingress
> > > # tc filter add dev enp4s0f0np0 ingress flower verbose ct_state +trk+new action drop
> > > Warning: mlx5_core: matching on ct_state +new isn't supported.
> > >
> > > # tc monitor
> > > qdisc ingress ffff: dev enp4s0f0np0 parent ffff:fff1 ----------------
> > > added chain dev enp4s0f0np0 parent ffff: chain 0
> > > added filter dev enp4s0f0np0 ingress protocol all pref 49152 flower chain 0 handle 0x1
> > >   ct_state +trk+new
> > >   not_in_hw
> > >         action order 1: gact action drop
> > >          random type none pass val 0
> > >          index 1 ref 1 bind 1
> > >
> > > mlx5_core: matching on ct_state +new isn't supported
> > > ^C
> >
> > Thanks for checking. I was worried from the quick glance that you will
> > end up calling the action code with extack from cls and that the
> > warning will be duplicated.
>
> The action info should be filled via dump function, which will not call
> tca_get_fill(). So I think it should be safe. Please correct me if I missed
> anything.


Right - for a similar scenario, it will only be called when you
offload an action independent of the filter.

cheers,
jamal

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

[Jakub Kicinski]

On Tue, 14 Mar 2023 14:58:02 +0800 Hangbin Liu wrote:
> --- 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_ACT_EXT_WARN_MSG,

Not TCA_ROOT_EXT_... ?
All other attrs in this set are called TCA_ROOT_x

>  	__TCA_ROOT_MAX,
>  #define	TCA_ROOT_MAX (__TCA_ROOT_MAX - 1)

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

[Hangbin Liu]

On Wed, Mar 15, 2023 at 12:45:32AM -0700, Jakub Kicinski wrote:
> On Tue, 14 Mar 2023 14:58:02 +0800 Hangbin Liu wrote:
> > --- 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_ACT_EXT_WARN_MSG,
> 
> Not TCA_ROOT_EXT_... ?
> All other attrs in this set are called TCA_ROOT_x

Hmm, when we discussed this issue, Jamal suggested to use TCAA_EXT_WARN_MSG.
I expand it to TCA_ACT_EXT_WARN_MSG to correspond with the format of TCA_*.
But your suggest TCA_ROOT_EXT_ also makes sense. I'm OK to change it.

Jamal, what do you think?

Thanks
Hangbin

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

[Jamal Hadi Salim]

On Wed, Mar 15, 2023 at 5:40 AM Hangbin Liu <liuhangbin@gmail.com> wrote:
>
> On Wed, Mar 15, 2023 at 12:45:32AM -0700, Jakub Kicinski wrote:
> > On Tue, 14 Mar 2023 14:58:02 +0800 Hangbin Liu wrote:
> > > --- 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_ACT_EXT_WARN_MSG,
> >
> > Not TCA_ROOT_EXT_... ?
> > All other attrs in this set are called TCA_ROOT_x
>
> Hmm, when we discussed this issue, Jamal suggested to use TCAA_EXT_WARN_MSG.
> I expand it to TCA_ACT_EXT_WARN_MSG to correspond with the format of TCA_*.
> But your suggest TCA_ROOT_EXT_ also makes sense. I'm OK to change it.
>
> Jamal, what do you think?

Yeah, sticking to the TCA_ROOT_ prefixes makes it neater. TCA_ROOT_EXT_WARN_MSG?

cheers,
jamal

[PATCH iproute2 1/2] Revert "tc: m_action: fix parsing of TCA_EXT_WARN_MSG"

[Hangbin Liu]

This reverts commit 70b9ebae63ce7e6f9911bdfbcf47a6d18f24159a.

The TCA_EXT_WARN_MSG is not sit within the TCA_ACT_TAB hierarchy. It's
belong to the TCA_MAX namespace. I will fix the issue in another patch.

Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 tc/m_action.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tc/m_action.c b/tc/m_action.c
index 6c91af2c..0400132c 100644
--- a/tc/m_action.c
+++ b/tc/m_action.c
@@ -586,7 +586,7 @@ int print_action(struct nlmsghdr *n, void *arg)
 
 	open_json_object(NULL);
 	tc_dump_action(fp, tb[TCA_ACT_TAB], tot_acts ? *tot_acts:0, false);
-	print_ext_msg(&tb[TCA_ACT_TAB]);
+	print_ext_msg(tb);
 	close_json_object();
 
 	return 0;
-- 
2.38.1

Re: [PATCH iproute2 1/2] Revert "tc: m_action: fix parsing of TCA_EXT_WARN_MSG"

[Andrea Claudi]

On Tue, Mar 14, 2023 at 03:04:49PM +0800, Hangbin Liu wrote:
> This reverts commit 70b9ebae63ce7e6f9911bdfbcf47a6d18f24159a.
> 
> The TCA_EXT_WARN_MSG is not sit within the TCA_ACT_TAB hierarchy. It's
> belong to the TCA_MAX namespace. I will fix the issue in another patch.
> 
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> ---
>  tc/m_action.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tc/m_action.c b/tc/m_action.c
> index 6c91af2c..0400132c 100644
> --- a/tc/m_action.c
> +++ b/tc/m_action.c
> @@ -586,7 +586,7 @@ int print_action(struct nlmsghdr *n, void *arg)
>  
>  	open_json_object(NULL);
>  	tc_dump_action(fp, tb[TCA_ACT_TAB], tot_acts ? *tot_acts:0, false);
> -	print_ext_msg(&tb[TCA_ACT_TAB]);
> +	print_ext_msg(tb);
>  	close_json_object();
>  
>  	return 0;
> -- 
> 2.38.1
>

As this patchset misses the cover letter, this covers patch 2/2 as
well.

Reviewed-by: Andrea Claudi <aclaudi@redhat.com>

Re: [PATCH iproute2 1/2] Revert "tc: m_action: fix parsing of TCA_EXT_WARN_MSG"

[Jakub Kicinski]

On Tue, 14 Mar 2023 15:04:49 +0800 Hangbin Liu wrote:
> This reverts commit 70b9ebae63ce7e6f9911bdfbcf47a6d18f24159a.
> 
> The TCA_EXT_WARN_MSG is not sit within the TCA_ACT_TAB hierarchy. It's
> belong to the TCA_MAX namespace. I will fix the issue in another patch.
> 
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>

Double check the posting format if it's not just a slip up:
https://www.kernel.org/doc/html/next/process/maintainer-netdev.html#co-posting-changes-to-user-space-components

Re: [PATCH iproute2 1/2] Revert "tc: m_action: fix parsing of TCA_EXT_WARN_MSG"

[Hangbin Liu]

On Wed, Mar 15, 2023 at 12:46:45AM -0700, Jakub Kicinski wrote:
> On Tue, 14 Mar 2023 15:04:49 +0800 Hangbin Liu wrote:
> > This reverts commit 70b9ebae63ce7e6f9911bdfbcf47a6d18f24159a.
> > 
> > The TCA_EXT_WARN_MSG is not sit within the TCA_ACT_TAB hierarchy. It's
> > belong to the TCA_MAX namespace. I will fix the issue in another patch.
> > 
> > Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> 
> Double check the posting format if it's not just a slip up:
> https://www.kernel.org/doc/html/next/process/maintainer-netdev.html#co-posting-changes-to-user-space-components

Thanks, I will take care of this in the future.

Hangbin

[PATCH iproute2 2/2] tc: m_action: fix parsing of TCA_EXT_WARN_MSG by using different enum

[Hangbin Liu]

We can't use TCA_EXT_WARN_MSG directly in tc action as it's using different
enum with filter. Let's use a new TCA_ACT_EXT_WARN_MSG for tc action
specifically.

Fixes: 6035995665b7 ("tc: add new attr TCA_EXT_WARN_MSG")
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 include/uapi/linux/rtnetlink.h | 1 +
 tc/m_action.c                  | 7 ++++++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
index 217b25b9..f598c78a 100644
--- a/include/uapi/linux/rtnetlink.h
+++ b/include/uapi/linux/rtnetlink.h
@@ -787,6 +787,7 @@ enum {
 	TCA_ROOT_FLAGS,
 	TCA_ROOT_COUNT,
 	TCA_ROOT_TIME_DELTA, /* in msecs */
+	TCA_ACT_EXT_WARN_MSG,
 	__TCA_ROOT_MAX,
 #define	TCA_ROOT_MAX (__TCA_ROOT_MAX - 1)
 };
diff --git a/tc/m_action.c b/tc/m_action.c
index 0400132c..f99d1170 100644
--- a/tc/m_action.c
+++ b/tc/m_action.c
@@ -586,7 +586,12 @@ int print_action(struct nlmsghdr *n, void *arg)
 
 	open_json_object(NULL);
 	tc_dump_action(fp, tb[TCA_ACT_TAB], tot_acts ? *tot_acts:0, false);
-	print_ext_msg(tb);
+
+	if (tb[TCA_ACT_EXT_WARN_MSG]) {
+		print_string(PRINT_ANY, "warn", "%s", rta_getattr_str(tb[TCA_ACT_EXT_WARN_MSG]));
+		print_nl();
+	}
+
 	close_json_object();
 
 	return 0;
-- 
2.38.1

Re: [PATCH iproute2 2/2] tc: m_action: fix parsing of TCA_EXT_WARN_MSG by using different enum

[Davide Caratti]

On Tue, Mar 14, 2023 at 03:08:41PM +0800, Hangbin Liu wrote:
> We can't use TCA_EXT_WARN_MSG directly in tc action as it's using different
> enum with filter. Let's use a new TCA_ACT_EXT_WARN_MSG for tc action
> specifically.
> 
> Fixes: 6035995665b7 ("tc: add new attr TCA_EXT_WARN_MSG")
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>

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

thanks!
-- 
davide

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

[Davide Caratti]

On Tue, Mar 14, 2023 at 02:58:00PM +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.

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

thanks!
-- 
davide

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

[Hangbin Liu]

On Tue, Mar 14, 2023 at 10:20:24AM +0100, Davide Caratti wrote:
> On Tue, Mar 14, 2023 at 02:58:00PM +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.
> 
> Reported-and-tested-by: Davide Caratti <dcaratti@redhat.com>

Thanks Davide, I forgot to add the Reported-by flag.

Hangbin