From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-11.2 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,NICE_REPLY_A, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8BC89C4742C for ; Mon, 16 Nov 2020 13:25:17 +0000 (UTC) Received: from vger.kernel.org (unknown [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 4B6B7222EC for ; Mon, 16 Nov 2020 13:25:17 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 4B6B7222EC Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=bpf-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728876AbgKPNZR (ORCPT ); Mon, 16 Nov 2020 08:25:17 -0500 Received: from mga03.intel.com ([134.134.136.65]:57515 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726615AbgKPNZQ (ORCPT ); Mon, 16 Nov 2020 08:25:16 -0500 IronPort-SDR: JzVf0gVGQK8zkqajKjwdU+SVWxh8eIk02ZrPkHdRIxvRfd0rFikWhm+7xhvVnRbVsEpf17Gl6k hFd3x3VDafPg== X-IronPort-AV: E=McAfee;i="6000,8403,9806"; a="170845733" X-IronPort-AV: E=Sophos;i="5.77,482,1596524400"; d="scan'208";a="170845733" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orsmga103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 16 Nov 2020 05:25:11 -0800 IronPort-SDR: nv2dTWv75IeIJ1bfCvVYlAbuaQOTjTPbCb6zwc76bMwk/lKBPcW8QSkuP9TFQszpYdcD9lAUWo 3dHMNxPTwwXA== X-IronPort-AV: E=Sophos;i="5.77,482,1596524400"; d="scan'208";a="543602527" Received: from syeghiay-mobl.ger.corp.intel.com (HELO btopel-mobl.ger.intel.com) ([10.249.37.125]) by orsmga005-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 16 Nov 2020 05:25:01 -0800 Subject: Re: [PATCH bpf-next v2 06/10] xsk: propagate napi_id to XDP socket Rx path To: "Michael S. Tsirkin" Cc: =?UTF-8?B?QmrDtnJuIFTDtnBlbA==?= , netdev@vger.kernel.org, bpf@vger.kernel.org, magnus.karlsson@intel.com, ast@kernel.org, daniel@iogearbox.net, maciej.fijalkowski@intel.com, sridhar.samudrala@intel.com, jesse.brandeburg@intel.com, qi.z.zhang@intel.com, kuba@kernel.org, edumazet@google.com, jonathan.lemon@gmail.com, maximmi@nvidia.com, intel-wired-lan@lists.osuosl.org, netanel@amazon.com, akiyano@amazon.com, michael.chan@broadcom.com, sgoutham@marvell.com, ioana.ciornei@nxp.com, ruxandra.radulescu@nxp.com, thomas.petazzoni@bootlin.com, mcroce@microsoft.com, saeedm@nvidia.com, tariqt@nvidia.com, aelior@marvell.com, ecree@solarflare.com, ilias.apalodimas@linaro.org, grygorii.strashko@ti.com, sthemmin@microsoft.com, kda@linux-powerpc.org References: <20201116110416.10719-1-bjorn.topel@gmail.com> <20201116110416.10719-7-bjorn.topel@gmail.com> <20201116064953-mutt-send-email-mst@kernel.org> <614a7ce4-2b6b-129b-de7d-71428f7a71f6@intel.com> <20201116073848-mutt-send-email-mst@kernel.org> From: =?UTF-8?B?QmrDtnJuIFTDtnBlbA==?= Message-ID: <585b011f-0817-a684-d1db-125bb55741fe@intel.com> Date: Mon, 16 Nov 2020 14:24:56 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.3.2 MIME-Version: 1.0 In-Reply-To: <20201116073848-mutt-send-email-mst@kernel.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org On 2020-11-16 13:42, Michael S. Tsirkin wrote: > On Mon, Nov 16, 2020 at 01:01:40PM +0100, Björn Töpel wrote: >> >> On 2020-11-16 12:55, Michael S. Tsirkin wrote: >>> On Mon, Nov 16, 2020 at 12:04:12PM +0100, BjÃf¶rn TÃf¶pel wrote: >>>> From: BjÃf¶rn TÃf¶pel >>>> >>>> Add napi_id to the xdp_rxq_info structure, and make sure the XDP >>>> socket pick up the napi_id in the Rx path. The napi_id is used to find >>>> the corresponding NAPI structure for socket busy polling. >>>> >>>> Acked-by: Ilias Apalodimas >>>> Signed-off-by: BjÃf¶rn TÃf¶pel >>> >>> A bunch of drivers just pass in 0. could you explain when >>> is that ok? how bad is it if the wrong id is used? >>> >> >> If zero is passed, which is a non-valid NAPI_ID, busy-polling will never >> be performed. >> >> Depending on the structure of the driver, napi might or might not be >> initialized (napi_id != 0) or even available. When it wasn't obvious, I >> simply set it to zero. >> >> So, short; No harm if zero, but busy-polling cannot be used in an XDP >> context. >> >> >> [...] >>>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c >>>> index 21b71148c532..d71fe41595b7 100644 >>>> --- a/drivers/net/virtio_net.c >>>> +++ b/drivers/net/virtio_net.c >>>> @@ -1485,7 +1485,7 @@ static int virtnet_open(struct net_device *dev) >>>> if (!try_fill_recv(vi, &vi->rq[i], GFP_KERNEL)) >>>> schedule_delayed_work(&vi->refill, 0); >>>> - err = xdp_rxq_info_reg(&vi->rq[i].xdp_rxq, dev, i); >>>> + err = xdp_rxq_info_reg(&vi->rq[i].xdp_rxq, dev, i, 0); >>>> if (err < 0) >>>> return err; >>> >>> Should this be rq.napi.napi_id ? >>> >> >> Yes, if rq.napi.napi_id is valid here! Is it? > > What initializes it? netif_napi_add? Then I think yes, it's > initialized for all queues ... > Needs to be tested naturally. > Yeah, netid_napi_add does the id generation. My idea was that driver would gradually move to a correct NAPI id (given that it's hard to test w/o HW. Virtio however is simpler to test. :-) Björn From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?unknown-8bit?q?Bj=C3=B6rn_T=C3=B6pel?= Date: Mon, 16 Nov 2020 14:24:56 +0100 Subject: [Intel-wired-lan] [PATCH bpf-next v2 06/10] xsk: propagate napi_id to XDP socket Rx path In-Reply-To: <20201116073848-mutt-send-email-mst@kernel.org> References: <20201116110416.10719-1-bjorn.topel@gmail.com> <20201116110416.10719-7-bjorn.topel@gmail.com> <20201116064953-mutt-send-email-mst@kernel.org> <614a7ce4-2b6b-129b-de7d-71428f7a71f6@intel.com> <20201116073848-mutt-send-email-mst@kernel.org> Message-ID: <585b011f-0817-a684-d1db-125bb55741fe@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: intel-wired-lan@osuosl.org List-ID: On 2020-11-16 13:42, Michael S. Tsirkin wrote: > On Mon, Nov 16, 2020 at 01:01:40PM +0100, Bj??rn T??pel wrote: >> >> On 2020-11-16 12:55, Michael S. Tsirkin wrote: >>> On Mon, Nov 16, 2020 at 12:04:12PM +0100, Bj?f??rn T?f??pel wrote: >>>> From: Bj?f??rn T?f??pel >>>> >>>> Add napi_id to the xdp_rxq_info structure, and make sure the XDP >>>> socket pick up the napi_id in the Rx path. The napi_id is used to find >>>> the corresponding NAPI structure for socket busy polling. >>>> >>>> Acked-by: Ilias Apalodimas >>>> Signed-off-by: Bj?f??rn T?f??pel >>> >>> A bunch of drivers just pass in 0. could you explain when >>> is that ok? how bad is it if the wrong id is used? >>> >> >> If zero is passed, which is a non-valid NAPI_ID, busy-polling will never >> be performed. >> >> Depending on the structure of the driver, napi might or might not be >> initialized (napi_id != 0) or even available. When it wasn't obvious, I >> simply set it to zero. >> >> So, short; No harm if zero, but busy-polling cannot be used in an XDP >> context. >> >> >> [...] >>>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c >>>> index 21b71148c532..d71fe41595b7 100644 >>>> --- a/drivers/net/virtio_net.c >>>> +++ b/drivers/net/virtio_net.c >>>> @@ -1485,7 +1485,7 @@ static int virtnet_open(struct net_device *dev) >>>> if (!try_fill_recv(vi, &vi->rq[i], GFP_KERNEL)) >>>> schedule_delayed_work(&vi->refill, 0); >>>> - err = xdp_rxq_info_reg(&vi->rq[i].xdp_rxq, dev, i); >>>> + err = xdp_rxq_info_reg(&vi->rq[i].xdp_rxq, dev, i, 0); >>>> if (err < 0) >>>> return err; >>> >>> Should this be rq.napi.napi_id ? >>> >> >> Yes, if rq.napi.napi_id is valid here! Is it? > > What initializes it? netif_napi_add? Then I think yes, it's > initialized for all queues ... > Needs to be tested naturally. > Yeah, netid_napi_add does the id generation. My idea was that driver would gradually move to a correct NAPI id (given that it's hard to test w/o HW. Virtio however is simpler to test. :-) Bj?rn