[dpdk-dev] net/mlx5: fix handling link status change event
Checks
Commit Message
Even though link of a port gets down, device still can receive traffic.
That is the reason why mlx5_set_link_up/down() switches rx/tx_pkt_burst().
However, if link gets down by an external command (e.g. ifconfig), it isn't
effective. It is better to change burst functions when link status change
is detected.
Fixes: 62072098b54e ("mlx5: support setting link up or down")
Cc: stable@dpdk.org
Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
---
drivers/net/mlx5/mlx5.c | 1 -
drivers/net/mlx5/mlx5.h | 1 +
drivers/net/mlx5/mlx5_ethdev.c | 112 +++++++++++++++++++++++++++++++---------
drivers/net/mlx5/mlx5_trigger.c | 23 ++-------
4 files changed, 93 insertions(+), 44 deletions(-)
Comments
Hi Yongseok,
Small nips.
On Mon, Jan 15, 2018 at 03:48:00PM -0800, Yongseok Koh wrote:
> Even though link of a port gets down, device still can receive traffic.
> That is the reason why mlx5_set_link_up/down() switches rx/tx_pkt_burst().
> However, if link gets down by an external command (e.g. ifconfig), it isn't
> effective. It is better to change burst functions when link status change
> is detected.
>
> Fixes: 62072098b54e ("mlx5: support setting link up or down")
> Cc: stable@dpdk.org
>
> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> ---
> drivers/net/mlx5/mlx5.c | 1 -
> drivers/net/mlx5/mlx5.h | 1 +
> drivers/net/mlx5/mlx5_ethdev.c | 112 +++++++++++++++++++++++++++++++---------
> drivers/net/mlx5/mlx5_trigger.c | 23 ++-------
> 4 files changed, 93 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
> index 1c95f3520..fc2d59fee 100644
> --- a/drivers/net/mlx5/mlx5.c
> +++ b/drivers/net/mlx5/mlx5.c
> @@ -869,7 +869,6 @@ mlx5_pci_probe(struct rte_pci_driver *pci_drv, struct rte_pci_device *pci_dev)
> /* Bring Ethernet device up. */
> DEBUG("forcing Ethernet interface up");
> priv_set_flags(priv, ~IFF_UP, IFF_UP);
> - mlx5_link_update(priv->dev, 1);
> /* Store device configuration on private structure. */
> priv->config = config;
> continue;
> diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
> index e740a4e77..9a4a59bd4 100644
> --- a/drivers/net/mlx5/mlx5.h
> +++ b/drivers/net/mlx5/mlx5.h
> @@ -225,6 +225,7 @@ int priv_set_flags(struct priv *, unsigned int, unsigned int);
> int mlx5_dev_configure(struct rte_eth_dev *);
> void mlx5_dev_infos_get(struct rte_eth_dev *, struct rte_eth_dev_info *);
> const uint32_t *mlx5_dev_supported_ptypes_get(struct rte_eth_dev *dev);
> +int priv_link_update(struct priv *, int);
> int mlx5_link_update(struct rte_eth_dev *, int);
> int mlx5_dev_set_mtu(struct rte_eth_dev *, uint16_t);
> int mlx5_dev_get_flow_ctrl(struct rte_eth_dev *, struct rte_eth_fc_conf *);
> diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
> index 6f78adc4a..16377a8cb 100644
> --- a/drivers/net/mlx5/mlx5_ethdev.c
> +++ b/drivers/net/mlx5/mlx5_ethdev.c
> @@ -885,7 +885,49 @@ mlx5_link_update_unlocked_gs(struct rte_eth_dev *dev, int wait_to_complete)
> }
>
> /**
> - * DPDK callback to retrieve physical link information.
> + * Enable receiving and transmitting traffic.
> + *
> + * @param dev
> + * Pointer to Ethernet device structure.
> + */
> +static void
> +priv_link_start(struct priv *priv)
Comments says dev but parameter is priv.
> +{
> + struct rte_eth_dev *dev = priv->dev;
> + int err;
> +
> + dev->tx_pkt_burst = priv_select_tx_function(priv, dev);
> + dev->rx_pkt_burst = priv_select_rx_function(priv, dev);
> + err = priv_dev_traffic_enable(priv, dev);
> + if (err)
> + ERROR("%p: error occurred while configuring control flows: %s",
> + (void *)priv, strerror(err));
> + err = priv_flow_start(priv, &priv->flows);
> + if (err)
> + ERROR("%p: error occurred while configuring flows: %s",
> + (void *)priv, strerror(err));
> +}
> +
> +/**
> + * Disable receiving and transmitting traffic.
> + *
> + * @param dev
> + * Pointer to Ethernet device structure.
> + */
> +static void
> +priv_link_stop(struct priv *priv)
Same here.
> +{
> + struct rte_eth_dev *dev = priv->dev;
> +
> + priv_flow_stop(priv, &priv->flows);
> + priv_dev_traffic_disable(priv, dev);
> + dev->rx_pkt_burst = removed_rx_burst;
> + dev->tx_pkt_burst = removed_tx_burst;
> +}
> +
> +/**
> + * Retrieve physical link information and update rx/tx_pkt_burst callbacks
> + * accordingly.
> *
> * @param dev
> * Pointer to Ethernet device structure.
> @@ -893,17 +935,54 @@ mlx5_link_update_unlocked_gs(struct rte_eth_dev *dev, int wait_to_complete)
> * Wait for request completion (ignored).
> */
> int
> -mlx5_link_update(struct rte_eth_dev *dev, int wait_to_complete)
> +priv_link_update(struct priv *priv, int wait_to_complete)
<snip/>
Seems to be also the case here.
Thanks,
@@ -869,7 +869,6 @@ mlx5_pci_probe(struct rte_pci_driver *pci_drv, struct rte_pci_device *pci_dev)
/* Bring Ethernet device up. */
DEBUG("forcing Ethernet interface up");
priv_set_flags(priv, ~IFF_UP, IFF_UP);
- mlx5_link_update(priv->dev, 1);
/* Store device configuration on private structure. */
priv->config = config;
continue;
@@ -225,6 +225,7 @@ int priv_set_flags(struct priv *, unsigned int, unsigned int);
int mlx5_dev_configure(struct rte_eth_dev *);
void mlx5_dev_infos_get(struct rte_eth_dev *, struct rte_eth_dev_info *);
const uint32_t *mlx5_dev_supported_ptypes_get(struct rte_eth_dev *dev);
+int priv_link_update(struct priv *, int);
int mlx5_link_update(struct rte_eth_dev *, int);
int mlx5_dev_set_mtu(struct rte_eth_dev *, uint16_t);
int mlx5_dev_get_flow_ctrl(struct rte_eth_dev *, struct rte_eth_fc_conf *);
@@ -885,7 +885,49 @@ mlx5_link_update_unlocked_gs(struct rte_eth_dev *dev, int wait_to_complete)
}
/**
- * DPDK callback to retrieve physical link information.
+ * Enable receiving and transmitting traffic.
+ *
+ * @param dev
+ * Pointer to Ethernet device structure.
+ */
+static void
+priv_link_start(struct priv *priv)
+{
+ struct rte_eth_dev *dev = priv->dev;
+ int err;
+
+ dev->tx_pkt_burst = priv_select_tx_function(priv, dev);
+ dev->rx_pkt_burst = priv_select_rx_function(priv, dev);
+ err = priv_dev_traffic_enable(priv, dev);
+ if (err)
+ ERROR("%p: error occurred while configuring control flows: %s",
+ (void *)priv, strerror(err));
+ err = priv_flow_start(priv, &priv->flows);
+ if (err)
+ ERROR("%p: error occurred while configuring flows: %s",
+ (void *)priv, strerror(err));
+}
+
+/**
+ * Disable receiving and transmitting traffic.
+ *
+ * @param dev
+ * Pointer to Ethernet device structure.
+ */
+static void
+priv_link_stop(struct priv *priv)
+{
+ struct rte_eth_dev *dev = priv->dev;
+
+ priv_flow_stop(priv, &priv->flows);
+ priv_dev_traffic_disable(priv, dev);
+ dev->rx_pkt_burst = removed_rx_burst;
+ dev->tx_pkt_burst = removed_tx_burst;
+}
+
+/**
+ * Retrieve physical link information and update rx/tx_pkt_burst callbacks
+ * accordingly.
*
* @param dev
* Pointer to Ethernet device structure.
@@ -893,17 +935,54 @@ mlx5_link_update_unlocked_gs(struct rte_eth_dev *dev, int wait_to_complete)
* Wait for request completion (ignored).
*/
int
-mlx5_link_update(struct rte_eth_dev *dev, int wait_to_complete)
+priv_link_update(struct priv *priv, int wait_to_complete)
{
+ struct rte_eth_dev *dev = priv->dev;
struct utsname utsname;
int ver[3];
+ int ret;
+ struct rte_eth_link dev_link = dev->data->dev_link;
if (uname(&utsname) == -1 ||
sscanf(utsname.release, "%d.%d.%d",
&ver[0], &ver[1], &ver[2]) != 3 ||
KERNEL_VERSION(ver[0], ver[1], ver[2]) < KERNEL_VERSION(4, 9, 0))
- return mlx5_link_update_unlocked_gset(dev, wait_to_complete);
- return mlx5_link_update_unlocked_gs(dev, wait_to_complete);
+ ret = mlx5_link_update_unlocked_gset(dev, wait_to_complete);
+ else
+ ret = mlx5_link_update_unlocked_gs(dev, wait_to_complete);
+ /* If lsc interrupt is disabled, should always be ready for traffic. */
+ if (!dev->data->dev_conf.intr_conf.lsc) {
+ priv_link_start(priv);
+ return ret;
+ }
+ /* 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)
+ priv_link_start(priv);
+ else
+ priv_link_stop(priv);
+ }
+ return ret;
+}
+
+/**
+ * DPDK callback to retrieve physical link information.
+ *
+ * @param dev
+ * Pointer to Ethernet device structure.
+ * @param wait_to_complete
+ * Wait for request completion (ignored).
+ */
+int
+mlx5_link_update(struct rte_eth_dev *dev, int wait_to_complete)
+{
+ struct priv *priv = dev->data->dev_private;
+ int ret;
+
+ priv_lock(priv);
+ ret = priv_link_update(priv, wait_to_complete);
+ priv_unlock(priv);
+ return ret;
}
/**
@@ -1113,7 +1192,7 @@ priv_link_status_update(struct priv *priv)
{
struct rte_eth_link *link = &priv->dev->data->dev_link;
- mlx5_link_update(priv->dev, 0);
+ priv_link_update(priv, 0);
if (((link->link_speed == 0) && link->link_status) ||
((link->link_speed != 0) && !link->link_status)) {
/*
@@ -1321,24 +1400,9 @@ priv_dev_interrupt_handler_install(struct priv *priv, struct rte_eth_dev *dev)
* 0 on success, errno value on failure.
*/
static int
-priv_dev_set_link(struct priv *priv, struct rte_eth_dev *dev, int up)
+priv_dev_set_link(struct priv *priv, int up)
{
- int err;
-
- if (up) {
- err = priv_set_flags(priv, ~IFF_UP, IFF_UP);
- if (err)
- return err;
- dev->tx_pkt_burst = priv_select_tx_function(priv, dev);
- dev->rx_pkt_burst = priv_select_rx_function(priv, dev);
- } else {
- err = priv_set_flags(priv, ~IFF_UP, ~IFF_UP);
- if (err)
- return err;
- dev->rx_pkt_burst = removed_rx_burst;
- dev->tx_pkt_burst = removed_tx_burst;
- }
- return 0;
+ return priv_set_flags(priv, ~IFF_UP, up ? IFF_UP : ~IFF_UP);
}
/**
@@ -1357,7 +1421,7 @@ mlx5_set_link_down(struct rte_eth_dev *dev)
int err;
priv_lock(priv);
- err = priv_dev_set_link(priv, dev, 0);
+ err = priv_dev_set_link(priv, 0);
priv_unlock(priv);
return err;
}
@@ -1378,7 +1442,7 @@ mlx5_set_link_up(struct rte_eth_dev *dev)
int err;
priv_lock(priv);
- err = priv_dev_set_link(priv, dev, 1);
+ err = priv_dev_set_link(priv, 1);
priv_unlock(priv);
return err;
}
@@ -151,38 +151,23 @@ mlx5_dev_start(struct rte_eth_dev *dev)
(void *)dev, strerror(err));
goto error;
}
- /* Update send callback. */
- dev->tx_pkt_burst = priv_select_tx_function(priv, dev);
err = priv_rxq_start(priv);
if (err) {
ERROR("%p: RXQ allocation failed: %s",
(void *)dev, strerror(err));
goto error;
}
- /* Update receive callback. */
- dev->rx_pkt_burst = priv_select_rx_function(priv, dev);
- err = priv_dev_traffic_enable(priv, dev);
- if (err) {
- ERROR("%p: an error occurred while configuring control flows:"
- " %s",
- (void *)priv, strerror(err));
- goto error;
- }
- err = priv_flow_start(priv, &priv->flows);
- if (err) {
- ERROR("%p: an error occurred while configuring flows:"
- " %s",
- (void *)priv, strerror(err));
- goto error;
- }
err = priv_rx_intr_vec_enable(priv);
if (err) {
ERROR("%p: RX interrupt vector creation failed",
(void *)priv);
goto error;
}
- priv_dev_interrupt_handler_install(priv, dev);
priv_xstats_init(priv);
+ /* Update link status and Tx/Rx callbacks for the first time. */
+ memset(&dev->data->dev_link, 0, sizeof(struct rte_eth_link));
+ priv_link_update(priv, 1);
+ priv_dev_interrupt_handler_install(priv, dev);
priv_unlock(priv);
return 0;
error: