[dpdk-stable] [PATCH v3] net/bonding: fix LACP system address check

[dpdk-stable] [PATCH v3] net/bonding: fix LACP system address check

[Ferruh Yigit]

From: Vadim Podovinnikov <podovinnikov@protei.ru>

In bond (LACP) we have several NICs (ports), when we have negotiation
with peer about what port we prefer, we send information about what
system we preferred in partner system name field. Peer also sends us
what partner system name it prefer.

When we receive a message from it we must compare its preferred system
name with our system name, but not with our port mac address

In my test I have several problems with that:
1. If master port (mac address same as system address) shuts down (I
   have two ports) I loose connection
2. If secondary port (mac address not same as system address) receives
   message before master port, my connection is not established.

Fixes: 56cbc0817399 ("net/bonding: fix LACP negotiation")
Cc: stable@dpdk.org

Signed-off-by: Vadim Podovinnikov <podovinnikov@protei.ru>
---
Cc: zhangliang@bigo.sg
Cc: Declan Doherty <declan.doherty@intel.com>

v3: Re-sent with rebase
* Patch title updated, commit log updated with info shared in email
* Sign-off updated with full name
* Debug log slightly updated
* Syntax slightly updated
---
 drivers/net/bonding/rte_eth_bond_8023ad.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/net/bonding/rte_eth_bond_8023ad.c b/drivers/net/bonding/rte_eth_bond_8023ad.c
index 5fe004e551de..128754f4595a 100644
--- a/drivers/net/bonding/rte_eth_bond_8023ad.c
+++ b/drivers/net/bonding/rte_eth_bond_8023ad.c
@@ -804,19 +804,34 @@ rx_machine_update(struct bond_dev_private *internals, uint16_t slave_id,
 		struct rte_mbuf *lacp_pkt) {
 	struct lacpdu_header *lacp;
 	struct lacpdu_actor_partner_params *partner;
+	struct port *port, *agg;
 
 	if (lacp_pkt != NULL) {
 		lacp = rte_pktmbuf_mtod(lacp_pkt, struct lacpdu_header *);
 		RTE_ASSERT(lacp->lacpdu.subtype == SLOW_SUBTYPE_LACP);
 
 		partner = &lacp->lacpdu.partner;
+		port = &bond_mode_8023ad_ports[slave_id];
+		agg = &bond_mode_8023ad_ports[port->aggregator_port_id];
+
 		if (rte_is_zero_ether_addr(&partner->port_params.system) ||
 			rte_is_same_ether_addr(&partner->port_params.system,
-			&internals->mode4.mac_addr)) {
+				&agg->actor.system)) {
 			/* This LACP frame is sending to the bonding port
 			 * so pass it to rx_machine.
 			 */
 			rx_machine(internals, slave_id, &lacp->lacpdu);
+		} else {
+			char preferred_system_name[RTE_ETHER_ADDR_FMT_SIZE];
+			char self_system_name[RTE_ETHER_ADDR_FMT_SIZE];
+
+			rte_ether_format_addr(preferred_system_name,
+				RTE_ETHER_ADDR_FMT_SIZE, &partner->port_params.system);
+			rte_ether_format_addr(self_system_name,
+				RTE_ETHER_ADDR_FMT_SIZE, &agg->actor.system);
+			MODE4_DEBUG("preferred partner system %s "
+				"is not equal with self system: %s\n",
+				preferred_system_name, self_system_name);
 		}
 		rte_pktmbuf_free(lacp_pkt);
 	} else
-- 
2.29.2

Re: [dpdk-stable] [PATCH v3] net/bonding: fix LACP system address check

[Ferruh Yigit]

On 2/17/2021 4:26 PM, Ferruh Yigit wrote:
> From: Vadim Podovinnikov <podovinnikov@protei.ru>
> 
> In bond (LACP) we have several NICs (ports), when we have negotiation
> with peer about what port we prefer, we send information about what
> system we preferred in partner system name field. Peer also sends us
> what partner system name it prefer.
> 
> When we receive a message from it we must compare its preferred system
> name with our system name, but not with our port mac address
> 
> In my test I have several problems with that:
> 1. If master port (mac address same as system address) shuts down (I
>     have two ports) I loose connection
> 2. If secondary port (mac address not same as system address) receives
>     message before master port, my connection is not established.
> 
> Fixes: 56cbc0817399 ("net/bonding: fix LACP negotiation")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Vadim Podovinnikov <podovinnikov@protei.ru>
> ---
> Cc: zhangliang@bigo.sg
> Cc: Declan Doherty <declan.doherty@intel.com>
> 
> v3: Re-sent with rebase
> * Patch title updated, commit log updated with info shared in email
> * Sign-off updated with full name
> * Debug log slightly updated
> * Syntax slightly updated

This patch is waiting for review for a long time, if there is no objection I am 
planning to get it early in the release to give it a time to fix any possible 
issues.

Re: [dpdk-stable] [dpdk-dev] [PATCH v3] net/bonding: fix LACP system address check

[Ferruh Yigit]

On 2/17/2021 4:30 PM, Ferruh Yigit wrote:
> On 2/17/2021 4:26 PM, Ferruh Yigit wrote:
>> From: Vadim Podovinnikov <podovinnikov@protei.ru>
>>
>> In bond (LACP) we have several NICs (ports), when we have negotiation
>> with peer about what port we prefer, we send information about what
>> system we preferred in partner system name field. Peer also sends us
>> what partner system name it prefer.
>>
>> When we receive a message from it we must compare its preferred system
>> name with our system name, but not with our port mac address
>>
>> In my test I have several problems with that:
>> 1. If master port (mac address same as system address) shuts down (I
>>     have two ports) I loose connection
>> 2. If secondary port (mac address not same as system address) receives
>>     message before master port, my connection is not established.
>>
>> Fixes: 56cbc0817399 ("net/bonding: fix LACP negotiation")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Vadim Podovinnikov <podovinnikov@protei.ru>
>> ---
>> Cc: zhangliang@bigo.sg
>> Cc: Declan Doherty <declan.doherty@intel.com>
>>
>> v3: Re-sent with rebase
>> * Patch title updated, commit log updated with info shared in email
>> * Sign-off updated with full name
>> * Debug log slightly updated
>> * Syntax slightly updated
> 
> This patch is waiting for review for a long time, if there is no objection I am 
> planning to get it early in the release to give it a time to fix any possible 
> issues.

Chas, Connor, any comment?

Re: [dpdk-stable] [PATCH v3] net/bonding: fix LACP system address check

[Min Hu (Connor)]

Acked-by: Min Hu (Connor) <humin29@huawei.com>

在 2021/2/18 0:26, Ferruh Yigit 写道:
> From: Vadim Podovinnikov <podovinnikov@protei.ru>
> 
> In bond (LACP) we have several NICs (ports), when we have negotiation
> with peer about what port we prefer, we send information about what
> system we preferred in partner system name field. Peer also sends us
> what partner system name it prefer.
> 
> When we receive a message from it we must compare its preferred system
> name with our system name, but not with our port mac address
> 
> In my test I have several problems with that:
> 1. If master port (mac address same as system address) shuts down (I
>     have two ports) I loose connection
> 2. If secondary port (mac address not same as system address) receives
>     message before master port, my connection is not established.
> 
> Fixes: 56cbc0817399 ("net/bonding: fix LACP negotiation")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Vadim Podovinnikov <podovinnikov@protei.ru>
> ---
> Cc: zhangliang@bigo.sg
> Cc: Declan Doherty <declan.doherty@intel.com>
> 
> v3: Re-sent with rebase
> * Patch title updated, commit log updated with info shared in email
> * Sign-off updated with full name
> * Debug log slightly updated
> * Syntax slightly updated
> ---
>   drivers/net/bonding/rte_eth_bond_8023ad.c | 17 ++++++++++++++++-
>   1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/bonding/rte_eth_bond_8023ad.c b/drivers/net/bonding/rte_eth_bond_8023ad.c
> index 5fe004e551de..128754f4595a 100644
> --- a/drivers/net/bonding/rte_eth_bond_8023ad.c
> +++ b/drivers/net/bonding/rte_eth_bond_8023ad.c
> @@ -804,19 +804,34 @@ rx_machine_update(struct bond_dev_private *internals, uint16_t slave_id,
>   		struct rte_mbuf *lacp_pkt) {
>   	struct lacpdu_header *lacp;
>   	struct lacpdu_actor_partner_params *partner;
> +	struct port *port, *agg;
>   
>   	if (lacp_pkt != NULL) {
>   		lacp = rte_pktmbuf_mtod(lacp_pkt, struct lacpdu_header *);
>   		RTE_ASSERT(lacp->lacpdu.subtype == SLOW_SUBTYPE_LACP);
>   
>   		partner = &lacp->lacpdu.partner;
> +		port = &bond_mode_8023ad_ports[slave_id];
> +		agg = &bond_mode_8023ad_ports[port->aggregator_port_id];
> +
>   		if (rte_is_zero_ether_addr(&partner->port_params.system) ||
>   			rte_is_same_ether_addr(&partner->port_params.system,
> -			&internals->mode4.mac_addr)) {
> +				&agg->actor.system)) {
>   			/* This LACP frame is sending to the bonding port
>   			 * so pass it to rx_machine.
>   			 */
>   			rx_machine(internals, slave_id, &lacp->lacpdu);
> +		} else {
> +			char preferred_system_name[RTE_ETHER_ADDR_FMT_SIZE];
> +			char self_system_name[RTE_ETHER_ADDR_FMT_SIZE];
> +
> +			rte_ether_format_addr(preferred_system_name,
> +				RTE_ETHER_ADDR_FMT_SIZE, &partner->port_params.system);
> +			rte_ether_format_addr(self_system_name,
> +				RTE_ETHER_ADDR_FMT_SIZE, &agg->actor.system);
> +			MODE4_DEBUG("preferred partner system %s "
> +				"is not equal with self system: %s\n",
> +				preferred_system_name, self_system_name);
>   		}
>   		rte_pktmbuf_free(lacp_pkt);
>   	} else
> 

Re: [dpdk-stable] [PATCH v3] net/bonding: fix LACP system address check

[Ferruh Yigit]

On 3/5/2021 1:43 AM, Min Hu (Connor) wrote:

> 在 2021/2/18 0:26, Ferruh Yigit 写道:
>> From: Vadim Podovinnikov <podovinnikov@protei.ru>
>>
>> In bond (LACP) we have several NICs (ports), when we have negotiation
>> with peer about what port we prefer, we send information about what
>> system we preferred in partner system name field. Peer also sends us
>> what partner system name it prefer.
>>
>> When we receive a message from it we must compare its preferred system
>> name with our system name, but not with our port mac address
>>
>> In my test I have several problems with that:
>> 1. If master port (mac address same as system address) shuts down (I
>>     have two ports) I loose connection
>> 2. If secondary port (mac address not same as system address) receives
>>     message before master port, my connection is not established.
>>
>> Fixes: 56cbc0817399 ("net/bonding: fix LACP negotiation")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Vadim Podovinnikov <podovinnikov@protei.ru>
 >
 > Acked-by: Min Hu (Connor) <humin29@huawei.com>
 >

Applied to dpdk-next-net/main, thanks.