From: Ed Czeck <ed.czeck@atomicrules.com>
To: Ferruh Yigit <ferruh.yigit@amd.com>
Cc: dev@dpdk.org, stephen@networkplumber.org,
Shepard Siegel <shepard.siegel@atomicrules.com>,
John Miller <john.miller@atomicrules.com>
Subject: Re: [PATCH v2 2/3] net/ark: support for single function with multiple port
Date: Mon, 20 Feb 2023 17:09:25 -0500 [thread overview]
Message-ID: <CALZ3GujrKBvgCe7YQgjKdZ=YefCZv8AyzCy6ZjeqgB1a1nB5jQ@mail.gmail.com> (raw)
In-Reply-To: <390a5b75-175d-3c03-a690-b062ff5de43b@amd.com>
[-- Attachment #1: Type: text/plain, Size: 5988 bytes --]
Hi Ferruh,
We had looked at providing this patch as an LTS backport, but there are
some ties to FPGA firmware which complicate its application. As this is a
uncommon net/ark feature we support users on an as-needed basis.
Thanks.
Ed.
On Mon, Feb 20, 2023 at 9:54 AM Ferruh Yigit <ferruh.yigit@amd.com> wrote:
> On 2/17/2023 9:59 PM, Ed Czeck wrote:
> > allows the creation of multiple ports from one ark device via
> > the use of ark pmd extension, though the splitting of queues
>
> Hi Ed,
>
> As far as I can see "single function with multiple port" support was
> already there but this commit is fixing queue index usage for it, if
> correct can you please update commit log accordingly? (with fixes line
> etc..)
> This also helps the fix to be backported to LTS versions.
>
>
> btw, how this feature was working until now, when queue ids "0 to
> ark_api_num_queues_per_port()" used for each port?
> Was it not tested at all, or is there something changed in FW causing
> this issue, if so is there any FW version dependency to this change?
>
> >
> > Add unique dev_private data for each port
> >
>
> Please check comment below related to this one.
>
> >
> > Signed-off-by: Ed Czeck <ed.czeck@atomicrules.com>
> > ---
> > drivers/net/ark/ark_ethdev.c | 14 +++++++++++++-
> > drivers/net/ark/ark_ethdev_rx.c | 6 +++---
> > drivers/net/ark/ark_ethdev_tx.c | 2 +-
> > drivers/net/ark/ark_global.h | 4 ++++
> > 4 files changed, 21 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/net/ark/ark_ethdev.c b/drivers/net/ark/ark_ethdev.c
> > index d237e80cf4..96d0c2b0f0 100644
> > --- a/drivers/net/ark/ark_ethdev.c
> > +++ b/drivers/net/ark/ark_ethdev.c
> > @@ -432,6 +432,7 @@ eth_ark_dev_init(struct rte_eth_dev *dev)
> > ark->user_ext.dev_get_port_count(dev,
> > ark->user_data[dev->data->port_id]);
> > ark->num_ports = port_count;
> > + ark->num_queues = ark_api_num_queues_per_port(ark->mpurx.v,
> port_count);
> >
> > for (p = 0; p < port_count; p++) {
> > struct rte_eth_dev *eth_dev;
> > @@ -457,7 +458,18 @@ eth_ark_dev_init(struct rte_eth_dev *dev)
> > }
> >
> > eth_dev->device = &pci_dev->device;
> > - eth_dev->data->dev_private = ark;
> > + /* Device requires new dev_private data */
> > + eth_dev->data->dev_private =
> > + rte_zmalloc_socket(name,
> > + sizeof(struct ark_adapter),
> > + RTE_CACHE_LINE_SIZE,
> > + rte_socket_id());
> > +
> > + memcpy(eth_dev->data->dev_private, ark,
> > + sizeof(struct ark_adapter))> + ark =
> eth_dev->data->dev_private;
> > + ark->qbase = p * ark->num_queues;
> > +
>
> These each are a new eth_dev, so nothing wrong for each to allocate
> device private data, but if the only difference in private data is
> 'ark->qbase', it is possible to use 'eth_dev->process_private' for it,
> which is per eth_dev. It is up to you.
>
>
> Btw, how there are handled in the secondary process? (previous patch)
> Since this code just creates new eth_dev, shouldn't secondary process
> needs some code to find and re-use them?
>
> > eth_dev->dev_ops = ark->eth_dev->dev_ops;
> > eth_dev->tx_pkt_burst = ark->eth_dev->tx_pkt_burst;
> > eth_dev->rx_pkt_burst = ark->eth_dev->rx_pkt_burst;
> > diff --git a/drivers/net/ark/ark_ethdev_rx.c
> b/drivers/net/ark/ark_ethdev_rx.c
> > index cbc0416bc2..38bc69dff4 100644
> > --- a/drivers/net/ark/ark_ethdev_rx.c
> > +++ b/drivers/net/ark/ark_ethdev_rx.c
> > @@ -68,7 +68,7 @@ struct ark_rx_queue {
> > static int
> > eth_ark_rx_hw_setup(struct rte_eth_dev *dev,
> > struct ark_rx_queue *queue,
> > - uint16_t rx_queue_id __rte_unused, uint16_t
> rx_queue_idx)
> > + uint16_t rx_queue_idx)
> > {
> > rte_iova_t queue_base;
> > rte_iova_t phys_addr_q_base;
> > @@ -124,7 +124,7 @@ eth_ark_dev_rx_queue_setup(struct rte_eth_dev *dev,
> > uint32_t i;
> > int status;
> >
> > - int qidx = queue_idx;
> > + int qidx = ark->qbase + queue_idx;
> >
> > /* We may already be setup, free memory prior to re-allocation */
> > if (dev->data->rx_queues[queue_idx] != NULL) {
> > @@ -215,7 +215,7 @@ eth_ark_dev_rx_queue_setup(struct rte_eth_dev *dev,
> > }
> > /* MPU Setup */
> > if (status == 0)
> > - status = eth_ark_rx_hw_setup(dev, queue, qidx, queue_idx);
> > + status = eth_ark_rx_hw_setup(dev, queue, queue_idx);
> >
> > if (unlikely(status != 0)) {
> > struct rte_mbuf **mbuf;
> > diff --git a/drivers/net/ark/ark_ethdev_tx.c
> b/drivers/net/ark/ark_ethdev_tx.c
> > index 5940a592a2..4792754f19 100644
> > --- a/drivers/net/ark/ark_ethdev_tx.c
> > +++ b/drivers/net/ark/ark_ethdev_tx.c
> > @@ -229,7 +229,7 @@ eth_ark_tx_queue_setup(struct rte_eth_dev *dev,
> > struct ark_tx_queue *queue;
> > int status;
> >
> > - int qidx = queue_idx;
> > + int qidx = ark->qbase + queue_idx;
> >
> > if (!rte_is_power_of_2(nb_desc)) {
> > ARK_PMD_LOG(ERR,
> > diff --git a/drivers/net/ark/ark_global.h b/drivers/net/ark/ark_global.h
> > index 71d0b53e03..176fbcda17 100644
> > --- a/drivers/net/ark/ark_global.h
> > +++ b/drivers/net/ark/ark_global.h
> > @@ -112,7 +112,11 @@ struct ark_adapter {
> > ark_pkt_chkr_t pc;
> > ark_pkt_dir_t pd;
> >
> > + /* For single function, multiple ports */
> > int num_ports;
> > + uint16_t qbase;
> > + uint16_t num_queues;
>
>
> it looks like 'num_queues' only used locally and not needed to be part
> of device data, unless there is some more usage planned for future.
>
> > +
> > bool isvf;
> >
> > /* Packet generator/checker args */
>
>
[-- Attachment #2: Type: text/html, Size: 7708 bytes --]
next prev parent reply other threads:[~2023-02-20 22:09 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-17 16:00 [PATCH 1/3] net/ark: support secondary process Ed Czeck
2023-02-17 16:00 ` [PATCH 2/3] net/ark: support for single function with multiple port Ed Czeck
2023-02-17 16:00 ` [PATCH 3/3] net/ark: resize data field to match fpga access Ed Czeck
2023-02-17 16:00 ` [PATCH] " Ed Czeck
2023-02-17 16:57 ` Ferruh Yigit
2023-02-17 21:59 ` [PATCH v2 1/3] net/ark: support secondary process Ed Czeck
2023-02-17 21:59 ` [PATCH v2 2/3] net/ark: support for single function with multiple port Ed Czeck
2023-02-20 14:54 ` Ferruh Yigit
2023-02-20 22:09 ` Ed Czeck [this message]
2023-02-20 23:21 ` Ferruh Yigit
2023-02-17 21:59 ` [PATCH v2 3/3] net/ark: resize data field to match fpga access Ed Czeck
2023-02-20 14:17 ` [PATCH v2 1/3] net/ark: support secondary process Ferruh Yigit
2023-02-20 22:04 ` Ed Czeck
2023-02-20 23:13 ` Ferruh Yigit
2023-02-20 22:11 ` [PATCH v3 1/2] net/ark: limited support for secondary processes Ed Czeck
2023-02-20 22:11 ` [PATCH v3 2/2] net/ark: support for single function with multiple port Ed Czeck
2023-02-21 16:30 ` [PATCH v4] " Ed Czeck
2023-02-23 14:25 ` Ferruh Yigit
2023-05-16 14:27 ` Ferruh Yigit
2023-05-16 15:33 ` [PATCH 1/3] net/ark: support secondary process Burakov, Anatoly
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CALZ3GujrKBvgCe7YQgjKdZ=YefCZv8AyzCy6ZjeqgB1a1nB5jQ@mail.gmail.com' \
--to=ed.czeck@atomicrules.com \
--cc=dev@dpdk.org \
--cc=ferruh.yigit@amd.com \
--cc=john.miller@atomicrules.com \
--cc=shepard.siegel@atomicrules.com \
--cc=stephen@networkplumber.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).