[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