[v2] net/i40e: allow VF to configure pctype mapping

Message ID 1558575434-57309-1-git-send-email-beilei.xing@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Qi Zhang
Headers
Series [v2] net/i40e: allow VF to configure pctype mapping |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/intel-Performance-Testing success Performance Testing PASS
ci/mellanox-Performance-Testing success Performance Testing PASS
ci/Intel-compilation success Compilation OK

Commit Message

Xing, Beilei May 23, 2019, 1:37 a.m. UTC
  This patch allows VF to get/update/reset pctype
mapping info.

Signed-off-by: Beilei Xing <beilei.xing@intel.com>
---
v2 changes:
 - Allow VF update/reset pctype mapping.

 drivers/net/i40e/rte_pmd_i40e.c | 8 --------
 1 file changed, 8 deletions(-)
  

Comments

Ferruh Yigit May 24, 2019, 1:52 p.m. UTC | #1
On 5/23/2019 2:37 AM, Beilei Xing wrote:
> This patch allows VF to get/update/reset pctype
> mapping info.
> 
> Signed-off-by: Beilei Xing <beilei.xing@intel.com>
> ---
> v2 changes:
>  - Allow VF update/reset pctype mapping.
> 
>  drivers/net/i40e/rte_pmd_i40e.c | 8 --------
>  1 file changed, 8 deletions(-)
> 
> diff --git a/drivers/net/i40e/rte_pmd_i40e.c b/drivers/net/i40e/rte_pmd_i40e.c
> index 7ae78e4..b3bdc61 100644
> --- a/drivers/net/i40e/rte_pmd_i40e.c
> +++ b/drivers/net/i40e/rte_pmd_i40e.c
> @@ -2405,8 +2405,6 @@ int rte_pmd_i40e_flow_type_mapping_reset(uint16_t port)
>  
>  	dev = &rte_eth_devices[port];
>  
> -	if (!is_i40e_supported(dev))
> -		return -ENOTSUP;

Hi Beilei,

Since these are public APIs, these checks are to prevent possible issues user
call these APIs with a port_id that is not 'i40e' at all, in that case the API
still will try to work on that device and this may lead a crash or corruption in
that device.

Instead of removing these checks, can it be possible to extend it to cover both
PF and VF? If there are APIs in this file that requires device explicitly to be
PF or VF, perhaps we can create two version of the check and use appropriate one
for the API.


>  
>  	i40e_set_default_pctype_table(dev);
>  
> @@ -2425,9 +2423,6 @@ int rte_pmd_i40e_flow_type_mapping_get(
>  
>  	dev = &rte_eth_devices[port];
>  
> -	if (!is_i40e_supported(dev))
> -		return -ENOTSUP;
> -
>  	ad = I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
>  
>  	for (i = 0; i < I40E_FLOW_TYPE_MAX; i++) {
> @@ -2453,9 +2448,6 @@ rte_pmd_i40e_flow_type_mapping_update(
>  
>  	dev = &rte_eth_devices[port];
>  
> -	if (!is_i40e_supported(dev))
> -		return -ENOTSUP;
> -
>  	if (count > I40E_FLOW_TYPE_MAX)
>  		return -EINVAL;
>  
>
  
Qi Zhang May 27, 2019, 4:36 p.m. UTC | #2
> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Friday, May 24, 2019 6:53 AM
> To: Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2] net/i40e: allow VF to configure pctype
> mapping
> 
> On 5/23/2019 2:37 AM, Beilei Xing wrote:
> > This patch allows VF to get/update/reset pctype mapping info.
> >
> > Signed-off-by: Beilei Xing <beilei.xing@intel.com>
> > ---
> > v2 changes:
> >  - Allow VF update/reset pctype mapping.
> >
> >  drivers/net/i40e/rte_pmd_i40e.c | 8 --------
> >  1 file changed, 8 deletions(-)
> >
> > diff --git a/drivers/net/i40e/rte_pmd_i40e.c
> > b/drivers/net/i40e/rte_pmd_i40e.c index 7ae78e4..b3bdc61 100644
> > --- a/drivers/net/i40e/rte_pmd_i40e.c
> > +++ b/drivers/net/i40e/rte_pmd_i40e.c
> > @@ -2405,8 +2405,6 @@ int
> > rte_pmd_i40e_flow_type_mapping_reset(uint16_t port)
> >
> >  	dev = &rte_eth_devices[port];
> >
> > -	if (!is_i40e_supported(dev))
> > -		return -ENOTSUP;
> 
> Hi Beilei,
> 
> Since these are public APIs, these checks are to prevent possible issues user
> call these APIs with a port_id that is not 'i40e' at all, in that case the API still
> will try to work on that device and this may lead a crash or corruption in that
> device.
> 
> Instead of removing these checks, can it be possible to extend it to cover both
> PF and VF? If there are APIs in this file that requires device explicitly to be PF
> or VF, perhaps we can create two version of the check and use appropriate one
> for the API.

+1 

> 
> 
> >
> >  	i40e_set_default_pctype_table(dev);
> >
> > @@ -2425,9 +2423,6 @@ int rte_pmd_i40e_flow_type_mapping_get(
> >
> >  	dev = &rte_eth_devices[port];
> >
> > -	if (!is_i40e_supported(dev))
> > -		return -ENOTSUP;
> > -
> >  	ad = I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
> >
> >  	for (i = 0; i < I40E_FLOW_TYPE_MAX; i++) { @@ -2453,9 +2448,6 @@
> > rte_pmd_i40e_flow_type_mapping_update(
> >
> >  	dev = &rte_eth_devices[port];
> >
> > -	if (!is_i40e_supported(dev))
> > -		return -ENOTSUP;
> > -
> >  	if (count > I40E_FLOW_TYPE_MAX)
> >  		return -EINVAL;
> >
> >
  

Patch

diff --git a/drivers/net/i40e/rte_pmd_i40e.c b/drivers/net/i40e/rte_pmd_i40e.c
index 7ae78e4..b3bdc61 100644
--- a/drivers/net/i40e/rte_pmd_i40e.c
+++ b/drivers/net/i40e/rte_pmd_i40e.c
@@ -2405,8 +2405,6 @@  int rte_pmd_i40e_flow_type_mapping_reset(uint16_t port)
 
 	dev = &rte_eth_devices[port];
 
-	if (!is_i40e_supported(dev))
-		return -ENOTSUP;
 
 	i40e_set_default_pctype_table(dev);
 
@@ -2425,9 +2423,6 @@  int rte_pmd_i40e_flow_type_mapping_get(
 
 	dev = &rte_eth_devices[port];
 
-	if (!is_i40e_supported(dev))
-		return -ENOTSUP;
-
 	ad = I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
 
 	for (i = 0; i < I40E_FLOW_TYPE_MAX; i++) {
@@ -2453,9 +2448,6 @@  rte_pmd_i40e_flow_type_mapping_update(
 
 	dev = &rte_eth_devices[port];
 
-	if (!is_i40e_supported(dev))
-		return -ENOTSUP;
-
 	if (count > I40E_FLOW_TYPE_MAX)
 		return -EINVAL;