diff mbox series

[bugfix] set same mac of bond slave

Message ID CAOE=1Z1+oQmrofXh7t4fEzCNL_DrHO8iYZ7r2TKym56cyJoA=A@mail.gmail.com (mailing list archive)
State Changes Requested
Delegated to: Ferruh Yigit
Headers
Series [bugfix] set same mac of bond slave |

Checks

Context Check Description
ci/loongarch-compilation warning apply patch failure
ci/Intel-compilation warning apply issues
ci/iol-testing warning apply patch failure

Commit Message

Simon Jones April 30, 2024, 1:17 a.m. UTC 
  Hi, all

Should bond slave need set same mac address?
Like this patch does:
https://github.com/batmancn/dpdk/commit/a484ac3f407a6a22d4eb63da23e98e8e76833722
```
commit a484ac3f407a6a22d4eb63da23e98e8e76833722 (HEAD ->
bugfix-bond-same-mac-20240429, batmancn/bugfix-bond-same-mac-20240429)
Author: batmancn <batmanustc@gmail.com>
Date:   Mon Apr 29 18:44:18 2024 +0800

    Bugfix, config same mac address of bond slave.

    Signed-off-by: Simon Jones <batmanustc@gmail.com>

~
```

----
Simon Jones

Comments

Stephen Hemminger April 30, 2024, 3:37 a.m. UTC | #1 
On Tue, 30 Apr 2024 09:17:30 +0800
Simon Jones <batmanustc@gmail.com> wrote:

> Hi, all
> 
> Should bond slave need set same mac address?
> Like this patch does:
> https://github.com/batmancn/dpdk/commit/a484ac3f407a6a22d4eb63da23e98e8e76833722
> ```
> commit a484ac3f407a6a22d4eb63da23e98e8e76833722 (HEAD ->
> bugfix-bond-same-mac-20240429, batmancn/bugfix-bond-same-mac-20240429)
> Author: batmancn <batmanustc@gmail.com>
> Date:   Mon Apr 29 18:44:18 2024 +0800
> 
>     Bugfix, config same mac address of bond slave.
> 
>     Signed-off-by: Simon Jones <batmanustc@gmail.com>

This won't apply to current DPDK since slave has been renamed to member.
Simon Jones April 30, 2024, 6:37 a.m. UTC | #2 
OK

Does you mean that, need this patch to set same mac, but change
"internals->slaves" to "internals->members".

What I want to say is, in my test (DPDK-21.11), bond mac is NOT same in
802.3ad mode.
So when bond connect to some SWITCH, will it NOT work?
So I want to set same mac, and discuss here if it's necessary.

----
Simon Jones


Stephen Hemminger <stephen@networkplumber.org> 于2024年4月30日周二 11:37写道：

> On Tue, 30 Apr 2024 09:17:30 +0800
> Simon Jones <batmanustc@gmail.com> wrote:
>
> > Hi, all
> >
> > Should bond slave need set same mac address?
> > Like this patch does:
> >
> https://github.com/batmancn/dpdk/commit/a484ac3f407a6a22d4eb63da23e98e8e76833722
> > ```
> > commit a484ac3f407a6a22d4eb63da23e98e8e76833722 (HEAD ->
> > bugfix-bond-same-mac-20240429, batmancn/bugfix-bond-same-mac-20240429)
> > Author: batmancn <batmanustc@gmail.com>
> > Date:   Mon Apr 29 18:44:18 2024 +0800
> >
> >     Bugfix, config same mac address of bond slave.
> >
> >     Signed-off-by: Simon Jones <batmanustc@gmail.com>
>
> This won't apply to current DPDK since slave has been renamed to member.
>
Stephen Hemminger April 30, 2024, 3:55 p.m. UTC | #3 
On Tue, 30 Apr 2024 09:17:30 +0800
Simon Jones <batmanustc@gmail.com> wrote:

> +               for (i = 0; i < internals->slave_count; i++) {
> +                       if (rte_eth_dev_default_mac_addr_set(
> +                                       internals->slaves[i].port_id,
> +                                       bonded_eth_dev->data->mac_addrs)) {
> +                               RTE_BOND_LOG(ERR, "Failed to update port Id
> %d MAC address",
> +
> internals->slaves[i].port_id);
> +                               return -1;
> +                       }
> +               }
>                 bond_mode_8023ad_mac_address_update(bonding_eth_dev);

This patch looks wrong. If you read bond_mode_8023ad_mac_address_update() it
expects each member to have different address.

Other modes, already set all members of the bond to have the same mac address.
diff mbox series

Patch

diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c
b/drivers/net/bonding/rte_eth_bond_pmd.c
index c40d18d128..0387a9b2aa 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -1562,6 +1562,15 @@  mac_address_members_update(struct rte_eth_dev
*bonding_eth_dev)
                }
                break;
        case BONDING_MODE_8023AD:
+               for (i = 0; i < internals->slave_count; i++) {
+                       if (rte_eth_dev_default_mac_addr_set(
+                                       internals->slaves[i].port_id,
+                                       bonded_eth_dev->data->mac_addrs)) {
+                               RTE_BOND_LOG(ERR, "Failed to update port Id
%d MAC address",
+
internals->slaves[i].port_id);
+                               return -1;
+                       }
+               }
                bond_mode_8023ad_mac_address_update(bonding_eth_dev);
                break;
        case BONDING_MODE_ACTIVE_BACKUP:
diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index f1c658f49e..c3f8b8d01e 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -1746,7 +1746,8 @@  rte_eth_dev_start(uint16_t port_id)
                return ret;

        /* Lets restore MAC now if device does not support live change */
-       if (*dev_info.dev_flags & RTE_ETH_DEV_NOLIVE_MAC_ADDR)
+       if ((*dev_info.dev_flags & RTE_ETH_DEV_NOLIVE_MAC_ADDR) &&
+               !(*dev_info.dev_flags & RTE_ETH_DEV_BONDED_SLAVE))
                eth_dev_mac_restore(dev, &dev_info);

        diag = (*dev->dev_ops->dev_start)(dev);