Print this page
11493 aggr needs support for multiple pseudo rx groups
Portions contributed by: Dan McDonald <danmcd@joyent.com>
Reviewed by: Patrick Mooney <patrick.mooney@joyent.com>
Reviewed by: Jerry Jelinek <jerry.jelinek@joyent.com>
Reviewed by: Robert Mustacchi <rm@joyent.com>

@@ -69,14 +69,14 @@
 static void
 aggr_port_destructor(void *buf, void *arg)
 {
         aggr_port_t *port = buf;
 
-        ASSERT(port->lp_mnh == NULL);
-        ASSERT(port->lp_mphp == NULL);
-        ASSERT(!port->lp_rx_grp_added && !port->lp_tx_grp_added);
-        ASSERT(port->lp_hwgh == NULL);
+        ASSERT3P(port->lp_mnh, ==, NULL);
+        ASSERT(!port->lp_tx_grp_added);
+        for (uint_t i = 0; i < MAX_GROUPS_PER_PORT; i++)
+                ASSERT3P(port->lp_hwghs[i], ==, NULL);
 }
 
 void
 aggr_port_init(void)
 {

@@ -126,11 +126,10 @@
          * (lp_closing is set).
          */
         aggr_grp_port_hold(port);
 }
 
-/* ARGSUSED */
 int
 aggr_port_create(aggr_grp_t *grp, const datalink_id_t linkid, boolean_t force,
     aggr_port_t **pp)
 {
         int err;

@@ -195,22 +194,22 @@
                 err = ENOMEM;
                 goto fail;
         }
 
         /*
-         * As the underlying mac's current margin size is used to determine
+         * As the underlying MAC's current margin size is used to determine
          * the margin size of the aggregation itself, request the underlying
-         * mac not to change to a smaller size.
+         * MAC not to change to a smaller size.
          */
         if ((err = mac_margin_add(mh, &margin, B_TRUE)) != 0) {
                 id_free(aggr_portids, portid);
                 goto fail;
         }
 
         if ((err = mac_unicast_add(mch, NULL, MAC_UNICAST_PRIMARY |
             MAC_UNICAST_DISABLE_TX_VID_CHECK, &mah, 0, &diag)) != 0) {
-                VERIFY(mac_margin_remove(mh, margin) == 0);
+                VERIFY3S(mac_margin_remove(mh, margin), ==, 0);
                 id_free(aggr_portids, portid);
                 goto fail;
         }
 
         port = kmem_cache_alloc(aggr_port_cache, KM_SLEEP);

@@ -261,26 +260,25 @@
         return (0);
 
 fail:
         if (mch != NULL)
                 mac_client_close(mch, MAC_CLOSE_FLAGS_EXCLUSIVE);
+
         mac_close(mh);
         return (err);
 }
 
 void
 aggr_port_delete(aggr_port_t *port)
 {
         aggr_lacp_port_t *pl = &port->lp_lacp;
 
-        ASSERT(port->lp_mphp == NULL);
         ASSERT(!port->lp_promisc_on);
-
         port->lp_closing = B_TRUE;
+        VERIFY0(mac_margin_remove(port->lp_mh, port->lp_margin));
+        mac_client_clear_flow_cb(port->lp_mch);
 
-        VERIFY(mac_margin_remove(port->lp_mh, port->lp_margin) == 0);
-        mac_rx_clear(port->lp_mch);
         /*
          * If the notification callback is already in process and waiting for
          * the aggr grp's mac perimeter, don't wait (otherwise there would be
          * deadlock). Otherwise, if mac_notify_remove() succeeds, we can
          * release the reference held when mac_notify_add() is called.

@@ -307,12 +305,14 @@
          * Restore the port MAC address. Note it is called after the
          * port's notification callback being removed. This prevent
          * port's MAC_NOTE_UNICST notify callback function being called.
          */
         (void) mac_unicast_primary_set(port->lp_mh, port->lp_addr);
+
         if (port->lp_mah != NULL)
                 (void) mac_unicast_remove(port->lp_mch, port->lp_mah);
+
         mac_client_close(port->lp_mch, MAC_CLOSE_FLAGS_EXCLUSIVE);
         mac_close(port->lp_mh);
         AGGR_PORT_REFRELE(port);
 }
 

@@ -519,46 +519,30 @@
 
         /* update the port state */
         port->lp_started = B_FALSE;
 }
 
+/*
+ * Set the promisc mode of the port. If the port is already in the
+ * requested mode then do nothing.
+ */
 int
 aggr_port_promisc(aggr_port_t *port, boolean_t on)
 {
         int rc;
 
         ASSERT(MAC_PERIM_HELD(port->lp_mh));
 
         if (on == port->lp_promisc_on)
-                /* already in desired promiscous mode */
                 return (0);
 
-        if (on) {
-                mac_rx_clear(port->lp_mch);
+        rc = mac_set_promisc(port->lp_mh, on);
 
-                /*
-                 * We use the promisc callback because without hardware
-                 * rings, we deliver through flows that will cause duplicate
-                 * delivery of packets when we've flipped into this mode
-                 * to compensate for the lack of hardware MAC matching
-                 */
-                rc = mac_promisc_add(port->lp_mch, MAC_CLIENT_PROMISC_ALL,
-                    aggr_recv_promisc_cb, port, &port->lp_mphp,
-                    MAC_PROMISC_FLAGS_NO_TX_LOOP);
-                if (rc != 0) {
-                        mac_rx_set(port->lp_mch, aggr_recv_cb, port);
-                        return (rc);
-                }
-        } else {
-                mac_promisc_remove(port->lp_mphp);
-                port->lp_mphp = NULL;
-                mac_rx_set(port->lp_mch, aggr_recv_cb, port);
-        }
-
+        if (rc == 0)
         port->lp_promisc_on = on;
 
-        return (0);
+        return (rc);
 }
 
 /*
  * Set the MAC address of a port.
  */

@@ -594,39 +578,49 @@
 {
         return (mac_stat_get(port->lp_mh, stat));
 }
 
 /*
- * Add a non-primary unicast address to the underlying port. If the port
- * supports HW Rx group, try to add the address into the HW Rx group of
- * the port first. If that fails, or if the port does not support HW Rx
- * group, enable the port's promiscous mode.
+ * Add a non-primary unicast address to the underlying port. If the
+ * port supports HW Rx groups, then try to add the address filter to
+ * the HW group first. If that fails, or if the port does not support
+ * RINGS capab, then enable the port's promiscous mode.
  */
 int
-aggr_port_addmac(aggr_port_t *port, const uint8_t *mac_addr)
+aggr_port_addmac(aggr_port_t *port, uint_t idx, const uint8_t *mac_addr)
 {
         aggr_unicst_addr_t      *addr, **pprev;
         mac_perim_handle_t      pmph;
         int                     err;
 
         ASSERT(MAC_PERIM_HELD(port->lp_grp->lg_mh));
+        ASSERT3U(idx, <, MAX_GROUPS_PER_PORT);
         mac_perim_enter_by_mh(port->lp_mh, &pmph);
 
         /*
-         * If the underlying port support HW Rx group, add the mac to its
-         * RX group directly.
+         * If the port doesn't have a HW group to back the aggr's
+         * pseudo group, then try using the port's default group and
+         * let the aggr SW classify its traffic. This scenario happens
+         * when mixing ports with a different number of HW groups.
          */
-        if ((port->lp_hwgh != NULL) &&
-            ((mac_hwgroup_addmac(port->lp_hwgh, mac_addr)) == 0)) {
+        if (port->lp_hwghs[idx] == NULL)
+                idx = 0;
+
+        /*
+         * If there is an underlying HW Rx group, then try adding this
+         * unicast address to it.
+         */
+        if ((port->lp_hwghs[idx] != NULL) &&
+            ((mac_hwgroup_addmac(port->lp_hwghs[idx], mac_addr)) == 0)) {
                 mac_perim_exit(pmph);
                 return (0);
         }
 
         /*
-         * If that fails, or if the port does not support HW Rx group, enable
-         * the port's promiscous mode. (Note that we turn on the promiscous
-         * mode only if the port is already started.
+         * If the port doesn't have HW groups, or we failed to add the
+         * HW filter, then enable the port's promiscuous mode. We
+         * enable promiscuous mode only if the port is already started.
          */
         if (port->lp_started &&
             ((err = aggr_port_promisc(port, B_TRUE)) != 0)) {
                 mac_perim_exit(pmph);
                 return (err);

@@ -654,17 +648,18 @@
  * must has been added by aggr_port_addmac(). As a result, we probably need to
  * remove the address from the port's HW Rx group, or to disable the port's
  * promiscous mode.
  */
 void
-aggr_port_remmac(aggr_port_t *port, const uint8_t *mac_addr)
+aggr_port_remmac(aggr_port_t *port, uint_t idx, const uint8_t *mac_addr)
 {
         aggr_grp_t              *grp = port->lp_grp;
         aggr_unicst_addr_t      *addr, **pprev;
         mac_perim_handle_t      pmph;
 
         ASSERT(MAC_PERIM_HELD(grp->lg_mh));
+        ASSERT3U(idx, <, MAX_GROUPS_PER_PORT);
         mac_perim_enter_by_mh(port->lp_mh, &pmph);
 
         /*
          * See whether this address is in the list of addresses that requires
          * the port being promiscous mode.

@@ -673,10 +668,11 @@
         while ((addr = *pprev) != NULL) {
                 if (bcmp(mac_addr, addr->aua_addr, ETHERADDRL) == 0)
                         break;
                 pprev = &addr->aua_next;
         }
+
         if (addr != NULL) {
                 /*
                  * This unicast address put the port into the promiscous mode,
                  * delete this address from the lp_prom_addr list. If this is
                  * the last address in that list, disable the promiscous mode

@@ -685,50 +681,67 @@
                 *pprev = addr->aua_next;
                 kmem_free(addr, sizeof (aggr_unicst_addr_t));
                 if (port->lp_prom_addr == NULL && !grp->lg_promisc)
                         (void) aggr_port_promisc(port, B_FALSE);
         } else {
-                ASSERT(port->lp_hwgh != NULL);
-                (void) mac_hwgroup_remmac(port->lp_hwgh, mac_addr);
+                /* See comment in aggr_port_addmac(). */
+                if (port->lp_hwghs[idx] == NULL)
+                        idx = 0;
+
+                ASSERT3P(port->lp_hwghs[idx], !=, NULL);
+                (void) mac_hwgroup_remmac(port->lp_hwghs[idx], mac_addr);
         }
+
         mac_perim_exit(pmph);
 }
 
 int
-aggr_port_addvlan(aggr_port_t *port, uint16_t vid)
+aggr_port_addvlan(aggr_port_t *port, uint_t idx, uint16_t vid)
 {
         mac_perim_handle_t      pmph;
         int                     err;
 
         ASSERT(MAC_PERIM_HELD(port->lp_grp->lg_mh));
+        ASSERT3U(idx, <, MAX_GROUPS_PER_PORT);
         mac_perim_enter_by_mh(port->lp_mh, &pmph);
 
+        /* See comment in aggr_port_addmac(). */
+        if (port->lp_hwghs[idx] == NULL)
+                idx = 0;
+
         /*
          * Add the VLAN filter to the HW group if the port has a HW
          * group. If the port doesn't have a HW group, then it will
          * implicitly allow tagged traffic to pass and there is
          * nothing to do.
          */
-        if (port->lp_hwgh == NULL)
-                return (0);
+        if (port->lp_hwghs[idx] == NULL)
+                err = 0;
+        else
+                err = mac_hwgroup_addvlan(port->lp_hwghs[idx], vid);
 
-        err = mac_hwgroup_addvlan(port->lp_hwgh, vid);
         mac_perim_exit(pmph);
         return (err);
 }
 
 int
-aggr_port_remvlan(aggr_port_t *port, uint16_t vid)
+aggr_port_remvlan(aggr_port_t *port, uint_t idx, uint16_t vid)
 {
         mac_perim_handle_t      pmph;
         int                     err;
 
         ASSERT(MAC_PERIM_HELD(port->lp_grp->lg_mh));
+        ASSERT3U(idx, <, MAX_GROUPS_PER_PORT);
         mac_perim_enter_by_mh(port->lp_mh, &pmph);
 
-        if (port->lp_hwgh == NULL)
-                return (0);
+        /* See comment in aggr_port_addmac(). */
+        if (port->lp_hwghs[idx] == NULL)
+                idx = 0;
 
-        err = mac_hwgroup_remvlan(port->lp_hwgh, vid);
+        if (port->lp_hwghs[idx] == NULL)
+                err = 0;
+        else
+                err = mac_hwgroup_remvlan(port->lp_hwghs[idx], vid);
+
         mac_perim_exit(pmph);
         return (err);
 }