> -----Original Message-----
> From: Jerin Jacob [mailto:jerin.ja...@caviumnetworks.com]
> Sent: Thursday, August 3, 2017 1:23 AM
> To: Eads, Gage <gage.e...@intel.com>
> Cc: Rao, Nikhil <nikhil....@intel.com>; dev@dpdk.org; tho...@monjalon.net;
> Richardson, Bruce <bruce.richard...@intel.com>; Van Haaren, Harry
> <harry.van.haa...@intel.com>; hemant.agra...@nxp.com;
> nipun.gu...@nxp.com; Vangati, Narender <narender.vang...@intel.com>;
> Gujjar, Abhinandan S <abhinandan.guj...@intel.com>
> Subject: Re: [PATCH 1/2] eventdev: add event adapter for ethernet Rx queues
> 
> -----Original Message-----
> > Date: Wed, 2 Aug 2017 19:19:32 +0000
> > From: "Eads, Gage" <gage.e...@intel.com>
> > To: Jerin Jacob <jerin.ja...@caviumnetworks.com>, "Rao, Nikhil"
> >  <nikhil....@intel.com>
> > CC: "dev@dpdk.org" <dev@dpdk.org>, "tho...@monjalon.net"
> >  <tho...@monjalon.net>, "Richardson, Bruce"
> > <bruce.richard...@intel.com>,  "Van Haaren, Harry"
> <harry.van.haa...@intel.com>, "hemant.agra...@nxp.com"
> >  <hemant.agra...@nxp.com>, "nipun.gu...@nxp.com"
> > <nipun.gu...@nxp.com>,  "Vangati, Narender"
> <narender.vang...@intel.com>, "Gujjar, Abhinandan S"
> >  <abhinandan.guj...@intel.com>
> > Subject: RE: [PATCH 1/2] eventdev: add event adapter for ethernet Rx
> > queues
> >
> >
> > <snip>
> >
> > > > >
> > > > > 5) specifying rte_event_eth_rx_adapter_conf.rx_event_port_id on
> > > > > rte_event_eth_rx_adapter_create() would waste one HW eventdev
> > > > > port if its happen to be used RX_ADAPTER_CAP_INBUILT_PORT on
> > > rte_event_eth_rx_adapter_queue_add().
> > > > > unlike SW eventdev port, HW eventdev ports are costly so I
> > > > > think, We need to have another eventdev PMD ops to create
> service/producer ports.
> > > > > Or any other scheme that creates
> > > > > rte_event_eth_rx_adapter_conf.rx_event_port_id
> > > > > on demand by common code.
> > > > >
> > > >
> > > > One solution is:
> > > >
> > > > struct rte_event_eth_rx_adapter_conf {
> > > >     uint8_t dev_id;
> > > >
> > > >     int (*conf_cb)(uint8_t id, uint8_t port_id, uint32_t flags,
> > > > struct rte_event_eth_rx_adapter_conf *conf);
> > > >
> > > >     unsigned int max_nb_rx;
> > > >
> > > >     int event_port_id;
> > > >
> > > >     char service_name[];
> > > > }
> > > >
> > > > Where dev_id and conf_cb have to be specified in the create call,
> > > > but event_port_id and service_name will be filled in when
> > > > conf_cb() is invoked
> > >
> > > I was thinking like event_port_id will be rte_event_port_count() + 1.
> > > ie When adapter needs the additional port, It can
> > > - stop the eventdev
> > > - reconfigure with rte_event_queue_count() , rte_event_port_count()
> > > + 1
> > > - start the eventdev.
> > >
> > > The only problem with callback is that all the application needs to 
> > > implement
> it.
> > > If you think, application need more control then we can expose
> > > callback and if it is NULL then default handler can be called in common 
> > > code.
> > >
> >
> > I don't think we can rely on there being another port available -- a user 
> > may
> have configured the sw eventdev with all 64 ports, for instance.
> 
> On that case, irrespective any scheme(callback vs non callback) the adapter
> creation would fail. Right?
> 
> > What if the user is required to calculate cfg.nb_event_ports as a function 
> > of
> the RX_ADAPTER_CAP_INBUILT_PORT capability (i.e. add a port if the capability
> is not set), such that a reconfigure is not required?
> 
> We have only one NON INBUILT eventdev port per adapter. Right? i.e in the v1
> spec it was rte_event_eth_rx_adapter_conf.event_port_id,
> How about it can be rte_event_port_count() + 1 ? Since we are NOT linking this
> port, the context call be kept in adapter itself. Right?

It could be. Thinking on it some more, I'm a little concerned about doing 
configuration without the application's knowledge. Possible issues that could 
arise:
- The user later reconfigures the event device with fewer ports and the 
adapter's port becomes invalid, or reconfigures it with more ports and begins 
using the port the adapter is using
- rte_event_port_count() + 1 extends beyond the PMD's capabilities (the sw PMD 
is hard-coded to support a max of 64 ports, for example)

Having the user be responsible for the port configuration could avoid these 
problems. Since the user needs to check the <eventdev, ethdev> pair's 
capabilities for the CAP_ADD_QUEUE anyway, they could also check for 
INBUILT_PORT and decide whether or not to request an additional port at 
eventdev configure time -- thereby ensuring they don't waste a port when using 
hardware with inbuilt ports. And this keeps the configuration code in one place 
(the app), rather than spread across the app, adapter, and potentially the 
conf_cb.

Besides these concerns, I think the transparent configuration approach (plus 
conf_cb when necessary to override) would work, but could have issues in the 
aforementioned edge cases.

> >
> > As for application control: that would be a useful option in the conf_cb
> scheme. Some apps will want to configure the adapter's port (its
> new_event_threshold, its queue depths) differently from the default.
> 
> struct rte_event_port_conf * can be passed on the adapter create if 
> application
> needs more control.
> 
> >
> > Thanks,
> > Gage

Reply via email to