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 DB061A04B7 for ; Tue, 13 Oct 2020 16:25:23 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id CC8001DD1A; Tue, 13 Oct 2020 16:25:22 +0200 (CEST) Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id B4BD71DD1A; Tue, 13 Oct 2020 16:25:19 +0200 (CEST) IronPort-SDR: xyf70tqAZtbBbc5TqiWUjYTT66b0jiGXtdH7oaWJdd/n5xmRS4SqFGFNXK5wu5uVs6lTN7b/Et 0dX4PIKSp78w== X-IronPort-AV: E=McAfee;i="6000,8403,9772"; a="163279421" X-IronPort-AV: E=Sophos;i="5.77,371,1596524400"; d="scan'208";a="163279421" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 13 Oct 2020 07:25:15 -0700 IronPort-SDR: UzkKl3+zHWvEZvRp3877OMeNZ1GqRNMqrRJDw1oOK7pH+IOiSziHKfxOM7hOvcTvJyLQq5sX+N WqqwTqDdSlQQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.77,371,1596524400"; d="scan'208";a="313815084" Received: from fmsmsx604.amr.corp.intel.com ([10.18.126.84]) by orsmga003.jf.intel.com with ESMTP; 13 Oct 2020 07:25:15 -0700 Received: from fmsmsx607.amr.corp.intel.com (10.18.126.87) by fmsmsx604.amr.corp.intel.com (10.18.126.84) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Tue, 13 Oct 2020 07:25:14 -0700 Received: from fmsmsx603.amr.corp.intel.com (10.18.126.83) by fmsmsx607.amr.corp.intel.com (10.18.126.87) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Tue, 13 Oct 2020 07:25:14 -0700 Received: from fmsedg601.ED.cps.intel.com (10.1.192.135) by fmsmsx603.amr.corp.intel.com (10.18.126.83) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5 via Frontend Transport; Tue, 13 Oct 2020 07:25:14 -0700 Received: from NAM04-SN1-obe.outbound.protection.outlook.com (104.47.44.52) by edgegateway.intel.com (192.55.55.70) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.1713.5; Tue, 13 Oct 2020 07:25:13 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=WhLenZWtry57EVVPQbXPbBkE4fWoH4n2iekoVldmJ0PH9pIzHLfwa3Nt0cT6oubsoIOMzmitChp4KauVgmaMFjOr7ipfP2NbxUjI7kliSKZ8Lfp5b3s1DchNXTitlt3/+jZrH53zFEv5QdmcTt2P0GUMpSWJ6bkLawSEeNymkipJy9YIaUj0WgFamqMZ8FXvfejGI7GsL0SjA9kUFeqSrttinRPOjLEhb+XSMl01nmR3ZqP3MB/nU4pYjC8xTEZkstl4Uq3EbfftZAGqXmcs8tK7hgEE/BD7qp4ETYtvzsi+0pxPkOE0t0JH7+eNSfkDwZ2gxnRCTzpuvQZH74hMAw== 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=K36Oi7MXlvnD/zrzIlOu/Y0nwL4geBFvgHVD6qF9LTg=; b=Y9fHD3aShlSAln+oOtQjo2yH1PwdsyO00+34aBo8n8hx3tz77cYYuLlJCwZswfgHJVEfkKeA1rCYeOIxctXdG+ziDoDmA14HYtMOC7sqPypyzYuywTUsufXstnDJa4oz2xvhYeoq0aOoIOXcc8YQCz5AQe+P/7124EgaaRppn1+KTT0c7a7X20dXsjf07VqJJ/lVGdTX1yCX89mbtcKtIDEqzFNXlC7hfX/2wjGbOGBH5BKlb4iPEWP+kCkoZefOi9V3R5d1iyaOPldJ3OocuyXCWMeHWdx2A/72+0usCCgND2O1vO1Fq25uIuA9ZTg8rvoxscTxZ4/YUPM97DnH1w== 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=K36Oi7MXlvnD/zrzIlOu/Y0nwL4geBFvgHVD6qF9LTg=; b=d8FREaH8tSn+pzElFH9qKfzMSNxxF7wdRhJDnKqgwCWevfIHsPBmV25JYPCCjqPTKNKBzxPtIGEmJ4YoDc1PE7pSO6iU5BuOqkgCOlrY/0Qfjd2vKkYR4Abp7knCivYaBSrUAvQ1rbwY/KV6lkLdNzliAHBwAJGdKAgcpCYQ/pg= Received: from BYAPR11MB3301.namprd11.prod.outlook.com (2603:10b6:a03:7f::26) by SJ0PR11MB5085.namprd11.prod.outlook.com (2603:10b6:a03:2db::10) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3455.30; Tue, 13 Oct 2020 14:25:10 +0000 Received: from BYAPR11MB3301.namprd11.prod.outlook.com ([fe80::f5a4:3f6b:ade3:296b]) by BYAPR11MB3301.namprd11.prod.outlook.com ([fe80::f5a4:3f6b:ade3:296b%3]) with mapi id 15.20.3455.030; Tue, 13 Oct 2020 14:25:10 +0000 From: "Ananyev, Konstantin" To: Honnappa Nagarahalli , "dev@dpdk.org" , "phil.yang@arm.com" , "thomas@monjalon.net" , "arybchenko@solarflare.com" , "Yigit, Ferruh" CC: "Gujjar, Abhinandan S" , "nd@arm.com" , "Richardson, Bruce" , "Mcnamara, John" , "Pattan, Reshma" , "stable@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH 2/2] lib/ethdev: fix memory ordering for call back functions Thread-Index: AQHWmFAxkcpvfovwNEGjVyIvkAneQqmVqJug Date: Tue, 13 Oct 2020 14:25:10 +0000 Message-ID: References: <20201002000711.41511-1-honnappa.nagarahalli@arm.com> <20201002000711.41511-2-honnappa.nagarahalli@arm.com> In-Reply-To: <20201002000711.41511-2-honnappa.nagarahalli@arm.com> Accept-Language: en-GB, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-reaction: no-action dlp-version: 11.5.1.3 authentication-results: arm.com; dkim=none (message not signed) header.d=none;arm.com; dmarc=none action=none header.from=intel.com; x-originating-ip: [46.7.39.127] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: f3cbe346-bb5c-4d59-69d4-08d86f83cae7 x-ms-traffictypediagnostic: SJ0PR11MB5085: 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:9508; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: w5/vU77qQSHa+MsAB8pHIq+YvdR3WxI3ThQqMcs9blOk9xHke7gNMAIRyQYite+gmaBeFzpzdRwHJM1x6DahtUvxg+UvGby7emAphujXQAv+WLvu3WUCS7mt8OkveJFHDjYi8nztllIDv+6HhDEcjJbqTOEGZk3TJcyheR2dW64RfLweIzOgdmJSC4qi+r3DiDA4rt4ylfup20ZSC6s+jJ1YL3bLNrrmjTUPGcqQcE/zX4H6GNo9AXoOcifPUAo05aebw0twgECUQCpIIodKIXBL3jrWKmpttUsjsvxacJv37sGdSFxg4zqewgK2yRj8Ig341Foof6JWTcZko+F6jA== x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:BYAPR11MB3301.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFS:(4636009)(136003)(376002)(346002)(39860400002)(366004)(396003)(83380400001)(52536014)(55016002)(5660300002)(26005)(316002)(2906002)(110136005)(186003)(54906003)(6636002)(8936002)(6506007)(66446008)(66476007)(64756008)(9686003)(33656002)(71200400001)(4326008)(478600001)(86362001)(66946007)(66556008)(7696005)(76116006); DIR:OUT; SFP:1102; x-ms-exchange-antispam-messagedata: 6rHy0WeVu5OEbInGIyFMBnvIXFHmoewqLTzYOdHelP42FVHPF7R1qU74LMyomNK+ycOaj/GdiR2avsw9FmzYKQotN/eyV5qkoJoHkZoW3sD6KjascS6CfF4vXR7ynQ+/V7lDqBH//c5nI5aVJUqz93sb1kfbtUk67kVWia9FJpzTfk4LWSVahxLL4tWtaFfQxP2MxxRzZcTRWV1xF5AnLyMrFTEpbBt9aIO9X+8eEQS6mhbMlbODugqMbbMIhC2BSAh6Gy5QCQdE+QaUgvhSxtO86Kyf8CZOm4NenxNJY79cmfSjWRvEvX8xXxUs18F0Zhgs6uSQ/C/jYf3n5SerHDhCfgl8EwshqEY1b6BU/fnYndNuKL3Zu2oJwUxgDQlyyViB2ftrP0lIoPCSO9XZPW4Ee8Gcnstu4t3AmHj9uT+GSQmWAbYIPtXrHjb2qO6GI2MTj0U6ZfTH9o/tkrpvNKaSbxDA3U4ovQXLlN+uFYrpq4Qnmfex72jrIsPsUSv8mIbBDPYtqoYCxFMT8e2HkwZMamPoYUdDvQYM/LwvkNC6bgZL3mKsqfPpjT4p6kiVNzRYfCxy2yjtBASh0mEcMQSN3Qd/AIAWo/KxsL5lCYNlR23cKjrmcVPW5d8WiMiXEQwHW41VvFUl0eiMQIdLrg== 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: BYAPR11MB3301.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: f3cbe346-bb5c-4d59-69d4-08d86f83cae7 X-MS-Exchange-CrossTenant-originalarrivaltime: 13 Oct 2020 14:25:10.5299 (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: y5qjNFpRdbZTNdyuxC7Em0+u9DORbA+W2+GmLHj2Eqq11ICMIyZXRDK6BGsZOtOqb5aualVEj1w6MxOKVXD9NqMVqULmNPuNaqP2WXmDoIY= X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR11MB5085 X-OriginatorOrg: intel.com Subject: Re: [dpdk-stable] [dpdk-dev] [PATCH 2/2] lib/ethdev: fix memory ordering for call back functions X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 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 Sender: "stable" >=20 > Call back functions are registered on the control plane. They > are accessed from the data plane. Hence, correct memory orderings > should be used to avoid race conditions. >=20 > Fixes: 4dc294158cac ("ethdev: support optional Rx and Tx callbacks") > Fixes: c8231c63ddcb ("ethdev: insert Rx callback as head of list") > Cc: bruce.richardson@intel.com > Cc: john.mcnamara@intel.com > Cc: reshma.pattan@intel.com > Cc: stable@dpdk.org >=20 > Signed-off-by: Honnappa Nagarahalli > Reviewed-by: Ola Liljedahl > --- > lib/librte_ethdev/rte_ethdev.c | 28 +++++++++++++++++++++------ > lib/librte_ethdev/rte_ethdev.h | 35 ++++++++++++++++++++++++++-------- > 2 files changed, 49 insertions(+), 14 deletions(-) >=20 > diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethde= v.c > index 59a41c07f..d89fcdc77 100644 > --- a/lib/librte_ethdev/rte_ethdev.c > +++ b/lib/librte_ethdev/rte_ethdev.c > @@ -4486,12 +4486,20 @@ rte_eth_add_rx_callback(uint16_t port_id, uint16_= t queue_id, > rte_eth_devices[port_id].post_rx_burst_cbs[queue_id]; >=20 > if (!tail) { > - rte_eth_devices[port_id].post_rx_burst_cbs[queue_id] =3D cb; > + /* Stores to cb->fn and cb->param should complete before > + * cb is visible to data plane. > + */ > + __atomic_store_n( > + &rte_eth_devices[port_id].post_rx_burst_cbs[queue_id], > + cb, __ATOMIC_RELEASE); >=20 > } else { > while (tail->next) > tail =3D tail->next; > - tail->next =3D cb; > + /* Stores to cb->fn and cb->param should complete before > + * cb is visible to data plane. > + */ > + __atomic_store_n(&tail->next, cb, __ATOMIC_RELEASE); > } > rte_spinlock_unlock(&rte_eth_rx_cb_lock); >=20 > @@ -4576,12 +4584,20 @@ rte_eth_add_tx_callback(uint16_t port_id, uint16_= t queue_id, > rte_eth_devices[port_id].pre_tx_burst_cbs[queue_id]; >=20 > if (!tail) { > - rte_eth_devices[port_id].pre_tx_burst_cbs[queue_id] =3D cb; > + /* Stores to cb->fn and cb->param should complete before > + * cb is visible to data plane. > + */ > + __atomic_store_n( > + &rte_eth_devices[port_id].pre_tx_burst_cbs[queue_id], > + cb, __ATOMIC_RELEASE); >=20 > } else { > while (tail->next) > tail =3D tail->next; > - tail->next =3D cb; > + /* Stores to cb->fn and cb->param should complete before > + * cb is visible to data plane. > + */ > + __atomic_store_n(&tail->next, cb, __ATOMIC_RELEASE); > } > rte_spinlock_unlock(&rte_eth_tx_cb_lock); >=20 > @@ -4612,7 +4628,7 @@ rte_eth_remove_rx_callback(uint16_t port_id, uint16= _t queue_id, > cb =3D *prev_cb; > if (cb =3D=3D user_cb) { > /* Remove the user cb from the callback list. */ > - *prev_cb =3D cb->next; > + __atomic_store_n(prev_cb, cb->next, __ATOMIC_RELAXED); > ret =3D 0; > break; > } > @@ -4646,7 +4662,7 @@ rte_eth_remove_tx_callback(uint16_t port_id, uint16= _t queue_id, > cb =3D *prev_cb; > if (cb =3D=3D user_cb) { > /* Remove the user cb from the callback list. */ > - *prev_cb =3D cb->next; > + __atomic_store_n(prev_cb, cb->next, __ATOMIC_RELAXED); > ret =3D 0; > break; > } > diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethde= v.h > index 70295d7ab..d810e3e38 100644 > --- a/lib/librte_ethdev/rte_ethdev.h > +++ b/lib/librte_ethdev/rte_ethdev.h > @@ -3734,7 +3734,8 @@ struct rte_eth_rxtx_callback; > * The callback function > * @param user_param > * A generic pointer parameter which will be passed to each invocation= of the > - * callback function on this port and queue. > + * callback function on this port and queue. Inter-thread synchronizat= ion > + * of any user data changes is the responsibility of the user. > * > * @return > * NULL on error. > @@ -3763,7 +3764,8 @@ rte_eth_add_rx_callback(uint16_t port_id, uint16_t = queue_id, > * The callback function > * @param user_param > * A generic pointer parameter which will be passed to each invocation= of the > - * callback function on this port and queue. > + * callback function on this port and queue. Inter-thread synchronizat= ion > + * of any user data changes is the responsibility of the user. > * > * @return > * NULL on error. > @@ -3791,7 +3793,8 @@ rte_eth_add_first_rx_callback(uint16_t port_id, uin= t16_t queue_id, > * The callback function > * @param user_param > * A generic pointer parameter which will be passed to each invocation= of the > - * callback function on this port and queue. > + * callback function on this port and queue. Inter-thread synchronizat= ion > + * of any user data changes is the responsibility of the user. > * > * @return > * NULL on error. > @@ -3816,7 +3819,9 @@ rte_eth_add_tx_callback(uint16_t port_id, uint16_t = queue_id, > * on that queue. > * > * - After a short delay - where the delay is sufficient to allow any > - * in-flight callbacks to complete. > + * in-flight callbacks to complete. Alternately, the RCU mechanism can= be > + * used to detect when data plane threads have ceased referencing the > + * callback memory. > * > * @param port_id > * The port identifier of the Ethernet device. > @@ -3849,7 +3854,9 @@ int rte_eth_remove_rx_callback(uint16_t port_id, ui= nt16_t queue_id, > * on that queue. > * > * - After a short delay - where the delay is sufficient to allow any > - * in-flight callbacks to complete. > + * in-flight callbacks to complete. Alternately, the RCU mechanism can= be > + * used to detect when data plane threads have ceased referencing the > + * callback memory. > * > * @param port_id > * The port identifier of the Ethernet device. > @@ -4510,10 +4517,16 @@ rte_eth_rx_burst(uint16_t port_id, uint16_t queue= _id, > rx_pkts, nb_pkts); >=20 > #ifdef RTE_ETHDEV_RXTX_CALLBACKS > - if (unlikely(dev->post_rx_burst_cbs[queue_id] !=3D NULL)) { > - struct rte_eth_rxtx_callback *cb =3D > - dev->post_rx_burst_cbs[queue_id]; > + /* __ATOMIC_RELEASE memory order was used when the > + * call back was inserted into the list. > + * Since there is a clear dependency between loading > + * cb and cb->fn/cb->next, __ATOMIC_ACQUIRE memory order is > + * not required. > + */ > + struct rte_eth_rxtx_callback *cb =3D > + dev->post_rx_burst_cbs[queue_id]; >=20 > + if (unlikely(cb !=3D NULL)) { > do { > nb_rx =3D cb->fn.rx(port_id, queue_id, rx_pkts, nb_rx, > nb_pkts, cb->param); > @@ -4775,6 +4788,12 @@ rte_eth_tx_burst(uint16_t port_id, uint16_t queue_= id, > #endif >=20 > #ifdef RTE_ETHDEV_RXTX_CALLBACKS > + /* __ATOMIC_RELEASE memory order was used when the > + * call back was inserted into the list. > + * Since there is a clear dependency between loading > + * cb and cb->fn/cb->next, __ATOMIC_ACQUIRE memory order is > + * not required. > + */ > struct rte_eth_rxtx_callback *cb =3D dev->pre_tx_burst_cbs[queue_id]; >=20 > if (unlikely(cb !=3D NULL)) { > -- Acked-by: Konstantin Ananyev > 2.17.1