* [PATCH net-next 0/3] Implement stats_update callback for pedit and skbedit
@ 2020-03-26 20:45 Petr Machata
2020-03-26 20:45 ` [PATCH net-next 1/3] sched: act_skbedit: Implement stats_update callback Petr Machata
` (4 more replies)
0 siblings, 5 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
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [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; +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
end of thread, other threads:[~2020-03-27 2:20 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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 ` [PATCH net-next 3/3] selftests: skbedit_priority: Test counters at the skbedit rule 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
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.