diff mbox

[dpdk-dev] eventdev: remove default queue overriding

Message ID 1488372542-156763-1-git-send-email-harry.van.haaren@intel.com (mailing list archive)
State Superseded, archived
Headers

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation success Compilation OK

Commit Message

Van Haaren, Harry March 1, 2017, 12:49 p.m. UTC 
  PMDs that only do a specific type of scheduling cannot provide
CFG_ALL_TYPES, so the Eventdev infrastructure should not demand
that every PMD supports CFG_ALL_TYPES.

By not overriding the default configuration of the queue as
suggested by the PMD, the eventdev_common unit tests can pass
on all PMDs, regardless of thier capabilities.

RTE_EVENT_QUEUE_CFG_DEFAULT is no longer used by the eventdev layer
it can be removed now. Applications should use CFG_ALL_TYPES
if they require enqueue of all types a queue.

Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
---
 lib/librte_eventdev/rte_eventdev.c | 1 -
 lib/librte_eventdev/rte_eventdev.h | 6 ------
 2 files changed, 7 deletions(-)

Comments

Jerin Jacob March 6, 2017, 1:45 p.m. UTC | #1 
On Wed, Mar 01, 2017 at 12:49:02PM +0000, Harry van Haaren wrote:
> PMDs that only do a specific type of scheduling cannot provide
> CFG_ALL_TYPES, so the Eventdev infrastructure should not demand
> that every PMD supports CFG_ALL_TYPES.

Sure. Then I think then we can enumerate CFG_ALL_TYPES as capability.
Meaning, New flag in event_dev_cap to denote PMD can support all the sched type
per queue. My reasoning for the capability flag is because, The application flow
will be depended on the logic of creating the queue with different flags.
Thoughts ?

I thought, In SW implementation, We can create 3 virtual queues per
queue. Based on en-queue's shed_type, implementation can choose the
correct underneath virtual queue. I guess, it has performance issues,
if so, Maybe capability is the way forward.

Setting to CFG_ALL_TYPES will be useful for flow based event pipeling
as I mentioned earlier in other email.

> By not overriding the default configuration of the queue as
> suggested by the PMD, the eventdev_common unit tests can pass
> on all PMDs, regardless of thier capabilities.

Make sense. We can remove the default as CFG_ALL_TYPES.

> 
> RTE_EVENT_QUEUE_CFG_DEFAULT is no longer used by the eventdev layer
> it can be removed now. Applications should use CFG_ALL_TYPES
> if they require enqueue of all types a queue.
> 
> Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> ---
>  lib/librte_eventdev/rte_eventdev.c | 1 -
>  lib/librte_eventdev/rte_eventdev.h | 6 ------
>  2 files changed, 7 deletions(-)
> 
> diff --git a/lib/librte_eventdev/rte_eventdev.c b/lib/librte_eventdev/rte_eventdev.c
> index 68bfc3b..c32a776 100644
> --- a/lib/librte_eventdev/rte_eventdev.c
> +++ b/lib/librte_eventdev/rte_eventdev.c
> @@ -593,7 +593,6 @@ rte_event_queue_setup(uint8_t dev_id, uint8_t queue_id,
>  		RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->queue_def_conf,
>  					-ENOTSUP);
>  		(*dev->dev_ops->queue_def_conf)(dev, queue_id, &def_conf);
> -		def_conf.event_queue_cfg = RTE_EVENT_QUEUE_CFG_DEFAULT;
>  		queue_conf = &def_conf;
>  	}
>  
> diff --git a/lib/librte_eventdev/rte_eventdev.h b/lib/librte_eventdev/rte_eventdev.h
> index 7073987..d836f61 100644
> --- a/lib/librte_eventdev/rte_eventdev.h
> +++ b/lib/librte_eventdev/rte_eventdev.h
> @@ -471,12 +471,6 @@ rte_event_dev_configure(uint8_t dev_id,
>  /* Event queue specific APIs */
>  
>  /* Event queue configuration bitmap flags */
> -#define RTE_EVENT_QUEUE_CFG_DEFAULT            (0)
> -/**< Default value of *event_queue_cfg* when rte_event_queue_setup() invoked
> - * with queue_conf == NULL
> - *
> - * @see rte_event_queue_setup()
> - */
>  #define RTE_EVENT_QUEUE_CFG_TYPE_MASK          (3ULL << 0)
>  /**< Mask for event queue schedule type configuration request */
>  #define RTE_EVENT_QUEUE_CFG_ALL_TYPES          (0ULL << 0)
> -- 
> 2.7.4
>
Van Haaren, Harry March 8, 2017, 12:11 p.m. UTC | #2 
> -----Original Message-----
> From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> Sent: Monday, March 6, 2017 1:45 PM
> To: Van Haaren, Harry <harry.van.haaren@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [PATCH] eventdev: remove default queue overriding
> 
> On Wed, Mar 01, 2017 at 12:49:02PM +0000, Harry van Haaren wrote:
> > PMDs that only do a specific type of scheduling cannot provide
> > CFG_ALL_TYPES, so the Eventdev infrastructure should not demand
> > that every PMD supports CFG_ALL_TYPES.
> 
> Sure. Then I think then we can enumerate CFG_ALL_TYPES as capability.
> Meaning, New flag in event_dev_cap to denote PMD can support all the sched type
> per queue. My reasoning for the capability flag is because, The application flow
> will be depended on the logic of creating the queue with different flags.
> Thoughts ?

Yes, makes sense.

 
> I thought, In SW implementation, We can create 3 virtual queues per
> queue. Based on en-queue's shed_type, implementation can choose the
> correct underneath virtual queue. I guess, it has performance issues,
> if so, Maybe capability is the way forward.

You're right that it is possible, you're also right that the performance
hit in switching the queue to enqueue to in software is prohibitive of
supporting it.

+1 for a capability approach. I'll respin the patch, adding a capability,
and we can discuss more / merge then.

Thanks for the input!


> Setting to CFG_ALL_TYPES will be useful for flow based event pipeling
> as I mentioned earlier in other email.
> 
> > By not overriding the default configuration of the queue as
> > suggested by the PMD, the eventdev_common unit tests can pass
> > on all PMDs, regardless of thier capabilities.
> 
> Make sense. We can remove the default as CFG_ALL_TYPES.
> 
> >
> > RTE_EVENT_QUEUE_CFG_DEFAULT is no longer used by the eventdev layer
> > it can be removed now. Applications should use CFG_ALL_TYPES
> > if they require enqueue of all types a queue.
> >
> > Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> > ---
> >  lib/librte_eventdev/rte_eventdev.c | 1 -
> >  lib/librte_eventdev/rte_eventdev.h | 6 ------
> >  2 files changed, 7 deletions(-)
> >
> > diff --git a/lib/librte_eventdev/rte_eventdev.c b/lib/librte_eventdev/rte_eventdev.c
> > index 68bfc3b..c32a776 100644
> > --- a/lib/librte_eventdev/rte_eventdev.c
> > +++ b/lib/librte_eventdev/rte_eventdev.c
> > @@ -593,7 +593,6 @@ rte_event_queue_setup(uint8_t dev_id, uint8_t queue_id,
> >  		RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->queue_def_conf,
> >  					-ENOTSUP);
> >  		(*dev->dev_ops->queue_def_conf)(dev, queue_id, &def_conf);
> > -		def_conf.event_queue_cfg = RTE_EVENT_QUEUE_CFG_DEFAULT;
> >  		queue_conf = &def_conf;
> >  	}
> >
> > diff --git a/lib/librte_eventdev/rte_eventdev.h b/lib/librte_eventdev/rte_eventdev.h
> > index 7073987..d836f61 100644
> > --- a/lib/librte_eventdev/rte_eventdev.h
> > +++ b/lib/librte_eventdev/rte_eventdev.h
> > @@ -471,12 +471,6 @@ rte_event_dev_configure(uint8_t dev_id,
> >  /* Event queue specific APIs */
> >
> >  /* Event queue configuration bitmap flags */
> > -#define RTE_EVENT_QUEUE_CFG_DEFAULT            (0)
> > -/**< Default value of *event_queue_cfg* when rte_event_queue_setup() invoked
> > - * with queue_conf == NULL
> > - *
> > - * @see rte_event_queue_setup()
> > - */
> >  #define RTE_EVENT_QUEUE_CFG_TYPE_MASK          (3ULL << 0)
> >  /**< Mask for event queue schedule type configuration request */
> >  #define RTE_EVENT_QUEUE_CFG_ALL_TYPES          (0ULL << 0)
> > --
> > 2.7.4
> >
diff mbox

Patch

diff --git a/lib/librte_eventdev/rte_eventdev.c b/lib/librte_eventdev/rte_eventdev.c
index 68bfc3b..c32a776 100644
--- a/lib/librte_eventdev/rte_eventdev.c
+++ b/lib/librte_eventdev/rte_eventdev.c
@@ -593,7 +593,6 @@  rte_event_queue_setup(uint8_t dev_id, uint8_t queue_id,
 		RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->queue_def_conf,
 					-ENOTSUP);
 		(*dev->dev_ops->queue_def_conf)(dev, queue_id, &def_conf);
-		def_conf.event_queue_cfg = RTE_EVENT_QUEUE_CFG_DEFAULT;
 		queue_conf = &def_conf;
 	}
 
diff --git a/lib/librte_eventdev/rte_eventdev.h b/lib/librte_eventdev/rte_eventdev.h
index 7073987..d836f61 100644
--- a/lib/librte_eventdev/rte_eventdev.h
+++ b/lib/librte_eventdev/rte_eventdev.h
@@ -471,12 +471,6 @@  rte_event_dev_configure(uint8_t dev_id,
 /* Event queue specific APIs */
 
 /* Event queue configuration bitmap flags */
-#define RTE_EVENT_QUEUE_CFG_DEFAULT            (0)
-/**< Default value of *event_queue_cfg* when rte_event_queue_setup() invoked
- * with queue_conf == NULL
- *
- * @see rte_event_queue_setup()
- */
 #define RTE_EVENT_QUEUE_CFG_TYPE_MASK          (3ULL << 0)
 /**< Mask for event queue schedule type configuration request */
 #define RTE_EVENT_QUEUE_CFG_ALL_TYPES          (0ULL << 0)