From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <nelio.laranjeiro@6wind.com>
Received: from mail-wm0-f66.google.com (mail-wm0-f66.google.com [74.125.82.66])
 by dpdk.org (Postfix) with ESMTP id 92B545B3C
 for <dev@dpdk.org>; Mon, 12 Mar 2018 14:44:40 +0100 (CET)
Received: by mail-wm0-f66.google.com with SMTP id w128so16732750wmw.0
 for <dev@dpdk.org>; Mon, 12 Mar 2018 06:44:40 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=6wind-com.20150623.gappssmtp.com; s=20150623;
 h=from:to:cc:subject:date:message-id:in-reply-to:references
 :in-reply-to:references;
 bh=d7Q9CCTkDQCmXMNuV16OeRLcZPJd5FaTGZvEYGisGlY=;
 b=g69CxOwDotUsSKZTp7n2d5NzOd89niFoc/gU6IoQopG7DyTcuG33L0Tk7F3mipvoag
 n+bVe9/UjWRp3RG0BspVJhelJSQGbNQZCS/LQ/mJR9Bwl94MktmJILohYbJ7+IZzk3J/
 wrhGu+9lhtd1ofKJDxEOBZ4QfWsbxhEU63y+beA8TIXxu5ISjKeZUbYH3ZDIil5wI4TE
 wBLPcw/Y9V8wvl28LuoB87TeLgAF+e8Ad19brPlfQ6eGv2g+ISDm12RUANpp+/WNAw99
 IR1huKo5WYb2R7aJcEVHUbVPbDK7G/NnS96Bymv/A0hEsAgKWqur67cz7Gg3jXFiG9lg
 En4Q==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20161025;
 h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to
 :references:in-reply-to:references;
 bh=d7Q9CCTkDQCmXMNuV16OeRLcZPJd5FaTGZvEYGisGlY=;
 b=kFCIQaH3ptGhC9e5QEwrCpPUDXnOpexfsL4Ye0PNt/7fDLoq1WwyU0YRjtGHs3ZCzG
 h9MQDgEodNa8eAXtqCkc9Ct2AhyWc26/EKU+nbzSUSi3VJPWOMEWJdA4vIvo5l7A+Nl4
 RZkcBvbcbQwbSI0yA3jyBO+GaX2rE2aUMLnglr6sEnW/QmJJ5KNpYn9hH3I03FyX24UV
 CbXSZiGvTzBFWi3m0KC3gARk1gHPhl8wd1Njvsl5XwZ1Lv2bN1R7+up61sWQpjfG+TtM
 XnLnqf9UpijmNAAVsUuzoJP4gMolCn3qoEOAbql/l2h0rBzaXSBewIl4haAotENAiO57
 C4Dg==
X-Gm-Message-State: AElRT7HZV145VBpfyF7e8g7IPNc14PPG2jfBjFswMV7LlZ5s6OJ74LR6
 LumH1ygW8RshezZFofaPxtxB24tQsg==
X-Google-Smtp-Source: AG47ELvVGTaN4YqUwMq04lib0QmPsphCKG/eeOPzphWZ+GdX7VBc8SXnTJfwNhdkNZYMp11h68P0HA==
X-Received: by 10.28.4.216 with SMTP id 207mr5601013wme.108.1520862280006;
 Mon, 12 Mar 2018 06:44:40 -0700 (PDT)
Received: from laranjeiro-vm.dev.6wind.com
 (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78])
 by smtp.gmail.com with ESMTPSA id y1sm6569198wrh.80.2018.03.12.06.44.39
 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);
 Mon, 12 Mar 2018 06:44:39 -0700 (PDT)
From: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
To: dev@dpdk.org
Cc: Adrien Mazarguil <adrien.mazarguil@6wind.com>,
 Yongseok Koh <yskoh@mellanox.com>, shahafs@mellanox.com, stable@dpdk.org
Date: Mon, 12 Mar 2018 14:43:18 +0100
Message-Id: <f0340cb5f3067ff5031794ad8fc7852dd10d9a76.1520862100.git.nelio.laranjeiro@6wind.com>
X-Mailer: git-send-email 2.11.0
In-Reply-To: <cover.1520862100.git.nelio.laranjeiro@6wind.com>
References: <cover.1520862100.git.nelio.laranjeiro@6wind.com>
In-Reply-To: <cover.1520862100.git.nelio.laranjeiro@6wind.com>
References: <21fb91002768a627d9c7f3d81e0c8a12fbf6811f.1518684427.git.nelio.laranjeiro@6wind.com>
 <cover.1520862100.git.nelio.laranjeiro@6wind.com>
Subject: [dpdk-dev] [PATCH v2 2/3] net/mlx5: fix link status behavior
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Mon, 12 Mar 2018 13:44:40 -0000

This behavior is mixed between what should be handled by the application
and what is under PMD responsibility.

According to DPDK API:
- link_update() should only query the link status [1]
- link_set_{up,down}() should only set the link to the according status [1]
- dev_{start,stop}() should enable/disable traffic reception/emission [2]

On this PMD, the link status is retrieved from the net device associated
owned by the Linux Kernel, it does not means that even when this interface
is down, the PMD cannot send/receive traffic from the NIC those two
information are unrelated, until the physical port is active and has a
link, the PMD can receive/send traffic on the wire.

According to DPDK API, calling the rte_eth_dev_start() even when the Linux
interface link is down is then possible and allowed, as the traffic will
flow between the DPDK application and the Physical port.

This also means that a synchronisation between the Linux interface and the
DPDK application remains under the DPDK application responsibility.

To handle such synchronisation the application should behave as the
following scheme, to start:

 rte_eth_get_link(port_id, &link);
 if (link.link_status == ETH_DOWN)
	rte_eth_dev_set_link_up(port_id);
 rte_eth_dev_start(port_id);

Taking in account the possible returned values fro each function.

and to stop:

 rte_eth_dev_stop(port_id);
 rte_eth_dev_set_link_down(port_id);

The application should also set the LSC interrupt callbacks to catch and
behave accordingly when the administrator set the Linux device down/up.
The same callbacks are called when the link on the medium falls/raise.

Fixes: c7bf62255edf ("net/mlx5: fix handling link status event")
Fixes: e313ef4c2fe8 ("net/mlx5: fix link state on device start")
Cc: yskoh@mellanox.com
Cc: shahafs@mellanox.com
Cc: stable@dpdk.org

[1] https://dpdk.org/browse/dpdk/tree/lib/librte_ether/rte_ethdev_core.h
[2] https://dpdk.org/browse/dpdk/tree/lib/librte_ether/rte_ethdev.h#n1677

Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
Acked-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
---
 drivers/net/mlx5/mlx5.c         |  2 +-
 drivers/net/mlx5/mlx5_ethdev.c  | 92 +----------------------------------------
 drivers/net/mlx5/mlx5_trigger.c | 15 ++++---
 3 files changed, 12 insertions(+), 97 deletions(-)

diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index 4300bafb7..35a018758 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -962,7 +962,7 @@ mlx5_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 		/* Bring Ethernet device up. */
 		DRV_LOG(DEBUG, "port %u forcing Ethernet interface up",
 			eth_dev->data->port_id);
-		mlx5_set_flags(eth_dev, ~IFF_UP, IFF_UP);
+		mlx5_set_link_up(eth_dev);
 		/* Store device configuration on private structure. */
 		priv->config = config;
 		continue;
diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
index 26f13fb1b..10ba27c79 100644
--- a/drivers/net/mlx5/mlx5_ethdev.c
+++ b/drivers/net/mlx5/mlx5_ethdev.c
@@ -645,80 +645,6 @@ mlx5_link_update_unlocked_gs(struct rte_eth_dev *dev)
 }
 
 /**
- * Enable receiving and transmitting traffic.
- *
- * @param dev
- *   Pointer to Ethernet device.
- */
-static void
-mlx5_link_start(struct rte_eth_dev *dev)
-{
-	struct priv *priv = dev->data->dev_private;
-	int ret;
-
-	dev->tx_pkt_burst = mlx5_select_tx_function(dev);
-	dev->rx_pkt_burst = mlx5_select_rx_function(dev);
-	ret = mlx5_traffic_enable(dev);
-	if (ret) {
-		DRV_LOG(ERR,
-			"port %u error occurred while configuring control"
-			" flows: %s",
-			dev->data->port_id, strerror(rte_errno));
-		return;
-	}
-	ret = mlx5_flow_start(dev, &priv->flows);
-	if (ret)
-		DRV_LOG(ERR,
-			"port %u error occurred while configuring flows: %s",
-			dev->data->port_id, strerror(rte_errno));
-}
-
-/**
- * Disable receiving and transmitting traffic.
- *
- * @param dev
- *   Pointer to Ethernet device.
- */
-static void
-mlx5_link_stop(struct rte_eth_dev *dev)
-{
-	struct priv *priv = dev->data->dev_private;
-
-	mlx5_flow_stop(dev, &priv->flows);
-	mlx5_traffic_disable(dev);
-	dev->rx_pkt_burst = removed_rx_burst;
-	dev->tx_pkt_burst = removed_tx_burst;
-}
-
-/**
- * Querying the link status till it changes to the desired state.
- * Number of query attempts is bounded by MLX5_MAX_LINK_QUERY_ATTEMPTS.
- *
- * @param dev
- *   Pointer to Ethernet device.
- * @param status
- *   Link desired status.
- *
- * @return
- *   0 on success, a negative errno value otherwise and rte_errno is set.
- */
-int
-mlx5_force_link_status_change(struct rte_eth_dev *dev, int status)
-{
-	int try = 0;
-
-	while (try < MLX5_MAX_LINK_QUERY_ATTEMPTS) {
-		mlx5_link_update(dev, 0);
-		if (dev->data->dev_link.link_status == status)
-			return 0;
-		try++;
-		sleep(1);
-	}
-	rte_errno = EAGAIN;
-	return -rte_errno;
-}
-
-/**
  * DPDK callback to retrieve physical link information.
  *
  * @param dev
@@ -733,26 +659,10 @@ int
 mlx5_link_update(struct rte_eth_dev *dev, int wait_to_complete __rte_unused)
 {
 	int ret;
-	struct rte_eth_link dev_link = dev->data->dev_link;
 
 	ret = mlx5_link_update_unlocked_gset(dev);
-	if (ret) {
+	if (ret)
 		ret = mlx5_link_update_unlocked_gs(dev);
-		if (ret)
-			return ret;
-	}
-	/* If lsc interrupt is disabled, should always be ready for traffic. */
-	if (!dev->data->dev_conf.intr_conf.lsc) {
-		mlx5_link_start(dev);
-		return 0;
-	}
-	/* Re-select burst callbacks only if link status has been changed. */
-	if (!ret && dev_link.link_status != dev->data->dev_link.link_status) {
-		if (dev->data->dev_link.link_status == ETH_LINK_UP)
-			mlx5_link_start(dev);
-		else
-			mlx5_link_stop(dev);
-	}
 	return 0;
 }
 
diff --git a/drivers/net/mlx5/mlx5_trigger.c b/drivers/net/mlx5/mlx5_trigger.c
index 28770b8eb..6bb4ffb14 100644
--- a/drivers/net/mlx5/mlx5_trigger.c
+++ b/drivers/net/mlx5/mlx5_trigger.c
@@ -176,15 +176,20 @@ mlx5_dev_start(struct rte_eth_dev *dev)
 		goto error;
 	}
 	mlx5_xstats_init(dev);
-	/* Update link status and Tx/Rx callbacks for the first time. */
-	memset(&dev->data->dev_link, 0, sizeof(struct rte_eth_link));
-	DRV_LOG(INFO, "forcing port %u link to be up", dev->data->port_id);
-	ret = mlx5_force_link_status_change(dev, ETH_LINK_UP);
+	ret = mlx5_traffic_enable(dev);
 	if (ret) {
-		DRV_LOG(DEBUG, "failed to set port %u link to be up",
+		DRV_LOG(DEBUG, "port %u failed to set defaults flows",
 			dev->data->port_id);
 		goto error;
 	}
+	ret = mlx5_flow_start(dev, &priv->flows);
+	if (ret) {
+		DRV_LOG(DEBUG, "port %u failed to set flows",
+			dev->data->port_id);
+		goto error;
+	}
+	dev->tx_pkt_burst = mlx5_select_tx_function(dev);
+	dev->rx_pkt_burst = mlx5_select_rx_function(dev);
 	mlx5_dev_interrupt_handler_install(dev);
 	return 0;
 error:
-- 
2.11.0