[dpdk-dev] [PATCH v2 05/39] examples/l3fwd: move to ethdev offloads API

Shahaf Shuler shahafs at mellanox.com
Thu Dec 21 15:08:17 CET 2017


>Looking at diff between v1 and v2, following lines are missing:
>
>-                       port_conf.rxmode.offloads &= dev_info.rx_offload_capa;
>-                       port_conf.txmode.offloads &= dev_info.tx_offload_capa;
>
>I can see this change is consistent across all examples. Is it intentional to enforce the capabilities this way?

Yes it was intentional.

The reason is that application is relying on its offloads to be set. So any offloads set by the application must reach the PMD, otherwise application won’t work correctly.
For example – if application needs Rx checksum, and it is silently masked (by v1 of this series) then it will wrongly assume all checksums are bad.
This is true almost to every offloads. One is exceptional - DEV_TX_OFFLOAD_MBUF_FAST_FREE. Application *can* work without it, as it is only a performance optimization for the mbuf free.

>If so, why enforcing DEV_RX_OFFLOAD_CHECKSUM if the feature is not used by l3fwd code?
>I.e. there is no reference to RX-side ol_flags so application can run without it.

You are right.
When I worked on this code I used the DEV_RX_OFFLOAD_CHECKSUM offloads because also the original example enabled it (hw_ip_checksum = 1)

Adding Thomas the maintainer to confirm.
If it doesn’t use , then we can safely remove this offload.

>
>In v1 the flag was optional which made sense for this particular case.


--Shahaf

From: Maciej Czekaj [mailto:mjc at semihalf.com]
Sent: Monday, December 18, 2017 6:00 PM
To: Shahaf Shuler <shahafs at mellanox.com>; dev at dpdk.org; konstantin.ananyev at intel.com; radu.nicolau at intel.com; arybchenko at solarflare.com
Subject: Re: [dpdk-dev] [PATCH v2 05/39] examples/l3fwd: move to ethdev offloads API



-- Oryginal 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 at mellanox.com><mailto:shahafs at mellanox.com>

---

 examples/l3fwd/main.c | 40 ++++++++++++++++++++++++++++++----------

 1 file changed, 30 insertions(+), 10 deletions(-)



diff --git a/examples/l3fwd/main.c b/examples/l3fwd/main.c

index 6229568..3bdf4d5 100644

--- a/examples/l3fwd/main.c

+++ b/examples/l3fwd/main.c

@@ -149,11 +149,9 @@ struct lcore_params {

                .mq_mode = ETH_MQ_RX_RSS,

                .max_rx_pkt_len = ETHER_MAX_LEN,

                .split_hdr_size = 0,

-               .header_split   = 0, /**< Header Split disabled */

-               .hw_ip_checksum = 1, /**< IP checksum offload enabled */

-               .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 |

+                            DEV_RX_OFFLOAD_CHECKSUM),

        },

        .rx_adv_conf = {

                .rss_conf = {

@@ -163,6 +161,7 @@ struct lcore_params {

        },

        .txmode = {

                .mq_mode = ETH_MQ_TX_NONE,

+               .offloads = DEV_TX_OFFLOAD_MBUF_FAST_FREE,

        },

 };



@@ -612,7 +611,8 @@ enum {

                        };



                        printf("%s\n", str8);

-                       port_conf.rxmode.jumbo_frame = 1;

+                       port_conf.rxmode.offloads |= DEV_RX_OFFLOAD_JUMBO_FRAME;

+                       port_conf.txmode.offloads |= DEV_TX_OFFLOAD_MULTI_SEGS;



                        /*

                         * if no max-pkt-len set, use the default

@@ -908,6 +908,22 @@ enum {

                        n_tx_queue = MAX_TX_QUEUE_PER_PORT;

                printf("Creating queues: nb_rxq=%d nb_txq=%u... ",

                        nb_rx_queue, (unsigned)n_tx_queue );

+

+               rte_eth_dev_info_get(portid, &dev_info);

+               if ((dev_info.rx_offload_capa & port_conf.rxmode.offloads) !=

+                   port_conf.rxmode.offloads) {

+                       printf("Some Rx offloads are not supported "

+                              "by port %d: requested 0x%lx supported 0x%lx\n",

+                              portid, port_conf.rxmode.offloads,

+                              dev_info.rx_offload_capa);

+               }

+               if ((dev_info.tx_offload_capa & port_conf.txmode.offloads) !=

+                   port_conf.txmode.offloads) {

+                       printf("Some Tx offloads are not supported "

+                              "by port %d: requested 0x%lx supported 0x%lx\n",

+                              portid, port_conf.txmode.offloads,

+                              dev_info.tx_offload_capa);

+               }

Looking at diff between v1 and v2, following lines are missing:

-                       port_conf.rxmode.offloads &= dev_info.rx_offload_capa;
-                       port_conf.txmode.offloads &= dev_info.tx_offload_capa;

I can see this change is consistent across all examples. Is it intentional to enforce the capabilities this way?
If so, why enforcing DEV_RX_OFFLOAD_CHECKSUM if the feature is not used by l3fwd code?
I.e. there is no reference to RX-side ol_flags so application can run without it.

In v1 the flag was optional which made sense for this particular case.




                ret = rte_eth_dev_configure(portid, nb_rx_queue,

                                       (uint16_t)n_tx_queue, &port_conf);

                if (ret < 0)

@@ -955,10 +971,9 @@ enum {

                        printf("txq=%u,%d,%d ", lcore_id, queueid, socketid);

                        fflush(stdout);



-                       rte_eth_dev_info_get(portid, &dev_info);

                        txconf = &dev_info.default_txconf;

-                       if (port_conf.rxmode.jumbo_frame)

-                              txconf->txq_flags = 0;

+                       txconf->txq_flags = ETH_TXQ_FLAGS_IGNORE;

+                       txconf->offloads = port_conf.txmode.offloads;

                        ret = rte_eth_tx_queue_setup(portid, queueid, nb_txd,

                                                    socketid, txconf);

                        if (ret < 0)

@@ -984,6 +999,8 @@ enum {

                fflush(stdout);

                /* init RX queues */

                for(queue = 0; queue < qconf->n_rx_queue; ++queue) {

+                       struct rte_eth_rxconf rxq_conf;

+

                        portid = qconf->rx_queue_list[queue].port_id;

                        queueid = qconf->rx_queue_list[queue].queue_id;



@@ -996,9 +1013,12 @@ enum {

                        printf("rxq=%d,%d,%d ", portid, queueid, socketid);

                        fflush(stdout);



+                       rte_eth_dev_info_get(portid, &dev_info);

+                       rxq_conf = dev_info.default_rxconf;

+                       rxq_conf.offloads = port_conf.rxmode.offloads;

                        ret = rte_eth_rx_queue_setup(portid, queueid, nb_rxd,

                                       socketid,

-                                      NULL,

+                                      &rxq_conf,

                                       pktmbuf_pool[socketid]);

                        if (ret < 0)

                               rte_exit(EXIT_FAILURE,



More information about the dev mailing list