From mboxrd@z Thu Jan  1 00:00:00 1970
From: Thomas Monjalon <thomas.monjalon@6wind.com>
Subject: Re: [PATCH 1/2] ethdev: add api to set default mac
	address
Date: Tue, 02 Jun 2015 14:23:22 +0200
Message-ID: <1821700.XUk45KJcQp@xps13>
References: <1432927612-12244-1-git-send-email-liang-min.wang@intel.com>
 <1432927612-12244-2-git-send-email-liang-min.wang@intel.com>
 <2601191342CEEE43887BDE71AB977258214345AB@irsmsx105.ger.corp.intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7Bit
Cc: dev@dpdk.org, "Wang, Liang-min" <liang-min.wang@intel.com>
To: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
Return-path: <dev-bounces@dpdk.org>
Received: from mail-wi0-f177.google.com (mail-wi0-f177.google.com
 [209.85.212.177]) by dpdk.org (Postfix) with ESMTP id 2E9DB9AD8
 for <dev@dpdk.org>; Tue,  2 Jun 2015 14:24:11 +0200 (CEST)
Received: by wibut5 with SMTP id ut5so67449571wib.1
 for <dev@dpdk.org>; Tue, 02 Jun 2015 05:24:11 -0700 (PDT)
In-Reply-To: <2601191342CEEE43887BDE71AB977258214345AB@irsmsx105.ger.corp.intel.com>
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

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.