Re: [dpdk-dev] [PATCH v3] ethdev: fix DCB config issue on ixgbe

[Thomas Monjalon <thomas.monjalon@6wind.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.