From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
by dpdk.space (Postfix) with ESMTP id 3509AA00E6
for <public@inbox.dpdk.org>; Thu, 18 Apr 2019 16:06:45 +0200 (CEST)
Received: from [92.243.14.124] (localhost [127.0.0.1])
by dpdk.org (Postfix) with ESMTP id 077221BAA7;
Thu, 18 Apr 2019 16:06:45 +0200 (CEST)
Received: from mail-it1-f193.google.com (mail-it1-f193.google.com
[209.85.166.193]) by dpdk.org (Postfix) with ESMTP id 0EC8A1BAA0
for <dev@dpdk.org>; Thu, 18 Apr 2019 16:06:43 +0200 (CEST)
Received: by mail-it1-f193.google.com with SMTP id k64so3457595itb.5
for <dev@dpdk.org>; Thu, 18 Apr 2019 07:06:42 -0700 (PDT)
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=1e100.net; s=20161025;
h=x-gm-message-state:mime-version:references:in-reply-to:from:date
:message-id:subject:to:cc;
bh=xBtfOXiLyJf4aU1CAKW0sNuKYNQe4JPkH4wlZevgEbM=;
b=kB2J/O48VnvdKYSGQlQB4URCNWgnfltHBaa1sugc087KaJaVIyuAY1BIQ4t50Ba79C
WVtkRtC3jsWUqEctwgWzmG53bdNPIlm3MqYtO9HhX6JOtsxdJ/fjbNq/3e5LN2t/9LA2
/vsRThSucv0EjavfD/ffOyT8C9j5aJPsT4l0e+KcS9NFUjny79yBUnqJbH/0KT5ovDO0
Yy1l3aRBmcPaqcRifeuKHmpo+GOo+d29BC7+nFtztpW+R84/0Sauh6/4EuZm3DXvUdCR
zk95OgRKJnNidqXsIZeGQzxtR/C5nUFbhJHVl9tFAH4P+aI3TMNc92XLxZ57vqISR8If
Ymbg==
X-Gm-Message-State: APjAAAWFRXZ3kCussqFD5p0hB/H5AUcGZ91lXvcq48G6uKk/+K/i5B28
j4T67zO2M12sPKnfH3D/8M1jqQM2qnjsw1Mjo0dWcg==
X-Google-Smtp-Source: APXvYqzf/JDAG9oipTXnlgUm43Jg5Zl1hLkGbIwwXqosTAVOZDQIpXkQXxXzxvyIckur4Hsq788GFWY3hyQn0pNkaJA=
X-Received: by 2002:a24:3ec6:: with SMTP id s189mr3563450its.138.1555596402402;
Thu, 18 Apr 2019 07:06:42 -0700 (PDT)
MIME-Version: 1.0
References: <20190418130419.25675-1-adrien.mazarguil@6wind.com>
In-Reply-To: <20190418130419.25675-1-adrien.mazarguil@6wind.com>
From: David Marchand <david.marchand@redhat.com>
Date: Thu, 18 Apr 2019 16:06:31 +0200
Message-ID:
<CAJFAV8zLVkmWp6_ENK2kaPHXig0=mtTOGZ2_ER1Kf0ASJAEBAQ@mail.gmail.com>
To: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Cc: Gaetan Rivet <gaetan.rivet@6wind.com>,
Ferruh Yigit <ferruh.yigit@intel.com>, dev <dev@dpdk.org>,
Chas Williams <3chas3@gmail.com>
Content-Type: text/plain; charset="UTF-8"
X-Content-Filtered-By: Mailman/MimeDel 2.1.15
Subject: Re: [dpdk-dev] [PATCH] net/failsafe: fix source port ID in Rx
packets
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>
Message-ID: <20190418140631.iWTHJwxDaznNrdDooS2pp4IpfHHTuS5EgVqZygfed4w@z>
Hey Adrien,
On Thu, Apr 18, 2019 at 3:12 PM Adrien Mazarguil <adrien.mazarguil@6wind.com>
wrote:
> When passed to the application, Rx packets retain the port ID value
> originally set by slave devices. Unfortunately these IDs have no meaning to
> applications, which are typically unaware of their existence.
>
> This confuses those caring about the source port field in mbufs (m->port)
> which experience issues ranging from traffic drop to crashes.
>
> Fixes: a46f8d584eb8 ("net/failsafe: add fail-safe PMD")
>
Cc: stable@dpdk.org
Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> ---
> drivers/net/failsafe/failsafe_rxtx.c | 21 +++++++++++++++++++++
> 1 file changed, 21 insertions(+)
>
> diff --git a/drivers/net/failsafe/failsafe_rxtx.c
> b/drivers/net/failsafe/failsafe_rxtx.c
> index 231c83291..e78624127 100644
> --- a/drivers/net/failsafe/failsafe_rxtx.c
> +++ b/drivers/net/failsafe/failsafe_rxtx.c
> @@ -61,6 +61,21 @@ failsafe_set_burst_fn(struct rte_eth_dev *dev, int
> force_safe)
> rte_wmb();
> }
>
> +/*
> + * Override source port in Rx packets.
> + *
> + * Make Rx packets originate from this PMD instance instead of one of its
> + * slaves. This is mandatory to avoid breaking applications.
> + */
> +static void
> +failsafe_rx_set_port(struct rte_mbuf **rx_pkts, uint16_t nb_pkts,
> uint16_t port)
> +{
> + unsigned int i;
> +
> + for (i = 0; i != nb_pkts; ++i)
> + rx_pkts[i]->port = port;
> +}
> +
> uint16_t
> failsafe_rx_burst(void *queue,
> struct rte_mbuf **rx_pkts,
> @@ -87,6 +102,9 @@ failsafe_rx_burst(void *queue,
> sdev = sdev->next;
> } while (nb_rx == 0 && sdev != rxq->sdev);
> rxq->sdev = sdev;
> + if (nb_rx)
> + failsafe_rx_set_port(rx_pkts, nb_rx,
> + rxq->priv->dev->data->port_id);
> return nb_rx;
> }
>
> @@ -112,6 +130,9 @@ failsafe_rx_burst_fast(void *queue,
> sdev = sdev->next;
> } while (nb_rx == 0 && sdev != rxq->sdev);
> rxq->sdev = sdev;
> + if (nb_rx)
> + failsafe_rx_set_port(rx_pkts, nb_rx,
> + rxq->priv->dev->data->port_id);
> return nb_rx;
> }
>
> --
> 2.11.0
>
Not a big fan of those duplicated rx_burst functions...
Reviewed-by: David Marchand <david.marchand@redhat.com>
I suppose the bonding pmd has the same issue.
--
David Marchand