[PATCH] ethdev: fix push new event

[PATCH] ethdev: fix push new event

[Min Hu (Connor)]

From: Huisong Li <lihuisong@huawei.com>

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>
Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 lib/ethdev/rte_ethdev.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index cea2f0b498..f555647c7a 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -4816,9 +4816,8 @@ 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);
 }
 
 int
-- 
2.33.0

Re: [PATCH] ethdev: fix push new event

[David Marchand]

On Sat, May 21, 2022 at 8:57 AM Min Hu (Connor) <humin29@huawei.com> wrote:
>
> From: Huisong Li <lihuisong@huawei.com>
>
> 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")

A patch moving code is unlikely to be at fault.


Looking at the patch which moved those notifications in this point of
the code, the state update was pushed after the notification on
purpose.
See be8cd210379a ("ethdev: fix port probing notification")

    ethdev: fix port probing notification

    The new device was notified as soon as it was allocated.
    It leads to use a device which is not yet initialized.

    The notification must be published after the initialization is done
    by the PMD, but before the state is changed, in order to let
    notified entities taking ownership before general availability.


Do we need an intermediate state during probing?


-- 
David Marchand

Re: [PATCH] ethdev: fix push new event

[Ferruh Yigit]

On 5/23/2022 10:51 AM, David Marchand wrote:
> [CAUTION: External Email]
> 
> On Sat, May 21, 2022 at 8:57 AM Min Hu (Connor) <humin29@huawei.com> wrote:
>>
>> From: Huisong Li <lihuisong@huawei.com>
>>
>> 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")
> 
> A patch moving code is unlikely to be at fault.
> 
> 
> Looking at the patch which moved those notifications in this point of
> the code, the state update was pushed after the notification on
> purpose.
> See be8cd210379a ("ethdev: fix port probing notification")
> 
>      ethdev: fix port probing notification
> 
>      The new device was notified as soon as it was allocated.
>      It leads to use a device which is not yet initialized.
> 
>      The notification must be published after the initialization is done
>      by the PMD, but before the state is changed, in order to let
>      notified entities taking ownership before general availability.
> 
> 
> Do we need an intermediate state during probing?
> 

As David highlight above commit, sending event before setting state to 
'RTE_ETH_DEV_ATTACHED' seems done intentionally.

I guess this is done mainly because of failsafe PMD, which registers the 
'RTE_ETH_EVENT_NEW' event and take sub-device ownership.

The ethdev iterate macros, like 'RTE_ETH_FOREACH_DEV', uses the 
condition that device is NOT owned and device state is NOT UNUSED.
Sending event before device state set to 'ATTACHED' lets failsafe to own 
the device so that it won't be visible to application at all via 
mentioned macros.


But I think the need for the libraries and application can be different, 
unlike failsafe PMD application may want notification on fully probed 
device, as the commit log of this patch mentions.

Perhaps solution can be as David mentioned, to create two events, one 
for libraries (failsafe) and other for applications.

Re: [PATCH] ethdev: fix push new event

[Thomas Monjalon]

23/05/2022 11:51, David Marchand:
> On Sat, May 21, 2022 at 8:57 AM Min Hu (Connor) <humin29@huawei.com> wrote:
> >
> > From: Huisong Li <lihuisong@huawei.com>
> >
> > 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")
> 
> A patch moving code is unlikely to be at fault.
> 
> 
> Looking at the patch which moved those notifications in this point of
> the code, the state update was pushed after the notification on
> purpose.
> See be8cd210379a ("ethdev: fix port probing notification")
> 
>     ethdev: fix port probing notification
> 
>     The new device was notified as soon as it was allocated.
>     It leads to use a device which is not yet initialized.
> 
>     The notification must be published after the initialization is done
>     by the PMD, but before the state is changed, in order to let
>     notified entities taking ownership before general availability.
> 
> 
> Do we need an intermediate state during probing?

Possibly. Currently we have only 3 states:
	RTE_ETH_DEV_UNUSED
	RTE_ETH_DEV_ATTACHED
	RTE_ETH_DEV_REMOVED

We may add RTE_ETH_DEV_ALLOCATED just before calling
	rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_NEW, NULL);
Then we would need to check against RTE_ETH_DEV_ALLOCATED
in some ethdev functions.

Re: [PATCH] ethdev: fix push new event

[lihuisong (C)]

[-- Attachment #1: Type: text/plain, Size: 2480 bytes --]


在 2022/5/23 22:36, Thomas Monjalon 写道:
> 23/05/2022 11:51, David Marchand:
>> On Sat, May 21, 2022 at 8:57 AM Min Hu (Connor)<humin29@huawei.com>  wrote:
>>> From: Huisong Li<lihuisong@huawei.com>
>>>
>>> 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")
>> A patch moving code is unlikely to be at fault.
>>
>>
>> Looking at the patch which moved those notifications in this point of
>> the code, the state update was pushed after the notification on
>> purpose.
>> See be8cd210379a ("ethdev: fix port probing notification")
>>
>>      ethdev: fix port probing notification
>>
>>      The new device was notified as soon as it was allocated.
>>      It leads to use a device which is not yet initialized.
>>
>>      The notification must be published after the initialization is done
>>      by the PMD, but before the state is changed, in order to let
>>      notified entities taking ownership before general availability.
>>
>>
>> Do we need an intermediate state during probing?
> Possibly. Currently we have only 3 states:
> 	RTE_ETH_DEV_UNUSED
> 	RTE_ETH_DEV_ATTACHED
> 	RTE_ETH_DEV_REMOVED
>
> We may add RTE_ETH_DEV_ALLOCATED just before calling
> 	rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_NEW, NULL);
> Then we would need to check against RTE_ETH_DEV_ALLOCATED
> in some ethdev functions.
>
Hi, Thomas,

Do you mean that we need to modify some funcions like following?

int rte_eth_dev_is_valid_port(uint16_t port_id)
{
     if (port_id >= RTE_MAX_ETHPORTS ||
         (rte_eth_devices[port_id].state != *RTE_ETH_DEV_ALLOCATED*))
         return 0;
     else
         return 1;
}

uint16_t rte_eth_find_next(uint16_t port_id)
{
     while (port_id < RTE_MAX_ETHPORTS &&
             rte_eth_devices[port_id].state != *RTE_ETH_DEV_ALLOCATED*)
         port_id++;

     if (port_id >= RTE_MAX_ETHPORTS)
         return RTE_MAX_ETHPORTS;

     return port_id;
}
>
> .

[-- Attachment #2: Type: text/html, Size: 3483 bytes --]

Re: [PATCH] ethdev: fix push new event

[Thomas Monjalon]

28/05/2022 10:53, lihuisong (C):
> 
> 在 2022/5/23 22:36, Thomas Monjalon 写道:
> > 23/05/2022 11:51, David Marchand:
> >> On Sat, May 21, 2022 at 8:57 AM Min Hu (Connor)<humin29@huawei.com>  wrote:
> >>> From: Huisong Li<lihuisong@huawei.com>
> >>>
> >>> 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")
> >> A patch moving code is unlikely to be at fault.
> >>
> >>
> >> Looking at the patch which moved those notifications in this point of
> >> the code, the state update was pushed after the notification on
> >> purpose.
> >> See be8cd210379a ("ethdev: fix port probing notification")
> >>
> >>      ethdev: fix port probing notification
> >>
> >>      The new device was notified as soon as it was allocated.
> >>      It leads to use a device which is not yet initialized.
> >>
> >>      The notification must be published after the initialization is done
> >>      by the PMD, but before the state is changed, in order to let
> >>      notified entities taking ownership before general availability.
> >>
> >>
> >> Do we need an intermediate state during probing?
> > Possibly. Currently we have only 3 states:
> > 	RTE_ETH_DEV_UNUSED
> > 	RTE_ETH_DEV_ATTACHED
> > 	RTE_ETH_DEV_REMOVED
> >
> > We may add RTE_ETH_DEV_ALLOCATED just before calling
> > 	rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_NEW, NULL);
> > Then we would need to check against RTE_ETH_DEV_ALLOCATED
> > in some ethdev functions.
> >
> Hi, Thomas,
> 
> Do you mean that we need to modify some funcions like following?
> 
> int rte_eth_dev_is_valid_port(uint16_t port_id)
> {
>      if (port_id >= RTE_MAX_ETHPORTS ||
>          (rte_eth_devices[port_id].state != *RTE_ETH_DEV_ALLOCATED*))
>          return 0;
>      else
>          return 1;
> }
> 
> uint16_t rte_eth_find_next(uint16_t port_id)
> {
>      while (port_id < RTE_MAX_ETHPORTS &&
>              rte_eth_devices[port_id].state != *RTE_ETH_DEV_ALLOCATED*)
>          port_id++;
> 
>      if (port_id >= RTE_MAX_ETHPORTS)
>          return RTE_MAX_ETHPORTS;
> 
>      return port_id;
> }

Yes this is what I mean.

Re: [PATCH] ethdev: fix push new event

[Ferruh Yigit]

On 5/30/2022 9:28 AM, Thomas Monjalon wrote:
> [CAUTION: External Email]
> 
> 28/05/2022 10:53, lihuisong (C):
>>
>> 在 2022/5/23 22:36, Thomas Monjalon 写道:
>>> 23/05/2022 11:51, David Marchand:
>>>> On Sat, May 21, 2022 at 8:57 AM Min Hu (Connor)<humin29@huawei.com>  wrote:
>>>>> From: Huisong Li<lihuisong@huawei.com>
>>>>>
>>>>> 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")
>>>> A patch moving code is unlikely to be at fault.
>>>>
>>>>
>>>> Looking at the patch which moved those notifications in this point of
>>>> the code, the state update was pushed after the notification on
>>>> purpose.
>>>> See be8cd210379a ("ethdev: fix port probing notification")
>>>>
>>>>       ethdev: fix port probing notification
>>>>
>>>>       The new device was notified as soon as it was allocated.
>>>>       It leads to use a device which is not yet initialized.
>>>>
>>>>       The notification must be published after the initialization is done
>>>>       by the PMD, but before the state is changed, in order to let
>>>>       notified entities taking ownership before general availability.
>>>>
>>>>
>>>> Do we need an intermediate state during probing?
>>> Possibly. Currently we have only 3 states:
>>>      RTE_ETH_DEV_UNUSED
>>>      RTE_ETH_DEV_ATTACHED
>>>      RTE_ETH_DEV_REMOVED
>>>
>>> We may add RTE_ETH_DEV_ALLOCATED just before calling
>>>      rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_NEW, NULL);
>>> Then we would need to check against RTE_ETH_DEV_ALLOCATED
>>> in some ethdev functions.
>>>
>> Hi, Thomas,
>>
>> Do you mean that we need to modify some funcions like following?
>>
>> int rte_eth_dev_is_valid_port(uint16_t port_id)
>> {
>>       if (port_id >= RTE_MAX_ETHPORTS ||
>>           (rte_eth_devices[port_id].state != *RTE_ETH_DEV_ALLOCATED*))
>>           return 0;

Won't this mark ATTACHED devices as invalid?

If the state flow will be as UNUSED -> ALLOCATED -> ATTACHED, above 
check should be against 'ATTACHED' I think.

>>       else
>>           return 1;
>> }
>>
>> uint16_t rte_eth_find_next(uint16_t port_id)
>> {
>>       while (port_id < RTE_MAX_ETHPORTS &&
>>               rte_eth_devices[port_id].state != *RTE_ETH_DEV_ALLOCATED*)
>>           port_id++;
>>
>>       if (port_id >= RTE_MAX_ETHPORTS)
>>           return RTE_MAX_ETHPORTS;
>>
>>       return port_id;
>> }
> 
> Yes this is what I mean.
> 
> 

Re: [PATCH] ethdev: fix push new event

[lihuisong (C)]

在 2022/5/30 19:10, Ferruh Yigit 写道:
> On 5/30/2022 9:28 AM, Thomas Monjalon wrote:
>> [CAUTION: External Email]
>>
>> 28/05/2022 10:53, lihuisong (C):
>>>
>>> 在 2022/5/23 22:36, Thomas Monjalon 写道:
>>>> 23/05/2022 11:51, David Marchand:
>>>>> On Sat, May 21, 2022 at 8:57 AM Min Hu 
>>>>> (Connor)<humin29@huawei.com>  wrote:
>>>>>> From: Huisong Li<lihuisong@huawei.com>
>>>>>>
>>>>>> 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")
>>>>> A patch moving code is unlikely to be at fault.
>>>>>
>>>>>
>>>>> Looking at the patch which moved those notifications in this point of
>>>>> the code, the state update was pushed after the notification on
>>>>> purpose.
>>>>> See be8cd210379a ("ethdev: fix port probing notification")
>>>>>
>>>>>       ethdev: fix port probing notification
>>>>>
>>>>>       The new device was notified as soon as it was allocated.
>>>>>       It leads to use a device which is not yet initialized.
>>>>>
>>>>>       The notification must be published after the initialization 
>>>>> is done
>>>>>       by the PMD, but before the state is changed, in order to let
>>>>>       notified entities taking ownership before general availability.
>>>>>
>>>>>
>>>>> Do we need an intermediate state during probing?
>>>> Possibly. Currently we have only 3 states:
>>>>      RTE_ETH_DEV_UNUSED
>>>>      RTE_ETH_DEV_ATTACHED
>>>>      RTE_ETH_DEV_REMOVED
>>>>
>>>> We may add RTE_ETH_DEV_ALLOCATED just before calling
>>>>      rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_NEW, NULL);
>>>> Then we would need to check against RTE_ETH_DEV_ALLOCATED
>>>> in some ethdev functions.
>>>>
>>> Hi, Thomas,
>>>
>>> Do you mean that we need to modify some funcions like following?
>>>
>>> int rte_eth_dev_is_valid_port(uint16_t port_id)
>>> {
>>>       if (port_id >= RTE_MAX_ETHPORTS ||
>>>           (rte_eth_devices[port_id].state != *RTE_ETH_DEV_ALLOCATED*))
>>>           return 0;
>
> Won't this mark ATTACHED devices as invalid?
Yes, You are right.
>
> If the state flow will be as UNUSED -> ALLOCATED -> ATTACHED, above 
> check should be against 'ATTACHED' I think.

If these check is against 'ATTACHED', it goes back to the issue this 
patch mentioned.

The failsafe PMD applications expect sending event before device state 
set to 'ATTACHED'.
But other applications expect the device with 'ATTACHED' state before 
send event.
They are in conflict with each other. So we can't solve this issue by 
adding an
'RTE_ETH_DEV_ALLOCATED' state.

>
>>>       else
>>>           return 1;
>>> }
>>>
>>> uint16_t rte_eth_find_next(uint16_t port_id)
>>> {
>>>       while (port_id < RTE_MAX_ETHPORTS &&
>>>               rte_eth_devices[port_id].state != 
>>> *RTE_ETH_DEV_ALLOCATED*)
>>>           port_id++;
>>>
>>>       if (port_id >= RTE_MAX_ETHPORTS)
>>>           return RTE_MAX_ETHPORTS;
>>>
>>>       return port_id;
>>> }
>>
>> Yes this is what I mean.
>>
>>
>
> .

Re: [PATCH] ethdev: fix push new event

[Thomas Monjalon]

02/06/2022 13:24, lihuisong (C):
> 
> 在 2022/5/30 19:10, Ferruh Yigit 写道:
> > On 5/30/2022 9:28 AM, Thomas Monjalon wrote:
> >> [CAUTION: External Email]
> >>
> >> 28/05/2022 10:53, lihuisong (C):
> >>>
> >>> 在 2022/5/23 22:36, Thomas Monjalon 写道:
> >>>> 23/05/2022 11:51, David Marchand:
> >>>>> On Sat, May 21, 2022 at 8:57 AM Min Hu 
> >>>>> (Connor)<humin29@huawei.com>  wrote:
> >>>>>> From: Huisong Li<lihuisong@huawei.com>
> >>>>>>
> >>>>>> 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")
> >>>>> A patch moving code is unlikely to be at fault.
> >>>>>
> >>>>>
> >>>>> Looking at the patch which moved those notifications in this point of
> >>>>> the code, the state update was pushed after the notification on
> >>>>> purpose.
> >>>>> See be8cd210379a ("ethdev: fix port probing notification")
> >>>>>
> >>>>>       ethdev: fix port probing notification
> >>>>>
> >>>>>       The new device was notified as soon as it was allocated.
> >>>>>       It leads to use a device which is not yet initialized.
> >>>>>
> >>>>>       The notification must be published after the initialization 
> >>>>> is done
> >>>>>       by the PMD, but before the state is changed, in order to let
> >>>>>       notified entities taking ownership before general availability.
> >>>>>
> >>>>>
> >>>>> Do we need an intermediate state during probing?
> >>>> Possibly. Currently we have only 3 states:
> >>>>      RTE_ETH_DEV_UNUSED
> >>>>      RTE_ETH_DEV_ATTACHED
> >>>>      RTE_ETH_DEV_REMOVED
> >>>>
> >>>> We may add RTE_ETH_DEV_ALLOCATED just before calling
> >>>>      rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_NEW, NULL);
> >>>> Then we would need to check against RTE_ETH_DEV_ALLOCATED
> >>>> in some ethdev functions.
> >>>>
> >>> Hi, Thomas,
> >>>
> >>> Do you mean that we need to modify some funcions like following?
> >>>
> >>> int rte_eth_dev_is_valid_port(uint16_t port_id)
> >>> {
> >>>       if (port_id >= RTE_MAX_ETHPORTS ||
> >>>           (rte_eth_devices[port_id].state != *RTE_ETH_DEV_ALLOCATED*))
> >>>           return 0;
> >
> > Won't this mark ATTACHED devices as invalid?
> Yes, You are right.
> 
> > If the state flow will be as UNUSED -> ALLOCATED -> ATTACHED, above 
> > check should be against 'ATTACHED' I think.

It should validate both ALLOCATED and ATTACHED.


> If these check is against 'ATTACHED', it goes back to the issue this 
> patch mentioned.
> 
> The failsafe PMD applications expect sending event before device state 
> set to 'ATTACHED'.
> But other applications expect the device with 'ATTACHED' state before 
> send event.
> They are in conflict with each other. So we can't solve this issue by 
> adding an
> 'RTE_ETH_DEV_ALLOCATED' state.

Re: [PATCH] ethdev: fix push new event

[lihuisong (C)]

在 2022/6/3 15:42, Thomas Monjalon 写道:
> 02/06/2022 13:24, lihuisong (C):
>> 在 2022/5/30 19:10, Ferruh Yigit 写道:
>>> On 5/30/2022 9:28 AM, Thomas Monjalon wrote:
>>>> [CAUTION: External Email]
>>>>
>>>> 28/05/2022 10:53, lihuisong (C):
>>>>> 在 2022/5/23 22:36, Thomas Monjalon 写道:
>>>>>> 23/05/2022 11:51, David Marchand:
>>>>>>> On Sat, May 21, 2022 at 8:57 AM Min Hu
>>>>>>> (Connor)<humin29@huawei.com>  wrote:
>>>>>>>> From: Huisong Li<lihuisong@huawei.com>
>>>>>>>>
>>>>>>>> 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")
>>>>>>> A patch moving code is unlikely to be at fault.
>>>>>>>
>>>>>>>
>>>>>>> Looking at the patch which moved those notifications in this point of
>>>>>>> the code, the state update was pushed after the notification on
>>>>>>> purpose.
>>>>>>> See be8cd210379a ("ethdev: fix port probing notification")
>>>>>>>
>>>>>>>        ethdev: fix port probing notification
>>>>>>>
>>>>>>>        The new device was notified as soon as it was allocated.
>>>>>>>        It leads to use a device which is not yet initialized.
>>>>>>>
>>>>>>>        The notification must be published after the initialization
>>>>>>> is done
>>>>>>>        by the PMD, but before the state is changed, in order to let
>>>>>>>        notified entities taking ownership before general availability.
>>>>>>>
>>>>>>>
>>>>>>> Do we need an intermediate state during probing?
>>>>>> Possibly. Currently we have only 3 states:
>>>>>>       RTE_ETH_DEV_UNUSED
>>>>>>       RTE_ETH_DEV_ATTACHED
>>>>>>       RTE_ETH_DEV_REMOVED
>>>>>>
>>>>>> We may add RTE_ETH_DEV_ALLOCATED just before calling
>>>>>>       rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_NEW, NULL);
>>>>>> Then we would need to check against RTE_ETH_DEV_ALLOCATED
>>>>>> in some ethdev functions.
>>>>>>
>>>>> Hi, Thomas,
>>>>>
>>>>> Do you mean that we need to modify some funcions like following?
>>>>>
>>>>> int rte_eth_dev_is_valid_port(uint16_t port_id)
>>>>> {
>>>>>        if (port_id >= RTE_MAX_ETHPORTS ||
>>>>>            (rte_eth_devices[port_id].state != *RTE_ETH_DEV_ALLOCATED*))
>>>>>            return 0;
>>> Won't this mark ATTACHED devices as invalid?
>> Yes, You are right.
>>
>>> If the state flow will be as UNUSED -> ALLOCATED -> ATTACHED, above
>>> check should be against 'ATTACHED' I think.
> It should validate both ALLOCATED and ATTACHED.
Actually, we can only pick one, because it is an enumeration.
>
>
>> If these check is against 'ATTACHED', it goes back to the issue this
>> patch mentioned.
>>
>> The failsafe PMD applications expect sending event before device state
>> set to 'ATTACHED'.
>> But other applications expect the device with 'ATTACHED' state before
>> send event.
>> They are in conflict with each other. So we can't solve this issue by
>> adding an
>> 'RTE_ETH_DEV_ALLOCATED' state.
>
>
> .

Re: [PATCH] ethdev: fix push new event

[Thomas Monjalon]

07/06/2022 03:23, lihuisong (C):
> 
> 在 2022/6/3 15:42, Thomas Monjalon 写道:
> > 02/06/2022 13:24, lihuisong (C):
> >> 在 2022/5/30 19:10, Ferruh Yigit 写道:
> >>> On 5/30/2022 9:28 AM, Thomas Monjalon wrote:
> >>>> [CAUTION: External Email]
> >>>>
> >>>> 28/05/2022 10:53, lihuisong (C):
> >>>>> 在 2022/5/23 22:36, Thomas Monjalon 写道:
> >>>>>> 23/05/2022 11:51, David Marchand:
> >>>>>>> On Sat, May 21, 2022 at 8:57 AM Min Hu
> >>>>>>> (Connor)<humin29@huawei.com>  wrote:
> >>>>>>>> From: Huisong Li<lihuisong@huawei.com>
> >>>>>>>>
> >>>>>>>> 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")
> >>>>>>> A patch moving code is unlikely to be at fault.
> >>>>>>>
> >>>>>>>
> >>>>>>> Looking at the patch which moved those notifications in this point of
> >>>>>>> the code, the state update was pushed after the notification on
> >>>>>>> purpose.
> >>>>>>> See be8cd210379a ("ethdev: fix port probing notification")
> >>>>>>>
> >>>>>>>        ethdev: fix port probing notification
> >>>>>>>
> >>>>>>>        The new device was notified as soon as it was allocated.
> >>>>>>>        It leads to use a device which is not yet initialized.
> >>>>>>>
> >>>>>>>        The notification must be published after the initialization
> >>>>>>> is done
> >>>>>>>        by the PMD, but before the state is changed, in order to let
> >>>>>>>        notified entities taking ownership before general availability.
> >>>>>>>
> >>>>>>>
> >>>>>>> Do we need an intermediate state during probing?
> >>>>>> Possibly. Currently we have only 3 states:
> >>>>>>       RTE_ETH_DEV_UNUSED
> >>>>>>       RTE_ETH_DEV_ATTACHED
> >>>>>>       RTE_ETH_DEV_REMOVED
> >>>>>>
> >>>>>> We may add RTE_ETH_DEV_ALLOCATED just before calling
> >>>>>>       rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_NEW, NULL);
> >>>>>> Then we would need to check against RTE_ETH_DEV_ALLOCATED
> >>>>>> in some ethdev functions.
> >>>>>>
> >>>>> Hi, Thomas,
> >>>>>
> >>>>> Do you mean that we need to modify some funcions like following?
> >>>>>
> >>>>> int rte_eth_dev_is_valid_port(uint16_t port_id)
> >>>>> {
> >>>>>        if (port_id >= RTE_MAX_ETHPORTS ||
> >>>>>            (rte_eth_devices[port_id].state != *RTE_ETH_DEV_ALLOCATED*))
> >>>>>            return 0;
> >>> Won't this mark ATTACHED devices as invalid?
> >> Yes, You are right.
> >>
> >>> If the state flow will be as UNUSED -> ALLOCATED -> ATTACHED, above
> >>> check should be against 'ATTACHED' I think.
> > It should validate both ALLOCATED and ATTACHED.
> Actually, we can only pick one, because it is an enumeration.

You can check it is either one state or the other.

> >> If these check is against 'ATTACHED', it goes back to the issue this
> >> patch mentioned.
> >>
> >> The failsafe PMD applications expect sending event before device state
> >> set to 'ATTACHED'.
> >> But other applications expect the device with 'ATTACHED' state before
> >> send event.
> >> They are in conflict with each other. So we can't solve this issue by
> >> adding an
> >> 'RTE_ETH_DEV_ALLOCATED' state.
> >
> >
> > .
> 

Re: [PATCH] ethdev: fix push new event

[lihuisong (C)]

在 2022/6/7 14:44, Thomas Monjalon 写道:
> 07/06/2022 03:23, lihuisong (C):
>> 在 2022/6/3 15:42, Thomas Monjalon 写道:
>>> 02/06/2022 13:24, lihuisong (C):
>>>> 在 2022/5/30 19:10, Ferruh Yigit 写道:
>>>>> On 5/30/2022 9:28 AM, Thomas Monjalon wrote:
>>>>>> [CAUTION: External Email]
>>>>>>
>>>>>> 28/05/2022 10:53, lihuisong (C):
>>>>>>> 在 2022/5/23 22:36, Thomas Monjalon 写道:
>>>>>>>> 23/05/2022 11:51, David Marchand:
>>>>>>>>> On Sat, May 21, 2022 at 8:57 AM Min Hu
>>>>>>>>> (Connor)<humin29@huawei.com>  wrote:
>>>>>>>>>> From: Huisong Li<lihuisong@huawei.com>
>>>>>>>>>>
>>>>>>>>>> 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")
>>>>>>>>> A patch moving code is unlikely to be at fault.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Looking at the patch which moved those notifications in this point of
>>>>>>>>> the code, the state update was pushed after the notification on
>>>>>>>>> purpose.
>>>>>>>>> See be8cd210379a ("ethdev: fix port probing notification")
>>>>>>>>>
>>>>>>>>>         ethdev: fix port probing notification
>>>>>>>>>
>>>>>>>>>         The new device was notified as soon as it was allocated.
>>>>>>>>>         It leads to use a device which is not yet initialized.
>>>>>>>>>
>>>>>>>>>         The notification must be published after the initialization
>>>>>>>>> is done
>>>>>>>>>         by the PMD, but before the state is changed, in order to let
>>>>>>>>>         notified entities taking ownership before general availability.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Do we need an intermediate state during probing?
>>>>>>>> Possibly. Currently we have only 3 states:
>>>>>>>>        RTE_ETH_DEV_UNUSED
>>>>>>>>        RTE_ETH_DEV_ATTACHED
>>>>>>>>        RTE_ETH_DEV_REMOVED
>>>>>>>>
>>>>>>>> We may add RTE_ETH_DEV_ALLOCATED just before calling
>>>>>>>>        rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_NEW, NULL);
>>>>>>>> Then we would need to check against RTE_ETH_DEV_ALLOCATED
>>>>>>>> in some ethdev functions.
>>>>>>>>
>>>>>>> Hi, Thomas,
>>>>>>>
>>>>>>> Do you mean that we need to modify some funcions like following?
>>>>>>>
>>>>>>> int rte_eth_dev_is_valid_port(uint16_t port_id)
>>>>>>> {
>>>>>>>         if (port_id >= RTE_MAX_ETHPORTS ||
>>>>>>>             (rte_eth_devices[port_id].state != *RTE_ETH_DEV_ALLOCATED*))
>>>>>>>             return 0;
>>>>> Won't this mark ATTACHED devices as invalid?
>>>> Yes, You are right.
>>>>
>>>>> If the state flow will be as UNUSED -> ALLOCATED -> ATTACHED, above
>>>>> check should be against 'ATTACHED' I think.
>>> It should validate both ALLOCATED and ATTACHED.
>> Actually, we can only pick one, because it is an enumeration.
> You can check it is either one state or the other.
uint16_t
rte_eth_find_next(uint16_t port_id)
{
     while (port_id < RTE_MAX_ETHPORTS &&
            !(rte_eth_devices[port_id].state == RTE_ETH_DEV_ALLOCATED ||
              rte_eth_devices[port_id].state == RTE_ETH_DEV_ATTACHED))
         port_id++;

     if (port_id >= RTE_MAX_ETHPORTS)
         return RTE_MAX_ETHPORTS;

     return port_id;
}
like this, right? If so, adding 'ALLOCATED' and setting to 'ALLOCATED' 
is the same with
setting to 'ATTACHED' before sending new event.
They both meet the requirements mentioned in this patch that the device 
is a valid port
when applications receive a new event.

However, if device is taken by failsafe PMD as sub-device, the 
processing above
still doesn't satisfy the purpose of failsafe PMD when this sub-device 
push new event.

I don't know if I'm missing something. Can you explain it, Ferruh and 
Thomas?
>>>> If these check is against 'ATTACHED', it goes back to the issue this
>>>> patch mentioned.
>>>>
>>>> The failsafe PMD applications expect sending event before device state
>>>> set to 'ATTACHED'.
>>>> But other applications expect the device with 'ATTACHED' state before
>>>> send event.
>>>> They are in conflict with each other. So we can't solve this issue by
>>>> adding an
>>>> 'RTE_ETH_DEV_ALLOCATED' state.
>>>
>>> .
>
>
>
>
> .

Re: [PATCH] ethdev: fix push new event

[Thomas Monjalon]

11/06/2022 10:59, lihuisong (C):
> 在 2022/6/7 14:44, Thomas Monjalon 写道:
> > 07/06/2022 03:23, lihuisong (C):
> >> 在 2022/6/3 15:42, Thomas Monjalon 写道:
> >>> 02/06/2022 13:24, lihuisong (C):
> >>>> 在 2022/5/30 19:10, Ferruh Yigit 写道:
> >>>>> On 5/30/2022 9:28 AM, Thomas Monjalon wrote:
> >>>>>> [CAUTION: External Email]
> >>>>>>
> >>>>>> 28/05/2022 10:53, lihuisong (C):
> >>>>>>> 在 2022/5/23 22:36, Thomas Monjalon 写道:
> >>>>>>>> 23/05/2022 11:51, David Marchand:
> >>>>>>>>> On Sat, May 21, 2022 at 8:57 AM Min Hu
> >>>>>>>>> (Connor)<humin29@huawei.com>  wrote:
> >>>>>>>>>> From: Huisong Li<lihuisong@huawei.com>
> >>>>>>>>>>
> >>>>>>>>>> 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")
> >>>>>>>>> A patch moving code is unlikely to be at fault.
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> Looking at the patch which moved those notifications in this point of
> >>>>>>>>> the code, the state update was pushed after the notification on
> >>>>>>>>> purpose.
> >>>>>>>>> See be8cd210379a ("ethdev: fix port probing notification")
> >>>>>>>>>
> >>>>>>>>>         ethdev: fix port probing notification
> >>>>>>>>>
> >>>>>>>>>         The new device was notified as soon as it was allocated.
> >>>>>>>>>         It leads to use a device which is not yet initialized.
> >>>>>>>>>
> >>>>>>>>>         The notification must be published after the initialization
> >>>>>>>>> is done
> >>>>>>>>>         by the PMD, but before the state is changed, in order to let
> >>>>>>>>>         notified entities taking ownership before general availability.
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> Do we need an intermediate state during probing?
> >>>>>>>> Possibly. Currently we have only 3 states:
> >>>>>>>>        RTE_ETH_DEV_UNUSED
> >>>>>>>>        RTE_ETH_DEV_ATTACHED
> >>>>>>>>        RTE_ETH_DEV_REMOVED
> >>>>>>>>
> >>>>>>>> We may add RTE_ETH_DEV_ALLOCATED just before calling
> >>>>>>>>        rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_NEW, NULL);
> >>>>>>>> Then we would need to check against RTE_ETH_DEV_ALLOCATED
> >>>>>>>> in some ethdev functions.
> >>>>>>>>
> >>>>>>> Hi, Thomas,
> >>>>>>>
> >>>>>>> Do you mean that we need to modify some funcions like following?
> >>>>>>>
> >>>>>>> int rte_eth_dev_is_valid_port(uint16_t port_id)
> >>>>>>> {
> >>>>>>>         if (port_id >= RTE_MAX_ETHPORTS ||
> >>>>>>>             (rte_eth_devices[port_id].state != *RTE_ETH_DEV_ALLOCATED*))
> >>>>>>>             return 0;
> >>>>> Won't this mark ATTACHED devices as invalid?
> >>>> Yes, You are right.
> >>>>
> >>>>> If the state flow will be as UNUSED -> ALLOCATED -> ATTACHED, above
> >>>>> check should be against 'ATTACHED' I think.
> >>> It should validate both ALLOCATED and ATTACHED.
> >> Actually, we can only pick one, because it is an enumeration.
> > You can check it is either one state or the other.
> uint16_t
> rte_eth_find_next(uint16_t port_id)
> {
>      while (port_id < RTE_MAX_ETHPORTS &&
>             !(rte_eth_devices[port_id].state == RTE_ETH_DEV_ALLOCATED ||
>               rte_eth_devices[port_id].state == RTE_ETH_DEV_ATTACHED))
>          port_id++;
> 
>      if (port_id >= RTE_MAX_ETHPORTS)
>          return RTE_MAX_ETHPORTS;
> 
>      return port_id;
> }
> like this, right? If so, adding 'ALLOCATED' and setting to 'ALLOCATED' 
> is the same with
> setting to 'ATTACHED' before sending new event.
> They both meet the requirements mentioned in this patch that the device 
> is a valid port
> when applications receive a new event.

Yes, when receiving the event, the port would valid
in state ALLOCATED.
Then we can set as ATTACHED when definitely initialized,
after the notifications.

> However, if device is taken by failsafe PMD as sub-device, the 
> processing above
> still doesn't satisfy the purpose of failsafe PMD when this sub-device 
> push new event.

I don't understand why you think failsafe is not satisfied.
> 
> I don't know if I'm missing something. Can you explain it, Ferruh and 
> Thomas?

Please explain what you think is failing with failsafe.

Re: [PATCH] ethdev: fix push new event

[lihuisong (C)]

在 2022/9/27 18:29, Thomas Monjalon 写道:
> 11/06/2022 10:59, lihuisong (C):
>> 在 2022/6/7 14:44, Thomas Monjalon 写道:
>>> 07/06/2022 03:23, lihuisong (C):
>>>> 在 2022/6/3 15:42, Thomas Monjalon 写道:
>>>>> 02/06/2022 13:24, lihuisong (C):
>>>>>> 在 2022/5/30 19:10, Ferruh Yigit 写道:
>>>>>>> On 5/30/2022 9:28 AM, Thomas Monjalon wrote:
>>>>>>>> [CAUTION: External Email]
>>>>>>>>
>>>>>>>> 28/05/2022 10:53, lihuisong (C):
>>>>>>>>> 在 2022/5/23 22:36, Thomas Monjalon 写道:
>>>>>>>>>> 23/05/2022 11:51, David Marchand:
>>>>>>>>>>> On Sat, May 21, 2022 at 8:57 AM Min Hu
>>>>>>>>>>> (Connor)<humin29@huawei.com>  wrote:
>>>>>>>>>>>> From: Huisong Li<lihuisong@huawei.com>
>>>>>>>>>>>>
>>>>>>>>>>>> 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")
>>>>>>>>>>> A patch moving code is unlikely to be at fault.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Looking at the patch which moved those notifications in this point of
>>>>>>>>>>> the code, the state update was pushed after the notification on
>>>>>>>>>>> purpose.
>>>>>>>>>>> See be8cd210379a ("ethdev: fix port probing notification")
>>>>>>>>>>>
>>>>>>>>>>>          ethdev: fix port probing notification
>>>>>>>>>>>
>>>>>>>>>>>          The new device was notified as soon as it was allocated.
>>>>>>>>>>>          It leads to use a device which is not yet initialized.
>>>>>>>>>>>
>>>>>>>>>>>          The notification must be published after the initialization
>>>>>>>>>>> is done
>>>>>>>>>>>          by the PMD, but before the state is changed, in order to let
>>>>>>>>>>>          notified entities taking ownership before general availability.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Do we need an intermediate state during probing?
>>>>>>>>>> Possibly. Currently we have only 3 states:
>>>>>>>>>>         RTE_ETH_DEV_UNUSED
>>>>>>>>>>         RTE_ETH_DEV_ATTACHED
>>>>>>>>>>         RTE_ETH_DEV_REMOVED
>>>>>>>>>>
>>>>>>>>>> We may add RTE_ETH_DEV_ALLOCATED just before calling
>>>>>>>>>>         rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_NEW, NULL);
>>>>>>>>>> Then we would need to check against RTE_ETH_DEV_ALLOCATED
>>>>>>>>>> in some ethdev functions.
>>>>>>>>>>
>>>>>>>>> Hi, Thomas,
>>>>>>>>>
>>>>>>>>> Do you mean that we need to modify some funcions like following?
>>>>>>>>>
>>>>>>>>> int rte_eth_dev_is_valid_port(uint16_t port_id)
>>>>>>>>> {
>>>>>>>>>          if (port_id >= RTE_MAX_ETHPORTS ||
>>>>>>>>>              (rte_eth_devices[port_id].state != *RTE_ETH_DEV_ALLOCATED*))
>>>>>>>>>              return 0;
>>>>>>> Won't this mark ATTACHED devices as invalid?
>>>>>> Yes, You are right.
>>>>>>
>>>>>>> If the state flow will be as UNUSED -> ALLOCATED -> ATTACHED, above
>>>>>>> check should be against 'ATTACHED' I think.
>>>>> It should validate both ALLOCATED and ATTACHED.
>>>> Actually, we can only pick one, because it is an enumeration.
>>> You can check it is either one state or the other.
>> uint16_t
>> rte_eth_find_next(uint16_t port_id)
>> {
>>       while (port_id < RTE_MAX_ETHPORTS &&
>>              !(rte_eth_devices[port_id].state == RTE_ETH_DEV_ALLOCATED ||
>>                rte_eth_devices[port_id].state == RTE_ETH_DEV_ATTACHED))
>>           port_id++;
>>
>>       if (port_id >= RTE_MAX_ETHPORTS)
>>           return RTE_MAX_ETHPORTS;
>>
>>       return port_id;
>> }
>> like this, right? If so, adding 'ALLOCATED' and setting to 'ALLOCATED'
>> is the same with
>> setting to 'ATTACHED' before sending new event.
>> They both meet the requirements mentioned in this patch that the device
>> is a valid port
>> when applications receive a new event.
> Yes, when receiving the event, the port would valid
> in state ALLOCATED.
> Then we can set as ATTACHED when definitely initialized,
> after the notifications.
>
>> However, if device is taken by failsafe PMD as sub-device, the
>> processing above
>> still doesn't satisfy the purpose of failsafe PMD when this sub-device
>> push new event.
> I don't understand why you think failsafe is not satisfied.
>> I don't know if I'm missing something. Can you explain it, Ferruh and
>> Thomas?
> Please explain what you think is failing with failsafe.
Please look the reply in new patchset.
>
>
>
> .