From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 802614264F; Wed, 27 Sep 2023 08:40:04 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 30285402D3; Wed, 27 Sep 2023 08:40:03 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mails.dpdk.org (Postfix) with ESMTP id C37B540271 for ; Wed, 27 Sep 2023 08:40:01 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1695796801; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=6uSvhps7x2tiBVyxgwkZYFL6HK5tRNJt4WgcEgAl4nk=; b=KP6YoxsJ3vY7eP0165NzqJddiqvQbUVczz058JXUBym/vRZe/ky+2zr5KrBHHCyrOfu+cL 4uVGq5eVM7uCnj7ie8txE6gROttPjB2Ov7WmtdYYQk0byR//L5Th3BFcaeEvE2aidPsBMD G7v9dsQ8FKXj6rjpz99byKpElTjIjWk= Received: from mail-lf1-f69.google.com (mail-lf1-f69.google.com [209.85.167.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-50-rI8SlusMOrmCKycCj42wSQ-1; Wed, 27 Sep 2023 02:39:59 -0400 X-MC-Unique: rI8SlusMOrmCKycCj42wSQ-1 Received: by mail-lf1-f69.google.com with SMTP id 2adb3069b0e04-5041a72d2edso16155108e87.1 for ; Tue, 26 Sep 2023 23:39:59 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1695796798; x=1696401598; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=6uSvhps7x2tiBVyxgwkZYFL6HK5tRNJt4WgcEgAl4nk=; b=iFD50uyRVag78sErThNMAZYIYMwzw6Rd9khtIi49p+ilE1ziJzwjKJm5R5gYWDarwe j45NFaOK9sx9jN4PqysUWVoW99Pmo8R+VUX4VPUWHAPXYRHXE+SzpVmxhYTwwPee59Xh Tza7Y9e/56IWAh8PmJYhgnSoYF+OZVXmJQbrmwKdbWVJ3mpFxCVypKTdDythLrHXTv7P Z8tZC1dwWJzSNrAmX/JA2HDneSAJbyuBbUZPRF5RxrSOYWouFNrVURQHSgazVzEaTFpV yMpjtsOvBeEHxQUJs19GHhhN/3fU/KgRwbssJHcNYhQTIMb6QlRE7mJ08EzPKdzoT82V EZ3A== X-Gm-Message-State: AOJu0YxR4R4hmAVdwnTmkNIMRbxJeWKSJmvQ5O6MS1hEkWLhOS44nglk bs9DO6n8tm1lmaQWUQKQpWsXXhh6IKrt0llOyD1zSbcT9Z9fPi7JxdcqYI210q4WXCQp40GyzrV 6bB/CsNFnYys43VupzHU= X-Received: by 2002:a05:6512:3c8f:b0:500:9d6c:913e with SMTP id h15-20020a0565123c8f00b005009d6c913emr984246lfv.52.1695796798288; Tue, 26 Sep 2023 23:39:58 -0700 (PDT) X-Google-Smtp-Source: AGHT+IFApQuzJrGZigR6v/BSDoifuXuAJddJ3UX3q66jEOABPeOzEWutn0xklvLMKuE6NT888M8uW3catnYOJ5YVxQk= X-Received: by 2002:a05:6512:3c8f:b0:500:9d6c:913e with SMTP id h15-20020a0565123c8f00b005009d6c913emr984218lfv.52.1695796797916; Tue, 26 Sep 2023 23:39:57 -0700 (PDT) MIME-Version: 1.0 References: <20230811094043.200995-1-shiyangx.he@intel.com> <20230915130249.425790-1-shiyangx.he@intel.com> In-Reply-To: From: David Marchand Date: Wed, 27 Sep 2023 08:39:46 +0200 Message-ID: Subject: Re: [PATCH v3] net/iavf: add devargs to enable vf auto-reset To: Bruce Richardson , "Zhang, Qi Z" Cc: "He, ShiyangX" , "dev@dpdk.org" , "Zhou, YidingX" , "Wang, Liang-min" , "Su, Simei" , "Wu, Wenjun1" , "Zhang, Yuying" , "Xing, Beilei" , "Yang, Qiming" , "Wu, Jingjing" , Thomas Monjalon , Ferruh Yigit X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Tue, Sep 26, 2023 at 4:06=E2=80=AFPM Bruce Richardson wrote: > > On Tue, Sep 26, 2023 at 01:15:28PM +0100, Zhang, Qi Z wrote: > > > > > > > -----Original Message----- > > > From: Bruce Richardson > > > Sent: Tuesday, September 26, 2023 3:49 PM > > > To: He, ShiyangX > > > Cc: dev@dpdk.org; Zhou, YidingX ; Wang, Liang= - > > > min ; Su, Simei ; Wu, > > > Wenjun1 ; Zhang, Yuying > > > ; Xing, Beilei ; Yang, > > > Qiming ; Wu, Jingjing > > > Subject: Re: [PATCH v3] net/iavf: add devargs to enable vf auto-reset > > > > > > On Fri, Sep 15, 2023 at 01:02:49PM +0000, Shiyang He wrote: > > > > Originally, the iavf PMD does not perform special actions when it > > > > receives a PF-to-VF reset event, resulting in vf being offline and > > > > unavailable. > > > > > > > > This patch enables vf auto-reset by setting 'watchdog_period' devar= gs > > > > to true. The iavf PMD will perform an automatic reset to bring the = vf > > > > back online when it receives a PF-to-VF event. > > > > > > > > v2: handling reset by event handler > > > > v3: change reset process > > > > > > > > Signed-off-by: Shiyang He > > > > Signed-off-by: Liang-Min Larry Wang > > > > --- > > > > doc/guides/nics/intel_vf.rst | 3 + > > > > doc/guides/rel_notes/release_23_11.rst | 3 + > > > > drivers/net/iavf/iavf.h | 7 +++ > > > > drivers/net/iavf/iavf_ethdev.c | 86 ++++++++++++++++++++++= +--- > > > > drivers/net/iavf/iavf_rxtx.c | 52 ++++++++++------ > > > > drivers/net/iavf/iavf_vchnl.c | 11 +++- > > > > 6 files changed, 135 insertions(+), 27 deletions(-) > > > > > > > > diff --git a/doc/guides/nics/intel_vf.rst > > > > b/doc/guides/nics/intel_vf.rst index d365dbc185..c0acd2a7f5 100644 > > > > --- a/doc/guides/nics/intel_vf.rst > > > > +++ b/doc/guides/nics/intel_vf.rst > > > > @@ -101,6 +101,9 @@ For more detail on SR-IOV, please refer to the > > > following documents: > > > > Set ``devargs`` parameter ``watchdog_period`` to adjust the wa= tchdog > > > period in microseconds, or set it to 0 to disable the watchdog, > > > > for example, ``-a 18:01.0,watchdog_period=3D5000`` or ``-a > > > 18:01.0,watchdog_period=3D0``. > > > > > > > > + Enable vf auto-reset by setting the ``devargs`` parameter like= ``-a > > > 18:01.0,enable_auto_reset=3D1`` when IAVF is backed > > > > + by an Intel(r) E810 device or an Intel(r) 700 Series Ethernet = device. > > > > + > > > > > > Why do we need a devargs for this? If the VF is unavailable - as you = mention > > > in the commit log above - should this behaviour not always be the cas= e > > > without the user having to ask? > > > > Ideally it does not need, but with below considerations: > > > > 1. Not break existing scenario, which still assume some application wil= l call dev_reset /dev_configure/ queue_setup / ... after receive RTE_ETH_E= VENT_INTR_RESET event to recover the VF manually, the devargs make sure app= lication be aware of this new feature and will not call rte_eth_dev_reset w= hich will fail now. > > > > 2. intent to ensure a smoother transition, in case some corner case iss= ues evaded our validation, keeping this devargs provides us with the flexib= ility to remove it once we determine that the implementation is stable enou= gh. > > > Thanks for the clear explanation. > > One small suggestion: in the commit log, at the end of the first paragrap= h > change "resulting in the VF being offline and unavailable" to "... offlin= e > and unavailable until the application resets the device on receipt of the > RTE_ETH_EVENT_INTR_RESET event". Similarly at the end of the second > paragraph you could add "This change removes the need for the application > to handle the reset event, as it is transparently handled inside the > driver". I did not read the patch. You mention that rte_eth_dev_reset will fail now when this (horrific) devargs is passed. Yet, will the driver send a RTE_ETH_EVENT_INTR_RESET event? Additionnally, how does the driver make sure that no application/datapath thread is polling/reconfiguring this hw? --=20 David Marchand