From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pg0-f66.google.com (mail-pg0-f66.google.com [74.125.83.66]) by dpdk.org (Postfix) with ESMTP id 4750D23D for ; Mon, 18 Dec 2017 11:53:49 +0100 (CET) Received: by mail-pg0-f66.google.com with SMTP id j4so9016302pgp.1 for ; Mon, 18 Dec 2017 02:53:49 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nfware-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=aBPI/xRi7Pl0L+i4aWuA0lm+ifZb4+qC4qy57Uzv7fA=; b=DyM8giJyaIA6N4vfOjuq3/PLf7/ZeLgW/fES/DDPkzUR/OFhpSNIYjB9ebY4jry2AG aXOli80//7QwjXOAcpYNO6LyEydnf0nkIsPEMkby9YbS2QHGwfTd6VTApFFGt5dJbrYz 2zb+dNWOoUo7kwXeS/mvI3TUus9q27AoGSjt5s8eAax/4e8MkEdL4VCRthEdzmaim5De WJgztI5GpbPnr+wOSvy5d2zHGelZNfZn9Ay36uEJGOa50v3Rn9eVHX36BRxa8/nOp+uQ 7KfABhXU7dNBPwQMjc+S8OrF3QQm8DVxhl1qpL22eLkNyk2gL72ewlrrGRo0kOSM6dXh xavQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=aBPI/xRi7Pl0L+i4aWuA0lm+ifZb4+qC4qy57Uzv7fA=; b=B/N8ByylFSVzyoKEhZh6o89r9h/BW7igPMLBTSLRG3cGAiBVJMOV1ygRWlV/C8Y3Og 04+dFHWifn8f7dB4+9Ynu79HK9TL6ay2hhRx1FWRMWLVvrPPNvxXA2cy+DpAvMg053aI m4yKQ1eHnyVwlC5TbuDLjNTkTrNCBgTe1UgUn2KNbysWECgu21OaJNZXPVq48i+phmRJ xgg03qmAyEvEOVvhtHUhYTAIk6PqtzqFxjqiwoEyhGHK5mKC7Uvl8Pgfu/eNWKe5GngW RlCFJlkX4AIHNCR1zy0S67O8ejgZcbejt7alZHdRrrAyeMdaz37G2cZu7hGwz1+uu+30 hmIg== X-Gm-Message-State: AKGB3mL8rOOXHikeLnCcGqybqe8SDgEuO3Trx3LgplqlDmuuy/zNnmOs kBubKi+Jo08iFncyEB7h4XBgsd6Lm5m//NPOWiiUCQ== X-Google-Smtp-Source: ACJfBosYLZtVQaQ7QCZWb/7pOqgsEWX4yygwgTxsujlo9MMluHNsv5OwrTeS8teZS7AC16GZ1msXBUHxg6D/QALX+G8= X-Received: by 10.99.123.24 with SMTP id w24mr19073353pgc.438.1513594428481; Mon, 18 Dec 2017 02:53:48 -0800 (PST) MIME-Version: 1.0 Received: by 10.100.133.130 with HTTP; Mon, 18 Dec 2017 02:53:47 -0800 (PST) In-Reply-To: References: <20171214171531.10506-1-olivier.matz@6wind.com> From: Igor Ryzhov Date: Mon, 18 Dec 2017 13:53:47 +0300 Message-ID: To: Andrew Rybchenko Cc: Olivier Matz , dev@dpdk.org, Thomas Monjalon , Laurent Hardy , stable@dpdk.org Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH] ethdev: fix setting of MAC address X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 18 Dec 2017 10:53:49 -0000 Hello Andrew, Don't you think that it's not correct that net/sfc works that way? If we go further, dev->dev_ops->mac_addr_set not only should be called before ether_addr_copy. It should return status code, and in case of error ether_addr_copy shouldn't be called at all. Am I wrong? Best regards, Igor On Mon, Dec 18, 2017 at 1:35 PM, Andrew Rybchenko wrote: > On 12/14/2017 08:15 PM, Olivier Matz wrote: > >> From: Laurent Hardy >> >> When a new mac address is set, it is saved in dev->data->mac_addrs >> before the ethdev handler is called. >> >> First, it is inconsistent with the other ethdev functions >> rte_eth_dev_mac_addr_remove() and rte_eth_dev_mac_addr_add(). >> >> Moreover, it prevents the drivers from wrongly comparing the old address >> and the new one, like it's done in i40evf driver: >> >> if (is_same_ether_addr(mac_addr, dev->data->mac_addrs)) >> return; >> >> Fixes: 943c2d899a0c ("net/i40e: set VF MAC from VF") >> Fixes: 854d8ad4ef68 ("ethdev: add default mac address modifier") >> Cc: stable@dpdk.org >> >> Signed-off-by: Laurent Hardy >> --- >> lib/librte_ether/rte_ethdev.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev. >> c >> index 4f492e3db..297c02a54 100644 >> --- a/lib/librte_ether/rte_ethdev.c >> +++ b/lib/librte_ether/rte_ethdev.c >> @@ -2643,11 +2643,11 @@ rte_eth_dev_default_mac_addr_set(uint16_t >> port_id, struct ether_addr *addr) >> dev = &rte_eth_devices[port_id]; >> RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->mac_addr_set, -ENOTSUP); >> + (*dev->dev_ops->mac_addr_set)(dev, addr); >> + >> /* Update default address in NIC data structure */ >> ether_addr_copy(addr, &dev->data->mac_addrs[0]); >> - (*dev->dev_ops->mac_addr_set)(dev, addr); >> - >> return 0; >> } >> > > NACK, unfortunately it will break net/sfc in one of branches when a new MAC > is set using restart. It relies on the fact that a new MAC is already > available in > device data. >