net/bonding: add add/remove mac addrs
Subsystems (2)
PATCH v2 1/1 net/bonding: add add/remove mac addrs
add functions to add/remove MAC addresses
--- drivers/net/bonding/rte_eth_bond_api.c | 8 +- drivers/net/bonding/rte_eth_bond_pmd.c | 123 ++++++++++++++++++++++++++++- drivers/net/bonding/rte_eth_bond_private.h | 8 ++ 3 files changed, 134 insertions(+), 5 deletions(-)
drivers/net/bonding/rte_eth_bond_api.c +7 -1
@@ -387,9 +387,12 @@ __eth_bond_slave_add_lock_free(uint16_t bonded_port_id, uint16_t slave_port_id)/* Add slave details to bonded device */ slave_eth_dev->data->dev_flags |=RTE_ETH_DEV_BONDED_SLAVE ; -/* Update all slave devices MACs*/ +/* Update all slave devices MACs */ mac_address_slaves_update (bonded_eth_dev); +/* Add additional MAC addresses to the slave */ +slave_add_mac_addresses (bonded_eth_dev, slave_port_id); +/* Register link status change callback with bonded device pointer as * argument*/ rte_eth_dev_callback_register(slave_port_id, RTE_ETH_EVENT_INTR_LSC,
@@ -491,6 +494,9 @@ __eth_bond_slave_remove_lock_free(uint16_t bonded_port_id,rte_eth_dev_default_mac_addr_set (slave_port_id, &(internals->slaves[slave_idx].persisted_mac_addr)); +/* remove additional MAC addresses from the slave */ +slave_remove_mac_addresses (bonded_eth_dev, slave_port_id); +/* * Remove bond device flows from slave device. * Note: don't restore flow isolate mode.
drivers/net/bonding/rte_eth_bond_pmd.c +119 -4
160 lines not on this page · raw message
drivers/net/bonding/rte_eth_bond_private.h +8 -0
@@ -230,6 +230,14 @@ mac_address_get(struct rte_eth_dev *eth_dev, struct ether_addr *dst_mac_addr);int mac_address_slaves_update (struct rte_eth_dev *bonded_eth_dev); +int +slave_add_mac_addresses (struct rte_eth_dev *bonded_eth_dev, +uint16_t slave_port_id); + +int +slave_remove_mac_addresses (struct rte_eth_dev *bonded_eth_dev, +uint16_t slave_port_id); +int bond_ethdev_mode_set (struct rte_eth_dev *eth_dev,int mode);
Signature · 1 line
2.16.1.windows.1
Quoted 14 lines · On Mon, 18 Jun 2018 15:27:16 +0300 Alex Kiselev <[email protected]> wrote:
drivers/net/bonding/rte_eth_bond_pmd.c +14
-0+static const struct ether_addr null_mac_addr; + +/* +* Add additional MAC addresses to the slave +*/ +int +slave_add_mac_addresses (struct rte_eth_dev *bonded_eth_dev, +uint16_t slave_port_id) +{ +int i, ret; +struct ether_addr *mac_addr; + +/* add additional MACs to the slave */ +for (i =1 ; i <BOND_MAX_MAC_ADDRS ; i++) {
drivers/net/bonding/rte_eth_bond_pmd.c mac_address_set(struct rte_eth_dev *eth_dev, struct ether_addr *new_mac_addr) · line 1615
Quoted 10 lines
drivers/net/bonding/rte_eth_bond_pmd.c +10
-0+ mac_addr = &bonded_eth_dev->data->mac_addrs[i]; +if (is_same_ether_addr (mac_addr, &null_mac_addr)) +break ; + + ret =rte_eth_dev_mac_addr_add (slave_port_id, mac_addr,0 ); +if (ret <0 ) +return ret; + } + +return 0 ;
You need to unwind if adding MAC address to one of the slave devices worked, and the second one did not.
Quoted 21 lines · Stephen Hemminger
Quoted 18 lines · On Mon, 18 Jun 2018 15:27:16 +0300 Alex Kiselev <[email protected]> wrote:
drivers/net/bonding/rte_eth_bond_pmd.c +18
-0+static const struct ether_addr null_mac_addr; + +/* +* Add additional MAC addresses to the slave +*/ +int +slave_add_mac_addresses (struct rte_eth_dev *bonded_eth_dev, +uint16_t slave_port_id) +{ +int i, ret; +struct ether_addr *mac_addr; + +/* add additional MACs to the slave */ +for (i =1 ; i <BOND_MAX_MAC_ADDRS ; i++) { + mac_addr = &bonded_eth_dev->data->mac_addrs[i]; +if (is_same_ether_addr (mac_addr, &null_mac_addr)) +break ; +
Quoted 9 lines
drivers/net/bonding/rte_eth_bond_pmd.c mac_address_set(struct rte_eth_dev *eth_dev, struct ether_addr *new_mac_addr) · line 1615
Quoted 6 lines
drivers/net/bonding/rte_eth_bond_pmd.c +6
-0+ ret =rte_eth_dev_mac_addr_add (slave_port_id, mac_addr,0 ); +if (ret <0 ) +return ret; + } + +return 0 ;You need to unwind if adding MAC address to one of the slave devices worked, and the second one did not.
Yes, probably. But that doesn't help with the new slave problem. If you add a new slave
and it is unable to add all the MAC addresses, what then? The only reasonable thing might
be to put that interface into promiscuous mode. At some point you need to draw the line,
where is the PMD and where is the application?
Thankfully, people tend to enslave the same types of PMDs and the capabilities are
generally similar.
Hi
From: Chas Williams
Quoted 22 lines
Quoted 19 lines · Stephen Hemminger
Quoted 16 lines · On Mon, 18 Jun 2018 15:27:16 +0300 Alex Kiselev <[email protected]> wrote:
drivers/net/bonding/rte_eth_bond_pmd.c +16
-0+static const struct ether_addr null_mac_addr; + +/* +* Add additional MAC addresses to the slave */ int +slave_add_mac_addresses (struct rte_eth_dev *bonded_eth_dev, +uint16_t slave_port_id) { +int i, ret; +struct ether_addr *mac_addr; + +/* add additional MACs to the slave */ +for (i =1 ; i <BOND_MAX_MAC_ADDRS ; i++) { + mac_addr = &bonded_eth_dev->data->mac_addrs[i]; +if (is_same_ether_addr (mac_addr, &null_mac_addr)) +break ; + + ret =rte_eth_dev_mac_addr_add (slave_port_id, mac_addr,0 );
Quoted 9 lines
Quoted 8 lines
drivers/net/bonding/rte_eth_bond_pmd.c mac_address_set(struct rte_eth_dev *eth_dev, struct ether_addr *new_mac_addr) · line 1615
Quoted 5 lines
drivers/net/bonding/rte_eth_bond_pmd.c +5
-0+if (ret <0 ) +return ret; + } + +return 0 ;You need to unwind if adding MAC address to one of the slave devices worked, and the second one did not.
Agree.
Quoted 3 lines
Yes, probably. But that doesn't help with the new slave problem. If you add a new slave and it is unable to add all the MAC addresses, what then? The only reasonable thing might be to put that interface into promiscuous mode.
I think that in this case the slave should be rejected.
The reason is that all the slaves should be able to receive\send the same traffic all the time.
Quoted 5 lines
At some point you need to draw the line, where is the PMD and where is the
application?Thankfully, people tend to enslave the same types of PMDs and the capabilities are generally similar.
So, I think we need to enforce it in the bonding PMD.
Please look at the bonding rte_flow design \ RSS conf (also in bonding documentation):
The main principle in the design is that all the time all the slaves able to get\send the same traffic:
So, a new slave which cannot apply all the current bonding flow rules will be rejected.
And a new rule which cannot be applied to all the current slaves will be rejected and deleted from all the slaves. I think it should be the same for mac address adding and all the other configurations which affect a traffic receiving\sending.
Matan
Quoted 32 lines
From: Chas Williams
Quoted 30 lines
Quoted 27 lines · Stephen Hemminger
Quoted 11 lines · On Mon, 18 Jun 2018 15:27:16 +0300 Alex Kiselev <[email protected]> wrote:
drivers/net/bonding/rte_eth_bond_pmd.c +11
-0+static const struct ether_addr null_mac_addr; + +/* +* Add additional MAC addresses to the slave */ int +slave_add_mac_addresses (struct rte_eth_dev *bonded_eth_dev, +uint16_t slave_port_id) { +int i, ret; +struct ether_addr *mac_addr; + +/* add additional MACs to the slave */ +for (i =1 ; i <BOND_MAX_MAC_ADDRS ; i++) {drivers/net/bonding/rte_eth_bond_pmd.c mac_address_set(struct rte_eth_dev *eth_dev, struct ether_addr *new_mac_addr) · line 1615
Quoted 10 lines
drivers/net/bonding/rte_eth_bond_pmd.c +10
-0+ mac_addr = &bonded_eth_dev->data->mac_addrs[i]; +if (is_same_ether_addr (mac_addr, &null_mac_addr)) +break ; + + ret =rte_eth_dev_mac_addr_add (slave_port_id, mac_addr,0 ); +if (ret <0 ) +return ret; + } + +return 0 ;You need to unwind if adding MAC address to one of the slave devices worked, and the second one did not.
Done in patch v3.
Also, I've added the same rollback to bond_ethdev_mac_addr_add().
Quoted 27 lines
Agree.
Quoted 3 lines
Yes, probably. But that doesn't help with the new slave problem. If you add a new slave and it is unable to add all the MAC addresses, what then? The only reasonable thing might be to put that interface into promiscuous mode.
I think that in this case the slave should be rejected.
The reason is that all the slaves should be able to receive\send the same traffic all the time.Quoted 5 lines
At some point you need to draw the line, where is the PMD and where is the
application?Thankfully, people tend to enslave the same types of PMDs and the capabilities are generally similar.
So, I think we need to enforce it in the bonding PMD.
Please look at the bonding
rte_flowdesign \ RSS conf (also in bonding documentation):The main principle in the design is that all the time all the
slaves able to get\send the same traffic:
So, a new slave which cannot apply all the current bonding flow rules will be rejected. And a new rule which cannot be applied to all the current slaves
will be rejected and deleted from all the slaves.
I think it should be the same for mac address adding and all the
other configurations which affect a traffic receiving\sending.
Signature · 1 line
Alex
Quoted 10 lines · On Mon, 18 Jun 2018 15:27:16 +0300 Alex Kiselev <[email protected]> wrote:
drivers/net/bonding/rte_eth_bond_pmd.c +10
-0+/* +* Remove additional MAC addresses from the slave +*/ +int +slave_remove_mac_addresses (struct rte_eth_dev *bonded_eth_dev, +uint16_t slave_port_id) +{ +int i, ret; +struct ether_addr *mac_addr; +
drivers/net/bonding/rte_eth_bond_pmd.c mac_address_set(struct rte_eth_dev *eth_dev, struct ether_addr *new_mac_addr) · line 1637
Quoted 10 lines
drivers/net/bonding/rte_eth_bond_pmd.c +10
-0+/* add additional MACs to the slave */ +for (i =1 ; i <BOND_MAX_MAC_ADDRS ; i++) { + mac_addr = &bonded_eth_dev->data->mac_addrs[i]; +if (is_same_ether_addr (mac_addr, &null_mac_addr)) +break ; + + ret =rte_eth_dev_mac_addr_remove (slave_port_id, mac_addr); +if (ret <0 ) +return ret; + }
Not sure this is the best semantic if remove fails on one of many slaves. Perhaps it should always remove it from all slaves.
Or maybe a first pass to see if the address exists, then a no-fail removal pass.
Quoted 15 lines · Stephen Hemminger
drivers/net/bonding/rte_eth_bond_pmd.c mac_address_set(struct rte_eth_dev *eth_dev, struct ether_addr *new_mac_addr) · line 1628
Quoted 11 lines · On Mon, 18 Jun 2018 15:27:16 +0300 Alex Kiselev <[email protected]> wrote:
drivers/net/bonding/rte_eth_bond_pmd.c +11
-0+/* +* Remove additional MAC addresses from the slave +*/ +int +slave_remove_mac_addresses (struct rte_eth_dev *bonded_eth_dev, +uint16_t slave_port_id) +{ +int i, ret; +struct ether_addr *mac_addr; + +/* add additional MACs to the slave */
And this should say remove as well.
Quoted 16 lines
drivers/net/bonding/rte_eth_bond_pmd.c mac_address_set(struct rte_eth_dev *eth_dev, struct ether_addr *new_mac_addr) · line 1637
Quoted 9 lines
drivers/net/bonding/rte_eth_bond_pmd.c +9
-0+for (i =1 ; i <BOND_MAX_MAC_ADDRS ; i++) { + mac_addr = &bonded_eth_dev->data->mac_addrs[i]; +if (is_same_ether_addr (mac_addr, &null_mac_addr)) +break ; + + ret =rte_eth_dev_mac_addr_remove (slave_port_id, mac_addr); +if (ret <0 ) +return ret; + }Not sure this is the best semantic if remove fails on one of many slaves. Perhaps it should always remove it from all slaves.
Or maybe a first pass to see if the address exists, then a no-fail removal pass.
Quoted 29 lines
Quoted 10 lines · On Mon, 18 Jun 2018 15:27:16 +0300 Alex Kiselev <[email protected]> wrote:
drivers/net/bonding/rte_eth_bond_pmd.c +10
-0+/* +* Remove additional MAC addresses from the slave +*/ +int +slave_remove_mac_addresses (struct rte_eth_dev *bonded_eth_dev, +uint16_t slave_port_id) +{ +int i, ret; +struct ether_addr *mac_addr; +drivers/net/bonding/rte_eth_bond_pmd.c mac_address_set(struct rte_eth_dev *eth_dev, struct ether_addr *new_mac_addr) · line 1637
Quoted 10 lines
drivers/net/bonding/rte_eth_bond_pmd.c +10
-0+/* add additional MACs to the slave */ +for (i =1 ; i <BOND_MAX_MAC_ADDRS ; i++) { + mac_addr = &bonded_eth_dev->data->mac_addrs[i]; +if (is_same_ether_addr (mac_addr, &null_mac_addr)) +break ; + + ret =rte_eth_dev_mac_addr_remove (slave_port_id, mac_addr); +if (ret <0 ) +return ret; + }Not sure this is the best semantic if remove fails on one of many slaves. Perhaps it should always remove it from all slaves.
Quoted 2 lines
Or maybe a first pass to see if the address exists, then a no-fail removal pass.
I think to always remove is the right thing.
Done in the patch v3
Signature · 1 line
Alex