diff mbox series

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

Message ID 20230606131304.4183359-1-edumazet@google.com (mailing list archive)
State Accepted
Commit 682881ee45c81daa883dcd4fe613b0b0d988bb22
Delegated to: Netdev Maintainers
Headers show
Series [net] net: sched: act_police: fix sparse errors in tcf_police_dump() | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net
netdev/fixes_present success Fixes tag present in non-next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 16 this patch: 8
netdev/cc_maintainers fail 1 blamed authors not CCed: zdai@linux.vnet.ibm.com; 1 maintainers not CCed: zdai@linux.vnet.ibm.com
netdev/build_clang success Errors and warnings before: 8 this patch: 8
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 16 this patch: 8
netdev/checkpatch warning WARNING: A patch subject line should describe the change not the tool that found it
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Eric Dumazet June 6, 2023, 1:13 p.m. UTC 
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(-)

Comments

Simon Horman June 6, 2023, 3:21 p.m. UTC | #1 
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>
Jamal Hadi Salim June 6, 2023, 4:01 p.m. UTC | #2 
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
Eric Dumazet June 6, 2023, 4:34 p.m. UTC | #3 
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.
Jamal Hadi Salim June 6, 2023, 5:07 p.m. UTC | #4 
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
patchwork-bot+netdevbpf@kernel.org June 7, 2023, 11:30 a.m. UTC | #5 
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!
diff mbox series

Patch

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,