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 4D9ADA0093 for ; Thu, 9 Dec 2021 17:29:05 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 1C68A40041; Thu, 9 Dec 2021 17:29:05 +0100 (CET) Received: from mail-wm1-f49.google.com (mail-wm1-f49.google.com [209.85.128.49]) by mails.dpdk.org (Postfix) with ESMTP id C690840041 for ; Thu, 9 Dec 2021 17:29:03 +0100 (CET) Received: by mail-wm1-f49.google.com with SMTP id r9-20020a7bc089000000b00332f4abf43fso5592971wmh.0 for ; Thu, 09 Dec 2021 08:29:03 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind.com; s=google; h=from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=fl1mHuw7ZP8RzV7X7DU4dkDJoNVXbJne5NC41SSf7L8=; b=Dqk8eYAwgUrNSLUHWFKwcv18Z4G15u5To/w2l/Q3k3Na0ZVrLjQqG/rMX4Tutv4tBr KtCUI7nRi9VfQRymm5JC4Fuen1hklqUmOnsbleMKJnrvw19R7ESXF5/DwBKVHEdJNigk C6vzprESXMb17RxzedBp5/H8Etu077tus87uV5PBiNn7TAuXsaL5eSieO9RS+AHWX5PF C+Ui4V2hU09kaywWQyZRqrDCWV4lL1nY4EhbIip5Gse5NetZvfldUHkCBdn20QBb6DeE ZNIxXl1EAnK3mCGaNNIcflU86vLgCwQUHALSEuGaf08TH9/4pNy/tTFsOB3E+AnZwgic WEEg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=fl1mHuw7ZP8RzV7X7DU4dkDJoNVXbJne5NC41SSf7L8=; b=zRhInPtIxN7oCuq4yGMB+GodStd18q29Cx8NTHlTMGvCXZZnihTNAR2n8Bln/Rg7I0 34mVye9P5s5wY9iOv8mcgkxsrwYb2e4eJMM75QfWZDOuQGLBA2xoOGKCWP7Pd7mE979L QdwTkwv7zVVoqRG9ZYZIJc0ud3BNPZ1lhz+0QAZB0NxF+EQN33NHHTFL8hweESApF+dP nblHoH7Op0AY5HNxESxUbKi2aph8AVYhOrdUT7BaUxwaTyEMD5DbnKn2+WwyEkUwP+yc cJxxOnQg6AYxDfpk0lnAJ6nojVaFIR9ZzAGEjsSf5LX7pfPLMiTEbpMYFAb/AUkNUaUE eVaA== X-Gm-Message-State: AOAM5337RoNpg41BsQzuqIwkrqRFtfrtWEiKQFvmf+gq3esw/GPhxJMh ACNjI704fbx2SHR00EX00FQvEv8EgA9lOg== X-Google-Smtp-Source: ABdhPJz9ZswlwKq87aMEcDT0denxWxmDfy5e5ETaWP+Hp5ojmlXrf2Uo5CBJHeD1OHsN7b/uD3KeRg== X-Received: by 2002:a1c:20d3:: with SMTP id g202mr8440486wmg.129.1639067343479; Thu, 09 Dec 2021 08:29:03 -0800 (PST) Received: from gojira.dev.6wind.com ([185.13.181.2]) by smtp.gmail.com with ESMTPSA id e8sm170284wrr.26.2021.12.09.08.29.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 09 Dec 2021 08:29:03 -0800 (PST) From: Olivier Matz To: stable@dpdk.org Cc: =?UTF-8?q?Morten=20Br=C3=B8rup?= , Ajit Khaparde , Konstantin Ananyev , Ali Alnubani Subject: [PATCH 19.11] mbuf: fix reset on mbuf free Date: Thu, 9 Dec 2021 17:24:06 +0100 Message-Id: <20211209162405.20142-1-olivier.matz@6wind.com> X-Mailer: git-send-email 2.30.2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org [ upstream commit efc6f9104c80d39ec168d9559accdc7609274eca ] m->nb_seg must be reset on mbuf free whatever the value of m->next, because it can happen that m->nb_seg is != 1. For instance in this case: m1 = rte_pktmbuf_alloc(mp); rte_pktmbuf_append(m1, 500); m2 = rte_pktmbuf_alloc(mp); rte_pktmbuf_append(m2, 500); rte_pktmbuf_chain(m1, m2); m0 = rte_pktmbuf_alloc(mp); rte_pktmbuf_append(m0, 500); rte_pktmbuf_chain(m0, m1); As rte_pktmbuf_chain() does not reset nb_seg in the initial m1 segment (this is not required), after this code the mbuf chain have 3 segments: - m0: next=m1, nb_seg=3 - m1: next=m2, nb_seg=2 - m2: next=NULL, nb_seg=1 Then split this chain between m1 and m2, it would result in 2 packets: - first packet - m0: next=m1, nb_seg=2 - m1: next=NULL, nb_seg=2 - second packet - m2: next=NULL, nb_seg=1 Freeing the first packet will not restore nb_seg=1 in the second segment. This is an issue because it is expected that mbufs stored in pool have their nb_seg field set to 1. Fixes: 8f094a9ac5d7 ("mbuf: set mbuf fields while in pool") Signed-off-by: Olivier Matz Acked-by: Morten Brørup Acked-by: Ajit Khaparde Acked-by: Konstantin Ananyev Tested-by: Ali Alnubani --- Hi Christian, Sorry, this backport arrives after the rc1. Hope it's not too late. Thanks, Olivier app/test/test_mbuf.c | 70 +++++++++++++++++++++++++++++++++ lib/librte_mbuf/rte_mbuf.h | 8 ++-- lib/librte_mbuf/rte_mbuf_core.h | 12 +++++- 3 files changed, 84 insertions(+), 6 deletions(-) diff --git a/app/test/test_mbuf.c b/app/test/test_mbuf.c index 0208c31196..da9d1d9323 100644 --- a/app/test/test_mbuf.c +++ b/app/test/test_mbuf.c @@ -2569,6 +2569,70 @@ test_mbuf_dyn(struct rte_mempool *pktmbuf_pool) return -1; } +/* check that m->nb_segs and m->next are reset on mbuf free */ +static int +test_nb_segs_and_next_reset(void) +{ + struct rte_mbuf *m0 = NULL, *m1 = NULL, *m2 = NULL; + struct rte_mempool *pool = NULL; + + pool = rte_pktmbuf_pool_create("test_mbuf_reset", + 3, 0, 0, MBUF_DATA_SIZE, SOCKET_ID_ANY); + if (pool == NULL) + GOTO_FAIL("Failed to create mbuf pool"); + + /* alloc mbufs */ + m0 = rte_pktmbuf_alloc(pool); + m1 = rte_pktmbuf_alloc(pool); + m2 = rte_pktmbuf_alloc(pool); + if (m0 == NULL || m1 == NULL || m2 == NULL) + GOTO_FAIL("Failed to allocate mbuf"); + + /* append data in all of them */ + if (rte_pktmbuf_append(m0, 500) == NULL || + rte_pktmbuf_append(m1, 500) == NULL || + rte_pktmbuf_append(m2, 500) == NULL) + GOTO_FAIL("Failed to append data in mbuf"); + + /* chain them in one mbuf m0 */ + rte_pktmbuf_chain(m1, m2); + rte_pktmbuf_chain(m0, m1); + if (m0->nb_segs != 3 || m0->next != m1 || m1->next != m2 || + m2->next != NULL) { + m1 = m2 = NULL; + GOTO_FAIL("Failed to chain mbufs"); + } + + /* split m0 chain in two, between m1 and m2 */ + m0->nb_segs = 2; + m1->next = NULL; + m2->nb_segs = 1; + + /* free the 2 mbuf chains m0 and m2 */ + rte_pktmbuf_free(m0); + rte_pktmbuf_free(m2); + + /* realloc the 3 mbufs */ + m0 = rte_mbuf_raw_alloc(pool); + m1 = rte_mbuf_raw_alloc(pool); + m2 = rte_mbuf_raw_alloc(pool); + if (m0 == NULL || m1 == NULL || m2 == NULL) + GOTO_FAIL("Failed to reallocate mbuf"); + + /* ensure that m->next and m->nb_segs are reset allocated mbufs */ + if (m0->nb_segs != 1 || m0->next != NULL || + m1->nb_segs != 1 || m1->next != NULL || + m2->nb_segs != 1 || m2->next != NULL) + GOTO_FAIL("nb_segs or next was not reset properly"); + + return 0; + +fail: + if (pool != NULL) + rte_mempool_free(pool); + return -1; +} + static int test_mbuf(void) { @@ -2753,6 +2817,12 @@ test_mbuf(void) goto err; } + /* test reset of m->nb_segs and m->next on mbuf free */ + if (test_nb_segs_and_next_reset() < 0) { + printf("test_nb_segs_and_next_reset() failed\n"); + goto err; + } + ret = 0; err: rte_mempool_free(pktmbuf_pool); diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h index 55205b8ece..66ae794eda 100644 --- a/lib/librte_mbuf/rte_mbuf.h +++ b/lib/librte_mbuf/rte_mbuf.h @@ -1183,10 +1183,10 @@ rte_pktmbuf_prefree_seg(struct rte_mbuf *m) if (!RTE_MBUF_DIRECT(m)) rte_pktmbuf_detach(m); - if (m->next != NULL) { + if (m->next != NULL) m->next = NULL; + if (m->nb_segs != 1) m->nb_segs = 1; - } return m; @@ -1195,10 +1195,10 @@ rte_pktmbuf_prefree_seg(struct rte_mbuf *m) if (!RTE_MBUF_DIRECT(m)) rte_pktmbuf_detach(m); - if (m->next != NULL) { + if (m->next != NULL) m->next = NULL; + if (m->nb_segs != 1) m->nb_segs = 1; - } rte_mbuf_refcnt_set(m, 1); return m; diff --git a/lib/librte_mbuf/rte_mbuf_core.h b/lib/librte_mbuf/rte_mbuf_core.h index b97b811366..deb876fea6 100644 --- a/lib/librte_mbuf/rte_mbuf_core.h +++ b/lib/librte_mbuf/rte_mbuf_core.h @@ -514,7 +514,11 @@ struct rte_mbuf { /** Non-atomically accessed refcnt */ uint16_t refcnt; }; - uint16_t nb_segs; /**< Number of segments. */ + /** + * Number of segments. Only valid for the first segment of an mbuf + * chain. + */ + uint16_t nb_segs; /** Input port (16 bits to support more than 256 virtual ports). * The event eth Tx adapter uses this field to specify the output port. @@ -621,7 +625,11 @@ struct rte_mbuf { }; struct rte_mempool *pool; /**< Pool from which mbuf was allocated. */ - struct rte_mbuf *next; /**< Next segment of scattered packet. */ + /** + * Next segment of scattered packet. Must be NULL in the last segment or + * in case of non-segmented packet. + */ + struct rte_mbuf *next; /* fields to support TX offloads */ RTE_STD_C11 -- 2.30.2