DPDK patches and discussions
 help / color / mirror / Atom feed
From: Shahaf Shuler <shahafs@mellanox.com>
To: Ferruh Yigit <ferruh.yigit@intel.com>, "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [RFC PATCH 2/4] ethdev: introduce Rx queue offloads API
Date: Wed, 30 Aug 2017 06:22:25 +0000	[thread overview]
Message-ID: <VI1PR05MB3149F7198162D0AC34B4EC17C39C0@VI1PR05MB3149.eurprd05.prod.outlook.com> (raw)
In-Reply-To: <81b3f08a-da63-6b5f-7592-d41521c395e7@intel.com>

Tuesday, August 29, 2017 3:50 PM, Ferruh Yigit:
> On 8/7/2017 11:54 AM, Shahaf Shuler wrote:
> > Introduce a new API to configure Rx offloads.
> >
> > The new API will re-use existing DEV_RX_OFFLOAD_* flags to enable the
> > different offloads. This will ease the process of adding a new Rx
> > offloads, as no ABI breakage is involved.
> > In addition, the offload configuration can be done per queue, instead
> > of per port.
> 
> If a device doesn't have capability to set the offload per queue how should it
> behave, I think it is good to define this.

Yes, will add documentation. 
How about If device cannot set offloads per queue, then the queue_setup function should return with ENOTSUP ?

> 
> >
> > The Rx queue offload API can be used only with devices which advertize
> > the RTE_ETH_DEV_RXQ_OFFLOAD capability.
> >
> > The old Rx offloads API is kept for the meanwhile, in order to enable
> > a smooth transition for PMDs and application to the new API.
> >
> > Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
> 
> <...>
> 
> > @@ -357,7 +357,14 @@ struct rte_eth_rxmode {
> >  		jumbo_frame      : 1, /**< Jumbo Frame Receipt enable. */
> >  		hw_strip_crc     : 1, /**< Enable CRC stripping by hardware. */
> >  		enable_scatter   : 1, /**< Enable scatter packets rx handler */
> > -		enable_lro       : 1; /**< Enable LRO */
> > +		enable_lro       : 1, /**< Enable LRO */
> > +		ignore		 : 1;
> 
> what do you think making this variable more verbose, like
> "ignore_rx_offloads"
> 
> "dev_conf.rxmode.ignore" doesn't say on its own what is ignored.

Maybe ignore_offloads ? Rx is quite explicit from rxomde.

> 
> > +		/**
> > +		 * When set the rxmode offloads should be ignored,
> > +		 * instead the Rx offloads will be set on rte_eth_rxq_conf.
> > +		 * This bit is temporary till rxmode Rx offloads API will
> > +		 * be deprecated.
> > +		 */
> >  };
> 
> <...>
> 
> > +/** Device supports the rte_eth_rxq_conf offloads API */ #define
> > +RTE_ETH_DEV_RXQ_OFFLOAD 0x0010
> Since this is temporary flag and with current implementation this is local to
> library, should we put this into public header?
> 
> Later when all PMDs implemented this new method and we want to remove
> the flag, can we remove them or do we have to keep them reserved for any
> conflict for further new values?
> 
> I guess this should be part of missing pmd-ethdev interface file
> (rte_ethdev_pmd.h ?).

Yes it is better fits to inner interface between ethdev and PMDs.
Wondering, do we have other motivation to have such header? 

> 
> >
> >  /**
> >   * @internal
> >


  reply	other threads:[~2017-08-30  6:22 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-07 10:54 [dpdk-dev] [RFC PATCH 0/4] ethdev new " Shahaf Shuler
2017-08-07 10:54 ` [dpdk-dev] [RFC PATCH 1/4] ethdev: rename Rx and Tx configuration structs Shahaf Shuler
2017-08-23 21:39   ` Thomas Monjalon
2017-08-07 10:54 ` [dpdk-dev] [RFC PATCH 2/4] ethdev: introduce Rx queue offloads API Shahaf Shuler
2017-08-23 12:21   ` Ananyev, Konstantin
2017-08-23 13:06     ` Shahaf Shuler
2017-08-23 21:48   ` Thomas Monjalon
2017-08-29 12:50   ` Ferruh Yigit
2017-08-30  6:22     ` Shahaf Shuler [this message]
2017-08-29 13:11   ` Ferruh Yigit
2017-08-07 10:54 ` [dpdk-dev] [RFC PATCH 3/4] ethdev: introduce Tx " Shahaf Shuler
2017-08-07 10:54 ` [dpdk-dev] [RFC PATCH 4/4] ethdev: add helpers to move to the new " Shahaf Shuler
2017-08-23 12:28   ` Ananyev, Konstantin
2017-08-23 13:13     ` Shahaf Shuler
2017-08-23 22:06       ` Thomas Monjalon
2017-08-24  7:12         ` Shahaf Shuler
2017-08-25 13:26           ` Thomas Monjalon
2017-08-29 12:55             ` Ferruh Yigit
2017-08-30  6:30               ` Shahaf Shuler
2017-08-30  7:50                 ` Ferruh Yigit
2017-08-30 10:16                   ` Ananyev, Konstantin
2017-08-30 12:42                     ` Ferruh Yigit
2017-08-30 13:25                       ` Thomas Monjalon
2017-08-30 14:15                       ` Ananyev, Konstantin
2017-08-28 14:12       ` Ananyev, Konstantin
2017-08-29  6:26         ` Shahaf Shuler
2017-08-29  9:43           ` Ananyev, Konstantin
2017-08-23  6:39 ` [dpdk-dev] [RFC PATCH 0/4] ethdev " Shahaf Shuler
2017-08-23 22:16 ` Thomas Monjalon
2017-08-25 10:31 ` Jerin Jacob
2017-08-27  6:05   ` Shahaf Shuler
2017-08-28  5:00     ` Jerin Jacob
2017-08-28 10:57       ` Thomas Monjalon
2017-09-05  7:07         ` Jerin Jacob

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=VI1PR05MB3149F7198162D0AC34B4EC17C39C0@VI1PR05MB3149.eurprd05.prod.outlook.com \
    --to=shahafs@mellanox.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).