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?
