On 12/3/15, 5:05 PM, "Stephen Hemminger" <stephen at networkplumber.org> wrote:
>From: Nachiketa Prachanda <nprachan at brocade.com>
>
>vmxnet3_dev_vlan_offload_set(dev, mask) was incorrectly treating the
>mask parameter as the bitmask for vlan_strip and vlan_filter, whereas
>the mask indicates only what has changed - the values for
>vlan_stripping and vlan_filter needs to be taken from dev_conf.rxmode.
>
>Signed-off-by: Nachiketa Prachanda <nprachan at brocade.com>
>Signed-off-by: Stephen Hemminger <stephen at networkplumber.org>
>---
> drivers/net/vmxnet3/vmxnet3_ethdev.c | 45 ++++++++++++++++++++++--------------
> 1 file changed, 28 insertions(+), 17 deletions(-)
>
>diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c
>b/drivers/net/vmxnet3/vmxnet3_ethdev.c
>index 2d7bf13..40c4537 100644
>--- a/drivers/net/vmxnet3/vmxnet3_ethdev.c
>+++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c
>@@ -90,7 +90,7 @@ static int vmxnet3_dev_vlan_filter_set(struct rte_eth_dev
>*dev,
> uint16_t vid, int on);
> static void vmxnet3_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask);
> static void vmxnet3_dev_vlan_offload_update(struct rte_eth_dev *dev,
>- int mask);
>+ int mask, int changed);
>
> #if PROCESS_SYS_EVENTS == 1
> static void vmxnet3_process_events(struct vmxnet3_hw *);
>@@ -521,7 +521,7 @@ vmxnet3_setup_driver_shared(struct rte_eth_dev *dev)
> if (dev->data->dev_conf.rxmode.hw_vlan_filter)
> mask |= ETH_VLAN_FILTER_MASK;
>
>- vmxnet3_dev_vlan_offload_update(dev, mask);
>+ vmxnet3_dev_vlan_offload_update(dev, mask, mask);
>
> PMD_INIT_LOG(DEBUG,
> "Writing MAC Address : %02x:%02x:%02x:%02x:%02x:%02x",
>@@ -838,34 +838,45 @@ vmxnet3_dev_vlan_filter_set(struct rte_eth_dev *dev,
>uint16_t vid, int on)
> }
>
> static void
>-vmxnet3_dev_vlan_offload_update(struct rte_eth_dev *dev, int mask)
>+vmxnet3_dev_vlan_offload_update(struct rte_eth_dev *dev, int mask, int
>changed)
> {
> struct vmxnet3_hw *hw = dev->data->dev_private;
> Vmxnet3_DSDevRead *devRead = &hw->shared->devRead;
> uint32_t *vf_table = devRead->rxFilterConf.vfTable;
>
>- if (mask & ETH_VLAN_STRIP_MASK)
>- devRead->misc.uptFeatures |= UPT1_F_RXVLAN;
>- else
>- devRead->misc.uptFeatures &= ~UPT1_F_RXVLAN;
>-
>- VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_CMD,
>- VMXNET3_CMD_UPDATE_FEATURE);
>+ if (changed & ETH_VLAN_STRIP_MASK) {
>+ if (mask & ETH_VLAN_STRIP_MASK)
>+ devRead->misc.uptFeatures |= UPT1_F_RXVLAN;
>+ else
>+ devRead->misc.uptFeatures &= ~UPT1_F_RXVLAN;
>
>- if (mask & ETH_VLAN_FILTER_MASK) {
>- memcpy(vf_table, hw->shadow_vfta, VMXNET3_VFT_TABLE_SIZE);
>- } else {
>- memset(vf_table, 0xff, VMXNET3_VFT_TABLE_SIZE);
>+ VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_CMD,
>+ VMXNET3_CMD_UPDATE_FEATURE);
> }
>
>- VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_CMD,
>- VMXNET3_CMD_UPDATE_VLAN_FILTERS);
>+ if (changed & ETH_VLAN_FILTER_MASK) {
>+ if (mask & ETH_VLAN_FILTER_MASK)
>+ memcpy(vf_table, hw->shadow_vfta,
>VMXNET3_VFT_TABLE_SIZE);
>+ else
>+ memset(vf_table, 0xff, VMXNET3_VFT_TABLE_SIZE);
>+
>+ VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_CMD,
>+ VMXNET3_CMD_UPDATE_VLAN_FILTERS);
>+ }
> }
>
> static void
> vmxnet3_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask)
> {
>- vmxnet3_dev_vlan_offload_update(dev, mask);
>+ int hw_mask = 0;
>+
>+ if (dev->data->dev_conf.rxmode.hw_vlan_strip)
>+ hw_mask |= ETH_VLAN_STRIP_MASK;
>+
>+ if (dev->data->dev_conf.rxmode.hw_vlan_filter)
>+ hw_mask |= ETH_VLAN_FILTER_MASK;
>+
>+ vmxnet3_dev_vlan_offload_update(dev, hw_mask, mask);
I don?t see a need to have a separate function for _update. You can singly
merge its implementation with vmxnet3_dev_vlan_offload_set, which seems to be
clearer and easier to follow IMO.
> }
>
> #if PROCESS_SYS_EVENTS == 1
>--
>2.1.4
>
