From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pg0-f67.google.com (mail-pg0-f67.google.com [74.125.83.67]) by dpdk.org (Postfix) with ESMTP id 54513E5D for ; Mon, 18 Dec 2017 11:53:49 +0100 (CET) Received: by mail-pg0-f67.google.com with SMTP id m25so8996027pgv.12 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=XM0ElHohezOldbvjwwsba+V31mMU6uVWPku8jtrDfpa/jl4KAObEioi2jCld+pgdE4 tspWtKv+o5zRGA/4b0yEnm4yuP6r+Hz6OYZztXjOcqj8VIQEhQzN01XnwFlWGrSBLoTw o91brd2F6xjOAEC6qg1fksZWt4+S1aw73ewLLSbQ9u42CowwR0tfCHl7ifoC6sjpHNd6 U/W3L3MzwqG85l3RNiVWccWPk+5wY5gswFWKGlhLjkFKN5KBoBMuVzyzKfTJBOaqyMzZ FZd0gHu4xuqO158TEV67ovNNci2JW6VezhxKHicNhdbvoULK42H7BL52tTEZip99OLLn feFQ== X-Gm-Message-State: AKGB3mJND5O8d3sF5NtlYfYCmj+CTl9N1yqFJuszllg+CP02PhsTKrEn Ae4hWnwmvUebnpyWaaSN1Ug+TsGBarh0b3jA5zt06uaW 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-stable] [dpdk-dev] [PATCH] ethdev: fix setting of MAC address X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches 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. >