diff mbox series

[1/2] net/bonding: fix MAC address when switching active port

Message ID 20200225092903.38455-2-huwei013@chinasoftinc.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series fixes for bonding |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/Intel-compilation success Compilation OK

Commit Message

Wei Hu (Xavier) Feb. 25, 2020, 9:29 a.m. UTC 
  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(-)

Comments

Ferruh Yigit Feb. 27, 2020, 5:03 p.m. UTC | #1 
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.
Wei Hu (Xavier) Feb. 28, 2020, 1:31 a.m. UTC | #2 
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
Wei Hu (Xavier) March 2, 2020, 12:58 a.m. UTC | #3 
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
diff mbox series

Patch

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);
 		}
 	}