From 68218b87c184e2dbea1ca3540a92a849240c115f Mon Sep 17 00:00:00 2001 From: David Marchand Date: Wed, 10 Apr 2019 14:53:49 +0200 Subject: [PATCH] net/bonding: prefer allmulti to promiscuous for LACP Rather than the global promiscuous mode on all slaves, let's try to use allmulti. To do this, we apply the same mechanism than for promiscuous and drop in rx_burst. When adding a slave, we first try to use allmulti, else we try promiscuous. Because of this, we must be able to handle allmulti on the bonding interface, so the associated dev_ops is added with checks on which mode has been applied on each slave. Rather than add a new flag in the bond private structure, we can remove promiscuous_en since ethdev already maintains this information. When starting the bond device, there is no promisc/allmulti re-apply as, again, ethdev does this itself. On reception, we must inspect if the packets are multicast or unicast and take a decision to drop based on which mode has been enabled on the bonding interface. Note: in the future, if we define an API so that we can add/remove mc addresses to a port (rather than the current global set_mc_addr_list API), we can refine this to only register the LACP multicast mac address. Signed-off-by: David Marchand Acked-by: Chas Williams --- drivers/net/bonding/rte_eth_bond_8023ad.c | 51 +++++++- .../net/bonding/rte_eth_bond_8023ad_private.h | 7 ++ drivers/net/bonding/rte_eth_bond_pmd.c | 113 ++++++++++++++---- drivers/net/bonding/rte_eth_bond_private.h | 3 - 4 files changed, 148 insertions(+), 26 deletions(-) diff --git a/drivers/net/bonding/rte_eth_bond_8023ad.c b/drivers/net/bonding/rte_eth_bond_8023ad.c index d764dad331..fbc69051a9 100644 --- a/drivers/net/bonding/rte_eth_bond_8023ad.c +++ b/drivers/net/bonding/rte_eth_bond_8023ad.c @@ -910,6 +910,49 @@ bond_mode_8023ad_periodic_cb(void *arg) bond_mode_8023ad_periodic_cb, arg); } +static int +bond_mode_8023ad_register_lacp_mac(uint16_t slave_id) +{ + rte_eth_allmulticast_enable(slave_id); + if (rte_eth_allmulticast_get(slave_id)) { + RTE_BOND_LOG(DEBUG, "forced allmulti for port %u", + slave_id); + bond_mode_8023ad_ports[slave_id].forced_rx_flags = + BOND_8023AD_FORCED_ALLMULTI; + return 0; + } + + rte_eth_promiscuous_enable(slave_id); + if (rte_eth_promiscuous_get(slave_id)) { + RTE_BOND_LOG(DEBUG, "forced promiscuous for port %u", + slave_id); + bond_mode_8023ad_ports[slave_id].forced_rx_flags = + BOND_8023AD_FORCED_PROMISC; + return 0; + } + + return -1; +} + +static void +bond_mode_8023ad_unregister_lacp_mac(uint16_t slave_id) +{ + switch (bond_mode_8023ad_ports[slave_id].forced_rx_flags) { + case BOND_8023AD_FORCED_ALLMULTI: + RTE_BOND_LOG(DEBUG, "unset allmulti for port %u", slave_id); + rte_eth_allmulticast_disable(slave_id); + break; + + case BOND_8023AD_FORCED_PROMISC: + RTE_BOND_LOG(DEBUG, "unset promisc for port %u", slave_id); + rte_eth_promiscuous_disable(slave_id); + break; + + default: + break; + } +} + void bond_mode_8023ad_activate_slave(struct rte_eth_dev *bond_dev, uint16_t slave_id) @@ -951,7 +994,11 @@ bond_mode_8023ad_activate_slave(struct rte_eth_dev *bond_dev, /* use this port as agregator */ port->aggregator_port_id = slave_id; - rte_eth_promiscuous_enable(slave_id); + + if (bond_mode_8023ad_register_lacp_mac(slave_id) < 0) { + RTE_BOND_LOG(WARNING, "slave %u is most likely broken and won't receive LACP packets", + slave_id); + } timer_cancel(&port->warning_timer); @@ -1025,6 +1072,8 @@ bond_mode_8023ad_deactivate_slave(struct rte_eth_dev *bond_dev __rte_unused, old_partner_state = port->partner_state; record_default(port); + bond_mode_8023ad_unregister_lacp_mac(slave_id); + /* If partner timeout state changes then disable timer */ if (!((old_partner_state ^ port->partner_state) & STATE_LACP_SHORT_TIMEOUT)) diff --git a/drivers/net/bonding/rte_eth_bond_8023ad_private.h b/drivers/net/bonding/rte_eth_bond_8023ad_private.h index d905de4259..1470bf7b01 100644 --- a/drivers/net/bonding/rte_eth_bond_8023ad_private.h +++ b/drivers/net/bonding/rte_eth_bond_8023ad_private.h @@ -109,6 +109,13 @@ struct port { uint16_t sm_flags; enum rte_bond_8023ad_selection selected; + /** Indicates if either allmulti or promisc has been enforced on the + * slave so that we can receive lacp packets + */ +#define BOND_8023AD_FORCED_ALLMULTI (1 << 0) +#define BOND_8023AD_FORCED_PROMISC (1 << 1) + uint8_t forced_rx_flags; + uint64_t current_while_timer; uint64_t periodic_timer; uint64_t wait_while_timer; diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c index 49e38f6b17..97ab3f29f3 100644 --- a/drivers/net/bonding/rte_eth_bond_pmd.c +++ b/drivers/net/bonding/rte_eth_bond_pmd.c @@ -273,7 +273,8 @@ rx_burst_8023ad(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts, uint16_t slave_count, idx; uint8_t collecting; /* current slave collecting status */ - const uint8_t promisc = internals->promiscuous_en; + const uint8_t promisc = rte_eth_promiscuous_get(internals->port_id); + const uint8_t allmulti = rte_eth_allmulticast_get(internals->port_id); uint8_t subtype; uint16_t i; uint16_t j; @@ -313,8 +314,10 @@ rx_burst_8023ad(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts, /* Remove packet from array if: * - it is slow packet but no dedicated rxq is present, * - slave is not in collecting state, - * - bonding interface is not in promiscuous mode and - * packet is not multicast and address does not match, + * - bonding interface is not in promiscuous mode: + * - packet is unicast and address does not match, + * - packet is multicast and bonding interface + * is not in allmulti, */ if (unlikely( (!dedicated_rxq && @@ -322,9 +325,11 @@ rx_burst_8023ad(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts, bufs[j])) || !collecting || (!promisc && - !rte_is_multicast_ether_addr(&hdr->d_addr) && - !rte_is_same_ether_addr(bond_mac, - &hdr->d_addr)))) { + ((rte_is_unicast_ether_addr(&hdr->d_addr) && + !rte_is_same_ether_addr(bond_mac, + &hdr->d_addr)) || + (!allmulti && + rte_is_multicast_ether_addr(&hdr->d_addr)))))) { if (hdr->ether_type == ether_type_slow_be) { bond_mode_8023ad_handle_slow_pkt( @@ -1936,10 +1941,6 @@ bond_ethdev_start(struct rte_eth_dev *eth_dev) } } - /* If bonded device is configure in promiscuous mode then re-apply config */ - if (internals->promiscuous_en) - bond_ethdev_promiscuous_enable(eth_dev); - if (internals->mode == BONDING_MODE_8023AD) { if (internals->mode4.dedicated_queues.enabled == 1) { internals->mode4.dedicated_queues.rx_qid = @@ -2457,18 +2458,17 @@ bond_ethdev_promiscuous_enable(struct rte_eth_dev *eth_dev) struct bond_dev_private *internals = eth_dev->data->dev_private; int i; - internals->promiscuous_en = 1; - switch (internals->mode) { /* Promiscuous mode is propagated to all slaves */ case BONDING_MODE_ROUND_ROBIN: case BONDING_MODE_BALANCE: case BONDING_MODE_BROADCAST: - for (i = 0; i < internals->slave_count; i++) - rte_eth_promiscuous_enable(internals->slaves[i].port_id); - break; - /* In mode4 promiscus mode is managed when slave is added/removed */ case BONDING_MODE_8023AD: + for (i = 0; i < internals->slave_count; i++) { + uint16_t port_id = internals->slaves[i].port_id; + + rte_eth_promiscuous_enable(port_id); + } break; /* Promiscuous mode is propagated only to primary slave */ case BONDING_MODE_ACTIVE_BACKUP: @@ -2488,18 +2488,21 @@ bond_ethdev_promiscuous_disable(struct rte_eth_dev *dev) struct bond_dev_private *internals = dev->data->dev_private; int i; - internals->promiscuous_en = 0; - switch (internals->mode) { /* Promiscuous mode is propagated to all slaves */ case BONDING_MODE_ROUND_ROBIN: case BONDING_MODE_BALANCE: case BONDING_MODE_BROADCAST: - for (i = 0; i < internals->slave_count; i++) - rte_eth_promiscuous_disable(internals->slaves[i].port_id); - break; - /* In mode4 promiscus mode is set managed when slave is added/removed */ case BONDING_MODE_8023AD: + for (i = 0; i < internals->slave_count; i++) { + uint16_t port_id = internals->slaves[i].port_id; + + if (internals->mode == BONDING_MODE_8023AD && + bond_mode_8023ad_ports[port_id].forced_rx_flags == + BOND_8023AD_FORCED_PROMISC) + continue; + rte_eth_promiscuous_disable(port_id); + } break; /* Promiscuous mode is propagated only to primary slave */ case BONDING_MODE_ACTIVE_BACKUP: @@ -2513,6 +2516,70 @@ bond_ethdev_promiscuous_disable(struct rte_eth_dev *dev) } } +static void +bond_ethdev_allmulticast_enable(struct rte_eth_dev *eth_dev) +{ + struct bond_dev_private *internals = eth_dev->data->dev_private; + int i; + + switch (internals->mode) { + /* allmulti mode is propagated to all slaves */ + case BONDING_MODE_ROUND_ROBIN: + case BONDING_MODE_BALANCE: + case BONDING_MODE_BROADCAST: + case BONDING_MODE_8023AD: + for (i = 0; i < internals->slave_count; i++) { + uint16_t port_id = internals->slaves[i].port_id; + + rte_eth_allmulticast_enable(port_id); + } + break; + /* allmulti mode is propagated only to primary slave */ + case BONDING_MODE_ACTIVE_BACKUP: + case BONDING_MODE_TLB: + case BONDING_MODE_ALB: + default: + /* Do not touch allmulti when there cannot be primary ports */ + if (internals->slave_count == 0) + break; + rte_eth_allmulticast_enable(internals->current_primary_port); + } +} + +static void +bond_ethdev_allmulticast_disable(struct rte_eth_dev *eth_dev) +{ + struct bond_dev_private *internals = eth_dev->data->dev_private; + int i; + + switch (internals->mode) { + /* allmulti mode is propagated to all slaves */ + case BONDING_MODE_ROUND_ROBIN: + case BONDING_MODE_BALANCE: + case BONDING_MODE_BROADCAST: + case BONDING_MODE_8023AD: + for (i = 0; i < internals->slave_count; i++) { + uint16_t port_id = internals->slaves[i].port_id; + + if (internals->mode == BONDING_MODE_8023AD && + bond_mode_8023ad_ports[port_id].forced_rx_flags == + BOND_8023AD_FORCED_ALLMULTI) + continue; + rte_eth_allmulticast_disable(port_id); + } + break; + /* allmulti mode is propagated only to primary slave */ + case BONDING_MODE_ACTIVE_BACKUP: + case BONDING_MODE_TLB: + case BONDING_MODE_ALB: + default: + /* Do not touch allmulti when there cannot be primary ports */ + if (internals->slave_count == 0) + break; + rte_eth_allmulticast_disable(internals->current_primary_port); + } +} + static void bond_ethdev_delayed_lsc_propagation(void *arg) { @@ -2907,6 +2974,8 @@ const struct eth_dev_ops default_dev_ops = { .stats_reset = bond_ethdev_stats_reset, .promiscuous_enable = bond_ethdev_promiscuous_enable, .promiscuous_disable = bond_ethdev_promiscuous_disable, + .allmulticast_enable = bond_ethdev_allmulticast_enable, + .allmulticast_disable = bond_ethdev_allmulticast_disable, .reta_update = bond_ethdev_rss_reta_update, .reta_query = bond_ethdev_rss_reta_query, .rss_hash_update = bond_ethdev_rss_hash_update, diff --git a/drivers/net/bonding/rte_eth_bond_private.h b/drivers/net/bonding/rte_eth_bond_private.h index 55c0b227b7..a5a2d7f760 100644 --- a/drivers/net/bonding/rte_eth_bond_private.h +++ b/drivers/net/bonding/rte_eth_bond_private.h @@ -122,9 +122,6 @@ struct bond_dev_private { uint8_t user_defined_mac; /**< Flag for whether MAC address is user defined or not */ - uint8_t promiscuous_en; - /**< Enabled/disable promiscuous mode on bonding device */ - uint8_t link_status_polling_enabled; uint32_t link_status_polling_interval_ms; -- 2.20.1