[PATCH net] net: sched: act_police: fix sparse errors in tcf_police_dump()

[PATCH net] net: sched: act_police: fix sparse errors in tcf_police_dump()

[Eric Dumazet]

Fixes following sparse errors:

net/sched/act_police.c:360:28: warning: dereference of noderef expression
net/sched/act_police.c:362:45: warning: dereference of noderef expression
net/sched/act_police.c:362:45: warning: dereference of noderef expression
net/sched/act_police.c:368:28: warning: dereference of noderef expression
net/sched/act_police.c:370:45: warning: dereference of noderef expression
net/sched/act_police.c:370:45: warning: dereference of noderef expression
net/sched/act_police.c:376:45: warning: dereference of noderef expression
net/sched/act_police.c:376:45: warning: dereference of noderef expression

Fixes: d1967e495a8d ("net_sched: act_police: add 2 new attributes to support police 64bit rate and peakrate")
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/sched/act_police.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/net/sched/act_police.c b/net/sched/act_police.c
index 227cba58ce9f30539ead12d1ca5258cbc69dc340..2e9dce03d1eccf4666b40fd6d8c04ff40f2d0780 100644
--- a/net/sched/act_police.c
+++ b/net/sched/act_police.c
@@ -357,23 +357,23 @@ static int tcf_police_dump(struct sk_buff *skb, struct tc_action *a,
 	opt.burst = PSCHED_NS2TICKS(p->tcfp_burst);
 	if (p->rate_present) {
 		psched_ratecfg_getrate(&opt.rate, &p->rate);
-		if ((police->params->rate.rate_bytes_ps >= (1ULL << 32)) &&
+		if ((p->rate.rate_bytes_ps >= (1ULL << 32)) &&
 		    nla_put_u64_64bit(skb, TCA_POLICE_RATE64,
-				      police->params->rate.rate_bytes_ps,
+				      p->rate.rate_bytes_ps,
 				      TCA_POLICE_PAD))
 			goto nla_put_failure;
 	}
 	if (p->peak_present) {
 		psched_ratecfg_getrate(&opt.peakrate, &p->peak);
-		if ((police->params->peak.rate_bytes_ps >= (1ULL << 32)) &&
+		if ((p->peak.rate_bytes_ps >= (1ULL << 32)) &&
 		    nla_put_u64_64bit(skb, TCA_POLICE_PEAKRATE64,
-				      police->params->peak.rate_bytes_ps,
+				      p->peak.rate_bytes_ps,
 				      TCA_POLICE_PAD))
 			goto nla_put_failure;
 	}
 	if (p->pps_present) {
 		if (nla_put_u64_64bit(skb, TCA_POLICE_PKTRATE64,
-				      police->params->ppsrate.rate_pkts_ps,
+				      p->ppsrate.rate_pkts_ps,
 				      TCA_POLICE_PAD))
 			goto nla_put_failure;
 		if (nla_put_u64_64bit(skb, TCA_POLICE_PKTBURST64,
-- 
2.41.0.rc0.172.g3f132b7071-goog

Re: [PATCH net] net: sched: act_police: fix sparse errors in tcf_police_dump()

[Simon Horman]

On Tue, Jun 06, 2023 at 01:13:04PM +0000, Eric Dumazet wrote:
> Fixes following sparse errors:
> 
> net/sched/act_police.c:360:28: warning: dereference of noderef expression
> net/sched/act_police.c:362:45: warning: dereference of noderef expression
> net/sched/act_police.c:362:45: warning: dereference of noderef expression
> net/sched/act_police.c:368:28: warning: dereference of noderef expression
> net/sched/act_police.c:370:45: warning: dereference of noderef expression
> net/sched/act_police.c:370:45: warning: dereference of noderef expression
> net/sched/act_police.c:376:45: warning: dereference of noderef expression
> net/sched/act_police.c:376:45: warning: dereference of noderef expression
> 
> Fixes: d1967e495a8d ("net_sched: act_police: add 2 new attributes to support police 64bit rate and peakrate")
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Reviewed-by: Simon Horman <simon.horman@corigine.com>

Re: [PATCH net] net: sched: act_police: fix sparse errors in tcf_police_dump()

[Jamal Hadi Salim]

On Tue, Jun 6, 2023 at 11:21 AM Simon Horman <simon.horman@corigine.com> wrote:
>
> On Tue, Jun 06, 2023 at 01:13:04PM +0000, Eric Dumazet wrote:
> > Fixes following sparse errors:
> >
> > net/sched/act_police.c:360:28: warning: dereference of noderef expression
> > net/sched/act_police.c:362:45: warning: dereference of noderef expression
> > net/sched/act_police.c:362:45: warning: dereference of noderef expression
> > net/sched/act_police.c:368:28: warning: dereference of noderef expression
> > net/sched/act_police.c:370:45: warning: dereference of noderef expression
> > net/sched/act_police.c:370:45: warning: dereference of noderef expression
> > net/sched/act_police.c:376:45: warning: dereference of noderef expression
> > net/sched/act_police.c:376:45: warning: dereference of noderef expression
> >
> > Fixes: d1967e495a8d ("net_sched: act_police: add 2 new attributes to support police 64bit rate and peakrate")
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
>
> Reviewed-by: Simon Horman <simon.horman@corigine.com>

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

Trivial comment: Eric, for completion, does it make sense to also convert
opt.action = police->tcf_action to opt.action = p->tcf_action;
and moving it after p = rcu_dereference_protected()?


cheers,
jamal

Re: [PATCH net] net: sched: act_police: fix sparse errors in tcf_police_dump()

[Eric Dumazet]

On Tue, Jun 6, 2023 at 6:01 PM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>
> On Tue, Jun 6, 2023 at 11:21 AM Simon Horman <simon.horman@corigine.com> wrote:
> >
> > On Tue, Jun 06, 2023 at 01:13:04PM +0000, Eric Dumazet wrote:
> > > Fixes following sparse errors:
> > >
> > > net/sched/act_police.c:360:28: warning: dereference of noderef expression
> > > net/sched/act_police.c:362:45: warning: dereference of noderef expression
> > > net/sched/act_police.c:362:45: warning: dereference of noderef expression
> > > net/sched/act_police.c:368:28: warning: dereference of noderef expression
> > > net/sched/act_police.c:370:45: warning: dereference of noderef expression
> > > net/sched/act_police.c:370:45: warning: dereference of noderef expression
> > > net/sched/act_police.c:376:45: warning: dereference of noderef expression
> > > net/sched/act_police.c:376:45: warning: dereference of noderef expression
> > >
> > > Fixes: d1967e495a8d ("net_sched: act_police: add 2 new attributes to support police 64bit rate and peakrate")
> > > Signed-off-by: Eric Dumazet <edumazet@google.com>
> >
> > Reviewed-by: Simon Horman <simon.horman@corigine.com>
>
> Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
>
> Trivial comment: Eric, for completion, does it make sense to also convert
> opt.action = police->tcf_action to opt.action = p->tcf_action;
> and moving it after p = rcu_dereference_protected()?
>

Not sure I understand, tcf_action is in police->tcf_action, not in
p->tcf_action ?

Field is read after spin_lock_bh(&police->tcf_lock); so the current
code seems fine to me.

Re: [PATCH net] net: sched: act_police: fix sparse errors in tcf_police_dump()

[Jamal Hadi Salim]

On Tue, Jun 6, 2023 at 12:34 PM Eric Dumazet <edumazet@google.com> wrote:
>
> On Tue, Jun 6, 2023 at 6:01 PM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> >
> > On Tue, Jun 6, 2023 at 11:21 AM Simon Horman <simon.horman@corigine.com> wrote:
> > >
> > > On Tue, Jun 06, 2023 at 01:13:04PM +0000, Eric Dumazet wrote:
> > > > Fixes following sparse errors:
> > > >
> > > > net/sched/act_police.c:360:28: warning: dereference of noderef expression
> > > > net/sched/act_police.c:362:45: warning: dereference of noderef expression
> > > > net/sched/act_police.c:362:45: warning: dereference of noderef expression
> > > > net/sched/act_police.c:368:28: warning: dereference of noderef expression
> > > > net/sched/act_police.c:370:45: warning: dereference of noderef expression
> > > > net/sched/act_police.c:370:45: warning: dereference of noderef expression
> > > > net/sched/act_police.c:376:45: warning: dereference of noderef expression
> > > > net/sched/act_police.c:376:45: warning: dereference of noderef expression
> > > >
> > > > Fixes: d1967e495a8d ("net_sched: act_police: add 2 new attributes to support police 64bit rate and peakrate")
> > > > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > >
> > > Reviewed-by: Simon Horman <simon.horman@corigine.com>
> >
> > Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
> >
> > Trivial comment: Eric, for completion, does it make sense to also convert
> > opt.action = police->tcf_action to opt.action = p->tcf_action;
> > and moving it after p = rcu_dereference_protected()?
> >
>
> Not sure I understand, tcf_action is in police->tcf_action, not in
> p->tcf_action ?
>
> Field is read after spin_lock_bh(&police->tcf_lock); so the current
> code seems fine to me.

Never mind - you are correct.

cheers,
jamal

Re: [PATCH net] net: sched: act_police: fix sparse errors in tcf_police_dump()

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by David S. Miller <davem@davemloft.net>:

On Tue,  6 Jun 2023 13:13:04 +0000 you wrote:
> Fixes following sparse errors:
> 
> net/sched/act_police.c:360:28: warning: dereference of noderef expression
> net/sched/act_police.c:362:45: warning: dereference of noderef expression
> net/sched/act_police.c:362:45: warning: dereference of noderef expression
> net/sched/act_police.c:368:28: warning: dereference of noderef expression
> net/sched/act_police.c:370:45: warning: dereference of noderef expression
> net/sched/act_police.c:370:45: warning: dereference of noderef expression
> net/sched/act_police.c:376:45: warning: dereference of noderef expression
> net/sched/act_police.c:376:45: warning: dereference of noderef expression
> 
> [...]

Here is the summary with links:
  - [net] net: sched: act_police: fix sparse errors in tcf_police_dump()
    https://git.kernel.org/netdev/net/c/682881ee45c8

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