Re: [dpdk-dev] [PATCH 5/6] ether: allow setting mac address

[Ivan Boule <ivan.boule@6wind.com>]

Hi Stephen,

Looking more precisely to your patch, I think that your requirement can 
beaddressed without adding
a new `mac_addr_set` function into the [already overloaded] set of 
operations exported by a PMD.

In fact, changing the default MAC address used by a port consists in 
executingthe following sequence of operations:
     1- rte_eth_macaddr_get(port_id, &cur_mac_addr) to obtain the 
default MACaddress currently used by the port,
     2- rte_eth_dev_mac_addr_remove(port_id, &cur_mac_addr) to remove 
thedefault MAC address currently used by the port,
     3- rte_eth_dev_mac_addr_add(port_id, &new_mac_addr) to add the new 
defaultMAC address to be used by the port.

This method takes advantage of the fact that the allocation of a 
freeentry into the array `data->mac_addrs` that
contains the MAC addresses of a port searches the pool from the entry at 
index 0, which contains the default MAC
address. This implementation simply needs to be mandatory from now.

The actual implementation might look like this.

Add the prototype of the function `rte_eth_dev_macaddr_set` in the 
file`rte_ethdev.h`:

     extern int rte_eth_dev_macaddr_set((uint8_t port_id, struct 
ether_addr *addr);

And add the following generic implementation of the function 
`rte_eth_dev_macaddr_set`in the file `rte_ethdev.c`:

int
rte_eth_dev_macaddr_set(uint8_t port_id, struct ether_addr *addr)
{
        struct rte_eth_dev *dev;
        struct ether_addr cur_addr;
        int diag;

        if (port_id >= nb_ports) {
                PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id);
                return (-ENODEV);
        }

        if (is_zero_ether_addr(addr)) {
                PMD_DEBUG_TRACE("port %d: Cannot add NULL MAC address\n",
                    port_id);
                return (-EINVAL);
        }

        dev = &rte_eth_devices[port_id];

        /*
         * Check that both mac_addr_remove and mac_addr_add operations are
         * supported by the PMD of the port.
         */
FUNC_PTR_OR_ERR_RET(*dev->dev_ops->mac_addr_remove, -ENOTSUP);
FUNC_PTR_OR_ERR_RET(*dev->dev_ops->mac_addr_add, -ENOTSUP);

        rte_eth_macaddr_get(port_id, &cur_addr);
        diag = rte_eth_dev_mac_addr_remove(port_id, &cur_addr);
        if (diag < 0)
                return diag;
        return rte_eth_dev_mac_addr_add(port_id, addr);
}

Regards,
Ivan



On 05/15/2014 11:04 AM, Ivan Boule wrote:
> Hi Stephen,
>
> By default, changing the [current] default MAC address of a device is 
> not equivalent to adding a MAC address to it ! At least, the minimum 
> to do should consist in:
>    1. checking that the PMD exports both mac_addr_remove() 
> and mac_addr_add() functions, returning a NON_SUPPORTED error code 
> otherwise,
>    2. removing the current default MAC address by invoking the PMD's 
> mac_addr_remove() function,
>    3. adding the new MAC address by invoking the PMD's mac_addr_add() 
> function.
>
> Regards,
> Ivan
>
> 2014-05-14 20:55 GMT+02:00 Stephen Hemminger 
> <stephen@networkplumber.org <mailto:stephen@networkplumber.org>>:
>
>     Allow setting the default Ethernet address used on device.
>     The underlying drivers allow it but the DPDK was blocking any
>     attempts to change Ethernet address on device.
>
>     For most devices, this is just the same as filling in address
>     in mac address table entry 0, but for some devices this requires
>     special override.
>
>     Signed-off-by: Stephen Hemminger <shemming@brocade.com
>     <mailto:shemming@brocade.com>>
>
>
>     --- a/lib/librte_ether/rte_ethdev.h     2014-05-14
>     11:44:27.373014227 -0700
>     +++ b/lib/librte_ether/rte_ethdev.h     2014-05-14
>     11:44:27.373014227 -0700
>     @@ -982,6 +982,11 @@ typedef void (*eth_mac_addr_add_t)(struc
>                                       struct ether_addr *mac_addr,
>                                       uint32_t index,
>                                       uint32_t vmdq);
>     +/**< @internal Change default MAC address */
>     +
>     +typedef void (*eth_mac_addr_set_t)(struct rte_eth_dev *dev,
>     +                                  struct ether_addr *mac_addr);
>     +
>      /**< @internal Set a MAC address into Receive Address Address
>     Register */
>
>      typedef int (*eth_uc_hash_table_set_t)(struct rte_eth_dev *dev,
>     @@ -1114,6 +1119,7 @@ struct eth_dev_ops {
>             priority_flow_ctrl_set_t priority_flow_ctrl_set; /**<
>     Setup priority flow control.*/
>             eth_mac_addr_remove_t      mac_addr_remove; /**< Remove
>     MAC address */
>             eth_mac_addr_add_t         mac_addr_add;  /**< Add a MAC
>     address */
>     +       eth_mac_addr_set_t         mac_addr_set;  /**< Change
>     default MAC address */
>             eth_uc_hash_table_set_t    uc_hash_table_set;  /**< Set
>     Unicast Table Array */
>             eth_uc_all_hash_table_set_t uc_all_hash_table_set;  /**<
>     Set Unicast hash bitmap */
>             eth_mirror_rule_set_t      mirror_rule_set;  /**< Add a
>     traffic mirror rule.*/
>     @@ -2538,6 +2544,21 @@ int rte_eth_dev_mac_addr_add(uint8_t por
>      int rte_eth_dev_mac_addr_remove(uint8_t port, struct ether_addr
>     *mac_addr);
>
>      /**
>     + * Change the default MAC address
>     + *
>     + * @param port
>     + *   The port identifier of the Ethernet device.
>     + * @param mac_addr
>     + *   MAC address to remove.
>     + * @return
>     + *   - (0) if successfully added or *mac_addr" was already added.
>     + *   - (-ENOTSUP) if hardware doesn't support this feature.
>     + *   - (-ENODEV) if *port* is invalid.
>     + *   - (-EINVAL) if MAC address is invalid.
>     + */
>     +int rte_eth_dev_macaddr_set(uint8_t port, struct ether_addr
>     *mac_addr);
>     +
>     +/**
>       * Update Redirection Table(RETA) of Receive Side Scaling of
>     Ethernet device.
>       *
>       * @param port
>     --- a/lib/librte_ether/rte_ethdev.c     2014-05-14
>     11:44:27.373014227 -0700
>     +++ b/lib/librte_ether/rte_ethdev.c     2014-05-14
>     11:44:27.373014227 -0700
>     @@ -1059,6 +1059,33 @@ rte_eth_macaddr_get(uint8_t port_id, str
>      }
>
>      int
>     +rte_eth_dev_macaddr_set(uint8_t port_id, struct ether_addr *addr)
>     +{
>     +       struct rte_eth_dev *dev;
>     +
>     +       if (port_id >= nb_ports) {
>     +               PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id);
>     +               return (-ENODEV);
>     +       }
>     +       if (is_zero_ether_addr(addr)) {
>     +               PMD_DEBUG_TRACE("port %d: Cannot add NULL MAC
>     address\n", port_id);
>     +               return (-EINVAL);
>     +       }
>     +       dev = &rte_eth_devices[port_id];
>     +
>     +       if (*dev->dev_ops->mac_addr_set)
>     +               (*dev->dev_ops->mac_addr_set)(dev, addr);
>     +       else {
>     + FUNC_PTR_OR_ERR_RET(*dev->dev_ops->mac_addr_add, -ENOTSUP);
>     +               (*dev->dev_ops->mac_addr_add)(dev, addr, 0, 0);
>     +       }
>     +
>     +       ether_addr_copy(addr, &dev->data->mac_addrs[0]);
>     +
>     +       return 0;
>     +}
>     +
>     +int
>      rte_eth_dev_vlan_filter(uint8_t port_id, uint16_t vlan_id, int on)
>      {
>             struct rte_eth_dev *dev;
>
>
>
> -- 
> Ivan BOULE
> 6WIND
> Software Engineer
>


-- 
Ivan Boule
6WIND Development Engineer