| Message ID | 20210217162656.1983277-1-ferruh.yigit@intel.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Delegated to: | Ferruh Yigit |
| Headers | show |
| Series | [v3] net/bonding: fix LACP system address check | expand |
| Context | Check | Description |
|---|---|---|
| ci/github-robot | success | github build: passed |
| ci/travis-robot | fail | travis build: failed |
| ci/iol-abi-testing | success | Testing PASS |
| ci/iol-testing | success | Testing PASS |
| ci/iol-intel-Performance | success | Performance Testing PASS |
| ci/iol-broadcom-Performance | success | Performance Testing PASS |
| ci/intel-Testing | success | Testing PASS |
| ci/iol-broadcom-Functional | success | Functional Testing PASS |
| ci/Intel-compilation | success | Compilation OK |
| ci/checkpatch | warning | coding style issues |
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.
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?
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 >
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.
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