2:25 a.m.
On 2020-12-11 10:41, Dan Carpenter wrote:
On Thu, Dec 10, 2020 at 05:07:28PM +0200, Maxim Mikityanskiy wrote:
> On 2020-12-10 10:28, Dan Carpenter wrote:
>> Hi Maxim,
>>
>>
>> url:
https://github.com/0day-ci/linux/commits/Maxim-Mikityanskiy/HTB-offload/2...
>> base:
>> https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git
>> afae3cc2da100ead3cd6ef4bb1fb8bc9d4b817c5
>> config: i386-randconfig-m021-20201209 (attached as .config)
>> compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
>>
>> If you fix the issue, kindly add following tag as appropriate
>> Reported-by: kernel test robot <lkp(a)intel.com>
>> Reported-by: Dan Carpenter <dan.carpenter(a)oracle.com>
>>
>> smatch warnings:
>> net/sched/sch_htb.c:1310 htb_dump_class_stats() error: we previously assumed
'cl->leaf.q' could be null (see line 1300)
>>
>> vim +1310 net/sched/sch_htb.c
>>
>> ^1da177e4c3f415 Linus Torvalds 2005-04-16 1289 static int
>> 87990467d387f92 Stephen Hemminger 2006-08-10 1290
htb_dump_class_stats(struct Qdisc *sch, unsigned long arg, struct gnet_dump *d)
>> ^1da177e4c3f415 Linus Torvalds 2005-04-16 1291 {
>> ^1da177e4c3f415 Linus Torvalds 2005-04-16 1292 struct htb_class *cl =
(struct htb_class *)arg;
>> 1e0ac0107df684e Maxim Mikityanskiy 2020-12-09 1293 struct htb_sched *q =
qdisc_priv(sch);
>> 338ed9b4de57c4b Eric Dumazet 2016-06-21 1294 struct gnet_stats_queue
qs = {
>> 338ed9b4de57c4b Eric Dumazet 2016-06-21 1295 .drops = cl->drops,
>> 3c75f6ee139d464 Eric Dumazet 2017-09-18 1296 .overlimits =
cl->overlimits,
>> 338ed9b4de57c4b Eric Dumazet 2016-06-21 1297 };
>> 6401585366326fc John Fastabend 2014-09-28 1298 __u32 qlen = 0;
>> ^1da177e4c3f415 Linus Torvalds 2005-04-16 1299
>> 5dd431b6b92c0db Paolo Abeni 2019-03-28 @1300 if (!cl->level
&& cl->leaf.q)
>>
^^^^^^^^^^
>> Check for NULL
>
> Well, I don't think this is real... I don't see any possibility how
> cl->leaf.q can be NULL for a leaf class. However, I'll add a similar check
> below anyway.
>
Another option is to remove this check if it's really impossible.
Yes, thanks, I see this option, but between these two options I'd pick
the one that for sure doesn't make any change to the non-offloaded HTB
logic. Even though to the best of my knowledge this check isn't needed,
I might miss something, because I tried tracking down the origin of this
code, and it was already there in the initial commit of 2005.
Respinning now with the CI issues fixed.
regards,
dan carpenter