From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <stable-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
by inbox.dpdk.org (Postfix) with ESMTP id 5ADF9A0C43
for <public@inbox.dpdk.org>; Fri, 27 Aug 2021 15:44:21 +0200 (CEST)
Received: from [217.70.189.124] (localhost [127.0.0.1])
by mails.dpdk.org (Postfix) with ESMTP id 0988A41134;
Fri, 27 Aug 2021 15:44:21 +0200 (CEST)
Received: from mail-wm1-f50.google.com (mail-wm1-f50.google.com
[209.85.128.50])
by mails.dpdk.org (Postfix) with ESMTP id 8407A40150;
Fri, 27 Aug 2021 15:44:17 +0200 (CEST)
Received: by mail-wm1-f50.google.com with SMTP id
f9-20020a05600c1549b029025b0f5d8c6cso9264228wmg.4;
Fri, 27 Aug 2021 06:44:17 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;
h=mime-version:references:in-reply-to:from:date:message-id:subject:to
:cc; bh=Ok9vcLtS68ytqVIvs9GBONWtC2NZVjKCoX0lWb0i2BQ=;
b=QRpsr1fiPN29CuzuNhGohaw27vbpfbuBkwa2wH3Lp4laRtjpgLjoXOWKz8uS4BN6im
7NDZVtVAXpWyFePQeNQkizDI89ZA+wXr6VC+ALUxPymVVVOg8JyjIIAfl2hB8DFZfYy7
RB4pixpq3FJoGm1z5wiw0luXC35ijHKqKWZL4sPolhD6UfLgddT8mMJIuKqW/e/9MV3p
Vl/FyOiCZ10uJIrxItFDdIBeenwiQFXOmJRZY4BkAzzfMwGwGtlZBfjHVnEpp8t91yr5
eNPBfpVHtxuGIZ7TCVoFaHK4o5Gqxl/8d93OFNwvHkZ2lp8IJVSWUb9MA1POcYU85pXI
+pVw==
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=Ok9vcLtS68ytqVIvs9GBONWtC2NZVjKCoX0lWb0i2BQ=;
b=JZVTvd1kuEUj0BjnJQfeNlUu92NNb0UUZwSncQwH1m6/GoHzdyq8M0WxR+rIHP4wNH
13xVXXkLFvJmwZWuCcFk64T4AiWhX0nMfYG8fbWmgZXhfhnBZBahi/rTxYK2XgNsyskd
n345U6vTQXJqUJV8YSbtNe1QVM6yF6Y4+qNcpv5J0B+esL949RFqW/EGtgP6vkyCzjEh
f//YtAZHexwkUNesYtWH7mysAYy+KWbYMl6qU8MXC0OS3V07xpr2srjbdya2CG82epoB
bji3M5e0/JMjjnIf7eLFlx/ZjzgGd+29kgVZ44mWK8vrRlos0xdePce1D3pTS0yrOaJN
99kw==
X-Gm-Message-State: AOAM530bc57ZOcHdl1MPMxhPO3qDMfGNW9qCksZuF2da0/Wlud0XmCLP
ic7C73ZUTqHmttfRo5y3Tp0fG4Ws8pnG3AB8Vkk=
X-Google-Smtp-Source: ABdhPJy/kJQgE8D4I1lyfblqgLBZqfXAHWVUxejJcZDdaPzaIpEDS9SC+eznfLQEjp9tvoaGzKAh5/GrUGh8jQZDaPM=
X-Received: by 2002:a05:600c:3514:: with SMTP id
h20mr19898695wmq.31.1630071857226;
Fri, 27 Aug 2021 06:44:17 -0700 (PDT)
MIME-Version: 1.0
References: <20210630110404.21209-1-mohsin.kazmi14@gmail.com>
<20210707094019.40945-1-mohsin.kazmi14@gmail.com>
<0efac6a6-f5d1-48f1-3a95-a8b87469cc23@oktetlabs.ru>
<YQPeeY3p8RyBaqEb@platinum>
<6d7c41bf-6e9f-8fbd-bfc8-11ca07c777d8@oktetlabs.ru>
<CAKkt9+LyKRqc0BzaK5n1CJ-a6_nGt6vA77urDt+oFSD3+YUhkA@mail.gmail.com>
In-Reply-To: <CAKkt9+LyKRqc0BzaK5n1CJ-a6_nGt6vA77urDt+oFSD3+YUhkA@mail.gmail.com>
From: Mohsin Kazmi <mohsin.kazmi14@gmail.com>
Date: Fri, 27 Aug 2021 14:44:06 +0100
Message-ID: <CAKkt9++RhinZTxFZvDiGBhN9xFmQbdfn3=N3WD2qRuK2FAfA6Q@mail.gmail.com>
To: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
Cc: Olivier Matz <olivier.matz@6wind.com>, dev@dpdk.org, stable@dpdk.org
Content-Type: text/plain; charset="UTF-8"
X-Content-Filtered-By: Mailman/MimeDel 2.1.29
Subject: Re: [dpdk-stable] [dpdk-dev] [PATCH v3] net: fix Intel-specific
Prepare the outer ipv4 hdr for checksum
X-BeenThere: stable@dpdk.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: patches for DPDK stable branches <stable.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/stable>,
<mailto:stable-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/stable/>
List-Post: <mailto:stable@dpdk.org>
List-Help: <mailto:stable-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/stable>,
<mailto:stable-request@dpdk.org?subject=subscribe>
Errors-To: stable-bounces@dpdk.org
Sender: "stable" <stable-bounces@dpdk.org>
Are we good with this patch with the current state? @Olivier: Any comments
on the above suggestions?
On Tue, Aug 3, 2021 at 1:49 PM Mohsin Kazmi <mohsin.kazmi14@gmail.com>
wrote:
>
>
> On Sat, Jul 31, 2021 at 1:49 PM Andrew Rybchenko <
> andrew.rybchenko@oktetlabs.ru> wrote:
>
>> On 7/30/21 2:11 PM, Olivier Matz wrote:
>> > On Wed, Jul 28, 2021 at 06:46:53PM +0300, Andrew Rybchenko wrote:
>> >> On 7/7/21 12:40 PM, Mohsin Kazmi wrote:
>> >>> Preparation the headers for the hardware offload
>> >>> misses the outer ipv4 checksum offload.
>> >>> It results in bad checksum computed by hardware NIC.
>> >>>
>> >>> This patch fixes the issue by setting the outer ipv4
>> >>> checksum field to 0.
>> >>>
>> >>> Fixes: 4fb7e803eb1a ("ethdev: add Tx preparation")
>> >>> Cc: stable@dpdk.org
>> >>>
>> >>> Signed-off-by: Mohsin Kazmi <mohsin.kazmi14@gmail.com>
>> >>> Acked-by: Qi Zhang <qi.z.zhang@intel.com>
>> >>> ---
>> >>> v3:
>> >>> * Update the conditional test with PKT_TX_OUTER_IP_CKSUM.
>> >>> * Update the commit title with "Intel-specific".
>> >>>
>> >>> v2:
>> >>> * Update the commit message with Fixes.
>> >>>
>> >>> lib/net/rte_net.h | 15 +++++++++++++--
>> >>> 1 file changed, 13 insertions(+), 2 deletions(-)
>> >>>
>> >>> diff --git a/lib/net/rte_net.h b/lib/net/rte_net.h
>> >>> index 434435ffa2..3f4c8c58b9 100644
>> >>> --- a/lib/net/rte_net.h
>> >>> +++ b/lib/net/rte_net.h
>> >>> @@ -125,11 +125,22 @@ rte_net_intel_cksum_flags_prepare(struct
>> rte_mbuf *m, uint64_t ol_flags)
>> >>> * Mainly it is required to avoid fragmented headers check if
>> >>> * no offloads are requested.
>> >>> */
>> >>> - if (!(ol_flags & (PKT_TX_IP_CKSUM | PKT_TX_L4_MASK |
>> PKT_TX_TCP_SEG)))
>> >>> + if (!(ol_flags & (PKT_TX_IP_CKSUM | PKT_TX_L4_MASK |
>> PKT_TX_TCP_SEG |
>> >>> + PKT_TX_OUTER_IP_CKSUM)))
>> >>> return 0;
>> >>> - if (ol_flags & (PKT_TX_OUTER_IPV4 | PKT_TX_OUTER_IPV6))
>> >>> + if (ol_flags & (PKT_TX_OUTER_IPV4 | PKT_TX_OUTER_IPV6)) {
>> >>> inner_l3_offset += m->outer_l2_len + m->outer_l3_len;
>> >>> + /*
>> >>> + * prepare outer ipv4 header checksum by setting it to 0,
>> >>> + * in order to be computed by hardware NICs.
>> >>> + */
>> >>> + if (ol_flags & PKT_TX_OUTER_IP_CKSUM) {
>> >>> + ipv4_hdr = rte_pktmbuf_mtod_offset(m,
>> >>> + struct rte_ipv4_hdr *,
>> m->outer_l2_len);
>> >>> + ipv4_hdr->hdr_checksum = 0;
>> >>
>> >> Here we assume that the field is located in the first segment.
>> >> Unlikely but it still could be false. We must handle it properly.
>> >
>> > This is specified in the API comment, so I think it has to be checked
>> > by the caller.
>>
>> If no, what's the point to spoil memory here if stricter check is
>> done few lines below.
>>
> We have two possibilities:
> 1) take the whole block of above code after the strict check: Then strict
> check will use m->outer_l2_len + m->outer_l3_len directly without any
> condition and we will be on the mercy of drivers to initialize these to 0
> if outer headers are not use. Drivers usually don't set the fields which
> they are not interested in because of performance reasons as
> setting these values per packet will cost them additional cycles.
> 2) Taking just PKT_TX_OUTER_IP_CKSUM conditional check after the strict
> fragmented check: In that case, each packet will hit an extra conditional
> check without getting benefit from it, again with a performance penalty.
>
> I am more inclined towards solution 1. But I also welcome other
> suggestions/comments.
>
>>
>> >>> + }
>> >>> + }
>> >>> /*
>> >>> * Check if headers are fragmented.
>> >>>
>> >>
>>
>>