From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-f66.google.com (mail-wr1-f66.google.com [209.85.221.66]) by dpdk.org (Postfix) with ESMTP id 3AC401BBF4 for ; Thu, 18 Apr 2019 17:51:58 +0200 (CEST) Received: by mail-wr1-f66.google.com with SMTP id k1so2382272wrw.0 for ; Thu, 18 Apr 2019 08:51:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=tbkrYmQUHA9+pmpJj24kft+AsRqDZMVHWzpQI2lfiEU=; b=PZhK0dB4UzuG9gyxi01s05eXKF1KChdQJWhmF+PY4f05FpiDFm9bIqt54le5rEn/J1 CCjMF4AT+eR5rZROqw1z4VCT+LtKWcHsrtqlEAe1nW+gLTE9m2qPoQG7A+FFNXaB/q23 XAzyjFIGKMlwY8mJoQcut/UkR96tpmG67d2CQmn2JrCnAbeD8nIo6gbl6Q5DRf76tQt3 DY3ZKH/Q1koRl5CVS+KMlI4m4RrVWfyOExre1cSXdxj1tUW8VZdMOfhAtzqNtVDeoWub W6kAOqQ9wAqXldHcNitwhs9+yLXdlqzNKJHPp7UAdecIe10XNaabRUv45B6mITF9UO2E zKaw== 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:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=tbkrYmQUHA9+pmpJj24kft+AsRqDZMVHWzpQI2lfiEU=; b=EWCFNRlNKWCgPlOuyiTYjehC/0jRZ4zrFCWKIGK7DxaPIbjyycdCnQl7/Oca9YKX2I poy5xN8BrpvfCN22JGCvAZYzYaYNH9vu0fhMc2Ssw/6CuOvZx9OjRakeDwDvt3WVK6Hm aPeWRCGdNxecPCQO3aYOJUlr4aVVcbAvmdBC4v6SmgDI3bLBM+tiuylwjBoUCpfh8VBJ I/1M5w8B6jfmAZBehJxybM+iKdWvTxcnkhZ7bF1x4YcUs7NmLp8JUi35OtXZlvJ/h5xf XFNy2bFoudm8jJYJXc5I4km5d99+RMsiQBIwFSiueKOCxBLy6aZWE58STbj2wfU8WC3N Y7UQ== X-Gm-Message-State: APjAAAVpSp3hB17ro40VBoWuch3hJUWKnI/b00W+oOVs8HYHPTGKD8so 7m3wsXtJiQLWDEj29JsmBhW1rQ== X-Google-Smtp-Source: APXvYqxKtCrNH8wLPphhYpKy2RWwIVUugrJDiRjZp87j18jJfo61gH7CHAFJmQlYe7tdjjjwqj/z6g== X-Received: by 2002:a05:6000:128a:: with SMTP id f10mr2201777wrx.296.1555602717921; Thu, 18 Apr 2019 08:51:57 -0700 (PDT) Received: from bidouze.vm.6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id i17sm3373436wrs.44.2019.04.18.08.51.56 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Thu, 18 Apr 2019 08:51:56 -0700 (PDT) Date: Thu, 18 Apr 2019 17:51:54 +0200 From: =?iso-8859-1?Q?Ga=EBtan?= Rivet To: Thomas Monjalon Cc: Adrien Mazarguil , dev@dpdk.org, Ferruh Yigit , David Marchand , stable@dpdk.org Message-ID: <20190418155154.e65prl6i4wjoe7af@bidouze.vm.6wind.com> References: <20190418130419.25675-1-adrien.mazarguil@6wind.com> <20190418152229.13554-1-adrien.mazarguil@6wind.com> <8815526.EAVJVpUmGC@xps> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <8815526.EAVJVpUmGC@xps> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [dpdk-dev] [PATCH v2] 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 18 Apr 2019 15:51:58 -0000 On Thu, Apr 18, 2019 at 05:39:37PM +0200, Thomas Monjalon wrote: > 18/04/2019 17:32, Adrien Mazarguil: > > 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 > > Reviewed-by: David Marchand > > Acked-by: Gaetan Rivet > > -- > > v2 changes: > > > > Modified "rxq->priv->dev->data->port_id" (v18.11-style) to > > "rxq->priv->data->port_id" (since v19.05) and checked compilation against > > master this time. > > > > Given the limited scope of that change, reviewed-by/acked-by lines were > > kept. > > --- > > +/* > > + * 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. > > "slave" is a wording from bonding. > In failsafe, it is sub-device, isn't it? > Yes, there is however one other comment in failsafe code refering to a sub-device as a slave. I'm not really up-to-par with the LSF CoC[1] and whether it is aligned with the Contributor Covenant also adopted by Linux[2]. I guess you were only referring to using the proper nomenclature and not this subject, but I can't pass on an opportunity to out-nitpick :D . This can be changed on merge as sub-device is more correct. Overall personally I don't really care either way. [1]: https://www.linuxfoundation.org/code-of-conduct/ [2]: https://www.kernel.org/doc/html/latest/process/code-of-conduct.html > > + */ > > +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->data->port_id); > > return nb_rx; > > } > > I'm afraid the performance drop to be hard. > How the port id in mbuf is used exactly? What crash are you seeing? > > -- Gaëtan Rivet 6WIND From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id BDDFBA00E6 for ; Thu, 18 Apr 2019 17:51:58 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 9118F1BBF6; Thu, 18 Apr 2019 17:51:58 +0200 (CEST) Received: from mail-wr1-f66.google.com (mail-wr1-f66.google.com [209.85.221.66]) by dpdk.org (Postfix) with ESMTP id 3AC401BBF4 for ; Thu, 18 Apr 2019 17:51:58 +0200 (CEST) Received: by mail-wr1-f66.google.com with SMTP id k1so2382272wrw.0 for ; Thu, 18 Apr 2019 08:51:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=tbkrYmQUHA9+pmpJj24kft+AsRqDZMVHWzpQI2lfiEU=; b=PZhK0dB4UzuG9gyxi01s05eXKF1KChdQJWhmF+PY4f05FpiDFm9bIqt54le5rEn/J1 CCjMF4AT+eR5rZROqw1z4VCT+LtKWcHsrtqlEAe1nW+gLTE9m2qPoQG7A+FFNXaB/q23 XAzyjFIGKMlwY8mJoQcut/UkR96tpmG67d2CQmn2JrCnAbeD8nIo6gbl6Q5DRf76tQt3 DY3ZKH/Q1koRl5CVS+KMlI4m4RrVWfyOExre1cSXdxj1tUW8VZdMOfhAtzqNtVDeoWub W6kAOqQ9wAqXldHcNitwhs9+yLXdlqzNKJHPp7UAdecIe10XNaabRUv45B6mITF9UO2E zKaw== 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:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=tbkrYmQUHA9+pmpJj24kft+AsRqDZMVHWzpQI2lfiEU=; b=EWCFNRlNKWCgPlOuyiTYjehC/0jRZ4zrFCWKIGK7DxaPIbjyycdCnQl7/Oca9YKX2I poy5xN8BrpvfCN22JGCvAZYzYaYNH9vu0fhMc2Ssw/6CuOvZx9OjRakeDwDvt3WVK6Hm aPeWRCGdNxecPCQO3aYOJUlr4aVVcbAvmdBC4v6SmgDI3bLBM+tiuylwjBoUCpfh8VBJ I/1M5w8B6jfmAZBehJxybM+iKdWvTxcnkhZ7bF1x4YcUs7NmLp8JUi35OtXZlvJ/h5xf XFNy2bFoudm8jJYJXc5I4km5d99+RMsiQBIwFSiueKOCxBLy6aZWE58STbj2wfU8WC3N Y7UQ== X-Gm-Message-State: APjAAAVpSp3hB17ro40VBoWuch3hJUWKnI/b00W+oOVs8HYHPTGKD8so 7m3wsXtJiQLWDEj29JsmBhW1rQ== X-Google-Smtp-Source: APXvYqxKtCrNH8wLPphhYpKy2RWwIVUugrJDiRjZp87j18jJfo61gH7CHAFJmQlYe7tdjjjwqj/z6g== X-Received: by 2002:a05:6000:128a:: with SMTP id f10mr2201777wrx.296.1555602717921; Thu, 18 Apr 2019 08:51:57 -0700 (PDT) Received: from bidouze.vm.6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id i17sm3373436wrs.44.2019.04.18.08.51.56 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Thu, 18 Apr 2019 08:51:56 -0700 (PDT) Date: Thu, 18 Apr 2019 17:51:54 +0200 From: =?iso-8859-1?Q?Ga=EBtan?= Rivet To: Thomas Monjalon Cc: Adrien Mazarguil , dev@dpdk.org, Ferruh Yigit , David Marchand , stable@dpdk.org Message-ID: <20190418155154.e65prl6i4wjoe7af@bidouze.vm.6wind.com> References: <20190418130419.25675-1-adrien.mazarguil@6wind.com> <20190418152229.13554-1-adrien.mazarguil@6wind.com> <8815526.EAVJVpUmGC@xps> MIME-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <8815526.EAVJVpUmGC@xps> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [dpdk-dev] [PATCH v2] 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Message-ID: <20190418155154.DYKenIwwlvczbxr3RG3-A5oqewUfV9x0tLiREq4vX9M@z> On Thu, Apr 18, 2019 at 05:39:37PM +0200, Thomas Monjalon wrote: > 18/04/2019 17:32, Adrien Mazarguil: > > 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 > > Reviewed-by: David Marchand > > Acked-by: Gaetan Rivet > > -- > > v2 changes: > > > > Modified "rxq->priv->dev->data->port_id" (v18.11-style) to > > "rxq->priv->data->port_id" (since v19.05) and checked compilation against > > master this time. > > > > Given the limited scope of that change, reviewed-by/acked-by lines were > > kept. > > --- > > +/* > > + * 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. > > "slave" is a wording from bonding. > In failsafe, it is sub-device, isn't it? > Yes, there is however one other comment in failsafe code refering to a sub-device as a slave. I'm not really up-to-par with the LSF CoC[1] and whether it is aligned with the Contributor Covenant also adopted by Linux[2]. I guess you were only referring to using the proper nomenclature and not this subject, but I can't pass on an opportunity to out-nitpick :D . This can be changed on merge as sub-device is more correct. Overall personally I don't really care either way. [1]: https://www.linuxfoundation.org/code-of-conduct/ [2]: https://www.kernel.org/doc/html/latest/process/code-of-conduct.html > > + */ > > +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->data->port_id); > > return nb_rx; > > } > > I'm afraid the performance drop to be hard. > How the port id in mbuf is used exactly? What crash are you seeing? > > -- Gaëtan Rivet 6WIND