[dpdk-dev] [PATCH] drivers/net: update link status

[dpdk-dev] [PATCH] drivers/net: update link status

[Ferruh Yigit]

Update link status related feature document items and minor updates in
some link status related functions.

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 doc/guides/nics/features/fm10k.ini      | 2 ++
 doc/guides/nics/features/fm10k_vf.ini   | 2 ++
 doc/guides/nics/features/i40e_vf.ini    | 1 +
 doc/guides/nics/features/igb_vf.ini     | 1 +
 doc/guides/nics/features/qede.ini       | 1 -
 doc/guides/nics/features/qede_vf.ini    | 1 -
 doc/guides/nics/features/vhost.ini      | 2 --
 doc/guides/nics/features/virtio_vec.ini | 1 +
 drivers/net/e1000/em_ethdev.c           | 2 +-
 drivers/net/ena/ena_ethdev.c            | 2 +-
 drivers/net/fm10k/fm10k_ethdev.c        | 6 ++----
 drivers/net/i40e/i40e_ethdev_vf.c       | 2 +-
 drivers/net/ixgbe/ixgbe_ethdev.c        | 2 +-
 drivers/net/mlx4/mlx4_ethdev.c          | 2 +-
 drivers/net/mlx5/mlx5_ethdev.c          | 2 +-
 15 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/doc/guides/nics/features/fm10k.ini b/doc/guides/nics/features/fm10k.ini
index f0f61a7d7..58f58b99c 100644
--- a/doc/guides/nics/features/fm10k.ini
+++ b/doc/guides/nics/features/fm10k.ini
@@ -5,6 +5,8 @@
 ;
 [Features]
 Speed capabilities   = P
+Link status          = Y
+Link status event    = Y
 Rx interrupt         = Y
 Queue start/stop     = Y
 Jumbo frame          = Y
diff --git a/doc/guides/nics/features/fm10k_vf.ini b/doc/guides/nics/features/fm10k_vf.ini
index 32b93df4b..44b50faa1 100644
--- a/doc/guides/nics/features/fm10k_vf.ini
+++ b/doc/guides/nics/features/fm10k_vf.ini
@@ -5,6 +5,8 @@
 ;
 [Features]
 Speed capabilities   = P
+Link status          = Y
+Link status event    = Y
 Rx interrupt         = Y
 Queue start/stop     = Y
 Jumbo frame          = Y
diff --git a/doc/guides/nics/features/i40e_vf.ini b/doc/guides/nics/features/i40e_vf.ini
index 46e0d9fce..ba2d8cbe9 100644
--- a/doc/guides/nics/features/i40e_vf.ini
+++ b/doc/guides/nics/features/i40e_vf.ini
@@ -5,6 +5,7 @@
 ;
 [Features]
 Rx interrupt         = Y
+Link status          = Y
 Queue start/stop     = Y
 Jumbo frame          = Y
 Scattered Rx         = Y
diff --git a/doc/guides/nics/features/igb_vf.ini b/doc/guides/nics/features/igb_vf.ini
index e641a2c97..d9653234b 100644
--- a/doc/guides/nics/features/igb_vf.ini
+++ b/doc/guides/nics/features/igb_vf.ini
@@ -4,6 +4,7 @@
 ; Refer to default.ini for the full list of available PMD features.
 ;
 [Features]
+Link status          = Y
 Rx interrupt         = Y
 Scattered Rx         = Y
 TSO                  = Y
diff --git a/doc/guides/nics/features/qede.ini b/doc/guides/nics/features/qede.ini
index cbadc1949..13e34ae33 100644
--- a/doc/guides/nics/features/qede.ini
+++ b/doc/guides/nics/features/qede.ini
@@ -6,7 +6,6 @@
 [Features]
 Speed capabilities   = Y
 Link status          = Y
-Link status event    = Y
 MTU update           = Y
 Jumbo frame          = Y
 Scattered Rx         = Y
diff --git a/doc/guides/nics/features/qede_vf.ini b/doc/guides/nics/features/qede_vf.ini
index 18857b6e3..70071a1bd 100644
--- a/doc/guides/nics/features/qede_vf.ini
+++ b/doc/guides/nics/features/qede_vf.ini
@@ -6,7 +6,6 @@
 [Features]
 Speed capabilities   = Y
 Link status          = Y
-Link status event    = Y
 MTU update           = Y
 Jumbo frame          = Y
 Scattered Rx         = Y
diff --git a/doc/guides/nics/features/vhost.ini b/doc/guides/nics/features/vhost.ini
index dffd1f493..31302745a 100644
--- a/doc/guides/nics/features/vhost.ini
+++ b/doc/guides/nics/features/vhost.ini
@@ -4,8 +4,6 @@
 ; Refer to default.ini for the full list of available PMD features.
 ;
 [Features]
-Link status          = Y
-Link status event    = Y
 Free Tx mbuf on demand = Y
 Queue status event   = Y
 Basic stats          = Y
diff --git a/doc/guides/nics/features/virtio_vec.ini b/doc/guides/nics/features/virtio_vec.ini
index c06c860d4..e60fe36ae 100644
--- a/doc/guides/nics/features/virtio_vec.ini
+++ b/doc/guides/nics/features/virtio_vec.ini
@@ -6,6 +6,7 @@
 [Features]
 Speed capabilities   = P
 Link status          = Y
+Link status event    = Y
 Rx interrupt         = Y
 Queue start/stop     = Y
 Promiscuous mode     = Y
diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
index 242375ff1..080df70c4 100644
--- a/drivers/net/e1000/em_ethdev.c
+++ b/drivers/net/e1000/em_ethdev.c
@@ -1210,7 +1210,7 @@ eth_em_link_update(struct rte_eth_dev *dev, int wait_to_complete)
 		link.link_autoneg = !(dev->data->dev_conf.link_speeds &
 				ETH_LINK_SPEED_FIXED);
 	} else if (!link_check && (link.link_status == ETH_LINK_UP)) {
-		link.link_speed = 0;
+		link.link_speed = ETH_SPEED_NUM_NONE;
 		link.link_duplex = ETH_LINK_HALF_DUPLEX;
 		link.link_status = ETH_LINK_DOWN;
 		link.link_autoneg = ETH_LINK_FIXED;
diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index 34b2a8d78..ad4e03dba 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -724,7 +724,7 @@ static int ena_link_update(struct rte_eth_dev *dev,
 {
 	struct rte_eth_link *link = &dev->data->dev_link;
 
-	link->link_status = 1;
+	link->link_status = ETH_LINK_UP;
 	link->link_speed = ETH_SPEED_NUM_10G;
 	link->link_duplex = ETH_LINK_FULL_DUPLEX;
 
diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index 94237610c..cc1a773a7 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -1233,13 +1233,11 @@ fm10k_link_update(struct rte_eth_dev *dev,
 		FM10K_DEV_PRIVATE_TO_INFO(dev->data->dev_private);
 	PMD_INIT_FUNC_TRACE();
 
-	/* The speed is ~50Gbps per Gen3 x8 PCIe interface. For now, we
-	 * leave the speed undefined since there is no 50Gbps Ethernet.
-	 */
-	dev->data->dev_link.link_speed  = 0;
+	dev->data->dev_link.link_speed  = ETH_SPEED_NUM_50G;
 	dev->data->dev_link.link_duplex = ETH_LINK_FULL_DUPLEX;
 	dev->data->dev_link.link_status =
 		dev_info->sm_down ? ETH_LINK_DOWN : ETH_LINK_UP;
+	dev->data->dev_link.link_autoneg = ETH_LINK_FIXED;
 
 	return 0;
 }
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index fd003fe01..c771edde5 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -2107,7 +2107,7 @@ i40evf_dev_link_update(struct rte_eth_dev *dev,
 	new_link.link_status = vf->link_up ? ETH_LINK_UP :
 					     ETH_LINK_DOWN;
 	new_link.link_autoneg =
-		dev->data->dev_conf.link_speeds & ETH_LINK_SPEED_FIXED;
+		!(dev->data->dev_conf.link_speeds & ETH_LINK_SPEED_FIXED);
 
 	i40evf_dev_atomic_write_link_status(dev, &new_link);
 
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 448325857..bad83968c 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -3957,7 +3957,7 @@ ixgbe_dev_link_update_share(struct rte_eth_dev *dev,
 	bool autoneg = false;
 
 	link.link_status = ETH_LINK_DOWN;
-	link.link_speed = 0;
+	link.link_speed = ETH_SPEED_NUM_NONE;
 	link.link_duplex = ETH_LINK_HALF_DUPLEX;
 	link.link_autoneg = ETH_LINK_AUTONEG;
 	memset(&old, 0, sizeof(old));
diff --git a/drivers/net/mlx4/mlx4_ethdev.c b/drivers/net/mlx4/mlx4_ethdev.c
index fbeef16c8..beecc53ba 100644
--- a/drivers/net/mlx4/mlx4_ethdev.c
+++ b/drivers/net/mlx4/mlx4_ethdev.c
@@ -710,7 +710,7 @@ mlx4_link_update(struct rte_eth_dev *dev, int wait_to_complete)
 	}
 	link_speed = ethtool_cmd_speed(&edata);
 	if (link_speed == -1)
-		dev_link.link_speed = 0;
+		dev_link.link_speed = ETH_SPEED_NUM_NONE;
 	else
 		dev_link.link_speed = link_speed;
 	dev_link.link_duplex = ((edata.duplex == DUPLEX_HALF) ?
diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
index b73cb53df..de5576099 100644
--- a/drivers/net/mlx5/mlx5_ethdev.c
+++ b/drivers/net/mlx5/mlx5_ethdev.c
@@ -536,7 +536,7 @@ mlx5_link_update_unlocked_gset(struct rte_eth_dev *dev, int wait_to_complete)
 	}
 	link_speed = ethtool_cmd_speed(&edata);
 	if (link_speed == -1)
-		dev_link.link_speed = 0;
+		dev_link.link_speed = ETH_SPEED_NUM_NONE;
 	else
 		dev_link.link_speed = link_speed;
 	priv->link_speed_capa = 0;
-- 
2.13.6

Re: [dpdk-dev] [PATCH] drivers/net: update link status

[Nélio Laranjeiro]

On Tue, Mar 13, 2018 at 06:05:34PM +0000, Ferruh Yigit wrote:
> Update link status related feature document items and minor updates in
> some link status related functions.
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
>  doc/guides/nics/features/fm10k.ini      | 2 ++
>  doc/guides/nics/features/fm10k_vf.ini   | 2 ++
>  doc/guides/nics/features/i40e_vf.ini    | 1 +
>  doc/guides/nics/features/igb_vf.ini     | 1 +
>  doc/guides/nics/features/qede.ini       | 1 -
>  doc/guides/nics/features/qede_vf.ini    | 1 -
>  doc/guides/nics/features/vhost.ini      | 2 --
>  doc/guides/nics/features/virtio_vec.ini | 1 +
>  drivers/net/e1000/em_ethdev.c           | 2 +-
>  drivers/net/ena/ena_ethdev.c            | 2 +-
>  drivers/net/fm10k/fm10k_ethdev.c        | 6 ++----
>  drivers/net/i40e/i40e_ethdev_vf.c       | 2 +-
>  drivers/net/ixgbe/ixgbe_ethdev.c        | 2 +-
>  drivers/net/mlx4/mlx4_ethdev.c          | 2 +-
>  drivers/net/mlx5/mlx5_ethdev.c          | 2 +-
>  15 files changed, 15 insertions(+), 14 deletions(-)
> 
> diff --git a/doc/guides/nics/features/fm10k.ini b/doc/guides/nics/features/fm10k.ini
> index f0f61a7d7..58f58b99c 100644
> --- a/doc/guides/nics/features/fm10k.ini
> +++ b/doc/guides/nics/features/fm10k.ini
> @@ -5,6 +5,8 @@
>  ;
>  [Features]
>  Speed capabilities   = P
> +Link status          = Y
> +Link status event    = Y
>  Rx interrupt         = Y
>  Queue start/stop     = Y
>  Jumbo frame          = Y
> diff --git a/doc/guides/nics/features/fm10k_vf.ini b/doc/guides/nics/features/fm10k_vf.ini
> index 32b93df4b..44b50faa1 100644
> --- a/doc/guides/nics/features/fm10k_vf.ini
> +++ b/doc/guides/nics/features/fm10k_vf.ini
> @@ -5,6 +5,8 @@
>  ;
>  [Features]
>  Speed capabilities   = P
> +Link status          = Y
> +Link status event    = Y
>  Rx interrupt         = Y
>  Queue start/stop     = Y
>  Jumbo frame          = Y
> diff --git a/doc/guides/nics/features/i40e_vf.ini b/doc/guides/nics/features/i40e_vf.ini
> index 46e0d9fce..ba2d8cbe9 100644
> --- a/doc/guides/nics/features/i40e_vf.ini
> +++ b/doc/guides/nics/features/i40e_vf.ini
> @@ -5,6 +5,7 @@
>  ;
>  [Features]
>  Rx interrupt         = Y
> +Link status          = Y
>  Queue start/stop     = Y
>  Jumbo frame          = Y
>  Scattered Rx         = Y
> diff --git a/doc/guides/nics/features/igb_vf.ini b/doc/guides/nics/features/igb_vf.ini
> index e641a2c97..d9653234b 100644
> --- a/doc/guides/nics/features/igb_vf.ini
> +++ b/doc/guides/nics/features/igb_vf.ini
> @@ -4,6 +4,7 @@
>  ; Refer to default.ini for the full list of available PMD features.
>  ;
>  [Features]
> +Link status          = Y
>  Rx interrupt         = Y
>  Scattered Rx         = Y
>  TSO                  = Y
> diff --git a/doc/guides/nics/features/qede.ini b/doc/guides/nics/features/qede.ini
> index cbadc1949..13e34ae33 100644
> --- a/doc/guides/nics/features/qede.ini
> +++ b/doc/guides/nics/features/qede.ini
> @@ -6,7 +6,6 @@
>  [Features]
>  Speed capabilities   = Y
>  Link status          = Y
> -Link status event    = Y
>  MTU update           = Y
>  Jumbo frame          = Y
>  Scattered Rx         = Y
> diff --git a/doc/guides/nics/features/qede_vf.ini b/doc/guides/nics/features/qede_vf.ini
> index 18857b6e3..70071a1bd 100644
> --- a/doc/guides/nics/features/qede_vf.ini
> +++ b/doc/guides/nics/features/qede_vf.ini
> @@ -6,7 +6,6 @@
>  [Features]
>  Speed capabilities   = Y
>  Link status          = Y
> -Link status event    = Y
>  MTU update           = Y
>  Jumbo frame          = Y
>  Scattered Rx         = Y
> diff --git a/doc/guides/nics/features/vhost.ini b/doc/guides/nics/features/vhost.ini
> index dffd1f493..31302745a 100644
> --- a/doc/guides/nics/features/vhost.ini
> +++ b/doc/guides/nics/features/vhost.ini
> @@ -4,8 +4,6 @@
>  ; Refer to default.ini for the full list of available PMD features.
>  ;
>  [Features]
> -Link status          = Y
> -Link status event    = Y
>  Free Tx mbuf on demand = Y
>  Queue status event   = Y
>  Basic stats          = Y
> diff --git a/doc/guides/nics/features/virtio_vec.ini b/doc/guides/nics/features/virtio_vec.ini
> index c06c860d4..e60fe36ae 100644
> --- a/doc/guides/nics/features/virtio_vec.ini
> +++ b/doc/guides/nics/features/virtio_vec.ini
> @@ -6,6 +6,7 @@
>  [Features]
>  Speed capabilities   = P
>  Link status          = Y
> +Link status event    = Y
>  Rx interrupt         = Y
>  Queue start/stop     = Y
>  Promiscuous mode     = Y
> diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
> index 242375ff1..080df70c4 100644
> --- a/drivers/net/e1000/em_ethdev.c
> +++ b/drivers/net/e1000/em_ethdev.c
> @@ -1210,7 +1210,7 @@ eth_em_link_update(struct rte_eth_dev *dev, int wait_to_complete)
>  		link.link_autoneg = !(dev->data->dev_conf.link_speeds &
>  				ETH_LINK_SPEED_FIXED);
>  	} else if (!link_check && (link.link_status == ETH_LINK_UP)) {
> -		link.link_speed = 0;
> +		link.link_speed = ETH_SPEED_NUM_NONE;
>  		link.link_duplex = ETH_LINK_HALF_DUPLEX;
>  		link.link_status = ETH_LINK_DOWN;
>  		link.link_autoneg = ETH_LINK_FIXED;
> diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
> index 34b2a8d78..ad4e03dba 100644
> --- a/drivers/net/ena/ena_ethdev.c
> +++ b/drivers/net/ena/ena_ethdev.c
> @@ -724,7 +724,7 @@ static int ena_link_update(struct rte_eth_dev *dev,
>  {
>  	struct rte_eth_link *link = &dev->data->dev_link;
>  
> -	link->link_status = 1;
> +	link->link_status = ETH_LINK_UP;
>  	link->link_speed = ETH_SPEED_NUM_10G;
>  	link->link_duplex = ETH_LINK_FULL_DUPLEX;
>  
> diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
> index 94237610c..cc1a773a7 100644
> --- a/drivers/net/fm10k/fm10k_ethdev.c
> +++ b/drivers/net/fm10k/fm10k_ethdev.c
> @@ -1233,13 +1233,11 @@ fm10k_link_update(struct rte_eth_dev *dev,
>  		FM10K_DEV_PRIVATE_TO_INFO(dev->data->dev_private);
>  	PMD_INIT_FUNC_TRACE();
>  
> -	/* The speed is ~50Gbps per Gen3 x8 PCIe interface. For now, we
> -	 * leave the speed undefined since there is no 50Gbps Ethernet.
> -	 */
> -	dev->data->dev_link.link_speed  = 0;
> +	dev->data->dev_link.link_speed  = ETH_SPEED_NUM_50G;
>  	dev->data->dev_link.link_duplex = ETH_LINK_FULL_DUPLEX;
>  	dev->data->dev_link.link_status =
>  		dev_info->sm_down ? ETH_LINK_DOWN : ETH_LINK_UP;
> +	dev->data->dev_link.link_autoneg = ETH_LINK_FIXED;
>  
>  	return 0;
>  }
> diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
> index fd003fe01..c771edde5 100644
> --- a/drivers/net/i40e/i40e_ethdev_vf.c
> +++ b/drivers/net/i40e/i40e_ethdev_vf.c
> @@ -2107,7 +2107,7 @@ i40evf_dev_link_update(struct rte_eth_dev *dev,
>  	new_link.link_status = vf->link_up ? ETH_LINK_UP :
>  					     ETH_LINK_DOWN;
>  	new_link.link_autoneg =
> -		dev->data->dev_conf.link_speeds & ETH_LINK_SPEED_FIXED;
> +		!(dev->data->dev_conf.link_speeds & ETH_LINK_SPEED_FIXED);
>  
>  	i40evf_dev_atomic_write_link_status(dev, &new_link);
>  
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
> index 448325857..bad83968c 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -3957,7 +3957,7 @@ ixgbe_dev_link_update_share(struct rte_eth_dev *dev,
>  	bool autoneg = false;
>  
>  	link.link_status = ETH_LINK_DOWN;
> -	link.link_speed = 0;
> +	link.link_speed = ETH_SPEED_NUM_NONE;
>  	link.link_duplex = ETH_LINK_HALF_DUPLEX;
>  	link.link_autoneg = ETH_LINK_AUTONEG;
>  	memset(&old, 0, sizeof(old));
> diff --git a/drivers/net/mlx4/mlx4_ethdev.c b/drivers/net/mlx4/mlx4_ethdev.c
> index fbeef16c8..beecc53ba 100644
> --- a/drivers/net/mlx4/mlx4_ethdev.c
> +++ b/drivers/net/mlx4/mlx4_ethdev.c
> @@ -710,7 +710,7 @@ mlx4_link_update(struct rte_eth_dev *dev, int wait_to_complete)
>  	}
>  	link_speed = ethtool_cmd_speed(&edata);
>  	if (link_speed == -1)
> -		dev_link.link_speed = 0;
> +		dev_link.link_speed = ETH_SPEED_NUM_NONE;
>  	else
>  		dev_link.link_speed = link_speed;
>  	dev_link.link_duplex = ((edata.duplex == DUPLEX_HALF) ?
> diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
> index b73cb53df..de5576099 100644
> --- a/drivers/net/mlx5/mlx5_ethdev.c
> +++ b/drivers/net/mlx5/mlx5_ethdev.c
> @@ -536,7 +536,7 @@ mlx5_link_update_unlocked_gset(struct rte_eth_dev *dev, int wait_to_complete)
>  	}
>  	link_speed = ethtool_cmd_speed(&edata);
>  	if (link_speed == -1)
> -		dev_link.link_speed = 0;
> +		dev_link.link_speed = ETH_SPEED_NUM_NONE;
>  	else
>  		dev_link.link_speed = link_speed;
>  	priv->link_speed_capa = 0;
> -- 
> 2.13.6
 
Hi Ferruh,

On mlx5 this hunk is conflicts with my series [1].

Regards,

[1] https://dpdk.org/ml/archives/dev/2018-March/092495.html

-- 
Nélio Laranjeiro
6WIND

Re: [dpdk-dev] [PATCH] drivers/net: update link status

[Ferruh Yigit]

On 3/14/2018 8:14 AM, Nélio Laranjeiro wrote:
> On Tue, Mar 13, 2018 at 06:05:34PM +0000, Ferruh Yigit wrote:
>> Update link status related feature document items and minor updates in
>> some link status related functions.
>>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>

<...>

>> @@ -536,7 +536,7 @@ mlx5_link_update_unlocked_gset(struct rte_eth_dev *dev, int wait_to_complete)
>>  	}
>>  	link_speed = ethtool_cmd_speed(&edata);
>>  	if (link_speed == -1)
>> -		dev_link.link_speed = 0;
>> +		dev_link.link_speed = ETH_SPEED_NUM_NONE;
>>  	else
>>  		dev_link.link_speed = link_speed;
>>  	priv->link_speed_capa = 0;
>> -- 
>> 2.13.6
>  
> Hi Ferruh,
> 
> On mlx5 this hunk is conflicts with my series [1].

Thanks Nélio,

The mlx5 change is so minor I thinks conflict can be handled while applying.

> 
> Regards,
> 
> [1] https://dpdk.org/ml/archives/dev/2018-March/092495.html
> 

Re: [dpdk-dev] [PATCH] drivers/net: update link status

[Ferruh Yigit]

On 3/13/2018 6:05 PM, Ferruh Yigit wrote:
> Update link status related feature document items and minor updates in
> some link status related functions.
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
>  doc/guides/nics/features/fm10k.ini      | 2 ++
>  doc/guides/nics/features/fm10k_vf.ini   | 2 ++
>  doc/guides/nics/features/i40e_vf.ini    | 1 +
>  doc/guides/nics/features/igb_vf.ini     | 1 +
>  doc/guides/nics/features/qede.ini       | 1 -
>  doc/guides/nics/features/qede_vf.ini    | 1 -
>  doc/guides/nics/features/vhost.ini      | 2 --
>  doc/guides/nics/features/virtio_vec.ini | 1 +
>  drivers/net/e1000/em_ethdev.c           | 2 +-
>  drivers/net/ena/ena_ethdev.c            | 2 +-
>  drivers/net/fm10k/fm10k_ethdev.c        | 6 ++----
>  drivers/net/i40e/i40e_ethdev_vf.c       | 2 +-
>  drivers/net/ixgbe/ixgbe_ethdev.c        | 2 +-
>  drivers/net/mlx4/mlx4_ethdev.c          | 2 +-
>  drivers/net/mlx5/mlx5_ethdev.c          | 2 +-
>  15 files changed, 15 insertions(+), 14 deletions(-)

Reminder of this patch.

Mostly trivial updates, but there is defect fix for i40e.

Also feature list updated for vhost, virtio, qede, igb_vf, ixgbe_vf, fm10k_vf,
please check.

Thanks,
ferruh

Re: [dpdk-dev] [PATCH] drivers/net: update link status

[Tiwei Bie]

On Tue, Mar 13, 2018 at 06:05:34PM +0000, Ferruh Yigit wrote:
> Update link status related feature document items and minor updates in
> some link status related functions.
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
>  doc/guides/nics/features/fm10k.ini      | 2 ++
>  doc/guides/nics/features/fm10k_vf.ini   | 2 ++
>  doc/guides/nics/features/i40e_vf.ini    | 1 +
>  doc/guides/nics/features/igb_vf.ini     | 1 +
>  doc/guides/nics/features/qede.ini       | 1 -
>  doc/guides/nics/features/qede_vf.ini    | 1 -
>  doc/guides/nics/features/vhost.ini      | 2 --
>  doc/guides/nics/features/virtio_vec.ini | 1 +
>  drivers/net/e1000/em_ethdev.c           | 2 +-
>  drivers/net/ena/ena_ethdev.c            | 2 +-
>  drivers/net/fm10k/fm10k_ethdev.c        | 6 ++----
>  drivers/net/i40e/i40e_ethdev_vf.c       | 2 +-
>  drivers/net/ixgbe/ixgbe_ethdev.c        | 2 +-
>  drivers/net/mlx4/mlx4_ethdev.c          | 2 +-
>  drivers/net/mlx5/mlx5_ethdev.c          | 2 +-
>  15 files changed, 15 insertions(+), 14 deletions(-)
[...]
> diff --git a/doc/guides/nics/features/vhost.ini b/doc/guides/nics/features/vhost.ini
> index dffd1f493..31302745a 100644
> --- a/doc/guides/nics/features/vhost.ini
> +++ b/doc/guides/nics/features/vhost.ini
> @@ -4,8 +4,6 @@
>  ; Refer to default.ini for the full list of available PMD features.
>  ;
>  [Features]
> -Link status          = Y
> -Link status event    = Y

I think vhost PMD supports above features.

>  Free Tx mbuf on demand = Y
>  Queue status event   = Y
>  Basic stats          = Y
> diff --git a/doc/guides/nics/features/virtio_vec.ini b/doc/guides/nics/features/virtio_vec.ini
> index c06c860d4..e60fe36ae 100644
> --- a/doc/guides/nics/features/virtio_vec.ini
> +++ b/doc/guides/nics/features/virtio_vec.ini

The doc/guides/nics/features/virtio.ini also needs to be updated.

Thanks for work!

> @@ -6,6 +6,7 @@
>  [Features]
>  Speed capabilities   = P
>  Link status          = Y
> +Link status event    = Y
>  Rx interrupt         = Y
>  Queue start/stop     = Y
>  Promiscuous mode     = Y
[...]

Re: [dpdk-dev] [PATCH] drivers/net: update link status

[Ferruh Yigit]

On 4/10/2018 4:41 PM, Tiwei Bie wrote:
> On Tue, Mar 13, 2018 at 06:05:34PM +0000, Ferruh Yigit wrote:
>> Update link status related feature document items and minor updates in
>> some link status related functions.
>>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>> ---
>>  doc/guides/nics/features/fm10k.ini      | 2 ++
>>  doc/guides/nics/features/fm10k_vf.ini   | 2 ++
>>  doc/guides/nics/features/i40e_vf.ini    | 1 +
>>  doc/guides/nics/features/igb_vf.ini     | 1 +
>>  doc/guides/nics/features/qede.ini       | 1 -
>>  doc/guides/nics/features/qede_vf.ini    | 1 -
>>  doc/guides/nics/features/vhost.ini      | 2 --
>>  doc/guides/nics/features/virtio_vec.ini | 1 +
>>  drivers/net/e1000/em_ethdev.c           | 2 +-
>>  drivers/net/ena/ena_ethdev.c            | 2 +-
>>  drivers/net/fm10k/fm10k_ethdev.c        | 6 ++----
>>  drivers/net/i40e/i40e_ethdev_vf.c       | 2 +-
>>  drivers/net/ixgbe/ixgbe_ethdev.c        | 2 +-
>>  drivers/net/mlx4/mlx4_ethdev.c          | 2 +-
>>  drivers/net/mlx5/mlx5_ethdev.c          | 2 +-
>>  15 files changed, 15 insertions(+), 14 deletions(-)
> [...]
>> diff --git a/doc/guides/nics/features/vhost.ini b/doc/guides/nics/features/vhost.ini
>> index dffd1f493..31302745a 100644
>> --- a/doc/guides/nics/features/vhost.ini
>> +++ b/doc/guides/nics/features/vhost.ini
>> @@ -4,8 +4,6 @@
>>  ; Refer to default.ini for the full list of available PMD features.
>>  ;
>>  [Features]
>> -Link status          = Y
>> -Link status event    = Y
> 
> I think vhost PMD supports above features.

I am not able to find where it is supported.

Some virtual PMDs report fixed link, with empty link_update() dev_ops, and they
are not reported as supporting Link status, as far as I can see vhost also one
of them.

And for Link status event, PMD needs to support LSC interrupts and should
register interrupt handler for it, which I can't find for vhost.

I will send next version without updating above one, please point me where these
support added if I missed them.

> 
>>  Free Tx mbuf on demand = Y
>>  Queue status event   = Y
>>  Basic stats          = Y
>> diff --git a/doc/guides/nics/features/virtio_vec.ini b/doc/guides/nics/features/virtio_vec.ini
>> index c06c860d4..e60fe36ae 100644
>> --- a/doc/guides/nics/features/virtio_vec.ini
>> +++ b/doc/guides/nics/features/virtio_vec.ini
> 
> The doc/guides/nics/features/virtio.ini also needs to be updated.

Done.

> 
> Thanks for work!
> 
>> @@ -6,6 +6,7 @@
>>  [Features]
>>  Speed capabilities   = P
>>  Link status          = Y
>> +Link status event    = Y
>>  Rx interrupt         = Y
>>  Queue start/stop     = Y
>>  Promiscuous mode     = Y
> [...]
> 

[dpdk-dev] [PATCH v2] drivers/net: update link status

[Ferruh Yigit]

Update link status related feature document items and minor updates in
some link status related functions.

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
Cc: Nélio Laranjeiro <nelio.laranjeiro@6wind.com>
Cc: Tiwei Bie <tiwei.bie@intel.com>
Cc: Qi Z Zhang <qi.z.zhang@intel.com>

v2:
* update virtio.ini (Tiwei)
* update i40e_vf_vec.ini (Qi)
* update avf.ini avf_vec.ini
---
 doc/guides/nics/features/avf.ini         | 1 -
 doc/guides/nics/features/avf_vec.ini     | 1 -
 doc/guides/nics/features/fm10k.ini       | 2 ++
 doc/guides/nics/features/fm10k_vf.ini    | 2 ++
 doc/guides/nics/features/i40e_vf.ini     | 1 +
 doc/guides/nics/features/i40e_vf_vec.ini | 1 +
 doc/guides/nics/features/igb_vf.ini      | 1 +
 doc/guides/nics/features/qede.ini        | 1 -
 doc/guides/nics/features/qede_vf.ini     | 1 -
 doc/guides/nics/features/vhost.ini       | 2 --
 doc/guides/nics/features/virtio.ini      | 1 +
 doc/guides/nics/features/virtio_vec.ini  | 1 +
 drivers/net/e1000/em_ethdev.c            | 2 +-
 drivers/net/ena/ena_ethdev.c             | 2 +-
 drivers/net/fm10k/fm10k_ethdev.c         | 6 ++----
 drivers/net/i40e/i40e_ethdev_vf.c        | 2 +-
 drivers/net/ixgbe/ixgbe_ethdev.c         | 2 +-
 drivers/net/mlx4/mlx4_ethdev.c           | 2 +-
 drivers/net/mlx5/mlx5_ethdev.c           | 2 +-
 19 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/doc/guides/nics/features/avf.ini b/doc/guides/nics/features/avf.ini
index ccb9edde3..35ceada24 100644
--- a/doc/guides/nics/features/avf.ini
+++ b/doc/guides/nics/features/avf.ini
@@ -6,7 +6,6 @@
 [Features]
 Speed capabilities   = Y
 Link status          = Y
-Link status event    = Y
 Rx interrupt         = Y
 Queue start/stop     = Y
 MTU update           = Y
diff --git a/doc/guides/nics/features/avf_vec.ini b/doc/guides/nics/features/avf_vec.ini
index 892499485..3050bc4a6 100644
--- a/doc/guides/nics/features/avf_vec.ini
+++ b/doc/guides/nics/features/avf_vec.ini
@@ -6,7 +6,6 @@
 [Features]
 Speed capabilities   = Y
 Link status          = Y
-Link status event    = Y
 Rx interrupt         = Y
 Queue start/stop     = Y
 MTU update           = Y
diff --git a/doc/guides/nics/features/fm10k.ini b/doc/guides/nics/features/fm10k.ini
index f0f61a7d7..58f58b99c 100644
--- a/doc/guides/nics/features/fm10k.ini
+++ b/doc/guides/nics/features/fm10k.ini
@@ -5,6 +5,8 @@
 ;
 [Features]
 Speed capabilities   = P
+Link status          = Y
+Link status event    = Y
 Rx interrupt         = Y
 Queue start/stop     = Y
 Jumbo frame          = Y
diff --git a/doc/guides/nics/features/fm10k_vf.ini b/doc/guides/nics/features/fm10k_vf.ini
index 32b93df4b..44b50faa1 100644
--- a/doc/guides/nics/features/fm10k_vf.ini
+++ b/doc/guides/nics/features/fm10k_vf.ini
@@ -5,6 +5,8 @@
 ;
 [Features]
 Speed capabilities   = P
+Link status          = Y
+Link status event    = Y
 Rx interrupt         = Y
 Queue start/stop     = Y
 Jumbo frame          = Y
diff --git a/doc/guides/nics/features/i40e_vf.ini b/doc/guides/nics/features/i40e_vf.ini
index 46e0d9fce..ba2d8cbe9 100644
--- a/doc/guides/nics/features/i40e_vf.ini
+++ b/doc/guides/nics/features/i40e_vf.ini
@@ -5,6 +5,7 @@
 ;
 [Features]
 Rx interrupt         = Y
+Link status          = Y
 Queue start/stop     = Y
 Jumbo frame          = Y
 Scattered Rx         = Y
diff --git a/doc/guides/nics/features/i40e_vf_vec.ini b/doc/guides/nics/features/i40e_vf_vec.ini
index c2c6c19fe..421ed9193 100644
--- a/doc/guides/nics/features/i40e_vf_vec.ini
+++ b/doc/guides/nics/features/i40e_vf_vec.ini
@@ -5,6 +5,7 @@
 ;
 [Features]
 Rx interrupt         = Y
+Link status          = Y
 Queue start/stop     = Y
 Jumbo frame          = Y
 Scattered Rx         = Y
diff --git a/doc/guides/nics/features/igb_vf.ini b/doc/guides/nics/features/igb_vf.ini
index e641a2c97..d9653234b 100644
--- a/doc/guides/nics/features/igb_vf.ini
+++ b/doc/guides/nics/features/igb_vf.ini
@@ -4,6 +4,7 @@
 ; Refer to default.ini for the full list of available PMD features.
 ;
 [Features]
+Link status          = Y
 Rx interrupt         = Y
 Scattered Rx         = Y
 TSO                  = Y
diff --git a/doc/guides/nics/features/qede.ini b/doc/guides/nics/features/qede.ini
index 605cfa968..2d6af365e 100644
--- a/doc/guides/nics/features/qede.ini
+++ b/doc/guides/nics/features/qede.ini
@@ -6,7 +6,6 @@
 [Features]
 Speed capabilities   = Y
 Link status          = Y
-Link status event    = Y
 MTU update           = Y
 Jumbo frame          = Y
 Scattered Rx         = Y
diff --git a/doc/guides/nics/features/qede_vf.ini b/doc/guides/nics/features/qede_vf.ini
index 18857b6e3..70071a1bd 100644
--- a/doc/guides/nics/features/qede_vf.ini
+++ b/doc/guides/nics/features/qede_vf.ini
@@ -6,7 +6,6 @@
 [Features]
 Speed capabilities   = Y
 Link status          = Y
-Link status event    = Y
 MTU update           = Y
 Jumbo frame          = Y
 Scattered Rx         = Y
diff --git a/doc/guides/nics/features/vhost.ini b/doc/guides/nics/features/vhost.ini
index dffd1f493..31302745a 100644
--- a/doc/guides/nics/features/vhost.ini
+++ b/doc/guides/nics/features/vhost.ini
@@ -4,8 +4,6 @@
 ; Refer to default.ini for the full list of available PMD features.
 ;
 [Features]
-Link status          = Y
-Link status event    = Y
 Free Tx mbuf on demand = Y
 Queue status event   = Y
 Basic stats          = Y
diff --git a/doc/guides/nics/features/virtio.ini b/doc/guides/nics/features/virtio.ini
index 16e577df0..a16b81721 100644
--- a/doc/guides/nics/features/virtio.ini
+++ b/doc/guides/nics/features/virtio.ini
@@ -6,6 +6,7 @@
 [Features]
 Speed capabilities   = P
 Link status          = Y
+Link status event    = Y
 Rx interrupt         = Y
 Queue start/stop     = Y
 Scattered Rx         = Y
diff --git a/doc/guides/nics/features/virtio_vec.ini b/doc/guides/nics/features/virtio_vec.ini
index c06c860d4..e60fe36ae 100644
--- a/doc/guides/nics/features/virtio_vec.ini
+++ b/doc/guides/nics/features/virtio_vec.ini
@@ -6,6 +6,7 @@
 [Features]
 Speed capabilities   = P
 Link status          = Y
+Link status event    = Y
 Rx interrupt         = Y
 Queue start/stop     = Y
 Promiscuous mode     = Y
diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
index de7db2650..694a6242c 100644
--- a/drivers/net/e1000/em_ethdev.c
+++ b/drivers/net/e1000/em_ethdev.c
@@ -1183,7 +1183,7 @@ eth_em_link_update(struct rte_eth_dev *dev, int wait_to_complete)
 		link.link_autoneg = !(dev->data->dev_conf.link_speeds &
 				ETH_LINK_SPEED_FIXED);
 	} else if (!link_check && (link.link_status == ETH_LINK_UP)) {
-		link.link_speed = 0;
+		link.link_speed = ETH_SPEED_NUM_NONE;
 		link.link_duplex = ETH_LINK_HALF_DUPLEX;
 		link.link_status = ETH_LINK_DOWN;
 		link.link_autoneg = ETH_LINK_FIXED;
diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index ab4e2af91..41b5638fd 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -728,7 +728,7 @@ static int ena_link_update(struct rte_eth_dev *dev,
 {
 	struct rte_eth_link *link = &dev->data->dev_link;
 
-	link->link_status = 1;
+	link->link_status = ETH_LINK_UP;
 	link->link_speed = ETH_SPEED_NUM_10G;
 	link->link_duplex = ETH_LINK_FULL_DUPLEX;
 
diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index 34affd1cc..7dfeddfe0 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -1260,13 +1260,11 @@ fm10k_link_update(struct rte_eth_dev *dev,
 		FM10K_DEV_PRIVATE_TO_INFO(dev->data->dev_private);
 	PMD_INIT_FUNC_TRACE();
 
-	/* The speed is ~50Gbps per Gen3 x8 PCIe interface. For now, we
-	 * leave the speed undefined since there is no 50Gbps Ethernet.
-	 */
-	dev->data->dev_link.link_speed  = 0;
+	dev->data->dev_link.link_speed  = ETH_SPEED_NUM_50G;
 	dev->data->dev_link.link_duplex = ETH_LINK_FULL_DUPLEX;
 	dev->data->dev_link.link_status =
 		dev_info->sm_down ? ETH_LINK_DOWN : ETH_LINK_UP;
+	dev->data->dev_link.link_autoneg = ETH_LINK_FIXED;
 
 	return 0;
 }
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index 031c70680..48e7ac21e 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -2112,7 +2112,7 @@ i40evf_dev_link_update(struct rte_eth_dev *dev,
 	new_link.link_status = vf->link_up ? ETH_LINK_UP :
 					     ETH_LINK_DOWN;
 	new_link.link_autoneg =
-		dev->data->dev_conf.link_speeds & ETH_LINK_SPEED_FIXED;
+		!(dev->data->dev_conf.link_speeds & ETH_LINK_SPEED_FIXED);
 
 	return rte_eth_linkstatus_set(dev, &new_link);
 }
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index a5e2fc0ca..c401250e9 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -3900,7 +3900,7 @@ ixgbe_dev_link_update_share(struct rte_eth_dev *dev,
 
 	memset(&link, 0, sizeof(link));
 	link.link_status = ETH_LINK_DOWN;
-	link.link_speed = 0;
+	link.link_speed = ETH_SPEED_NUM_NONE;
 	link.link_duplex = ETH_LINK_HALF_DUPLEX;
 	link.link_autoneg = ETH_LINK_AUTONEG;
 
diff --git a/drivers/net/mlx4/mlx4_ethdev.c b/drivers/net/mlx4/mlx4_ethdev.c
index d1b71c805..9a76670d8 100644
--- a/drivers/net/mlx4/mlx4_ethdev.c
+++ b/drivers/net/mlx4/mlx4_ethdev.c
@@ -713,7 +713,7 @@ mlx4_link_update(struct rte_eth_dev *dev, int wait_to_complete)
 	}
 	link_speed = ethtool_cmd_speed(&edata);
 	if (link_speed == -1)
-		dev_link.link_speed = 0;
+		dev_link.link_speed = ETH_SPEED_NUM_NONE;
 	else
 		dev_link.link_speed = link_speed;
 	dev_link.link_duplex = ((edata.duplex == DUPLEX_HALF) ?
diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
index ef44cc91f..59d6a805f 100644
--- a/drivers/net/mlx5/mlx5_ethdev.c
+++ b/drivers/net/mlx5/mlx5_ethdev.c
@@ -539,7 +539,7 @@ mlx5_link_update_unlocked_gset(struct rte_eth_dev *dev,
 	}
 	link_speed = ethtool_cmd_speed(&edata);
 	if (link_speed == -1)
-		dev_link.link_speed = 0;
+		dev_link.link_speed = ETH_SPEED_NUM_NONE;
 	else
 		dev_link.link_speed = link_speed;
 	priv->link_speed_capa = 0;
-- 
2.14.3

Re: [dpdk-dev] [PATCH] drivers/net: update link status

[Tiwei Bie]

On Fri, Apr 13, 2018 at 10:53:55PM +0100, Ferruh Yigit wrote:
> On 4/10/2018 4:41 PM, Tiwei Bie wrote:
> > On Tue, Mar 13, 2018 at 06:05:34PM +0000, Ferruh Yigit wrote:
> >> Update link status related feature document items and minor updates in
> >> some link status related functions.
> >>
> >> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> >> ---
> >>  doc/guides/nics/features/fm10k.ini      | 2 ++
> >>  doc/guides/nics/features/fm10k_vf.ini   | 2 ++
> >>  doc/guides/nics/features/i40e_vf.ini    | 1 +
> >>  doc/guides/nics/features/igb_vf.ini     | 1 +
> >>  doc/guides/nics/features/qede.ini       | 1 -
> >>  doc/guides/nics/features/qede_vf.ini    | 1 -
> >>  doc/guides/nics/features/vhost.ini      | 2 --
> >>  doc/guides/nics/features/virtio_vec.ini | 1 +
> >>  drivers/net/e1000/em_ethdev.c           | 2 +-
> >>  drivers/net/ena/ena_ethdev.c            | 2 +-
> >>  drivers/net/fm10k/fm10k_ethdev.c        | 6 ++----
> >>  drivers/net/i40e/i40e_ethdev_vf.c       | 2 +-
> >>  drivers/net/ixgbe/ixgbe_ethdev.c        | 2 +-
> >>  drivers/net/mlx4/mlx4_ethdev.c          | 2 +-
> >>  drivers/net/mlx5/mlx5_ethdev.c          | 2 +-
> >>  15 files changed, 15 insertions(+), 14 deletions(-)
> > [...]
> >> diff --git a/doc/guides/nics/features/vhost.ini b/doc/guides/nics/features/vhost.ini
> >> index dffd1f493..31302745a 100644
> >> --- a/doc/guides/nics/features/vhost.ini
> >> +++ b/doc/guides/nics/features/vhost.ini
> >> @@ -4,8 +4,6 @@
> >>  ; Refer to default.ini for the full list of available PMD features.
> >>  ;
> >>  [Features]
> >> -Link status          = Y
> >> -Link status event    = Y
> > 
> > I think vhost PMD supports above features.
> 
> I am not able to find where it is supported.
> 
> Some virtual PMDs report fixed link, with empty link_update() dev_ops, and they
> are not reported as supporting Link status, as far as I can see vhost also one
> of them.
> 
> And for Link status event, PMD needs to support LSC interrupts and should
> register interrupt handler for it, which I can't find for vhost.
> 
> I will send next version without updating above one, please point me where these
> support added if I missed them.

In drivers/net/vhost/rte_eth_vhost.c you could find below functions:

static int
new_device(int vid)
{
	......

	eth_dev->data->dev_link.link_status = ETH_LINK_UP;

	......

	_rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_INTR_LSC, NULL);

	......
}

static void
destroy_device(int vid)
{
	......

	eth_dev->data->dev_link.link_status = ETH_LINK_DOWN;

	......

	_rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_INTR_LSC, NULL);

	......
}

They are the callbacks for vhost library.

When a frontend (e.g. QEMU) is connected to this vhost backend
and the frontend virtio device becomes ready, new_device() will
be called by the vhost library, and the link status will be
updated to UP.

And when e.g. the connection is closed, destroy_device() will be
called by the vhost library, and the link status will be updated
to DOWN.

So vhost PMD reports meaningful link status and also generates
link status events.

Thanks

Re: [dpdk-dev] [PATCH v2] drivers/net: update link status

[Adrien Mazarguil]

On Fri, Apr 13, 2018 at 11:02:04PM +0100, Ferruh Yigit wrote:
> Update link status related feature document items and minor updates in
> some link status related functions.
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
> Cc: Nélio Laranjeiro <nelio.laranjeiro@6wind.com>
> Cc: Tiwei Bie <tiwei.bie@intel.com>
> Cc: Qi Z Zhang <qi.z.zhang@intel.com>
> 
> v2:
> * update virtio.ini (Tiwei)
> * update i40e_vf_vec.ini (Qi)
> * update avf.ini avf_vec.ini

For mlx4 and mlx5:

Acked-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>

-- 
Adrien Mazarguil
6WIND

Re: [dpdk-dev] [PATCH] drivers/net: update link status

[Ferruh Yigit]

On 4/14/2018 11:55 AM, Tiwei Bie wrote:
> On Fri, Apr 13, 2018 at 10:53:55PM +0100, Ferruh Yigit wrote:
>> On 4/10/2018 4:41 PM, Tiwei Bie wrote:
>>> On Tue, Mar 13, 2018 at 06:05:34PM +0000, Ferruh Yigit wrote:
>>>> Update link status related feature document items and minor updates in
>>>> some link status related functions.
>>>>
>>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>>>> ---
>>>>  doc/guides/nics/features/fm10k.ini      | 2 ++
>>>>  doc/guides/nics/features/fm10k_vf.ini   | 2 ++
>>>>  doc/guides/nics/features/i40e_vf.ini    | 1 +
>>>>  doc/guides/nics/features/igb_vf.ini     | 1 +
>>>>  doc/guides/nics/features/qede.ini       | 1 -
>>>>  doc/guides/nics/features/qede_vf.ini    | 1 -
>>>>  doc/guides/nics/features/vhost.ini      | 2 --
>>>>  doc/guides/nics/features/virtio_vec.ini | 1 +
>>>>  drivers/net/e1000/em_ethdev.c           | 2 +-
>>>>  drivers/net/ena/ena_ethdev.c            | 2 +-
>>>>  drivers/net/fm10k/fm10k_ethdev.c        | 6 ++----
>>>>  drivers/net/i40e/i40e_ethdev_vf.c       | 2 +-
>>>>  drivers/net/ixgbe/ixgbe_ethdev.c        | 2 +-
>>>>  drivers/net/mlx4/mlx4_ethdev.c          | 2 +-
>>>>  drivers/net/mlx5/mlx5_ethdev.c          | 2 +-
>>>>  15 files changed, 15 insertions(+), 14 deletions(-)
>>> [...]
>>>> diff --git a/doc/guides/nics/features/vhost.ini b/doc/guides/nics/features/vhost.ini
>>>> index dffd1f493..31302745a 100644
>>>> --- a/doc/guides/nics/features/vhost.ini
>>>> +++ b/doc/guides/nics/features/vhost.ini
>>>> @@ -4,8 +4,6 @@
>>>>  ; Refer to default.ini for the full list of available PMD features.
>>>>  ;
>>>>  [Features]
>>>> -Link status          = Y
>>>> -Link status event    = Y
>>>
>>> I think vhost PMD supports above features.
>>
>> I am not able to find where it is supported.
>>
>> Some virtual PMDs report fixed link, with empty link_update() dev_ops, and they
>> are not reported as supporting Link status, as far as I can see vhost also one
>> of them.
>>
>> And for Link status event, PMD needs to support LSC interrupts and should
>> register interrupt handler for it, which I can't find for vhost.
>>
>> I will send next version without updating above one, please point me where these
>> support added if I missed them.
> 
> In drivers/net/vhost/rte_eth_vhost.c you could find below functions:
> 
> static int
> new_device(int vid)
> {
> 	......
> 
> 	eth_dev->data->dev_link.link_status = ETH_LINK_UP;
> 
> 	......
> 
> 	_rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_INTR_LSC, NULL);
> 
> 	......
> }
> 
> static void
> destroy_device(int vid)
> {
> 	......
> 
> 	eth_dev->data->dev_link.link_status = ETH_LINK_DOWN;
> 
> 	......
> 
> 	_rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_INTR_LSC, NULL);
> 
> 	......
> }
> 
> They are the callbacks for vhost library.
> 
> When a frontend (e.g. QEMU) is connected to this vhost backend
> and the frontend virtio device becomes ready, new_device() will
> be called by the vhost library, and the link status will be
> updated to UP.
> 
> And when e.g. the connection is closed, destroy_device() will be
> called by the vhost library, and the link status will be updated
> to DOWN.


Got it. This behavior is similar for virtual PMDs. Provide static link
information and update link as UP during start and update it as DOWN during stop.

Other virtual PMDs doesn't report this feature, so removed from vhost as well
for consistency.

> 
> So vhost PMD reports meaningful link status and also generates
> link status events.

Yes PMD process user callbacks on link change [1], but I am not sure that is
what meant from "link status event", what I understand is link interrupts
supported in PMD level which seems not the case for vhost.

[1]
This is something else but why calling user callback in link update is in PMD
discretion, shouldn't it be something done automatically in ethdev layer, somehow.

> 
> Thanks
> 

Re: [dpdk-dev] [PATCH] drivers/net: update link status

[Tiwei Bie]

On Mon, Apr 16, 2018 at 05:10:24PM +0100, Ferruh Yigit wrote:
> On 4/14/2018 11:55 AM, Tiwei Bie wrote:
> > On Fri, Apr 13, 2018 at 10:53:55PM +0100, Ferruh Yigit wrote:
> >> On 4/10/2018 4:41 PM, Tiwei Bie wrote:
> >>> On Tue, Mar 13, 2018 at 06:05:34PM +0000, Ferruh Yigit wrote:
> >>>> Update link status related feature document items and minor updates in
> >>>> some link status related functions.
> >>>>
> >>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> >>>> ---
> >>>>  doc/guides/nics/features/fm10k.ini      | 2 ++
> >>>>  doc/guides/nics/features/fm10k_vf.ini   | 2 ++
> >>>>  doc/guides/nics/features/i40e_vf.ini    | 1 +
> >>>>  doc/guides/nics/features/igb_vf.ini     | 1 +
> >>>>  doc/guides/nics/features/qede.ini       | 1 -
> >>>>  doc/guides/nics/features/qede_vf.ini    | 1 -
> >>>>  doc/guides/nics/features/vhost.ini      | 2 --
> >>>>  doc/guides/nics/features/virtio_vec.ini | 1 +
> >>>>  drivers/net/e1000/em_ethdev.c           | 2 +-
> >>>>  drivers/net/ena/ena_ethdev.c            | 2 +-
> >>>>  drivers/net/fm10k/fm10k_ethdev.c        | 6 ++----
> >>>>  drivers/net/i40e/i40e_ethdev_vf.c       | 2 +-
> >>>>  drivers/net/ixgbe/ixgbe_ethdev.c        | 2 +-
> >>>>  drivers/net/mlx4/mlx4_ethdev.c          | 2 +-
> >>>>  drivers/net/mlx5/mlx5_ethdev.c          | 2 +-
> >>>>  15 files changed, 15 insertions(+), 14 deletions(-)
> >>> [...]
> >>>> diff --git a/doc/guides/nics/features/vhost.ini b/doc/guides/nics/features/vhost.ini
> >>>> index dffd1f493..31302745a 100644
> >>>> --- a/doc/guides/nics/features/vhost.ini
> >>>> +++ b/doc/guides/nics/features/vhost.ini
> >>>> @@ -4,8 +4,6 @@
> >>>>  ; Refer to default.ini for the full list of available PMD features.
> >>>>  ;
> >>>>  [Features]
> >>>> -Link status          = Y
> >>>> -Link status event    = Y
> >>>
> >>> I think vhost PMD supports above features.
> >>
> >> I am not able to find where it is supported.
> >>
> >> Some virtual PMDs report fixed link, with empty link_update() dev_ops, and they
> >> are not reported as supporting Link status, as far as I can see vhost also one
> >> of them.
> >>
> >> And for Link status event, PMD needs to support LSC interrupts and should
> >> register interrupt handler for it, which I can't find for vhost.
> >>
> >> I will send next version without updating above one, please point me where these
> >> support added if I missed them.
> > 
> > In drivers/net/vhost/rte_eth_vhost.c you could find below functions:
> > 
> > static int
> > new_device(int vid)
> > {
> > 	......
> > 
> > 	eth_dev->data->dev_link.link_status = ETH_LINK_UP;
> > 
> > 	......
> > 
> > 	_rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_INTR_LSC, NULL);
> > 
> > 	......
> > }
> > 
> > static void
> > destroy_device(int vid)
> > {
> > 	......
> > 
> > 	eth_dev->data->dev_link.link_status = ETH_LINK_DOWN;
> > 
> > 	......
> > 
> > 	_rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_INTR_LSC, NULL);
> > 
> > 	......
> > }
> > 
> > They are the callbacks for vhost library.
> > 
> > When a frontend (e.g. QEMU) is connected to this vhost backend
> > and the frontend virtio device becomes ready, new_device() will
> > be called by the vhost library, and the link status will be
> > updated to UP.
> > 
> > And when e.g. the connection is closed, destroy_device() will be
> > called by the vhost library, and the link status will be updated
> > to DOWN.
> 
> 
> Got it. This behavior is similar for virtual PMDs. Provide static link
> information and update link as UP during start and update it as DOWN during stop.

No, the link status isn't updated during vhost PMD start
and stop. When the vhost PMD has been started, the link
status still may be DOWN. The link status becomes UP only
when the QEMU (it's another virtual machine process which
has a virtio device) connects to this vhost PMD via a UNIX
socket and the virtio driver in the virtual machine has
setup the virtio device of the virtual machine.

So if vhost PMD reports the link status as DOWN, it means
there is no QEMU (virtual machine) connects to it or the
virtio device in the virtual machine hasn't been setup.
(PS. The frontend can also be virtio-user PMD besides QEMU)

Thanks

> 
> Other virtual PMDs doesn't report this feature, so removed from vhost as well
> for consistency.
> 
> > 
> > So vhost PMD reports meaningful link status and also generates
> > link status events.
> 
> Yes PMD process user callbacks on link change [1], but I am not sure that is
> what meant from "link status event", what I understand is link interrupts
> supported in PMD level which seems not the case for vhost.
> 
> [1]
> This is something else but why calling user callback in link update is in PMD
> discretion, shouldn't it be something done automatically in ethdev layer, somehow.
> 
> > 
> > Thanks
> > 
> 

Re: [dpdk-dev] [PATCH] drivers/net: update link status

[Ferruh Yigit]

On 4/17/2018 5:54 AM, Tiwei Bie wrote:
> On Mon, Apr 16, 2018 at 05:10:24PM +0100, Ferruh Yigit wrote:
>> On 4/14/2018 11:55 AM, Tiwei Bie wrote:
>>> On Fri, Apr 13, 2018 at 10:53:55PM +0100, Ferruh Yigit wrote:
>>>> On 4/10/2018 4:41 PM, Tiwei Bie wrote:
>>>>> On Tue, Mar 13, 2018 at 06:05:34PM +0000, Ferruh Yigit wrote:
>>>>>> Update link status related feature document items and minor updates in
>>>>>> some link status related functions.
>>>>>>
>>>>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>>>>>> ---
>>>>>>  doc/guides/nics/features/fm10k.ini      | 2 ++
>>>>>>  doc/guides/nics/features/fm10k_vf.ini   | 2 ++
>>>>>>  doc/guides/nics/features/i40e_vf.ini    | 1 +
>>>>>>  doc/guides/nics/features/igb_vf.ini     | 1 +
>>>>>>  doc/guides/nics/features/qede.ini       | 1 -
>>>>>>  doc/guides/nics/features/qede_vf.ini    | 1 -
>>>>>>  doc/guides/nics/features/vhost.ini      | 2 --
>>>>>>  doc/guides/nics/features/virtio_vec.ini | 1 +
>>>>>>  drivers/net/e1000/em_ethdev.c           | 2 +-
>>>>>>  drivers/net/ena/ena_ethdev.c            | 2 +-
>>>>>>  drivers/net/fm10k/fm10k_ethdev.c        | 6 ++----
>>>>>>  drivers/net/i40e/i40e_ethdev_vf.c       | 2 +-
>>>>>>  drivers/net/ixgbe/ixgbe_ethdev.c        | 2 +-
>>>>>>  drivers/net/mlx4/mlx4_ethdev.c          | 2 +-
>>>>>>  drivers/net/mlx5/mlx5_ethdev.c          | 2 +-
>>>>>>  15 files changed, 15 insertions(+), 14 deletions(-)
>>>>> [...]
>>>>>> diff --git a/doc/guides/nics/features/vhost.ini b/doc/guides/nics/features/vhost.ini
>>>>>> index dffd1f493..31302745a 100644
>>>>>> --- a/doc/guides/nics/features/vhost.ini
>>>>>> +++ b/doc/guides/nics/features/vhost.ini
>>>>>> @@ -4,8 +4,6 @@
>>>>>>  ; Refer to default.ini for the full list of available PMD features.
>>>>>>  ;
>>>>>>  [Features]
>>>>>> -Link status          = Y
>>>>>> -Link status event    = Y
>>>>>
>>>>> I think vhost PMD supports above features.
>>>>
>>>> I am not able to find where it is supported.
>>>>
>>>> Some virtual PMDs report fixed link, with empty link_update() dev_ops, and they
>>>> are not reported as supporting Link status, as far as I can see vhost also one
>>>> of them.
>>>>
>>>> And for Link status event, PMD needs to support LSC interrupts and should
>>>> register interrupt handler for it, which I can't find for vhost.
>>>>
>>>> I will send next version without updating above one, please point me where these
>>>> support added if I missed them.
>>>
>>> In drivers/net/vhost/rte_eth_vhost.c you could find below functions:
>>>
>>> static int
>>> new_device(int vid)
>>> {
>>> 	......
>>>
>>> 	eth_dev->data->dev_link.link_status = ETH_LINK_UP;
>>>
>>> 	......
>>>
>>> 	_rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_INTR_LSC, NULL);
>>>
>>> 	......
>>> }
>>>
>>> static void
>>> destroy_device(int vid)
>>> {
>>> 	......
>>>
>>> 	eth_dev->data->dev_link.link_status = ETH_LINK_DOWN;
>>>
>>> 	......
>>>
>>> 	_rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_INTR_LSC, NULL);
>>>
>>> 	......
>>> }
>>>
>>> They are the callbacks for vhost library.
>>>
>>> When a frontend (e.g. QEMU) is connected to this vhost backend
>>> and the frontend virtio device becomes ready, new_device() will
>>> be called by the vhost library, and the link status will be
>>> updated to UP.
>>>
>>> And when e.g. the connection is closed, destroy_device() will be
>>> called by the vhost library, and the link status will be updated
>>> to DOWN.
>>
>>
>> Got it. This behavior is similar for virtual PMDs. Provide static link
>> information and update link as UP during start and update it as DOWN during stop.
> 
> No, the link status isn't updated during vhost PMD start
> and stop. When the vhost PMD has been started, the link
> status still may be DOWN. The link status becomes UP only
> when the QEMU (it's another virtual machine process which
> has a virtio device) connects to this vhost PMD via a UNIX
> socket and the virtio driver in the virtual machine has
> setup the virtio device of the virtual machine.
> 
> So if vhost PMD reports the link status as DOWN, it means
> there is no QEMU (virtual machine) connects to it or the
> virtio device in the virtual machine hasn't been setup.
> (PS. The frontend can also be virtio-user PMD besides QEMU)

I believe announcing link feature reporting on virtual pmds still in gray area,
but because of qemu involvement in vhost case, I will keep link feature but will
drop link event.

Will send a new patch to reflect this.

Thanks,
ferruh

> 
> Thanks
> 
>>
>> Other virtual PMDs doesn't report this feature, so removed from vhost as well
>> for consistency.
>>
>>>
>>> So vhost PMD reports meaningful link status and also generates
>>> link status events.
>>
>> Yes PMD process user callbacks on link change [1], but I am not sure that is
>> what meant from "link status event", what I understand is link interrupts
>> supported in PMD level which seems not the case for vhost.
>>
>> [1]
>> This is something else but why calling user callback in link update is in PMD
>> discretion, shouldn't it be something done automatically in ethdev layer, somehow.
>>
>>>
>>> Thanks
>>>
>>

[dpdk-dev] [PATCH v3] drivers/net: update link status

[Ferruh Yigit]

Update link status related feature document items and minor updates in
some link status related functions.

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
Acked-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
---
Cc: Nélio Laranjeiro <nelio.laranjeiro@6wind.com>
Cc: Tiwei Bie <tiwei.bie@intel.com>
Cc: Qi Z Zhang <qi.z.zhang@intel.com>

v2:
* update virtio.ini (Tiwei)
* update i40e_vf_vec.ini (Qi)
* update avf.ini avf_vec.ini

v3:
* Put "Link status" back for vhost (Tiwei)
---
 doc/guides/nics/features/avf.ini         | 1 -
 doc/guides/nics/features/avf_vec.ini     | 1 -
 doc/guides/nics/features/fm10k.ini       | 2 ++
 doc/guides/nics/features/fm10k_vf.ini    | 2 ++
 doc/guides/nics/features/i40e_vf.ini     | 1 +
 doc/guides/nics/features/i40e_vf_vec.ini | 1 +
 doc/guides/nics/features/igb_vf.ini      | 1 +
 doc/guides/nics/features/qede.ini        | 1 -
 doc/guides/nics/features/qede_vf.ini     | 1 -
 doc/guides/nics/features/vhost.ini       | 1 -
 doc/guides/nics/features/virtio.ini      | 1 +
 doc/guides/nics/features/virtio_vec.ini  | 1 +
 drivers/net/e1000/em_ethdev.c            | 2 +-
 drivers/net/ena/ena_ethdev.c             | 2 +-
 drivers/net/fm10k/fm10k_ethdev.c         | 6 ++----
 drivers/net/i40e/i40e_ethdev_vf.c        | 2 +-
 drivers/net/ixgbe/ixgbe_ethdev.c         | 2 +-
 drivers/net/mlx4/mlx4_ethdev.c           | 2 +-
 drivers/net/mlx5/mlx5_ethdev.c           | 2 +-
 19 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/doc/guides/nics/features/avf.ini b/doc/guides/nics/features/avf.ini
index ccb9edde3..35ceada24 100644
--- a/doc/guides/nics/features/avf.ini
+++ b/doc/guides/nics/features/avf.ini
@@ -6,7 +6,6 @@
 [Features]
 Speed capabilities   = Y
 Link status          = Y
-Link status event    = Y
 Rx interrupt         = Y
 Queue start/stop     = Y
 MTU update           = Y
diff --git a/doc/guides/nics/features/avf_vec.ini b/doc/guides/nics/features/avf_vec.ini
index 892499485..3050bc4a6 100644
--- a/doc/guides/nics/features/avf_vec.ini
+++ b/doc/guides/nics/features/avf_vec.ini
@@ -6,7 +6,6 @@
 [Features]
 Speed capabilities   = Y
 Link status          = Y
-Link status event    = Y
 Rx interrupt         = Y
 Queue start/stop     = Y
 MTU update           = Y
diff --git a/doc/guides/nics/features/fm10k.ini b/doc/guides/nics/features/fm10k.ini
index f0f61a7d7..58f58b99c 100644
--- a/doc/guides/nics/features/fm10k.ini
+++ b/doc/guides/nics/features/fm10k.ini
@@ -5,6 +5,8 @@
 ;
 [Features]
 Speed capabilities   = P
+Link status          = Y
+Link status event    = Y
 Rx interrupt         = Y
 Queue start/stop     = Y
 Jumbo frame          = Y
diff --git a/doc/guides/nics/features/fm10k_vf.ini b/doc/guides/nics/features/fm10k_vf.ini
index 32b93df4b..44b50faa1 100644
--- a/doc/guides/nics/features/fm10k_vf.ini
+++ b/doc/guides/nics/features/fm10k_vf.ini
@@ -5,6 +5,8 @@
 ;
 [Features]
 Speed capabilities   = P
+Link status          = Y
+Link status event    = Y
 Rx interrupt         = Y
 Queue start/stop     = Y
 Jumbo frame          = Y
diff --git a/doc/guides/nics/features/i40e_vf.ini b/doc/guides/nics/features/i40e_vf.ini
index 46e0d9fce..ba2d8cbe9 100644
--- a/doc/guides/nics/features/i40e_vf.ini
+++ b/doc/guides/nics/features/i40e_vf.ini
@@ -5,6 +5,7 @@
 ;
 [Features]
 Rx interrupt         = Y
+Link status          = Y
 Queue start/stop     = Y
 Jumbo frame          = Y
 Scattered Rx         = Y
diff --git a/doc/guides/nics/features/i40e_vf_vec.ini b/doc/guides/nics/features/i40e_vf_vec.ini
index c2c6c19fe..421ed9193 100644
--- a/doc/guides/nics/features/i40e_vf_vec.ini
+++ b/doc/guides/nics/features/i40e_vf_vec.ini
@@ -5,6 +5,7 @@
 ;
 [Features]
 Rx interrupt         = Y
+Link status          = Y
 Queue start/stop     = Y
 Jumbo frame          = Y
 Scattered Rx         = Y
diff --git a/doc/guides/nics/features/igb_vf.ini b/doc/guides/nics/features/igb_vf.ini
index e641a2c97..d9653234b 100644
--- a/doc/guides/nics/features/igb_vf.ini
+++ b/doc/guides/nics/features/igb_vf.ini
@@ -4,6 +4,7 @@
 ; Refer to default.ini for the full list of available PMD features.
 ;
 [Features]
+Link status          = Y
 Rx interrupt         = Y
 Scattered Rx         = Y
 TSO                  = Y
diff --git a/doc/guides/nics/features/qede.ini b/doc/guides/nics/features/qede.ini
index 605cfa968..2d6af365e 100644
--- a/doc/guides/nics/features/qede.ini
+++ b/doc/guides/nics/features/qede.ini
@@ -6,7 +6,6 @@
 [Features]
 Speed capabilities   = Y
 Link status          = Y
-Link status event    = Y
 MTU update           = Y
 Jumbo frame          = Y
 Scattered Rx         = Y
diff --git a/doc/guides/nics/features/qede_vf.ini b/doc/guides/nics/features/qede_vf.ini
index 18857b6e3..70071a1bd 100644
--- a/doc/guides/nics/features/qede_vf.ini
+++ b/doc/guides/nics/features/qede_vf.ini
@@ -6,7 +6,6 @@
 [Features]
 Speed capabilities   = Y
 Link status          = Y
-Link status event    = Y
 MTU update           = Y
 Jumbo frame          = Y
 Scattered Rx         = Y
diff --git a/doc/guides/nics/features/vhost.ini b/doc/guides/nics/features/vhost.ini
index dffd1f493..ef81abb43 100644
--- a/doc/guides/nics/features/vhost.ini
+++ b/doc/guides/nics/features/vhost.ini
@@ -5,7 +5,6 @@
 ;
 [Features]
 Link status          = Y
-Link status event    = Y
 Free Tx mbuf on demand = Y
 Queue status event   = Y
 Basic stats          = Y
diff --git a/doc/guides/nics/features/virtio.ini b/doc/guides/nics/features/virtio.ini
index 16e577df0..a16b81721 100644
--- a/doc/guides/nics/features/virtio.ini
+++ b/doc/guides/nics/features/virtio.ini
@@ -6,6 +6,7 @@
 [Features]
 Speed capabilities   = P
 Link status          = Y
+Link status event    = Y
 Rx interrupt         = Y
 Queue start/stop     = Y
 Scattered Rx         = Y
diff --git a/doc/guides/nics/features/virtio_vec.ini b/doc/guides/nics/features/virtio_vec.ini
index c06c860d4..e60fe36ae 100644
--- a/doc/guides/nics/features/virtio_vec.ini
+++ b/doc/guides/nics/features/virtio_vec.ini
@@ -6,6 +6,7 @@
 [Features]
 Speed capabilities   = P
 Link status          = Y
+Link status event    = Y
 Rx interrupt         = Y
 Queue start/stop     = Y
 Promiscuous mode     = Y
diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
index de7db2650..694a6242c 100644
--- a/drivers/net/e1000/em_ethdev.c
+++ b/drivers/net/e1000/em_ethdev.c
@@ -1183,7 +1183,7 @@ eth_em_link_update(struct rte_eth_dev *dev, int wait_to_complete)
 		link.link_autoneg = !(dev->data->dev_conf.link_speeds &
 				ETH_LINK_SPEED_FIXED);
 	} else if (!link_check && (link.link_status == ETH_LINK_UP)) {
-		link.link_speed = 0;
+		link.link_speed = ETH_SPEED_NUM_NONE;
 		link.link_duplex = ETH_LINK_HALF_DUPLEX;
 		link.link_status = ETH_LINK_DOWN;
 		link.link_autoneg = ETH_LINK_FIXED;
diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index ab4e2af91..41b5638fd 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -728,7 +728,7 @@ static int ena_link_update(struct rte_eth_dev *dev,
 {
 	struct rte_eth_link *link = &dev->data->dev_link;
 
-	link->link_status = 1;
+	link->link_status = ETH_LINK_UP;
 	link->link_speed = ETH_SPEED_NUM_10G;
 	link->link_duplex = ETH_LINK_FULL_DUPLEX;
 
diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index 34affd1cc..7dfeddfe0 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -1260,13 +1260,11 @@ fm10k_link_update(struct rte_eth_dev *dev,
 		FM10K_DEV_PRIVATE_TO_INFO(dev->data->dev_private);
 	PMD_INIT_FUNC_TRACE();
 
-	/* The speed is ~50Gbps per Gen3 x8 PCIe interface. For now, we
-	 * leave the speed undefined since there is no 50Gbps Ethernet.
-	 */
-	dev->data->dev_link.link_speed  = 0;
+	dev->data->dev_link.link_speed  = ETH_SPEED_NUM_50G;
 	dev->data->dev_link.link_duplex = ETH_LINK_FULL_DUPLEX;
 	dev->data->dev_link.link_status =
 		dev_info->sm_down ? ETH_LINK_DOWN : ETH_LINK_UP;
+	dev->data->dev_link.link_autoneg = ETH_LINK_FIXED;
 
 	return 0;
 }
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index 031c70680..48e7ac21e 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -2112,7 +2112,7 @@ i40evf_dev_link_update(struct rte_eth_dev *dev,
 	new_link.link_status = vf->link_up ? ETH_LINK_UP :
 					     ETH_LINK_DOWN;
 	new_link.link_autoneg =
-		dev->data->dev_conf.link_speeds & ETH_LINK_SPEED_FIXED;
+		!(dev->data->dev_conf.link_speeds & ETH_LINK_SPEED_FIXED);
 
 	return rte_eth_linkstatus_set(dev, &new_link);
 }
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index a5e2fc0ca..c401250e9 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -3900,7 +3900,7 @@ ixgbe_dev_link_update_share(struct rte_eth_dev *dev,
 
 	memset(&link, 0, sizeof(link));
 	link.link_status = ETH_LINK_DOWN;
-	link.link_speed = 0;
+	link.link_speed = ETH_SPEED_NUM_NONE;
 	link.link_duplex = ETH_LINK_HALF_DUPLEX;
 	link.link_autoneg = ETH_LINK_AUTONEG;
 
diff --git a/drivers/net/mlx4/mlx4_ethdev.c b/drivers/net/mlx4/mlx4_ethdev.c
index d1b71c805..9a76670d8 100644
--- a/drivers/net/mlx4/mlx4_ethdev.c
+++ b/drivers/net/mlx4/mlx4_ethdev.c
@@ -713,7 +713,7 @@ mlx4_link_update(struct rte_eth_dev *dev, int wait_to_complete)
 	}
 	link_speed = ethtool_cmd_speed(&edata);
 	if (link_speed == -1)
-		dev_link.link_speed = 0;
+		dev_link.link_speed = ETH_SPEED_NUM_NONE;
 	else
 		dev_link.link_speed = link_speed;
 	dev_link.link_duplex = ((edata.duplex == DUPLEX_HALF) ?
diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
index ef44cc91f..59d6a805f 100644
--- a/drivers/net/mlx5/mlx5_ethdev.c
+++ b/drivers/net/mlx5/mlx5_ethdev.c
@@ -539,7 +539,7 @@ mlx5_link_update_unlocked_gset(struct rte_eth_dev *dev,
 	}
 	link_speed = ethtool_cmd_speed(&edata);
 	if (link_speed == -1)
-		dev_link.link_speed = 0;
+		dev_link.link_speed = ETH_SPEED_NUM_NONE;
 	else
 		dev_link.link_speed = link_speed;
 	priv->link_speed_capa = 0;
-- 
2.14.3

Re: [dpdk-dev] [PATCH] drivers/net: update link status

[Tan, Jianfeng]

Hi Ferruh,


On 4/17/2018 7:26 PM, Ferruh Yigit wrote:
> On 4/17/2018 5:54 AM, Tiwei Bie wrote:
>> On Mon, Apr 16, 2018 at 05:10:24PM +0100, Ferruh Yigit wrote:
>>> On 4/14/2018 11:55 AM, Tiwei Bie wrote:
>>>> On Fri, Apr 13, 2018 at 10:53:55PM +0100, Ferruh Yigit wrote:
>>>>> On 4/10/2018 4:41 PM, Tiwei Bie wrote:
>>>>>> On Tue, Mar 13, 2018 at 06:05:34PM +0000, Ferruh Yigit wrote:
>>>>>>> Update link status related feature document items and minor updates in
>>>>>>> some link status related functions.
>>>>>>>
>>>>>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>>>>>>> ---
>>>>>>>   doc/guides/nics/features/fm10k.ini      | 2 ++
>>>>>>>   doc/guides/nics/features/fm10k_vf.ini   | 2 ++
>>>>>>>   doc/guides/nics/features/i40e_vf.ini    | 1 +
>>>>>>>   doc/guides/nics/features/igb_vf.ini     | 1 +
>>>>>>>   doc/guides/nics/features/qede.ini       | 1 -
>>>>>>>   doc/guides/nics/features/qede_vf.ini    | 1 -
>>>>>>>   doc/guides/nics/features/vhost.ini      | 2 --
>>>>>>>   doc/guides/nics/features/virtio_vec.ini | 1 +
>>>>>>>   drivers/net/e1000/em_ethdev.c           | 2 +-
>>>>>>>   drivers/net/ena/ena_ethdev.c            | 2 +-
>>>>>>>   drivers/net/fm10k/fm10k_ethdev.c        | 6 ++----
>>>>>>>   drivers/net/i40e/i40e_ethdev_vf.c       | 2 +-
>>>>>>>   drivers/net/ixgbe/ixgbe_ethdev.c        | 2 +-
>>>>>>>   drivers/net/mlx4/mlx4_ethdev.c          | 2 +-
>>>>>>>   drivers/net/mlx5/mlx5_ethdev.c          | 2 +-
>>>>>>>   15 files changed, 15 insertions(+), 14 deletions(-)
>>>>>> [...]
>>>>>>> diff --git a/doc/guides/nics/features/vhost.ini b/doc/guides/nics/features/vhost.ini
>>>>>>> index dffd1f493..31302745a 100644
>>>>>>> --- a/doc/guides/nics/features/vhost.ini
>>>>>>> +++ b/doc/guides/nics/features/vhost.ini
>>>>>>> @@ -4,8 +4,6 @@
>>>>>>>   ; Refer to default.ini for the full list of available PMD features.
>>>>>>>   ;
>>>>>>>   [Features]
>>>>>>> -Link status          = Y
>>>>>>> -Link status event    = Y
>>>>>> I think vhost PMD supports above features.
>>>>> I am not able to find where it is supported.
>>>>>
>>>>> Some virtual PMDs report fixed link, with empty link_update() dev_ops, and they
>>>>> are not reported as supporting Link status, as far as I can see vhost also one
>>>>> of them.
>>>>>
>>>>> And for Link status event, PMD needs to support LSC interrupts and should
>>>>> register interrupt handler for it, which I can't find for vhost.
>>>>>
>>>>> I will send next version without updating above one, please point me where these
>>>>> support added if I missed them.
>>>> In drivers/net/vhost/rte_eth_vhost.c you could find below functions:
>>>>
>>>> static int
>>>> new_device(int vid)
>>>> {
>>>> 	......
>>>>
>>>> 	eth_dev->data->dev_link.link_status = ETH_LINK_UP;
>>>>
>>>> 	......
>>>>
>>>> 	_rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_INTR_LSC, NULL);
>>>>
>>>> 	......
>>>> }
>>>>
>>>> static void
>>>> destroy_device(int vid)
>>>> {
>>>> 	......
>>>>
>>>> 	eth_dev->data->dev_link.link_status = ETH_LINK_DOWN;
>>>>
>>>> 	......
>>>>
>>>> 	_rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_INTR_LSC, NULL);
>>>>
>>>> 	......
>>>> }
>>>>
>>>> They are the callbacks for vhost library.
>>>>
>>>> When a frontend (e.g. QEMU) is connected to this vhost backend
>>>> and the frontend virtio device becomes ready, new_device() will
>>>> be called by the vhost library, and the link status will be
>>>> updated to UP.
>>>>
>>>> And when e.g. the connection is closed, destroy_device() will be
>>>> called by the vhost library, and the link status will be updated
>>>> to DOWN.
>>>
>>> Got it. This behavior is similar for virtual PMDs. Provide static link
>>> information and update link as UP during start and update it as DOWN during stop.
>> No, the link status isn't updated during vhost PMD start
>> and stop. When the vhost PMD has been started, the link
>> status still may be DOWN. The link status becomes UP only
>> when the QEMU (it's another virtual machine process which
>> has a virtio device) connects to this vhost PMD via a UNIX
>> socket and the virtio driver in the virtual machine has
>> setup the virtio device of the virtual machine.
>>
>> So if vhost PMD reports the link status as DOWN, it means
>> there is no QEMU (virtual machine) connects to it or the
>> virtio device in the virtual machine hasn't been setup.
>> (PS. The frontend can also be virtio-user PMD besides QEMU)
> I believe announcing link feature reporting on virtual pmds still in gray area,
> but because of qemu involvement in vhost case, I will keep link feature but will
> drop link event.

AFAIK, link status means we can get link status through APIs like 
rte_eth_link_get(); while link status event means applications can 
register link status events, and those events get called if link status 
is changed.

If I understand it correctly, for vhost, we can keep both link status 
and link status event for vhost.

Could you specify the reason why we remove link status event?

Thanks,
Jianfeng

>
> Will send a new patch to reflect this.
>
> Thanks,
> ferruh

Re: [dpdk-dev] [PATCH] drivers/net: update link status

[Ferruh Yigit]

On 4/18/2018 7:49 AM, Tan, Jianfeng wrote:
> Hi Ferruh,
> 
> 
> On 4/17/2018 7:26 PM, Ferruh Yigit wrote:
>> On 4/17/2018 5:54 AM, Tiwei Bie wrote:
>>> On Mon, Apr 16, 2018 at 05:10:24PM +0100, Ferruh Yigit wrote:
>>>> On 4/14/2018 11:55 AM, Tiwei Bie wrote:
>>>>> On Fri, Apr 13, 2018 at 10:53:55PM +0100, Ferruh Yigit wrote:
>>>>>> On 4/10/2018 4:41 PM, Tiwei Bie wrote:
>>>>>>> On Tue, Mar 13, 2018 at 06:05:34PM +0000, Ferruh Yigit wrote:
>>>>>>>> Update link status related feature document items and minor updates in
>>>>>>>> some link status related functions.
>>>>>>>>
>>>>>>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>>>>>>>> ---
>>>>>>>>   doc/guides/nics/features/fm10k.ini      | 2 ++
>>>>>>>>   doc/guides/nics/features/fm10k_vf.ini   | 2 ++
>>>>>>>>   doc/guides/nics/features/i40e_vf.ini    | 1 +
>>>>>>>>   doc/guides/nics/features/igb_vf.ini     | 1 +
>>>>>>>>   doc/guides/nics/features/qede.ini       | 1 -
>>>>>>>>   doc/guides/nics/features/qede_vf.ini    | 1 -
>>>>>>>>   doc/guides/nics/features/vhost.ini      | 2 --
>>>>>>>>   doc/guides/nics/features/virtio_vec.ini | 1 +
>>>>>>>>   drivers/net/e1000/em_ethdev.c           | 2 +-
>>>>>>>>   drivers/net/ena/ena_ethdev.c            | 2 +-
>>>>>>>>   drivers/net/fm10k/fm10k_ethdev.c        | 6 ++----
>>>>>>>>   drivers/net/i40e/i40e_ethdev_vf.c       | 2 +-
>>>>>>>>   drivers/net/ixgbe/ixgbe_ethdev.c        | 2 +-
>>>>>>>>   drivers/net/mlx4/mlx4_ethdev.c          | 2 +-
>>>>>>>>   drivers/net/mlx5/mlx5_ethdev.c          | 2 +-
>>>>>>>>   15 files changed, 15 insertions(+), 14 deletions(-)
>>>>>>> [...]
>>>>>>>> diff --git a/doc/guides/nics/features/vhost.ini b/doc/guides/nics/features/vhost.ini
>>>>>>>> index dffd1f493..31302745a 100644
>>>>>>>> --- a/doc/guides/nics/features/vhost.ini
>>>>>>>> +++ b/doc/guides/nics/features/vhost.ini
>>>>>>>> @@ -4,8 +4,6 @@
>>>>>>>>   ; Refer to default.ini for the full list of available PMD features.
>>>>>>>>   ;
>>>>>>>>   [Features]
>>>>>>>> -Link status          = Y
>>>>>>>> -Link status event    = Y
>>>>>>> I think vhost PMD supports above features.
>>>>>> I am not able to find where it is supported.
>>>>>>
>>>>>> Some virtual PMDs report fixed link, with empty link_update() dev_ops, and they
>>>>>> are not reported as supporting Link status, as far as I can see vhost also one
>>>>>> of them.
>>>>>>
>>>>>> And for Link status event, PMD needs to support LSC interrupts and should
>>>>>> register interrupt handler for it, which I can't find for vhost.
>>>>>>
>>>>>> I will send next version without updating above one, please point me where these
>>>>>> support added if I missed them.
>>>>> In drivers/net/vhost/rte_eth_vhost.c you could find below functions:
>>>>>
>>>>> static int
>>>>> new_device(int vid)
>>>>> {
>>>>> 	......
>>>>>
>>>>> 	eth_dev->data->dev_link.link_status = ETH_LINK_UP;
>>>>>
>>>>> 	......
>>>>>
>>>>> 	_rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_INTR_LSC, NULL);
>>>>>
>>>>> 	......
>>>>> }
>>>>>
>>>>> static void
>>>>> destroy_device(int vid)
>>>>> {
>>>>> 	......
>>>>>
>>>>> 	eth_dev->data->dev_link.link_status = ETH_LINK_DOWN;
>>>>>
>>>>> 	......
>>>>>
>>>>> 	_rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_INTR_LSC, NULL);
>>>>>
>>>>> 	......
>>>>> }
>>>>>
>>>>> They are the callbacks for vhost library.
>>>>>
>>>>> When a frontend (e.g. QEMU) is connected to this vhost backend
>>>>> and the frontend virtio device becomes ready, new_device() will
>>>>> be called by the vhost library, and the link status will be
>>>>> updated to UP.
>>>>>
>>>>> And when e.g. the connection is closed, destroy_device() will be
>>>>> called by the vhost library, and the link status will be updated
>>>>> to DOWN.
>>>>
>>>> Got it. This behavior is similar for virtual PMDs. Provide static link
>>>> information and update link as UP during start and update it as DOWN during stop.
>>> No, the link status isn't updated during vhost PMD start
>>> and stop. When the vhost PMD has been started, the link
>>> status still may be DOWN. The link status becomes UP only
>>> when the QEMU (it's another virtual machine process which
>>> has a virtio device) connects to this vhost PMD via a UNIX
>>> socket and the virtio driver in the virtual machine has
>>> setup the virtio device of the virtual machine.
>>>
>>> So if vhost PMD reports the link status as DOWN, it means
>>> there is no QEMU (virtual machine) connects to it or the
>>> virtio device in the virtual machine hasn't been setup.
>>> (PS. The frontend can also be virtio-user PMD besides QEMU)
>> I believe announcing link feature reporting on virtual pmds still in gray area,
>> but because of qemu involvement in vhost case, I will keep link feature but will
>> drop link event.
> 
> AFAIK, link status means we can get link status through APIs like 
> rte_eth_link_get(); while link status event means applications can 
> register link status events, and those events get called if link status 
> is changed.
> 
> If I understand it correctly, for vhost, we can keep both link status 
> and link status event for vhost.
> 
> Could you specify the reason why we remove link status event?

Hi Jianfeng,

I think problem is the definition of the features are not clear, that is why I
started a doc to document them (doc/guides/nics/features.rst)

"Link status", I think we agree on this one. PMD should provide up-to-date,
valid link data on rte_eth_dev_data->dev_link. So that this link information can
be get by rte_eth_link_get(), rte_eth_link_get_nowait() ethdev APIs.

rte_eth_link_get() uses dev_ops->link_update() to get the latest link
information, for virtual PMDs, including vhost, this function does nothing,
because there is no actual physical link. That is why I was not sure advertising
link status feature for vhost, and other virtual PMDs doesn't report this
feature. After Tiwei's comment that link status shows that qemu connected to
vhost, added this feature back to vhost PMD.


"Link status event", can be
a- PMD calls application callback in link change
b- PMD registers interrupt handler for link status change interrupts

Based on how other PMDs report this feature, I believe it is (b), and I have
documented that way. And vhost "Link status event" feature removed based on this.

There are some set of config options and flags to control the LSC interrupt,
that also effects rte_eth_link_get() and rte_eth_link_get_nowait() APIs, I
believe that is the main concern here.

As commented above, I don't understand why calling user register callback for
link change event is something on PMD decision, it should be default behavior
for PMD. What is the point of leaving this into PMD and think this as a feature
of PMD?


Overall, practical reason of this table is to inform developer/user about PMD
features, which is indeed device + driver features, and help her to in
development or on setting expectations. We can always discuss what helps more to
developer/user and update the features table.

Thanks,
ferruh

> 
> Thanks,
> Jianfeng
> 
>>
>> Will send a new patch to reflect this.
>>
>> Thanks,
>> ferruh
> 

Re: [dpdk-dev] [PATCH] drivers/net: update link status

[Tiwei Bie]

On Wed, Apr 18, 2018 at 11:42:56AM +0100, Ferruh Yigit wrote:
> On 4/18/2018 7:49 AM, Tan, Jianfeng wrote:
> > Hi Ferruh,
> > 
> > 
> > On 4/17/2018 7:26 PM, Ferruh Yigit wrote:
> >> On 4/17/2018 5:54 AM, Tiwei Bie wrote:
> >>> On Mon, Apr 16, 2018 at 05:10:24PM +0100, Ferruh Yigit wrote:
> >>>> On 4/14/2018 11:55 AM, Tiwei Bie wrote:
> >>>>> On Fri, Apr 13, 2018 at 10:53:55PM +0100, Ferruh Yigit wrote:
> >>>>>> On 4/10/2018 4:41 PM, Tiwei Bie wrote:
> >>>>>>> On Tue, Mar 13, 2018 at 06:05:34PM +0000, Ferruh Yigit wrote:
> >>>>>>>> Update link status related feature document items and minor updates in
> >>>>>>>> some link status related functions.
> >>>>>>>>
> >>>>>>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> >>>>>>>> ---
> >>>>>>>>   doc/guides/nics/features/fm10k.ini      | 2 ++
> >>>>>>>>   doc/guides/nics/features/fm10k_vf.ini   | 2 ++
> >>>>>>>>   doc/guides/nics/features/i40e_vf.ini    | 1 +
> >>>>>>>>   doc/guides/nics/features/igb_vf.ini     | 1 +
> >>>>>>>>   doc/guides/nics/features/qede.ini       | 1 -
> >>>>>>>>   doc/guides/nics/features/qede_vf.ini    | 1 -
> >>>>>>>>   doc/guides/nics/features/vhost.ini      | 2 --
> >>>>>>>>   doc/guides/nics/features/virtio_vec.ini | 1 +
> >>>>>>>>   drivers/net/e1000/em_ethdev.c           | 2 +-
> >>>>>>>>   drivers/net/ena/ena_ethdev.c            | 2 +-
> >>>>>>>>   drivers/net/fm10k/fm10k_ethdev.c        | 6 ++----
> >>>>>>>>   drivers/net/i40e/i40e_ethdev_vf.c       | 2 +-
> >>>>>>>>   drivers/net/ixgbe/ixgbe_ethdev.c        | 2 +-
> >>>>>>>>   drivers/net/mlx4/mlx4_ethdev.c          | 2 +-
> >>>>>>>>   drivers/net/mlx5/mlx5_ethdev.c          | 2 +-
> >>>>>>>>   15 files changed, 15 insertions(+), 14 deletions(-)
> >>>>>>> [...]
> >>>>>>>> diff --git a/doc/guides/nics/features/vhost.ini b/doc/guides/nics/features/vhost.ini
> >>>>>>>> index dffd1f493..31302745a 100644
> >>>>>>>> --- a/doc/guides/nics/features/vhost.ini
> >>>>>>>> +++ b/doc/guides/nics/features/vhost.ini
> >>>>>>>> @@ -4,8 +4,6 @@
> >>>>>>>>   ; Refer to default.ini for the full list of available PMD features.
> >>>>>>>>   ;
> >>>>>>>>   [Features]
> >>>>>>>> -Link status          = Y
> >>>>>>>> -Link status event    = Y
> >>>>>>> I think vhost PMD supports above features.
> >>>>>> I am not able to find where it is supported.
> >>>>>>
> >>>>>> Some virtual PMDs report fixed link, with empty link_update() dev_ops, and they
> >>>>>> are not reported as supporting Link status, as far as I can see vhost also one
> >>>>>> of them.
> >>>>>>
> >>>>>> And for Link status event, PMD needs to support LSC interrupts and should
> >>>>>> register interrupt handler for it, which I can't find for vhost.
> >>>>>>
> >>>>>> I will send next version without updating above one, please point me where these
> >>>>>> support added if I missed them.
> >>>>> In drivers/net/vhost/rte_eth_vhost.c you could find below functions:
> >>>>>
> >>>>> static int
> >>>>> new_device(int vid)
> >>>>> {
> >>>>> 	......
> >>>>>
> >>>>> 	eth_dev->data->dev_link.link_status = ETH_LINK_UP;
> >>>>>
> >>>>> 	......
> >>>>>
> >>>>> 	_rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_INTR_LSC, NULL);
> >>>>>
> >>>>> 	......
> >>>>> }
> >>>>>
> >>>>> static void
> >>>>> destroy_device(int vid)
> >>>>> {
> >>>>> 	......
> >>>>>
> >>>>> 	eth_dev->data->dev_link.link_status = ETH_LINK_DOWN;
> >>>>>
> >>>>> 	......
> >>>>>
> >>>>> 	_rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_INTR_LSC, NULL);
> >>>>>
> >>>>> 	......
> >>>>> }
> >>>>>
> >>>>> They are the callbacks for vhost library.
> >>>>>
> >>>>> When a frontend (e.g. QEMU) is connected to this vhost backend
> >>>>> and the frontend virtio device becomes ready, new_device() will
> >>>>> be called by the vhost library, and the link status will be
> >>>>> updated to UP.
> >>>>>
> >>>>> And when e.g. the connection is closed, destroy_device() will be
> >>>>> called by the vhost library, and the link status will be updated
> >>>>> to DOWN.
> >>>>
> >>>> Got it. This behavior is similar for virtual PMDs. Provide static link
> >>>> information and update link as UP during start and update it as DOWN during stop.
> >>> No, the link status isn't updated during vhost PMD start
> >>> and stop. When the vhost PMD has been started, the link
> >>> status still may be DOWN. The link status becomes UP only
> >>> when the QEMU (it's another virtual machine process which
> >>> has a virtio device) connects to this vhost PMD via a UNIX
> >>> socket and the virtio driver in the virtual machine has
> >>> setup the virtio device of the virtual machine.
> >>>
> >>> So if vhost PMD reports the link status as DOWN, it means
> >>> there is no QEMU (virtual machine) connects to it or the
> >>> virtio device in the virtual machine hasn't been setup.
> >>> (PS. The frontend can also be virtio-user PMD besides QEMU)
> >> I believe announcing link feature reporting on virtual pmds still in gray area,
> >> but because of qemu involvement in vhost case, I will keep link feature but will
> >> drop link event.
> > 
> > AFAIK, link status means we can get link status through APIs like 
> > rte_eth_link_get(); while link status event means applications can 
> > register link status events, and those events get called if link status 
> > is changed.
> > 
> > If I understand it correctly, for vhost, we can keep both link status 
> > and link status event for vhost.
> > 
> > Could you specify the reason why we remove link status event?
> 
> Hi Jianfeng,
> 
> I think problem is the definition of the features are not clear, that is why I
> started a doc to document them (doc/guides/nics/features.rst)
> 
> "Link status", I think we agree on this one. PMD should provide up-to-date,
> valid link data on rte_eth_dev_data->dev_link. So that this link information can
> be get by rte_eth_link_get(), rte_eth_link_get_nowait() ethdev APIs.
> 
> rte_eth_link_get() uses dev_ops->link_update() to get the latest link
> information, for virtual PMDs, including vhost, this function does nothing,
> because there is no actual physical link. That is why I was not sure advertising
> link status feature for vhost, and other virtual PMDs doesn't report this
> feature. After Tiwei's comment that link status shows that qemu connected to
> vhost, added this feature back to vhost PMD.
> 
> 
> "Link status event", can be
> a- PMD calls application callback in link change
> b- PMD registers interrupt handler for link status change interrupts
> 
> Based on how other PMDs report this feature, I believe it is (b), and I have
> documented that way. And vhost "Link status event" feature removed based on this.
> 
> There are some set of config options and flags to control the LSC interrupt,
> that also effects rte_eth_link_get() and rte_eth_link_get_nowait() APIs, I
> believe that is the main concern here.
> 
> As commented above, I don't understand why calling user register callback for
> link change event is something on PMD decision, it should be default behavior
> for PMD.

Do you mean you don't understand why vhost PMD calls
_rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_INTR_LSC, NULL)
in new_device() and destroy_device()? While other physical
NIC drivers call this in their interrupt handlers which
are registered via rte_intr_callback_register().

The new_device() and destroy_device() in vhost PMD can be
treated as something like the interrupt handlers in physical
NIC drivers. They are the callbacks which are registered via
rte_vhost_driver_callback_register() and will be called by
vhost library when vhost events happen (and they should be
translated to link status change events when we try to wrap
it as a net PMD).

Thanks

> What is the point of leaving this into PMD and think this as a feature
> of PMD?
> 
> 
> Overall, practical reason of this table is to inform developer/user about PMD
> features, which is indeed device + driver features, and help her to in
> development or on setting expectations. We can always discuss what helps more to
> developer/user and update the features table.
> 
> Thanks,
> ferruh
> 
> > 
> > Thanks,
> > Jianfeng
> > 
> >>
> >> Will send a new patch to reflect this.
> >>
> >> Thanks,
> >> ferruh
> > 
> 

Re: [dpdk-dev] [PATCH] drivers/net: update link status

[Ferruh Yigit]

On 4/18/2018 12:36 PM, Tiwei Bie wrote:
> On Wed, Apr 18, 2018 at 11:42:56AM +0100, Ferruh Yigit wrote:
>> On 4/18/2018 7:49 AM, Tan, Jianfeng wrote:
>>> Hi Ferruh,
>>>
>>>
>>> On 4/17/2018 7:26 PM, Ferruh Yigit wrote:
>>>> On 4/17/2018 5:54 AM, Tiwei Bie wrote:
>>>>> On Mon, Apr 16, 2018 at 05:10:24PM +0100, Ferruh Yigit wrote:
>>>>>> On 4/14/2018 11:55 AM, Tiwei Bie wrote:
>>>>>>> On Fri, Apr 13, 2018 at 10:53:55PM +0100, Ferruh Yigit wrote:
>>>>>>>> On 4/10/2018 4:41 PM, Tiwei Bie wrote:
>>>>>>>>> On Tue, Mar 13, 2018 at 06:05:34PM +0000, Ferruh Yigit wrote:
>>>>>>>>>> Update link status related feature document items and minor updates in
>>>>>>>>>> some link status related functions.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>>>>>>>>>> ---
>>>>>>>>>>   doc/guides/nics/features/fm10k.ini      | 2 ++
>>>>>>>>>>   doc/guides/nics/features/fm10k_vf.ini   | 2 ++
>>>>>>>>>>   doc/guides/nics/features/i40e_vf.ini    | 1 +
>>>>>>>>>>   doc/guides/nics/features/igb_vf.ini     | 1 +
>>>>>>>>>>   doc/guides/nics/features/qede.ini       | 1 -
>>>>>>>>>>   doc/guides/nics/features/qede_vf.ini    | 1 -
>>>>>>>>>>   doc/guides/nics/features/vhost.ini      | 2 --
>>>>>>>>>>   doc/guides/nics/features/virtio_vec.ini | 1 +
>>>>>>>>>>   drivers/net/e1000/em_ethdev.c           | 2 +-
>>>>>>>>>>   drivers/net/ena/ena_ethdev.c            | 2 +-
>>>>>>>>>>   drivers/net/fm10k/fm10k_ethdev.c        | 6 ++----
>>>>>>>>>>   drivers/net/i40e/i40e_ethdev_vf.c       | 2 +-
>>>>>>>>>>   drivers/net/ixgbe/ixgbe_ethdev.c        | 2 +-
>>>>>>>>>>   drivers/net/mlx4/mlx4_ethdev.c          | 2 +-
>>>>>>>>>>   drivers/net/mlx5/mlx5_ethdev.c          | 2 +-
>>>>>>>>>>   15 files changed, 15 insertions(+), 14 deletions(-)
>>>>>>>>> [...]
>>>>>>>>>> diff --git a/doc/guides/nics/features/vhost.ini b/doc/guides/nics/features/vhost.ini
>>>>>>>>>> index dffd1f493..31302745a 100644
>>>>>>>>>> --- a/doc/guides/nics/features/vhost.ini
>>>>>>>>>> +++ b/doc/guides/nics/features/vhost.ini
>>>>>>>>>> @@ -4,8 +4,6 @@
>>>>>>>>>>   ; Refer to default.ini for the full list of available PMD features.
>>>>>>>>>>   ;
>>>>>>>>>>   [Features]
>>>>>>>>>> -Link status          = Y
>>>>>>>>>> -Link status event    = Y
>>>>>>>>> I think vhost PMD supports above features.
>>>>>>>> I am not able to find where it is supported.
>>>>>>>>
>>>>>>>> Some virtual PMDs report fixed link, with empty link_update() dev_ops, and they
>>>>>>>> are not reported as supporting Link status, as far as I can see vhost also one
>>>>>>>> of them.
>>>>>>>>
>>>>>>>> And for Link status event, PMD needs to support LSC interrupts and should
>>>>>>>> register interrupt handler for it, which I can't find for vhost.
>>>>>>>>
>>>>>>>> I will send next version without updating above one, please point me where these
>>>>>>>> support added if I missed them.
>>>>>>> In drivers/net/vhost/rte_eth_vhost.c you could find below functions:
>>>>>>>
>>>>>>> static int
>>>>>>> new_device(int vid)
>>>>>>> {
>>>>>>> 	......
>>>>>>>
>>>>>>> 	eth_dev->data->dev_link.link_status = ETH_LINK_UP;
>>>>>>>
>>>>>>> 	......
>>>>>>>
>>>>>>> 	_rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_INTR_LSC, NULL);
>>>>>>>
>>>>>>> 	......
>>>>>>> }
>>>>>>>
>>>>>>> static void
>>>>>>> destroy_device(int vid)
>>>>>>> {
>>>>>>> 	......
>>>>>>>
>>>>>>> 	eth_dev->data->dev_link.link_status = ETH_LINK_DOWN;
>>>>>>>
>>>>>>> 	......
>>>>>>>
>>>>>>> 	_rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_INTR_LSC, NULL);
>>>>>>>
>>>>>>> 	......
>>>>>>> }
>>>>>>>
>>>>>>> They are the callbacks for vhost library.
>>>>>>>
>>>>>>> When a frontend (e.g. QEMU) is connected to this vhost backend
>>>>>>> and the frontend virtio device becomes ready, new_device() will
>>>>>>> be called by the vhost library, and the link status will be
>>>>>>> updated to UP.
>>>>>>>
>>>>>>> And when e.g. the connection is closed, destroy_device() will be
>>>>>>> called by the vhost library, and the link status will be updated
>>>>>>> to DOWN.
>>>>>>
>>>>>> Got it. This behavior is similar for virtual PMDs. Provide static link
>>>>>> information and update link as UP during start and update it as DOWN during stop.
>>>>> No, the link status isn't updated during vhost PMD start
>>>>> and stop. When the vhost PMD has been started, the link
>>>>> status still may be DOWN. The link status becomes UP only
>>>>> when the QEMU (it's another virtual machine process which
>>>>> has a virtio device) connects to this vhost PMD via a UNIX
>>>>> socket and the virtio driver in the virtual machine has
>>>>> setup the virtio device of the virtual machine.
>>>>>
>>>>> So if vhost PMD reports the link status as DOWN, it means
>>>>> there is no QEMU (virtual machine) connects to it or the
>>>>> virtio device in the virtual machine hasn't been setup.
>>>>> (PS. The frontend can also be virtio-user PMD besides QEMU)
>>>> I believe announcing link feature reporting on virtual pmds still in gray area,
>>>> but because of qemu involvement in vhost case, I will keep link feature but will
>>>> drop link event.
>>>
>>> AFAIK, link status means we can get link status through APIs like 
>>> rte_eth_link_get(); while link status event means applications can 
>>> register link status events, and those events get called if link status 
>>> is changed.
>>>
>>> If I understand it correctly, for vhost, we can keep both link status 
>>> and link status event for vhost.
>>>
>>> Could you specify the reason why we remove link status event?
>>
>> Hi Jianfeng,
>>
>> I think problem is the definition of the features are not clear, that is why I
>> started a doc to document them (doc/guides/nics/features.rst)
>>
>> "Link status", I think we agree on this one. PMD should provide up-to-date,
>> valid link data on rte_eth_dev_data->dev_link. So that this link information can
>> be get by rte_eth_link_get(), rte_eth_link_get_nowait() ethdev APIs.
>>
>> rte_eth_link_get() uses dev_ops->link_update() to get the latest link
>> information, for virtual PMDs, including vhost, this function does nothing,
>> because there is no actual physical link. That is why I was not sure advertising
>> link status feature for vhost, and other virtual PMDs doesn't report this
>> feature. After Tiwei's comment that link status shows that qemu connected to
>> vhost, added this feature back to vhost PMD.
>>
>>
>> "Link status event", can be
>> a- PMD calls application callback in link change
>> b- PMD registers interrupt handler for link status change interrupts
>>
>> Based on how other PMDs report this feature, I believe it is (b), and I have
>> documented that way. And vhost "Link status event" feature removed based on this.
>>
>> There are some set of config options and flags to control the LSC interrupt,
>> that also effects rte_eth_link_get() and rte_eth_link_get_nowait() APIs, I
>> believe that is the main concern here.
>>
>> As commented above, I don't understand why calling user register callback for
>> link change event is something on PMD decision, it should be default behavior
>> for PMD.
> 
> Do you mean you don't understand why vhost PMD calls
> _rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_INTR_LSC, NULL)
> in new_device() and destroy_device()? While other physical
> NIC drivers call this in their interrupt handlers which
> are registered via rte_intr_callback_register().

No, I understand this part.

My question is why not all PMDs call callback_process() when link changed as a
requirement? Why PMD is giving decision to call or not call callback_process() ?

> 
> The new_device() and destroy_device() in vhost PMD can be
> treated as something like the interrupt handlers in physical
> NIC drivers. They are the callbacks which are registered via
> rte_vhost_driver_callback_register() and will be called by
> vhost library when vhost events happen (and they should be
> translated to link status change events when we try to wrap
> it as a net PMD).
> 
> Thanks
> 
>> What is the point of leaving this into PMD and think this as a feature
>> of PMD?
>>
>>
>> Overall, practical reason of this table is to inform developer/user about PMD
>> features, which is indeed device + driver features, and help her to in
>> development or on setting expectations. We can always discuss what helps more to
>> developer/user and update the features table.
>>
>> Thanks,
>> ferruh
>>
>>>
>>> Thanks,
>>> Jianfeng
>>>
>>>>
>>>> Will send a new patch to reflect this.
>>>>
>>>> Thanks,
>>>> ferruh
>>>
>>

Re: [dpdk-dev] [PATCH] drivers/net: update link status

[Tiwei Bie]

On Wed, Apr 18, 2018 at 12:44:54PM +0100, Ferruh Yigit wrote:
> On 4/18/2018 12:36 PM, Tiwei Bie wrote:
> > On Wed, Apr 18, 2018 at 11:42:56AM +0100, Ferruh Yigit wrote:
> >> On 4/18/2018 7:49 AM, Tan, Jianfeng wrote:
> >>> Hi Ferruh,
> >>>
> >>>
> >>> On 4/17/2018 7:26 PM, Ferruh Yigit wrote:
> >>>> On 4/17/2018 5:54 AM, Tiwei Bie wrote:
> >>>>> On Mon, Apr 16, 2018 at 05:10:24PM +0100, Ferruh Yigit wrote:
> >>>>>> On 4/14/2018 11:55 AM, Tiwei Bie wrote:
> >>>>>>> On Fri, Apr 13, 2018 at 10:53:55PM +0100, Ferruh Yigit wrote:
> >>>>>>>> On 4/10/2018 4:41 PM, Tiwei Bie wrote:
> >>>>>>>>> On Tue, Mar 13, 2018 at 06:05:34PM +0000, Ferruh Yigit wrote:
> >>>>>>>>>> Update link status related feature document items and minor updates in
> >>>>>>>>>> some link status related functions.
> >>>>>>>>>>
> >>>>>>>>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> >>>>>>>>>> ---
> >>>>>>>>>>   doc/guides/nics/features/fm10k.ini      | 2 ++
> >>>>>>>>>>   doc/guides/nics/features/fm10k_vf.ini   | 2 ++
> >>>>>>>>>>   doc/guides/nics/features/i40e_vf.ini    | 1 +
> >>>>>>>>>>   doc/guides/nics/features/igb_vf.ini     | 1 +
> >>>>>>>>>>   doc/guides/nics/features/qede.ini       | 1 -
> >>>>>>>>>>   doc/guides/nics/features/qede_vf.ini    | 1 -
> >>>>>>>>>>   doc/guides/nics/features/vhost.ini      | 2 --
> >>>>>>>>>>   doc/guides/nics/features/virtio_vec.ini | 1 +
> >>>>>>>>>>   drivers/net/e1000/em_ethdev.c           | 2 +-
> >>>>>>>>>>   drivers/net/ena/ena_ethdev.c            | 2 +-
> >>>>>>>>>>   drivers/net/fm10k/fm10k_ethdev.c        | 6 ++----
> >>>>>>>>>>   drivers/net/i40e/i40e_ethdev_vf.c       | 2 +-
> >>>>>>>>>>   drivers/net/ixgbe/ixgbe_ethdev.c        | 2 +-
> >>>>>>>>>>   drivers/net/mlx4/mlx4_ethdev.c          | 2 +-
> >>>>>>>>>>   drivers/net/mlx5/mlx5_ethdev.c          | 2 +-
> >>>>>>>>>>   15 files changed, 15 insertions(+), 14 deletions(-)
> >>>>>>>>> [...]
> >>>>>>>>>> diff --git a/doc/guides/nics/features/vhost.ini b/doc/guides/nics/features/vhost.ini
> >>>>>>>>>> index dffd1f493..31302745a 100644
> >>>>>>>>>> --- a/doc/guides/nics/features/vhost.ini
> >>>>>>>>>> +++ b/doc/guides/nics/features/vhost.ini
> >>>>>>>>>> @@ -4,8 +4,6 @@
> >>>>>>>>>>   ; Refer to default.ini for the full list of available PMD features.
> >>>>>>>>>>   ;
> >>>>>>>>>>   [Features]
> >>>>>>>>>> -Link status          = Y
> >>>>>>>>>> -Link status event    = Y
> >>>>>>>>> I think vhost PMD supports above features.
> >>>>>>>> I am not able to find where it is supported.
> >>>>>>>>
> >>>>>>>> Some virtual PMDs report fixed link, with empty link_update() dev_ops, and they
> >>>>>>>> are not reported as supporting Link status, as far as I can see vhost also one
> >>>>>>>> of them.
> >>>>>>>>
> >>>>>>>> And for Link status event, PMD needs to support LSC interrupts and should
> >>>>>>>> register interrupt handler for it, which I can't find for vhost.
> >>>>>>>>
> >>>>>>>> I will send next version without updating above one, please point me where these
> >>>>>>>> support added if I missed them.
> >>>>>>> In drivers/net/vhost/rte_eth_vhost.c you could find below functions:
> >>>>>>>
> >>>>>>> static int
> >>>>>>> new_device(int vid)
> >>>>>>> {
> >>>>>>> 	......
> >>>>>>>
> >>>>>>> 	eth_dev->data->dev_link.link_status = ETH_LINK_UP;
> >>>>>>>
> >>>>>>> 	......
> >>>>>>>
> >>>>>>> 	_rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_INTR_LSC, NULL);
> >>>>>>>
> >>>>>>> 	......
> >>>>>>> }
> >>>>>>>
> >>>>>>> static void
> >>>>>>> destroy_device(int vid)
> >>>>>>> {
> >>>>>>> 	......
> >>>>>>>
> >>>>>>> 	eth_dev->data->dev_link.link_status = ETH_LINK_DOWN;
> >>>>>>>
> >>>>>>> 	......
> >>>>>>>
> >>>>>>> 	_rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_INTR_LSC, NULL);
> >>>>>>>
> >>>>>>> 	......
> >>>>>>> }
> >>>>>>>
> >>>>>>> They are the callbacks for vhost library.
> >>>>>>>
> >>>>>>> When a frontend (e.g. QEMU) is connected to this vhost backend
> >>>>>>> and the frontend virtio device becomes ready, new_device() will
> >>>>>>> be called by the vhost library, and the link status will be
> >>>>>>> updated to UP.
> >>>>>>>
> >>>>>>> And when e.g. the connection is closed, destroy_device() will be
> >>>>>>> called by the vhost library, and the link status will be updated
> >>>>>>> to DOWN.
> >>>>>>
> >>>>>> Got it. This behavior is similar for virtual PMDs. Provide static link
> >>>>>> information and update link as UP during start and update it as DOWN during stop.
> >>>>> No, the link status isn't updated during vhost PMD start
> >>>>> and stop. When the vhost PMD has been started, the link
> >>>>> status still may be DOWN. The link status becomes UP only
> >>>>> when the QEMU (it's another virtual machine process which
> >>>>> has a virtio device) connects to this vhost PMD via a UNIX
> >>>>> socket and the virtio driver in the virtual machine has
> >>>>> setup the virtio device of the virtual machine.
> >>>>>
> >>>>> So if vhost PMD reports the link status as DOWN, it means
> >>>>> there is no QEMU (virtual machine) connects to it or the
> >>>>> virtio device in the virtual machine hasn't been setup.
> >>>>> (PS. The frontend can also be virtio-user PMD besides QEMU)
> >>>> I believe announcing link feature reporting on virtual pmds still in gray area,
> >>>> but because of qemu involvement in vhost case, I will keep link feature but will
> >>>> drop link event.
> >>>
> >>> AFAIK, link status means we can get link status through APIs like 
> >>> rte_eth_link_get(); while link status event means applications can 
> >>> register link status events, and those events get called if link status 
> >>> is changed.
> >>>
> >>> If I understand it correctly, for vhost, we can keep both link status 
> >>> and link status event for vhost.
> >>>
> >>> Could you specify the reason why we remove link status event?
> >>
> >> Hi Jianfeng,
> >>
> >> I think problem is the definition of the features are not clear, that is why I
> >> started a doc to document them (doc/guides/nics/features.rst)
> >>
> >> "Link status", I think we agree on this one. PMD should provide up-to-date,
> >> valid link data on rte_eth_dev_data->dev_link. So that this link information can
> >> be get by rte_eth_link_get(), rte_eth_link_get_nowait() ethdev APIs.
> >>
> >> rte_eth_link_get() uses dev_ops->link_update() to get the latest link
> >> information, for virtual PMDs, including vhost, this function does nothing,
> >> because there is no actual physical link. That is why I was not sure advertising
> >> link status feature for vhost, and other virtual PMDs doesn't report this
> >> feature. After Tiwei's comment that link status shows that qemu connected to
> >> vhost, added this feature back to vhost PMD.
> >>
> >>
> >> "Link status event", can be
> >> a- PMD calls application callback in link change
> >> b- PMD registers interrupt handler for link status change interrupts
> >>
> >> Based on how other PMDs report this feature, I believe it is (b), and I have
> >> documented that way. And vhost "Link status event" feature removed based on this.
> >>
> >> There are some set of config options and flags to control the LSC interrupt,
> >> that also effects rte_eth_link_get() and rte_eth_link_get_nowait() APIs, I
> >> believe that is the main concern here.
> >>
> >> As commented above, I don't understand why calling user register callback for
> >> link change event is something on PMD decision, it should be default behavior
> >> for PMD.
> > 
> > Do you mean you don't understand why vhost PMD calls
> > _rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_INTR_LSC, NULL)
> > in new_device() and destroy_device()? While other physical
> > NIC drivers call this in their interrupt handlers which
> > are registered via rte_intr_callback_register().
> 
> No, I understand this part.
> 
> My question is why not all PMDs call callback_process() when link changed as a
> requirement? Why PMD is giving decision to call or not call callback_process() ?

I got your point now. Maybe because in the library level,
it just knows an interrupt happened but doesn't know whether
it's a LSC interrupt or not (sometimes it needs to check
some vendor specific registers). So the simplest way is
just to ask each PMD to call callback_process() directly.
But unfortunately, some PMDs didn't do it as we expected..
(I didn't look into other PMDs, so I'm not sure about the
details.)

Thanks


> 
> > 
> > The new_device() and destroy_device() in vhost PMD can be
> > treated as something like the interrupt handlers in physical
> > NIC drivers. They are the callbacks which are registered via
> > rte_vhost_driver_callback_register() and will be called by
> > vhost library when vhost events happen (and they should be
> > translated to link status change events when we try to wrap
> > it as a net PMD).
> > 
> > Thanks
> > 
> >> What is the point of leaving this into PMD and think this as a feature
> >> of PMD?
> >>
> >>
> >> Overall, practical reason of this table is to inform developer/user about PMD
> >> features, which is indeed device + driver features, and help her to in
> >> development or on setting expectations. We can always discuss what helps more to
> >> developer/user and update the features table.
> >>
> >> Thanks,
> >> ferruh
> >>
> >>>
> >>> Thanks,
> >>> Jianfeng
> >>>
> >>>>
> >>>> Will send a new patch to reflect this.
> >>>>
> >>>> Thanks,
> >>>> ferruh
> >>>
> >>
> 

Re: [dpdk-dev] [PATCH] drivers/net: update link status

[Ferruh Yigit]

On 4/18/2018 1:08 PM, Tiwei Bie wrote:
> On Wed, Apr 18, 2018 at 12:44:54PM +0100, Ferruh Yigit wrote:
>> On 4/18/2018 12:36 PM, Tiwei Bie wrote:
>>> On Wed, Apr 18, 2018 at 11:42:56AM +0100, Ferruh Yigit wrote:
>>>> On 4/18/2018 7:49 AM, Tan, Jianfeng wrote:
>>>>> Hi Ferruh,
>>>>>
>>>>>
>>>>> On 4/17/2018 7:26 PM, Ferruh Yigit wrote:
>>>>>> On 4/17/2018 5:54 AM, Tiwei Bie wrote:
>>>>>>> On Mon, Apr 16, 2018 at 05:10:24PM +0100, Ferruh Yigit wrote:
>>>>>>>> On 4/14/2018 11:55 AM, Tiwei Bie wrote:
>>>>>>>>> On Fri, Apr 13, 2018 at 10:53:55PM +0100, Ferruh Yigit wrote:
>>>>>>>>>> On 4/10/2018 4:41 PM, Tiwei Bie wrote:
>>>>>>>>>>> On Tue, Mar 13, 2018 at 06:05:34PM +0000, Ferruh Yigit wrote:
>>>>>>>>>>>> Update link status related feature document items and minor updates in
>>>>>>>>>>>> some link status related functions.
>>>>>>>>>>>>
>>>>>>>>>>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>>>>>>>>>>>> ---
>>>>>>>>>>>>   doc/guides/nics/features/fm10k.ini      | 2 ++
>>>>>>>>>>>>   doc/guides/nics/features/fm10k_vf.ini   | 2 ++
>>>>>>>>>>>>   doc/guides/nics/features/i40e_vf.ini    | 1 +
>>>>>>>>>>>>   doc/guides/nics/features/igb_vf.ini     | 1 +
>>>>>>>>>>>>   doc/guides/nics/features/qede.ini       | 1 -
>>>>>>>>>>>>   doc/guides/nics/features/qede_vf.ini    | 1 -
>>>>>>>>>>>>   doc/guides/nics/features/vhost.ini      | 2 --
>>>>>>>>>>>>   doc/guides/nics/features/virtio_vec.ini | 1 +
>>>>>>>>>>>>   drivers/net/e1000/em_ethdev.c           | 2 +-
>>>>>>>>>>>>   drivers/net/ena/ena_ethdev.c            | 2 +-
>>>>>>>>>>>>   drivers/net/fm10k/fm10k_ethdev.c        | 6 ++----
>>>>>>>>>>>>   drivers/net/i40e/i40e_ethdev_vf.c       | 2 +-
>>>>>>>>>>>>   drivers/net/ixgbe/ixgbe_ethdev.c        | 2 +-
>>>>>>>>>>>>   drivers/net/mlx4/mlx4_ethdev.c          | 2 +-
>>>>>>>>>>>>   drivers/net/mlx5/mlx5_ethdev.c          | 2 +-
>>>>>>>>>>>>   15 files changed, 15 insertions(+), 14 deletions(-)
>>>>>>>>>>> [...]
>>>>>>>>>>>> diff --git a/doc/guides/nics/features/vhost.ini b/doc/guides/nics/features/vhost.ini
>>>>>>>>>>>> index dffd1f493..31302745a 100644
>>>>>>>>>>>> --- a/doc/guides/nics/features/vhost.ini
>>>>>>>>>>>> +++ b/doc/guides/nics/features/vhost.ini
>>>>>>>>>>>> @@ -4,8 +4,6 @@
>>>>>>>>>>>>   ; Refer to default.ini for the full list of available PMD features.
>>>>>>>>>>>>   ;
>>>>>>>>>>>>   [Features]
>>>>>>>>>>>> -Link status          = Y
>>>>>>>>>>>> -Link status event    = Y
>>>>>>>>>>> I think vhost PMD supports above features.
>>>>>>>>>> I am not able to find where it is supported.
>>>>>>>>>>
>>>>>>>>>> Some virtual PMDs report fixed link, with empty link_update() dev_ops, and they
>>>>>>>>>> are not reported as supporting Link status, as far as I can see vhost also one
>>>>>>>>>> of them.
>>>>>>>>>>
>>>>>>>>>> And for Link status event, PMD needs to support LSC interrupts and should
>>>>>>>>>> register interrupt handler for it, which I can't find for vhost.
>>>>>>>>>>
>>>>>>>>>> I will send next version without updating above one, please point me where these
>>>>>>>>>> support added if I missed them.
>>>>>>>>> In drivers/net/vhost/rte_eth_vhost.c you could find below functions:
>>>>>>>>>
>>>>>>>>> static int
>>>>>>>>> new_device(int vid)
>>>>>>>>> {
>>>>>>>>> 	......
>>>>>>>>>
>>>>>>>>> 	eth_dev->data->dev_link.link_status = ETH_LINK_UP;
>>>>>>>>>
>>>>>>>>> 	......
>>>>>>>>>
>>>>>>>>> 	_rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_INTR_LSC, NULL);
>>>>>>>>>
>>>>>>>>> 	......
>>>>>>>>> }
>>>>>>>>>
>>>>>>>>> static void
>>>>>>>>> destroy_device(int vid)
>>>>>>>>> {
>>>>>>>>> 	......
>>>>>>>>>
>>>>>>>>> 	eth_dev->data->dev_link.link_status = ETH_LINK_DOWN;
>>>>>>>>>
>>>>>>>>> 	......
>>>>>>>>>
>>>>>>>>> 	_rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_INTR_LSC, NULL);
>>>>>>>>>
>>>>>>>>> 	......
>>>>>>>>> }
>>>>>>>>>
>>>>>>>>> They are the callbacks for vhost library.
>>>>>>>>>
>>>>>>>>> When a frontend (e.g. QEMU) is connected to this vhost backend
>>>>>>>>> and the frontend virtio device becomes ready, new_device() will
>>>>>>>>> be called by the vhost library, and the link status will be
>>>>>>>>> updated to UP.
>>>>>>>>>
>>>>>>>>> And when e.g. the connection is closed, destroy_device() will be
>>>>>>>>> called by the vhost library, and the link status will be updated
>>>>>>>>> to DOWN.
>>>>>>>>
>>>>>>>> Got it. This behavior is similar for virtual PMDs. Provide static link
>>>>>>>> information and update link as UP during start and update it as DOWN during stop.
>>>>>>> No, the link status isn't updated during vhost PMD start
>>>>>>> and stop. When the vhost PMD has been started, the link
>>>>>>> status still may be DOWN. The link status becomes UP only
>>>>>>> when the QEMU (it's another virtual machine process which
>>>>>>> has a virtio device) connects to this vhost PMD via a UNIX
>>>>>>> socket and the virtio driver in the virtual machine has
>>>>>>> setup the virtio device of the virtual machine.
>>>>>>>
>>>>>>> So if vhost PMD reports the link status as DOWN, it means
>>>>>>> there is no QEMU (virtual machine) connects to it or the
>>>>>>> virtio device in the virtual machine hasn't been setup.
>>>>>>> (PS. The frontend can also be virtio-user PMD besides QEMU)
>>>>>> I believe announcing link feature reporting on virtual pmds still in gray area,
>>>>>> but because of qemu involvement in vhost case, I will keep link feature but will
>>>>>> drop link event.
>>>>>
>>>>> AFAIK, link status means we can get link status through APIs like 
>>>>> rte_eth_link_get(); while link status event means applications can 
>>>>> register link status events, and those events get called if link status 
>>>>> is changed.
>>>>>
>>>>> If I understand it correctly, for vhost, we can keep both link status 
>>>>> and link status event for vhost.
>>>>>
>>>>> Could you specify the reason why we remove link status event?
>>>>
>>>> Hi Jianfeng,
>>>>
>>>> I think problem is the definition of the features are not clear, that is why I
>>>> started a doc to document them (doc/guides/nics/features.rst)
>>>>
>>>> "Link status", I think we agree on this one. PMD should provide up-to-date,
>>>> valid link data on rte_eth_dev_data->dev_link. So that this link information can
>>>> be get by rte_eth_link_get(), rte_eth_link_get_nowait() ethdev APIs.
>>>>
>>>> rte_eth_link_get() uses dev_ops->link_update() to get the latest link
>>>> information, for virtual PMDs, including vhost, this function does nothing,
>>>> because there is no actual physical link. That is why I was not sure advertising
>>>> link status feature for vhost, and other virtual PMDs doesn't report this
>>>> feature. After Tiwei's comment that link status shows that qemu connected to
>>>> vhost, added this feature back to vhost PMD.
>>>>
>>>>
>>>> "Link status event", can be
>>>> a- PMD calls application callback in link change
>>>> b- PMD registers interrupt handler for link status change interrupts
>>>>
>>>> Based on how other PMDs report this feature, I believe it is (b), and I have
>>>> documented that way. And vhost "Link status event" feature removed based on this.
>>>>
>>>> There are some set of config options and flags to control the LSC interrupt,
>>>> that also effects rte_eth_link_get() and rte_eth_link_get_nowait() APIs, I
>>>> believe that is the main concern here.
>>>>
>>>> As commented above, I don't understand why calling user register callback for
>>>> link change event is something on PMD decision, it should be default behavior
>>>> for PMD.
>>>
>>> Do you mean you don't understand why vhost PMD calls
>>> _rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_INTR_LSC, NULL)
>>> in new_device() and destroy_device()? While other physical
>>> NIC drivers call this in their interrupt handlers which
>>> are registered via rte_intr_callback_register().
>>
>> No, I understand this part.
>>
>> My question is why not all PMDs call callback_process() when link changed as a
>> requirement? Why PMD is giving decision to call or not call callback_process() ?
> 
> I got your point now. Maybe because in the library level,
> it just knows an interrupt happened but doesn't know whether
> it's a LSC interrupt or not (sometimes it needs to check
> some vendor specific registers). So the simplest way is
> just to ask each PMD to call callback_process() directly.

Yep.

> But unfortunately, some PMDs didn't do it as we expected..
> (I didn't look into other PMDs, so I'm not sure about the
> details.)

Yes, some PMDs doesn't call callback_process() even they support link status and
link status interrupts. This may be because this is not documented clearly.
Best option can be handling this in ethdev somehow.

> 
> Thanks
> 
> 
>>
>>>
>>> The new_device() and destroy_device() in vhost PMD can be
>>> treated as something like the interrupt handlers in physical
>>> NIC drivers. They are the callbacks which are registered via
>>> rte_vhost_driver_callback_register() and will be called by
>>> vhost library when vhost events happen (and they should be
>>> translated to link status change events when we try to wrap
>>> it as a net PMD).
>>>
>>> Thanks
>>>
>>>> What is the point of leaving this into PMD and think this as a feature
>>>> of PMD?
>>>>
>>>>
>>>> Overall, practical reason of this table is to inform developer/user about PMD
>>>> features, which is indeed device + driver features, and help her to in
>>>> development or on setting expectations. We can always discuss what helps more to
>>>> developer/user and update the features table.
>>>>
>>>> Thanks,
>>>> ferruh
>>>>
>>>>>
>>>>> Thanks,
>>>>> Jianfeng
>>>>>
>>>>>>
>>>>>> Will send a new patch to reflect this.
>>>>>>
>>>>>> Thanks,
>>>>>> ferruh
>>>>>
>>>>
>>

Re: [dpdk-dev] [PATCH v3] drivers/net: update link status

[Ferruh Yigit]

On 4/17/2018 12:30 PM, Ferruh Yigit wrote:
> Update link status related feature document items and minor updates in
> some link status related functions.
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> Acked-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>

Applied to dpdk-next-net/master, thanks.