Re: [dpdk-dev] [PATCH] net/cxgbevf: fix inter-vm traffic when physical link down

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 5/18/2018 7:59 PM, Rahul Lakkireddy wrote:
> From: Shagun Agrawal <shaguna@chelsio.com>
> 
> Add force_link_up devargs to always force link as up for VFs.
> This enables VFs on the same NIC to send traffic to each other
> even when physical link is down.
> 
> Fixes: 011ebc236ddc ("net/cxgbe: add skeleton VF driver")
> 
> Signed-off-by: Shagun Agrawal <shaguna@chelsio.com>
> Signed-off-by: Kumar Sanghvi <kumaras@chelsio.com>
> Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
> ---
>  doc/guides/nics/cxgbe.rst        |  6 ++++++
>  drivers/net/cxgbe/cxgbe.h        |  4 ++++
>  drivers/net/cxgbe/cxgbe_ethdev.c | 14 ++++++--------
>  drivers/net/cxgbe/cxgbe_main.c   | 17 ++++++++++++++++-
>  4 files changed, 32 insertions(+), 9 deletions(-)
> 
> diff --git a/doc/guides/nics/cxgbe.rst b/doc/guides/nics/cxgbe.rst
> index 38d434802..78e391473 100644
> --- a/doc/guides/nics/cxgbe.rst
> +++ b/doc/guides/nics/cxgbe.rst
> @@ -143,6 +143,12 @@ be passed as part of EAL arguments. For example,
>    enabled, the outer VLAN tag is preserved in Q-in-Q packets. Otherwise,
>    the outer VLAN tag is stripped in Q-in-Q packets.
>  
> +- ``force_link_up`` (default **0**)
> +
> +  When set to 1, CXGBEVF PMD always forces link as up for all VFs on
> +  underlying Chelsio NICs. This enables multiple VFs on the same NIC
> +  to send traffic to each other even when the physical link is down.
> +
>  .. _driver-compilation:
>  
>  Driver compilation and testing
> diff --git a/drivers/net/cxgbe/cxgbe.h b/drivers/net/cxgbe/cxgbe.h
> index b1a4e08df..75e0b21a8 100644
> --- a/drivers/net/cxgbe/cxgbe.h
> +++ b/drivers/net/cxgbe/cxgbe.h
> @@ -25,6 +25,9 @@
>  			  ETH_RSS_NONFRAG_IPV6_TCP | \
>  			  ETH_RSS_NONFRAG_IPV6_UDP)
>  
> +#define CXGBE_DEVARG_FORCE_LINK_UP "force_link_up"

You may consider to move existing "CXGBE_DEVARG_KEEP_OVLAN" to here, to keep
them together.

Also can you please add "RTE_PMD_REGISTER_PARAM_STRING" to document devargs,
please put it below RTE_PMD_REGISTER_KMOD_DEP.

> +
> +bool force_linkup(struct adapter *adap);
>  int cxgbe_probe(struct adapter *adapter);
>  int cxgbevf_probe(struct adapter *adapter);
>  void cxgbe_get_speed_caps(struct port_info *pi, u32 *speed_caps);
> @@ -45,5 +48,6 @@ int setup_rss(struct port_info *pi);
>  void cxgbe_enable_rx_queues(struct port_info *pi);
>  void print_port_info(struct adapter *adap);
>  void print_adapter_info(struct adapter *adap);
> +int cxgbe_get_devargs(struct rte_devargs *devargs, const char *key);
>  
>  #endif /* _CXGBE_H_ */
> diff --git a/drivers/net/cxgbe/cxgbe_ethdev.c b/drivers/net/cxgbe/cxgbe_ethdev.c
> index 3ee7c44b1..2f1e4f691 100644
> --- a/drivers/net/cxgbe/cxgbe_ethdev.c
> +++ b/drivers/net/cxgbe/cxgbe_ethdev.c
> @@ -204,19 +204,17 @@ int cxgbe_dev_link_update(struct rte_eth_dev *eth_dev,
>  	struct port_info *pi = (struct port_info *)(eth_dev->data->dev_private);
>  	struct adapter *adapter = pi->adapter;
>  	struct sge *s = &adapter->sge;
> -	struct rte_eth_link *old_link = &eth_dev->data->dev_link;
> +	struct rte_eth_link new_link;
>  	unsigned int work_done, budget = 4;
>  
>  	cxgbe_poll(&s->fw_evtq, NULL, budget, &work_done);
> -	if (old_link->link_status == pi->link_cfg.link_ok)
> -		return -1;  /* link not changed */
>  
> -	eth_dev->data->dev_link.link_status = pi->link_cfg.link_ok;
> -	eth_dev->data->dev_link.link_duplex = ETH_LINK_FULL_DUPLEX;
> -	eth_dev->data->dev_link.link_speed = pi->link_cfg.speed;
> +	new_link.link_status = force_linkup(adapter) ?
> +			       ETH_LINK_UP : pi->link_cfg.link_ok;
> +	new_link.link_duplex = ETH_LINK_FULL_DUPLEX;
> +	new_link.link_speed = pi->link_cfg.speed;
>  
> -	/* link has changed */
> -	return 0;
> +	return rte_eth_linkstatus_set(eth_dev, &new_link);

+1 to use rte_eth_linkstatus_set()