From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Michael S. Tsirkin" Subject: Re: [PATCH net-next] virtio_net: force_napi_tx module param. Date: Tue, 24 Jul 2018 13:42:49 +0300 Message-ID: <20180724134138-mutt-send-email-mst@kernel.org> References: <20180723231119.142904-1-caleb.raitto@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: jasowang@redhat.com, davem@davemloft.net, netdev@vger.kernel.org, Caleb Raitto To: Caleb Raitto Return-path: Received: from mx3-rdu2.redhat.com ([66.187.233.73]:59418 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726457AbeGXLsk (ORCPT ); Tue, 24 Jul 2018 07:48:40 -0400 Content-Disposition: inline In-Reply-To: <20180723231119.142904-1-caleb.raitto@gmail.com> Sender: netdev-owner@vger.kernel.org List-ID: On Mon, Jul 23, 2018 at 04:11:19PM -0700, Caleb Raitto wrote: > From: Caleb Raitto > > The driver disables tx napi if it's not certain that completions will > be processed affine with tx service. > > Its heuristic doesn't account for some scenarios where it is, such as > when the queue pair count matches the core but not hyperthread count. > > Allow userspace to override the heuristic. This is an alternative > solution to that in the linked patch. That added more logic in the > kernel for these cases, but the agreement was that this was better left > to user control. > > Do not expand the existing napi_tx variable to a ternary value, > because doing so can break user applications that expect > boolean ('Y'/'N') instead of integer output. Add a new param instead. > > Link: https://patchwork.ozlabs.org/patch/725249/ > Acked-by: Willem de Bruijn > Acked-by: Jon Olson > Signed-off-by: Caleb Raitto Is there a reason the same rule should apply to all devices? If not shouldn't this be an ethtool option? > --- > drivers/net/virtio_net.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index 2ff08bc103a9..d9aca4e90d6b 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -39,10 +39,11 @@ > static int napi_weight = NAPI_POLL_WEIGHT; > module_param(napi_weight, int, 0444); > > -static bool csum = true, gso = true, napi_tx; > +static bool csum = true, gso = true, napi_tx, force_napi_tx; > module_param(csum, bool, 0444); > module_param(gso, bool, 0444); > module_param(napi_tx, bool, 0644); > +module_param(force_napi_tx, bool, 0644); > > /* FIXME: MTU in config. */ > #define GOOD_PACKET_LEN (ETH_HLEN + VLAN_HLEN + ETH_DATA_LEN) > @@ -1201,7 +1202,7 @@ static void virtnet_napi_tx_enable(struct virtnet_info *vi, > /* Tx napi touches cachelines on the cpu handling tx interrupts. Only > * enable the feature if this is likely affine with the transmit path. > */ > - if (!vi->affinity_hint_set) { > + if (!vi->affinity_hint_set && !force_napi_tx) { > napi->weight = 0; > return; > } > @@ -2646,7 +2647,7 @@ static int virtnet_alloc_queues(struct virtnet_info *vi) > netif_napi_add(vi->dev, &vi->rq[i].napi, virtnet_poll, > napi_weight); > netif_tx_napi_add(vi->dev, &vi->sq[i].napi, virtnet_poll_tx, > - napi_tx ? napi_weight : 0); > + (napi_tx || force_napi_tx) ? napi_weight : 0); > > sg_init_table(vi->rq[i].sg, ARRAY_SIZE(vi->rq[i].sg)); > ewma_pkt_len_init(&vi->rq[i].mrg_avg_pkt_len); > -- > 2.18.0.233.g985f88cf7e-goog