Re: [PATCH V2 3/6] ethdev: fix push new event

["lihuisong (C)" <lihuisong@huawei.com>]

在 2022/10/8 12:09, lihuisong (C) 写道:
>
> 在 2022/9/27 18:49, Thomas Monjalon 写道:
>> 15/09/2022 14:45, Huisong Li:
>>> The 'state' in struct rte_eth_dev may be used to update some 
>>> information
>>> when app receive these events. For example, when app receives a new 
>>> event,
>>> app may get the socket id of this port by calling 
>>> rte_eth_dev_socket_id to
>>> setup the attached port. The 'state' is used in rte_eth_dev_socket_id.
>>>
>>> If the state isn't modified to RTE_ETH_DEV_ATTACHED before pushing 
>>> the new
>>> event, app will get the socket id failed. So this patch moves 
>>> pushing event
>>> operation after the state updated.
>>>
>>> Fixes: 99a2dd955fba ("lib: remove librte_ prefix from directory names")
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>>> ---
>>>   lib/ethdev/ethdev_driver.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/lib/ethdev/ethdev_driver.c b/lib/ethdev/ethdev_driver.c
>>> index a285f213f0..a6616f072b 100644
>>> --- a/lib/ethdev/ethdev_driver.c
>>> +++ b/lib/ethdev/ethdev_driver.c
>>> @@ -206,9 +206,9 @@ rte_eth_dev_probing_finish(struct rte_eth_dev *dev)
>>>       if (rte_eal_process_type() == RTE_PROC_SECONDARY)
>>>           eth_dev_fp_ops_setup(rte_eth_fp_ops + dev->data->port_id, 
>>> dev);
>>>   -    rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_NEW, NULL);
>>>         dev->state = RTE_ETH_DEV_ATTACHED;
>>> +    rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_NEW, NULL);
>>>   }
>> As explained in the first patch, I don't think it is a good solution.
>> We should not allow the port to be used until the end of probing.
>> When RTE_ETH_EVENT_NEW is sent, the device is allocated but
>> not ready for use. If an entity like failsafe decides to take ownership
>> of the port, then the application should not consider it at all.
>> For these reasons, we should limit which operations can be done
>> during RTE_ETH_EVENT_NEW processing.
>> That's why I've proposed creating a new state RTE_ETH_DEV_ALLOCATED,
>> not sure why you didn't follow this advice.
> I want to put this problem to this case this patchset mentions, so as to
> have a better discussion. From the first patch, I know what you mean.
> But I still have some confusion:
> When a device taken by failsafe PMD push new event, all event callback
> will be called under this device. As you suggested, the device is a valid
> port if its state is ALLOCATED or ATTACHED. Now, the macro 
> RTE_ETH_FOREACH_DEV
> uses the condition that device is valid(state is ALLOCATED or ATTACHED)
> and NOT owned.
>
> Even if we add the new ALLOCATED state, this device taken by failsafe is
> also a valid port in new event callback in application, and is in 'NO 
> OWNER'.
> if event callback of application is called before one of failsafe PMD.
> As a result, application still can operate this device directly.
> Can we make sure that event callback of failsafe PMD is before one of 
> application?
Hi Thomas,
Can you look at my confusion?
>>
>>
>>
>>
>> .
> .