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 D2AB6A0350; Tue, 1 Mar 2022 18:48:26 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 691F540DF6; Tue, 1 Mar 2022 18:48:26 +0100 (CET) Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by mails.dpdk.org (Postfix) with ESMTP id A9FA840040 for ; Tue, 1 Mar 2022 18:48:24 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1646156904; x=1677692904; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-transfer-encoding:mime-version; bh=lGIXHv/phCUxw+FS224qAM/BEhnU77qMTQ1URqkrQNE=; b=Pz9EwRj/DjxlLFttqZ/mS34mXJsvqJTDPVLDcUuCpXxrNjagxW2qOHsV 1hyIooztfO1Vx6d5PAILK8CeTFlaYoGNBbqMeTD+3JXrFcKY5YfpotwdO LT1V/arw+QHfS9iHjnbfZXxLMHgtAIMSnCpPwy+UakDVsR7iIAbRB67bn 56Rg8oOeijbpPsUg3GLNR1RM3GkkfYS4Nu2tyNjAawunmi/HiS6FkTvxj FETIC3qWnVAEnFP15sroARaRW27WOfr7VM3wAHtYqvQQuxhCVPGuIs92+ x+g+ZTGQJ+zTgzX9AfPt9KQiWuEqJyqQRzsBNcG0W0fZosDdZQRVSnx4V Q==; X-IronPort-AV: E=McAfee;i="6200,9189,10273"; a="252026354" X-IronPort-AV: E=Sophos;i="5.90,146,1643702400"; d="scan'208";a="252026354" Received: from orsmga008.jf.intel.com ([10.7.209.65]) by fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 01 Mar 2022 09:48:23 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.90,146,1643702400"; d="scan'208";a="550825693" Received: from fmsmsx605.amr.corp.intel.com ([10.18.126.85]) by orsmga008.jf.intel.com with ESMTP; 01 Mar 2022 09:48:23 -0800 Received: from fmsmsx612.amr.corp.intel.com (10.18.126.92) by fmsmsx605.amr.corp.intel.com (10.18.126.85) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2308.21; Tue, 1 Mar 2022 09:48:22 -0800 Received: from fmsmsx609.amr.corp.intel.com (10.18.126.89) by fmsmsx612.amr.corp.intel.com (10.18.126.92) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2308.21; Tue, 1 Mar 2022 09:48:22 -0800 Received: from fmsedg602.ED.cps.intel.com (10.1.192.136) by fmsmsx609.amr.corp.intel.com (10.18.126.89) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2308.21 via Frontend Transport; Tue, 1 Mar 2022 09:48:22 -0800 Received: from NAM04-BN8-obe.outbound.protection.outlook.com (104.47.74.40) by edgegateway.intel.com (192.55.55.71) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2308.20; Tue, 1 Mar 2022 09:48:21 -0800 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=T3aadeWlBjIxXqvt+54Avp2rOpV5peM7uaWwYfQD6Ozqqcic0oIjpUQjWEY0A68W5Hvi9PfdKAesMIFUk9WVDKai6mIOLtHnJsxT5nq3SUU69gbk2npHxQ2T8vBfdBwbtcr/4wV+5DQ2aumJZeGVFGQH/zdZMJZJL4P5zzp0wkQ0swPIKzdkyU+rCEDOMZijCsHCDaiyWkXrtzpD/cUGzrH2muDX4LaqQnF30LYe/DSiLvk27ZXVnGbTpLNk45/G6mTmtak/Yhs+4nF21c+Ztny10uJIVtyNsLohYarpUv/7I8f5MIeIVzJYNvufckGc0Oc/gVGtGIvE1g6PO/V9tg== 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-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=98vDdJxTCzuKv12nvtSMNC3TLImEykA1jNETmtsjjWg=; b=nTt3Xn185VfeDiWkTrZc0tWgCBUnhpNg2FDBnkeAqWzN0vbVLP5m6syudi3JQjpRwqyQ+Yd/+FRAGGmlJef+yRHWNSxkcJ/nsf9Y4Bpk5KS/U51hBexQh6ScZytqTu3jJ6o+iGM0g+ION1UCDiGp7mC1t0+p2IxQDYgnYvCzVQGZk6K6COvyr6QMYsV2DrZnhIV60gv88xZzBj0tU1XYcVPUTDPQQVgKGUWkYW6EW/Jvn5xwT0RBILaAGky7a7BEs97HBgUl676w+/hYhSACxJIEXu0uyKKR6dsS+CejRXjzMjzACVFOsQyUVdpzdwcLVtHbuTDpy2FiY2/qAKyV7Q== 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 Received: from DM8PR11MB5670.namprd11.prod.outlook.com (2603:10b6:8:37::12) by SJ0PR11MB4957.namprd11.prod.outlook.com (2603:10b6:a03:2df::21) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5017.22; Tue, 1 Mar 2022 17:48:19 +0000 Received: from DM8PR11MB5670.namprd11.prod.outlook.com ([fe80::69dc:2fe0:11fb:f157]) by DM8PR11MB5670.namprd11.prod.outlook.com ([fe80::69dc:2fe0:11fb:f157%4]) with mapi id 15.20.5017.027; Tue, 1 Mar 2022 17:48:19 +0000 From: "Dumitrescu, Cristian" To: "skori@marvell.com" , Thomas Monjalon , "Yigit, Ferruh" , "Andrew Rybchenko" , Ray Kinsella CC: "dev@dpdk.org" , Jerin Jacob Subject: RE: [PATCH v3 1/1] ethdev: mtr: support input color selection Thread-Topic: [PATCH v3 1/1] ethdev: mtr: support input color selection Thread-Index: AQHYLUqRS4Gue5XPLEydpbjLv3rNVayqvwWQ Date: Tue, 1 Mar 2022 17:48:19 +0000 Message-ID: References: <20220214120246.4181470-1-jerinj@marvell.com> <20220301085824.1041009-1-skori@marvell.com> In-Reply-To: <20220301085824.1041009-1-skori@marvell.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-version: 11.6.401.20 dlp-reaction: no-action dlp-product: dlpe-windows authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=intel.com; x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 6b14a341-3f6f-4426-a6ae-08d9fbabac08 x-ms-traffictypediagnostic: SJ0PR11MB4957:EE_ x-microsoft-antispam-prvs: x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: lxvBUuNRcbXNnUAnHXjbDRiZmKqVRpn1X4B4W9dEaTszcbnVvEnTM04oNdRzTpoqUXIMwIrwdAxzkkOgSXi9gbSODRw4AIqE9ArXpR4IktFnO39NJnSaqFSh7qHLPDMthNIhvm5WE8ENy5XWLHJqJnJnv4aTHgBdERcceLqvCIwBfMSip2oME+mOd7dPLnZ5efp17OFmfSSmq24WTATIruRgH2FEJ5RkVejyJnJLOXPO2Uk3BPP14WCTP/bQKslSRiDfKxjAd6SsI/lgXNv1w2yp4Di0u3npLyM/OVEnkRwmgnsjHe1ZmnF5Bw6WNVTP3/BOgMkAA/tbdPMzLT4t87xqSjE0YovpLN0BquY5RNBMOK/onrxM3TNp5ksaYtBMI+nyjZkNiyHiGNIlYAspIm6AHiRXQNKyyDTzrpirmNm24dhtEjCy7ao9XrrpAJb4yCcrh0MQ/NkW6kNIyLBhOzVqdzMb1XkhEh1CTxMZ++pJeucEU0xFHyVf5gn/0AmfdZdDCB3UPJved/HtTzWtHtw6Eib1oJ8AUOU4xT1yWuSWvHpEj6xz1r73QzIUMHfA2B8Y1tOtSxLluS2pCGRB2bAAEtuuh3buGdWrUeL4zsIxQjgY86B6wirzvRMj7R/98li7i3t0I5nNKwsw9QQzQRYWUT/M+j5i2PRy8A0IgNLp3iunu4oaZ9UcK6o1isaQodojs7renO5T/s9pNOYOJg== x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:DM8PR11MB5670.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230001)(366004)(2906002)(66446008)(66946007)(4326008)(110136005)(8676002)(76116006)(508600001)(64756008)(66556008)(66476007)(8936002)(71200400001)(55016003)(5660300002)(52536014)(9686003)(54906003)(316002)(6506007)(7696005)(122000001)(33656002)(38100700002)(186003)(83380400001)(82960400001)(26005)(86362001)(38070700005); DIR:OUT; SFP:1102; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?FPF75BRKYsI6wDq6jaCOw3CLkMh64ei7DJ7gUvY/aOdvKWxY+PCXVLJzjVKz?= =?us-ascii?Q?GxBP6YjwSFn5v0FMvICpiuye20ik2DSQns8YdznVBzRwEAcMzAa6vm/DTMnP?= =?us-ascii?Q?OlNl8iVp8Y0hZb1V/4XUE5YexVfc2mq7zsf6mBkWMV9KLD4FPmnsWF04en63?= =?us-ascii?Q?RmwqDvINx5Vo7OARpnKgY6cUhXq9dNYR2jb/iZDFRdggRwLHbWqFdfB9ZGZu?= =?us-ascii?Q?TRK48gTh194sHni5q5gX/MiNyhh6MGFCKJ6SjuoDnnNqLprBYL3EW/pUXTqO?= =?us-ascii?Q?uMTruUmVyEDa5D8RBvX4Rb3s1djDmp6eTILVlA42oM8BzjgvH/KQk8PpM9OF?= =?us-ascii?Q?heCI+7US34Ms/+xFVWCw0M69vypVnl9Ufh1H6OyqpPD+Oj3Iyfk8a28u8q/2?= =?us-ascii?Q?5VOKpZsgIlWvjyaBX0b8QF2K20+TC/pb32EaZjjQ5OS4uI6yd4om91uI1OQo?= =?us-ascii?Q?R/xNei31J/qUszbKrFuvgr5eviW4mEi/XTWfbXtegsKVreb48dj7jA08usbI?= =?us-ascii?Q?1poSpl4wtBZ7yUAeHjouoe8IRYBtJvcviLpXfIdCABVteQli3tlIe+66KA2D?= =?us-ascii?Q?JFHR0mh2Jaeil0iAmg3KzcUGofwXPG8+IUdcw76mLNwXSzBPK7gVT7eNt3MB?= =?us-ascii?Q?dLS368ZnnIyxjlpPNUf4A6r9cu3J0WGM/x/hNVBL65uIfycT0d5H7CfN05i+?= =?us-ascii?Q?0QJlaOTZYU35pcHmnPcYQfrvTWaGbeTk8AaZkdhVfa/jD2DhXMQZQ45v9mTF?= =?us-ascii?Q?T1E/eWWfw/d7bpUNrnXxZft0Rg79l0UVOAxTOgDQShQoiFX//Z7hRwhMSpad?= =?us-ascii?Q?J9sfquOeaO+aZ/J0rvtTsZUKoB5ybSb4qhOT17JKM603tDrArG2M1JC64sc7?= =?us-ascii?Q?z3WXU9XiiJ5ASJp+xpJiNLCiJGQPWS6m6DJ82lPpJ9Jriv7fmDQ0bEnoYONA?= =?us-ascii?Q?8obB2RreXnPGPckCymBRzd88CZkr4pxsE0AQ/Zw6ZC1lRVvErKlljqvysa7P?= =?us-ascii?Q?vLY58caotrY7BxsPtEOX0qhayI7PUknPUVNopuuYwZyfwkp2CyBtdfEWUBb5?= =?us-ascii?Q?ZX7fdj+9+0sJdno3PJ6QIuXDXLCCszZK2HC9fJR39NHgbGXP3ufGyCCLOgtn?= =?us-ascii?Q?JtTBEAmKPl3f9b0FConURBG5l/SVGCoQ0CoSoaOpcUDLnDaINSC8ZxabUWuI?= =?us-ascii?Q?w7wMilpBE5OmwQT/Qj49yYmd8reV0JUzhC1XG/XuQgP+oMc8c59nwcuwBixM?= =?us-ascii?Q?lXhwqrNekOCZkHN8/8kwybgUh8peJfqAPodf62ap8V52llNE20AFIcOzMSB3?= =?us-ascii?Q?2Kg8dEdUIaKIkeS0o0T0YJfEZxgy/XPU5CHyFcRJ65wwIeZdIc2vLrkPhx51?= =?us-ascii?Q?vaCxhqMcCWFchFBCrxwtGEfPtLo/Ove4ykhIVnMiaWmGoQw+grRODcQylON8?= =?us-ascii?Q?7I4v2KfeLaV+xvSwj3FTVlB65cocoClgM+Ohz3WuaiPbOkkfBc+uzezAu1ZF?= =?us-ascii?Q?JZW5X1RWUhUTYgFMYPC7hDxjkYiHOWJbDNiCuB6vuzK/YP8H1+YnvszhqHYy?= =?us-ascii?Q?eTP5z+X7OuXy5W/WBDhNzylZ8V4ARddWeFudDba/x9TLWmdF2nwGDyosyqxZ?= =?us-ascii?Q?ZvC6RgkwWplAxUJav8G0gTpQwgxv2aBtiLOVJ08xjEmglYT/5nyUT4+XX38v?= =?us-ascii?Q?OLd1ZA=3D=3D?= 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: DM8PR11MB5670.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 6b14a341-3f6f-4426-a6ae-08d9fbabac08 X-MS-Exchange-CrossTenant-originalarrivaltime: 01 Mar 2022 17:48:19.2769 (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: Hjmu8CfZBciRRLS02LQojpInfi//ftFLah1JyHcdu8OyeZrSfCMiconWOPZs4QvEUabdkr0BCU+BgRa/y193eZ4DA2rKOSeAmci/qZHwphU= X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR11MB4957 X-OriginatorOrg: intel.com 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 HI Jerin, Thanks for your patch! I think we are making great progress, here are a few= more comments:=20 > +/** > + * Input color method > + */ > +enum rte_mtr_input_color_method { We should clean up the names of these methods a bit: we should not mix head= er names (VLAN, IP) with header field names (DSCP, PCP), in the sense that = to me METHOD_VLAN_DSCP should be replaced with either: * METHOD_OUTER_VLAN_IP :shorter name, as only the headers are mentioned (my= preference, but I am OK with both) * METHOD_OUTER_VLAN_PCP_IP_DSCP: longer name, as both the headers and the h= eader fields are mentioned Please put a blank line in between these methods to better readability. I see some issues in the list of methods below, I am trying to do my best t= o catch them all: > + /** > + * The input color is always green. > + * The default_input_color is ignored for this method. > + * @see struct rte_mtr_params::default_input_color > + */ > + RTE_MTR_INPUT_COLOR_METHOD_COLOR_BLIND =3D RTE_BIT64(0), OK. > + /** > + * If the input packet has at least one VLAN label, its input color is > + * detected by the outermost VLAN DEI(1bit), PCP(3 bits) > + * indexing into the struct rte_mtr_params::vlan_table. > + * Otherwise, the default_input_color is applied. > + * @see struct rte_mtr_params::default_input_color > + * @see struct rte_mtr_params::vlan_table > + */ > + RTE_MTR_INPUT_COLOR_METHOD_VLAN =3D RTE_BIT64(1), OK. Does your HW use PCP+DEI , or just PCP? > + /** > + * If the input packet is IPv4 or IPv6, its input color is detected by > + * the outermost DSCP field indexing into the > + * struct rte_mtr_params::dscp_table. > + * Otherwise, the default_input_color is applied. > + * @see struct rte_mtr_params::default_input_color > + * @see struct rte_mtr_params::dscp_table > + */ > + RTE_MTR_INPUT_COLOR_METHOD_DSCP =3D RTE_BIT64(2), OK. Please change name to METHOD_IP. Description: Change the "outermost DSCP" to "the DSCP field of the outermos= t IP header". I would move this up on the second position (to follow immediately after th= e color blind method). > + /** > + * If the input packet has at least one VLAN label, its input color is > + * detected by the outermost VLAN DEI(1bit), PCP(3 bits) > + * indexing into the struct rte_mtr_params::vlan_table. > + * If the input packet is IPv4 or IPv6, its input color is detected by > + * the outermost DSCP field indexing into the > + * struct rte_mtr_params::dscp_table. > + * Otherwise, the default_input_color is applied. > + * @see struct rte_mtr_params::default_input_color > + * @see struct rte_mtr_params::vlan_table > + * @see struct rte_mtr_params::dscp_table > + */ > + RTE_MTR_INPUT_COLOR_METHOD_VLAN_DSCP =3D RTE_BIT64(3), OK. Please change name to METHOD_VLAN_IP. This should follow immediately after the METHOD_VLAN. Description: please use "Otherwise" before "if the input packet is IP"; ple= ase replace "outermost DSCP" as above. Is your HW using DEI + PCP or just PCP? > + /** > + * If the input packet has at least one VLAN label, its input color is > + * detected by the innermost VLAN DEI(1bit), PCP(3 bits) > + * indexing into the struct rte_mtr_params::vlan_table. > + * Otherwise, the default_input_color is applied. > + * @see struct rte_mtr_params::default_input_color > + * @see struct rte_mtr_params::vlan_table > + */ > + RTE_MTR_INPUT_COLOR_METHOD_INNER_VLAN =3D RTE_BIT64(4), OK. Is your HW using DEI + PCP or just PCP? > + /** > + * If the input packet is IPv4 or IPv6, its input color is detected by > + * the innermost DSCP field indexing into the > + * struct rte_mtr_params::dscp_table. > + * Otherwise, the default_input_color is applied. > + * @see struct rte_mtr_params::default_input_color > + * @see struct rte_mtr_params::dscp_table > + */ > + RTE_MTR_INPUT_COLOR_METHOD_INNER_DSCP =3D RTE_BIT64(5), This is very confusing to me, I don't get what this one is about: The "inne= r" word in the name suggests that inner VLAN is attempted first, then IP DS= CP (if no VLAN is present), but the description only talks about IP. > + /** > + * If the input packet has at least one VLAN label, its input color is > + * detected by the innermost VLAN DEI(1bit), PCP(3 bits) > + * indexing into the struct rte_mtr_params::vlan_table. > + * If the input packet is IPv4 or IPv6, its input color is detected by > + * the innermost DSCP field indexing into the > + * struct rte_mtr_params::dscp_table. > + * Otherwise, the default_input_color is applied. > + * @see struct rte_mtr_params::default_input_color > + * @see struct rte_mtr_params::vlan_table > + * @see struct rte_mtr_params::dscp_table > + */ > + RTE_MTR_INPUT_COLOR_METHOD_INNER_VLAN_DSCP =3D > RTE_BIT64(6), OK. Description fixes: Use "otherwise" before "if IP"; replace innermost DSCP w= ith "DSCP field of the outermost IP header". Regards, Cristian