Re: [dpdk-dev] [EXT] Re: [PATCH v1 2/7] eal/interrupts: implement get set APIs

[Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>]

2021-10-04 10:37 (UTC+0000), Harman Kalra:
> [...]
> > > +struct rte_intr_handle *rte_intr_handle_instance_index_get(
> > > +				struct rte_intr_handle *intr_handle, int  
> > index)
> > 
> > If rte_intr_handle_instance_alloc() returns a pointer to an array, this function
> > is useless since the user can simply manipulate a pointer.  
> 
> <HK> User wont be able to manipulate the pointer as he is not aware of size of struct rte_intr_handle.
> He will observe "dereferencing pointer to incomplete type" compilation error.

Sorry, my bad.

> > If we want to make a distinction between a single struct rte_intr_handle and
> > a commonly allocated bunch of such (but why?), then they should be
> > represented by distinct types.  
> 
> <HK> Do you mean, we should have separate APIs for single allocation and batch allocation? As get API
> will be useful only in case of batch allocation. Currently interrupt autotests and ifpga_rawdev driver makes
> batch allocation. 
> I think common API for single and batch is fine, get API is required for returning a particular intr_handle instance.
> But one problem I see in current implementation is there should be upper limit check for index in get/set
> API, which I will fix.

I don't think we need different APIs, I was asking if it was your intention.
Now I understand it and agree with you.

> > > +int rte_intr_handle_instance_index_set(struct rte_intr_handle  
> > *intr_handle,  
> > > +				       const struct rte_intr_handle *src,
> > > +				       int index)  
> > 
> > See above regarding the "index" parameter. If it can be removed, a better
> > name for this function would be rte_intr_handle_copy().  
> 
> <HK> I think get API is required.

Maybe index is still not needed: "intr_handle" can just be a pointer to the
right item obtained with rte_intr_handle_instance_index_get(). This way you
also don't need to duplicate the index-checking logic.