[openib-general] RE: [RFC][PATCH] OpenIB uDAPL extension proposal - sample immed data and atomic api's

[Arlin Davis]

Kanevsky, Arkady wrote:

>comments inline.
>
>>>
>>As mentioned on the con-call, there are two separate items to 
>>consider while looking at the proposal. The first is the 
>>ability to extend DAT for specific provider value-add and the 
>>second is to validate the need for general atomic and 
>>immediate data functionality in the basic set of API's for 
>>all providers. I included atomics and immediate data as 
>>examples since it is specific to one provider (IB), it 
>>includes operations that require new ops, events, and event 
>>data types, and it also provides a working model to validate 
>>the extension model from request to completion events. I 
>>would like to concentrate on getting consensus on the 
>>extension proposal first if possible. Just try to think of 
>>the actual operations as some opaque dat_ext_foobar_op().
>>    
>>
>
>The thing that bothers me is that we already have several APIs
>that are transport specific. While some are possible to implement
>on other transports the others, like Socket CM, can not.
>So I view both of your specific extensions as transport specific
>amd hence prefer to add them as normal APIs not extensions.
>  
>
That would work for me.

>The secondary goal is that Provider can add extensions without requiring
>to change to DAT. These fall into 3 categories.
>1. New memory types including privilages and protection attributes.
>We can add "extension" entry to these structures. We need to check
>if this is sufficient. Think of shared memory for example.
>I am assuming no changes to PZ.
>2. New DTOs. The main issue is not DTOs but their completions and
>async errors. This is why Immediate data is better handled by
>incorporating into
>DAT spec while atomic can be handled by extensions. That is completion
>will return
>"extention" and Consumer will do the secondary switch on the extension
>type.
>Extension should not impact backwards compatibility.
>We had not looked at errors. But assuming a simple model that async
>errors
>break connection and we can return "extension error" with extensions
>defining
>new reason. Again details need to be polished.
>3. new connection types or CM models... New connections seems to have
>little impact
>on existing API assuming that EP type can be extended. The new
>connection can even
>restrict which DTO they can handle. CM model is more problematic.
>  
>
Nice summary. Yes, we need be thorough when flushing out all the
requirements for extensions in general. I am not sure how much I can
share at this point regarding any "other extensions" but if we think in
general terms we should cover all the necessary requirements. Do you
want to update the proposal based on your statements above? I would be
happy to work it into a real patch for feasibility and to provide
feedback based on future extentability.

>Arlin, it would be nice to consider some of your other extensions that
>are not
>transport specific to see how it will fit before we make the final
>decision.
>This should give us idea how extensible DAT "extension" model is.
>
>
>  
>
>>> 
>>>In general, extension route was intended for RNIC|HCA providers to 
>>>expose HW capabilities beyond IBTA, iWARP and VIA standards. The 
>>>standard RDMA functionality is best handle via spec addition.
>>>DAT 2.0 does it for FMR, remote and local memory 
>>>      
>>>
>>invalidation as well 
>>    
>>
>>>as others.
>>>      
>>>
>>True, but the extension route is not fully defined, 
>>documented, nor implemented. This is what I would like to 
>>work on getting completed in time for 2.0 if possible. 
>>
>>BTW: The existing implementation actually uses 
>>dapl_provider->extension to store the hca_ptr but the 
>>specification states that it is reserved for the providers 
>>private use (8.2.1 in DAPL1.2 spec). This is why I had to 
>>defined another extension_func in the patch.
>>
>>    
>>
>>> 
>>>I had posted a complete list of changes/addition to DAT 2.0 about a 
>>>month ago.
>>>But we had not discussed yet version change from 1.3 to 2.0 nor how 
>>>much backwards compatibility spec will provide.
>>> 
>>>2. What is IMMED_EVENT? is it just immediate data without 
>>>      
>>>
>>any payload one?
>>    
>>
>>>I suggest chnaging the name so it will not use "EVENT". 
>>>      
>>>
>>Just call it 
>>    
>>
>>>NO_PAYLOAD.
>>>Do you want to support 2 different way to delivery immediate data?
>>>One in event and one in data payload?
>>>Why? I would think that just an event way will do.
>>>      
>>>
>>This was modeled after the immediate data discussions on the 
>>DAT reflector based on iWARP requirements.
>>
>>http://groups.yahoo.com/group/dat-discussions/message/3285
>>
>>    
>>
>
>I recall it now.
>I want to consider a few usage cases.
>1. Existing app running on the Provider with extensions.
>Want to make sure we do not require any App changes beyond recompile
>due to extensions.
>  
>
agree

>2. App wants to be modified to use Immediate data. How big impact it
>has on existing code. For example buffer size allocation and completion
>handling
>  
>
It really depends on transport capabilities. Our current thinking has
two delivery mechanisms for the two transports (event and payload) which
is not optimal. If we can come to a consensus on delivering one way
(events) and simply add a new DTO post option it would reduce the
complexity considerably.

>for immediate data over existing connection.
>2a. Can application take advantage if it knows that Provider will return
>immediate data in event?
>  
>
Yes, and it requires no additional buffer management on the consumers if
providers support this model.

>2b. Immediate data inline only?
>  
>
That requires more buffer management on the applications part.

>3. Ditto for atomic operations over existing connection.
>
>  
>
>>> 
>>>3. I suggest beefing up DAT_DTO_COMPLETION_EVENT_DATA and 
>>>      
>>>
>>DAT_DTOS to 
>>    
>>
>>>convey which operation completed and return Immediate data 
>>>      
>>>
>>if complete 
>>    
>>
>>>operation had immediate data.
>>>Since we already modified these 2 struct as part of DAT 2.0 change 
>>>lets add your proposal to the change.
>>>This will allow Consumers to use single approach to deal with 
>>>completions, extension to the current one but not a 
>>>      
>>>
>>structural one. No 
>>    
>>
>>>need for DAT_EXTENSION_DATA, DAT_EXT_EVENT_TYPE, DAT_EXT_OP nor the 
>>>whole mechanism for extended ops.
>>>      
>>>
>>You still need extension types for the "other" value-add 
>>operations/evnts that will not be accepted as standard and 
>>are vendor specific.
>> 
>>I would like to defer the rest of the questions for now since 
>>they touch on actual operations and not the extension 
>>mechanism. Although, I do need to think about how to extend 
>>memory registration privledges. Any suggestions?
>>    
>>
>
>Going with your generic extension design
>we add "extension" entry to relevant data structures.
>And then outside DAT define the structure for its values which can be
>extensible. This imply that adding extension by Provider
>will force apps to be recompiled. I hope this is enough.
>I am assuming that apps use values not position the structures.
>  
>
Yes, a new build against the extented definitions would suffice, along
with some validation at load and open times.

Can you and James send out an extension patch based on some of your
ideas if they differ from the original patch. Or, like I said before, if
you update the proposal I would be happy to work on a new patch.

thanks,

-arlin