From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id D8B09A00C2; Thu, 10 Feb 2022 11:19:46 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 641CA4013F; Thu, 10 Feb 2022 11:19:46 +0100 (CET) Received: from mail-io1-f45.google.com (mail-io1-f45.google.com [209.85.166.45]) by mails.dpdk.org (Postfix) with ESMTP id 43C0440041 for ; Thu, 10 Feb 2022 11:19:45 +0100 (CET) Received: by mail-io1-f45.google.com with SMTP id n17so6627443iod.4 for ; Thu, 10 Feb 2022 02:19:45 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=TzOhmLhhTGygRYuoxVsr34SfDjQ6Ka8OSnz7MQlcrws=; b=L/LvuWlR7wUFZbgd5WJnlBI5JODHs1QmHcU0uYm1hAOlOIpwCn5oi/u7ARPbIPar9n 2SASUwURv9/ZjR3g7RjH3EtS4ufj8fimrwJxIim17abzXUaBxhM6wdOe4TDYd5GQgBcy eW9E54sFngWzFIukMCgn07Hz9ZaWGLVUr7jTiALMojIilaU8oiu6ZKMdvvtst2Glctm6 XCbpBiPST9iu9YECNaZtQ0q5di4u6hZpTVAMA1pMfeT7pOVccuaY0Hv3ubMb9Kr9YCX0 hLHEixsxCidxve58NyTYWs5K2ad19Ebym0Vp7Hun/7FpDvfABwGVhEHzrnVwIhRYPhw4 k3ow== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=TzOhmLhhTGygRYuoxVsr34SfDjQ6Ka8OSnz7MQlcrws=; b=sfPkBtb6lkKVyg4bE2M7lLFaJIDW4NfwwYJ6dBBDcwXtGTzMkInZuTyrQhOfkodAFP j0vTyB5Xjl7zQH1WKGXrfOu/hV92nBiWbQDSfiafGNb2VCsaAXAazEBfNTn4oD9f0tJ8 NHrLOlkhW8o3s8wEZlY8ZWouHyKAyp4hiQAEuYQaZHRWxsd4yseiqaonHUIfzehIe/Iw aT2Lg/j8iAlJLT5dZ/vmiNzLPK/hZcw7um9FP5Old6nxxXdr4GmS7CyDOKLT44bS3txY o/Kc4b6uF1YiIjJerEz0GEP4KbFer5q6RY9LGKkf1xNGOzXMwK4TXnGmS1df74LaaysB 5/5g== X-Gm-Message-State: AOAM530Anmno79fE53BKOBMgkh+vdwMmQ91dmzMwkx5jFVgCLCN/GDis wTKUTPqK5PhvHr/rFZPCeffewyM5trj+V3MRTLA= X-Google-Smtp-Source: ABdhPJxOBb6O/9IHq7iYI05wUrEZPKea1jxMkmLEDyiK3X3TG4ubLaQmEzh4f+YyAyLPxrf/R7P/2PkpoGBsIEEg38g= X-Received: by 2002:a05:6638:2105:: with SMTP id n5mr3692499jaj.266.1644488384562; Thu, 10 Feb 2022 02:19:44 -0800 (PST) MIME-Version: 1.0 References: <20220119071323.3650-1-pbhagavatula@marvell.com> <20220210101314.1215-1-pbhagavatula@marvell.com> In-Reply-To: <20220210101314.1215-1-pbhagavatula@marvell.com> From: Jerin Jacob Date: Thu, 10 Feb 2022 15:49:18 +0530 Message-ID: Subject: Re: [PATCH v3] net/cnxk: avoid command copy from Tx queue To: Pavan Nikhilesh Cc: Jerin Jacob , Nithin Dabilpuram , Kiran Kumar K , Sunil Kumar Kori , Satha Rao , Ankur Dwivedi , Anoob Joseph , Tejasree Kondoj , Shijith Thotton , dpdk-dev Content-Type: text/plain; charset="UTF-8" X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Thu, Feb 10, 2022 at 3:43 PM wrote: > > From: Pavan Nikhilesh > > Tx command is prepared based on offloads enabled and stored in > Tx queue structure at tx_queue_setup phase. > In fastpath the command is copied from Tx queue to LMT line for > all the packets. > Since, the command contents are mostly constants we can move the > command preparation to fastpath and avoid accessing Tx queue > memory. > > Signed-off-by: Pavan Nikhilesh > --- > v3 Changes: > - Rebase. > - Split patches. > - Refactoring large function. > > v2 Changes: > - Rebase. > - Fix incorrect use of RoC API > > drivers/common/cnxk/roc_io.h | 33 ++++- > +static void > +cnxk_sso_tx_queue_data_init(struct cnxk_sso_evdev *dev, uint64_t *txq_data, > + uint16_t eth_port_id, uint16_t tx_queue_id) > +{ > + uint64_t offset = 0; > + int i, j; > + > + dev->max_queue_id[0] = RTE_MAX(dev->max_queue_id[0], eth_port_id); > + for (i = 1; i < eth_port_id; i++) { > + offset += (dev->max_queue_id[i - 1] + 1); > + txq_data[i] |= offset << 48; > + } > + dev->max_port_id = RTE_MAX(dev->max_port_id, eth_port_id); > + dev->max_queue_id[eth_port_id] = > + RTE_MAX(dev->max_queue_id[eth_port_id], tx_queue_id); > +} > + > +static void > +cnxk_sso_tx_queue_data_rewrite(struct cnxk_sso_evdev *dev, uint64_t *txq_data, > + uint16_t eth_port_id, uint16_t tx_queue_id, > + uint64_t *otxq_data, uint16_t max_port_id, > + uint16_t max_queue_id) > +{ > + uint64_t offset = 0; > + int i, j; > + > + for (i = 0; i < dev->max_queue_id[0] + 1; i++) > + txq_data[i] |= (otxq_data[i] & ~((BIT_ULL(16) - 1) << 48)); > + > + if (eth_port_id > max_port_id) { > + dev->max_queue_id[0] = > + RTE_MAX(dev->max_queue_id[0], eth_port_id); > + dev->max_port_id = RTE_MAX(dev->max_port_id, eth_port_id); > + > + for (i = 1; i < eth_port_id; i++) { > + offset += (dev->max_queue_id[i - 1] + 1); > + txq_data[i] |= offset << 48; > + for (j = 0; (i < dev->max_port_id) && > + (j < dev->max_queue_id[i] + 1); > + j++) > + txq_data[offset + j] = > + otxq_data[(otxq_data[i] >> 48) + j]; > + } > + dev->max_queue_id[eth_port_id] = > + RTE_MAX(dev->max_queue_id[eth_port_id], tx_queue_id); Could you move this as a separate static function? Too much depth > + } else if (tx_queue_id > max_queue_id) { > + dev->max_queue_id[eth_port_id] = > + RTE_MAX(dev->max_queue_id[eth_port_id], tx_queue_id); > + dev->max_port_id = RTE_MAX(max_port_id, eth_port_id); > + for (i = 1; i < max_port_id + 1; i++) { > + offset += (dev->max_queue_id[i - 1] + 1); > + txq_data[i] |= offset << 48; > + for (j = 0; j < dev->max_queue_id[i] + 1; j++) { > + if (i == eth_port_id && j > max_queue_id) > + continue; > + txq_data[offset + j] = > + otxq_data[(otxq_data[i] >> 48) + j]; > + } > + } > + } > +} Could you move this as a separate static function? Too much depth > + > static int > cnxk_sso_updt_tx_queue_data(const struct rte_eventdev *event_dev, > uint16_t eth_port_id, uint16_t tx_queue_id, > void *txq) > { > struct cnxk_sso_evdev *dev = cnxk_sso_pmd_priv(event_dev); > + uint16_t max_queue_id = dev->max_queue_id[eth_port_id]; > uint16_t max_port_id = dev->max_port_id; > - uint64_t *txq_data = dev->tx_adptr_data; > - > - if (txq_data == NULL || eth_port_id > max_port_id) { > - max_port_id = RTE_MAX(max_port_id, eth_port_id); > - txq_data = rte_realloc_socket( > - txq_data, > - (sizeof(uint64_t) * (max_port_id + 1) * > - RTE_MAX_QUEUES_PER_PORT), > - RTE_CACHE_LINE_SIZE, event_dev->data->socket_id); > + uint64_t offset = 0, row = 0; > + uint64_t *txq_data = NULL; > + size_t size = 0; > + int i, j; > + > + if (((uint64_t)txq) & 0xFFFF000000000000) > + return -EINVAL; > + > + if (dev->tx_adptr_data == NULL) { > + size = (eth_port_id + 1); > + size += (eth_port_id + tx_queue_id); > + row = 2 * eth_port_id; > + } else { > + if (eth_port_id > max_port_id) { > + size = (RTE_MAX(eth_port_id, dev->max_queue_id[0]) + 1); > + for (i = 1; i < eth_port_id; i++) > + size += (dev->max_queue_id[i] + 1); > + row = size; > + size += (tx_queue_id + 1); > + } else if (tx_queue_id > max_queue_id) { > + size = !eth_port_id ? tx_queue_id + 1 : > + RTE_MAX(max_port_id, > + dev->max_queue_id[0]) + > + 1; See below > + for (i = 1; i < max_port_id + 1; i++) { > + if (i == eth_port_id) { > + row = size; > + size += tx_queue_id + 1; > + } else { > + size += dev->max_queue_id[i] + 1; > + } > + } > + } > + } Could you move this as a separate static function? Too much depth The rest looks good.