From: Adrien Mazarguil Date: Thu, 12 Oct 2017 12:19:29 +0000 (+0200) Subject: net/mlx4: simplify trigger code for flow rules X-Git-Tag: spdx-start~1357 X-Git-Url: http://git.droids-corp.org/?a=commitdiff_plain;h=fee75e14f31cbd6c479e4fdbe367bdc640adbe01;p=dpdk.git net/mlx4: simplify trigger code for flow rules Since flow rules synchronization function mlx4_flow_sync() takes into account the state of the device (whether it is started), trigger functions mlx4_flow_start() and mlx4_flow_stop() are redundant. Standardize on mlx4_flow_sync(). Use this opportunity to enhance this function with better error reporting as the inability to start the device due to a problem with a flow rule otherwise results in a nondescript error code. Signed-off-by: Adrien Mazarguil Acked-by: Nelio Laranjeiro --- diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c index 40c0ee2974..256aa3d66c 100644 --- a/drivers/net/mlx4/mlx4.c +++ b/drivers/net/mlx4/mlx4.c @@ -60,6 +60,7 @@ #include #include #include +#include #include #include #include @@ -97,13 +98,17 @@ static int mlx4_dev_configure(struct rte_eth_dev *dev) { struct priv *priv = dev->data->dev_private; + struct rte_flow_error error; int ret; /* Prepare internal flow rules. */ - ret = mlx4_flow_sync(priv); - if (ret) - ERROR("cannot set up internal flow rules: %s", - strerror(-ret)); + ret = mlx4_flow_sync(priv, &error); + if (ret) { + ERROR("cannot set up internal flow rules (code %d, \"%s\")," + " flow error type %d, cause %p, message: %s", + -ret, strerror(-ret), error.type, error.cause, + error.message ? error.message : "(unspecified)"); + } return ret; } @@ -122,6 +127,7 @@ static int mlx4_dev_start(struct rte_eth_dev *dev) { struct priv *priv = dev->data->dev_private; + struct rte_flow_error error; int ret; if (priv->started) @@ -134,10 +140,13 @@ mlx4_dev_start(struct rte_eth_dev *dev) (void *)dev); goto err; } - ret = mlx4_flow_start(priv); + ret = mlx4_flow_sync(priv, &error); if (ret) { - ERROR("%p: flow start failed: %s", - (void *)dev, strerror(ret)); + ERROR("%p: cannot attach flow rules (code %d, \"%s\")," + " flow error type %d, cause %p, message: %s", + (void *)dev, + -ret, strerror(-ret), error.type, error.cause, + error.message ? error.message : "(unspecified)"); goto err; } return 0; @@ -164,7 +173,7 @@ mlx4_dev_stop(struct rte_eth_dev *dev) return; DEBUG("%p: detaching flows from all RX queues", (void *)dev); priv->started = 0; - mlx4_flow_stop(priv); + mlx4_flow_sync(priv, NULL); mlx4_intr_uninstall(priv); } diff --git a/drivers/net/mlx4/mlx4_flow.c b/drivers/net/mlx4/mlx4_flow.c index e1290a8609..ec6c28ff18 100644 --- a/drivers/net/mlx4/mlx4_flow.c +++ b/drivers/net/mlx4/mlx4_flow.c @@ -956,14 +956,9 @@ mlx4_flow_isolate(struct rte_eth_dev *dev, if (!!enable == !!priv->isolated) return 0; priv->isolated = !!enable; - if (mlx4_flow_sync(priv)) { + if (mlx4_flow_sync(priv, error)) { priv->isolated = !enable; - return rte_flow_error_set(error, rte_errno, - RTE_FLOW_ERROR_TYPE_UNSPECIFIED, - NULL, - enable ? - "cannot enter isolated mode" : - "cannot leave isolated mode"); + return -rte_errno; } return 0; } @@ -1075,12 +1070,14 @@ mlx4_flow_internal(struct priv *priv, struct rte_flow_error *error) * * @param priv * Pointer to private structure. + * @param[out] error + * Perform verbose error reporting if not NULL. * * @return * 0 on success, a negative errno value otherwise and rte_errno is set. */ int -mlx4_flow_sync(struct priv *priv) +mlx4_flow_sync(struct priv *priv, struct rte_flow_error *error) { struct rte_flow *flow; int ret; @@ -1094,20 +1091,27 @@ mlx4_flow_sync(struct priv *priv) for (flow = LIST_FIRST(&priv->flows); flow && flow->internal; flow = LIST_FIRST(&priv->flows)) - claim_zero(mlx4_flow_destroy(priv->dev, flow, NULL)); + claim_zero(mlx4_flow_destroy(priv->dev, flow, error)); } else if (!LIST_FIRST(&priv->flows) || !LIST_FIRST(&priv->flows)->internal) { /* * If the first rule is not internal outside isolated mode, * they must be added back. */ - ret = mlx4_flow_internal(priv, NULL); + ret = mlx4_flow_internal(priv, error); + if (ret) + return ret; + } + /* Toggle the remaining flow rules . */ + for (flow = LIST_FIRST(&priv->flows); + flow; + flow = LIST_NEXT(flow, next)) { + ret = mlx4_flow_toggle(priv, flow, priv->started, error); if (ret) return ret; } - if (priv->started) - return mlx4_flow_start(priv); - mlx4_flow_stop(priv); + if (!priv->started) + assert(!priv->drop); return 0; } @@ -1129,52 +1133,6 @@ mlx4_flow_clean(struct priv *priv) mlx4_flow_destroy(priv->dev, flow, NULL); } -/** - * Disable flow rules. - * - * @param priv - * Pointer to private structure. - */ -void -mlx4_flow_stop(struct priv *priv) -{ - struct rte_flow *flow; - - for (flow = LIST_FIRST(&priv->flows); - flow; - flow = LIST_NEXT(flow, next)) { - claim_zero(mlx4_flow_toggle(priv, flow, 0, NULL)); - } - assert(!priv->drop); -} - -/** - * Enable flow rules. - * - * @param priv - * Pointer to private structure. - * - * @return - * 0 on success, a negative errno value otherwise and rte_errno is set. - */ -int -mlx4_flow_start(struct priv *priv) -{ - int ret; - struct rte_flow *flow; - - for (flow = LIST_FIRST(&priv->flows); - flow; - flow = LIST_NEXT(flow, next)) { - ret = mlx4_flow_toggle(priv, flow, 1, NULL); - if (unlikely(ret)) { - mlx4_flow_stop(priv); - return ret; - } - } - return 0; -} - static const struct rte_flow_ops mlx4_flow_ops = { .validate = mlx4_flow_validate, .create = mlx4_flow_create, diff --git a/drivers/net/mlx4/mlx4_flow.h b/drivers/net/mlx4/mlx4_flow.h index c2ffa8d233..13495d7400 100644 --- a/drivers/net/mlx4/mlx4_flow.h +++ b/drivers/net/mlx4/mlx4_flow.h @@ -72,10 +72,8 @@ struct rte_flow { /* mlx4_flow.c */ -int mlx4_flow_sync(struct priv *priv); +int mlx4_flow_sync(struct priv *priv, struct rte_flow_error *error); void mlx4_flow_clean(struct priv *priv); -int mlx4_flow_start(struct priv *priv); -void mlx4_flow_stop(struct priv *priv); int mlx4_filter_ctrl(struct rte_eth_dev *dev, enum rte_filter_type filter_type, enum rte_filter_op filter_op, diff --git a/drivers/net/mlx4/mlx4_rxq.c b/drivers/net/mlx4/mlx4_rxq.c index 7bb2f9e2f2..bcb7b9490c 100644 --- a/drivers/net/mlx4/mlx4_rxq.c +++ b/drivers/net/mlx4/mlx4_rxq.c @@ -54,6 +54,7 @@ #include #include #include +#include #include #include #include @@ -401,7 +402,7 @@ mlx4_rx_queue_setup(struct rte_eth_dev *dev, uint16_t idx, uint16_t desc, } dev->data->rx_queues[idx] = NULL; /* Disable associated flows. */ - mlx4_flow_sync(priv); + mlx4_flow_sync(priv, NULL); mlx4_rxq_cleanup(rxq); } else { rxq = rte_calloc_socket("RXQ", 1, sizeof(*rxq), 0, socket); @@ -416,13 +417,20 @@ mlx4_rx_queue_setup(struct rte_eth_dev *dev, uint16_t idx, uint16_t desc, if (ret) { rte_free(rxq); } else { + struct rte_flow_error error; + rxq->stats.idx = idx; DEBUG("%p: adding Rx queue %p to list", (void *)dev, (void *)rxq); dev->data->rx_queues[idx] = rxq; /* Re-enable associated flows. */ - ret = mlx4_flow_sync(priv); + ret = mlx4_flow_sync(priv, &error); if (ret) { + ERROR("cannot re-attach flow rules to queue %u" + " (code %d, \"%s\"), flow error type %d," + " cause %p, message: %s", idx, + -ret, strerror(-ret), error.type, error.cause, + error.message ? error.message : "(unspecified)"); dev->data->rx_queues[idx] = NULL; mlx4_rxq_cleanup(rxq); rte_free(rxq); @@ -457,7 +465,7 @@ mlx4_rx_queue_release(void *dpdk_rxq) priv->dev->data->rx_queues[i] = NULL; break; } - mlx4_flow_sync(priv); + mlx4_flow_sync(priv, NULL); mlx4_rxq_cleanup(rxq); rte_free(rxq); }