Re: [dpdk-dev] [PATCH 1/4] lib: introduce IF Proxy library

["Andrzej Ostruszka [C]" <aostruszka@marvell.com>]

On 4/8/20 5:04 AM, Varghese, Vipin wrote:
> Hi Andrzej,
> 
> Thanks for the reply. Please find explanations for some of the queries 
> 
> snipped
>>>> +uint64_t rte_ifpx_events_available(void) {
>>>> +	/* All events are supported on Linux. */
>>>> +	return (1ULL << RTE_IFPX_NUM_EVENTS) - 1;
>>> Should we give the available from the used count?
>>
>> I'm not sure I follow what you wanted to ask.  I want to return bitmask with
>> each bit being lit for every event type.  I could go with or'ing of all (1ULL <<
>> RTE_IFPX_MAC_CHANGE) | (1ULL << RTE_IFPX_MTU_CHANGE) ...
>> but deemed that this would be simpler.
> 
> I assume the function `rte_ifpx_events_available` returns current available events. That is at time t0, if we have used 3 events the return of function will give back ` return ((1ULL << RTE_IFPX_NUM_EVENTS) - 1 -  ifpx_consumed_events);`.

It returns events available on given platform - static thing, dependent
on the implementation of IF Proxy (currently only Linux supported though
- and it supports all events defined so far).

>>>> +
>>>> +void rte_ifpx_callbacks_unregister(void)
>>>> +{
>>>> +	rte_spinlock_lock(&ifpx_lock);
>>>> +	memset(&ifpx_callbacks.cbs, 0, sizeof(ifpx_callbacks.cbs));
>>> What would happen to pending events, are agreeing to drop all?
>>
>> ifpx_events_notify() is called under the same lock.  So either someone calls this
>> unregister and then notify will not find any callback or the other way.  Note
>> that notify drops the lock for the time of callback call (to allow modifications
>> from the callback) but the pointer is first copied - so the behaviour would be as
>> if the unregister was called later.
>>
>> I'm not sure I answered your question though - if not then please ask again
>> with some more details.
> 
> Let us assume we have 3 callbacks to service for event_a namely cb-1, cb-2, and cb-3. So tail-list cb-1->cb-2->cb3, the user invoked unregister. What will happen to the 3 events? Should we finish the 3 callback handler and then remove.

Hhhmmm, have you been reviewing latest version?  With the introduction
of event queues there is now only one global set of callbacks (no list),
so only 1 callback for each possible event type.

>>> Assuming all the events are executed `if and only if` the current process if
>> Primary? If it is secondary for physical interface certain `rte_eth_api` will fail.
>> Can we have check the events are processed for primary only?
>>
>> Yes that was my assumption however at the moment I'm using:
>> - rte_eth_iterator_init/next/cleanup()
>> - rte_eth_dev_info_get()
>> - rte_eth_dev_get_mtu()
>> - rte_eth_macaddr_get()
>> - rte_eth_dev_mac_addr_add()
>> - rte_dev_probe/remove()
>>
>> Is there a problem with these?  If it is, then I'll think about adding check for
>> secondary.
> Based on my limited testing with PF and VF, certain functions works and other do not. In case of TUN PMD set/get mac_addr is not present.

TUN is not being used (for that reason) - only TAP.  I could add check
for PRIMARY, but that way I would be artificially excluding cases where
that would work without the change.  So for now I intend to leave things
like they are and address the actual problem (if it pops up).  Note also
that I'm not checking errors for the mac_get/set so if given
functionality is not supported nothing will happen.

With regards
Andrzej Ostruszka