From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id 7CC58A0096 for ; Tue, 4 Jun 2019 12:45:29 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id B58231BC0C; Tue, 4 Jun 2019 12:45:28 +0200 (CEST) Received: from mail-ot1-f67.google.com (mail-ot1-f67.google.com [209.85.210.67]) by dpdk.org (Postfix) with ESMTP id C7D1C1BC07 for ; Tue, 4 Jun 2019 12:45:26 +0200 (CEST) Received: by mail-ot1-f67.google.com with SMTP id c3so19074248otr.3 for ; Tue, 04 Jun 2019 03:45:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=semihalf-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=1SowllDk9/sWaxrdIa2SbfsBWi/ZXnIFYiHNumnnvNA=; b=OhHTsbKRjIe11da5khF47zvQKFYJukP8N7SXNLtEQCEQKyJJnVck9UjorHuX0cG3pz loF3GV5KOqQp/CmcQJ89w1WpU8LRmP6O/CBNALdb4taKf4qT6rmdahu+y6pCpJkuJbW+ eOr708DBkUsBFte4iyXXpy2Y2LN5YT+RK5Mm1WF9dQaFG/qN6KX+8sNIy5RFfe8LJ+oz zIV/jQk2KIzRDMyaPNutIrXxqo6ooCz0/rbXD5VFiHuO0DhZjL94PPtD0Kngx8WVllOc oIk/kTlgpkT7Fn04mQ2FayXu2sjJIDm0i2DxDOM2fhq46Zov3yBOtgpa6p9dEUQF/WLI D/Dw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=1SowllDk9/sWaxrdIa2SbfsBWi/ZXnIFYiHNumnnvNA=; b=IlTrTw8oZ3ZFjyHTYgHhl8dzmPEPJgO0Mdv9g88qA2gvxBEoKX5by/qmjIwIoXh4n2 wRg0sPAqq1+7vOlXMgoBSO2g+e5vHO8UZZPj5q/K2R0dZVFsl08P9u6QBHzj6mvnXakF 2Yl1l0BwTxVN2vsrT04+GKseR9lJigfS4LDOguBXfbGkuSJc5g0N8+5YdrlwebQg0s7q DFJL5taT9I0n4+qFXGuHv7+SlhPxvuvD9oGfAydYI+slmMDSXGMMBk16q63QlfoIjpj9 zi1PoX1lOFPyrMSv9bgDHDihSktFJHGcqav/GiM2gjDOyBvZMfWZWN2+boDQCApqDokR gEqQ== X-Gm-Message-State: APjAAAUn2yX+k+pUmEPm/5/bOBpWvTQYoDskAm/2zWyQlmN1EoTeFMff XIYh+2omkUCWr9ZzQ+lDnwAsH4+ZT/nIsF9eq/BUDg== X-Google-Smtp-Source: APXvYqwaSunH06bvTl0IgfUuNNnm+F0YAxSU1E9+E+EE0EtACACWjciU7qOInjBWzBS8zKerBzLunEMpArqCmSOgor4= X-Received: by 2002:a9d:8:: with SMTP id 8mr3288688ota.169.1559645126026; Tue, 04 Jun 2019 03:45:26 -0700 (PDT) MIME-Version: 1.0 References: <5f6e26e27ad524f85ee9a911aeebae69f1ec0c1a.1559147228.git.anatoly.burakov@intel.com> <2f73f49d-e13b-ac1f-9e32-80b9d39b1166@semihalf.com> <4faeb0be-fe10-d866-1027-0f3ef351cd3a@semihalf.com> In-Reply-To: From: =?UTF-8?Q?Micha=C5=82_Krawczyk?= Date: Tue, 4 Jun 2019 12:45:15 +0200 Message-ID: To: "Burakov, Anatoly" Cc: dev@dpdk.org, Marcin Wojtas , Guy Tzalik , Evgeny Schemeilin , stephen@networkplumber.org, Thomas Monjalon , david.marchand@redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Subject: Re: [dpdk-dev] [PATCH 24/25] net/ena: fix direct access to shared memory config X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" wt., 4 cze 2019 o 12:28 Burakov, Anatoly napisa= =C5=82(a): > > On 03-Jun-19 2:36 PM, Micha=C5=82 Krawczyk wrote: > > On 03.06.2019 09:33, Micha=C5=82 Krawczyk wrote: > >> On 29.05.2019 18:31, Anatoly Burakov wrote: > >>> The ENA driver calculates a ring's NUMA node affinity by directly > >>> accessing the memzone list. Fix it to do it through the public > >>> API's instead. > >>> > >>> Signed-off-by: Anatoly Burakov > >>> --- > >>> drivers/net/ena/ena_ethdev.c | 18 +++--------------- > >>> 1 file changed, 3 insertions(+), 15 deletions(-) > >>> > >>> diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethde= v.c > >>> index b6651fc0f..e745e9e92 100644 > >>> --- a/drivers/net/ena/ena_ethdev.c > >>> +++ b/drivers/net/ena/ena_ethdev.c > >>> @@ -274,20 +274,6 @@ static const struct eth_dev_ops ena_dev_ops =3D = { > >>> #define NUMA_NO_NODE SOCKET_ID_ANY > >>> -static inline int ena_cpu_to_node(int cpu) > >>> -{ > >>> - struct rte_config *config =3D rte_eal_get_configuration(); > >>> - struct rte_fbarray *arr =3D &config->mem_config->memzones; > >>> - const struct rte_memzone *mz; > >>> - > >>> - if (unlikely(cpu >=3D RTE_MAX_MEMZONE)) > >>> - return NUMA_NO_NODE; > >>> - > >>> - mz =3D rte_fbarray_get(arr, cpu); > >>> - > >>> - return mz->socket_id; > >>> -} > >>> - > >>> static inline void ena_rx_mbuf_prepare(struct rte_mbuf *mbuf, > >>> struct ena_com_rx_ctx *ena_rx_ctx) > >>> { > >>> @@ -1099,6 +1085,7 @@ static int ena_create_io_queue(struct ena_ring > >>> *ring) > >>> { > >>> struct ena_adapter *adapter; > >>> struct ena_com_dev *ena_dev; > >>> + struct rte_memseg_list *msl; > >>> struct ena_com_create_io_ctx ctx =3D > >>> /* policy set to _HOST just to satisfy icc compiler */ > >>> { ENA_ADMIN_PLACEMENT_POLICY_HOST, > >>> @@ -1126,7 +1113,8 @@ static int ena_create_io_queue(struct ena_ring > >>> *ring) > >>> } > >>> ctx.qid =3D ena_qid; > >>> ctx.msix_vector =3D -1; /* interrupts not used */ > >>> - ctx.numa_node =3D ena_cpu_to_node(ring->id); > >>> + msl =3D rte_mem_virt2memseg_list(ring); > >>> + ctx.numa_node =3D msl->socket_id; > >>> rc =3D ena_com_create_io_queue(ena_dev, &ctx); > >>> if (rc) { > >>> > >> > >> Hi Anatoly, > >> > >> I'm not sure why the previous maintainers implemented this that way, I > >> can only guess. I think that they were assuming, that each queue will > >> be assigned to the lcore which is equal to ring id. They probably also > >> misunderstood how the memzones are working and they thought that each > >> lcore is having assigned only one memzone which is being mapped 1 to 1= . > >> > >> They wanted to prevent cross NUMA data acces, when the CPU is > >> operating in the different NUMA zone and the IO queues memory resides > >> in the other. I think that above solution won't prevent that neither, > >> as you are using ring address, which is being allocated together with > >> struct ena_adapter (it is just an array), so it will probably reside > >> in the single numa zone. > >> > >> I'm currently thinking on solution that could help us to determine on > >> which numa zone the queue descriptors will be allocated and on which > >> the lcore assigned to the queue will be working, but have no any ideas > >> for now :) > >> > >> Anyway, your fix won't break anything, as the previous solution wasn't > >> working as it was supposed to work, so before I will fix that, we can > >> keep that patch to prevent direct usage of the memzone. > >> > >> Thanks, > >> Michal > > > > After investigation I think that we should use socket_id provided by th= e > > tx/rx queue setup functions. > > Could you, please, abandon this patch? I will send the proper fix soon. > > > > I can't really "abandon" it as it will break ENA compilation once the > structure is hidden in the last patch. What i can do is wait for you to > submit your patch, and either rebase my patchset on top of it, or > (better) include it in the patchset itself. Ok, I've just uploaded the patch (second version has fixed commit log), you can find it below https://patches.dpdk.org/patch/54352/ I'm fine with including the patch into this patchset. > > > Thanks, > > Michal > > > > > -- > Thanks, > Anatoly