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 E482141C58; Fri, 10 Feb 2023 03:46:21 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id C155440EE6; Fri, 10 Feb 2023 03:46:21 +0100 (CET) Received: from mail-pj1-f48.google.com (mail-pj1-f48.google.com [209.85.216.48]) by mails.dpdk.org (Postfix) with ESMTP id B197740EE3 for ; Fri, 10 Feb 2023 03:46:20 +0100 (CET) Received: by mail-pj1-f48.google.com with SMTP id f15-20020a17090ac28f00b00230a32f0c9eso4209692pjt.4 for ; Thu, 09 Feb 2023 18:46:20 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20210112.gappssmtp.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=u8piyHqlSrktOG+x8lrngaAMplzHPoD50nsyPPnqusE=; b=gwQQQwHzqT2K8CV59/ERL4VwGTPXivffn72ebTqOaq7DNElFzNO0e2eI6Un6bjH1lv AnC6WTTOPe/QNEjc40qDXkWzqVtkBdikQQ/j7WZmgmJeUFlbuu/tUTxYENiE3H9jMBdI PAYpR+Qo+rOtXBeLhIAVq+zxIXCDgtoIl3sbw1E8kBmIxO2Ba5CAU82jWkC5I9CoTxN9 Ub2Haj2a+5AdMZozlZtxQOUTn0MFzqT4uYrkdOcOUKmt3zADU2uHi1egUgN8HYn6rKNt SUDF7VOvXJlPEUxnL5qzS0dZdiBF/ecIlFT8uPaAj9yqQaA/u0Z6fzs0i1a0i5AbHDZE y2yw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=u8piyHqlSrktOG+x8lrngaAMplzHPoD50nsyPPnqusE=; b=GWWHXP1ZjfArWVcx3xDEwWr7YRerRIgzVxnji8THuqV0+1eRCsjcfeeCmIwlMuYarL 1MR6+9FLcX+X3uwXYKQ+6n4ENK0WpkIMiajQUfxBtQbOELTLIRtLb+7BG6kkzdBo8ckA BSi+RkzTIaUEtYS2NpXj0dffToPiZ78xryIVHOOnhsk1zVJtruZhbxE/GYs5Z/xm3BTH W6fg0ILWOAE+2khXw+kCa/eQ0JfjVvU3noq2So8fSIw3Wd5eKhn3TOlPPCopkbSun/50 NVfNWdQyxS4P2xehEKjRS0zwtGZYlemhdEXVRBPKvxK+vD02oGmtNao4Fk2yVp/YQ9ZN TYbg== X-Gm-Message-State: AO0yUKWqFQgJkyEyAPvii3ghmoe6R+NgpScztocqWOVXMsrzeksDk9/1 7z3Ao8NoT3Iy7Zu1WecoVMaw6g== X-Google-Smtp-Source: AK7set/9992pyQNJ9s5WLLiqVadWuqOyeBS5RrN45UxBqYFEecwUQbSRAF60eDQDfd/F39jOo39Iqg== X-Received: by 2002:a17:902:e303:b0:19a:5b7b:24c6 with SMTP id q3-20020a170902e30300b0019a5b7b24c6mr3422362plc.48.1675997179782; Thu, 09 Feb 2023 18:46:19 -0800 (PST) Received: from hermes.local (204-195-120-218.wavecable.com. [204.195.120.218]) by smtp.gmail.com with ESMTPSA id c7-20020a170902848700b00194ac38bc86sm2211171plo.131.2023.02.09.18.46.19 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 09 Feb 2023 18:46:19 -0800 (PST) Date: Thu, 9 Feb 2023 18:46:17 -0800 From: Stephen Hemminger To: "Hu, Jiayu" Cc: "dev@dpdk.org" , Konstantin Ananyev , Mark Kavanagh Subject: Re: [PATCH v3 01/16] gso: remove logtype Message-ID: <20230209184617.569693f8@hermes.local> In-Reply-To: References: <20230207204151.1503491-1-stephen@networkplumber.org> <20230210010724.890413-1-stephen@networkplumber.org> <20230210010724.890413-2-stephen@networkplumber.org> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit 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 Fri, 10 Feb 2023 02:29:44 +0000 "Hu, Jiayu" wrote: > Hi Stephen, > > > -----Original Message----- > > From: Stephen Hemminger > > Sent: Friday, February 10, 2023 9:07 AM > > To: dev@dpdk.org > > Cc: Stephen Hemminger ; Hu, Jiayu > > ; Konstantin Ananyev > > ; Mark Kavanagh > > > > Subject: [PATCH v3 01/16] gso: remove logtype > > > > If a large packet is passed into GSO routines of unknown protocol then library > > would log a message and pass it through. This is incorrect behaviour on many > > levels: > > - it allows oversize packet to get passed on to NIC driver > > Applications use SW GSO only if NIC segmentation is not > supported. For a burst of packets mixed with different packet > types, GSO doesn't process the packet with unsupported types, > and the oversize packets go to NIC and get dropped finally. > I presume this case frequently happen in real cases. > > > - no direct return is visible to applications > > - if it happens once, many more will follow and log will fill. > > - bonus it is only log message with GSO type. > > > > The fix is to just return -EINVAL which is what this library does in many other > > places when looking at headers. > > > > Fixes: 119583797b6a ("gso: support TCP/IPv4 GSO") > > Cc: jiayu.hu@intel.com > > Signed-off-by: Stephen Hemminger > > --- > > lib/eal/common/eal_common_log.c | 2 +- > > lib/eal/include/rte_log.h | 1 - > > lib/gso/rte_gso.c | 3 +-- > > 3 files changed, 2 insertions(+), 4 deletions(-) > > > > diff --git a/lib/eal/common/eal_common_log.c > > b/lib/eal/common/eal_common_log.c index bd7b188ceb4a..c369154cb1ea > > 100644 > > --- a/lib/eal/common/eal_common_log.c > > +++ b/lib/eal/common/eal_common_log.c > > @@ -368,7 +368,7 @@ static const struct logtype logtype_strings[] = { > > {RTE_LOGTYPE_CRYPTODEV, "lib.cryptodev"}, > > {RTE_LOGTYPE_EFD, "lib.efd"}, > > {RTE_LOGTYPE_EVENTDEV, "lib.eventdev"}, > > - {RTE_LOGTYPE_GSO, "lib.gso"}, > > + > > {RTE_LOGTYPE_USER1, "user1"}, > > {RTE_LOGTYPE_USER2, "user2"}, > > {RTE_LOGTYPE_USER3, "user3"}, > > diff --git a/lib/eal/include/rte_log.h b/lib/eal/include/rte_log.h index > > 6d2b0856a565..97d6b26a9967 100644 > > --- a/lib/eal/include/rte_log.h > > +++ b/lib/eal/include/rte_log.h > > @@ -46,7 +46,6 @@ extern "C" { > > #define RTE_LOGTYPE_CRYPTODEV 17 /**< Log related to cryptodev. */ > > #define RTE_LOGTYPE_EFD 18 /**< Log related to EFD. */ > > #define RTE_LOGTYPE_EVENTDEV 19 /**< Log related to eventdev. */ > > -#define RTE_LOGTYPE_GSO 20 /**< Log related to GSO. */ > > > > /* these log types can be used in an application */ > > #define RTE_LOGTYPE_USER1 24 /**< User-defined log type 1. */ > > diff --git a/lib/gso/rte_gso.c b/lib/gso/rte_gso.c index > > 4b59217c16ee..19c351769fcc 100644 > > --- a/lib/gso/rte_gso.c > > +++ b/lib/gso/rte_gso.c > > @@ -81,8 +81,7 @@ rte_gso_segment(struct rte_mbuf *pkt, > > indirect_pool, pkts_out, nb_pkts_out); > > } else { > > /* unsupported packet, skip */ > > - RTE_LOG(DEBUG, GSO, "Unsupported packet type\n"); > > - ret = 0; > > + ret = -EINVAL; > > If applications want to know that the packet is failed on SW GSO, > I think ENOTSUP is better than EINVAL, as it is not invalid input > from users. Ok, just wanted to do the same thing as what would happen earlier in the code path for cases where the arguments are incorrect. Doing -ENOTSUP makes sense.