Hi Matan.
> Hi Alex
> Please see comments below.
>> +
>> + ret = rte_eth_dev_mac_addr_add(slave_port_id, mac_addr, 0);
>> + if (ret < 0) {
>> + /* rollback */
>> + for (i--; i > 0; i--)
>> +
> In case of failure in the first mac address(i=1) you are going to
> remove the default mac address(i=0) from the slave.
In that case i will be incremented first and will be equal to 0, then for
condition will fail
and the loop body will not be executed.
>> rte_eth_dev_mac_addr_remove(slave_port_id,
>> + &bonded_eth_dev->data-
>> >mac_addrs[i]);
>> + return ret;
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +/*
>> + * Remove additional MAC addresses from the slave */ int
>> +slave_remove_mac_addresses(struct rte_eth_dev *bonded_eth_dev,
>> + uint16_t slave_port_id)
>> +{
>> + int i, ret = 0;
>> + struct ether_addr *mac_addr;
>> +
>> + for (i = 1; i < BOND_MAX_MAC_ADDRS; i++) {
>> + mac_addr = &bonded_eth_dev->data->mac_addrs[i];
>> + if (is_same_ether_addr(mac_addr, &null_mac_addr))
>> + break;
>> +
>> + ret = rte_eth_dev_mac_addr_remove(slave_port_id,
>> mac_addr);
>> + }
> I suggest to return the first error, also in case of all success
> with last failure, the code here wrongly returns success.
Yeah, you are right. I'll fix it.
>> +
>> + for (i = 0; i < internals->slave_count; i++) {
>> + ret = rte_eth_dev_mac_addr_add(internals->slaves[i].port_id,
>> + mac_addr, vmdq);
>> + if (ret < 0) {
>> + /* rollback */
>> + for (i--; i >= 0; i--)
> In case of failure in the first slave(i=0) you are going probably to get
> memory error (i=-1).
The same logic apply here. When i ==-1 the condition will fail and the loop
body will not be executed;
--
Alex
