[dpdk-dev] [PATCH v8] ethdev: check Rx/Tx offloads
Ferruh Yigit
ferruh.yigit at intel.com
Thu May 10 00:40:44 CEST 2018
On 5/9/2018 3:11 PM, Ferruh Yigit wrote:
> On 5/8/2018 6:51 PM, Andrew Rybchenko wrote:
>> On 05/08/2018 01:10 PM, Wei Dai wrote:
>>> This patch check if a input requested offloading is valid or not.
>>> Any reuqested offloading must be supported in the device capabilities.
>>> Any offloading is disabled by default if it is not set in the parameter
>>> dev_conf->[rt]xmode.offloads to rte_eth_dev_configure( ) and
>>> [rt]x_conf->offloads to rte_eth_[rt]x_queue_setup( ).
>>> If any offloading is enabled in rte_eth_dev_configure( ) by application,
>>> it is enabled on all queues no matter whether it is per-queue or
>>> per-port type and no matter whether it is set or cleared in
>>> [rt]x_conf->offloads to rte_eth_[rt]x_queue_setup( ).
>>> If a per-queue offloading hasn't be enabled in rte_eth_dev_configure( ),
>>> it can be enabled or disabled for individual queue in
>>> ret_eth_[rt]x_queue_setup( ).
>>> A new added offloading is the one which hasn't been enabled in
>>> rte_eth_dev_configure( ) and is reuqested to be enabled in
>>> rte_eth_[rt]x_queue_setup( ), it must be per-queue type,
>>> otherwise return error.
>>> The underlying PMD must be aware that the requested offloadings
>>> to PMD specific queue_setup( ) function only carries those
>>> new added offloadings of per-queue type.
>>>
>>> This patch can make above such checking in a common way in rte_ethdev
>>> layer to avoid same checking in underlying PMD.
>>>
>>> This patch assumes that all PMDs in 18.05-rc2 have already
>>> converted to offload API defined in 17.11 . It also assumes
>>> that all PMDs can return correct offloading capabilities
>>> in rte_eth_dev_infos_get( ).
>>>
>>> In the beginning of [rt]x_queue_setup( ) of underlying PMD,
>>> add offloads = [rt]xconf->offloads |
>>> dev->data->dev_conf.[rt]xmode.offloads; to keep same as offload API
>>> defined in 17.11 to avoid upper application broken due to offload
>>> API change.
>>> PMD can use the info that input [rt]xconf->offloads only carry
>>> the new added per-queue offloads to do some optimization or some
>>> code change on base of this patch.
>>>
>>> Signed-off-by: Wei Dai <wei.dai at intel.com>
>>> Signed-off-by: Ferruh Yigit <ferruh.yigit at intel.com>
>>> Signed-off-by: Qi Zhang <qi.z.zhang at intel.com>
>>
>> [...]
>>
>>> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
>>> index e560524..523a07b 100644
>>> --- a/lib/librte_ethdev/rte_ethdev.c
>>> +++ b/lib/librte_ethdev/rte_ethdev.c
>>> @@ -1139,6 +1139,28 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
>>> ETHER_MAX_LEN;
>>> }
>>>
>>> + /* Any requested offloading must be within its device capabilities */
>>> + if ((local_conf.rxmode.offloads & dev_info.rx_offload_capa) !=
>>> + local_conf.rxmode.offloads) {
>>> + RTE_PMD_DEBUG_TRACE("ethdev port_id=%d requested Rx offloads "
>>> + "0x%" PRIx64 " doesn't match Rx offloads "
>>> + "capabilities 0x%" PRIx64 "\n",
>>> + port_id,
>>> + local_conf.rxmode.offloads,
>>> + dev_info.rx_offload_capa);
>>> + return -EINVAL;
>>> + }
>>> + if ((local_conf.txmode.offloads & dev_info.tx_offload_capa) !=
>>> + local_conf.txmode.offloads) {
>>> + RTE_PMD_DEBUG_TRACE("ethdev port_id=%d requested Tx offloads "
>>> + "0x%" PRIx64 " doesn't match Tx offloads "
>>> + "capabilities 0x%" PRIx64 "\n",
>>> + port_id,
>>> + local_conf.txmode.offloads,
>>> + dev_info.tx_offload_capa);
>>> + return -EINVAL;
>>> + }
>>> +
>>> /* Check that device supports requested rss hash functions. */
>>> if ((dev_info.flow_type_rss_offloads |
>>> dev_conf->rx_adv_conf.rss_conf.rss_hf) !=
>>> @@ -1504,6 +1526,39 @@ rte_eth_rx_queue_setup(uint16_t port_id, uint16_t rx_queue_id,
>>> &local_conf.offloads);
>>> }
>>>
>>> + /*
>>> + * If an offloading has already been enabled in
>>> + * rte_eth_dev_configure(), it has been enabled on all queues,
>>> + * so there is no need to enable it in this queue again.
>>> + * The local_conf.offloads input to underlying PMD only carries
>>> + * those offloadings which are only enabled on this queue and
>>> + * not enabled on all queues.
>>> + * The underlying PMD must be aware of this point.
>>> + */
>>> + local_conf.offloads &= ~dev->data->dev_conf.rxmode.offloads;
>>
>> In fact it has a problem with MULTI_SEG.
>> It is a new offload which was introduced to substitute NOMULTISEG TxQ flag.
>> If PMD reports the offload on device level, but application does not use
>> the new interface and request the offload on queue level using txq_flags
>> (absent NOMULTISEG flag). As the result below check fails.
>> Sounds like it requires dedicated code to handle it. Anything else?
>
> Old applications don't set Tx offloads on configure() at all. But they set them
> in tx_queue_setup().
> For the PMDs that support only port offloads old applications will fail in
> tx_queue_setup(), if I don't miss anything.
>
> I suggest keeping the log but remove the error return until all applications
> switch to new offloading API which should be on next release. What do you think?
Also virtual devices will give an error with this because of current testpmd
implementation [1].
We may hit similar unexpected issues, as Shahaf voiced same concern.
What do you think removing error return from all three locations,
rte_eth_dev_configure(), rte_eth_rx_queue_setup(), rte_eth_tx_queue_setup() and
just keep error log, for this release? Next release we will already update the
application interface, can add error returns next release.
[1]
https://dpdk.org/dev/patchwork/patch/39643/
>
>
> btw, the log macro "RTE_PMD_DEBUG_TRACE" is in ERR log type but it controlled by
> CONFIG_RTE_LIBRTE_ETHDEV_DEBUG config option which is disabled by default. So by
> default we don't get the error log but the error only. I suggest switching to
> new dynamic logging ethdev_log()
>
>
>>
>>> +
>>> + /*
>>> + * New added offloadings for this queue are those not enabled in
>>> + * rte_eth_dev_configure( ) and they must be per-queue type.
>>> + * A pure per-port offloading can't be enabled on a queue while
>>> + * disabled on another queue. A pure per-port offloading can't
>>> + * be enabled for any queue as new added one if it hasn't been
>>> + * enabled in rte_eth_dev_configure( ).
>>> + */
>>> + if ((local_conf.offloads & dev_info.rx_queue_offload_capa) !=
>>> + local_conf.offloads) {
>>> + RTE_PMD_DEBUG_TRACE("Ethdev port_id=%d rx_queue_id=%d, new "
>>> + "added offloads 0x" PRIx64 " must be "
>>> + "within pre-queue offload capabilities 0x"
>>> + PRIx64 " in %s\n",
>>> + port_id,
>>> + rx_queue_id,
>>> + local_conf.offloads,
>>> + dev_info.rx_queue_offload_capa,
>>> + __func__);
>>> + return -EINVAL;
>>> + }
>>> +
>>> ret = (*dev->dev_ops->rx_queue_setup)(dev, rx_queue_id, nb_rx_desc,
>>> socket_id, &local_conf, mp);
>>> if (!ret) {
>>
>> [...]
>>
>
More information about the dev
mailing list