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 DB8ABA0C4B; Wed, 7 Jul 2021 17:54:28 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id BFD9F413EE; Wed, 7 Jul 2021 17:54:28 +0200 (CEST) Received: from office2.cesnet.cz (office2.cesnet.cz [195.113.144.244]) by mails.dpdk.org (Postfix) with ESMTP id F0B24413B6 for ; Wed, 7 Jul 2021 17:54:26 +0200 (CEST) Received: from coaster.localdomain (unknown [IPv6:2001:67c:1220:80e:a9:edd0:2e93:a6c4]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by office2.cesnet.cz (Postfix) with ESMTPSA id 4E33C40006B; Wed, 7 Jul 2021 17:54:26 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cesnet.cz; s=office2-2020; t=1625673266; bh=drUiwe8RQeoYdZk5d0ylHQAy08R+L+NQGTE3Zv9hzTM=; h=Date:From:To:Cc:Subject:In-Reply-To:References; b=DT9m9pI3AxaeV9S0qGiWE6KcTzwgWhZBuq930OtC6iKbSgCaAfak7ICxP1HT33rnh NkpM6mTmE130fDMZ+fh9BebAVraAJtRqli1pB/YZHQ/FlcKk63f6yIP955qd3DOrqq JeYfjCHnN23Hq7X7HQUKOm6CaNzgmYur9jsNIDJ5ubod3/2ezvYwedUVWPrD86f2Oc b9h7cT2eHbWGoBiMEQDqBWQL/qlotp3bjGfTjlCK/NH/cnHj4EwZR4KT7/Kx6hnjX2 XkVe3PtpwZN5oCDpj0sAdcaCo6fV05mQxMzE8Fa7YoDzTQ0nanfkbzDLp9e/8zQN9j mPCA+NNGw3QwA== Date: Wed, 7 Jul 2021 17:54:20 +0200 From: Jan Viktorin To: Matan Azrad Cc: =?UTF-8?B?SGF2bMOtaw==?= Martin , "Min Hu (Connor)" , "chas3@att.com" , "dev@dpdk.org" , Shahaf Shuler , Slava Ovsiienko Message-ID: <20210707175420.4d81fdaf@coaster.localdomain> In-Reply-To: References: <20210622092531.73112-1-xhavli56@stud.fit.vutbr.cz> <20210622092531.73112-4-xhavli56@stud.fit.vutbr.cz> <5f2cd5e6-1c46-227c-7ab1-5a2a34eb337d@huawei.com> <01bd8d8fcd364584fecdea3bc70abd1c@stud.fit.vutbr.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [dpdk-dev] [PATCH 3/3] net/bonding: start ethdev prior to setting 8023ad flow 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 Sender: "dev" On Sun, 4 Jul 2021 15:18:01 +0000 Matan Azrad wrote: > From: Havl=C3=ADk Martin > > Dne 2021-06-23 09:04, Min Hu (Connor) napsal: =20 > > > =E5=9C=A8 2021/6/22 17:25, Martin Havlik =E5=86=99=E9=81=93: =20 > > >> When dedicated queues are enabled, mlx5 PMD fails to install RTE > > >> Flows if the underlying ethdev is not started: > > >> bond_ethdev_8023ad_flow_set(267) - bond_ethdev_8023ad_flow_set: =20 > > port =20 > > >> not started (slave_port=3D0 queue_id=3D1) > > >> =20 > > > Why mlx5 PMD doing flow create relys on port started ? > > > I noticed other PMDs did not has that reliance. > > > =20 > > After looking into it, I really can't answer this mlx5 centered > > question. Closest related info we found so far is the 5th point in > > https://doc.dpdk.org/guides/prog_guide/rte_flow.html#caveats > > but it only specifies it's the application's responsibility and > > that flow rules are assumed invalid after port stop/close/restart > > but doesn't say anything about vs > > where the former is the point of > > failure on mlx5. I'm addressing the maintainers for mlx5, who might > > know a bit more on the topic. =20 >=20 Hello Matan, > From rte_ethdev.h >=20 > * Please note that some configuration is not stored between calls to > * rte_eth_dev_stop()/rte_eth_dev_start(). The following > configuration will > * be retained: > * > * - MTU > * - flow control settings > * - receive mode configuration (promiscuous mode, all-multicast > mode, > * hardware checksum mode, RSS/VMDQ settings etc.) > * - VLAN filtering configuration > * - default MAC address > * - MAC addresses supplied to MAC address array > * - flow director filtering mode (but not filtering rules) > * - NIC queue statistics mappings just after this section, you can find the following statement: * Any other configuration will not be stored and will need to be re-entered * before a call to rte_eth_dev_start(). It is not very clear how is this exactly related to flows (and this applies for all the quoted section, I think) but at least it can be used as a counter argument. >=20 >=20 > Mlx5 assumes flows are allowed to be configured only after > rte_eth_dev_start(). Before start \ after stop - no flow is valid > anymore. I believe that this discussion is not about validity of flows. Let the flows be invalid after calling to rte_eth_dev_stop(). This is OK, flows must be recreated and the bonding driver works this way. But why not *before start*? Does somebody know how other drivers behaves in this situation? (We know and can check for Intel, there it does not seem to be an issue.) By the way, the mlx5 behaviour opens a (probably short) time window between starting of a port and configuation of filtering flows. You may want to start the port with thousands of flows that apply just when the port starts (not after, that's late). This may introduce glitches in filtering and measuring of traffic (well, it is a question how serious issue could it be...). This matters for the bonding case as well, doesn't it?. It is not desirable to accidently omit a packet that was received by primary ingress logic instead of being redirected into the dedicated queue. Are there any chances that for mlx5 it would be possible to insert flow rules before calling rte_eth_dev_start? Anyway, the behaviour should be specified and documented in DPDK more precisely to avoid such uncertainty in the future. Jan >=20 > Matan >=20 > > >> Signed-off-by: Martin Havlik > > >> Cc: Jan Viktorin > > >> --- > > >> drivers/net/bonding/rte_eth_bond_pmd.c | 26 > > >> ++++++++++++++++++-------- > > >> 1 file changed, 18 insertions(+), 8 deletions(-) > > >> > > >> diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c > > >> b/drivers/net/bonding/rte_eth_bond_pmd.c > > >> index a6755661c..fea3bc537 100644 > > >> --- a/drivers/net/bonding/rte_eth_bond_pmd.c > > >> +++ b/drivers/net/bonding/rte_eth_bond_pmd.c > > >> @@ -1818,25 +1818,35 @@ slave_configure(struct rte_eth_dev > > >> *bonded_eth_dev, > > >> rte_flow_destroy(slave_eth_dev->data->port_id, > > >> > > >> internals- =20 > > >mode4.dedicated_queues.flow[slave_eth_dev->data->port_id], =20 > > >> &flow_error); > > >> + } > > >> + /* Start device */ > > >> + errval =3D rte_eth_dev_start(slave_eth_dev->data->port_id); > > >> + if (errval !=3D 0) { > > >> + RTE_BOND_LOG(ERR, "rte_eth_dev_start: port=3D%u, err > > >> (%d)", > > >> + slave_eth_dev->data->port_id, > > >> errval); > > >> + return -1; > > >> + } > > >> + > > >> + if (internals->mode =3D=3D BONDING_MODE_8023AD && > > >> + internals->mode4.dedicated_queues.enabled > > >> =3D=3D 1) > > >> + { > > >> errval =3D bond_ethdev_8023ad_flow_set(bonded_eth_dev, > > >> slave_eth_dev->data->port_id); > > >> if (errval !=3D 0) { > > >> RTE_BOND_LOG(ERR, > > >> "bond_ethdev_8023ad_flow_set: > > >> port=3D%d, err (%d)", slave_eth_dev->data->port_id, errval); > > >> + > > >> + errval =3D > > >> rte_eth_dev_stop(slave_eth_dev->data->port_id); > > >> + if (errval < 0) { > > >> + RTE_BOND_LOG(ERR, > > >> + "rte_eth_dev_stop: port=3D%d, > > >> err (%d)", > > >> + > > >> slave_eth_dev->data->port_id, errval); > > >> + } > > >> return errval; > > >> } > > >> } > > >> - /* Start device */ > > >> - errval =3D rte_eth_dev_start(slave_eth_dev->data->port_id); > > >> - if (errval !=3D 0) { > > >> - RTE_BOND_LOG(ERR, "rte_eth_dev_start: port=3D%u, err > > >> (%d)", > > >> - slave_eth_dev->data->port_id, > > >> errval); > > >> - return -1; > > >> - } > > >> - > > >> /* If RSS is enabled for bonding, synchronize RETA */ > > >> if (bonded_eth_dev->data->dev_conf.rxmode.mq_mode & > > >> ETH_MQ_RX_RSS) { > > >> int i; > > >> =20