[dpdk-dev] [PATCH] net/ixgbe: fix vlan insert parameter type and its use

Lu, Wenzhuo wenzhuo.lu at intel.com
Wed Oct 19 07:13:45 CEST 2016


Hi Daniels,


> -----Original Message-----
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of E. Scott Daniels
> Sent: Wednesday, October 19, 2016 3:13 AM
> To: Zhang, Helin; Iremonger, Bernard
> Cc: dev at dpdk.org; az5157 at att.com; E. Scott Daniels
> Subject: [dpdk-dev] [PATCH] net/ixgbe: fix vlan insert parameter type and its use
> 
> The final parameter to rte_pmd_ixgbe_set_vf_vlan_insert is uint8_t and treated
> as a binary flag when it needs to be a uint16_t and treated as a VLAN id.  The
> data sheet (sect 8.2.3.27.13) describes the right most 16 bits as the VLAN id that
> is to be inserted; the
> 16.11  code is accepting only a 1 or 0 thus effectively only allowing the VLAN id 1
> to be inserted (0 disables the insertion setting).
> 
> This patch changes the final parm name to represent the data that is being
> accepted (vlan_id), changes the type to permit all valid VLAN ids, and validates
> the parameter based on the range of 0 to 4095. Corresponding changes to
> prototype and documentation in the .h file.
> 
> Fixes:  49e248223e9f71 ("net/ixgbe: add API for VF management")
> 
> Signed-off-by: E. Scott Daniels <daniels at research.att.com>
> ---
>  drivers/net/ixgbe/ixgbe_ethdev.c  | 8 ++++----
> drivers/net/ixgbe/rte_pmd_ixgbe.h | 9 +++++----
>  2 files changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
> index 4ca5747..316af73 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -4727,7 +4727,7 @@ rte_pmd_ixgbe_set_vf_mac_anti_spoof(uint8_t port,
> uint16_t vf, uint8_t on)  }
> 
>  int
> -rte_pmd_ixgbe_set_vf_vlan_insert(uint8_t port, uint16_t vf, uint8_t on)
> +rte_pmd_ixgbe_set_vf_vlan_insert(uint8_t port, uint16_t vf, uint16_t
> +vlan_id)
>  {
>  	struct ixgbe_hw *hw;
>  	uint32_t ctrl;
> @@ -4742,13 +4742,13 @@ rte_pmd_ixgbe_set_vf_vlan_insert(uint8_t port,
> uint16_t vf, uint8_t on)
>  	if (vf >= dev_info.max_vfs)
>  		return -EINVAL;
> 
> -	if (on > 1)
> +	if (vlan_id > 4095)
>  		return -EINVAL;
> 
>  	hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
>  	ctrl = IXGBE_READ_REG(hw, IXGBE_VMVIR(vf));
> -	if (on) {
> -		ctrl = on;
> +	if (vlan_id) {
> +		ctrl = vlan_id;
I believe this patch is a good idea of an enhancement. But you cannot leverage the existing code like this.
The register IXGBE_VMVIR is only for enable/disable. We cannot write the VLAN ID into it.
If you want to merge the 2 things, enabling/disabling and setting VLAN ID together. I think we need a totally new implementation. So NACK.

>  		ctrl |= IXGBE_VMVIR_VLANA_DEFAULT;
>  	} else {
>  		ctrl = 0;
> diff --git a/drivers/net/ixgbe/rte_pmd_ixgbe.h
> b/drivers/net/ixgbe/rte_pmd_ixgbe.h
> index 2fdf530..c2fb826 100644
> --- a/drivers/net/ixgbe/rte_pmd_ixgbe.h
> +++ b/drivers/net/ixgbe/rte_pmd_ixgbe.h
> @@ -99,16 +99,17 @@ int rte_pmd_ixgbe_set_vf_mac_anti_spoof(uint8_t port,
> uint16_t vf, uint8_t on);
>   *    The port identifier of the Ethernet device.
>   * @param vf
>   *    ID specifying VF.
> - * @param on
> - *    1 - Enable VF's vlan insert.
> - *    0 - Disable VF's vlan insert
> + * @param vlan_id
> + *    0 - Disable VF's vlan insert.
> + *    n - Enable; n is inserted as the vlan id.
>   *
>   * @return
>   *   - (0) if successful.
>   *   - (-ENODEV) if *port* invalid.
>   *   - (-EINVAL) if bad parameter.
>   */
> -int rte_pmd_ixgbe_set_vf_vlan_insert(uint8_t port, uint16_t vf, uint8_t on);
> +int rte_pmd_ixgbe_set_vf_vlan_insert(uint8_t port, uint16_t vf,
> +		uint16_t vlan_id);
> 
>  /**
>   * Enable/Disable tx loopback
> --
> 1.9.1



More information about the dev mailing list