From mboxrd@z Thu Jan 1 00:00:00 1970 From: Edward Cree Subject: Re: [PATCH net 2/2] sfc: limit ARFS workitems in flight per channel Date: Thu, 12 Apr 2018 16:24:46 +0100 Message-ID: References: <1713aaa9-2803-0d3b-127a-5240876da7b1@solarflare.com> <20180412.111132.159310781108534969.davem@davemloft.net> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit Cc: , To: David Miller Return-path: Received: from dispatch1-us1.ppe-hosted.com ([67.231.154.164]:37050 "EHLO dispatch1-us1.ppe-hosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753765AbeDLPZZ (ORCPT ); Thu, 12 Apr 2018 11:25:25 -0400 In-Reply-To: <20180412.111132.159310781108534969.davem@davemloft.net> Content-Language: en-GB Sender: netdev-owner@vger.kernel.org List-ID: On 12/04/18 16:11, David Miller wrote: > From: Edward Cree > Date: Thu, 12 Apr 2018 15:02:50 +0100 > >> A misconfigured system (e.g. with all interrupts affinitised to all CPUs) >> may produce a storm of ARFS steering events. With the existing sfc ARFS >> implementation, that could create a backlog of workitems that grinds the >> system to a halt. To prevent this, limit the number of workitems that >> may be in flight for a given SFC device to 8 (EFX_RPS_MAX_IN_FLIGHT), and >> return EBUSY from our ndo_rx_flow_steer method if the limit is reached. >> Given this limit, also store the workitems in an array of slots within the >> struct efx_nic, rather than dynamically allocating for each request. >> >> Signed-off-by: Edward Cree > I don't think this behavior is all that great. > > If you really have to queue up these operations because they take a long > time, I think it is better to enter a synchronous mode and sleep once > you hit this in-flight limit of 8. I don't think we can sleep at this point, ndo_rx_flow_steer is called from  the RX path (netif_receive_skb_internal() -> get_rps_cpu() ->  set_rps_cpu()). > Either that or make the expiration work smarter when it has lots of events > to process. I'm afraid I don't understand what you mean here. This code is not handling expiration of old ARFS filters, it's inserting  new ones. -Ed