From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dispatch1-us1.ppe-hosted.com (dispatch1-us1.ppe-hosted.com [67.231.154.164]) by dpdk.org (Postfix) with ESMTP id 6D3941E34 for ; Sun, 16 Jul 2017 14:46:12 +0200 (CEST) Received: from pure.maildistiller.com (unknown [10.110.50.29]) by dispatch1-us1.ppe-hosted.com (Proofpoint Essentials ESMTP Server) with ESMTP id ED3848004A; Sun, 16 Jul 2017 12:46:11 +0000 (UTC) X-Virus-Scanned: Proofpoint Essentials engine Received: from mx2-us1.ppe-hosted.com (unknown [10.110.49.251]) by pure.maildistiller.com (Proofpoint Essentials ESMTP Server) with ESMTPS id 6BB9860049; Sun, 16 Jul 2017 12:46:11 +0000 (UTC) Received: from webmail.solarflare.com (webmail.solarflare.com [12.187.104.26]) (using TLSv1 with cipher ECDHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx2-us1.ppe-hosted.com (Proofpoint Essentials ESMTP Server) with ESMTPS id 3913E8006C; Sun, 16 Jul 2017 12:46:11 +0000 (UTC) Received: from [192.168.239.128] (85.187.13.33) by ocex03.SolarFlarecom.com (10.20.40.36) with Microsoft SMTP Server (TLS) id 15.0.1044.25; Sun, 16 Jul 2017 05:46:07 -0700 To: Stephen Hemminger , CC: Stephen Hemminger References: <20170714183027.16021-1-stephen@networkplumber.org> <20170714183027.16021-14-stephen@networkplumber.org> From: Andrew Rybchenko Message-ID: <5872a2c2-7d3c-2dcd-a9b5-570763d40a4a@solarflare.com> Date: Sun, 16 Jul 2017 15:46:02 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <20170714183027.16021-14-stephen@networkplumber.org> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US X-Originating-IP: [85.187.13.33] X-ClientProxiedBy: ocex03.SolarFlarecom.com (10.20.40.36) To ocex03.SolarFlarecom.com (10.20.40.36) X-MDID: 1500209171-d8UeGEe6uDVa Subject: Re: [dpdk-dev] [RFC 13/14] szedata: use _rte_eth_link_update 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: Sun, 16 Jul 2017 12:46:12 -0000 On 07/14/2017 09:30 PM, Stephen Hemminger wrote: > Yet another driver which was not returing correct value on > link change. > > Signed-off-by: Stephen Hemminger > --- > drivers/net/szedata2/rte_eth_szedata2.c | 18 ++++++++---------- > 1 file changed, 8 insertions(+), 10 deletions(-) > > diff --git a/drivers/net/szedata2/rte_eth_szedata2.c b/drivers/net/szedata2/rte_eth_szedata2.c > index 9c0d57cc14c0..b81ba8e79c64 100644 > --- a/drivers/net/szedata2/rte_eth_szedata2.c > +++ b/drivers/net/szedata2/rte_eth_szedata2.c > @@ -50,7 +50,6 @@ > #include > #include > #include > -#include > > #include "rte_eth_szedata2.h" > #include "szedata2_iobuf.h" > @@ -1171,9 +1170,7 @@ static int > eth_link_update(struct rte_eth_dev *dev, > int wait_to_complete __rte_unused) > { > - struct rte_eth_link link; > - struct rte_eth_link *link_ptr = &link; > - struct rte_eth_link *dev_link = &dev->data->dev_link; > + struct rte_eth_link link, old; > struct pmd_internals *internals = (struct pmd_internals *) > dev->data->dev_private; > const volatile struct szedata2_ibuf *ibuf; > @@ -1195,8 +1192,12 @@ eth_link_update(struct rte_eth_dev *dev, > break; > } > > + _rte_eth_link_read(dev, &old); > + memset(&link, 0, sizeof(link)); > + > /* szedata2 uses only full duplex */ > link.link_duplex = ETH_LINK_FULL_DUPLEX; > + link.link_autoneg = ETH_LINK_SPEED_FIXED; See my comment for net/virtio patch. It is applicable here as well. > > for (i = 0; i < szedata2_ibuf_count; i++) { > ibuf = ibuf_ptr_by_index(internals->pci_rsc, i); > @@ -1210,14 +1211,11 @@ eth_link_update(struct rte_eth_dev *dev, > } > } > > - link.link_status = (link_is_up) ? ETH_LINK_UP : ETH_LINK_DOWN; > + link.link_status = link_is_up ? ETH_LINK_UP : ETH_LINK_DOWN; > > - link.link_autoneg = ETH_LINK_SPEED_FIXED; In fact, the bug was here before the patch. > + _rte_eth_link_write(dev, &link); > > - rte_atomic64_cmpset((uint64_t *)dev_link, *(uint64_t *)dev_link, > - *(uint64_t *)link_ptr); > - > - return 0; > + return (old.link_status == link.link_status) ? -1 : 0; > } > > static int