diff mbox

[dpdk-dev,v2,15/32] net/i40e: add VF vlan strip func

Message ID 1481081535-37448-16-git-send-email-wenzhuo.lu@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
checkpatch/checkpatch success coding style OK

Commit Message

Wenzhuo Lu Dec. 7, 2016, 3:31 a.m. UTC 
  Add a function to configure vlan strip enable/disable for specific
SRIOV VF device.

Signed-off-by: Chen Jing D(Mark) <jing.d.chen@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c            | 26 ++++++++++++++++++++++++++
 drivers/net/i40e/rte_pmd_i40e.h           | 20 ++++++++++++++++++++
 drivers/net/i40e/rte_pmd_i40e_version.map |  1 +
 3 files changed, 47 insertions(+)

Comments

Ferruh Yigit Dec. 7, 2016, 2:18 p.m. UTC | #1 
On 12/7/2016 3:31 AM, Wenzhuo Lu wrote:
> Add a function to configure vlan strip enable/disable for specific
> SRIOV VF device.
> 
> Signed-off-by: Chen Jing D(Mark) <jing.d.chen@intel.com>
> ---

<...>

> +
> +/* Set vlan strip on/off for specific VF from host */
> +int
> +rte_pmd_i40e_set_vf_vlan_stripq(uint8_t port, uint16_t vf_id, uint8_t on)
> +{
> +	struct rte_eth_dev *dev;
> +	struct i40e_pf *pf;
> +	struct i40e_vsi *vsi;
> +
> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
> +
> +	dev = &rte_eth_devices[port];
> +	pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
> +
> +	if (vf_id > pf->vf_num - 1 || !pf->vfs) {
> +		PMD_DRV_LOG(ERR, "Invalid argument.");
> +		return -EINVAL;
> +	}
> +
> +	vsi = pf->vfs[vf_id].vsi;
> +
> +	if (vsi)
> +		return i40e_vsi_config_vlan_stripping(vsi, !!on);
> +	else

if vd_if is valid, can vsi be NULL? If so this check may be required in
some prev patches too.

> +		return -EINVAL;
> +}
> diff --git a/drivers/net/i40e/rte_pmd_i40e.h b/drivers/net/i40e/rte_pmd_i40e.h
> index ca5e05a..043ae62 100644
> --- a/drivers/net/i40e/rte_pmd_i40e.h
> +++ b/drivers/net/i40e/rte_pmd_i40e.h
> @@ -187,4 +187,24 @@ int rte_pmd_i40e_set_vf_multicast_promisc(uint8_t port,
>  int rte_pmd_i40e_set_vf_mac_addr(uint8_t port, uint16_t vf_id,
>  				 struct ether_addr *mac_addr);
>  
> +/**
> + * Enable/Disable vf vlan strip for all queues in a pool
> + *
> + * @param port
> + *    The port identifier of the Ethernet device.
> + * @param vf
> + *    ID specifying VF.
> + * @param on
> + *    1 - Enable VF's vlan strip on RX queues.
> + *    0 - Disable VF's vlan strip on RX queues.
> + *
> + * @return
> + *   - (0) if successful.
> + *   - (-ENOTSUP) if hardware doesn't support this feature.

Is this error type returned?

> + *   - (-ENODEV) if *port* invalid.
> + *   - (-EINVAL) if bad parameter.
> + */
<...>
Chen, Jing D Dec. 8, 2016, 9:10 a.m. UTC | #2 
HI, Ferruh,

Best Regards,
Mark


> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Wednesday, December 07, 2016 10:18 PM
> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; dev@dpdk.org
> Cc: Chen, Jing D <jing.d.chen@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v2 15/32] net/i40e: add VF vlan strip func
> 
> On 12/7/2016 3:31 AM, Wenzhuo Lu wrote:
> > Add a function to configure vlan strip enable/disable for specific
> > SRIOV VF device.
> >
> > Signed-off-by: Chen Jing D(Mark) <jing.d.chen@intel.com>
> > ---
> 
> <...>
> 
> > +
> > +/* Set vlan strip on/off for specific VF from host */
> > +int
> > +rte_pmd_i40e_set_vf_vlan_stripq(uint8_t port, uint16_t vf_id, uint8_t on)
> > +{
> > +	struct rte_eth_dev *dev;
> > +	struct i40e_pf *pf;
> > +	struct i40e_vsi *vsi;
> > +
> > +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
> > +
> > +	dev = &rte_eth_devices[port];
> > +	pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
> > +
> > +	if (vf_id > pf->vf_num - 1 || !pf->vfs) {
> > +		PMD_DRV_LOG(ERR, "Invalid argument.");
> > +		return -EINVAL;
> > +	}
> > +
> > +	vsi = pf->vfs[vf_id].vsi;
> > +
> > +	if (vsi)
> > +		return i40e_vsi_config_vlan_stripping(vsi, !!on);
> > +	else
> 
> if vd_if is valid, can vsi be NULL? If so this check may be required in
> some prev patches too.

It's a little impossible. This sanity check just make the code stronger.

> 
> > +		return -EINVAL;
> > +}
> > diff --git a/drivers/net/i40e/rte_pmd_i40e.h b/drivers/net/i40e/rte_pmd_i40e.h
> > index ca5e05a..043ae62 100644
> > --- a/drivers/net/i40e/rte_pmd_i40e.h
> > +++ b/drivers/net/i40e/rte_pmd_i40e.h
> > @@ -187,4 +187,24 @@ int rte_pmd_i40e_set_vf_multicast_promisc(uint8_t port,
> >  int rte_pmd_i40e_set_vf_mac_addr(uint8_t port, uint16_t vf_id,
> >  				 struct ether_addr *mac_addr);
> >
> > +/**
> > + * Enable/Disable vf vlan strip for all queues in a pool
> > + *
> > + * @param port
> > + *    The port identifier of the Ethernet device.
> > + * @param vf
> > + *    ID specifying VF.
> > + * @param on
> > + *    1 - Enable VF's vlan strip on RX queues.
> > + *    0 - Disable VF's vlan strip on RX queues.
> > + *
> > + * @return
> > + *   - (0) if successful.
> > + *   - (-ENOTSUP) if hardware doesn't support this feature.
> 
> Is this error type returned?

Good catch. Only -EINVAL and -ENODEV would be returned.

> 
> > + *   - (-ENODEV) if *port* invalid.
> > + *   - (-EINVAL) if bad parameter.
> > + */
> <...>
Ferruh Yigit Dec. 8, 2016, 10:43 a.m. UTC | #3 
On 12/8/2016 9:10 AM, Chen, Jing D wrote:
> HI, Ferruh,
> 
> Best Regards,
> Mark
> 
> 
>> -----Original Message-----
>> From: Yigit, Ferruh
>> Sent: Wednesday, December 07, 2016 10:18 PM
>> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; dev@dpdk.org
>> Cc: Chen, Jing D <jing.d.chen@intel.com>
>> Subject: Re: [dpdk-dev] [PATCH v2 15/32] net/i40e: add VF vlan strip func
>>
>> On 12/7/2016 3:31 AM, Wenzhuo Lu wrote:
>>> Add a function to configure vlan strip enable/disable for specific
>>> SRIOV VF device.
>>>
>>> Signed-off-by: Chen Jing D(Mark) <jing.d.chen@intel.com>
>>> ---
>>
>> <...>
>>
>>> +
>>> +/* Set vlan strip on/off for specific VF from host */
>>> +int
>>> +rte_pmd_i40e_set_vf_vlan_stripq(uint8_t port, uint16_t vf_id, uint8_t on)
>>> +{
>>> +	struct rte_eth_dev *dev;
>>> +	struct i40e_pf *pf;
>>> +	struct i40e_vsi *vsi;
>>> +
>>> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
>>> +
>>> +	dev = &rte_eth_devices[port];
>>> +	pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
>>> +
>>> +	if (vf_id > pf->vf_num - 1 || !pf->vfs) {
>>> +		PMD_DRV_LOG(ERR, "Invalid argument.");
>>> +		return -EINVAL;
>>> +	}
>>> +
>>> +	vsi = pf->vfs[vf_id].vsi;
>>> +
>>> +	if (vsi)
>>> +		return i40e_vsi_config_vlan_stripping(vsi, !!on);
>>> +	else
>>
>> if vd_if is valid, can vsi be NULL? If so this check may be required in
>> some prev patches too.
> 
> It's a little impossible. This sanity check just make the code stronger.
> 

If it is impossible, do you agree to remove this? And if this can be
possible we must update other patches, almost all other patches assume
this can't be NULL.


>>
>>> +		return -EINVAL;
>>> +}
>>> diff --git a/drivers/net/i40e/rte_pmd_i40e.h b/drivers/net/i40e/rte_pmd_i40e.h
>>> index ca5e05a..043ae62 100644
>>> --- a/drivers/net/i40e/rte_pmd_i40e.h
>>> +++ b/drivers/net/i40e/rte_pmd_i40e.h
>>> @@ -187,4 +187,24 @@ int rte_pmd_i40e_set_vf_multicast_promisc(uint8_t port,
>>>  int rte_pmd_i40e_set_vf_mac_addr(uint8_t port, uint16_t vf_id,
>>>  				 struct ether_addr *mac_addr);
>>>
>>> +/**
>>> + * Enable/Disable vf vlan strip for all queues in a pool
>>> + *
>>> + * @param port
>>> + *    The port identifier of the Ethernet device.
>>> + * @param vf
>>> + *    ID specifying VF.
>>> + * @param on
>>> + *    1 - Enable VF's vlan strip on RX queues.
>>> + *    0 - Disable VF's vlan strip on RX queues.
>>> + *
>>> + * @return
>>> + *   - (0) if successful.
>>> + *   - (-ENOTSUP) if hardware doesn't support this feature.
>>
>> Is this error type returned?
> 
> Good catch. Only -EINVAL and -ENODEV would be returned.
> 
>>
>>> + *   - (-ENODEV) if *port* invalid.
>>> + *   - (-EINVAL) if bad parameter.
>>> + */
>> <...>
Chen, Jing D Dec. 9, 2016, 3:07 a.m. UTC | #4 
Hi, Ferruh,

> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Thursday, December 08, 2016 6:43 PM
> To: Chen, Jing D <jing.d.chen@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>;
> dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2 15/32] net/i40e: add VF vlan strip func
> 
> On 12/8/2016 9:10 AM, Chen, Jing D wrote:
> > HI, Ferruh,
> >
> > Best Regards,
> > Mark
> >
> >
> >> -----Original Message-----
> >> From: Yigit, Ferruh
> >> Sent: Wednesday, December 07, 2016 10:18 PM
> >> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; dev@dpdk.org
> >> Cc: Chen, Jing D <jing.d.chen@intel.com>
> >> Subject: Re: [dpdk-dev] [PATCH v2 15/32] net/i40e: add VF vlan strip func
> >>
> >> On 12/7/2016 3:31 AM, Wenzhuo Lu wrote:
> >>> Add a function to configure vlan strip enable/disable for specific
> >>> SRIOV VF device.
> >>>
> >>> Signed-off-by: Chen Jing D(Mark) <jing.d.chen@intel.com>
> >>> ---
> >>
> >> <...>
> >>
> >>> +
> >>> +/* Set vlan strip on/off for specific VF from host */
> >>> +int
> >>> +rte_pmd_i40e_set_vf_vlan_stripq(uint8_t port, uint16_t vf_id, uint8_t on)
> >>> +{
> >>> +	struct rte_eth_dev *dev;
> >>> +	struct i40e_pf *pf;
> >>> +	struct i40e_vsi *vsi;
> >>> +
> >>> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
> >>> +
> >>> +	dev = &rte_eth_devices[port];
> >>> +	pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
> >>> +
> >>> +	if (vf_id > pf->vf_num - 1 || !pf->vfs) {
> >>> +		PMD_DRV_LOG(ERR, "Invalid argument.");
> >>> +		return -EINVAL;
> >>> +	}
> >>> +
> >>> +	vsi = pf->vfs[vf_id].vsi;
> >>> +
> >>> +	if (vsi)
> >>> +		return i40e_vsi_config_vlan_stripping(vsi, !!on);
> >>> +	else
> >>
> >> if vd_if is valid, can vsi be NULL? If so this check may be required in
> >> some prev patches too.
> >
> > It's a little impossible. This sanity check just make the code stronger.
> >
> 
> If it is impossible, do you agree to remove this? And if this can be
> possible we must update other patches, almost all other patches assume
> this can't be NULL.

I'll recommend other patches to add it, too.
The reason is we can't image if there is some code change have impact in
future, the necessary sanity check in slow patch make code stronger.
diff mbox

Patch

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 832995a..253209b 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -10335,3 +10335,29 @@  static void i40e_set_default_mac_addr(struct rte_eth_dev *dev,
 
 	return 0;
 }
+
+/* Set vlan strip on/off for specific VF from host */
+int
+rte_pmd_i40e_set_vf_vlan_stripq(uint8_t port, uint16_t vf_id, uint8_t on)
+{
+	struct rte_eth_dev *dev;
+	struct i40e_pf *pf;
+	struct i40e_vsi *vsi;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
+
+	dev = &rte_eth_devices[port];
+	pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
+
+	if (vf_id > pf->vf_num - 1 || !pf->vfs) {
+		PMD_DRV_LOG(ERR, "Invalid argument.");
+		return -EINVAL;
+	}
+
+	vsi = pf->vfs[vf_id].vsi;
+
+	if (vsi)
+		return i40e_vsi_config_vlan_stripping(vsi, !!on);
+	else
+		return -EINVAL;
+}
diff --git a/drivers/net/i40e/rte_pmd_i40e.h b/drivers/net/i40e/rte_pmd_i40e.h
index ca5e05a..043ae62 100644
--- a/drivers/net/i40e/rte_pmd_i40e.h
+++ b/drivers/net/i40e/rte_pmd_i40e.h
@@ -187,4 +187,24 @@  int rte_pmd_i40e_set_vf_multicast_promisc(uint8_t port,
 int rte_pmd_i40e_set_vf_mac_addr(uint8_t port, uint16_t vf_id,
 				 struct ether_addr *mac_addr);
 
+/**
+ * Enable/Disable vf vlan strip for all queues in a pool
+ *
+ * @param port
+ *    The port identifier of the Ethernet device.
+ * @param vf
+ *    ID specifying VF.
+ * @param on
+ *    1 - Enable VF's vlan strip on RX queues.
+ *    0 - Disable VF's vlan strip on RX queues.
+ *
+ * @return
+ *   - (0) if successful.
+ *   - (-ENOTSUP) if hardware doesn't support this feature.
+ *   - (-ENODEV) if *port* invalid.
+ *   - (-EINVAL) if bad parameter.
+ */
+int
+rte_pmd_i40e_set_vf_vlan_stripq(uint8_t port, uint16_t vf, uint8_t on);
+
 #endif /* _PMD_I40E_H_ */
diff --git a/drivers/net/i40e/rte_pmd_i40e_version.map b/drivers/net/i40e/rte_pmd_i40e_version.map
index 64ba93a..2497b3e 100644
--- a/drivers/net/i40e/rte_pmd_i40e_version.map
+++ b/drivers/net/i40e/rte_pmd_i40e_version.map
@@ -13,5 +13,6 @@  DPDK_17.02 {
 	rte_pmd_i40e_set_vf_unicast_promisc;
 	rte_pmd_i40e_set_vf_multicast_promisc;
 	rte_pmd_i40e_set_vf_mac_addr;
+	rte_pmd_i40e_set_vf_vlan_stripq;
 
 } DPDK_2.0;