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

[Thomas Monjalon <thomas.monjalon@6wind.com>]

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.