Re: [dpdk-dev] [PATCH] examples/bond: wait for slaves to become active

[Radu Nicolau <radu.nicolau@intel.com>]

Hi


On 11/28/2018 11:08 AM, Ferruh Yigit wrote:
> On 11/14/2018 12:19 PM, Radu Nicolau wrote:
>> Do not start the packet processing threads until all configured
>> slaves become active.
> Hi Radu,
>
> What happens if packet processing threads started before all slaves active? Exit
> app, error, crash?
>
> So can we say this patch is fixing packet forwarding? (fix in title?)
>
> And do we know what break it, why this was not required previously but required
> now? (Fixes line ?)
 From what I see, the problem was always there: bond_ethdev_rx_burst 
will cycle through slaves, but if called more times with no active 
slaves the active slave index will point out of bounds, resulting in a 
segfault.
While this may require a better fix, this patch is an improvement even 
if that fix comes - the configured slaves needs to be checked, and if 
none became active there is no point going further.

in bond_ethdev_rx_burst:

slave_count = internals->active_slave_count;
...
     if (++internals->active_slave == slave_count)
         internals->active_slave = 0;
slave_count is zero, the if() will never be true and active_slave will 
be continuously incremented. It was not written to work with no active 
slaves.

>
> Thanks,
> ferruh
>
>> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
>> ---
>>   examples/bond/main.c | 15 +++++++++++++++
>>   1 file changed, 15 insertions(+)
>>
>> diff --git a/examples/bond/main.c b/examples/bond/main.c
>> index b282e68..6623cae 100644
>> --- a/examples/bond/main.c
>> +++ b/examples/bond/main.c
>> @@ -220,6 +220,7 @@ bond_port_init(struct rte_mempool *mbuf_pool)
>>   	struct rte_eth_rxconf rxq_conf;
>>   	struct rte_eth_txconf txq_conf;
>>   	struct rte_eth_conf local_port_conf = port_conf;
>> +	uint16_t wait_counter = 20;
>>   
>>   	retval = rte_eth_bond_create("net_bonding0", BONDING_MODE_ALB,
>>   			0 /*SOCKET_ID_ANY*/);
>> @@ -274,6 +275,20 @@ bond_port_init(struct rte_mempool *mbuf_pool)
>>   	if (retval < 0)
>>   		rte_exit(retval, "Start port %d failed (res=%d)", BOND_PORT, retval);
>>   
>> +	printf("Waiting for slaves to become active...");
>> +	while (wait_counter) {
>> +		uint16_t act_slaves[16] = {0};
>> +		if (rte_eth_bond_active_slaves_get(BOND_PORT, act_slaves, 16) ==
>> +				slaves_count) {
>> +			printf("\n");
>> +			break;
>> +		}
>> +		sleep(1);
>> +		printf("...");
>> +		if (--wait_counter == 0)
>> +			rte_exit(-1, "\nFailed to activate slaves\n");
>> +	}
>> +
>>   	rte_eth_promiscuous_enable(BOND_PORT);
>>   
>>   	struct ether_addr addr;
>>