From: Thomas Monjalon <thomas.monjalon@6wind.com>
To: Wenzhuo Lu <wenzhuo.lu@intel.com>
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v3] ethdev: fix DCB config issue on ixgbe
Date: Wed, 22 Jun 2016 19:01:34 +0200 [thread overview]
Message-ID: <1871393.ccgjmFpxqt@xps13> (raw)
In-Reply-To: <1462484040-1702-1-git-send-email-wenzhuo.lu@intel.com>
2016-05-06 05:33, Wenzhuo Lu:
> An issue is found that DCB cannot be configured on ixgbe
> NICs. It's said the TX queue number is not right.
> On ixgbe the max TX queue number is not fixed, it depends
> on the multi-queue mode. The API rte_eth_dev_configure
> should be used to configure this mode. But the input of
> this API includes TX queue number. The problem is before
> the mode is configured, we cannot decide the TX queue
> number.
>
> This patch adds an API to configure RX & TX multi-queue mode
> separately. After the mode is configured, the max RX & TX
> queue number is decided. Then we can set the appropriate
> RX & TX queue number.
[...]
> +/**
> + * Set RX & TX multi_queue mode.
> + *
> + * @param port_id
> + * The port identifier of the Ethernet device.
> + * @param rx_mq_mode
> + * RX multi_queue mode.
> + * @param tx_mq_mode
> + * TX multi_queue mode.
> + *
> + * @return
> + * - (0) if successful.
> + * - (-ENODEV) if port identifier is invalid.
> + */
> +int
> +rte_eth_dev_mq_mode_set(uint8_t port_id,
> + enum rte_eth_rx_mq_mode rx_mq_mode,
> + enum rte_eth_tx_mq_mode tx_mq_mode);
I've really tried to think about it and I think it is more or less
a hack.
First, it is not explained in the doc when we should use
rte_eth_dev_mq_mode_set() instead of a simple call to
rte_eth_dev_configure().
Second, I don't understand why having a function which configures the
"multiqueue modes" without configuring properly RSS/VMDq/DCB.
Last, it is said that rte_eth_dev_configure() "must be invoked first
before any other function in the Ethernet API".
My opinion is that the primary goal of rte_eth_dev_configure() was
"Embedding all configuration information in a single data structure"
but it is currently configuring only speed and some flow steering
(only RSS, VMDq, DCB and flow director).
This bug and the state of the ethdev API clearly shows that we must
have one function per feature (or group of features) and drop
rte_eth_dev_configure().
You can argue it is a just a personal feeling and this comment comes
late, but I promise it is not easy to give a negative opinion because
of design perspective.
I strongly feel we must stop workarounding the ethdev API issues
and start really fixing it.
Hope you understand and agree to work on a new API.
next prev parent reply other threads:[~2016-06-22 17:01 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-11 8:24 [dpdk-dev] [PATCH] lib: " Wenzhuo Lu
2016-04-11 9:52 ` Thomas Monjalon
2016-04-12 0:39 ` Lu, Wenzhuo
2016-05-04 21:47 ` [dpdk-dev] [PATCH v2] " Wenzhuo Lu
2016-05-05 21:33 ` [dpdk-dev] [PATCH v3] ethdev: " Wenzhuo Lu
2016-06-17 11:21 ` De Lara Guarch, Pablo
2016-06-22 17:01 ` Thomas Monjalon [this message]
2016-06-23 1:04 ` Lu, Wenzhuo
2016-06-23 12:21 ` Thomas Monjalon
2016-06-24 0:45 ` Lu, Wenzhuo
2016-06-30 1:40 ` Lu, Wenzhuo
2016-06-30 7:41 ` Thomas Monjalon
2016-06-30 8:23 ` Lu, Wenzhuo
2016-06-30 8:47 ` Lu, Wenzhuo
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=1871393.ccgjmFpxqt@xps13 \
--to=thomas.monjalon@6wind.com \
--cc=dev@dpdk.org \
--cc=wenzhuo.lu@intel.com \
/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).