Re: [PATCH net] net/sched: flower: fix infinite loop in fl_walk()

[Davide Caratti <dcaratti@redhat.com>]

hello Cong, thanks for reading.

On Wed, 2019-06-19 at 15:04 -0700, Cong Wang wrote:
> On Wed, Jun 19, 2019 at 2:10 PM Davide Caratti <dcaratti@redhat.com> wrote:
> > on some CPUs (e.g. i686), tcf_walker.cookie has the same size as the IDR.
> > In this situation, the following script:
> > 
> >  # tc filter add dev eth0 ingress handle 0xffffffff flower action ok
> >  # tc filter show dev eth0 ingress
> > 
> > results in an infinite loop. It happened also on other CPUs (e.g x86_64),
> > before commit 061775583e35 ("net: sched: flower: introduce reference
> > counting for filters"), because 'handle' + 1 made the u32 overflow before
> > it was assigned to 'cookie'; but that commit replaced the assignment with
> > a self-increment of 'cookie', so the problem was indirectly fixed.
> 
> Interesting... Is this really specific to cls_flower? To me it looks like
> a bug of idr_*_ul() API's, especially for idr_for_each_entry_ul().

good question, I have to investigate this better (idr_for_each_entry_ul()
expands in a iteration of idr_get_next_ul()). It surely got in cls_flower
when it was converted to use IDRs, but it's true that there might be other
points in TC where IDR are used and the same pattern is present (see
below).

> Can you test if the following command has the same problem on i386?
> 
> tc actions add action ok index 4294967295

the action is added, but then reading it back results in an infinite loop.
And again, the infinite loop happens on i686 and not on x86_64. I will try
to see where's the problem also here.

-- 
davide