Re: [dpdk-dev] [PATCH v2] fix lacp check system address

[podovinnikov <podovinnikov@protei.ru>]

Hi Ferruh!

Unfortunately I didn't see any patch from this link

https://patchwork.dpdk.org/user/todo/dpdk/?series=8679

10.04.2020 13:24, Ferruh Yigit пишет:
> On 4/10/2020 11:21 AM, Ferruh Yigit wrote:
>> On 4/10/2020 11:15 AM, Ferruh Yigit wrote:
>>> On 11/26/2019 3:09 PM, podovinnikov wrote:
>>>> Hi Ferruh
>>>>
>>>>> This is bonding patch, please cc bonding maintainers (Chas, instead of John W.
>>>>> Linville).
>>>> Sorry about that. I have 2 patches ( one for af packet, one for bonding).
>>>>
>>>> I am a little confused.
>>>>
>>>>
>>>>> Can you please give more details, what is failing what is the impact etc.
>>>> 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.
>>> Hi Vadim,
>>>
>>> Thanks for the info and sorry for late response, it is sitting in backlog for a
>>> long time now.
>>>
>>> @Chas, do you have any objection on the fix, if not I am planning to merge it soon.
>>>
>>> Thanks,
>>> ferruh
>> cc'ed Xavier, he also has a bonding patch in the backlog waiting for review [1].
>>
>>
>> Vadim, Xavier,
>>
>> Since you both send bonding patches, I assume you both know about bonding at
>> some level, at least more than me, so would you mind reviewing eachother's
>> patch? So both can go it.
> + other email address of the Xavier.
>
>> Thanks,
>> ferruh
>>
>>
>> [1]
>> https://patchwork.dpdk.org/user/todo/dpdk/?series=8679
>>
>>>>
>>>> 26.11.2019 16:27, Ferruh Yigit пишет:
>>>>> Hi Vadim,
>>>>>
>>>>> On 11/26/2019 11:55 AM, Vadim wrote:
>>>>>> Signed-off-by: Vadim <podovinnikov@protei.ru>
>>>>> Can you please provide full "Name Surname <email@adress>" signature?
>>>>>
>>>>>
>>>>> This is bonding patch, please cc bonding maintainers (Chas, instead of John W.
>>>>> Linville).
>>>>>
>>>>>> fix lacp check system address
>>>>> Can you please give more details, what is failing what is the impact etc.
>>>>>
>>>>>> ---
>>>>>>    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 b77a37ddb..d4dda790a 100644
>>>>>> --- a/drivers/net/bonding/rte_eth_bond_8023ad.c
>>>>>> +++ b/drivers/net/bonding/rte_eth_bond_8023ad.c
>>>>>> @@ -792,18 +792,33 @@ 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_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 not equal "
>>>>>> +						"self system: %s\n",
>>>>>> +				preferred_system_name, self_system_name);
>>>>>>    		}
>>>>>>    		rte_pktmbuf_free(lacp_pkt);
>>>>>>    	} else
>>>>>>