mbox series

[SRU,F:linux-bluefield,v1,0/3] Support hardware stats for tc actions

Message ID 20230403223841.252583-1-witu@nvidia.com
Headers show
Series Support hardware stats for tc actions | expand

Message

William Tu April 3, 2023, 10:38 p.m. UTC
BugLink: https://bugs.launchpad.net/bugs/2015136

* Explain the bug(s)

tc hardware stats are not offloaded from some offloaded actions (pedit, sample, skbedit)

* brief explanation of fixes

Implement the necessary callbacks to report back stats for these actions.

* How to test

Configure two mlx5 rep devices and VFs.
Add tc offloaded tc rules with pedit action, and hit that rule with traffic

Ido Schimmel (1):
  sched: act_sample: Implement stats_update callback

Petr Machata (2):
  sched: act_skbedit: Implement stats_update callback
  sched: act_pedit: Implement stats_update callback

 net/sched/act_pedit.c   | 11 +++++++++++
 net/sched/act_sample.c  | 11 +++++++++++
 net/sched/act_skbedit.c | 11 +++++++++++
 3 files changed, 33 insertions(+)

Comments

Tim Gardner April 4, 2023, 12:19 p.m. UTC | #1
On 4/3/23 4:38 PM, William Tu wrote:
> BugLink: https://bugs.launchpad.net/bugs/2015136
> 
> * Explain the bug(s)
> 
> tc hardware stats are not offloaded from some offloaded actions (pedit, sample, skbedit)
> 
> * brief explanation of fixes
> 
> Implement the necessary callbacks to report back stats for these actions.
> 
> * How to test
> 
> Configure two mlx5 rep devices and VFs.
> Add tc offloaded tc rules with pedit action, and hit that rule with traffic
> 
> Ido Schimmel (1):
>    sched: act_sample: Implement stats_update callback
> 
> Petr Machata (2):
>    sched: act_skbedit: Implement stats_update callback
>    sched: act_pedit: Implement stats_update callback
> 
>   net/sched/act_pedit.c   | 11 +++++++++++
>   net/sched/act_sample.c  | 11 +++++++++++
>   net/sched/act_skbedit.c | 11 +++++++++++
>   3 files changed, 33 insertions(+)
> 
Acked-by: Tim Gardner <tim.gardner@canonical.com>
Bartlomiej Zolnierkiewicz April 5, 2023, 11:07 a.m. UTC | #2
Acked-by: Bartlomiej Zolnierkiewicz <bartlomiej.zolnierkiewicz@canonical.com>

On Tue, Apr 4, 2023 at 12:39 AM William Tu <witu@nvidia.com> wrote:
>
> BugLink: https://bugs.launchpad.net/bugs/2015136
>
> * Explain the bug(s)
>
> tc hardware stats are not offloaded from some offloaded actions (pedit, sample, skbedit)
>
> * brief explanation of fixes
>
> Implement the necessary callbacks to report back stats for these actions.
>
> * How to test
>
> Configure two mlx5 rep devices and VFs.
> Add tc offloaded tc rules with pedit action, and hit that rule with traffic
>
> Ido Schimmel (1):
>   sched: act_sample: Implement stats_update callback
>
> Petr Machata (2):
>   sched: act_skbedit: Implement stats_update callback
>   sched: act_pedit: Implement stats_update callback
>
>  net/sched/act_pedit.c   | 11 +++++++++++
>  net/sched/act_sample.c  | 11 +++++++++++
>  net/sched/act_skbedit.c | 11 +++++++++++
>  3 files changed, 33 insertions(+)
>
Bartlomiej Zolnierkiewicz April 5, 2023, 11:09 a.m. UTC | #3
Applied to focal:linux-bluefield/master-next. Thanks.

--
Best regards,
Bartlomiej

On Tue, Apr 4, 2023 at 12:39 AM William Tu <witu@nvidia.com> wrote:
>
> BugLink: https://bugs.launchpad.net/bugs/2015136
>
> * Explain the bug(s)
>
> tc hardware stats are not offloaded from some offloaded actions (pedit, sample, skbedit)
>
> * brief explanation of fixes
>
> Implement the necessary callbacks to report back stats for these actions.
>
> * How to test
>
> Configure two mlx5 rep devices and VFs.
> Add tc offloaded tc rules with pedit action, and hit that rule with traffic
>
> Ido Schimmel (1):
>   sched: act_sample: Implement stats_update callback
>
> Petr Machata (2):
>   sched: act_skbedit: Implement stats_update callback
>   sched: act_pedit: Implement stats_update callback
>
>  net/sched/act_pedit.c   | 11 +++++++++++
>  net/sched/act_sample.c  | 11 +++++++++++
>  net/sched/act_skbedit.c | 11 +++++++++++
>  3 files changed, 33 insertions(+)
>
William Tu April 10, 2023, 4:29 p.m. UTC | #4
Hi Bartlomiej,

Sorry! If it’s not too late, olease do not apply this series, we found compile error due to missing commit.
We’re working on the fix.
William

From: Bartlomiej Zolnierkiewicz <bartlomiej.zolnierkiewicz@canonical.com>
Date: Wednesday, April 5, 2023 at 4:09 AM
To: William Tu <witu@nvidia.com>
Cc: Ubuntu Kernel Team <kernel-team@lists.ubuntu.com>, Bodong Wang <bodong@nvidia.com>, Vladimir Sokolovsky <vlad@nvidia.com>, Paul Blakey <paulb@nvidia.com>, Dann Frazier <dann.frazier@canonical.com>
Subject: APPLIED: [SRU][F:linux-bluefield][PATCH v1 0/3] Support hardware stats for tc actions
External email: Use caution opening links or attachments


Applied to focal:linux-bluefield/master-next. Thanks.

--
Best regards,
Bartlomiej

On Tue, Apr 4, 2023 at 12:39 AM William Tu <witu@nvidia.com> wrote:
>
> BugLink: https://bugs.launchpad.net/bugs/2015136
>
> * Explain the bug(s)
>
> tc hardware stats are not offloaded from some offloaded actions (pedit, sample, skbedit)
>
> * brief explanation of fixes
>
> Implement the necessary callbacks to report back stats for these actions.
>
> * How to test
>
> Configure two mlx5 rep devices and VFs.
> Add tc offloaded tc rules with pedit action, and hit that rule with traffic
>
> Ido Schimmel (1):
>   sched: act_sample: Implement stats_update callback
>
> Petr Machata (2):
>   sched: act_skbedit: Implement stats_update callback
>   sched: act_pedit: Implement stats_update callback
>
>  net/sched/act_pedit.c   | 11 +++++++++++
>  net/sched/act_sample.c  | 11 +++++++++++
>  net/sched/act_skbedit.c | 11 +++++++++++
>  3 files changed, 33 insertions(+)
>
William Tu April 10, 2023, 4:44 p.m. UTC | #5
I found it’s already merged.
https://git.launchpad.net/~canonical-kernel/ubuntu/+source/linux-bluefield/+git/focal
I will send revert patch, thanks
William

From: William Tu <witu@nvidia.com>
Date: Monday, April 10, 2023 at 9:29 AM
To: Bartlomiej Zolnierkiewicz <bartlomiej.zolnierkiewicz@canonical.com>
Cc: Ubuntu Kernel Team <kernel-team@lists.ubuntu.com>, Bodong Wang <bodong@nvidia.com>, Vladimir Sokolovsky <vlad@nvidia.com>, Paul Blakey <paulb@nvidia.com>, Dann Frazier <dann.frazier@canonical.com>
Subject: Re: APPLIED: [SRU][F:linux-bluefield][PATCH v1 0/3] Support hardware stats for tc actions
Hi Bartlomiej,

Sorry! If it’s not too late, olease do not apply this series, we found compile error due to missing commit.
We’re working on the fix.
William

From: Bartlomiej Zolnierkiewicz <bartlomiej.zolnierkiewicz@canonical.com>
Date: Wednesday, April 5, 2023 at 4:09 AM
To: William Tu <witu@nvidia.com>
Cc: Ubuntu Kernel Team <kernel-team@lists.ubuntu.com>, Bodong Wang <bodong@nvidia.com>, Vladimir Sokolovsky <vlad@nvidia.com>, Paul Blakey <paulb@nvidia.com>, Dann Frazier <dann.frazier@canonical.com>
Subject: APPLIED: [SRU][F:linux-bluefield][PATCH v1 0/3] Support hardware stats for tc actions
External email: Use caution opening links or attachments


Applied to focal:linux-bluefield/master-next. Thanks.

--
Best regards,
Bartlomiej

On Tue, Apr 4, 2023 at 12:39 AM William Tu <witu@nvidia.com> wrote:
>
> BugLink: https://bugs.launchpad.net/bugs/2015136
>
> * Explain the bug(s)
>
> tc hardware stats are not offloaded from some offloaded actions (pedit, sample, skbedit)
>
> * brief explanation of fixes
>
> Implement the necessary callbacks to report back stats for these actions.
>
> * How to test
>
> Configure two mlx5 rep devices and VFs.
> Add tc offloaded tc rules with pedit action, and hit that rule with traffic
>
> Ido Schimmel (1):
>   sched: act_sample: Implement stats_update callback
>
> Petr Machata (2):
>   sched: act_skbedit: Implement stats_update callback
>   sched: act_pedit: Implement stats_update callback
>
>  net/sched/act_pedit.c   | 11 +++++++++++
>  net/sched/act_sample.c  | 11 +++++++++++
>  net/sched/act_skbedit.c | 11 +++++++++++
>  3 files changed, 33 insertions(+)
>
Vladimir Sokolovsky April 10, 2023, 4:44 p.m. UTC | #6
Hi,
This series was applied already as we tried to compile the “master-next” branch and it failed:

/workspace/kernel/linux/net/sched/act_sample.c:293:18: error: initialization of 'void (*)(struct tc_action *, u64,  u32,  u64,  bool)' {aka 'void (*)(struct tc_action *, long long unsigned int,  unsigned int,  long long unsigned int,  _Bool)'} from incompatible pointer type 'void (*)(struct tc_action *, u64,  u64,  u64,  u64,  bool)' {aka 'void (*)(struct tc_action *, long long unsigned int,  long long unsigned int,  long long unsigned int,  long long unsigned int,  _Bool)'} [-Werror=incompatible-pointer-types]
  293 |  .stats_update = tcf_sample_stats_update,
      |                  ^~~~~~~~~~~~~~~~~~~~~~~
/workspace/kernel/linux/net/sched/act_sample.c:293:18: note: (near initialization for 'act_sample_ops.stats_update')


cc1: some warnings being treated as errors



make[4]: *** [/workspace/kernel/linux/scripts/Makefile.build:272: net/sched/act_sample.o] Error 1

make[4]: *** Waiting for unfinished jobs....


Bartlomiej,
Do you have some sort of CI that can catch these issues?

Regards,
Vladimir


From: William Tu <witu@nvidia.com>
Sent: Monday, April 10, 2023 11:30 AM
To: Bartlomiej Zolnierkiewicz <bartlomiej.zolnierkiewicz@canonical.com>
Cc: Ubuntu Kernel Team <kernel-team@lists.ubuntu.com>; Bodong Wang <bodong@nvidia.com>; Vladimir Sokolovsky <vlad@nvidia.com>; Paul Blakey <paulb@nvidia.com>; Dann Frazier <dann.frazier@canonical.com>
Subject: Re: APPLIED: [SRU][F:linux-bluefield][PATCH v1 0/3] Support hardware stats for tc actions

Hi Bartlomiej,

Sorry! If it’s not too late, olease do not apply this series, we found compile error due to missing commit.
We’re working on the fix.
William

From: Bartlomiej Zolnierkiewicz <bartlomiej.zolnierkiewicz@canonical.com<mailto:bartlomiej.zolnierkiewicz@canonical.com>>
Date: Wednesday, April 5, 2023 at 4:09 AM
To: William Tu <witu@nvidia.com<mailto:witu@nvidia.com>>
Cc: Ubuntu Kernel Team <kernel-team@lists.ubuntu.com<mailto:kernel-team@lists.ubuntu.com>>, Bodong Wang <bodong@nvidia.com<mailto:bodong@nvidia.com>>, Vladimir Sokolovsky <vlad@nvidia.com<mailto:vlad@nvidia.com>>, Paul Blakey <paulb@nvidia.com<mailto:paulb@nvidia.com>>, Dann Frazier <dann.frazier@canonical.com<mailto:dann.frazier@canonical.com>>
Subject: APPLIED: [SRU][F:linux-bluefield][PATCH v1 0/3] Support hardware stats for tc actions
External email: Use caution opening links or attachments


Applied to focal:linux-bluefield/master-next. Thanks.

--
Best regards,
Bartlomiej

On Tue, Apr 4, 2023 at 12:39 AM William Tu <witu@nvidia.com<mailto:witu@nvidia.com>> wrote:
>
> BugLink: https://bugs.launchpad.net/bugs/2015136
>
> * Explain the bug(s)
>
> tc hardware stats are not offloaded from some offloaded actions (pedit, sample, skbedit)
>
> * brief explanation of fixes
>
> Implement the necessary callbacks to report back stats for these actions.
>
> * How to test
>
> Configure two mlx5 rep devices and VFs.
> Add tc offloaded tc rules with pedit action, and hit that rule with traffic
>
> Ido Schimmel (1):
>   sched: act_sample: Implement stats_update callback
>
> Petr Machata (2):
>   sched: act_skbedit: Implement stats_update callback
>   sched: act_pedit: Implement stats_update callback
>
>  net/sched/act_pedit.c   | 11 +++++++++++
>  net/sched/act_sample.c  | 11 +++++++++++
>  net/sched/act_skbedit.c | 11 +++++++++++
>  3 files changed, 33 insertions(+)
>
Bartlomiej Zolnierkiewicz April 11, 2023, 10:01 a.m. UTC | #7
Hi Vladimir,
Hi William,

On Mon, Apr 10, 2023 at 6:44 PM Vladimir Sokolovsky <vlad@nvidia.com> wrote:
>
> Hi,
>
> This series was applied already as we tried to compile the “master-next” branch and it failed:
>
>
>
> /workspace/kernel/linux/net/sched/act_sample.c:293:18: error: initialization of 'void (*)(struct tc_action *, u64,  u32,  u64,  bool)' {aka 'void (*)(struct tc_action *, long long unsigned int,  unsigned int,  long long unsigned int,  _Bool)'} from incompatible pointer type 'void (*)(struct tc_action *, u64,  u64,  u64,  u64,  bool)' {aka 'void (*)(struct tc_action *, long long unsigned int,  long long unsigned int,  long long unsigned int,  long long unsigned int,  _Bool)'} [-Werror=incompatible-pointer-types]
>
>   293 |  .stats_update = tcf_sample_stats_update,
>
>       |                  ^~~~~~~~~~~~~~~~~~~~~~~
>
> /workspace/kernel/linux/net/sched/act_sample.c:293:18: note: (near initialization for 'act_sample_ops.stats_update')
>
>
>
> cc1: some warnings being treated as errors
>
>
>
>
>
> make[4]: *** [/workspace/kernel/linux/scripts/Makefile.build:272: net/sched/act_sample.o] Error 1
>
> make[4]: *** Waiting for unfinished jobs....

I've dropped the "[SRU][F:linux-bluefield][PATCH v1 0/3] Support
hardware stats for tc actions" patchset for now from the master-next
branch (by rebasing the branch).

> Bartlomiej,
>
> Do you have some sort of CI that can catch these issues?

We currently don't have CI for automatically build-testing all
submitted patches during review and I've been assuming that the
changes being submitted are actually tested but it doesn't seem to be
always the case (as some submissions don't apply or build). I'll from
now on apply+build test all submissions (I've used to skip it for
smaller, "obviously looking" ones) before acking and applying them.

--
Best regards,
Bartlomiej


> Regards,
>
> Vladimir
>
>
>
>
>
> From: William Tu <witu@nvidia.com>
> Sent: Monday, April 10, 2023 11:30 AM
> To: Bartlomiej Zolnierkiewicz <bartlomiej.zolnierkiewicz@canonical.com>
> Cc: Ubuntu Kernel Team <kernel-team@lists.ubuntu.com>; Bodong Wang <bodong@nvidia.com>; Vladimir Sokolovsky <vlad@nvidia.com>; Paul Blakey <paulb@nvidia.com>; Dann Frazier <dann.frazier@canonical.com>
> Subject: Re: APPLIED: [SRU][F:linux-bluefield][PATCH v1 0/3] Support hardware stats for tc actions
>
>
>
> Hi Bartlomiej,
>
>
>
> Sorry! If it’s not too late, olease do not apply this series, we found compile error due to missing commit.
>
> We’re working on the fix.
>
> William
>
>
>
> From: Bartlomiej Zolnierkiewicz <bartlomiej.zolnierkiewicz@canonical.com>
> Date: Wednesday, April 5, 2023 at 4:09 AM
> To: William Tu <witu@nvidia.com>
> Cc: Ubuntu Kernel Team <kernel-team@lists.ubuntu.com>, Bodong Wang <bodong@nvidia.com>, Vladimir Sokolovsky <vlad@nvidia.com>, Paul Blakey <paulb@nvidia.com>, Dann Frazier <dann.frazier@canonical.com>
> Subject: APPLIED: [SRU][F:linux-bluefield][PATCH v1 0/3] Support hardware stats for tc actions
>
> External email: Use caution opening links or attachments
>
>
> Applied to focal:linux-bluefield/master-next. Thanks.
>
> --
> Best regards,
> Bartlomiej
>
> On Tue, Apr 4, 2023 at 12:39 AM William Tu <witu@nvidia.com> wrote:
> >
> > BugLink: https://bugs.launchpad.net/bugs/2015136
> >
> > * Explain the bug(s)
> >
> > tc hardware stats are not offloaded from some offloaded actions (pedit, sample, skbedit)
> >
> > * brief explanation of fixes
> >
> > Implement the necessary callbacks to report back stats for these actions.
> >
> > * How to test
> >
> > Configure two mlx5 rep devices and VFs.
> > Add tc offloaded tc rules with pedit action, and hit that rule with traffic
> >
> > Ido Schimmel (1):
> >   sched: act_sample: Implement stats_update callback
> >
> > Petr Machata (2):
> >   sched: act_skbedit: Implement stats_update callback
> >   sched: act_pedit: Implement stats_update callback
> >
> >  net/sched/act_pedit.c   | 11 +++++++++++
> >  net/sched/act_sample.c  | 11 +++++++++++
> >  net/sched/act_skbedit.c | 11 +++++++++++
> >  3 files changed, 33 insertions(+)
> >
William Tu April 11, 2023, 2:43 p.m. UTC | #8
Hi Bartlomiej,
Thanks for the rebase!
We’ll double check patches sent out next time
William

From: Bartlomiej Zolnierkiewicz <bartlomiej.zolnierkiewicz@canonical.com>
Date: Tuesday, April 11, 2023 at 3:02 AM
To: Vladimir Sokolovsky <vlad@nvidia.com>
Cc: William Tu <witu@nvidia.com>, Ubuntu Kernel Team <kernel-team@lists.ubuntu.com>, Bodong Wang <bodong@nvidia.com>, Paul Blakey <paulb@nvidia.com>, Dann Frazier <dann.frazier@canonical.com>
Subject: Re: APPLIED: [SRU][F:linux-bluefield][PATCH v1 0/3] Support hardware stats for tc actions
External email: Use caution opening links or attachments


Hi Vladimir,
Hi William,

On Mon, Apr 10, 2023 at 6:44 PM Vladimir Sokolovsky <vlad@nvidia.com> wrote:
>
> Hi,
>
> This series was applied already as we tried to compile the “master-next” branch and it failed:
>
>
>
> /workspace/kernel/linux/net/sched/act_sample.c:293:18: error: initialization of 'void (*)(struct tc_action *, u64,  u32,  u64,  bool)' {aka 'void (*)(struct tc_action *, long long unsigned int,  unsigned int,  long long unsigned int,  _Bool)'} from incompatible pointer type 'void (*)(struct tc_action *, u64,  u64,  u64,  u64,  bool)' {aka 'void (*)(struct tc_action *, long long unsigned int,  long long unsigned int,  long long unsigned int,  long long unsigned int,  _Bool)'} [-Werror=incompatible-pointer-types]
>
>   293 |  .stats_update = tcf_sample_stats_update,
>
>       |                  ^~~~~~~~~~~~~~~~~~~~~~~
>
> /workspace/kernel/linux/net/sched/act_sample.c:293:18: note: (near initialization for 'act_sample_ops.stats_update')
>
>
>
> cc1: some warnings being treated as errors
>
>
>
>
>
> make[4]: *** [/workspace/kernel/linux/scripts/Makefile.build:272: net/sched/act_sample.o] Error 1
>
> make[4]: *** Waiting for unfinished jobs....

I've dropped the "[SRU][F:linux-bluefield][PATCH v1 0/3] Support
hardware stats for tc actions" patchset for now from the master-next
branch (by rebasing the branch).

> Bartlomiej,
>
> Do you have some sort of CI that can catch these issues?

We currently don't have CI for automatically build-testing all
submitted patches during review and I've been assuming that the
changes being submitted are actually tested but it doesn't seem to be
always the case (as some submissions don't apply or build). I'll from
now on apply+build test all submissions (I've used to skip it for
smaller, "obviously looking" ones) before acking and applying them.

--
Best regards,
Bartlomiej


> Regards,
>
> Vladimir
>
>
>
>
>
> From: William Tu <witu@nvidia.com>
> Sent: Monday, April 10, 2023 11:30 AM
> To: Bartlomiej Zolnierkiewicz <bartlomiej.zolnierkiewicz@canonical.com>
> Cc: Ubuntu Kernel Team <kernel-team@lists.ubuntu.com>; Bodong Wang <bodong@nvidia.com>; Vladimir Sokolovsky <vlad@nvidia.com>; Paul Blakey <paulb@nvidia.com>; Dann Frazier <dann.frazier@canonical.com>
> Subject: Re: APPLIED: [SRU][F:linux-bluefield][PATCH v1 0/3] Support hardware stats for tc actions
>
>
>
> Hi Bartlomiej,
>
>
>
> Sorry! If it’s not too late, olease do not apply this series, we found compile error due to missing commit.
>
> We’re working on the fix.
>
> William
>
>
>
> From: Bartlomiej Zolnierkiewicz <bartlomiej.zolnierkiewicz@canonical.com>
> Date: Wednesday, April 5, 2023 at 4:09 AM
> To: William Tu <witu@nvidia.com>
> Cc: Ubuntu Kernel Team <kernel-team@lists.ubuntu.com>, Bodong Wang <bodong@nvidia.com>, Vladimir Sokolovsky <vlad@nvidia.com>, Paul Blakey <paulb@nvidia.com>, Dann Frazier <dann.frazier@canonical.com>
> Subject: APPLIED: [SRU][F:linux-bluefield][PATCH v1 0/3] Support hardware stats for tc actions
>
> External email: Use caution opening links or attachments
>
>
> Applied to focal:linux-bluefield/master-next. Thanks.
>
> --
> Best regards,
> Bartlomiej
>
> On Tue, Apr 4, 2023 at 12:39 AM William Tu <witu@nvidia.com> wrote:
> >
> > BugLink: https://bugs.launchpad.net/bugs/2015136
> >
> > * Explain the bug(s)
> >
> > tc hardware stats are not offloaded from some offloaded actions (pedit, sample, skbedit)
> >
> > * brief explanation of fixes
> >
> > Implement the necessary callbacks to report back stats for these actions.
> >
> > * How to test
> >
> > Configure two mlx5 rep devices and VFs.
> > Add tc offloaded tc rules with pedit action, and hit that rule with traffic
> >
> > Ido Schimmel (1):
> >   sched: act_sample: Implement stats_update callback
> >
> > Petr Machata (2):
> >   sched: act_skbedit: Implement stats_update callback
> >   sched: act_pedit: Implement stats_update callback
> >
> >  net/sched/act_pedit.c   | 11 +++++++++++
> >  net/sched/act_sample.c  | 11 +++++++++++
> >  net/sched/act_skbedit.c | 11 +++++++++++
> >  3 files changed, 33 insertions(+)
> >