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

[Van Haaren, Harry]

> 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.


> > * 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?

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.


> > 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 */

		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;
	};
};


-Harry