[dpdk-dev] [PATCH 0/2] fixes for bonding

[dpdk-dev] [PATCH 0/2] fixes for bonding

[Wei Hu (Xavier)]

This series include two fixes patches for bonding PMD driver.

Wei Hu (Xavier) (2):
  net/bonding: fix MAC address when switching active port
  net/bonding: fix MAC address when one port resets

 drivers/net/bonding/rte_eth_bond_api.c |  1 +
 drivers/net/bonding/rte_eth_bond_pmd.c | 11 ++++++++---
 2 files changed, 9 insertions(+), 3 deletions(-)

-- 
2.23.0

[dpdk-dev] [PATCH 1/2] net/bonding: fix MAC address when switching active port

[Wei Hu (Xavier)]

From: "Wei Hu (Xavier)" <xavier.huwei@huawei.com>

Currently, based on a active-backup bond device, when the link status of
the primary port changes from up to down, one slave port changes to the
primary port, but the new primary port's MAC address cannot change to the
bond device's MAC address. And we can't continue receive packets whose
destination MAC addresses are the same as the bond devices's MAC address.

The current bonding PMD driver call mac_address_slaves_update function to
modify the MAC address of all slaves devices: the primary port using bond
device's MAC address, and other slaves devices using the respective MAC
address. We found that one error using primary_port instead of
current_primary_port in mac_address_slaves_update function.

On the other hand, The current bonding PMD driver sets slave devices's MAC
address according to the variable named current_primary_port. The variable
named current_primary_port changes in the following scenario:
1. Add the slave devices to bond, the first slave port will be regardes as
   the current_primary_port. If changing the order of adding the slave
   devices, the value of the variable named current_primary_port will be
   different.
2. The upper application specifies primary_port via calling the
   rte_eth_bond_primary_set API function.
3. Delete the primary slave device.
4. The link status of the primary port changes from up to down.

We have tested the above 4 cases and found that there are problems that
the new primary port's MAC address didn't change to the bond device's MAC
address when running case 3 and 4. When current_primary_port changes, the
new primary port's MAC address should change at the same time. We also need
to call mac_address_slaves_update function to update MAC addresses in case
3 and 4.

For more details please refer to:
https://bugs.dpdk.org/show_bug.cgi?id=256

Bugzilla ID: 256
Fixes: 2efb58cbab6e ("bond: new link bonding library")
Cc: stable@dpdk.org

Reported-by: Chunsong Feng <fengchunsong@huawei.com>
Signed-off-by: Chunsong Feng <fengchunsong@huawei.com>
Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
---
 drivers/net/bonding/rte_eth_bond_api.c | 1 +
 drivers/net/bonding/rte_eth_bond_pmd.c | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/bonding/rte_eth_bond_api.c b/drivers/net/bonding/rte_eth_bond_api.c
index f38eb3b47..07780a2ac 100644
--- a/drivers/net/bonding/rte_eth_bond_api.c
+++ b/drivers/net/bonding/rte_eth_bond_api.c
@@ -698,6 +698,7 @@ __eth_bond_slave_remove_lock_free(uint16_t bonded_port_id,
 			internals->current_primary_port = internals->slaves[0].port_id;
 		else
 			internals->primary_port = 0;
+		mac_address_slaves_update(bonded_eth_dev);
 	}
 
 	if (internals->active_slave_count < 1) {
diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index 707a0f3cd..ddae3518c 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -1533,7 +1533,7 @@ mac_address_slaves_update(struct rte_eth_dev *bonded_eth_dev)
 			if (internals->slaves[i].port_id ==
 					internals->current_primary_port) {
 				if (rte_eth_dev_default_mac_addr_set(
-						internals->primary_port,
+						internals->current_primary_port,
 						bonded_eth_dev->data->mac_addrs)) {
 					RTE_BOND_LOG(ERR, "Failed to update port Id %d MAC address",
 							internals->current_primary_port);
@@ -2873,6 +2873,7 @@ bond_ethdev_lsc_event_callback(uint16_t port_id, enum rte_eth_event_type type,
 						internals->active_slaves[0]);
 			else
 				internals->current_primary_port = internals->primary_port;
+			mac_address_slaves_update(bonded_eth_dev);
 		}
 	}
 
-- 
2.23.0

Re: [dpdk-dev] [PATCH 1/2] net/bonding: fix MAC address when switching active port

[Ferruh Yigit]

On 2/25/2020 9:29 AM, Wei Hu (Xavier) wrote:
> From: "Wei Hu (Xavier)" <xavier.huwei@huawei.com>
> 
> Currently, based on a active-backup bond device, when the link status of
> the primary port changes from up to down, one slave port changes to the
> primary port, but the new primary port's MAC address cannot change to the
> bond device's MAC address. And we can't continue receive packets whose
> destination MAC addresses are the same as the bond devices's MAC address.
> 
> The current bonding PMD driver call mac_address_slaves_update function to
> modify the MAC address of all slaves devices: the primary port using bond
> device's MAC address, and other slaves devices using the respective MAC
> address. We found that one error using primary_port instead of
> current_primary_port in mac_address_slaves_update function.
> 
> On the other hand, The current bonding PMD driver sets slave devices's MAC
> address according to the variable named current_primary_port. The variable
> named current_primary_port changes in the following scenario:
> 1. Add the slave devices to bond, the first slave port will be regardes as
>    the current_primary_port. If changing the order of adding the slave
>    devices, the value of the variable named current_primary_port will be
>    different.
> 2. The upper application specifies primary_port via calling the
>    rte_eth_bond_primary_set API function.
> 3. Delete the primary slave device.
> 4. The link status of the primary port changes from up to down.
> 
> We have tested the above 4 cases and found that there are problems that
> the new primary port's MAC address didn't change to the bond device's MAC
> address when running case 3 and 4. When current_primary_port changes, the
> new primary port's MAC address should change at the same time. We also need
> to call mac_address_slaves_update function to update MAC addresses in case
> 3 and 4.
> 
> For more details please refer to:
> https://bugs.dpdk.org/show_bug.cgi?id=256
> 
> Bugzilla ID: 256
> Fixes: 2efb58cbab6e ("bond: new link bonding library")
> Cc: stable@dpdk.org
> 
> Reported-by: Chunsong Feng <fengchunsong@huawei.com>
> Signed-off-by: Chunsong Feng <fengchunsong@huawei.com>
> Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>

Please cc the maintainer on the patches. cc'ed Chas for this one.

I am using './devtools/get-maintainer.sh' script, once you set it up, it helps a
lot.

Re: [dpdk-dev] [PATCH 1/2] net/bonding: fix MAC address when switching active port

[Wei Hu (Xavier)]

Hi,Ferruh Yigit

On 2020/2/28 1:03, Ferruh Yigit wrote:
> On 2/25/2020 9:29 AM, Wei Hu (Xavier) wrote:
>> From: "Wei Hu (Xavier)" <xavier.huwei@huawei.com>
>>
>> Currently, based on a active-backup bond device, when the link status of
>> the primary port changes from up to down, one slave port changes to the
>> primary port, but the new primary port's MAC address cannot change to the
>> bond device's MAC address. And we can't continue receive packets whose
>> destination MAC addresses are the same as the bond devices's MAC address.
>>
>> The current bonding PMD driver call mac_address_slaves_update function to
>> modify the MAC address of all slaves devices: the primary port using bond
>> device's MAC address, and other slaves devices using the respective MAC
>> address. We found that one error using primary_port instead of
>> current_primary_port in mac_address_slaves_update function.
>>
>> On the other hand, The current bonding PMD driver sets slave devices's MAC
>> address according to the variable named current_primary_port. The variable
>> named current_primary_port changes in the following scenario:
>> 1. Add the slave devices to bond, the first slave port will be regardes as
>>     the current_primary_port. If changing the order of adding the slave
>>     devices, the value of the variable named current_primary_port will be
>>     different.
>> 2. The upper application specifies primary_port via calling the
>>     rte_eth_bond_primary_set API function.
>> 3. Delete the primary slave device.
>> 4. The link status of the primary port changes from up to down.
>>
>> We have tested the above 4 cases and found that there are problems that
>> the new primary port's MAC address didn't change to the bond device's MAC
>> address when running case 3 and 4. When current_primary_port changes, the
>> new primary port's MAC address should change at the same time. We also need
>> to call mac_address_slaves_update function to update MAC addresses in case
>> 3 and 4.
>>
>> For more details please refer to:
>> https://bugs.dpdk.org/show_bug.cgi?id=256
>>
>> Bugzilla ID: 256
>> Fixes: 2efb58cbab6e ("bond: new link bonding library")
>> Cc: stable@dpdk.org
>>
>> Reported-by: Chunsong Feng <fengchunsong@huawei.com>
>> Signed-off-by: Chunsong Feng <fengchunsong@huawei.com>
>> Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
> 
> Please cc the maintainer on the patches. cc'ed Chas for this one.
> 
> I am using './devtools/get-maintainer.sh' script, once you set it up, it helps a
> lot.
> 
Thanks for your suggestion.
   Regards
Xavier

Re: [dpdk-dev] [PATCH 1/2] net/bonding: fix MAC address when switching active port

[Wei Hu (Xavier)]

Hi, Chas Williams
    Could you please give some suggestion on these patches?
    Thanks.

    Regards
Xavier

On 2020/2/28 9:31, Wei Hu (Xavier) wrote:
> Hi,Ferruh Yigit
> 
> On 2020/2/28 1:03, Ferruh Yigit wrote:
>> On 2/25/2020 9:29 AM, Wei Hu (Xavier) wrote:
>>> From: "Wei Hu (Xavier)" <xavier.huwei@huawei.com>
>>>
>>> Currently, based on a active-backup bond device, when the link status of
>>> the primary port changes from up to down, one slave port changes to the
>>> primary port, but the new primary port's MAC address cannot change to 
>>> the
>>> bond device's MAC address. And we can't continue receive packets whose
>>> destination MAC addresses are the same as the bond devices's MAC 
>>> address.
>>>
>>> The current bonding PMD driver call mac_address_slaves_update 
>>> function to
>>> modify the MAC address of all slaves devices: the primary port using 
>>> bond
>>> device's MAC address, and other slaves devices using the respective MAC
>>> address. We found that one error using primary_port instead of
>>> current_primary_port in mac_address_slaves_update function.
>>>
>>> On the other hand, The current bonding PMD driver sets slave 
>>> devices's MAC
>>> address according to the variable named current_primary_port. The 
>>> variable
>>> named current_primary_port changes in the following scenario:
>>> 1. Add the slave devices to bond, the first slave port will be 
>>> regardes as
>>>     the current_primary_port. If changing the order of adding the slave
>>>     devices, the value of the variable named current_primary_port 
>>> will be
>>>     different.
>>> 2. The upper application specifies primary_port via calling the
>>>     rte_eth_bond_primary_set API function.
>>> 3. Delete the primary slave device.
>>> 4. The link status of the primary port changes from up to down.
>>>
>>> We have tested the above 4 cases and found that there are problems that
>>> the new primary port's MAC address didn't change to the bond device's 
>>> MAC
>>> address when running case 3 and 4. When current_primary_port changes, 
>>> the
>>> new primary port's MAC address should change at the same time. We 
>>> also need
>>> to call mac_address_slaves_update function to update MAC addresses in 
>>> case
>>> 3 and 4.
>>>
>>> For more details please refer to:
>>> https://bugs.dpdk.org/show_bug.cgi?id=256
>>>
>>> Bugzilla ID: 256
>>> Fixes: 2efb58cbab6e ("bond: new link bonding library")
>>> Cc: stable@dpdk.org
>>>
>>> Reported-by: Chunsong Feng <fengchunsong@huawei.com>
>>> Signed-off-by: Chunsong Feng <fengchunsong@huawei.com>
>>> Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
>>
>> Please cc the maintainer on the patches. cc'ed Chas for this one.
>>
>> I am using './devtools/get-maintainer.sh' script, once you set it up, 
>> it helps a
>> lot.
>>
> Thanks for your suggestion.
>    Regards
> Xavier

[dpdk-dev] [PATCH 2/2] net/bonding: fix MAC address when one port resets

[Wei Hu (Xavier)]

From: "Wei Hu (Xavier)" <xavier.huwei@huawei.com>

Currently, based on a active-backup bond device, in the following 2 cases:
1) The primary port resets. The link status of the primary port changes
   from up to down.
2) When switching the active port, one slave port resets at the same time.
one slave port changes to the primary port, but the new primary port's MAC
address probably cannot change to the bond device's MAC address. And we
can't continue receive packets whose destination MAC addresses are the same
as the bond devices's MAC address.

The current bonding PMD driver call mac_address_slaves_update function to
modify the MAC address of all slaves devices. In mac_address_slaves_update
function, the rte_eth_dev_default_mac_addr_set API function is called to
set the MAC address of the slave devices in turn in the for loop statement.

When one port reset, calling rte_eth_dev_default_mac_addr_set API fails
because the firmware will not respond to the commands from the driver,
and exit the loop, so other slave devices cannot continue to update the
MAC address.

This patch fixes the issue by avoid exiting the loop when calling
rte_eth_dev_default_mac_addr_set fails.

Fixes: 2efb58cbab6e ("bond: new link bonding library")
Cc: stable@dpdk.org

Signed-off-by: Hongbo Zheng <zhenghongbo3@huawei.com>
Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
Signed-off-by: Chunsong Feng <fengchunsong@huawei.com>
Signed-off-by: Xuan Li <lixuan47@hisilicon.com>
---
 drivers/net/bonding/rte_eth_bond_pmd.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index ddae3518c..ba3f342e7 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -1502,6 +1502,7 @@ int
 mac_address_slaves_update(struct rte_eth_dev *bonded_eth_dev)
 {
 	struct bond_dev_private *internals = bonded_eth_dev->data->dev_private;
+	bool setted;
 	int i;
 
 	/* Update slave devices MAC addresses */
@@ -1529,6 +1530,7 @@ mac_address_slaves_update(struct rte_eth_dev *bonded_eth_dev)
 	case BONDING_MODE_TLB:
 	case BONDING_MODE_ALB:
 	default:
+		setted = true;
 		for (i = 0; i < internals->slave_count; i++) {
 			if (internals->slaves[i].port_id ==
 					internals->current_primary_port) {
@@ -1537,7 +1539,7 @@ mac_address_slaves_update(struct rte_eth_dev *bonded_eth_dev)
 						bonded_eth_dev->data->mac_addrs)) {
 					RTE_BOND_LOG(ERR, "Failed to update port Id %d MAC address",
 							internals->current_primary_port);
-					return -1;
+					setted = false;
 				}
 			} else {
 				if (rte_eth_dev_default_mac_addr_set(
@@ -1545,10 +1547,12 @@ mac_address_slaves_update(struct rte_eth_dev *bonded_eth_dev)
 						&internals->slaves[i].persisted_mac_addr)) {
 					RTE_BOND_LOG(ERR, "Failed to update port Id %d MAC address",
 							internals->slaves[i].port_id);
-					return -1;
+					setted = false;
 				}
 			}
 		}
+		if (!setted)
+			return -1;
 	}
 
 	return 0;
-- 
2.23.0

Re: [dpdk-dev] [PATCH 2/2] net/bonding: fix MAC address when one port resets

[Chas Williams]

The behavior is probably going to be inconsistent. Only one of the calls
to mac_address_slaves_update() is checked for failure. The two calls
are in bond_ethdev_lsc_event_callback()

                 if (internals->active_slave_count < 1) {
                         mac_address_slaves_update(bonded_eth_dev);

and in bond_ethdev_start():

         /* Update all slave devices MACs*/
         if (mac_address_slaves_update(eth_dev) != 0)
                 goto out_err;

So if the bond device is running, you safely fail to the backup device.
But if you stop and start the bond device, it will then fail to start?

What devices are you bonding together that aren't able to support
changing their MAC addresses reliably? ixgbe VF devices by any chance?
One solution is to always assume success or ignore the error from devices
that you know are not an issue. I agree that we shouldn't early exit
from mac_address_slaves_update(). I am not sure how to handle the error.

We probably shouldn't allow you to bond devices that don't support setting
MAC address. I don't think this check exists currently. That's possibly
one reason for this check.

On 2/25/20 4:29 AM, Wei Hu (Xavier) wrote:
 > From: "Wei Hu (Xavier)" <xavier.huwei@huawei.com>
 >
 > Currently, based on a active-backup bond device, in the following 2 
cases:
 > 1) The primary port resets. The link status of the primary port changes
 >     from up to down.
 > 2) When switching the active port, one slave port resets at the same 
time.
 > one slave port changes to the primary port, but the new primary 
port's MAC
 > address probably cannot change to the bond device's MAC address. And we
 > can't continue receive packets whose destination MAC addresses are 
the same
 > as the bond devices's MAC address.
 >
 > The current bonding PMD driver call mac_address_slaves_update function to
 > modify the MAC address of all slaves devices. In 
mac_address_slaves_update
 > function, the rte_eth_dev_default_mac_addr_set API function is called to
 > set the MAC address of the slave devices in turn in the for loop 
statement.
 >
 > When one port reset, calling rte_eth_dev_default_mac_addr_set API fails
 > because the firmware will not respond to the commands from the driver,
 > and exit the loop, so other slave devices cannot continue to update the
 > MAC address.
 >
 > This patch fixes the issue by avoid exiting the loop when calling
 > rte_eth_dev_default_mac_addr_set fails.
 >
 > Fixes: 2efb58cbab6e ("bond: new link bonding library")
 > Cc: stable@dpdk.org
 >
 > Signed-off-by: Hongbo Zheng <zhenghongbo3@huawei.com>
 > Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
 > Signed-off-by: Chunsong Feng <fengchunsong@huawei.com>
 > Signed-off-by: Xuan Li <lixuan47@hisilicon.com>
 > ---
 >   drivers/net/bonding/rte_eth_bond_pmd.c | 8 ++++++--
 >   1 file changed, 6 insertions(+), 2 deletions(-)
 >
 > diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c 
b/drivers/net/bonding/rte_eth_bond_pmd.c
 > index ddae3518c..ba3f342e7 100644
 > --- a/drivers/net/bonding/rte_eth_bond_pmd.c
 > +++ b/drivers/net/bonding/rte_eth_bond_pmd.c
 > @@ -1502,6 +1502,7 @@ int
 >   mac_address_slaves_update(struct rte_eth_dev *bonded_eth_dev)
 >   {
 >   	struct bond_dev_private *internals = 
bonded_eth_dev->data->dev_private;
 > +	bool setted;
 >   	int i;
 >
 >   	/* Update slave devices MAC addresses */
 > @@ -1529,6 +1530,7 @@ mac_address_slaves_update(struct rte_eth_dev 
*bonded_eth_dev)
 >   	case BONDING_MODE_TLB:
 >   	case BONDING_MODE_ALB:
 >   	default:
 > +		setted = true;
 >   		for (i = 0; i < internals->slave_count; i++) {
 >   			if (internals->slaves[i].port_id ==
 >   					internals->current_primary_port) {
 > @@ -1537,7 +1539,7 @@ mac_address_slaves_update(struct rte_eth_dev 
*bonded_eth_dev)
 >   						bonded_eth_dev->data->mac_addrs)) {
 >   					RTE_BOND_LOG(ERR, "Failed to update port Id %d MAC address",
 >   							internals->current_primary_port);
 > -					return -1;
 > +					setted = false;
 >   				}
 >   			} else {
 >   				if (rte_eth_dev_default_mac_addr_set(
 > @@ -1545,10 +1547,12 @@ mac_address_slaves_update(struct rte_eth_dev 
*bonded_eth_dev)
 >   						&internals->slaves[i].persisted_mac_addr)) {
 >   					RTE_BOND_LOG(ERR, "Failed to update port Id %d MAC address",
 >   							internals->slaves[i].port_id);
 > -					return -1;
 > +					setted = false;
 >   				}
 >   			}
 >   		}
 > +		if (!setted)
 > +			return -1;
 >   	}
 >
 >   	return 0;
 >

Re: [dpdk-dev] [PATCH 2/2] net/bonding: fix MAC address when one port resets

[Wei Hu (Xavier)]

Hi, Chas



On 2020/4/4 22:07, Chas Williams wrote:
> The behavior is probably going to be inconsistent. Only one of the calls
> to mac_address_slaves_update() is checked for failure. The two calls
> are in bond_ethdev_lsc_event_callback()
> 
>                  if (internals->active_slave_count < 1) {
>                          mac_address_slaves_update(bonded_eth_dev);
> 
> and in bond_ethdev_start():
> 
>          /* Update all slave devices MACs*/
>          if (mac_address_slaves_update(eth_dev) != 0)
>                  goto out_err;
> 
> So if the bond device is running, you safely fail to the backup device.
> But if you stop and start the bond device, it will then fail to start?
We will make modification as below，and will send patch V2.
After modification，It will then fail to start when failed to setting 
bond device's MAC to the new primary port.

int
mac_address_slaves_update(struct rte_eth_dev *bonded_eth_dev)
{
     <snip>
     bool setted;

     <snip>
     switch (internals->mode) {
     <snip>
     default:
         setted = true;
         for (i = 0; i < internals->slave_count; i++) {
             if (internals->slaves[i].port_id ==
                 internals->current_primary_port) {
                 if (rte_eth_dev_default_mac_addr_set(
                         internals->primary_port,
                         bonded_eth_dev->data->mac_addrs)) {
                             RTE_BOND_LOG(ERR, "Failed to xxx",);
                             setted = false;
                 }
             } else {
                 if (rte_eth_dev_default_mac_addr_set(
                         internals->slaves[i].port_id,
                         &internals->slaves[i].persisted_mac_addr)) {
                             RTE_BOND_LOG(ERR, "Failed to xxx",);
                 }
             }
        }
        if (!setted)
            return -1;
     }
}
I think we can ignore the failure when updating salves's MAC address in 
the mac_address_slaves_update function, because it doesn't affect
the bond's functional characteristics.

> 
> What devices are you bonding together that aren't able to support
> changing their MAC addresses reliably? ixgbe VF devices by any chance?
> One solution is to always assume success or ignore the error from devices
> that you know are not an issue. I agree that we shouldn't early exit
> from mac_address_slaves_update(). I am not sure how to handle the error.
> 
We are running based on huawei's hns3 network engine of kunpeng 920 SoC.
When the driver detects that the hardware status is abnormal, the reset 
process is executed, after hardware reset we will restore the hardware 
configuration including MAC address, and then network engine will be 
able to work normally again. During the hardware reset, the command 
sending to firmware will timeout.

> We probably shouldn't allow you to bond devices that don't support setting
> MAC address. I don't think this check exists currently. That's possibly
> one reason for this check.
> 
> On 2/25/20 4:29 AM, Wei Hu (Xavier) wrote:
>  > From: "Wei Hu (Xavier)" <xavier.huwei@huawei.com>
>  >
>  > Currently, based on a active-backup bond device, in the following 2 
> cases:
>  > 1) The primary port resets. The link status of the primary port changes
>  >     from up to down.
>  > 2) When switching the active port, one slave port resets at the same 
> time.
>  > one slave port changes to the primary port, but the new primary 
> port's MAC
>  > address probably cannot change to the bond device's MAC address. And we
>  > can't continue receive packets whose destination MAC addresses are 
> the same
>  > as the bond devices's MAC address.
>  >
>  > The current bonding PMD driver call mac_address_slaves_update 
> function to
>  > modify the MAC address of all slaves devices. In 
> mac_address_slaves_update
>  > function, the rte_eth_dev_default_mac_addr_set API function is called to
>  > set the MAC address of the slave devices in turn in the for loop 
> statement.
>  >
>  > When one port reset, calling rte_eth_dev_default_mac_addr_set API fails
>  > because the firmware will not respond to the commands from the driver,
>  > and exit the loop, so other slave devices cannot continue to update the
>  > MAC address.
>  >
>  > This patch fixes the issue by avoid exiting the loop when calling
>  > rte_eth_dev_default_mac_addr_set fails.
>  >
>  > Fixes: 2efb58cbab6e ("bond: new link bonding library")
>  > Cc: stable@dpdk.org
>  >
>  > Signed-off-by: Hongbo Zheng <zhenghongbo3@huawei.com>
>  > Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
>  > Signed-off-by: Chunsong Feng <fengchunsong@huawei.com>
>  > Signed-off-by: Xuan Li <lixuan47@hisilicon.com>
>  > ---
>  >   drivers/net/bonding/rte_eth_bond_pmd.c | 8 ++++++--
>  >   1 file changed, 6 insertions(+), 2 deletions(-)
>  >
>  > diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c 
> b/drivers/net/bonding/rte_eth_bond_pmd.c
>  > index ddae3518c..ba3f342e7 100644
>  > --- a/drivers/net/bonding/rte_eth_bond_pmd.c
>  > +++ b/drivers/net/bonding/rte_eth_bond_pmd.c
>  > @@ -1502,6 +1502,7 @@ int
>  >   mac_address_slaves_update(struct rte_eth_dev *bonded_eth_dev)
>  >   {
>  >       struct bond_dev_private *internals = 
> bonded_eth_dev->data->dev_private;
>  > +    bool setted;
>  >       int i;
>  >
>  >       /* Update slave devices MAC addresses */
>  > @@ -1529,6 +1530,7 @@ mac_address_slaves_update(struct rte_eth_dev 
> *bonded_eth_dev)
>  >       case BONDING_MODE_TLB:
>  >       case BONDING_MODE_ALB:
>  >       default:
>  > +        setted = true;
>  >           for (i = 0; i < internals->slave_count; i++) {
>  >               if (internals->slaves[i].port_id ==
>  >                       internals->current_primary_port) {
>  > @@ -1537,7 +1539,7 @@ mac_address_slaves_update(struct rte_eth_dev 
> *bonded_eth_dev)
>  >                           bonded_eth_dev->data->mac_addrs)) {
>  >                       RTE_BOND_LOG(ERR, "Failed to update port Id %d 
> MAC address",
>  >                               internals->current_primary_port);
>  > -                    return -1;
>  > +                    setted = false;
>  >                   }
>  >               } else {
>  >                   if (rte_eth_dev_default_mac_addr_set(
>  > @@ -1545,10 +1547,12 @@ mac_address_slaves_update(struct rte_eth_dev 
> *bonded_eth_dev)
>  >                           &internals->slaves[i].persisted_mac_addr)) {
>  >                       RTE_BOND_LOG(ERR, "Failed to update port Id %d 
> MAC address",
>  >                               internals->slaves[i].port_id);
>  > -                    return -1;
>  > +                    setted = false;
>  >                   }
>  >               }
>  >           }
>  > +        if (!setted)
>  > +            return -1;
>  >       }
>  >
>  >       return 0;
>  >

Re: [dpdk-dev] [PATCH 0/2] fixes for bonding

[Wei Hu (Xavier)]

Are there any comments on this series?
Thanks.

Xavier

On 2020/2/25 17:29, Wei Hu (Xavier) wrote:
> This series include two fixes patches for bonding PMD driver.
> 
> Wei Hu (Xavier) (2):
>    net/bonding: fix MAC address when switching active port
>    net/bonding: fix MAC address when one port resets
> 
>   drivers/net/bonding/rte_eth_bond_api.c |  1 +
>   drivers/net/bonding/rte_eth_bond_pmd.c | 11 ++++++++---
>   2 files changed, 9 insertions(+), 3 deletions(-)
> 

Re: [dpdk-dev] [PATCH 0/2] fixes for bonding

[Wei Hu (Xavier)]

Are there any comments on this series?
Thanks.

Xavier

On 2020/2/25 17:29, Wei Hu (Xavier) wrote:
> This series include two fixes patches for bonding PMD driver.
>
> Wei Hu (Xavier) (2):
>    net/bonding: fix MAC address when switching active port
>    net/bonding: fix MAC address when one port resets
>
>   drivers/net/bonding/rte_eth_bond_api.c |  1 +
>   drivers/net/bonding/rte_eth_bond_pmd.c | 11 ++++++++---
>   2 files changed, 9 insertions(+), 3 deletions(-)
>