diff mbox

[dpdk-dev,v2,2/2] drivers/net/vmxnet3: add ethdev functions

Message ID 1457105124-23650-3-git-send-email-remy.horton@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Bruce Richardson
Headers

Commit Message

Remy Horton March 4, 2016, 3:25 p.m. UTC 
  Implements driver support for setting of MAC address.

Signed-off-by: Remy Horton <remy.horton@intel.com>
---
 doc/guides/rel_notes/release_16_04.rst |  4 ++++
 drivers/net/vmxnet3/vmxnet3_ethdev.c   | 19 +++++++++++++++++++
 2 files changed, 23 insertions(+)

Comments

Yong Wang March 4, 2016, 7:12 p.m. UTC | #1 
On 3/4/16, 7:25 AM, "Remy Horton" <remy.horton@intel.com> wrote:


>Implements driver support for setting of MAC address.

>

>Signed-off-by: Remy Horton <remy.horton@intel.com>


Acked-by: Yong Wang <yongwang@vmware.com>


mac_hi is not initialized so the upper 16bits are random when calling
VMXNET3_WRITE_BAR1_REG().  The handler of the mac changing ignores
them so it should be fine.


>---

> doc/guides/rel_notes/release_16_04.rst |  4 ++++

> drivers/net/vmxnet3/vmxnet3_ethdev.c   | 19 +++++++++++++++++++

> 2 files changed, 23 insertions(+)

>

>diff --git a/doc/guides/rel_notes/release_16_04.rst b/doc/guides/rel_notes/release_16_04.rst

>index 4e0112e..39db674 100644

>--- a/doc/guides/rel_notes/release_16_04.rst

>+++ b/doc/guides/rel_notes/release_16_04.rst

>@@ -62,6 +62,10 @@ This section should contain new features added in this release. Sample format:

>   Implemented driver functions for Register dumping, EEPROM dumping, and

>   setting of MAC address.

> 

>+* **vmxnet3: Added ethdev support functions.**

>+

>+  Implemented driver functionality for setting MAC address.

>+

> 

> Resolved Issues

> ---------------

>diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c

>index c363bf6..bc83524 100644

>--- a/drivers/net/vmxnet3/vmxnet3_ethdev.c

>+++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c

>@@ -91,6 +91,8 @@ static int vmxnet3_dev_vlan_filter_set(struct rte_eth_dev *dev,

> static void vmxnet3_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask);

> static void vmxnet3_dev_vlan_offload_set_clear(struct rte_eth_dev *dev,

> 						int mask, int clear);

>+static void vmxnet3_mac_addr_set(struct rte_eth_dev *dev,

>+				 struct ether_addr *mac_addr);

> 

> #if PROCESS_SYS_EVENTS == 1

> static void vmxnet3_process_events(struct vmxnet3_hw *);

>@@ -124,6 +126,7 @@ static const struct eth_dev_ops vmxnet3_eth_dev_ops = {

> 	.rx_queue_release     = vmxnet3_dev_rx_queue_release,

> 	.tx_queue_setup       = vmxnet3_dev_tx_queue_setup,

> 	.tx_queue_release     = vmxnet3_dev_tx_queue_release,

>+	.mac_addr_set         = vmxnet3_mac_addr_set,

> };

> 

> static const struct rte_memzone *

>@@ -922,6 +925,22 @@ vmxnet3_process_events(struct vmxnet3_hw *hw)

> }

> #endif

> 

>+static void vmxnet3_mac_addr_set(struct rte_eth_dev *dev,

>+				 struct ether_addr *mac_addr)

>+{

>+	struct vmxnet3_hw *hw = dev->data->dev_private;

>+	uint32_t mac_hi, mac_lo;

>+

>+	if (!is_valid_assigned_ether_addr(mac_addr)) {

>+		PMD_DRV_LOG(ERR, "Tried to set invalid MAC address.");

>+		return;

>+	}

>+	memcpy(&mac_lo, mac_addr, 4);

>+	memcpy(&mac_hi, mac_addr + 4, 2);

>+	VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_MACL, mac_lo);

>+	VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_MACH, mac_hi);

>+}

>+

> static struct rte_driver rte_vmxnet3_driver = {

> 	.type = PMD_PDEV,

> 	.init = rte_vmxnet3_pmd_init,

>-- 

>2.5.0

>
Stephen Hemminger March 4, 2016, 8:34 p.m. UTC | #2 
On Fri,  4 Mar 2016 15:25:24 +0000
Remy Horton <remy.horton@intel.com> wrote:

> Implements driver support for setting of MAC address.
> 
> Signed-off-by: Remy Horton <remy.horton@intel.com>
> ---
>  doc/guides/rel_notes/release_16_04.rst |  4 ++++
>  drivers/net/vmxnet3/vmxnet3_ethdev.c   | 19 +++++++++++++++++++
>  2 files changed, 23 insertions(+)
> 
> diff --git a/doc/guides/rel_notes/release_16_04.rst b/doc/guides/rel_notes/release_16_04.rst
> index 4e0112e..39db674 100644
> --- a/doc/guides/rel_notes/release_16_04.rst
> +++ b/doc/guides/rel_notes/release_16_04.rst
> @@ -62,6 +62,10 @@ This section should contain new features added in this release. Sample format:
>    Implemented driver functions for Register dumping, EEPROM dumping, and
>    setting of MAC address.
>  
> +* **vmxnet3: Added ethdev support functions.**
> +
> +  Implemented driver functionality for setting MAC address.
> +
>  
>  Resolved Issues
>  ---------------
> diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c
> index c363bf6..bc83524 100644
> --- a/drivers/net/vmxnet3/vmxnet3_ethdev.c
> +++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c
> @@ -91,6 +91,8 @@ static int vmxnet3_dev_vlan_filter_set(struct rte_eth_dev *dev,
>  static void vmxnet3_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask);
>  static void vmxnet3_dev_vlan_offload_set_clear(struct rte_eth_dev *dev,
>  						int mask, int clear);
> +static void vmxnet3_mac_addr_set(struct rte_eth_dev *dev,
> +				 struct ether_addr *mac_addr);
>  
>  #if PROCESS_SYS_EVENTS == 1
>  static void vmxnet3_process_events(struct vmxnet3_hw *);
> @@ -124,6 +126,7 @@ static const struct eth_dev_ops vmxnet3_eth_dev_ops = {
>  	.rx_queue_release     = vmxnet3_dev_rx_queue_release,
>  	.tx_queue_setup       = vmxnet3_dev_tx_queue_setup,
>  	.tx_queue_release     = vmxnet3_dev_tx_queue_release,
> +	.mac_addr_set         = vmxnet3_mac_addr_set,
>  };
>  
>  static const struct rte_memzone *
> @@ -922,6 +925,22 @@ vmxnet3_process_events(struct vmxnet3_hw *hw)
>  }
>  #endif
>  
> +static void vmxnet3_mac_addr_set(struct rte_eth_dev *dev,
> +				 struct ether_addr *mac_addr)
> +{
> +	struct vmxnet3_hw *hw = dev->data->dev_private;
> +	uint32_t mac_hi, mac_lo;
> +
> +	if (!is_valid_assigned_ether_addr(mac_addr)) {
> +		PMD_DRV_LOG(ERR, "Tried to set invalid MAC address.");
> +		return;
> +	}
> +	memcpy(&mac_lo, mac_addr, 4);
> +	memcpy(&mac_hi, mac_addr + 4, 2);
> +	VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_MACL, mac_lo);
> +	VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_MACH, mac_hi);
> +}
> +
>  static struct rte_driver rte_vmxnet3_driver = {
>  	.type = PMD_PDEV,
>  	.init = rte_vmxnet3_pmd_init,

The version I posted is simpler and reuses existing code paths.
Remy Horton March 7, 2016, 12:26 p.m. UTC | #3 
On 04/03/2016 20:34, Stephen Hemminger wrote:
> On Fri,  4 Mar 2016 15:25:24 +0000
> Remy Horton <remy.horton@intel.com> wrote:
>
>> Implements driver support for setting of MAC address.
>>
>> Signed-off-by: Remy Horton <remy.horton@intel.com>
[..]
> The version I posted is simpler and reuses existing code paths.
>

That version doesn't include a MAC address sanity check - is such a 
check redundant (or incorrect) in this case?

..Remy
Stephen Hemminger March 7, 2016, 4:07 p.m. UTC | #4 
On Mon, 07 Mar 2016 12:26:13 +0000
Remy Horton <remy.horton@intel.com> wrote:

> 
> On 04/03/2016 20:34, Stephen Hemminger wrote:
> > On Fri,  4 Mar 2016 15:25:24 +0000
> > Remy Horton <remy.horton@intel.com> wrote:
> >
> >> Implements driver support for setting of MAC address.
> >>
> >> Signed-off-by: Remy Horton <remy.horton@intel.com>
> [..]
> > The version I posted is simpler and reuses existing code paths.
> >
> 
> That version doesn't include a MAC address sanity check - is such a 
> check redundant (or incorrect) in this case?
> 
> ..Remy

That belongs in common code, not in each driver.
Remy Horton March 7, 2016, 5:06 p.m. UTC | #5 
On 07/03/2016 16:07, Stephen Hemminger wrote:
>> That version doesn't include a MAC address sanity check - is such a
>> check redundant (or incorrect) in this case?
>>
>> ..Remy
>
> That belongs in common code, not in each driver.

That differs from previous feedback for a different driver, where such a 
check was actually asked to be added.. :/

Probably need to audit the upstream functions for such checks.

..Remy
Stephen Hemminger March 7, 2016, 7:19 p.m. UTC | #6 
On Mon, 07 Mar 2016 17:06:20 +0000
Remy Horton <remy.horton@intel.com> wrote:

> 
> 
> On 07/03/2016 16:07, Stephen Hemminger wrote:
> >> That version doesn't include a MAC address sanity check - is such a
> >> check redundant (or incorrect) in this case?
> >>
> >> ..Remy
> >
> > That belongs in common code, not in each driver.
> 
> That differs from previous feedback for a different driver, where such a 
> check was actually asked to be added.. :/
> 
> Probably need to audit the upstream functions for such checks.
> 
> ..Remy

Probably need checks in both places.  Zero is bad for any device,
some devices allow multicast addresses as MAC, and some clustering
seemed to want it.
diff mbox

Patch

diff --git a/doc/guides/rel_notes/release_16_04.rst b/doc/guides/rel_notes/release_16_04.rst
index 4e0112e..39db674 100644
--- a/doc/guides/rel_notes/release_16_04.rst
+++ b/doc/guides/rel_notes/release_16_04.rst
@@ -62,6 +62,10 @@  This section should contain new features added in this release. Sample format:
   Implemented driver functions for Register dumping, EEPROM dumping, and
   setting of MAC address.
 
+* **vmxnet3: Added ethdev support functions.**
+
+  Implemented driver functionality for setting MAC address.
+
 
 Resolved Issues
 ---------------
diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c
index c363bf6..bc83524 100644
--- a/drivers/net/vmxnet3/vmxnet3_ethdev.c
+++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c
@@ -91,6 +91,8 @@  static int vmxnet3_dev_vlan_filter_set(struct rte_eth_dev *dev,
 static void vmxnet3_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask);
 static void vmxnet3_dev_vlan_offload_set_clear(struct rte_eth_dev *dev,
 						int mask, int clear);
+static void vmxnet3_mac_addr_set(struct rte_eth_dev *dev,
+				 struct ether_addr *mac_addr);
 
 #if PROCESS_SYS_EVENTS == 1
 static void vmxnet3_process_events(struct vmxnet3_hw *);
@@ -124,6 +126,7 @@  static const struct eth_dev_ops vmxnet3_eth_dev_ops = {
 	.rx_queue_release     = vmxnet3_dev_rx_queue_release,
 	.tx_queue_setup       = vmxnet3_dev_tx_queue_setup,
 	.tx_queue_release     = vmxnet3_dev_tx_queue_release,
+	.mac_addr_set         = vmxnet3_mac_addr_set,
 };
 
 static const struct rte_memzone *
@@ -922,6 +925,22 @@  vmxnet3_process_events(struct vmxnet3_hw *hw)
 }
 #endif
 
+static void vmxnet3_mac_addr_set(struct rte_eth_dev *dev,
+				 struct ether_addr *mac_addr)
+{
+	struct vmxnet3_hw *hw = dev->data->dev_private;
+	uint32_t mac_hi, mac_lo;
+
+	if (!is_valid_assigned_ether_addr(mac_addr)) {
+		PMD_DRV_LOG(ERR, "Tried to set invalid MAC address.");
+		return;
+	}
+	memcpy(&mac_lo, mac_addr, 4);
+	memcpy(&mac_hi, mac_addr + 4, 2);
+	VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_MACL, mac_lo);
+	VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_MACH, mac_hi);
+}
+
 static struct rte_driver rte_vmxnet3_driver = {
 	.type = PMD_PDEV,
 	.init = rte_vmxnet3_pmd_init,