[dpdk-dev,v2,05/39] examples/l3fwd: move to 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/l3fwd/main.c | 40 ++++++++++++++++++++++++++++++----------
1 file changed, 30 insertions(+), 10 deletions(-)
Comments
> -----Original Message-----
> From: Shahaf Shuler [mailto:shahafs@mellanox.com]
> Sent: Tuesday, December 12, 2017 12:26 PM
> To: dev@dpdk.org; Ananyev, Konstantin <konstantin.ananyev@intel.com>; Nicolau, Radu <radu.nicolau@intel.com>;
> arybchenko@solarflare.com
> Subject: [PATCH v2 05/39] examples/l3fwd: move to 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>
> ---
> 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,
Hmm, does it mean a new warning for all PMDs (majority) which don't support DEV_TX_OFFLOAD_MBUF_FAST_FREE?
Konstantin
> },
> };
>
> @@ -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);
> + }
> 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,
> --
> 1.8.3.1
Tuesday, December 12, 2017 7:12 PM, Ananyev, Konstantin:
> > -----Original Message-----
> > From: Shahaf Shuler [mailto:shahafs@mellanox.com]
> > Sent: Tuesday, December 12, 2017 12:26 PM
> > To: dev@dpdk.org; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>;
> > Nicolau, Radu <radu.nicolau@intel.com>; arybchenko@solarflare.com
> > Subject: [PATCH v2 05/39] examples/l3fwd: move to 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>
> > ---
> > 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,
>
> Hmm, does it mean a new warning for all PMDs (majority) which don't
> support DEV_TX_OFFLOAD_MBUF_FAST_FREE?
Good point.
Unlike other offloads which are must for the application proper run, this one it only for optimizing the performance and should be set only if PMD supports.
Am continuing to aggregate reasons why the DEV_TX_OFFLOAD_MBUF_FAST_FREE should not be defined as an offload. Anyway we passed that...
I will fix on v3.
> Konstantin
>
> > },
> > };
> >
> > @@ -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);
> > + }
> > 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,
> > --
> > 1.8.3.1
-----Original Message-----
> Date: Wed, 13 Dec 2017 07:21:01 +0000
> From: Shahaf Shuler <shahafs@mellanox.com>
> To: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>, "dev@dpdk.org"
> <dev@dpdk.org>, "Nicolau, Radu" <radu.nicolau@intel.com>,
> "arybchenko@solarflare.com" <arybchenko@solarflare.com>
> Subject: Re: [dpdk-dev] [PATCH v2 05/39] examples/l3fwd: move to ethdev
> offloads API
>
> Tuesday, December 12, 2017 7:12 PM, Ananyev, Konstantin:
> > > -----Original Message-----
> > > From: Shahaf Shuler [mailto:shahafs@mellanox.com]
> > > Sent: Tuesday, December 12, 2017 12:26 PM
> > > To: dev@dpdk.org; Ananyev, Konstantin
> > <konstantin.ananyev@intel.com>;
> > > Nicolau, Radu <radu.nicolau@intel.com>; arybchenko@solarflare.com
> > > Subject: [PATCH v2 05/39] examples/l3fwd: move to 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>
> > > ---
> > > 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,
> >
> > Hmm, does it mean a new warning for all PMDs (majority) which don't
> > support DEV_TX_OFFLOAD_MBUF_FAST_FREE?
>
> Good point.
> Unlike other offloads which are must for the application proper run, this one it only for optimizing the performance and should be set only if PMD supports.
> Am continuing to aggregate reasons why the DEV_TX_OFFLOAD_MBUF_FAST_FREE should not be defined as an offload. Anyway we passed that...
>
> I will fix on v3.
Removing is not an option as the PMDs rely on that flag to will have the
impact.
# I see DEV_TX_OFFLOAD_MBUF_FAST_FREE as hint driver to depict the application requirements
# All the drivers by default can support DEV_TX_OFFLOAD_MBUF_FAST_FREE(They are using the hint or
not is a different question)
So, How about setting DEV_TX_OFFLOAD_MBUF_FAST_FREE in all PMD driver as
dummy one? I think, currently, it can be moved to old API to new API
transition function till the drivers change to new offload flag scheme.
We are planning to change nicvf driver to new offload scheme for this
release so with this change, we have the performance impact on l3fwd
application.
I think, the other option could be to change usage/meaning of
DEV_TX_OFFLOAD_MBUF_FAST_FREE flag where when the application needs
multi-pool and reference count scheme then "it sets" the offload flags.
If so, we don't need to set by default on the these applications.
Hi Jerin,
> -----Original Message-----
> From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> Sent: Wednesday, December 13, 2017 7:55 AM
> To: Shahaf Shuler <shahafs@mellanox.com>
> Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; dev@dpdk.org; Nicolau, Radu <radu.nicolau@intel.com>;
> arybchenko@solarflare.com
> Subject: Re: [dpdk-dev] [PATCH v2 05/39] examples/l3fwd: move to ethdev offloads API
>
> -----Original Message-----
> > Date: Wed, 13 Dec 2017 07:21:01 +0000
> > From: Shahaf Shuler <shahafs@mellanox.com>
> > To: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>, "dev@dpdk.org"
> > <dev@dpdk.org>, "Nicolau, Radu" <radu.nicolau@intel.com>,
> > "arybchenko@solarflare.com" <arybchenko@solarflare.com>
> > Subject: Re: [dpdk-dev] [PATCH v2 05/39] examples/l3fwd: move to ethdev
> > offloads API
> >
> > Tuesday, December 12, 2017 7:12 PM, Ananyev, Konstantin:
> > > > -----Original Message-----
> > > > From: Shahaf Shuler [mailto:shahafs@mellanox.com]
> > > > Sent: Tuesday, December 12, 2017 12:26 PM
> > > > To: dev@dpdk.org; Ananyev, Konstantin
> > > <konstantin.ananyev@intel.com>;
> > > > Nicolau, Radu <radu.nicolau@intel.com>; arybchenko@solarflare.com
> > > > Subject: [PATCH v2 05/39] examples/l3fwd: move to 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>
> > > > ---
> > > > 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,
> > >
> > > Hmm, does it mean a new warning for all PMDs (majority) which don't
> > > support DEV_TX_OFFLOAD_MBUF_FAST_FREE?
> >
> > Good point.
> > Unlike other offloads which are must for the application proper run, this one it only for optimizing the performance and should be set only
> if PMD supports.
> > Am continuing to aggregate reasons why the DEV_TX_OFFLOAD_MBUF_FAST_FREE should not be defined as an offload. Anyway we
> passed that...
> >
> > I will fix on v3.
>
> Removing is not an option as the PMDs rely on that flag to will have the
> impact.
> # I see DEV_TX_OFFLOAD_MBUF_FAST_FREE as hint driver to depict the application requirements
> # All the drivers by default can support DEV_TX_OFFLOAD_MBUF_FAST_FREE(They are using the hint or
> not is a different question)
>
> So, How about setting DEV_TX_OFFLOAD_MBUF_FAST_FREE in all PMD driver as
> dummy one? I think, currently, it can be moved to old API to new API
> transition function till the drivers change to new offload flag scheme.
I don't think anyone plans to remove it right now.
If you believe your PMD does need it, that's ok by me.
Though I still think it is a very limited usage for it, and I don't think
we have to make that flag supported by all PMDs.
Konstantin
>
> We are planning to change nicvf driver to new offload scheme for this
> release so with this change, we have the performance impact on l3fwd
> application.
>
> I think, the other option could be to change usage/meaning of
> DEV_TX_OFFLOAD_MBUF_FAST_FREE flag where when the application needs
> multi-pool and reference count scheme then "it sets" the offload flags.
> If so, we don't need to set by default on the these applications.
-----Original Message-----
> Date: Wed, 13 Dec 2017 12:10:26 +0000
> From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>, Shahaf Shuler
> <shahafs@mellanox.com>
> CC: "dev@dpdk.org" <dev@dpdk.org>, "Nicolau, Radu"
> <radu.nicolau@intel.com>, "arybchenko@solarflare.com"
> <arybchenko@solarflare.com>
> Subject: RE: [dpdk-dev] [PATCH v2 05/39] examples/l3fwd: move to ethdev
> offloads API
>
> Hi Jerin,
Hi Konstantin,
>
> > -----Original Message-----
> > From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> > Sent: Wednesday, December 13, 2017 7:55 AM
> > To: Shahaf Shuler <shahafs@mellanox.com>
> > Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; dev@dpdk.org; Nicolau, Radu <radu.nicolau@intel.com>;
> > arybchenko@solarflare.com
> > Subject: Re: [dpdk-dev] [PATCH v2 05/39] examples/l3fwd: move to ethdev offloads API
> >
> > -----Original Message-----
> > > Date: Wed, 13 Dec 2017 07:21:01 +0000
> > > From: Shahaf Shuler <shahafs@mellanox.com>
> > > To: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>, "dev@dpdk.org"
> > > <dev@dpdk.org>, "Nicolau, Radu" <radu.nicolau@intel.com>,
> > > "arybchenko@solarflare.com" <arybchenko@solarflare.com>
> > > Subject: Re: [dpdk-dev] [PATCH v2 05/39] examples/l3fwd: move to ethdev
> > > offloads API
> > >
> > > Tuesday, December 12, 2017 7:12 PM, Ananyev, Konstantin:
> > > > > -----Original Message-----
> > > > > From: Shahaf Shuler [mailto:shahafs@mellanox.com]
> > > > > Sent: Tuesday, December 12, 2017 12:26 PM
> > > > > To: dev@dpdk.org; Ananyev, Konstantin
> > > > <konstantin.ananyev@intel.com>;
> > > > > Nicolau, Radu <radu.nicolau@intel.com>; arybchenko@solarflare.com
> > > > > Subject: [PATCH v2 05/39] examples/l3fwd: move to 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>
> > > > > ---
> > > > > 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,
> > > >
> > > > Hmm, does it mean a new warning for all PMDs (majority) which don't
> > > > support DEV_TX_OFFLOAD_MBUF_FAST_FREE?
> > >
> > > Good point.
> > > Unlike other offloads which are must for the application proper run, this one it only for optimizing the performance and should be set only
> > if PMD supports.
> > > Am continuing to aggregate reasons why the DEV_TX_OFFLOAD_MBUF_FAST_FREE should not be defined as an offload. Anyway we
> > passed that...
> > >
> > > I will fix on v3.
> >
> > Removing is not an option as the PMDs rely on that flag to will have the
> > impact.
> > # I see DEV_TX_OFFLOAD_MBUF_FAST_FREE as hint driver to depict the application requirements
> > # All the drivers by default can support DEV_TX_OFFLOAD_MBUF_FAST_FREE(They are using the hint or
> > not is a different question)
> >
> > So, How about setting DEV_TX_OFFLOAD_MBUF_FAST_FREE in all PMD driver as
> > dummy one? I think, currently, it can be moved to old API to new API
> > transition function till the drivers change to new offload flag scheme.
>
> I don't think anyone plans to remove it right now.
> If you believe your PMD does need it, that's ok by me.
OK.
> Though I still think it is a very limited usage for it, and I don't think
> we have to make that flag supported by all PMDs.
OK. I just suggested because adding the flag in PMD is harmless and we can
avoid an extra check(setting the DEV_TX_OFFLOAD_MBUF_FAST_FREE only
when PMD supports it) in application to hide warning as you pointed out.
No strong opinion on the specifics, I am just cared only reaching the flag to
driver.
> Konstantin
>
> >
> > We are planning to change nicvf driver to new offload scheme for this
> > release so with this change, we have the performance impact on l3fwd
> > application.
> >
> > I think, the other option could be to change usage/meaning of
> > DEV_TX_OFFLOAD_MBUF_FAST_FREE flag where when the application needs
> > multi-pool and reference count scheme then "it sets" the offload flags.
> > If so, we don't need to set by default on the these applications.
-- 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@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,
>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@semihalf.com]
Sent: Monday, December 18, 2017 6:00 PM
To: Shahaf Shuler <shahafs@mellanox.com>; dev@dpdk.org; konstantin.ananyev@intel.com; radu.nicolau@intel.com; arybchenko@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@mellanox.com><mailto:shahafs@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,
Wednesday, December 13, 2017 7:32 PM, Jerin Jacob:
>
> OK. I just suggested because adding the flag in PMD is harmless and we can
> avoid an extra check(setting the DEV_TX_OFFLOAD_MBUF_FAST_FREE only
> when PMD supports it) in application to hide warning as you pointed out.
> No strong opinion on the specifics, I am just cared only reaching the flag to
> driver.
Following some other comments from different examples I believe the way to go is the following:
1. application will set the offloads it *must have* in order to properly function. PMD will fail the configuration if not supported. Such kind of offloads are basically all the offloads besides
DEV_TX_OFFLOAD_MBUF_FAST_FREE as it is an optimization.
2. application will set optional features only if supported (such as DEV_TX_OFFLOAD_MBUF_FAST_FREE)
3. I will remove the redundant print for offloads set but not supported. This can be done in the PMD level based on its implemented (as I understood some PMDs don't set some offloads but do expect to have them enabled. This though should be fixed IMO).
>
>
@@ -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);
+ }
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,