From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id B3A9BA0540; Thu, 16 Jul 2020 11:16:01 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 944511BF70; Thu, 16 Jul 2020 11:16:01 +0200 (CEST) Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by dpdk.org (Postfix) with ESMTP id D97971BF6C for ; Thu, 16 Jul 2020 11:15:59 +0200 (CEST) IronPort-SDR: m0svxQ7aRIPg/pnpgi0L72OPTCccQXTmUYFbNff3TdPbFd95btUfYNGp9HizIimsri24ShOGQm uCwHugxhYYdg== X-IronPort-AV: E=McAfee;i="6000,8403,9683"; a="136796692" X-IronPort-AV: E=Sophos;i="5.75,358,1589266800"; d="scan'208";a="136796692" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga006.jf.intel.com ([10.7.209.51]) by orsmga106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 16 Jul 2020 02:15:58 -0700 IronPort-SDR: XDXbPjx3bSj9em3hJvqnkr6HcPntdSRI/Sj9e/T4GHulgiF2pnoDKBKGauVxTc3XFxUVqBMJ+H ttoxw5OZl7VQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.75,358,1589266800"; d="scan'208";a="286417240" Received: from orsmsx601.amr.corp.intel.com ([10.22.229.14]) by orsmga006.jf.intel.com with ESMTP; 16 Jul 2020 02:15:58 -0700 Received: from orsmsx608.amr.corp.intel.com (10.22.229.21) by ORSMSX601.amr.corp.intel.com (10.22.229.14) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Thu, 16 Jul 2020 02:15:58 -0700 Received: from ORSEDG002.ED.cps.intel.com (10.7.248.5) by orsmsx608.amr.corp.intel.com (10.22.229.21) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256) id 15.1.1713.5 via Frontend Transport; Thu, 16 Jul 2020 02:15:58 -0700 Received: from NAM02-SN1-obe.outbound.protection.outlook.com (104.47.36.57) by edgegateway.intel.com (134.134.137.101) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 16 Jul 2020 02:15:58 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=ibvqMkavtECBaVeasvl5wd0I+0O4hGqHBbC4yyhVl6yzGUsX9F1hmeqUpGEkf5CGLxJUoNl6IYi9Juvr6Ox0aSMDYhDeSpl8NmsHgM+PcWM8Q60Qp+YZTYAjSsaviOOD+ObbEpqBd6EdLcOxexFxTqZ/+wxrib82p/LtmAPnfbHRy2mKGMtCDYX652k/e643r1Kk5p6EDgbLYFpkgVKudUTx+IOprhT1VmZr0uv1I90vN/txkILLOWazBWVFxoOng0XwQvU5ryOsNIZ5DnqhM1f1nhpxC7BdIaX3/Bui5hDPZhzjGmfQxuxHM5q32TOw4LDMTVzDDIPthoJdmC/dqg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=N6UoaOTgqcbF+Vnmq/oZLiMP00iJz79H3lp3Xiws7hw=; b=EjBvelQO8nNog1zRMgcCOn2UCtUyNtRiHrhI0SVvL/Hf6Ei+iCWFJLUA/fl3I43soQw7sapwVS0kwJxn+6i7bi3gxvGEupq35niBMTd+E1C2y4Mgv+9OmevuPJd28nrFsIDQpYK1SK2piWR0d9Y0RAOUpQiCz/1bzYoF4qK8Lez3PeHT043kC7sn1XQ2p/J21TNciVrXYqk9hCJuLWmvsodFNg57HbsnzEiUymShl/+TL5Y0CyB8wESQ3eBAx9eqkO2CHOaLtyjOcK7XKUq9neIq/G/NIQplUTiqIZ47skwcOYg+1ci9GF3s50kfRqWuQijpUu8t6EefcfbPWTz9JA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel.onmicrosoft.com; s=selector2-intel-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=N6UoaOTgqcbF+Vnmq/oZLiMP00iJz79H3lp3Xiws7hw=; b=tLjbLLrdu97HTFqIsk9HgMdXGrWSpQm1W0FtDkKNtoThlmt7obeHHmfozfvqGuybEz3xld8s8GZy9qSzmUUFbMJrHUp1Lv/nNam9FYE/AB6JAsNofXuO1KiMYTrLqyMfTFJ9o5loe0iCwHW1nobSLIzGgqy7mrITPeak4eAUav0= Received: from MWHPR11MB1391.namprd11.prod.outlook.com (2603:10b6:300:23::15) by MWHPR11MB1920.namprd11.prod.outlook.com (2603:10b6:300:107::12) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3174.20; Thu, 16 Jul 2020 09:15:56 +0000 Received: from MWHPR11MB1391.namprd11.prod.outlook.com ([fe80::f8f5:b48c:be92:ac17]) by MWHPR11MB1391.namprd11.prod.outlook.com ([fe80::f8f5:b48c:be92:ac17%3]) with mapi id 15.20.3195.018; Thu, 16 Jul 2020 09:15:56 +0000 From: "Zhao1, Wei" To: "Guo, Jia" CC: "bmcfall@redhat.com" , "dev@dpdk.org" Thread-Topic: [dpdk-dev] net/e1000: fix segfault on tx done clean up Thread-Index: AQHWW07sjoBqnaI5+k+237AE6cf2c6kJ7KWA Date: Thu, 16 Jul 2020 09:15:56 +0000 Message-ID: References: <20200716085354.57211-1-jia.guo@intel.com> In-Reply-To: <20200716085354.57211-1-jia.guo@intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMWJkMGVlMTctOTc3NS00NGM2LTk4ZWUtZGU4OTJhYjJkNDE2IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiY0dKd1A2Z0M5Q0dwb2JTcnB6NzN4TVdTVmFsY1BvU1pXeFNVVVFTSWdXTWtFd1lcL2dRQkhIU2ZMRHo5b3hnY04ifQ== dlp-version: 11.0.600.7 dlp-product: dlpe-windows x-ctpclassification: CTP_NT dlp-reaction: no-action authentication-results: intel.com; dkim=none (message not signed) header.d=none;intel.com; dmarc=none action=none header.from=intel.com; x-originating-ip: [192.198.147.223] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: a1c3799d-9d52-45b7-9a41-08d82968d8d6 x-ms-traffictypediagnostic: MWHPR11MB1920: x-ld-processed: 46c98d88-e344-4ed4-8496-4ed7712e255d,ExtAddr x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:7219; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: wfbV9zmLbJTmdIwXUExt/2PYxtjNUXyg6pyWZ3JAl/ObdMs6bztfxokUci4Ed3xbmOAlILmblZvL7DTX1IMM3HkjOmKSAJ4BsmO/Nt3sAZaVQZs2LcaH4Y6KUGdeHWuIfOzhyFApcb0GUShEaktGGartR4VcI9RBraubKafkQ6SvEm3brBZJQ/GpNYX7EnX29bBDGZvXoT1oXfevTJvYiF78ttrUN7fghJ92+DbUws3s2VYRsrUX1JF8l+CUT4YCvM1dpggHyJO3GdF40qUDLjNS9o646jo7/yiIiE88Pfw/eX93kNhzvBFWDzobLkdS08SxKvw/A9nNs1Nm0RZPFw== x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:MWHPR11MB1391.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFTY:; SFS:(4636009)(366004)(376002)(136003)(346002)(39860400002)(396003)(26005)(8936002)(66476007)(8676002)(6636002)(66446008)(54906003)(76116006)(64756008)(66946007)(2906002)(316002)(86362001)(9686003)(4326008)(6862004)(53546011)(5660300002)(478600001)(55016002)(66556008)(83380400001)(7696005)(71200400001)(33656002)(52536014)(6506007)(186003); DIR:OUT; SFP:1102; x-ms-exchange-antispam-messagedata: QjWj+cUNpeMBoGoZLArgWbegJX8jnJ0vAKQ0Bicyo9hizMlH+YX7I/O9HRYsj7+lFh0Yk/r1LNIkNQhS/MQa+aNLLDxYDKqdUcqLox/wMLTJxLxZD9WqyEXo66M/bTBZw0buyk/69uDD6MP2Bl9t1Z4TAVEU5nzWTh+8AeokM2rRZWRJZhs4XjM0dCvp4Y7f1O20mJvUZtKh89QwiVAHcv+fzKt7PKgCUlNCIgOxdcb0X2REjW3GdgBXRd2MfAhTKoO517QTsliEnrqx4SCgYq/aV3TFqxoEqtGvumjmR7j3vUyOA0X3OX9qXWPFnCpK/QJgPkGrrUCqyKyb2mpK76JEKPoftVXt6Ru/L1S+Ew2qBJ2lcM6HvAtMaW+o4SW65o9lTtzLWZi3b4GTnuer/GF8E9/VHqMTSs2NpyK0zZj7Y62Wtrb1LDv8lP6Ov0jorBsU2ZmugLmXyXBZjNdwEuggLNsMpRYy4VWbV8Lp/wmtmqH9LO8UrgxHyjS/ZL0W Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: MWHPR11MB1391.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: a1c3799d-9d52-45b7-9a41-08d82968d8d6 X-MS-Exchange-CrossTenant-originalarrivaltime: 16 Jul 2020 09:15:56.2656 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: 3+jtWJ3D9BoYSSDdctk7XDUV1rA9P/MRjUTQ15F3wQvIpdtfDxJdRCId1GIlE5HsOW3rPSy7WdTGN6PAwNkPyw== X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR11MB1920 X-OriginatorOrg: intel.com Subject: Re: [dpdk-dev] net/e1000: fix segfault on tx done clean up 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" Reviewed-by: Wei Zhao > -----Original Message----- > From: Guo, Jia > Sent: Thursday, July 16, 2020 4:54 PM > To: Zhao1, Wei > Cc: bmcfall@redhat.com; dev@dpdk.org; Guo, Jia > Subject: [dpdk-dev] net/e1000: fix segfault on tx done clean up >=20 > As tx mbuf is not set for some advanced descriptors, if there is no mbuf > checking before rte_pktmbuf_free_seg() function be called on the process = of tx > done clean up, that will cause a segfault. So add a NULL pointer check to= fix it. >=20 > Bugzilla ID: 501 > Fixes: 8d907d2b79f7 (net/igb: free consumed Tx buffers on demand) >=20 > Signed-off-by: Jeff Guo > --- > drivers/net/e1000/igb_rxtx.c | 170 +++++++++++++++++------------------ > 1 file changed, 82 insertions(+), 88 deletions(-) >=20 > diff --git a/drivers/net/e1000/igb_rxtx.c b/drivers/net/e1000/igb_rxtx.c = index > 5717cdb70..115a723e4 100644 > --- a/drivers/net/e1000/igb_rxtx.c > +++ b/drivers/net/e1000/igb_rxtx.c > @@ -1295,113 +1295,107 @@ igb_tx_done_cleanup(struct igb_tx_queue *txq, > uint32_t free_cnt) > uint16_t tx_id; /* Current segment being processed. */ > uint16_t tx_last; /* Last segment in the current packet. */ > uint16_t tx_next; /* First segment of the next packet. */ > - int count; > + int count =3D 0; >=20 > - if (txq !=3D NULL) { > - count =3D 0; > - sw_ring =3D txq->sw_ring; > - txr =3D txq->tx_ring; > + if (!txq) > + return =3D -ENODEV; >=20 > - /* > - * tx_tail is the last sent packet on the sw_ring. Goto the end > - * of that packet (the last segment in the packet chain) and > - * then the next segment will be the start of the oldest segment > - * in the sw_ring. This is the first packet that will be > - * attempted to be freed. > - */ > + sw_ring =3D txq->sw_ring; > + txr =3D txq->tx_ring; >=20 > - /* Get last segment in most recently added packet. */ > - tx_first =3D sw_ring[txq->tx_tail].last_id; > + /* tx_tail is the last sent packet on the sw_ring. Goto the end > + * of that packet (the last segment in the packet chain) and > + * then the next segment will be the start of the oldest segment > + * in the sw_ring. This is the first packet that will be > + * attempted to be freed. > + */ >=20 > - /* Get the next segment, which is the oldest segment in ring. */ > - tx_first =3D sw_ring[tx_first].next_id; > + /* Get last segment in most recently added packet. */ > + tx_first =3D sw_ring[txq->tx_tail].last_id; >=20 > - /* Set the current index to the first. */ > - tx_id =3D tx_first; > + /* Get the next segment, which is the oldest segment in ring. */ > + tx_first =3D sw_ring[tx_first].next_id; >=20 > - /* > - * Loop through each packet. For each packet, verify that an > - * mbuf exists and that the last segment is free. If so, free > - * it and move on. > - */ > - while (1) { > - tx_last =3D sw_ring[tx_id].last_id; > - > - if (sw_ring[tx_last].mbuf) { > - if (txr[tx_last].wb.status & > - E1000_TXD_STAT_DD) { > - /* > - * Increment the number of packets > - * freed. > - */ > - count++; > - > - /* Get the start of the next packet. */ > - tx_next =3D sw_ring[tx_last].next_id; > - > - /* > - * Loop through all segments in a > - * packet. > - */ > - do { > - rte_pktmbuf_free_seg(sw_ring[tx_id].mbuf); > + /* Set the current index to the first. */ > + tx_id =3D tx_first; > + > + /* Loop through each packet. For each packet, verify that an > + * mbuf exists and that the last segment is free. If so, free > + * it and move on. > + */ > + while (1) { > + tx_last =3D sw_ring[tx_id].last_id; > + > + if (sw_ring[tx_last].mbuf) { > + if (txr[tx_last].wb.status & > + E1000_TXD_STAT_DD) { > + /* Increment the number of packets > + * freed. > + */ > + count++; > + > + /* Get the start of the next packet. */ > + tx_next =3D sw_ring[tx_last].next_id; > + > + /* Loop through all segments in a > + * packet. > + */ > + do { > + if (sw_ring[tx_id].mbuf) { > + rte_pktmbuf_free_seg( > + sw_ring[tx_id].mbuf); > sw_ring[tx_id].mbuf =3D NULL; > sw_ring[tx_id].last_id =3D tx_id; > + } >=20 > - /* Move to next segemnt. */ > - tx_id =3D sw_ring[tx_id].next_id; > + /* Move to next segemnt. */ > + tx_id =3D sw_ring[tx_id].next_id; >=20 > - } while (tx_id !=3D tx_next); > + } while (tx_id !=3D tx_next); >=20 > - if (unlikely(count =3D=3D (int)free_cnt)) > - break; > - } else > - /* > - * mbuf still in use, nothing left to > - * free. > - */ > + if (unlikely(count =3D=3D (int)free_cnt)) > break; > } else { > - /* > - * There are multiple reasons to be here: > - * 1) All the packets on the ring have been > - * freed - tx_id is equal to tx_first > - * and some packets have been freed. > - * - Done, exit > - * 2) Interfaces has not sent a rings worth of > - * packets yet, so the segment after tail is > - * still empty. Or a previous call to this > - * function freed some of the segments but > - * not all so there is a hole in the list. > - * Hopefully this is a rare case. > - * - Walk the list and find the next mbuf. If > - * there isn't one, then done. > + /* mbuf still in use, nothing left to > + * free. > */ > - if (likely((tx_id =3D=3D tx_first) && (count !=3D 0))) > - break; > + break; > + } > + } else { > + /* There are multiple reasons to be here: > + * 1) All the packets on the ring have been > + * freed - tx_id is equal to tx_first > + * and some packets have been freed. > + * - Done, exit > + * 2) Interfaces has not sent a rings worth of > + * packets yet, so the segment after tail is > + * still empty. Or a previous call to this > + * function freed some of the segments but > + * not all so there is a hole in the list. > + * Hopefully this is a rare case. > + * - Walk the list and find the next mbuf. If > + * there isn't one, then done. > + */ > + if (likely(tx_id =3D=3D tx_first && count !=3D 0)) > + break; >=20 > - /* > - * Walk the list and find the next mbuf, if any. > - */ > - do { > - /* Move to next segemnt. */ > - tx_id =3D sw_ring[tx_id].next_id; > + /* Walk the list and find the next mbuf, if any. */ > + do { > + /* Move to next segemnt. */ > + tx_id =3D sw_ring[tx_id].next_id; >=20 > - if (sw_ring[tx_id].mbuf) > - break; > + if (sw_ring[tx_id].mbuf) > + break; >=20 > - } while (tx_id !=3D tx_first); > + } while (tx_id !=3D tx_first); >=20 > - /* > - * Determine why previous loop bailed. If there > - * is not an mbuf, done. > - */ > - if (sw_ring[tx_id].mbuf =3D=3D NULL) > - break; > - } > + /* Determine why previous loop bailed. If there > + * is not an mbuf, done. > + */ > + if (!sw_ring[tx_id].mbuf) > + break; > } > - } else > - count =3D -ENODEV; > + } >=20 > return count; > } > -- > 2.20.1