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 226971B01F for ; Tue, 9 Jan 2018 17:27:40 +0100 (CET) Received: by mail-pg0-f66.google.com with SMTP id r2so8303909pgq.13 for ; Tue, 09 Jan 2018 08:27:39 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=/C11WbXNPkoT2g3dUAYThI6mkdxFEy36909KHj9n+T4=; b=WDY4yyneFEFkdooEISe90qctxaAj2d2/xJqTJWjQwKGN/r0aFIdTX8nYTNCr3yHFKe /zRQM0d2k02GxBOcgL7obYx5BR74lAaJe9BWA2kJap85BPVi0roLppQqNgL4425WjjSQ bdbTyjj5ODbcqkgrLaUZlv6ygv/+DksEqjG1K9n7a73Ap/0zeAGgTey9MYUrT6J1IR9o xMzGqpg5O8ncM3qE15VVksg32CElsper2x9929Ui5l9dZGyma8Oq6BQVItFlnBo5Ob95 QNcYUVfyjGQE0ewnbztfDVwc/em2Vh0Rn8OTjuVDIRdddt0/v0wy1USMtrGAc2Omu5kN gkVw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=/C11WbXNPkoT2g3dUAYThI6mkdxFEy36909KHj9n+T4=; b=aMZ32yiVn9WjCRQ/b8LAAuUIaXjYOE7yY41a5i/1NdcUNqM1ReG31+ZqzyOQs2dGBs LiLk+UlzViQjD+Iowy/DtyTqta/z9Zt3DBw/4Gnw1lSFa1i95ZODhBUy/4WeYDKjj1BE /ReKaCoyOpLdlsSh17eZ2lzmFN9HnhOGx3W8WBEHlqH2skcG4RYNpGRa50YqinEkhmVf o91VQFPLaDJJVwg4UjILGkfqhBRelH2swadtzyH6GfxcE1OcVPBTsHEg6ReqNacGHx5A K8jtAH3XPnf/m/J8Meu6l8wgFENq0GKxf0BoNj7+y0fEc+Y2MR2QXTSAXa/0+2P8CM7u DC3A== X-Gm-Message-State: AKGB3mKGc4jU0XVBj9ZYeRlExhx0r2YcuP+4wXncC4zMTUAoXUiZZLK1 P1P0cb7p7kcOiC/fU5YGZvuWOQ== X-Google-Smtp-Source: ACJfBotzkiQTycI7enoYWvOeCkZUmEpPvzbiL0ZzG/g+lzwULeDVTyq3zNZ+z9DEDdsS8bR+IM31Zw== X-Received: by 10.84.137.169 with SMTP id 38mr16459106pln.246.1515515259134; Tue, 09 Jan 2018 08:27:39 -0800 (PST) Received: from xeon-e3 (204-195-18-133.wavecable.com. [204.195.18.133]) by smtp.gmail.com with ESMTPSA id d22sm9436859pfl.135.2018.01.09.08.27.38 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 09 Jan 2018 08:27:39 -0800 (PST) Date: Tue, 9 Jan 2018 08:27:32 -0800 From: Stephen Hemminger To: Andrew Rybchenko Cc: Message-ID: <20180109082732.4ba9115b@xeon-e3> In-Reply-To: References: <20180108174514.14688-1-stephen@networkplumber.org> <20180108174514.14688-10-stephen@networkplumber.org> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v3 09/15] net/sfc: use new rte_eth_linkstatus functions 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: Tue, 09 Jan 2018 16:27:40 -0000 On Tue, 9 Jan 2018 13:35:58 +0300 Andrew Rybchenko wrote: > On 01/08/2018 08:45 PM, Stephen Hemminger wrote: > > Use the new API (_rte_eth_linkstatus_set) to handle link status update. > > > > Signed-off-by: Stephen Hemminger > > --- > > drivers/net/sfc/sfc_ethdev.c | 27 +++++++-------------------- > > drivers/net/sfc/sfc_ev.c | 23 ++++------------------- > > 2 files changed, 11 insertions(+), 39 deletions(-) > > > > diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c > > index 2f5f86f84877..e0a12b32b1a3 100644 > > --- a/drivers/net/sfc/sfc_ethdev.c > > +++ b/drivers/net/sfc/sfc_ethdev.c > > @@ -238,22 +238,12 @@ static int > > sfc_dev_link_update(struct rte_eth_dev *dev, int wait_to_complete) > > { > > struct sfc_adapter *sa = dev->data->dev_private; > > - struct rte_eth_link *dev_link = &dev->data->dev_link; > > - struct rte_eth_link old_link; > > struct rte_eth_link current_link; > > > > sfc_log_init(sa, "entry"); > > > > -retry: > > - EFX_STATIC_ASSERT(sizeof(*dev_link) == sizeof(rte_atomic64_t)); > > - *(int64_t *)&old_link = rte_atomic64_read((rte_atomic64_t *)dev_link); > > - > > if (sa->state != SFC_ADAPTER_STARTED) { > > sfc_port_link_mode_to_info(EFX_LINK_UNKNOWN, ¤t_link); > > - if (!rte_atomic64_cmpset((volatile uint64_t *)dev_link, > > - *(uint64_t *)&old_link, > > - *(uint64_t *)¤t_link)) > > - goto retry; > > } else if (wait_to_complete) { > > efx_link_mode_t link_mode; > > > > @@ -261,21 +251,18 @@ sfc_dev_link_update(struct rte_eth_dev *dev, int wait_to_complete) > > link_mode = EFX_LINK_UNKNOWN; > > sfc_port_link_mode_to_info(link_mode, ¤t_link); > > > > - if (!rte_atomic64_cmpset((volatile uint64_t *)dev_link, > > - *(uint64_t *)&old_link, > > - *(uint64_t *)¤t_link)) > > - goto retry; > > } else { > > sfc_ev_mgmt_qpoll(sa); > > - *(int64_t *)¤t_link = > > - rte_atomic64_read((rte_atomic64_t *)dev_link); > > + _rte_eth_linkstatus_get(dev, ¤t_link); > > } > > > > - if (old_link.link_status != current_link.link_status) > > - sfc_info(sa, "Link status is %s", > > - current_link.link_status ? "UP" : "DOWN"); > > + if (_rte_eth_linkstatus_set(dev, ¤t_link) == 0) > > + return 0; > > + > > + sfc_info(sa, "Link status is %s", > > + current_link.link_status ? "UP" : "DOWN"); > > > > - return old_link.link_status == current_link.link_status ? 0 : -1; > > + return -1; > > } > > > > static void > > diff --git a/drivers/net/sfc/sfc_ev.c b/drivers/net/sfc/sfc_ev.c > > index a16dc27b380e..3e96536a9d60 100644 > > --- a/drivers/net/sfc/sfc_ev.c > > +++ b/drivers/net/sfc/sfc_ev.c > > @@ -404,29 +404,14 @@ sfc_ev_link_change(void *arg, efx_link_mode_t link_mode) > > { > > struct sfc_evq *evq = arg; > > struct sfc_adapter *sa = evq->sa; > > - struct rte_eth_link *dev_link = &sa->eth_dev->data->dev_link; > > struct rte_eth_link new_link; > > - uint64_t new_link_u64; > > - uint64_t old_link_u64; > > - > > - EFX_STATIC_ASSERT(sizeof(*dev_link) == sizeof(rte_atomic64_t)); > > > > sfc_port_link_mode_to_info(link_mode, &new_link); > > + if (_rte_eth_linkstatus_set(sa->eth_dev, &new_link) == 0) > > + return B_FALSE; > > > > - new_link_u64 = *(uint64_t *)&new_link; > > - do { > > - old_link_u64 = rte_atomic64_read((rte_atomic64_t *)dev_link); > > - if (old_link_u64 == new_link_u64) > > - break; > > - > > - if (rte_atomic64_cmpset((volatile uint64_t *)dev_link, > > - old_link_u64, new_link_u64)) { > > - evq->sa->port.lsc_seq++; > > - break; > > - } > > - } while (B_TRUE); > > - > > - return B_FALSE; > > + evq->sa->port.lsc_seq++; > > + return B_TRUE; > > It still returns B_TRUE, but should return B_FALSE as before. > Also before the patch lsc_seq is incremented in the case of any > changes in link status, but now in the case of up/down change only. The old code looked broken and did not match the comments. It always returned B_FALSE independent of whether link status changed or not.