Subject: Re: [PATCH] net: sort queues in xps maps

From: Jakub Kicinski <[email-masked]>
Date: Wed, 13 Jul 2022 20:32:03 -0700

On Thu, 14 Jul 2022 11:24:31 +0800 Yonglong Li wrote:
> >> @@ -2654,6 +2660,13 @@ int __netif_set_xps_queue(struct net_device *dev, const unsigned long *mask,
> >>  					  skip_tc);
> >>  	}
> >>  
> >> +	for (j = -1; j = netif_attrmask_next_and(j, online_mask, mask, nr_ids),
> >> +	     j < nr_ids;) {
> >> +		tci = j * num_tc + tc;
> >> +		map = xmap_dereference(new_dev_maps->attr_map[tci]);
> >> +		sort(map->queues, map->len, sizeof(u16), cmp_u16, NULL);
> >> +	}
> >> +  
> > 
> > Can we instead make sure that expand_xps_map() maintains order?
> >   
> expand_xps_map() only alloc new_map and copy old map's queue to new_map.
> I think it is not suitable to do it in expand_xps_map().
> WDYT?

Oh, right, sorry for the confusion, I assumed since it reallocates that
it also fills the entry. It probably doesn't to make sure that all
allocations succeed before making any modifications.

Can we factor out the inside of the next loop - starting from the 
"add tx-queue to CPU/rx-queue maps" comment into a helper? My worry is
that __netif_set_xps_queue() is already pretty long and complicated we
should try to move some code out rather than make it longer.