[PATCH v3 3/5] app/testpmd: fix port status of slave device
Ferruh Yigit
ferruh.yigit at xilinx.com
Wed May 11 12:05:58 CEST 2022
On 5/11/2022 3:16 AM, Min Hu (Connor) wrote:
<...>
>>>>>> @@ -609,8 +658,25 @@ eth_dev_start_mp(uint16_t port_id)
>>>>>> static int
>>>>>> eth_dev_stop_mp(uint16_t port_id)
>>>>>> {
>>>>>> - if (is_proc_primary())
>>>>>> - return rte_eth_dev_stop(port_id);
>>>>>> + int ret;
>>>>>> +
>>>>>> + if (is_proc_primary()) {
>>>>>> + ret = rte_eth_dev_stop(port_id);
>>>>>> + if (ret != 0)
>>>>>> + return ret;
>>>>>> +
>>>>>> +#ifdef RTE_NET_BOND
>>>>>
>>>>> Here and in other places - probably no need to pollute the code
>>>>> with all these 'ifdef RTE_NET_BOND'.
>>>>> I suppose this logic (for checking is bonding API present or not)
>>>>> can be hidden inside change_bonding_slave_port_status() itself.
>>>>>
>>>> I think it does not pollute the code. anyone can tell according to
>>>> the flag 'ifdef RTE_NET_BOND'.
>>>> if hiddle inside 'change_bonding_slave_port_status', it will be weird.
>>>> For example, if the port is not bonding port, It will also invoke
>>>> 'change_bonding_slave_port_status'. That is unreasonable.
>>>>
>>>
>>> Hi Konstantin,
>>>
>>> I also was not happy to have bonding (or any PMD) ifdef in the
>>> generic (start()/stop()) functions, but the ifdef blocks updates
>>> testpmd (application) level status. So that can't be handled in the
>>> PMD and need to be in the application level.
>>> Which is enforcing to have same PMD specific code in the testpmd level,
>>> if you have any suggestion to prevent this, I am for it.
>>
>>
>> I am not aking to move it to PMD.
>> What I am saying that this ifdef logic could be grouped in one place
>> (inside change_bonding_slave_port_status()) instead of spreading it
>> around multiple places.
>>
>
> Hi, Konstantin,
> fixed in v4, thanks.
Hi Conor,
What do you think to apply same on patch 4/5?
More information about the stable
mailing list