[13/14] net/mlx5: update event handler for multiport IB devices

Message ID 1553155888-27498-14-git-send-email-viacheslavo@mellanox.com (mailing list archive)
State Superseded, archived
Delegated to: Shahaf Shuler
Headers
Series net/mlx5: add support for multiport IB devices |

Checks

Context Check Description
ci/Intel-compilation success Compilation OK
ci/checkpatch success coding style OK

Commit Message

Slava Ovsiienko March 21, 2019, 8:11 a.m. UTC
  This patch modifies asynchronous event handler to support multiport
Infiniband devices. Handler queries the event parameters, including
event source port index, and invokes the handler for specific
devices with appropriate port_id.

Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
---
 drivers/net/mlx5/mlx5_ethdev.c | 101 +++++++++++++++++++++--------------------
 1 file changed, 51 insertions(+), 50 deletions(-)
  

Comments

Shahaf Shuler March 21, 2019, 12:15 p.m. UTC | #1
Thursday, March 21, 2019 10:11 AM, Viacheslav Ovsiienko:
> Subject: [PATCH 13/14] net/mlx5: update event handler for multiport IB
> devices
> 
> This patch modifies asynchronous event handler to support multiport
> Infiniband devices. Handler queries the event parameters, including event
> source port index, and invokes the handler for specific devices with
> appropriate port_id.

This commit should be along w/ the previous one, since interrupts will not work after commit #12 in this series. 

> 
> Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> ---
>  drivers/net/mlx5/mlx5_ethdev.c | 101 +++++++++++++++++++++------------
> --------
>  1 file changed, 51 insertions(+), 50 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_ethdev.c
> b/drivers/net/mlx5/mlx5_ethdev.c index 8358cd2..710e6b5 100644
> --- a/drivers/net/mlx5/mlx5_ethdev.c
> +++ b/drivers/net/mlx5/mlx5_ethdev.c
> @@ -1032,66 +1032,67 @@ int mlx5_fw_version_get(struct rte_eth_dev
> *dev, char *fw_ver, size_t fw_size)  }
> 
>  /**
> - * Device status handler.
> + * Handle shared asynchronous events the NIC (removal event
> + * and link status change). Supports multiport IB device.
>   *
> - * @param dev
> - *   Pointer to Ethernet device.
> - * @param events
> - *   Pointer to event flags holder.
> - *
> - * @return
> - *   Events bitmap of callback process which can be called immediately.
> + * @param cb_arg
> + *   Callback argument.
>   */
> -static uint32_t
> -mlx5_dev_status_handler(struct rte_eth_dev *dev)
> +void
> +mlx5_dev_interrupt_handler(void *cb_arg)
>  {
> -	struct mlx5_priv *priv = dev->data->dev_private;
> +	struct mlx5_ibv_shared *sh = cb_arg;
>  	struct ibv_async_event event;
> -	uint32_t ret = 0;
> 
> -	if (mlx5_link_update(dev, 0) == -EAGAIN) {
> -		usleep(0);
> -		return 0;
> -	}
> -	/* Read all message and acknowledge them. */
> +	/* Read all message from the IB device and acknowledge them. */
>  	for (;;) {
> -		if (mlx5_glue->get_async_event(priv->sh->ctx, &event))
> +		struct rte_eth_dev *dev;
> +		uint32_t tmp;
> +
> +		if (mlx5_glue->get_async_event(sh->ctx, &event))
>  			break;
> +		/* Retrieve and check IB port index. */
> +		tmp = (uint32_t)event.element.port_num;
> +		assert(tmp && (tmp <= sh->max_port));
> +		if (!tmp ||
> +		    tmp > sh->max_port ||
> +		    sh->port[tmp - 1].port_id >= RTE_MAX_ETHPORTS) {
> +			/*
> +			 * Invalid IB port index or no handler
> +			 * installed for this port.
> +			 */
> +			mlx5_glue->ack_async_event(&event);
> +			continue;
> +		}
> +		/* Retrieve ethernet device descriptor. */
> +		tmp = sh->port[tmp - 1].port_id;
> +		dev = &rte_eth_devices[tmp];

Is there a guarantee that the representors ethedev indexes will be contiguous?  
I think we need a mapping between ibdev port and ethedv index. 

> +		tmp = 0;
> +		assert(dev);
>  		if ((event.event_type == IBV_EVENT_PORT_ACTIVE ||
> -			event.event_type == IBV_EVENT_PORT_ERR) &&
> -			(dev->data->dev_conf.intr_conf.lsc == 1))
> -			ret |= (1 << RTE_ETH_EVENT_INTR_LSC);
> -		else if (event.event_type == IBV_EVENT_DEVICE_FATAL &&
> -			dev->data->dev_conf.intr_conf.rmv == 1)
> -			ret |= (1 << RTE_ETH_EVENT_INTR_RMV);
> -		else
> -			DRV_LOG(DEBUG,
> -				"port %u event type %d on not handled",
> -				dev->data->port_id, event.event_type);
> +		     event.event_type == IBV_EVENT_PORT_ERR) &&
> +			dev->data->dev_conf.intr_conf.lsc) {
> +			mlx5_glue->ack_async_event(&event);
> +			if (mlx5_link_update(dev, 0) == -EAGAIN) {
> +				usleep(0);
> +				continue;
> +			}
> +			_rte_eth_dev_callback_process
> +				(dev, RTE_ETH_EVENT_INTR_LSC, NULL);
> +			continue;
> +		}
> +		if (event.event_type == IBV_EVENT_DEVICE_FATAL &&
> +		    dev->data->dev_conf.intr_conf.rmv) {
> +			mlx5_glue->ack_async_event(&event);
> +			_rte_eth_dev_callback_process
> +				(dev, RTE_ETH_EVENT_INTR_RMV, NULL);
> +			continue;
> +		}
> +		DRV_LOG(DEBUG,
> +			"port %u event type %d on not handled",
> +			dev->data->port_id, event.event_type);
>  		mlx5_glue->ack_async_event(&event);
>  	}
> -	return ret;
> -}
> -
> -/**
> - * Handle interrupts from the NIC.
> - *
> - * @param[in] intr_handle
> - *   Interrupt handler.
> - * @param cb_arg
> - *   Callback argument.
> - */
> -void
> -mlx5_dev_interrupt_handler(void *cb_arg) -{
> -	struct rte_eth_dev *dev = cb_arg;
> -	uint32_t events;
> -
> -	events = mlx5_dev_status_handler(dev);
> -	if (events & (1 << RTE_ETH_EVENT_INTR_LSC))
> -		_rte_eth_dev_callback_process(dev,
> RTE_ETH_EVENT_INTR_LSC, NULL);
> -	if (events & (1 << RTE_ETH_EVENT_INTR_RMV))
> -		_rte_eth_dev_callback_process(dev,
> RTE_ETH_EVENT_INTR_RMV, NULL);
>  }
> 
>  /**
> --
> 1.8.3.1
  
Slava Ovsiienko March 21, 2019, 2:08 p.m. UTC | #2
> -----Original Message-----
> From: Shahaf Shuler
> Sent: Thursday, March 21, 2019 14:15
> To: Slava Ovsiienko <viacheslavo@mellanox.com>; dev@dpdk.org
> Subject: RE: [PATCH 13/14] net/mlx5: update event handler for multiport IB
> devices
> 
> Thursday, March 21, 2019 10:11 AM, Viacheslav Ovsiienko:
> > Subject: [PATCH 13/14] net/mlx5: update event handler for multiport IB
> > devices
> >
> > This patch modifies asynchronous event handler to support multiport
> > Infiniband devices. Handler queries the event parameters, including
> > event source port index, and invokes the handler for specific devices
> > with appropriate port_id.
> 
> This commit should be along w/ the previous one, since interrupts will not
> work after commit #12 in this series.

It was 😊
I tried first to do these parts in single commit. But the resulting diff looked
as very weird thing - it intermixed the handler and install/uninstall
routines. So, it was intentionally  splitted in two for better readability.
Nonetheless, if you insist - the is no any problem to squash these ones.
Please, let me know.

> 
> >
> > Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> > ---
> >  drivers/net/mlx5/mlx5_ethdev.c | 101
> > +++++++++++++++++++++------------
> > --------
> >  1 file changed, 51 insertions(+), 50 deletions(-)
> >
> > diff --git a/drivers/net/mlx5/mlx5_ethdev.c
> > b/drivers/net/mlx5/mlx5_ethdev.c index 8358cd2..710e6b5 100644
> > --- a/drivers/net/mlx5/mlx5_ethdev.c
> > +++ b/drivers/net/mlx5/mlx5_ethdev.c
> > @@ -1032,66 +1032,67 @@ int mlx5_fw_version_get(struct rte_eth_dev
> > *dev, char *fw_ver, size_t fw_size)  }
> >
> >  /**
> > - * Device status handler.
> > + * Handle shared asynchronous events the NIC (removal event
> > + * and link status change). Supports multiport IB device.
> >   *
> > - * @param dev
> > - *   Pointer to Ethernet device.
> > - * @param events
> > - *   Pointer to event flags holder.
> > - *
> > - * @return
> > - *   Events bitmap of callback process which can be called immediately.
> > + * @param cb_arg
> > + *   Callback argument.
> >   */
> > -static uint32_t
> > -mlx5_dev_status_handler(struct rte_eth_dev *dev)
> > +void
> > +mlx5_dev_interrupt_handler(void *cb_arg)
> >  {
> > -	struct mlx5_priv *priv = dev->data->dev_private;
> > +	struct mlx5_ibv_shared *sh = cb_arg;
> >  	struct ibv_async_event event;
> > -	uint32_t ret = 0;
> >
> > -	if (mlx5_link_update(dev, 0) == -EAGAIN) {
> > -		usleep(0);
> > -		return 0;
> > -	}
> > -	/* Read all message and acknowledge them. */
> > +	/* Read all message from the IB device and acknowledge them. */
> >  	for (;;) {
> > -		if (mlx5_glue->get_async_event(priv->sh->ctx, &event))
> > +		struct rte_eth_dev *dev;
> > +		uint32_t tmp;
> > +
> > +		if (mlx5_glue->get_async_event(sh->ctx, &event))
> >  			break;
> > +		/* Retrieve and check IB port index. */
> > +		tmp = (uint32_t)event.element.port_num;
> > +		assert(tmp && (tmp <= sh->max_port));
> > +		if (!tmp ||
> > +		    tmp > sh->max_port ||
> > +		    sh->port[tmp - 1].port_id >= RTE_MAX_ETHPORTS) {
> > +			/*
> > +			 * Invalid IB port index or no handler
> > +			 * installed for this port.
> > +			 */
> > +			mlx5_glue->ack_async_event(&event);
> > +			continue;
> > +		}
> > +		/* Retrieve ethernet device descriptor. */
> > +		tmp = sh->port[tmp - 1].port_id;
> > +		dev = &rte_eth_devices[tmp];
> 
> Is there a guarantee that the representors ethedev indexes will be
> contiguous?
No any. And no assumptions of such kind were made during development.

> I think we need a mapping between ibdev port and ethedv index.
Yes, it is exactly done. Array contains the DPDK port_ids, array index is ibv_port.

> 
> > +		tmp = 0;
> > +		assert(dev);
> >  		if ((event.event_type == IBV_EVENT_PORT_ACTIVE ||
> > -			event.event_type == IBV_EVENT_PORT_ERR) &&
> > -			(dev->data->dev_conf.intr_conf.lsc == 1))
> > -			ret |= (1 << RTE_ETH_EVENT_INTR_LSC);
> > -		else if (event.event_type == IBV_EVENT_DEVICE_FATAL &&
> > -			dev->data->dev_conf.intr_conf.rmv == 1)
> > -			ret |= (1 << RTE_ETH_EVENT_INTR_RMV);
> > -		else
> > -			DRV_LOG(DEBUG,
> > -				"port %u event type %d on not handled",
> > -				dev->data->port_id, event.event_type);
> > +		     event.event_type == IBV_EVENT_PORT_ERR) &&
> > +			dev->data->dev_conf.intr_conf.lsc) {
> > +			mlx5_glue->ack_async_event(&event);
> > +			if (mlx5_link_update(dev, 0) == -EAGAIN) {
> > +				usleep(0);
> > +				continue;
> > +			}
> > +			_rte_eth_dev_callback_process
> > +				(dev, RTE_ETH_EVENT_INTR_LSC, NULL);
> > +			continue;
> > +		}
> > +		if (event.event_type == IBV_EVENT_DEVICE_FATAL &&
> > +		    dev->data->dev_conf.intr_conf.rmv) {
> > +			mlx5_glue->ack_async_event(&event);
> > +			_rte_eth_dev_callback_process
> > +				(dev, RTE_ETH_EVENT_INTR_RMV, NULL);
> > +			continue;
> > +		}
> > +		DRV_LOG(DEBUG,
> > +			"port %u event type %d on not handled",
> > +			dev->data->port_id, event.event_type);
> >  		mlx5_glue->ack_async_event(&event);
> >  	}
> > -	return ret;
> > -}
> > -
> > -/**
> > - * Handle interrupts from the NIC.
> > - *
> > - * @param[in] intr_handle
> > - *   Interrupt handler.
> > - * @param cb_arg
> > - *   Callback argument.
> > - */
> > -void
> > -mlx5_dev_interrupt_handler(void *cb_arg) -{
> > -	struct rte_eth_dev *dev = cb_arg;
> > -	uint32_t events;
> > -
> > -	events = mlx5_dev_status_handler(dev);
> > -	if (events & (1 << RTE_ETH_EVENT_INTR_LSC))
> > -		_rte_eth_dev_callback_process(dev,
> > RTE_ETH_EVENT_INTR_LSC, NULL);
> > -	if (events & (1 << RTE_ETH_EVENT_INTR_RMV))
> > -		_rte_eth_dev_callback_process(dev,
> > RTE_ETH_EVENT_INTR_RMV, NULL);
> >  }
> >
> >  /**
> > --
> > 1.8.3.1
  

Patch

diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
index 8358cd2..710e6b5 100644
--- a/drivers/net/mlx5/mlx5_ethdev.c
+++ b/drivers/net/mlx5/mlx5_ethdev.c
@@ -1032,66 +1032,67 @@  int mlx5_fw_version_get(struct rte_eth_dev *dev, char *fw_ver, size_t fw_size)
 }
 
 /**
- * Device status handler.
+ * Handle shared asynchronous events the NIC (removal event
+ * and link status change). Supports multiport IB device.
  *
- * @param dev
- *   Pointer to Ethernet device.
- * @param events
- *   Pointer to event flags holder.
- *
- * @return
- *   Events bitmap of callback process which can be called immediately.
+ * @param cb_arg
+ *   Callback argument.
  */
-static uint32_t
-mlx5_dev_status_handler(struct rte_eth_dev *dev)
+void
+mlx5_dev_interrupt_handler(void *cb_arg)
 {
-	struct mlx5_priv *priv = dev->data->dev_private;
+	struct mlx5_ibv_shared *sh = cb_arg;
 	struct ibv_async_event event;
-	uint32_t ret = 0;
 
-	if (mlx5_link_update(dev, 0) == -EAGAIN) {
-		usleep(0);
-		return 0;
-	}
-	/* Read all message and acknowledge them. */
+	/* Read all message from the IB device and acknowledge them. */
 	for (;;) {
-		if (mlx5_glue->get_async_event(priv->sh->ctx, &event))
+		struct rte_eth_dev *dev;
+		uint32_t tmp;
+
+		if (mlx5_glue->get_async_event(sh->ctx, &event))
 			break;
+		/* Retrieve and check IB port index. */
+		tmp = (uint32_t)event.element.port_num;
+		assert(tmp && (tmp <= sh->max_port));
+		if (!tmp ||
+		    tmp > sh->max_port ||
+		    sh->port[tmp - 1].port_id >= RTE_MAX_ETHPORTS) {
+			/*
+			 * Invalid IB port index or no handler
+			 * installed for this port.
+			 */
+			mlx5_glue->ack_async_event(&event);
+			continue;
+		}
+		/* Retrieve ethernet device descriptor. */
+		tmp = sh->port[tmp - 1].port_id;
+		dev = &rte_eth_devices[tmp];
+		tmp = 0;
+		assert(dev);
 		if ((event.event_type == IBV_EVENT_PORT_ACTIVE ||
-			event.event_type == IBV_EVENT_PORT_ERR) &&
-			(dev->data->dev_conf.intr_conf.lsc == 1))
-			ret |= (1 << RTE_ETH_EVENT_INTR_LSC);
-		else if (event.event_type == IBV_EVENT_DEVICE_FATAL &&
-			dev->data->dev_conf.intr_conf.rmv == 1)
-			ret |= (1 << RTE_ETH_EVENT_INTR_RMV);
-		else
-			DRV_LOG(DEBUG,
-				"port %u event type %d on not handled",
-				dev->data->port_id, event.event_type);
+		     event.event_type == IBV_EVENT_PORT_ERR) &&
+			dev->data->dev_conf.intr_conf.lsc) {
+			mlx5_glue->ack_async_event(&event);
+			if (mlx5_link_update(dev, 0) == -EAGAIN) {
+				usleep(0);
+				continue;
+			}
+			_rte_eth_dev_callback_process
+				(dev, RTE_ETH_EVENT_INTR_LSC, NULL);
+			continue;
+		}
+		if (event.event_type == IBV_EVENT_DEVICE_FATAL &&
+		    dev->data->dev_conf.intr_conf.rmv) {
+			mlx5_glue->ack_async_event(&event);
+			_rte_eth_dev_callback_process
+				(dev, RTE_ETH_EVENT_INTR_RMV, NULL);
+			continue;
+		}
+		DRV_LOG(DEBUG,
+			"port %u event type %d on not handled",
+			dev->data->port_id, event.event_type);
 		mlx5_glue->ack_async_event(&event);
 	}
-	return ret;
-}
-
-/**
- * Handle interrupts from the NIC.
- *
- * @param[in] intr_handle
- *   Interrupt handler.
- * @param cb_arg
- *   Callback argument.
- */
-void
-mlx5_dev_interrupt_handler(void *cb_arg)
-{
-	struct rte_eth_dev *dev = cb_arg;
-	uint32_t events;
-
-	events = mlx5_dev_status_handler(dev);
-	if (events & (1 << RTE_ETH_EVENT_INTR_LSC))
-		_rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC, NULL);
-	if (events & (1 << RTE_ETH_EVENT_INTR_RMV))
-		_rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_RMV, NULL);
 }
 
 /**