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 CABFA41C4A; Fri, 10 Feb 2023 02:47:56 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id AA0FD40EE6; Fri, 10 Feb 2023 02:47:56 +0100 (CET) Received: from szxga02-in.huawei.com (szxga02-in.huawei.com [45.249.212.188]) by mails.dpdk.org (Postfix) with ESMTP id D271C40EE3 for ; Fri, 10 Feb 2023 02:47:54 +0100 (CET) Received: from dggpeml500024.china.huawei.com (unknown [172.30.72.56]) by szxga02-in.huawei.com (SkyGuard) with ESMTP id 4PCc5Y3NgtzJsLP; Fri, 10 Feb 2023 09:46:09 +0800 (CST) Received: from [10.67.100.224] (10.67.100.224) by dggpeml500024.china.huawei.com (7.185.36.10) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.34; Fri, 10 Feb 2023 09:47:52 +0800 Subject: Re: [PATCH v3 01/16] gso: remove logtype To: Stephen Hemminger , CC: , Konstantin Ananyev , Mark Kavanagh References: <20230207204151.1503491-1-stephen@networkplumber.org> <20230210010724.890413-1-stephen@networkplumber.org> <20230210010724.890413-2-stephen@networkplumber.org> From: fengchengwen Message-ID: Date: Fri, 10 Feb 2023 09:47:52 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.11.0 MIME-Version: 1.0 In-Reply-To: <20230210010724.890413-2-stephen@networkplumber.org> Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [10.67.100.224] X-ClientProxiedBy: dggems706-chm.china.huawei.com (10.3.19.183) To dggpeml500024.china.huawei.com (7.185.36.10) X-CFilter-Loop: Reflected 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 2023/2/10 9:07, Stephen Hemminger wrote: > 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 > - 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. Hi Stephen, 1. this patch do two thing (remove logtype and return -EINVAL), suggest seperate them. 2. I think we should keep rte_gso_segment return 0 when unmatch: a. the rte_gso_segment API requirement input mbuf set right ol_flags: * Before calling rte_gso_segment(), applications must set proper ol_flags * for the packet. The GSO library uses the same macros as that of TSO. * For example, set RTE_MBUF_F_TX_TCP_SEG and RTE_MBUF_F_TX_IPV4 in ol_flags to segment * a TCP/IPv4 packet. If rte_gso_segment() succeeds, the RTE_MBUF_F_TX_TCP_SEG * flag is removed for all GSO segments and the input packet. b. the rte_gso_segment filter and process mbuf according "struct rte_gso_ctx", for one stream which combine with udp and vxlan-tcp, we could only segment vxlan-tcp. Thanks. > > 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 (ret < 0) { >