From patchwork Tue Aug 1 16:54:00 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adrien Mazarguil X-Patchwork-Id: 27325 Return-Path: X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id 38BB9A12A; Tue, 1 Aug 2017 18:55:28 +0200 (CEST) Received: from mail-wm0-f50.google.com (mail-wm0-f50.google.com [74.125.82.50]) by dpdk.org (Postfix) with ESMTP id 4A18CA0C0 for ; Tue, 1 Aug 2017 18:55:09 +0200 (CEST) Received: by mail-wm0-f50.google.com with SMTP id t201so20374237wmt.0 for ; Tue, 01 Aug 2017 09:55:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=from:to:subject:date:message-id:in-reply-to:references; bh=j3VEYiFsIAR6up3bjttygxYNByGItUJTsHnsplRK6l4=; b=cfAc7CBfeAPBlxuA9iRI1qm0cnvwz4OdXs+TYO0B/SC28G5wJhMK6aIdM70EVn85xi nfwi+mgj63RpDI5najiXKgWq8uHUlqeVbh/MzRtB1fqZIQNdRkG2mHe4f3lDZSpCxLlz fA/30t/Jk4zRYCCndj5qs+H4kt7zAvT49uYF/gUMzUj0JWqCeCjHqFraa1YwA/nmZrB0 s0oOpq39q3VK7Sb+wLIE2AgzSbQFpcRLr7OHK13rY4X6S/SZyd6wSdAEuP7lr4l4Rbd6 FStWqVtmVMz8FOwq0ELClkq0oQ/G3VvCMrHuYfrvltvcERZtTjnkYAQNwp5AXngN2FjR Ar1Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references; bh=j3VEYiFsIAR6up3bjttygxYNByGItUJTsHnsplRK6l4=; b=BwzwV9wDHKLWBJ5AFnUKomy274qzvn9BhjX5IHXeX+hEcZaj/nHj8ts4jstU+WxO55 HA1/AnO1XDwct9OzG0csTtFsl+JkoYbn5fOPSJCgCLxd47J3+AR/iE0JCX6KuErFt6mY HVeNs+NpxZ3yc4StAJO9zdlADKg7DOb/lQmX2TSmiApJKbuMtEA1+tiqAWn+63vhQnXb GkjcpcTO4Nok5HkDo5oo8uP6c0NuVV2vik7RvE70KIPCPoHiPRbouPEA4xHWD/RR5dt6 uahRwgbDTvAnYRLzhqve33F0LBCSZDBvE0oKt+LpErYGk+v6Ux/D3BDH41WCCuIX2nMQ 9FFQ== X-Gm-Message-State: AIVw113iepwhUwZO7wPbupmZA/R6qUooc1l9teVYdb4HJiZ5E3DHj4ez mGHDgZDu4yCver4ktJQ= X-Received: by 10.28.131.193 with SMTP id f184mr1992249wmd.117.1501606508535; Tue, 01 Aug 2017 09:55:08 -0700 (PDT) Received: from 6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id p72sm16921464wmb.1.2017.08.01.09.55.07 for (version=TLS1_2 cipher=AES128-SHA bits=128/128); Tue, 01 Aug 2017 09:55:07 -0700 (PDT) From: Adrien Mazarguil To: dev@dpdk.org Date: Tue, 1 Aug 2017 18:54:00 +0200 Message-Id: <0ce4226fc8542df33039a93b04a6638d8f44d921.1501598384.git.adrien.mazarguil@6wind.com> X-Mailer: git-send-email 2.1.4 In-Reply-To: References: Subject: [dpdk-dev] [PATCH v1 13/48] net/mlx4: drop MAC flows affecting all Rx queues X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Configuring several Rx queues enables RSS, which causes an additional special parent queue to be created to manage them. MAC flows are associated with the queue supposed to receive packets; either the parent one in case of RSS or the single orphan otherwise. For historical reasons the current implementation supports another scenario with multiple orphans, in which case MAC flows are configured on all of them. This is harmless but useless since it cannot happen. Removing this feature allows dissociating the remaining MAC flow from Rx queues and store it inside the private structure where it belongs. Signed-off-by: Adrien Mazarguil --- drivers/net/mlx4/mlx4.c | 215 +++++++++++-------------------------------- drivers/net/mlx4/mlx4.h | 2 +- 2 files changed, 57 insertions(+), 160 deletions(-) diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c index dd42c96..c11e789 100644 --- a/drivers/net/mlx4/mlx4.c +++ b/drivers/net/mlx4/mlx4.c @@ -515,6 +515,9 @@ rxq_setup(struct rte_eth_dev *dev, struct rxq *rxq, uint16_t desc, static void rxq_cleanup(struct rxq *rxq); +static void +priv_mac_addr_del(struct priv *priv); + /** * Create RSS parent queue. * @@ -641,6 +644,7 @@ dev_configure(struct rte_eth_dev *dev) for (i = 0; (i != priv->rxqs_n); ++i) if ((*priv->rxqs)[i] != NULL) return EINVAL; + priv_mac_addr_del(priv); priv_parent_list_cleanup(priv); priv->rss = 0; priv->rxqs_n = 0; @@ -2065,46 +2069,57 @@ rxq_free_elts(struct rxq *rxq) } /** - * Unregister a MAC address from a RX queue. + * Unregister a MAC address. * - * @param rxq - * Pointer to RX queue structure. + * @param priv + * Pointer to private structure. */ static void -rxq_mac_addr_del(struct rxq *rxq) +priv_mac_addr_del(struct priv *priv) { #ifndef NDEBUG - struct priv *priv = rxq->priv; - const uint8_t (*mac)[ETHER_ADDR_LEN] = - (const uint8_t (*)[ETHER_ADDR_LEN]) - priv->mac.addr_bytes; + uint8_t (*mac)[ETHER_ADDR_LEN] = &priv->mac.addr_bytes; #endif - if (!rxq->mac_flow) + + if (!priv->mac_flow) return; DEBUG("%p: removing MAC address %02x:%02x:%02x:%02x:%02x:%02x", - (void *)rxq, + (void *)priv, (*mac)[0], (*mac)[1], (*mac)[2], (*mac)[3], (*mac)[4], (*mac)[5]); - claim_zero(ibv_destroy_flow(rxq->mac_flow)); - rxq->mac_flow = NULL; + claim_zero(ibv_destroy_flow(priv->mac_flow)); + priv->mac_flow = NULL; } /** - * Register a MAC address in a RX queue. + * Register a MAC address. * - * @param rxq - * Pointer to RX queue structure. + * In RSS mode, the MAC address is registered in the parent queue, + * otherwise it is registered in queue 0. + * + * @param priv + * Pointer to private structure. * * @return * 0 on success, errno value on failure. */ static int -rxq_mac_addr_add(struct rxq *rxq) +priv_mac_addr_add(struct priv *priv) { + uint8_t (*mac)[ETHER_ADDR_LEN] = &priv->mac.addr_bytes; + struct rxq *rxq; struct ibv_flow *flow; - struct priv *priv = rxq->priv; - const uint8_t (*mac)[ETHER_ADDR_LEN] = - (const uint8_t (*)[ETHER_ADDR_LEN]) - priv->mac.addr_bytes; + + /* If device isn't started, this is all we need to do. */ + if (!priv->started) + return 0; + if (priv->isolated) + return 0; + if (priv->rss) + rxq = LIST_FIRST(&priv->parents); + else if (*priv->rxqs && (*priv->rxqs)[0]) + rxq = (*priv->rxqs)[0]; + else + return 0; /* Allocate flow specification on the stack. */ struct __attribute__((packed)) { @@ -2114,8 +2129,8 @@ rxq_mac_addr_add(struct rxq *rxq) struct ibv_flow_attr *attr = &data.attr; struct ibv_flow_spec_eth *spec = &data.spec; - if (rxq->mac_flow) - rxq_mac_addr_del(rxq); + if (priv->mac_flow) + priv_mac_addr_del(priv); /* * No padding must be inserted by the compiler between attr and spec. * This layout is expected by libibverbs. @@ -2142,7 +2157,7 @@ rxq_mac_addr_add(struct rxq *rxq) } }; DEBUG("%p: adding MAC address %02x:%02x:%02x:%02x:%02x:%02x", - (void *)rxq, + (void *)priv, (*mac)[0], (*mac)[1], (*mac)[2], (*mac)[3], (*mac)[4], (*mac)[5]); /* Create related flow. */ errno = 0; @@ -2156,60 +2171,8 @@ rxq_mac_addr_add(struct rxq *rxq) return errno; return EINVAL; } - assert(rxq->mac_flow == NULL); - rxq->mac_flow = flow; - return 0; -} - -/** - * Register a MAC address. - * - * In RSS mode, the MAC address is registered in the parent queue, - * otherwise it is registered in each queue directly. - * - * @param priv - * Pointer to private structure. - * @param mac - * MAC address to register. - * - * @return - * 0 on success, errno value on failure. - */ -static int -priv_mac_addr_add(struct priv *priv, const uint8_t (*mac)[ETHER_ADDR_LEN]) -{ - unsigned int i; - int ret; - - priv->mac = (struct ether_addr){ - { - (*mac)[0], (*mac)[1], (*mac)[2], - (*mac)[3], (*mac)[4], (*mac)[5] - } - }; - /* If device isn't started, this is all we need to do. */ - if (!priv->started) { - goto end; - } - if (priv->rss) { - ret = rxq_mac_addr_add(LIST_FIRST(&priv->parents)); - if (ret) - return ret; - goto end; - } - for (i = 0; (i != priv->rxqs_n); ++i) { - if ((*priv->rxqs)[i] == NULL) - continue; - ret = rxq_mac_addr_add((*priv->rxqs)[i]); - if (!ret) - continue; - /* Failure, rollback. */ - while (i != 0) - if ((*priv->rxqs)[(--i)] != NULL) - rxq_mac_addr_del((*priv->rxqs)[i]); - return ret; - } -end: + assert(priv->mac_flow == NULL); + priv->mac_flow = flow; return 0; } @@ -2253,9 +2216,6 @@ rxq_cleanup(struct rxq *rxq) rxq->if_cq, ¶ms)); } - if (rxq->qp != NULL && !rxq->priv->isolated) { - rxq_mac_addr_del(rxq); - } if (rxq->qp != NULL) claim_zero(ibv_destroy_qp(rxq->qp)); if (rxq->cq != NULL) @@ -2894,12 +2854,6 @@ rxq_rehash(struct rte_eth_dev *dev, struct rxq *rxq) DEBUG("%p: nothing to do", (void *)dev); return 0; } - /* Remove attached flows if RSS is disabled (no parent queue). */ - if (!priv->rss && !priv->isolated) { - rxq_mac_addr_del(&tmpl); - /* Update original queue in case of failure. */ - rxq->mac_flow = NULL; - } /* From now on, any failure will render the queue unusable. * Reinitialize QP. */ if (!tmpl.qp) @@ -2933,12 +2887,6 @@ rxq_rehash(struct rte_eth_dev *dev, struct rxq *rxq) assert(err > 0); return err; } - /* Reconfigure flows. Do not care for errors. */ - if (!priv->rss && !priv->isolated) { - rxq_mac_addr_add(&tmpl); - /* Update original queue in case of failure. */ - rxq->mac_flow = NULL; - } /* Allocate pool. */ pool = rte_malloc(__func__, (mbuf_n * sizeof(*pool)), 0); if (pool == NULL) { @@ -3081,15 +3029,6 @@ rxq_create_qp(struct rxq *rxq, strerror(ret)); return ret; } - if (!priv->isolated && (parent || !priv->rss)) { - /* Configure MAC and broadcast addresses. */ - ret = rxq_mac_addr_add(rxq); - if (ret) { - ERROR("QP flow attachment failed: %s", - strerror(ret)); - return ret; - } - } if (!parent) { ret = ibv_post_recv(rxq->qp, (rxq->sp ? @@ -3359,6 +3298,8 @@ mlx4_rx_queue_setup(struct rte_eth_dev *dev, uint16_t idx, uint16_t desc, return -EEXIST; } (*priv->rxqs)[idx] = NULL; + if (idx == 0) + priv_mac_addr_del(priv); rxq_cleanup(rxq); } else { rxq = rte_calloc_socket("RXQ", 1, sizeof(*rxq), 0, socket); @@ -3418,6 +3359,8 @@ mlx4_rx_queue_release(void *dpdk_rxq) DEBUG("%p: removing RX queue %p from list", (void *)priv->dev, (void *)rxq); (*priv->rxqs)[i] = NULL; + if (i == 0) + priv_mac_addr_del(priv); break; } rxq_cleanup(rxq); @@ -3449,9 +3392,6 @@ static int mlx4_dev_start(struct rte_eth_dev *dev) { struct priv *priv = dev->data->dev_private; - unsigned int i = 0; - unsigned int r; - struct rxq *rxq; int ret; priv_lock(priv); @@ -3461,28 +3401,9 @@ mlx4_dev_start(struct rte_eth_dev *dev) } DEBUG("%p: attaching configured flows to all RX queues", (void *)dev); priv->started = 1; - if (priv->isolated) { - rxq = NULL; - r = 1; - } else if (priv->rss) { - rxq = LIST_FIRST(&priv->parents); - r = 1; - } else { - rxq = (*priv->rxqs)[0]; - r = priv->rxqs_n; - } - /* Iterate only once when RSS is enabled. */ - do { - /* Ignore nonexistent RX queues. */ - if (rxq == NULL) - continue; - ret = rxq_mac_addr_add(rxq); - if (!ret) - continue; - WARN("%p: QP flow attachment failed: %s", - (void *)dev, strerror(ret)); + ret = priv_mac_addr_add(priv); + if (ret) goto err; - } while ((--r) && ((rxq = (*priv->rxqs)[++i]), i)); ret = priv_dev_link_interrupt_handler_install(priv, dev); if (ret) { ERROR("%p: LSC handler install failed", @@ -3511,12 +3432,7 @@ mlx4_dev_start(struct rte_eth_dev *dev) return 0; err: /* Rollback. */ - while (i != 0) { - rxq = (*priv->rxqs)[i--]; - if (rxq != NULL) { - rxq_mac_addr_del(rxq); - } - } + priv_mac_addr_del(priv); priv->started = 0; priv_unlock(priv); return -ret; @@ -3534,9 +3450,6 @@ static void mlx4_dev_stop(struct rte_eth_dev *dev) { struct priv *priv = dev->data->dev_private; - unsigned int i = 0; - unsigned int r; - struct rxq *rxq; priv_lock(priv); if (!priv->started) { @@ -3545,24 +3458,8 @@ mlx4_dev_stop(struct rte_eth_dev *dev) } DEBUG("%p: detaching flows from all RX queues", (void *)dev); priv->started = 0; - if (priv->isolated) { - rxq = NULL; - r = 1; - } else if (priv->rss) { - rxq = LIST_FIRST(&priv->parents); - r = 1; - } else { - rxq = (*priv->rxqs)[0]; - r = priv->rxqs_n; - } mlx4_priv_flow_stop(priv); - /* Iterate only once when RSS is enabled. */ - do { - /* Ignore nonexistent RX queues. */ - if (rxq == NULL) - continue; - rxq_mac_addr_del(rxq); - } while ((--r) && ((rxq = (*priv->rxqs)[++i]), i)); + priv_mac_addr_del(priv); priv_unlock(priv); } @@ -3647,6 +3544,7 @@ mlx4_dev_close(struct rte_eth_dev *dev) DEBUG("%p: closing device \"%s\"", (void *)dev, ((priv->ctx != NULL) ? priv->ctx->device->name : "")); + priv_mac_addr_del(priv); /* Prevent crashes when queues are still in use. This is unfortunately * still required for DPDK 1.3 because some programs (such as testpmd) * never release them before closing the device. */ @@ -4036,6 +3934,8 @@ mlx4_dev_set_mtu(struct rte_eth_dev *dev, uint16_t mtu) } else DEBUG("adapter port %u MTU set to %u", priv->port, mtu); priv->mtu = mtu; + /* Remove MAC flow. */ + priv_mac_addr_del(priv); /* Temporarily replace RX handler with a fake one, assuming it has not * been copied elsewhere. */ dev->rx_pkt_burst = removed_rx_burst; @@ -4064,11 +3964,6 @@ mlx4_dev_set_mtu(struct rte_eth_dev *dev, uint16_t mtu) rx_func = mlx4_rx_burst_sp; break; } - /* Reenable non-RSS queue attributes. No need to check - * for errors at this stage. */ - if (!priv->rss && !priv->isolated) { - rxq_mac_addr_add(rxq); - } /* Scattered burst function takes priority. */ if (rxq->sp) rx_func = mlx4_rx_burst_sp; @@ -4076,6 +3971,8 @@ mlx4_dev_set_mtu(struct rte_eth_dev *dev, uint16_t mtu) /* Burst functions can now be called again. */ rte_wmb(); dev->rx_pkt_burst = rx_func; + /* Restore MAC flow. */ + ret = priv_mac_addr_add(priv); out: priv_unlock(priv); assert(ret >= 0); @@ -5125,9 +5022,9 @@ mlx4_pci_probe(struct rte_pci_driver *pci_drv, struct rte_pci_device *pci_dev) mac.addr_bytes[2], mac.addr_bytes[3], mac.addr_bytes[4], mac.addr_bytes[5]); /* Register MAC address. */ - claim_zero(priv_mac_addr_add(priv, - (const uint8_t (*)[ETHER_ADDR_LEN]) - mac.addr_bytes)); + priv->mac = mac; + if (priv_mac_addr_add(priv)) + goto port_error; #ifndef NDEBUG { char ifname[IF_NAMESIZE]; diff --git a/drivers/net/mlx4/mlx4.h b/drivers/net/mlx4/mlx4.h index addc2d5..23ffc87 100644 --- a/drivers/net/mlx4/mlx4.h +++ b/drivers/net/mlx4/mlx4.h @@ -166,7 +166,6 @@ struct rxq { struct ibv_exp_qp_burst_family *if_qp; /* QP burst interface. */ struct ibv_exp_cq_family *if_cq; /* CQ interface. */ struct ibv_comp_channel *channel; - struct ibv_flow *mac_flow; /* Flow associated with MAC address. */ unsigned int port_id; /* Port ID for incoming packets. */ unsigned int elts_n; /* (*elts)[] length. */ unsigned int elts_head; /* Current index in (*elts)[]. */ @@ -243,6 +242,7 @@ struct priv { struct ibv_device_attr device_attr; /* Device properties. */ struct ibv_pd *pd; /* Protection Domain. */ struct ether_addr mac; /* MAC address. */ + struct ibv_flow *mac_flow; /* Flow associated with MAC address. */ /* Device properties. */ uint16_t mtu; /* Configured MTU. */ uint8_t port; /* Physical port number. */