From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <thomas.monjalon@6wind.com>
Received: from mail-wi0-f178.google.com (mail-wi0-f178.google.com
 [209.85.212.178]) by dpdk.org (Postfix) with ESMTP id E9642532D
 for <dev@dpdk.org>; Sun, 25 Oct 2015 23:56:50 +0100 (CET)
Received: by wicll6 with SMTP id ll6so91582318wic.0
 for <dev@dpdk.org>; Sun, 25 Oct 2015 15:56:50 -0700 (PDT)
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20130820;
 h=x-gm-message-state:from:to:cc:subject:date:message-id:organization
 :user-agent:in-reply-to:references:mime-version
 :content-transfer-encoding:content-type;
 bh=ya5W5unPmi/1aR9oB/5vsqewTuaNEYEFKbE29H5zAoA=;
 b=DMmRtvRfE4SsyFJLvXlPjkoL1WqzVOAfft/vXBagKHmBVagib+w018o27taPPQHru2
 FXREVQAPZ7zdfU2xA9qYyqEoZRwr/2dvVOMxdJFzQEhr1MYS6TSP67lAgExtAll2No0Q
 coKLEBErLksen8L2PxrARleCiF77sdgpp4W3RN1RvpPSVWhOCWegKLeEDbVYtt2x7CxH
 aMBkiga9mEYNUIsE966SnVZUR082hMAVnP/qNRhXCVEqLD249JJX7fgX2IUgRX8uz8o8
 YnqjgpiUxTq/+C7Gm/Y+gus0U/LPtqIcyW0OMm5VoJ4fNO1lggVRIiMVaw5PNKdhvfKq
 OaeA==
X-Gm-Message-State: ALoCoQmdjWghzhei7/o/Xo04YN/918oVnfpesMFzcNFZZp43+yAiI062YwfOu1Hekmfy4HRhSxR1
X-Received: by 10.180.72.146 with SMTP id d18mr17431744wiv.50.1445813810769;
 Sun, 25 Oct 2015 15:56:50 -0700 (PDT)
Received: from xps13.localnet (136-92-190-109.dsl.ovh.fr. [109.190.92.136])
 by smtp.gmail.com with ESMTPSA id h4sm15742877wjx.41.2015.10.25.15.56.49
 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);
 Sun, 25 Oct 2015 15:56:50 -0700 (PDT)
From: Thomas Monjalon <thomas.monjalon@6wind.com>
To: wenzhuo.lu@intel.com
Date: Sun, 25 Oct 2015 23:55:43 +0100
Message-ID: <6060754.bW1Xky3pZ1@xps13>
Organization: 6WIND
User-Agent: KMail/4.14.10 (Linux/4.1.6-1-ARCH; KDE/4.14.11; x86_64; ; )
In-Reply-To: <33526A3108217C45B7DAFFA5277E4B67485277B3@mbx024-e1-nj-2.exch024.domain.local>
References: <33526A3108217C45B7DAFFA5277E4B67485277B3@mbx024-e1-nj-2.exch024.domain.local>
MIME-Version: 1.0
Content-Transfer-Encoding: 7Bit
Content-Type: text/plain; charset="us-ascii"
Cc: dev@dpdk.org, Tim Shearer <tim.shearer@overturenetworks.com>
Subject: Re: [dpdk-dev] [PATCH] librte: Link status interrupt race condition,
	IGB E1000
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://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: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Sun, 25 Oct 2015 22:56:51 -0000

Wenzhuo,
Please could you have a look?
Thanks

2015-09-24 20:44, Tim Shearer:
> I encountered an issue with DPDK 2.1.0  which occasionally causes the link status interrupt callback not to be called after the interface is started for the first time. I traced the problem back to the function eth_igb_link_update(), which is used to determine if the link has changed state since the previous time it was called. It appears that this function can be called simultaneously from two different threads:
> 
> (1) From the main application/configuration thread, via rte_eth_dev_start() - pointed to by (*dev->dev_ops->link_update)
> (2) From the eal interrupt thread, via eth_igb_interrupt_action(), to check if the link state has transitioned up or down. The user callback is only executed if the link has changed state.
> 
> The race condition manifests itself as follows:
>  - Main thread configures the interface with link status interrupt (LSI) enabled, sets up the queues etc.
>  - Main thread calls rte_eth_dev_start. The interface is started and then we call eth_igb_link_update()
>  - While in this call, the link goes up. Accordingly, we  detect the transition, and write the new link state (up) into the global rte_eth_dev struct
>  - The interrupt fires, which also drops into the eth_igb_link_update function, finds that the global link status has already been set to up (no change)
>  - Therefore, the handler thinks the interrupt was spurious, and the callback doesn't get called.
> 
> I suspect that rte_eth_dev_start shouldn't be checking the link state if interrupts are enabled. Would someone mind taking a quick look at the patch below?
> 
> Thanks!
> Tim
> 
> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
> @@ -1300,7 +1300,7 @@ rte_eth_dev_start(uint8_t port_id)
>  
>         rte_eth_dev_config_restore(port_id);
>  
> -       if (dev->data->dev_conf.intr_conf.lsc != 0) {
> +       if (dev->data->dev_conf.intr_conf.lsc == 0) {
>                 FUNC_PTR_OR_ERR_RET(*dev->dev_ops->link_update, -ENOTSUP);
>                 (*dev->dev_ops->link_update)(dev, 0);
>         }
> 
>