[dpdk-dev] [PATCH v2 1/6] eventdev: introduce event driven programming model

Jerin Jacob jerin.jacob at caviumnetworks.com
Wed Dec 14 14:13:58 CET 2016


On Thu, Dec 08, 2016 at 11:02:16AM +0000, Van Haaren, Harry wrote:
> > From: Jerin Jacob [mailto:jerin.jacob at caviumnetworks.com]
> > Sent: Thursday, December 8, 2016 1:24 AM
> > To: Van Haaren, Harry <harry.van.haaren at intel.com>
> 
> <snip>
> 
> > > * Operation and sched_type *increased* to 4 bits each (from previous value of 2) to
> > allow future expansion without ABI changes
> > 
> > Anyway it will break ABI if we add new operation. I would propose to keep 4bit
> > reserved and add it when required.
> 
> Ok sounds good. I'll suggest to move it to the middle between operation or sched type, which would allow expanding operation without ABI breaks. On expanding the field would remain in the same place with the same bits available in that place (no ABI break), but new bits can be added into the currently reserved space.

OK. We will move the rsvd field as you suggested.

> 
> 
> > > * Restore flow_id to 24 bits of a 32 bit int (previous size was 20 bits)
> > > * sub-event-type reduced to 4 bits (previous value was 8 bits). Can we think of
> > situations where 16 values for application specified identifiers of each event-type is
> > genuinely not enough?
> > One packet will not go beyond 16 stages but an application may have more stages and
> > each packet may go mutually exclusive stages. For example,
> > 
> > packet 0: stagex_0 ->stagex_1
> > packet 1: stagey_0 ->stagey_1
> > 
> > In that sense, IMO, more than 16 is required.(AFIAK, VPP has any much larger limit on
> > number of stages)
> 
> My understanding was that stages are linked to event queues, so the application can determine the stage the packet comes from by reading queue_id?

That is one way of doing it. But it is limited to number of queues
therefore scalability issues.Another approach is through
sub_event_type scheme without depended on the number of queues.

> 
> I'm not opposed to having an 8 bit sub_event_type, but it seems unnecessarily large from my point of view. If you have a use for it, I'm ok with 8 bits.

OK

> 
> 
> > > In my opinion this structure layout is more balanced, and will perform better due to
> > less loads that will need masking to access the required value.
> > OK. Considering more balanced layout and above points. I propose following scheme(based on
> > your input)
> > 
> > 	union {
> > 		uint64_t event;
> > 		struct {
> > 			uint32_t flow_id: 20;
> > 			uint32_t sub_event_type : 8;
> > 			uint32_t event_type : 4;
> > 
> > 			uint8_t rsvd: 4; /* for future additions */
> > 			uint8_t operation  : 2; /* new fwd drop */
> > 			uint8_t sched_type : 2;
> > 
> > 			uint8_t queue_id;
> > 			uint8_t priority;
> > 			uint8_t impl_opaque;
> > 		};
> > 	};
> > 
> > Feedback and improvements welcomed,
> 
> 
> So incorporating my latest suggestions on moving fields around, excluding sub_event_type *size* changes:
> 
> union {
> 	uint64_t event;
> 	struct {
> 		uint32_t flow_id: 20;
> 		uint32_t event_type : 4;
> 		uint32_t sub_event_type : 8; /* 8 bits now naturally aligned */

Just one suggestion here. I am not sure about the correct split between
number of bits to represent flow_id and sub_event_type fields. And its
connected in our implementation, so I propose to move sub_event_type up so
that future flow_id/sub_event_type bit field size change request wont
impact our implementation. Since it is represented as 32bit as whole, I
don't think there is an alignment issue.

So incorporating my latest suggestions on moving sub_event_type field around:

union {
	uint64_t event;
	struct {
		uint32_t flow_id: 20;
		uint32_t sub_event_type : 8;
		uint32_t event_type : 4;

		uint8_t operation  : 2; /* new fwd drop */
		uint8_t rsvd: 4; /* for future additions, can be expanded into without ABI break */
		uint8_t sched_type : 2;

		uint8_t queue_id;
		uint8_t priority;
		uint8_t impl_opaque;
	};
};



More information about the dev mailing list