[dpdk-dev,v3,01/39] examples/l2fwd: convert to new ethdev offloads API
Checks
Commit Message
Ethdev offloads API has changed since:
commit ce17eddefc20 ("ethdev: introduce Rx queue offloads API")
commit cba7f53b717d ("ethdev: introduce Tx queue offloads API")
This commit support the new API.
Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
---
examples/l2fwd/main.c | 28 +++++++++++++++++++---------
1 file changed, 19 insertions(+), 9 deletions(-)
Comments
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Shahaf Shuler
> Sent: Tuesday, December 26, 2017 9:23 AM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH v3 01/39] examples/l2fwd: convert to new
> ethdev offloads API
>
> Ethdev offloads API has changed since:
>
> commit ce17eddefc20 ("ethdev: introduce Rx queue offloads API") commit
> cba7f53b717d ("ethdev: introduce Tx queue offloads API")
>
> This commit support the new API.
>
> Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
On 12/26/2017 9:23 AM, Shahaf Shuler wrote:
> Ethdev offloads API has changed since:
>
> commit ce17eddefc20 ("ethdev: introduce Rx queue offloads API")
> commit cba7f53b717d ("ethdev: introduce Tx queue offloads API")
>
> This commit support the new API.
This patch does three things:
1- Convert bit-field Rx Offload information to new bitwise "offloads" variable.
2- Use new queue specific offload configuration for Rx/Tx
3- Enable new mbuf fast free Tx offload
1 and 2 can be classified as "convert to new ethdev offloads", but I am not sure
about 3.
Wouldn't be better to enable new offloadings in a separate patch, other than
convert one? And I don't know if we want to enable that specific offload for all
samples.
>
> Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
> ---
> examples/l2fwd/main.c | 28 +++++++++++++++++++---------
> 1 file changed, 19 insertions(+), 9 deletions(-)
>
> diff --git a/examples/l2fwd/main.c b/examples/l2fwd/main.c
> index e89e2e1bf..4436ea587 100644
> --- a/examples/l2fwd/main.c
> +++ b/examples/l2fwd/main.c
> @@ -110,14 +110,11 @@ struct lcore_queue_conf lcore_queue_conf[RTE_MAX_LCORE];
>
> static struct rte_eth_dev_tx_buffer *tx_buffer[RTE_MAX_ETHPORTS];
>
> -static const struct rte_eth_conf port_conf = {
> +static struct rte_eth_conf port_conf = {
> .rxmode = {
> .split_hdr_size = 0,
> - .header_split = 0, /**< Header Split disabled */
> - .hw_ip_checksum = 0, /**< IP checksum offload disabled */
> - .hw_vlan_filter = 0, /**< VLAN filtering disabled */
> - .jumbo_frame = 0, /**< Jumbo Frame Support disabled */
> - .hw_strip_crc = 1, /**< CRC stripped by hardware */
> + .ignore_offload_bitfield = 1,
> + .offloads = DEV_RX_OFFLOAD_CRC_STRIP,
> },
> .txmode = {
> .mq_mode = ETH_MQ_TX_NONE,
> @@ -649,6 +646,10 @@ main(int argc, char **argv)
>
> /* Initialise each port */
> for (portid = 0; portid < nb_ports; portid++) {
> + struct rte_eth_rxconf rxq_conf;
> + struct rte_eth_txconf txq_conf;
> + struct rte_eth_conf local_port_conf = port_conf;
> +
> /* skip ports that are not enabled */
> if ((l2fwd_enabled_port_mask & (1 << portid)) == 0) {
> printf("Skipping disabled port %u\n", portid);
> @@ -658,7 +659,11 @@ main(int argc, char **argv)
> /* init port */
> printf("Initializing port %u... ", portid);
> fflush(stdout);
> - ret = rte_eth_dev_configure(portid, 1, 1, &port_conf);
> + rte_eth_dev_info_get(portid, &dev_info);
> + if (dev_info.tx_offload_capa & DEV_TX_OFFLOAD_MBUF_FAST_FREE)
> + local_port_conf.txmode.offloads |=
> + DEV_TX_OFFLOAD_MBUF_FAST_FREE;
> + ret = rte_eth_dev_configure(portid, 1, 1, &local_port_conf);
> if (ret < 0)
> rte_exit(EXIT_FAILURE, "Cannot configure device: err=%d, port=%u\n",
> ret, portid);
> @@ -674,9 +679,11 @@ main(int argc, char **argv)
>
> /* init one RX queue */
> fflush(stdout);
> + rxq_conf = dev_info.default_rxconf;
> + rxq_conf.offloads = local_port_conf.rxmode.offloads;
> ret = rte_eth_rx_queue_setup(portid, 0, nb_rxd,
> rte_eth_dev_socket_id(portid),
> - NULL,
> + &rxq_conf,
> l2fwd_pktmbuf_pool);
> if (ret < 0)
> rte_exit(EXIT_FAILURE, "rte_eth_rx_queue_setup:err=%d, port=%u\n",
> @@ -684,9 +691,12 @@ main(int argc, char **argv)
>
> /* init one TX queue on each port */
> fflush(stdout);
> + txq_conf = dev_info.default_txconf;
> + txq_conf.txq_flags = ETH_TXQ_FLAGS_IGNORE;
> + txq_conf.offloads = local_port_conf.txmode.offloads;
> ret = rte_eth_tx_queue_setup(portid, 0, nb_txd,
> rte_eth_dev_socket_id(portid),
> - NULL);
> + &txq_conf);
> if (ret < 0)
> rte_exit(EXIT_FAILURE, "rte_eth_tx_queue_setup:err=%d, port=%u\n",
> ret, portid);
>
Friday, January 12, 2018 3:31 PM, Ferruh Yigit:
> On 12/26/2017 9:23 AM, Shahaf Shuler wrote:
> > Ethdev offloads API has changed since:
> >
> > commit ce17eddefc20 ("ethdev: introduce Rx queue offloads API") commit
> > cba7f53b717d ("ethdev: introduce Tx queue offloads API")
> >
> > This commit support the new API.
>
> This patch does three things:
> 1- Convert bit-field Rx Offload information to new bitwise "offloads" variable.
> 2- Use new queue specific offload configuration for Rx/Tx
> 3- Enable new mbuf fast free Tx offload
>
>
> 1 and 2 can be classified as "convert to new ethdev offloads", but I am not
> sure about 3.
I think all of the 3 should be in a single patch.
The reason is that the convert patch should maintain the same offloads configuration needed for the application.
Before the convert patch the examples were using the default configuration set by the PMD. In there the txq flags were set to ignore ref count and to declare all mbufs are from the same pool.
The fast free Tx offload was added in order to keep this old offloads configuration.
>
> Wouldn't be better to enable new offloadings in a separate patch, other than
> convert one? And I don't know if we want to enable that specific offload for
> all samples.
As you can see, not all the examples has the FAST_FREE offloads, only the entitled ones (i.e. single mempool and no ref count).
For example, ipv4_multicast doesn't set this offload flag.
On 1/14/2018 10:37 AM, Shahaf Shuler wrote:
> Friday, January 12, 2018 3:31 PM, Ferruh Yigit:
>> On 12/26/2017 9:23 AM, Shahaf Shuler wrote:
>>> Ethdev offloads API has changed since:
>>>
>>> commit ce17eddefc20 ("ethdev: introduce Rx queue offloads API") commit
>>> cba7f53b717d ("ethdev: introduce Tx queue offloads API")
>>>
>>> This commit support the new API.
>>
>> This patch does three things:
>> 1- Convert bit-field Rx Offload information to new bitwise "offloads" variable.
>> 2- Use new queue specific offload configuration for Rx/Tx
>> 3- Enable new mbuf fast free Tx offload
>>
>>
>> 1 and 2 can be classified as "convert to new ethdev offloads", but I am not
>> sure about 3.
>
> I think all of the 3 should be in a single patch.
> The reason is that the convert patch should maintain the same offloads configuration needed for the application.
Perhaps I am missing some details about "mbuf fast free" offload, can you please
give more details about it, what does having or not having it mean?
Currently no PMD seems implemented it.
> Before the convert patch the examples were using the default configuration set by the PMD. In there the txq flags were set to ignore ref count and to declare all mbufs are from the same pool.
> The fast free Tx offload was added in order to keep this old offloads configuration.
>
>>
>> Wouldn't be better to enable new offloadings in a separate patch, other than
>> convert one? And I don't know if we want to enable that specific offload for
>> all samples.
>
> As you can see, not all the examples has the FAST_FREE offloads, only the entitled ones (i.e. single mempool and no ref count).
> For example, ipv4_multicast doesn't set this offload flag.
>
Monday, January 15, 2018 12:21 PM, Ferruh Yigit:
> > I think all of the 3 should be in a single patch.
> > The reason is that the convert patch should maintain the same offloads
> configuration needed for the application.
>
> Perhaps I am missing some details about "mbuf fast free" offload, can you
> please give more details about it, what does having or not having it mean?
> Currently no PMD seems implemented it.
Sure,
FAST_FREE offload is the logical AND between the old txqflags of:
ETH_TXQ_FLAGS_NOREFCOUNT
ETH_TXQ_FLAGS_NOMULTMEM
The offload is just a performance optimization. As specified in the documentation [1] it enables the PMDs to further optimize the data path given the guarantees from the application.
Not having it means possible performance degradation for some PMD which rely on it.
There is no PMD which implement it yet since not all PMDs moved to the new offloads API. However this flag is tested and translated into txqflags as part of rte_eth_convert_txq_offloads function.
Relevant PMDs for this offload will be: sfc, thunderx and i40e.
[1]
/**< Device supports optimization for fast release of mbufs.
* When set application must guarantee that per-queue all mbufs comes from
* the same mempool and has refcnt = 1.
*/
>
> > Before the convert patch the examples were using the default
> configuration set by the PMD. In there the txq flags were set to ignore ref
> count and to declare all mbufs are from the same pool.
> > The fast free Tx offload was added in order to keep this old offloads
> configuration.
> >
> >>
> >> Wouldn't be better to enable new offloadings in a separate patch,
> >> other than convert one? And I don't know if we want to enable that
> >> specific offload for all samples.
> >
> > As you can see, not all the examples has the FAST_FREE offloads, only the
> entitled ones (i.e. single mempool and no ref count).
> > For example, ipv4_multicast doesn't set this offload flag.
> >
On 1/15/2018 11:02 AM, Shahaf Shuler wrote:
> Monday, January 15, 2018 12:21 PM, Ferruh Yigit:
>>> I think all of the 3 should be in a single patch.
>>> The reason is that the convert patch should maintain the same offloads
>> configuration needed for the application.
>>
>> Perhaps I am missing some details about "mbuf fast free" offload, can you
>> please give more details about it, what does having or not having it mean?
>> Currently no PMD seems implemented it.
>
> Sure,
>
> FAST_FREE offload is the logical AND between the old txqflags of:
> ETH_TXQ_FLAGS_NOREFCOUNT
> ETH_TXQ_FLAGS_NOMULTMEM
>
> The offload is just a performance optimization. As specified in the documentation [1] it enables the PMDs to further optimize the data path given the guarantees from the application.
> Not having it means possible performance degradation for some PMD which rely on it.
>
> There is no PMD which implement it yet since not all PMDs moved to the new offloads API. However this flag is tested and translated into txqflags as part of rte_eth_convert_txq_offloads function.
> Relevant PMDs for this offload will be: sfc, thunderx and i40e.
Thank you for clarification, I am OK to have it.
But since currently no PMD provide "DEV_TX_OFFLOAD_MBUF_FAST_FREE" capability,
and default txq_flags is overwritten, some PMDs lost this optimization until
they implement new capability, right?
>
>
> [1]
> /**< Device supports optimization for fast release of mbufs.
> * When set application must guarantee that per-queue all mbufs comes from
> * the same mempool and has refcnt = 1.
> */
>
>>
>>> Before the convert patch the examples were using the default
>> configuration set by the PMD. In there the txq flags were set to ignore ref
>> count and to declare all mbufs are from the same pool.
>>> The fast free Tx offload was added in order to keep this old offloads
>> configuration.
>>>
>>>>
>>>> Wouldn't be better to enable new offloadings in a separate patch,
>>>> other than convert one? And I don't know if we want to enable that
>>>> specific offload for all samples.
>>>
>>> As you can see, not all the examples has the FAST_FREE offloads, only the
>> entitled ones (i.e. single mempool and no ref count).
>>> For example, ipv4_multicast doesn't set this offload flag.
>>>
>
Monday, January 15, 2018 1:34 PM, Ferruh Yigit:
> >> Currently no PMD seems implemented it.
> >
> > Sure,
> >
> > FAST_FREE offload is the logical AND between the old txqflags of:
> > ETH_TXQ_FLAGS_NOREFCOUNT
> > ETH_TXQ_FLAGS_NOMULTMEM
> >
> > The offload is just a performance optimization. As specified in the
> documentation [1] it enables the PMDs to further optimize the data path
> given the guarantees from the application.
> > Not having it means possible performance degradation for some PMD
> which rely on it.
> >
> > There is no PMD which implement it yet since not all PMDs moved to the
> new offloads API. However this flag is tested and translated into txqflags as
> part of rte_eth_convert_txq_offloads function.
> > Relevant PMDs for this offload will be: sfc, thunderx and i40e.
>
> Thank you for clarification, I am OK to have it.
>
> But since currently no PMD provide "DEV_TX_OFFLOAD_MBUF_FAST_FREE"
> capability, and default txq_flags is overwritten, some PMDs lost this
> optimization until they implement new capability, right?
Yes this is correct, a good motivation to convert :).
From the examples and testpmd part everything is ready. Just need for the PMD to report back the capability to have this offload.
On 1/15/2018 11:41 AM, Shahaf Shuler wrote:
> Monday, January 15, 2018 1:34 PM, Ferruh Yigit:
>>>> Currently no PMD seems implemented it.
>>>
>>> Sure,
>>>
>>> FAST_FREE offload is the logical AND between the old txqflags of:
>>> ETH_TXQ_FLAGS_NOREFCOUNT
>>> ETH_TXQ_FLAGS_NOMULTMEM
>>>
>>> The offload is just a performance optimization. As specified in the
>> documentation [1] it enables the PMDs to further optimize the data path
>> given the guarantees from the application.
>>> Not having it means possible performance degradation for some PMD
>> which rely on it.
>>>
>>> There is no PMD which implement it yet since not all PMDs moved to the
>> new offloads API. However this flag is tested and translated into txqflags as
>> part of rte_eth_convert_txq_offloads function.
>>> Relevant PMDs for this offload will be: sfc, thunderx and i40e.
>>
>> Thank you for clarification, I am OK to have it.
>>
>> But since currently no PMD provide "DEV_TX_OFFLOAD_MBUF_FAST_FREE"
>> capability, and default txq_flags is overwritten, some PMDs lost this
>> optimization until they implement new capability, right?
>
> Yes this is correct, a good motivation to convert :).
My concern is being close to the integration deadline and if this patch cause
performance drop for some PMDs.
But I guess we can get the related PMD fixes almost until release if an issue
observed, so this gives time to PMD maintainers.
For series,
Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
> From the examples and testpmd part everything is ready. Just need for the PMD to report back the capability to have this offload.
>
@@ -110,14 +110,11 @@ struct lcore_queue_conf lcore_queue_conf[RTE_MAX_LCORE];
static struct rte_eth_dev_tx_buffer *tx_buffer[RTE_MAX_ETHPORTS];
-static const struct rte_eth_conf port_conf = {
+static struct rte_eth_conf port_conf = {
.rxmode = {
.split_hdr_size = 0,
- .header_split = 0, /**< Header Split disabled */
- .hw_ip_checksum = 0, /**< IP checksum offload disabled */
- .hw_vlan_filter = 0, /**< VLAN filtering disabled */
- .jumbo_frame = 0, /**< Jumbo Frame Support disabled */
- .hw_strip_crc = 1, /**< CRC stripped by hardware */
+ .ignore_offload_bitfield = 1,
+ .offloads = DEV_RX_OFFLOAD_CRC_STRIP,
},
.txmode = {
.mq_mode = ETH_MQ_TX_NONE,
@@ -649,6 +646,10 @@ main(int argc, char **argv)
/* Initialise each port */
for (portid = 0; portid < nb_ports; portid++) {
+ struct rte_eth_rxconf rxq_conf;
+ struct rte_eth_txconf txq_conf;
+ struct rte_eth_conf local_port_conf = port_conf;
+
/* skip ports that are not enabled */
if ((l2fwd_enabled_port_mask & (1 << portid)) == 0) {
printf("Skipping disabled port %u\n", portid);
@@ -658,7 +659,11 @@ main(int argc, char **argv)
/* init port */
printf("Initializing port %u... ", portid);
fflush(stdout);
- ret = rte_eth_dev_configure(portid, 1, 1, &port_conf);
+ rte_eth_dev_info_get(portid, &dev_info);
+ if (dev_info.tx_offload_capa & DEV_TX_OFFLOAD_MBUF_FAST_FREE)
+ local_port_conf.txmode.offloads |=
+ DEV_TX_OFFLOAD_MBUF_FAST_FREE;
+ ret = rte_eth_dev_configure(portid, 1, 1, &local_port_conf);
if (ret < 0)
rte_exit(EXIT_FAILURE, "Cannot configure device: err=%d, port=%u\n",
ret, portid);
@@ -674,9 +679,11 @@ main(int argc, char **argv)
/* init one RX queue */
fflush(stdout);
+ rxq_conf = dev_info.default_rxconf;
+ rxq_conf.offloads = local_port_conf.rxmode.offloads;
ret = rte_eth_rx_queue_setup(portid, 0, nb_rxd,
rte_eth_dev_socket_id(portid),
- NULL,
+ &rxq_conf,
l2fwd_pktmbuf_pool);
if (ret < 0)
rte_exit(EXIT_FAILURE, "rte_eth_rx_queue_setup:err=%d, port=%u\n",
@@ -684,9 +691,12 @@ main(int argc, char **argv)
/* init one TX queue on each port */
fflush(stdout);
+ txq_conf = dev_info.default_txconf;
+ txq_conf.txq_flags = ETH_TXQ_FLAGS_IGNORE;
+ txq_conf.offloads = local_port_conf.txmode.offloads;
ret = rte_eth_tx_queue_setup(portid, 0, nb_txd,
rte_eth_dev_socket_id(portid),
- NULL);
+ &txq_conf);
if (ret < 0)
rte_exit(EXIT_FAILURE, "rte_eth_tx_queue_setup:err=%d, port=%u\n",
ret, portid);