* [PATCH net-next 1/3] sched: act_skbedit: Implement stats_update callback
2020-03-26 20:45 [PATCH net-next 0/3] Implement stats_update callback for pedit and skbedit Petr Machata
@ 2020-03-26 20:45 ` Petr Machata
2020-03-26 20:45 ` [PATCH net-next 2/3] sched: act_pedit: " Petr Machata
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Petr Machata @ 2020-03-26 20:45 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Petr Machata, idosch, jiri, alexpe
Implement this callback in order to get the offloaded stats added to the
kernel stats.
Reported-by: Alexander Petrovskiy <alexpe@mellanox.com>
Signed-off-by: Petr Machata <petrm@mellanox.com>
---
net/sched/act_skbedit.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c
index e857424c387c..b125b2be4467 100644
--- a/net/sched/act_skbedit.c
+++ b/net/sched/act_skbedit.c
@@ -73,6 +73,16 @@ static int tcf_skbedit_act(struct sk_buff *skb, const struct tc_action *a,
return TC_ACT_SHOT;
}
+static void tcf_skbedit_stats_update(struct tc_action *a, u64 bytes,
+ u32 packets, u64 lastuse, bool hw)
+{
+ struct tcf_skbedit *d = to_skbedit(a);
+ struct tcf_t *tm = &d->tcf_tm;
+
+ tcf_action_update_stats(a, bytes, packets, false, hw);
+ tm->lastuse = max_t(u64, tm->lastuse, lastuse);
+}
+
static const struct nla_policy skbedit_policy[TCA_SKBEDIT_MAX + 1] = {
[TCA_SKBEDIT_PARMS] = { .len = sizeof(struct tc_skbedit) },
[TCA_SKBEDIT_PRIORITY] = { .len = sizeof(u32) },
@@ -323,6 +333,7 @@ static struct tc_action_ops act_skbedit_ops = {
.id = TCA_ID_SKBEDIT,
.owner = THIS_MODULE,
.act = tcf_skbedit_act,
+ .stats_update = tcf_skbedit_stats_update,
.dump = tcf_skbedit_dump,
.init = tcf_skbedit_init,
.cleanup = tcf_skbedit_cleanup,
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH net-next 2/3] sched: act_pedit: Implement stats_update callback
2020-03-26 20:45 [PATCH net-next 0/3] Implement stats_update callback for pedit and skbedit Petr Machata
2020-03-26 20:45 ` [PATCH net-next 1/3] sched: act_skbedit: Implement stats_update callback Petr Machata
@ 2020-03-26 20:45 ` Petr Machata
2020-03-26 20:45 ` [PATCH net-next 3/3] selftests: skbedit_priority: Test counters at the skbedit rule Petr Machata
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Petr Machata @ 2020-03-26 20:45 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Petr Machata, idosch, jiri, alexpe
Implement this callback in order to get the offloaded stats added to the
kernel stats.
Reported-by: Alexander Petrovskiy <alexpe@mellanox.com>
Signed-off-by: Petr Machata <petrm@mellanox.com>
---
net/sched/act_pedit.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index 3ad718576304..d41d6200d9de 100644
--- a/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -409,6 +409,16 @@ static int tcf_pedit_act(struct sk_buff *skb, const struct tc_action *a,
return p->tcf_action;
}
+static void tcf_pedit_stats_update(struct tc_action *a, u64 bytes, u32 packets,
+ u64 lastuse, bool hw)
+{
+ struct tcf_pedit *d = to_pedit(a);
+ struct tcf_t *tm = &d->tcf_tm;
+
+ tcf_action_update_stats(a, bytes, packets, false, hw);
+ tm->lastuse = max_t(u64, tm->lastuse, lastuse);
+}
+
static int tcf_pedit_dump(struct sk_buff *skb, struct tc_action *a,
int bind, int ref)
{
@@ -485,6 +495,7 @@ static struct tc_action_ops act_pedit_ops = {
.id = TCA_ID_PEDIT,
.owner = THIS_MODULE,
.act = tcf_pedit_act,
+ .stats_update = tcf_pedit_stats_update,
.dump = tcf_pedit_dump,
.cleanup = tcf_pedit_cleanup,
.init = tcf_pedit_init,
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH net-next 3/3] selftests: skbedit_priority: Test counters at the skbedit rule
2020-03-26 20:45 [PATCH net-next 0/3] Implement stats_update callback for pedit and skbedit Petr Machata
2020-03-26 20:45 ` [PATCH net-next 1/3] sched: act_skbedit: Implement stats_update callback Petr Machata
2020-03-26 20:45 ` [PATCH net-next 2/3] sched: act_pedit: " Petr Machata
@ 2020-03-26 20:45 ` Petr Machata
2020-03-27 0:01 ` [PATCH net-next 0/3] Implement stats_update callback for pedit and skbedit Alexander Petrovskiy
2020-03-27 2:20 ` David Miller
4 siblings, 0 replies; 6+ messages in thread
From: Petr Machata @ 2020-03-26 20:45 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Petr Machata, idosch, jiri, alexpe
Currently the test checks the observable effect of skbedit priority:
queueing of packets at the correct qdisc band. It therefore misses the fact
that the counters for offloaded rules are not updated. Add an extra check
for the counter.
Signed-off-by: Petr Machata <petrm@mellanox.com>
---
.../testing/selftests/net/forwarding/skbedit_priority.sh | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/net/forwarding/skbedit_priority.sh b/tools/testing/selftests/net/forwarding/skbedit_priority.sh
index 0e7693297765..e3bd8a6bb8b4 100755
--- a/tools/testing/selftests/net/forwarding/skbedit_priority.sh
+++ b/tools/testing/selftests/net/forwarding/skbedit_priority.sh
@@ -120,14 +120,19 @@ test_skbedit_priority_one()
flower action skbedit priority $prio
local pkt0=$(qdisc_parent_stats_get $swp2 $classid .packets)
+ local pkt2=$(tc_rule_handle_stats_get "$locus" 101)
$MZ $h1 -t udp "sp=54321,dp=12345" -c 10 -d 20msec -p 100 \
-a own -b $h2mac -A 192.0.2.1 -B 192.0.2.2 -q
+
local pkt1
pkt1=$(busywait "$HIT_TIMEOUT" until_counter_is ">= $((pkt0 + 10))" \
qdisc_parent_stats_get $swp2 $classid .packets)
+ check_err $? "Expected to get 10 packets on class $classid, but got $((pkt1 - pkt0))."
+
+ local pkt3=$(tc_rule_handle_stats_get "$locus" 101)
+ ((pkt3 >= pkt2 + 10))
+ check_err $? "Expected to get 10 packets on skbedit rule but got $((pkt3 - pkt2))."
- check_err $? "Expected to get 10 packets on class $classid, but got
-$((pkt1 - pkt0))."
log_test "$locus skbedit priority $prio -> classid $classid"
tc filter del $locus pref 1
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net-next 0/3] Implement stats_update callback for pedit and skbedit
2020-03-26 20:45 [PATCH net-next 0/3] Implement stats_update callback for pedit and skbedit Petr Machata
` (2 preceding siblings ...)
2020-03-26 20:45 ` [PATCH net-next 3/3] selftests: skbedit_priority: Test counters at the skbedit rule Petr Machata
@ 2020-03-27 0:01 ` Alexander Petrovskiy
2020-03-27 2:20 ` David Miller
4 siblings, 0 replies; 6+ messages in thread
From: Alexander Petrovskiy @ 2020-03-27 0:01 UTC (permalink / raw)
To: Petr Machata, netdev@vger.kernel.org
Cc: David Miller, Ido Schimmel, Jiri Pirko
On 26.03.2020, 23:46, "Petr Machata" <petrm@mellanox.com> wrote:
>The stats_update callback is used for adding HW counters to the SW ones.
>Both skbedit and pedit actions are actually recognized by flow_offload.h,
>but do not implement these callbacks. As a consequence, the reported values
>are only the SW ones, even where there is a HW counter available.
>
>Patch #1 adds the callback to action skbedit, patch #2 adds it to action
>pedit. Patch #3 tweaks an skbedit selftest with a check that would have
>caught this problem.
>
>The pedit test is not likewise tweaked, because the iproute2 pedit action
>currently does not support JSON dumping. This will be addressed later.
>
>Petr Machata (3):
> sched: act_skbedit: Implement stats_update callback
> sched: act_pedit: Implement stats_update callback
> selftests: skbedit_priority: Test counters at the skbedit rule
>
> net/sched/act_pedit.c | 11 +++++++++++
> net/sched/act_skbedit.c | 11 +++++++++++
> .../selftests/net/forwarding/skbedit_priority.sh | 9 +++++++--
> 3 files changed, 29 insertions(+), 2 deletions(-)
>
>--
>2.20.1
Tested-by: Alexander Petrovskiy <alexpe@mellanox.com>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH net-next 0/3] Implement stats_update callback for pedit and skbedit
2020-03-26 20:45 [PATCH net-next 0/3] Implement stats_update callback for pedit and skbedit Petr Machata
` (3 preceding siblings ...)
2020-03-27 0:01 ` [PATCH net-next 0/3] Implement stats_update callback for pedit and skbedit Alexander Petrovskiy
@ 2020-03-27 2:20 ` David Miller
4 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2020-03-27 2:20 UTC (permalink / raw)
To: petrm; +Cc: netdev, idosch, jiri, alexpe
From: Petr Machata <petrm@mellanox.com>
Date: Thu, 26 Mar 2020 22:45:54 +0200
> The stats_update callback is used for adding HW counters to the SW ones.
> Both skbedit and pedit actions are actually recognized by flow_offload.h,
> but do not implement these callbacks. As a consequence, the reported values
> are only the SW ones, even where there is a HW counter available.
>
> Patch #1 adds the callback to action skbedit, patch #2 adds it to action
> pedit. Patch #3 tweaks an skbedit selftest with a check that would have
> caught this problem.
>
> The pedit test is not likewise tweaked, because the iproute2 pedit action
> currently does not support JSON dumping. This will be addressed later.
Series applied, thanks Petr.
^ permalink raw reply [flat|nested] 6+ messages in thread