net/bonding: fix adding itself as its slave
[dpdk.git] / drivers / net / bonding / rte_eth_bond_api.c
index 49fa2d7..eb8d15d 100644 (file)
@@ -6,20 +6,23 @@
 
 #include <rte_mbuf.h>
 #include <rte_malloc.h>
-#include <rte_ethdev_driver.h>
+#include <ethdev_driver.h>
 #include <rte_tcp.h>
 #include <rte_bus_vdev.h>
 #include <rte_kvargs.h>
 
 #include "rte_eth_bond.h"
-#include "rte_eth_bond_private.h"
-#include "rte_eth_bond_8023ad_private.h"
+#include "eth_bond_private.h"
+#include "eth_bond_8023ad_private.h"
 
 int
 check_for_bonded_ethdev(const struct rte_eth_dev *eth_dev)
 {
        /* Check valid pointer */
-       if (eth_dev->device->driver->name == NULL)
+       if (eth_dev == NULL ||
+               eth_dev->device == NULL ||
+               eth_dev->device->driver == NULL ||
+               eth_dev->device->driver->name == NULL)
                return -1;
 
        /* return 0 if driver name matches */
@@ -53,19 +56,25 @@ check_for_master_bonded_ethdev(const struct rte_eth_dev *eth_dev)
 }
 
 int
-valid_slave_port_id(uint16_t port_id, uint8_t mode)
+valid_slave_port_id(struct bond_dev_private *internals, uint16_t slave_port_id)
 {
-       RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -1);
+       RTE_ETH_VALID_PORTID_OR_ERR_RET(slave_port_id, -1);
 
-       /* Verify that port_id refers to a non bonded port */
-       if (check_for_bonded_ethdev(&rte_eth_devices[port_id]) == 0 &&
-                       mode == BONDING_MODE_8023AD) {
+       /* Verify that slave_port_id refers to a non bonded port */
+       if (check_for_bonded_ethdev(&rte_eth_devices[slave_port_id]) == 0 &&
+                       internals->mode == BONDING_MODE_8023AD) {
                RTE_BOND_LOG(ERR, "Cannot add slave to bonded device in 802.3ad"
                                " mode as slave is also a bonded device, only "
                                "physical devices can be support in this mode.");
                return -1;
        }
 
+       if (internals->port_id == slave_port_id) {
+               RTE_BOND_LOG(ERR,
+                       "Cannot add the bonded device itself as its slave.");
+               return -1;
+       }
+
        return 0;
 }
 
@@ -73,7 +82,7 @@ void
 activate_slave(struct rte_eth_dev *eth_dev, uint16_t port_id)
 {
        struct bond_dev_private *internals = eth_dev->data->dev_private;
-       uint8_t active_count = internals->active_slave_count;
+       uint16_t active_count = internals->active_slave_count;
 
        if (internals->mode == BONDING_MODE_8023AD)
                bond_mode_8023ad_activate_slave(eth_dev, port_id);
@@ -158,7 +167,7 @@ rte_eth_bond_create(const char *name, uint8_t mode, uint8_t socket_id)
 
        ret = rte_vdev_init(name, devargs);
        if (ret)
-               return -ENOMEM;
+               return ret;
 
        ret = rte_eth_dev_get_port_by_name(name, &port_id);
        RTE_ASSERT(!ret);
@@ -234,7 +243,12 @@ slave_rte_flow_prepare(uint16_t slave_id, struct bond_dev_private *internals)
        uint16_t slave_port_id = internals->slaves[slave_id].port_id;
 
        if (internals->flow_isolated_valid != 0) {
-               rte_eth_dev_stop(slave_port_id);
+               if (rte_eth_dev_stop(slave_port_id) != 0) {
+                       RTE_BOND_LOG(ERR, "Failed to stop device on port %u",
+                                    slave_port_id);
+                       return -1;
+               }
+
                if (rte_flow_isolate(slave_port_id, internals->flow_isolated,
                    &ferror)) {
                        RTE_BOND_LOG(ERR, "rte_flow_isolate failed for slave"
@@ -245,9 +259,9 @@ slave_rte_flow_prepare(uint16_t slave_id, struct bond_dev_private *internals)
        }
        TAILQ_FOREACH(flow, &internals->flow_list, next) {
                flow->flows[slave_id] = rte_flow_create(slave_port_id,
-                                                       &flow->fd->attr,
-                                                       flow->fd->items,
-                                                       flow->fd->actions,
+                                                       flow->rule.attr,
+                                                       flow->rule.pattern,
+                                                       flow->rule.actions,
                                                        &ferror);
                if (flow->flows[slave_id] == NULL) {
                        RTE_BOND_LOG(ERR, "Cannot create flow for slave"
@@ -269,6 +283,173 @@ slave_rte_flow_prepare(uint16_t slave_id, struct bond_dev_private *internals)
        return 0;
 }
 
+static void
+eth_bond_slave_inherit_dev_info_rx_first(struct bond_dev_private *internals,
+                                        const struct rte_eth_dev_info *di)
+{
+       struct rte_eth_rxconf *rxconf_i = &internals->default_rxconf;
+
+       internals->reta_size = di->reta_size;
+
+       /* Inherit Rx offload capabilities from the first slave device */
+       internals->rx_offload_capa = di->rx_offload_capa;
+       internals->rx_queue_offload_capa = di->rx_queue_offload_capa;
+       internals->flow_type_rss_offloads = di->flow_type_rss_offloads;
+
+       /* Inherit maximum Rx packet size from the first slave device */
+       internals->candidate_max_rx_pktlen = di->max_rx_pktlen;
+
+       /* Inherit default Rx queue settings from the first slave device */
+       memcpy(rxconf_i, &di->default_rxconf, sizeof(*rxconf_i));
+
+       /*
+        * Turn off descriptor prefetch and writeback by default for all
+        * slave devices. Applications may tweak this setting if need be.
+        */
+       rxconf_i->rx_thresh.pthresh = 0;
+       rxconf_i->rx_thresh.hthresh = 0;
+       rxconf_i->rx_thresh.wthresh = 0;
+
+       /* Setting this to zero should effectively enable default values */
+       rxconf_i->rx_free_thresh = 0;
+
+       /* Disable deferred start by default for all slave devices */
+       rxconf_i->rx_deferred_start = 0;
+}
+
+static void
+eth_bond_slave_inherit_dev_info_tx_first(struct bond_dev_private *internals,
+                                        const struct rte_eth_dev_info *di)
+{
+       struct rte_eth_txconf *txconf_i = &internals->default_txconf;
+
+       /* Inherit Tx offload capabilities from the first slave device */
+       internals->tx_offload_capa = di->tx_offload_capa;
+       internals->tx_queue_offload_capa = di->tx_queue_offload_capa;
+
+       /* Inherit default Tx queue settings from the first slave device */
+       memcpy(txconf_i, &di->default_txconf, sizeof(*txconf_i));
+
+       /*
+        * Turn off descriptor prefetch and writeback by default for all
+        * slave devices. Applications may tweak this setting if need be.
+        */
+       txconf_i->tx_thresh.pthresh = 0;
+       txconf_i->tx_thresh.hthresh = 0;
+       txconf_i->tx_thresh.wthresh = 0;
+
+       /*
+        * Setting these parameters to zero assumes that default
+        * values will be configured implicitly by slave devices.
+        */
+       txconf_i->tx_free_thresh = 0;
+       txconf_i->tx_rs_thresh = 0;
+
+       /* Disable deferred start by default for all slave devices */
+       txconf_i->tx_deferred_start = 0;
+}
+
+static void
+eth_bond_slave_inherit_dev_info_rx_next(struct bond_dev_private *internals,
+                                       const struct rte_eth_dev_info *di)
+{
+       struct rte_eth_rxconf *rxconf_i = &internals->default_rxconf;
+       const struct rte_eth_rxconf *rxconf = &di->default_rxconf;
+
+       internals->rx_offload_capa &= di->rx_offload_capa;
+       internals->rx_queue_offload_capa &= di->rx_queue_offload_capa;
+       internals->flow_type_rss_offloads &= di->flow_type_rss_offloads;
+
+       /*
+        * If at least one slave device suggests enabling this
+        * setting by default, enable it for all slave devices
+        * since disabling it may not be necessarily supported.
+        */
+       if (rxconf->rx_drop_en == 1)
+               rxconf_i->rx_drop_en = 1;
+
+       /*
+        * Adding a new slave device may cause some of previously inherited
+        * offloads to be withdrawn from the internal rx_queue_offload_capa
+        * value. Thus, the new internal value of default Rx queue offloads
+        * has to be masked by rx_queue_offload_capa to make sure that only
+        * commonly supported offloads are preserved from both the previous
+        * value and the value being inhereted from the new slave device.
+        */
+       rxconf_i->offloads = (rxconf_i->offloads | rxconf->offloads) &
+                            internals->rx_queue_offload_capa;
+
+       /*
+        * RETA size is GCD of all slaves RETA sizes, so, if all sizes will be
+        * the power of 2, the lower one is GCD
+        */
+       if (internals->reta_size > di->reta_size)
+               internals->reta_size = di->reta_size;
+
+       if (!internals->max_rx_pktlen &&
+           di->max_rx_pktlen < internals->candidate_max_rx_pktlen)
+               internals->candidate_max_rx_pktlen = di->max_rx_pktlen;
+}
+
+static void
+eth_bond_slave_inherit_dev_info_tx_next(struct bond_dev_private *internals,
+                                       const struct rte_eth_dev_info *di)
+{
+       struct rte_eth_txconf *txconf_i = &internals->default_txconf;
+       const struct rte_eth_txconf *txconf = &di->default_txconf;
+
+       internals->tx_offload_capa &= di->tx_offload_capa;
+       internals->tx_queue_offload_capa &= di->tx_queue_offload_capa;
+
+       /*
+        * Adding a new slave device may cause some of previously inherited
+        * offloads to be withdrawn from the internal tx_queue_offload_capa
+        * value. Thus, the new internal value of default Tx queue offloads
+        * has to be masked by tx_queue_offload_capa to make sure that only
+        * commonly supported offloads are preserved from both the previous
+        * value and the value being inhereted from the new slave device.
+        */
+       txconf_i->offloads = (txconf_i->offloads | txconf->offloads) &
+                            internals->tx_queue_offload_capa;
+}
+
+static void
+eth_bond_slave_inherit_desc_lim_first(struct rte_eth_desc_lim *bond_desc_lim,
+               const struct rte_eth_desc_lim *slave_desc_lim)
+{
+       memcpy(bond_desc_lim, slave_desc_lim, sizeof(*bond_desc_lim));
+}
+
+static int
+eth_bond_slave_inherit_desc_lim_next(struct rte_eth_desc_lim *bond_desc_lim,
+               const struct rte_eth_desc_lim *slave_desc_lim)
+{
+       bond_desc_lim->nb_max = RTE_MIN(bond_desc_lim->nb_max,
+                                       slave_desc_lim->nb_max);
+       bond_desc_lim->nb_min = RTE_MAX(bond_desc_lim->nb_min,
+                                       slave_desc_lim->nb_min);
+       bond_desc_lim->nb_align = RTE_MAX(bond_desc_lim->nb_align,
+                                         slave_desc_lim->nb_align);
+
+       if (bond_desc_lim->nb_min > bond_desc_lim->nb_max ||
+           bond_desc_lim->nb_align > bond_desc_lim->nb_max) {
+               RTE_BOND_LOG(ERR, "Failed to inherit descriptor limits");
+               return -EINVAL;
+       }
+
+       /* Treat maximum number of segments equal to 0 as unspecified */
+       if (slave_desc_lim->nb_seg_max != 0 &&
+           (bond_desc_lim->nb_seg_max == 0 ||
+            slave_desc_lim->nb_seg_max < bond_desc_lim->nb_seg_max))
+               bond_desc_lim->nb_seg_max = slave_desc_lim->nb_seg_max;
+       if (slave_desc_lim->nb_mtu_seg_max != 0 &&
+           (bond_desc_lim->nb_mtu_seg_max == 0 ||
+            slave_desc_lim->nb_mtu_seg_max < bond_desc_lim->nb_mtu_seg_max))
+               bond_desc_lim->nb_mtu_seg_max = slave_desc_lim->nb_mtu_seg_max;
+
+       return 0;
+}
+
 static int
 __eth_bond_slave_add_lock_free(uint16_t bonded_port_id, uint16_t slave_port_id)
 {
@@ -276,11 +457,12 @@ __eth_bond_slave_add_lock_free(uint16_t bonded_port_id, uint16_t slave_port_id)
        struct bond_dev_private *internals;
        struct rte_eth_link link_props;
        struct rte_eth_dev_info dev_info;
+       int ret;
 
        bonded_eth_dev = &rte_eth_devices[bonded_port_id];
        internals = bonded_eth_dev->data->dev_private;
 
-       if (valid_slave_port_id(slave_port_id, internals->mode) != 0)
+       if (valid_slave_port_id(internals, slave_port_id) != 0)
                return -1;
 
        slave_eth_dev = &rte_eth_devices[slave_port_id];
@@ -289,7 +471,14 @@ __eth_bond_slave_add_lock_free(uint16_t bonded_port_id, uint16_t slave_port_id)
                return -1;
        }
 
-       rte_eth_dev_info_get(slave_port_id, &dev_info);
+       ret = rte_eth_dev_info_get(slave_port_id, &dev_info);
+       if (ret != 0) {
+               RTE_BOND_LOG(ERR,
+                       "%s: Error during getting device (port %u) info: %s\n",
+                       __func__, slave_port_id, strerror(-ret));
+
+               return ret;
+       }
        if (dev_info.max_rx_pktlen < internals->max_rx_pktlen) {
                RTE_BOND_LOG(ERR, "Slave (port %u) max_rx_pktlen too small",
                             slave_port_id);
@@ -314,10 +503,6 @@ __eth_bond_slave_add_lock_free(uint16_t bonded_port_id, uint16_t slave_port_id)
                        }
                }
 
-               /* Inherit eth dev link properties from first slave */
-               link_properties_set(bonded_eth_dev,
-                               &(slave_eth_dev->data->dev_link));
-
                /* Make primary slave */
                internals->primary_port = slave_port_id;
                internals->current_primary_port = slave_port_id;
@@ -326,42 +511,28 @@ __eth_bond_slave_add_lock_free(uint16_t bonded_port_id, uint16_t slave_port_id)
                internals->nb_rx_queues = slave_eth_dev->data->nb_rx_queues;
                internals->nb_tx_queues = slave_eth_dev->data->nb_tx_queues;
 
-               internals->reta_size = dev_info.reta_size;
-
-               /* Take the first dev's offload capabilities */
-               internals->rx_offload_capa = dev_info.rx_offload_capa;
-               internals->tx_offload_capa = dev_info.tx_offload_capa;
-               internals->rx_queue_offload_capa = dev_info.rx_queue_offload_capa;
-               internals->tx_queue_offload_capa = dev_info.tx_queue_offload_capa;
-               internals->flow_type_rss_offloads = dev_info.flow_type_rss_offloads;
-
-               /* Inherit first slave's max rx packet size */
-               internals->candidate_max_rx_pktlen = dev_info.max_rx_pktlen;
+               eth_bond_slave_inherit_dev_info_rx_first(internals, &dev_info);
+               eth_bond_slave_inherit_dev_info_tx_first(internals, &dev_info);
 
+               eth_bond_slave_inherit_desc_lim_first(&internals->rx_desc_lim,
+                                                     &dev_info.rx_desc_lim);
+               eth_bond_slave_inherit_desc_lim_first(&internals->tx_desc_lim,
+                                                     &dev_info.tx_desc_lim);
        } else {
-               internals->rx_offload_capa &= dev_info.rx_offload_capa;
-               internals->tx_offload_capa &= dev_info.tx_offload_capa;
-               internals->rx_queue_offload_capa &= dev_info.rx_queue_offload_capa;
-               internals->tx_queue_offload_capa &= dev_info.tx_queue_offload_capa;
-               internals->flow_type_rss_offloads &= dev_info.flow_type_rss_offloads;
-
-               if (link_properties_valid(bonded_eth_dev,
-                               &slave_eth_dev->data->dev_link) != 0) {
-                       RTE_BOND_LOG(ERR, "Invalid link properties for slave %d"
-                                       " in bonding mode %d", slave_port_id,
-                                       internals->mode);
-                       return -1;
-               }
+               int ret;
 
-               /* RETA size is GCD of all slaves RETA sizes, so, if all sizes will be
-                * the power of 2, the lower one is GCD
-                */
-               if (internals->reta_size > dev_info.reta_size)
-                       internals->reta_size = dev_info.reta_size;
+               eth_bond_slave_inherit_dev_info_rx_next(internals, &dev_info);
+               eth_bond_slave_inherit_dev_info_tx_next(internals, &dev_info);
 
-               if (!internals->max_rx_pktlen &&
-                   dev_info.max_rx_pktlen < internals->candidate_max_rx_pktlen)
-                       internals->candidate_max_rx_pktlen = dev_info.max_rx_pktlen;
+               ret = eth_bond_slave_inherit_desc_lim_next(
+                               &internals->rx_desc_lim, &dev_info.rx_desc_lim);
+               if (ret != 0)
+                       return ret;
+
+               ret = eth_bond_slave_inherit_desc_lim_next(
+                               &internals->tx_desc_lim, &dev_info.tx_desc_lim);
+               if (ret != 0)
+                       return ret;
        }
 
        bonded_eth_dev->data->dev_conf.rx_adv_conf.rss_conf.rss_hf &=
@@ -391,9 +562,6 @@ __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 */
        mac_address_slaves_update(bonded_eth_dev);
 
@@ -405,21 +573,30 @@ __eth_bond_slave_add_lock_free(uint16_t bonded_port_id, uint16_t slave_port_id)
        /* If bonded device is started then we can add the slave to our active
         * slave array */
        if (bonded_eth_dev->data->dev_started) {
-               rte_eth_link_get_nowait(slave_port_id, &link_props);
+               ret = rte_eth_link_get_nowait(slave_port_id, &link_props);
+               if (ret < 0) {
+                       rte_eth_dev_callback_unregister(slave_port_id,
+                                       RTE_ETH_EVENT_INTR_LSC,
+                                       bond_ethdev_lsc_event_callback,
+                                       &bonded_eth_dev->data->port_id);
+                       internals->slave_count--;
+                       RTE_BOND_LOG(ERR,
+                               "Slave (port %u) link get failed: %s\n",
+                               slave_port_id, rte_strerror(-ret));
+                       return -1;
+               }
 
                 if (link_props.link_status == ETH_LINK_UP) {
                        if (internals->active_slave_count == 0 &&
                            !internals->user_defined_primary_port)
                                bond_ethdev_primary_set(internals,
                                                        slave_port_id);
-
-                       if (find_slave_by_id(internals->active_slaves,
-                                            internals->active_slave_count,
-                                            slave_port_id) == internals->active_slave_count)
-                               activate_slave(bonded_eth_dev, slave_port_id);
                }
        }
 
+       /* Add slave details to bonded device */
+       slave_eth_dev->data->dev_flags |= RTE_ETH_DEV_BONDED_SLAVE;
+
        slave_vlan_filter_set(bonded_port_id, slave_port_id);
 
        return 0;
@@ -434,13 +611,15 @@ rte_eth_bond_slave_add(uint16_t bonded_port_id, uint16_t slave_port_id)
 
        int retval;
 
-       /* Verify that port id's are valid bonded and slave ports */
        if (valid_bonded_port_id(bonded_port_id) != 0)
                return -1;
 
        bonded_eth_dev = &rte_eth_devices[bonded_port_id];
        internals = bonded_eth_dev->data->dev_private;
 
+       if (valid_slave_port_id(internals, slave_port_id) != 0)
+               return -1;
+
        rte_spinlock_lock(&internals->lock);
 
        retval = __eth_bond_slave_add_lock_free(bonded_port_id, slave_port_id);
@@ -464,7 +643,7 @@ __eth_bond_slave_remove_lock_free(uint16_t bonded_port_id,
        bonded_eth_dev = &rte_eth_devices[bonded_port_id];
        internals = bonded_eth_dev->data->dev_private;
 
-       if (valid_slave_port_id(slave_port_id, internals->mode) < 0)
+       if (valid_slave_port_id(internals, slave_port_id) < 0)
                return -1;
 
        /* first remove from active slave list */
@@ -526,6 +705,7 @@ __eth_bond_slave_remove_lock_free(uint16_t bonded_port_id,
                        internals->current_primary_port = internals->slaves[0].port_id;
                else
                        internals->primary_port = 0;
+               mac_address_slaves_update(bonded_eth_dev);
        }
 
        if (internals->active_slave_count < 1) {
@@ -611,7 +791,7 @@ rte_eth_bond_primary_set(uint16_t bonded_port_id, uint16_t slave_port_id)
 
        internals = rte_eth_devices[bonded_port_id].data->dev_private;
 
-       if (valid_slave_port_id(slave_port_id, internals->mode) != 0)
+       if (valid_slave_port_id(internals, slave_port_id) != 0)
                return -1;
 
        internals->user_defined_primary_port = 1;
@@ -643,7 +823,7 @@ rte_eth_bond_slaves_get(uint16_t bonded_port_id, uint16_t slaves[],
                        uint16_t len)
 {
        struct bond_dev_private *internals;
-       uint8_t i;
+       uint16_t i;
 
        if (valid_bonded_port_id(bonded_port_id) != 0)
                return -1;
@@ -687,7 +867,7 @@ rte_eth_bond_active_slaves_get(uint16_t bonded_port_id, uint16_t slaves[],
 
 int
 rte_eth_bond_mac_address_set(uint16_t bonded_port_id,
-               struct ether_addr *mac_addr)
+               struct rte_ether_addr *mac_addr)
 {
        struct rte_eth_dev *bonded_eth_dev;
        struct bond_dev_private *internals;