Re: [PATCH RESEND v7 0/5] app/testpmd: support multiple process attach and detach port

[Ferruh Yigit <ferruh.yigit@amd.com>]

On 10/18/2024 3:48 AM, lihuisong (C) wrote:
> Hi Ferruh,
> 
> Thanks for your considering again. please see reply inline.
> 
> 在 2024/10/18 9:04, Ferruh Yigit 写道:
>> On 10/8/2024 3:32 AM, lihuisong (C) wrote:
>>> Hi Thomas and Ferruh,
>>>
>>> We've discussed it on and off a few times, and we've reached some
>>> consensus.
>>> They've been going through more than 2 years😅
>>> Can you have a look at this series again?
>>> If we really don't need it, I will drop it from my upstreaming list.
>>>
>> Hi Huisong,
>>
>> I was not really convinced with the patch series, but did not want to
>> block it outright, sorry that this caused patch series stay around.
>>
>> As checked again, still feels like adding unnecessary complexity, and I
>> am for rejecting this series.
>>
>> Overall target is to be able to support hotplug with primary/secondary
>> process, and uses event handlers for this but this requires adding a new
>> ethdev state to be able iterate over devices etc...
>> Perhaps better way to support this without relying on event handlers.
> Ignoring the modification of tesptmd is ok to me.
> But we need to restrict testpmd not to support attach and detach port in
> multiple process case.
> Otherwise. these issues this series solved will be encountered.
> 
> BTW, I want to say the patch [2/5] which introduced
> RTE_ETH_DEV_ALLOCATED should be thought again.
> Because it is an real issue in ethdev layer. This is also the fruit that
> Thomas, you and I discussed before.
> Please look at this patch again.
> 

RTE_ETH_DEV_ALLOCATED is added to run RTE_ETH_FOREACH_DEV in the event
handler, more specifically on the 'RTE_ETH_EVENT_NEW' event handler, right?
Without testpmd event handler update, what is the reason/usecase for
above ethdev change?

Thomas, Andrew, Stephen, please feel free to chime in.


> /Huisong
>>
>>
>>> /Huisong
>>>
>>>
>>> 在 2024/9/29 13:52, Huisong Li 写道:
>>>> This patchset fix some bugs and support attaching and detaching port
>>>> in primary and secondary.
>>>>
>>>> ---
>>>>    -v7: fix conflicts
>>>>    -v6: adjust rte_eth_dev_is_used position based on alphabetical order
>>>>         in version.map
>>>>    -v5: move 'ALLOCATED' state to the back of 'REMOVED' to avoid abi
>>>> break.
>>>>    -v4: fix a misspelling.
>>>>    -v3:
>>>>      #1 merge patch 1/6 and patch 2/6 into patch 1/5, and add
>>>> modification
>>>>         for other bus type.
>>>>      #2 add a RTE_ETH_DEV_ALLOCATED state in rte_eth_dev_state to
>>>> resolve
>>>>         the probelm in patch 2/5.
>>>>    -v2: resend due to CI unexplained failure.
>>>>
>>>> Huisong Li (5):
>>>>     drivers/bus: restore driver assignment at front of probing
>>>>     ethdev: fix skip valid port in probing callback
>>>>     app/testpmd: check the validity of the port
>>>>     app/testpmd: add attach and detach port for multiple process
>>>>     app/testpmd: stop forwarding in new or destroy event
>>>>
>>>>    app/test-pmd/testpmd.c                   | 47 ++++++++++++++
>>>> +---------
>>>>    app/test-pmd/testpmd.h                   |  1 -
>>>>    drivers/bus/auxiliary/auxiliary_common.c |  9 ++++-
>>>>    drivers/bus/dpaa/dpaa_bus.c              |  9 ++++-
>>>>    drivers/bus/fslmc/fslmc_bus.c            |  8 +++-
>>>>    drivers/bus/ifpga/ifpga_bus.c            | 12 ++++--
>>>>    drivers/bus/pci/pci_common.c             |  9 ++++-
>>>>    drivers/bus/vdev/vdev.c                  | 10 ++++-
>>>>    drivers/bus/vmbus/vmbus_common.c         |  9 ++++-
>>>>    drivers/net/bnxt/bnxt_ethdev.c           |  3 +-
>>>>    drivers/net/bonding/bonding_testpmd.c    |  1 -
>>>>    drivers/net/mlx5/mlx5.c                  |  2 +-
>>>>    lib/ethdev/ethdev_driver.c               | 13 +++++--
>>>>    lib/ethdev/ethdev_driver.h               | 12 ++++++
>>>>    lib/ethdev/ethdev_pci.h                  |  2 +-
>>>>    lib/ethdev/rte_class_eth.c               |  2 +-
>>>>    lib/ethdev/rte_ethdev.c                  |  4 +-
>>>>    lib/ethdev/rte_ethdev.h                  |  4 +-
>>>>    lib/ethdev/version.map                   |  1 +
>>>>    19 files changed, 114 insertions(+), 44 deletions(-)
>>>>
>> .