From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <iryzhov@nfware.com>
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 <dev@dpdk.org>; Mon, 18 Dec 2017 11:53:49 +0100 (CET)
Received: by mail-pg0-f66.google.com with SMTP id j4so9016302pgp.1
for <dev@dpdk.org>; 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: <adea1787-9c1c-3942-3b9e-ea42a2e44728@solarflare.com>
References: <20171214171531.10506-1-olivier.matz@6wind.com>
<adea1787-9c1c-3942-3b9e-ea42a2e44728@solarflare.com>
From: Igor Ryzhov <iryzhov@nfware.com>
Date: Mon, 18 Dec 2017 13:53:47 +0300
Message-ID: <CAF+s_FyBQfevMSaLHH9xOV=+DvTwk70XNcOrtGToumhKvs-kMA@mail.gmail.com>
To: Andrew Rybchenko <arybchenko@solarflare.com>
Cc: Olivier Matz <olivier.matz@6wind.com>, dev@dpdk.org,
Thomas Monjalon <thomas@monjalon.net>, Laurent Hardy <laurent.hardy@6wind.com>,
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 <dev.dpdk.org>
List-Unsubscribe: <https://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: <https://dpdk.org/ml/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=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 <arybchenko@solarflare.com
> wrote:
> On 12/14/2017 08:15 PM, Olivier Matz wrote:
>
>> From: Laurent Hardy <laurent.hardy@6wind.com>
>>
>> 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 <laurent.hardy@6wind.com>
>> ---
>> 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.
>