diff mbox

[dpdk-dev,3/3] vmxnet3: fix vlan_offload_set

Message ID 1449191107-14222-4-git-send-email-stephen@networkplumber.org (mailing list archive)
State Changes Requested, archived
Delegated to: Bruce Richardson
Headers

Commit Message

Stephen Hemminger Dec. 4, 2015, 1:05 a.m. UTC 
  From: Nachiketa Prachanda <nprachan@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@brocade.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/vmxnet3/vmxnet3_ethdev.c | 45 ++++++++++++++++++++++--------------
 1 file changed, 28 insertions(+), 17 deletions(-)

Comments

Yong Wang Dec. 9, 2015, 8:38 p.m. UTC | #1 
On 12/3/15, 5:05 PM, "Stephen Hemminger" <stephen@networkplumber.org> wrote:


>From: Nachiketa Prachanda <nprachan@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@brocade.com>

>Signed-off-by: Stephen Hemminger <stephen@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

>
diff mbox

Patch

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);
 }
 
 #if PROCESS_SYS_EVENTS == 1