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 <david.marchand@redhat.com>
Acked-by: Chas Williams <chas3@att.com>

diff --git a/drivers/net/bonding/rte_eth_bond_8023ad.c b/drivers/net/bonding/rte_eth_bond_8023ad.c
index d764dad..fbc6905 100644 (file)
--- 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 d905de4..1470bf7 100644 (file)
--- 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 49e38f6..97ab3f2 100644 (file)
--- 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 55c0b22..a5a2d7f 100644 (file)
--- 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;