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 6AB18A0032; Tue, 13 Sep 2022 15:46:50 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 026174021D; Tue, 13 Sep 2022 15:46:50 +0200 (CEST) Received: from NAM12-DM6-obe.outbound.protection.outlook.com (mail-dm6nam12on2075.outbound.protection.outlook.com [40.107.243.75]) by mails.dpdk.org (Postfix) with ESMTP id 39FCF40151 for ; Tue, 13 Sep 2022 15:46:47 +0200 (CEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=jc1fBuFKzAilGgqZi/a0tY8XlH6MviuGytbTI41VgVILEEDIP6WAHoigBHMtWD132MAp171usPLUmZ5Ews0OMCL8R4ORj/y3BDPZa5Z3VXzipmYuTN5jz0gKgb9W0FFryKk6+QG5jBKnFtnwUrZic8RerpUiXwZdRs9BB/TxG4FMqrnmfmrkFGKknDTgVboe9bBrj/f7FiEmYuyawy+c4FQkiXRiQvzZkMsi4EHJLLSdptJRSoHYoNhVvsvyhokIp2MpjbeMbOVsyiV0Z4QZtM2ty1MQvuGgZI5+j7iylyq7CNR5QI2sFWT1PkXg4GGm5rmSym2R4H/B0FBhn9EYGg== 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=PsGMujAMVGDkrgjlkA8a3Z8KAui8EgjMVrP4poceeBU=; b=YyV+yi8ZoWwWfBt05NEr50gsn/VIThNMbpjyOQUpLCfAo5XQEUPeYhooWpmWZsxEmw4kV+KNecrrVNXp5bB6DY1GybuN9ThARJf+I1ZOz0PxtkHbIgw3Uxux5TNq6XAivSOvSDmzchEnBw0bE6h2Sc/yYPnT0RjGnDq4Sliej9anmZt0YJn8blFygdR2Zi6NOY+EaWY5k7wuuRb+itub+0cee1OwqZuk1s7GqALJyrEsoJIBkadni5lOcechr3KTtspalN2cRgM3uBr5w6Djk8l1wXFoQFTzkrTbWZiE6hbeLYdxQPZ/COk2HCTGkWsK9gJPAFP1/xpt9A7cIPNOvg== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=nvidia.com; dmarc=pass action=none header.from=nvidia.com; dkim=pass header.d=nvidia.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Nvidia.com; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=PsGMujAMVGDkrgjlkA8a3Z8KAui8EgjMVrP4poceeBU=; b=OWlCAjwY8YzpazYy2FsNBouNwMUGO1o84eX0oQpiP1WIytMGpAH372YtUaItjY1CjT1EWF41NESozaX4knFb4fQmGnh27Yv7iVcEhSrIm+eP2IonvGjLaZmW1+0IZ/Udrxss/VmKbMdOprz+IkFSDTs3mMeXJxBQp7z+68DxYEgfjp/xEaPHlskN427Zp0NsQHCpzg97tTxxH/XnOX90YtLfmz5U3ok2+z9dIpmpTtAJw3W8hwHhgK7JTi1xRZFeeezx9k7KCI0WX53rUHiXQ+nQVppQsw2+AtpoVQHNjAYtVzk1MskdKA6t4T4LTRdh0+qx3lK6968tn7F3B7zTqw== Received: from BN9PR12MB5273.namprd12.prod.outlook.com (2603:10b6:408:11e::22) by DS0PR12MB7558.namprd12.prod.outlook.com (2603:10b6:8:133::21) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5612.18; Tue, 13 Sep 2022 13:46:45 +0000 Received: from BN9PR12MB5273.namprd12.prod.outlook.com ([fe80::a426:af46:a459:d88b]) by BN9PR12MB5273.namprd12.prod.outlook.com ([fe80::a426:af46:a459:d88b%3]) with mapi id 15.20.5612.022; Tue, 13 Sep 2022 13:46:45 +0000 From: Rongwei Liu To: Ivan Malov CC: Matan Azrad , Slava Ovsiienko , Ori Kam , "NBU-Contact-Thomas Monjalon (EXTERNAL)" , Aman Singh , Yuying Zhang , Andrew Rybchenko , "dev@dpdk.org" , Raslan Darawsheh Subject: RE: [PATCH v1] ethdev: add direction info when creating the transfer table Thread-Topic: [PATCH v1] ethdev: add direction info when creating the transfer table Thread-Index: AQHYwmNUlJ87k3rsrk6Ble4p/RnGnq3cDLKAgAFR3AA= Date: Tue, 13 Sep 2022 13:46:44 +0000 Message-ID: References: <20220907024020.2474860-1-rongweil@nvidia.com> <1be72d6-be5b-88b2-f15-16fd2c6d0c0@oktetlabs.ru> In-Reply-To: <1be72d6-be5b-88b2-f15-16fd2c6d0c0@oktetlabs.ru> Accept-Language: zh-CN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=nvidia.com; x-ms-publictraffictype: Email x-ms-traffictypediagnostic: BN9PR12MB5273:EE_|DS0PR12MB7558:EE_ x-ms-office365-filtering-correlation-id: 0d1621b4-cb73-4dba-556c-08da958e65ac x-ld-processed: 43083d15-7273-40c1-b7db-39efd9ccc17a,ExtAddr x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: vNaBcV7xh3CJ9Coarnhfks7jvPs86LIAz4wJ+XFTCZWCW6EZFLp8lyXTotzA+BT30zXDCY7zS4J0KZ2i4ja1LXniJf+cARM4n8ZAqw93ugPD6ABl2Usi+lietrhRjtVUp90oZ+zwsl0JN6egtpNZ9F5v3fHbnE11JnIB5gOKb0PSODVqb2Xkfzx1PLgMyXkszplFmxHXtai8P7tfdsK9SGKhVUzk4AYknRz2Bwn2/LrgssrY82s6V7+ZGdb2YUQHXhUOLnYqzpjze/Un5W5zm96cR0+nwSzGnd+f2tCl1pvFVnBlB0MtDSyZ2SCf1+3lupTozVEuNjMSj+ZwabCBYvxX0QPO3OoIsHUCdFvZV/4EAPH2gp42iYlKOyQc5chMRrDlUH0SV+EZq21l+VxGFx7z2RnZxArU6xnchdwbFEb6bDhpXma81qA2ca2Xz4mNwOEBGSsToS5uMCS58hB8Dg5xvmNBUGEcHzCxCPb465k+98Ps64/qVm/ryJwmibz1p07RLVeZHhuwu+53R4PCwtSrMa0ZZsTC+E1Bxx7js9mUiwrPGPdmkW+AA3wu9+TAJnFcHMLbDR7zqicLrpom+zupxFpAfAUxBNAt/Q0Th0vjlQV6KwSttvT4RwYVvUVzMoQnBK3RXrKof65aTUt2oNrYwCOl1r0kmuR/zP2e9+6+v1N816DNOBaJkxlm6FodYw6mU5qKD9bfhfoBqyvOR5Rzylr8VHk1ubYTw7ta6hAtghk8p8gkWDoaSg6jPV3hT0TV+YG4r6l93/FuF1j9lw== x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:BN9PR12MB5273.namprd12.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230022)(4636009)(396003)(39860400002)(136003)(346002)(376002)(366004)(451199015)(66946007)(26005)(8936002)(54906003)(2906002)(66446008)(71200400001)(64756008)(55016003)(6916009)(9686003)(8676002)(478600001)(52536014)(83380400001)(316002)(186003)(5660300002)(33656002)(76116006)(86362001)(66556008)(41300700001)(66476007)(38100700002)(122000001)(38070700005)(7696005)(4326008)(107886003)(53546011)(6506007); DIR:OUT; SFP:1101; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?+cPafY2cd7ZTgjEGphjc30fbUzoHMYF/jgl8J3ev9L0Nxj+H+HYB4wcRQI3D?= =?us-ascii?Q?kDtEoq+cBWix9z16nLEiB4CbxNzUIlmqWWDkAXhhux2V2EuA3eYYXj434co5?= =?us-ascii?Q?tdDl/xMmg2MKdyUegewUc/JhnMd+bh0iS4Jequ0lDIRGuxtV3cHm6D6g7qjE?= =?us-ascii?Q?V4B7Mgz4h5ue6uOAIjYXIJGVzZSWifuzm2g14F+1KhwJgjP0VsbFYzE9bWfK?= =?us-ascii?Q?4ZuIYqJEgCh7d3Bfp95R8HM1d1Bk/Q/XbBdSxsoIbzPeAGqJEShiDmWNqrJ/?= =?us-ascii?Q?G3VpJeTyYtDoeXjNmdGJf0512Jb4fwAQEvaXPwPIQ7wBOmToHUvGO79C3rEO?= =?us-ascii?Q?l0s0exv2fLsq0bHzRDZWMe46Qx1fiuCcdxguBUSHdFQXfpOIGbOzHfMG+hjt?= =?us-ascii?Q?3OP+wbu1f+i4GofknoCfefLa8TX1ZMuL/LwJU4i7AXsaO1ho30YlJBu9lGSa?= =?us-ascii?Q?1Lw055QTt+FrNl0vyBi+KCxWIjonJgXHxBzFcWSK4MEXWoz1FD3d9SpLKL5Z?= =?us-ascii?Q?fsTGqrXwXQCjirb9MoFsKEW3/62nQ3mZlfxMzfP6/ytKN6odsmEG32l3AEsS?= =?us-ascii?Q?vjZkRlnnNMR20fDPQRoF0um4piigWzGuEOZdD/1gzx7eE3Wz4PE6pobOeRMY?= =?us-ascii?Q?kbrBpuuJd2ddYCqMRSBs0gv2yqk1Bq8p1Y/uoiND2GHBO1T/5zSkJrwpO01Q?= =?us-ascii?Q?5nxZpxC/rEr6XaDU6+vdkcqHrMWLXeQito0Q8yuk4BgKbX7vsG0oE5NXPQRs?= =?us-ascii?Q?1WTN3P2lXVPZvAccQ5tXhUaWI2JS37Au0+IzS5nSkKHONR/FasmAWO92JU7l?= =?us-ascii?Q?C7H8Wl7hFFjOfrBwMrAUB2NSugVZsH8I2pGwJP6/Yg+oOPbpQMDlTiq423F3?= =?us-ascii?Q?wfy6/Ffw7tCsyM19u15zSOP/YiwS1utu+IoB3HoQjbgTQBGmjq5JzTE8ApUl?= =?us-ascii?Q?EiOPlLjjv59OeAHEQXYyjyalvUY8GapnwtZlAfj3BcFPeUCp3D/fLEBefzAm?= =?us-ascii?Q?nd/FrOOqBtEbxJ0KvtGfDzQbmEaQqpXy2GBVah2agl53sNYNoQrf6AA2suZe?= =?us-ascii?Q?U8LVQKB4yYAhfGNUyRYnbfM7CcYcBClMq/d61ZhXl49CdAGUSPwlKslSng7Y?= =?us-ascii?Q?VErLL6UZiiixO8uTDuZp4duZdIAe07mACYFapBkGdKXf7Ypf8gsMTj2ludXE?= =?us-ascii?Q?6z5NIyRN5of1PDpSvcUggkz16hIZjlBSyxDrqDfS6BslvMpuMCoIqa92pb2/?= =?us-ascii?Q?fBDY22W9307BoPROKDK5MAeK1MRu+ZIVn10Zi7csHFh0gSyGy0gnZQ//CMod?= =?us-ascii?Q?AG9IMF8bjtlxOgOIGOzrgs17izUC++fw48QFUrlu8T6+w/DwO75wk8uUXJUm?= =?us-ascii?Q?veXAle/zX54POp4Zi8pVJi4rmOnRr9u1RA8J6wuqZJcr00EEp3HBCGffzIXH?= =?us-ascii?Q?YhKKYal8GXXSP8IPJTcyB2/QT6xRoIxb1XUh0At8ul+aac3LU7UJCuq1LVyw?= =?us-ascii?Q?cxu6DqYHWR/NpxO1sGSmXXF59EeWpWzxtcFHiV/CBFLQon697XR8EhWOzCA4?= =?us-ascii?Q?qJIfbkcZcrf1jEJbe1w314mJxme09FEr98GhMbw+?= Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: Nvidia.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: BN9PR12MB5273.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 0d1621b4-cb73-4dba-556c-08da958e65ac X-MS-Exchange-CrossTenant-originalarrivaltime: 13 Sep 2022 13:46:44.9721 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 43083d15-7273-40c1-b7db-39efd9ccc17a X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: NC0AfXsrPiHjmUJXRER0XroXXLx3revrg02XCAtf093i8940uQI1AbQ+m3PHKgqS3CoqYiQxFUELeiYjyFT7Ww== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR12MB7558 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=20 BR Rongwei > -----Original Message----- > From: Ivan Malov > Sent: Tuesday, September 13, 2022 00:57 > To: Rongwei Liu > Cc: Matan Azrad ; Slava Ovsiienko > ; Ori Kam ; NBU-Contact- > Thomas Monjalon (EXTERNAL) ; Aman Singh > ; Yuying Zhang ; > Andrew Rybchenko ; dev@dpdk.org; Raslan > Darawsheh > Subject: Re: [PATCH v1] ethdev: add direction info when creating the tran= sfer > table >=20 > External email: Use caution opening links or attachments >=20 >=20 > Hi, >=20 > On Wed, 7 Sep 2022, Rongwei Liu wrote: >=20 > > The transfer domain rule is able to match traffic wire/vf origin and > > it means two directions' underlayer resource. >=20 > The point of fact is that matching traffic coming from some entity like w= ire / > VF has been long generalised in the form of representors. So, a flow rule= with > attribute "transfer" is able to match traffic coming from either a > REPRESENTED_PORT or from a PORT_REPRESENTOR (please find these items). >=20 > > > > In customer deployments, they usually match only one direction traffic > > in single flow table: either from wire or from vf. >=20 > Which customer deployments? Could you please provide detailed examples? >=20 > >=20 We saw a lot of customers' deployment like: 1. Match overlay traffic from wire and do decap, then send to specific vpor= t. 2. Match specific 5-tuples and do encap, then send to wire. The matching criteria has obvious direction preference. =20 > > Introduce one new member transfer_mode into rte_flow_attr to indicate > > the flow table direction property: from wire, from vf or > > bi-direction(default). >=20 > AFAIK, 'rte_flow_attr' serves both traditional flow rule insertion and > asynchronous (table) approach. The patch adds the attributes to generic > 'rte_flow_attr' but, for some reason, ignores non-table rules. >=20 > >=20 Sync API uses one rule to contain everything. It' hard for PMD to determine= if this rule has direction preference or not. Image a situation, just for an example: 1. Vport 1 VxLAN do decap send to vport 2. 1 million scale 2. Vport 0 (wire) VxLAN do decap send to vport 3. 1 hundred scale. 1 and 2 share the same matching conditions (eth / ipv4 / udp / vxlan /...),= so sync API consider them share matching determination logic.=20 It means "2" have 1M scale capability too. Obviously, it wastes a lot of re= sources. In async API, there is pattern_template introduced. We can mark "1" to use = pattern_tempate id 1 and "2" to use pattern_template 2. They will be separated from each other, don't share anymore. > For example, the diff below adds the attributes to "table" commands in > testpmd but does not add them to regular (non-table) commands like "flow > create". Why? >=20 > > "table" command limits pattern_template to single direction or bidirection= per user specified attribute. "rule" command must tight with one "table_id", so the rule will inherit th= e "table" direction property, no need to specify again. > > It helps to save underlayer memory also on insertion rate. >=20 > Which memory? Host memory? NIC memory? Term "underlayer" is vague. > I suggest that the commit message be revised to first explain how such > memory is spent currently, then explain why this is not optimal and, fina= lly, > which way the patch is supposed to improve that. I.e. be more specific. >=20 > >=20 For large scalable rules, HW (depends on implementation) always needs memor= y to hold the rules' patterns and actions, either from NIC or from host. The memory footprint highly depends on "user rules' complexity", also diff = between NICs. ~50% memory saving is expected if one-direction is cut. > > By default, the transfer domain is bi-direction, and no behavior change= s. > > > > 1. Match wire origin only > > flow template_table 0 create group 0 priority 0 transfer wire_orig... > > 2. Match vf origin only > > flow template_table 0 create group 0 priority 0 transfer vf_orig... > > > > Signed-off-by: Rongwei Liu > > --- > > app/test-pmd/cmdline_flow.c | 26 +++++++++++++++++++++ > > doc/guides/testpmd_app_ug/testpmd_funcs.rst | 3 ++- > > lib/ethdev/rte_flow.h | 9 ++++++- > > 3 files changed, 36 insertions(+), 2 deletions(-) > > > > diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c > > index 7f50028eb7..b25b595e82 100644 > > --- a/app/test-pmd/cmdline_flow.c > > +++ b/app/test-pmd/cmdline_flow.c > > @@ -177,6 +177,8 @@ enum index { > > TABLE_INGRESS, > > TABLE_EGRESS, > > TABLE_TRANSFER, > > + TABLE_TRANSFER_WIRE_ORIG, > > + TABLE_TRANSFER_VF_ORIG, > > TABLE_RULES_NUMBER, > > TABLE_PATTERN_TEMPLATE, > > TABLE_ACTIONS_TEMPLATE, > > @@ -1141,6 +1143,8 @@ static const enum index next_table_attr[] =3D { > > TABLE_INGRESS, > > TABLE_EGRESS, > > TABLE_TRANSFER, > > + TABLE_TRANSFER_WIRE_ORIG, > > + TABLE_TRANSFER_VF_ORIG, > > TABLE_RULES_NUMBER, > > TABLE_PATTERN_TEMPLATE, > > TABLE_ACTIONS_TEMPLATE, > > @@ -2881,6 +2885,18 @@ static const struct token token_list[] =3D { > > .next =3D NEXT(next_table_attr), > > .call =3D parse_table, > > }, > > + [TABLE_TRANSFER_WIRE_ORIG] =3D { > > + .name =3D "wire_orig", > > + .help =3D "affect rule direction to transfer", >=20 > This does not explain the "wire" aspect. It's too broad. >=20 > > + .next =3D NEXT(next_table_attr), > > + .call =3D parse_table, > > + }, > > + [TABLE_TRANSFER_VF_ORIG] =3D { > > + .name =3D "vf_orig", > > + .help =3D "affect rule direction to transfer", >=20 > This explanation simply duplicates such of the "wire_orig". > It does not explain the "vf" part. Should be more specific. >=20 > > + .next =3D NEXT(next_table_attr), > > + .call =3D parse_table, > > + }, > > [TABLE_RULES_NUMBER] =3D { > > .name =3D "rules_number", > > .help =3D "number of rules in table", @@ -8894,6 +8910,16 > > @@ parse_table(struct context *ctx, const struct token *token, > > case TABLE_TRANSFER: > > out->args.table.attr.flow_attr.transfer =3D 1; > > return len; > > + case TABLE_TRANSFER_WIRE_ORIG: > > + if (!out->args.table.attr.flow_attr.transfer) > > + return -1; > > + out->args.table.attr.flow_attr.transfer_mode =3D 1; > > + return len; > > + case TABLE_TRANSFER_VF_ORIG: > > + if (!out->args.table.attr.flow_attr.transfer) > > + return -1; > > + out->args.table.attr.flow_attr.transfer_mode =3D 2; > > + return len; > > default: > > return -1; > > } > > diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst > > b/doc/guides/testpmd_app_ug/testpmd_funcs.rst > > index 330e34427d..603b7988dd 100644 > > --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst > > +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst > > @@ -3332,7 +3332,8 @@ It is bound to > ``rte_flow_template_table_create()``:: > > > > flow template_table {port_id} create > > [table_id {id}] [group {group_id}] > > - [priority {level}] [ingress] [egress] [transfer] > > + [priority {level}] [ingress] [egress] > > + [transfer [vf_orig] [wire_orig]] >=20 > Is it correct? Shouldn't it rather be > [transfer] [vf_orig] [wire_orig] > ? >=20 > > rules_number {number} > > pattern_template {pattern_template_id} > > actions_template {actions_template_id} diff --git > > a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h index > > a79f1e7ef0..512b08d817 100644 > > --- a/lib/ethdev/rte_flow.h > > +++ b/lib/ethdev/rte_flow.h > > @@ -130,7 +130,14 @@ struct rte_flow_attr { > > * through a suitable port. @see rte_flow_pick_transfer_proxy(). > > */ > > uint32_t transfer:1; > > - uint32_t reserved:29; /**< Reserved, must be zero. */ > > + /** > > + * 0 means bidirection, > > + * 0x1 origin uplink, >=20 > What does "uplink" mean? It's too vague. Hardly a good term. >=20 > > + * 0x2 origin vport, >=20 > What does "origin vport" mean? Hardly a good term as well. >=20 > > + * N/A both set. >=20 > What's this? >=20 > > + */ > > + uint32_t transfer_mode:2; > > + uint32_t reserved:27; /**< Reserved, must be zero. */ > > }; > > > > /** > > -- > > 2.27.0 > > >=20 > Since the attributes are added to generic 'struct rte_flow_attr', non-tab= le > (synchronous) flow rules are supposed to support them, too. If that is in= deed > the case, then I'm afraid such proposal does not agree with the existing = items > PORT_REPRESENTOR and REPRESENTED_PORT. They do exactly the same > thing, but they are designed to be way more generic. Why not use them? >=20 > Ivan