Re: [PATCH v3 13/13] flow: Avoid moving flow entries to compact table

[Stefano Brivio <sbrivio@redhat.com>]

On Mon, 1 Jan 2024 21:44:54 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Thu, Dec 28, 2023 at 07:25:25PM +0100, Stefano Brivio wrote:
> > On Thu, 21 Dec 2023 17:15:49 +1100
> > David Gibson <david@gibson.dropbear.id.au> wrote:
> >   
> > [...]
> >
> > >  void flow_defer_handler(const struct ctx *c, const struct timespec *now)
> > >  {
> > > +	struct flow_free_block *free_head = NULL;
> > > +	unsigned *last_next = &flow_first_free;
> > >  	bool timer = false;
> > > -	union flow *flow;
> > > +	unsigned idx;
> > >  
> > >  	if (timespec_diff_ms(now, &flow_timer_run) >= FLOW_TIMER_INTERVAL) {
> > >  		timer = true;
> > >  		flow_timer_run = *now;
> > >  	}
> > >  
> > > -	for (flow = flowtab + flow_count - 1; flow >= flowtab; flow--) {
> > > +	for (idx = 0; idx < FLOW_MAX; idx++) {
> > > +		union flow *flow = &flowtab[idx];
> > >  		bool closed = false;
> > >  
> > > +		if (flow->f.type == FLOW_TYPE_NONE) {
> > > +			/* Start of a free block */
> > > +			free_head = &flow->free;
> > > +			*last_next = idx;
> > > +			last_next = &free_head->next;
> > > +			/* Skip the rest of the block */
> > > +			idx += free_head->n - 1;
> > > +			continue;
> > > +		}
> > > +
> > > +		  
> > 
> > Stray tabs.
> >   
> > >  		switch (flow->f.type) {
> > > +		case FLOW_TYPE_NONE:
> > > +			closed = true;
> > > +			break;

...more important than stray tabs, I noticed only now: how would we ever
hit this switch case, if you're checking for this in the if clause just
before?

> > >  		case FLOW_TCP:
> > >  			closed = tcp_flow_defer(flow);
> > >  			break;

-- 
Stefano