[dpdk-dev,1/2] ethdev: add api to set default mac address

  On Tue, 02 Jun 2015 14:23:22 +0200
Thomas Monjalon <thomas.monjalon@6wind.com> wrote:

> 2015-06-02 10:52, Ananyev, Konstantin:
> > From: Wang, Liang-min  
> > >  int
> > > +rte_eth_dev_default_mac_addr_set(uint8_t port_id, struct ether_addr *addr)
> > > +{
> > > +	struct rte_eth_dev *dev;
> > > +
> > > +	if (!rte_eth_dev_is_valid_port(port_id)) {
> > > +		PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id);
> > > +		return -ENODEV;
> > > +	}
> > > +
> > > +	dev = &rte_eth_devices[port_id];
> > > +	FUNC_PTR_OR_ERR_RET(*dev->dev_ops->mac_addr_set, -ENOTSUP);
> > > +
> > > +	return (*dev->dev_ops->mac_addr_set)(dev, addr);
> > > +}  
> > 
> > As I can see mac_addr_set() is implemented now only for virtio.
> > Which means that for all Intel HW your new rte_eth_dev_default_mac_addr_set()
> > would not work right now?
> > Probably rte_eth_dev_default_mac_addr_set() should combine both approaches:
> > If mac_addr_set() is implemented by dev, then use it, otherwise try to use addr_remove()/addr_add()
> > (as your first version did)?
> > Konstantin     
> 
> Not sure it is a good idea to use remove/add to set the default unicast mac address.
> It would be clearer to add comments to remove/add functions to specify that
> they don't apply to the default adress but only to secondary ones. Then use
> the same logic for both API and driver ops.
> It is the responsibility of the driver implementation to use common functions
> for default_set and remove/add functions.

Only vmxnet3 and virtio need special treatment. virtio is already covered.
Here is patch for vmxnet3. We have used this for several releases.


From eef188102338c5687b9075d468eabbe87693b075 Mon Sep 17 00:00:00 2001
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Tue, 2 Jun 2015 07:49:53 -0700
Subject: [PATCH] vmxnet3: support setting primary MAC address

This allows setting primary MAC address on VMXNET3.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/vmxnet3/vmxnet3_ethdev.c | 45 +++++++++++++++++++++++++-----------
 1 file changed, 31 insertions(+), 14 deletions(-)
  

>On Tue, 02 Jun 2015 14:23:22 +0200
>Thomas Monjalon <thomas.monjalon@6wind.com> wrote:

>> 2015-06-02 10:52, Ananyev, Konstantin:
> >> From: Wang, Liang-min
> > >>  int
> > >>+rte_eth_dev_default_mac_addr_set(uint8_t port_id, struct 
> > >> +ether_addr *addr) {
> > >> +	struct rte_eth_dev *dev;
> > >> +
> > >> +	if (!rte_eth_dev_is_valid_port(port_id)) {
> > >> +		PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id);
> > >> +		return -ENODEV;
> > >> +	}
> > >> +
> > >> +	dev = &rte_eth_devices[port_id];
> > >> +	FUNC_PTR_OR_ERR_RET(*dev->dev_ops->mac_addr_set, -ENOTSUP);
> > >> +
> > >> +	return (*dev->dev_ops->mac_addr_set)(dev, addr); }
> >> 
> >> As I can see mac_addr_set() is implemented now only for virtio.
> >> Which means that for all Intel HW your new 
> >> rte_eth_dev_default_mac_addr_set()
> >> would not work right now?
> >> Probably rte_eth_dev_default_mac_addr_set() should combine both approaches:
> >> If mac_addr_set() is implemented by dev, then use it, otherwise try 
> >> to use addr_remove()/addr_add() (as your first version did)?
> >> Konstantin     
>> 
>> Not sure it is a good idea to use remove/add to set the default unicast mac address.
>> It would be clearer to add comments to remove/add functions to specify 
>> that they don't apply to the default adress but only to secondary 
>> ones. Then use the same logic for both API and driver ops.
>> It is the responsibility of the driver implementation to use common 
>> functions for default_set and remove/add functions.

>Only vmxnet3 and virtio need special treatment. virtio is already covered.
>Here is patch for vmxnet3. We have used this for several releases.

To be consistent with implementation of ethdev API, rte_eth_macaddr_get, should mac_addr_set ops in both virtio and vmxnet3 update
 dev->data->mac_addrs[0]. So, the mac_addr_set and mac_addr_get are consistent.

>From eef188102338c5687b9075d468eabbe87693b075 Mon Sep 17 00:00:00 2001
>From: Stephen Hemminger <stephen@networkplumber.org>
>Date: Tue, 2 Jun 2015 07:49:53 -0700
>Subject: [PATCH] vmxnet3: support setting primary MAC address
>
>This allows setting primary MAC address on VMXNET3.
>
>Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
>---
> drivers/net/vmxnet3/vmxnet3_ethdev.c | 45 +++++++++++++++++++++++++-----------
> 1 file changed, 31 insertions(+), 14 deletions(-)
>
>diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c
>index 1685ce4..6515f74 100644
>--- a/drivers/net/vmxnet3/vmxnet3_ethdev.c
>+++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c
>@@ -85,6 +85,9 @@ static void vmxnet3_dev_stats_get(struct rte_eth_dev *dev,
> 				struct rte_eth_stats *stats);
> static void vmxnet3_dev_info_get(struct rte_eth_dev *dev,
> 				struct rte_eth_dev_info *dev_info);
>+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 *);  #endif @@ -110,6 +113,7 @@ static const struct eth_dev_ops vmxnet3_eth_dev_ops = {
> 	.allmulticast_disable = vmxnet3_dev_allmulticast_disable,
> 	.link_update          = vmxnet3_dev_link_update,
> 	.stats_get            = vmxnet3_dev_stats_get,
>+	.mac_addr_set	      = vmxnet3_mac_addr_set,
> 	.dev_infos_get        = vmxnet3_dev_info_get,
> 	.rx_queue_setup       = vmxnet3_dev_rx_queue_setup,
> 	.rx_queue_release     = vmxnet3_dev_rx_queue_release,
>@@ -359,6 +363,23 @@ vmxnet3_dev_configure(struct rte_eth_dev *dev)
> 	return 0;
> }
> 
>+static void
>+vmxnet3_write_mac(struct vmxnet3_hw *hw, const uint8_t *addr) {
>+	uint32_t val;
>+
>+	PMD_INIT_LOG(DEBUG,
>+		     "Writing MAC Address : %02x:%02x:%02x:%02x:%02x:%02x",
>+		     addr[0], addr[1], addr[2],
>+		     addr[3], addr[4], addr[5]);
>+
>+	val = *(const uint32_t *)addr;
>+	VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_MACL, val);
>+
>+	val = (addr[5] << 8) | addr[4];
>+	VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_MACH, val); }
>+
> static int
> vmxnet3_setup_driver_shared(struct rte_eth_dev *dev)  { @@ -366,8 +387,7 @@ vmxnet3_setup_driver_shared(struct rte_eth_dev *dev)
> 	struct vmxnet3_hw *hw = dev->data->dev_private;
> 	Vmxnet3_DriverShared *shared = hw->shared;
> 	Vmxnet3_DSDevRead *devRead = &shared->devRead;
>-	uint32_t *mac_ptr;
>-	uint32_t val, i;
>+	uint32_t i;
> 	int ret;
> 
> 	shared->magic = VMXNET3_REV1_MAGIC;
>@@ -459,18 +479,7 @@ vmxnet3_setup_driver_shared(struct rte_eth_dev *dev)
> 			return ret;
> 	}
> 
>-	PMD_INIT_LOG(DEBUG,
>-		     "Writing MAC Address : %02x:%02x:%02x:%02x:%02x:%02x",
>-		     hw->perm_addr[0], hw->perm_addr[1], hw->perm_addr[2],
>-		     hw->perm_addr[3], hw->perm_addr[4], hw->perm_addr[5]);
>-
>-	/* Write MAC Address back to device */
>-	mac_ptr = (uint32_t *)hw->perm_addr;
>-	val = *mac_ptr;
>-	VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_MACL, val);
>-
>-	val = (hw->perm_addr[5] << 8) | hw->perm_addr[4];
>-	VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_MACH, val);
>+	vmxnet3_write_mac(hw, hw->perm_addr);
> 
> 	return VMXNET3_SUCCESS;
> }
>@@ -645,6 +654,14 @@ vmxnet3_dev_info_get(__attribute__((unused))struct rte_eth_dev *dev, struct rte_
> 	dev_info->flow_type_rss_offloads = VMXNET3_RSS_OFFLOAD_ALL;  }
> 
>+static void
>+vmxnet3_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr 
>+*mac_addr) {
>+	struct vmxnet3_hw *hw = dev->data->dev_private;
>+
>+	vmxnet3_write_mac(hw, mac_addr->addr_bytes); }
>+
>/* return 0 means link status changed, -1 means not changed */  static int  vmxnet3_dev_link_update(struct rte_eth_dev *dev, __attribute__((unused)) int wait_to_complete)
--
2.1.4