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 C0DD341BB4; Fri, 3 Feb 2023 03:57:08 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 765F14014F; Fri, 3 Feb 2023 03:57:08 +0100 (CET) Received: from NAM12-DM6-obe.outbound.protection.outlook.com (mail-dm6nam12on2074.outbound.protection.outlook.com [40.107.243.74]) by mails.dpdk.org (Postfix) with ESMTP id 5D35940141 for ; Fri, 3 Feb 2023 03:57:07 +0100 (CET) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=FmF99EauXAtDSYi1x1bis4RaVXRljKoQXHsOyycFST5sf7KpLQrJmwA+z2IGWumrbJEVLgsOQHHh1k9DJklOMvPF5BZKW50rllZocZkkvH0FS501LUp+yFy8jC6ORjgul226aqspgYB9REiE6Tqrbjrpxrj7huI3nwHkPe/gl1WI/ysiIX1LQByzdISpMm/Bj7ODSF8szgnBF2q6SddVv1VuDIWIOJcGYxKUbntu8U4JGZRbGQa4sH6ln4u1hqsd4cueB2W06uzw+pbzo+2Idp/Cp58d4G825B7fjbSXlRMyFFy7PDgeEvPEgjUAUw2bcGPG8pIXgqS0GpGZZiMVaA== 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=0e+7Sf7XqFwUVmZKFfZuZyFqbvdWZawbxZVZ56Tp5V4=; b=iyFd3fa/8Siskl2jH8S6A5JcZo287qi/j/IWfqrVgLTkeml90CIVfn5aeWeeCbR6iI1WYDtBFA3tb7l3ClDHsaB6eu7LYO3/TO+jIA5+UviW1f/Iy8lUQ6YyrcnRdUnXz935HPwrvqcE4mrF61UUofAXGEWFmpi6/wYanlHehOd9VvfeW2TFwo2xpyJa9yBbnVtCEA45d06Chl8zISEudygabxDqz2tFs49UE6dNU7KEyvghnoY4v0+T372kFpDj+1/8Vh7C0FDef9AVEBceJhBAK5YzJBLm6+TyYVLOFF9vQc68bWrbX+uSfGnZGVau2X+w9QuZfwUiZFtkqFmSzA== 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=0e+7Sf7XqFwUVmZKFfZuZyFqbvdWZawbxZVZ56Tp5V4=; b=AOweCvS6XcOfg3erp/15auha4/Wb3p1e5OUfuz+X4jhO6lqsjwPJObcGaXaYSvQkaHGYzjzz5dsWGBV9cRg5ZaqQv5oluL0Yf3bH1uIe7rpqq3DT28RTybvqaIHAdes66vSEvxZgFMx2DBUKq8+mjiJWE6DTRD7t8evKEQbQbipyyFKheI1jFYFL8jEdNF44q1UODVSPK+jxKkpFplABCkzuwSaBrm5WsRUpTunuiYRvOXkb7nfZJJc+0RLFrx6dv8U6gGROCz0N6HgHEgncFnAQ8ogttZ7CBKHTvvE06PtwAb0Me/XXECgvzN47ZLFKz6c636uN98GYTLhW8Me+7w== Received: from CH2PR12MB4248.namprd12.prod.outlook.com (2603:10b6:610:7a::23) by PH0PR12MB5497.namprd12.prod.outlook.com (2603:10b6:510:eb::22) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6064.29; Fri, 3 Feb 2023 02:56:47 +0000 Received: from CH2PR12MB4248.namprd12.prod.outlook.com ([fe80::f2a4:bf61:61d5:2950]) by CH2PR12MB4248.namprd12.prod.outlook.com ([fe80::f2a4:bf61:61d5:2950%9]) with mapi id 15.20.6064.027; Fri, 3 Feb 2023 02:56:47 +0000 From: "Leo Xu (Networking SW)" To: "NBU-Contact-Thomas Monjalon (EXTERNAL)" CC: "dev@dpdk.org" , Bing Zhao , Ori Kam , Aman Singh , Yuying Zhang , Ferruh Yigit , Andrew Rybchenko , Olivier Matz , "david.marchand@redhat.com" Subject: RE: [PATCH v2 1/3] ethdev: add ICMPv6 ID and sequence Thread-Topic: [PATCH v2 1/3] ethdev: add ICMPv6 ID and sequence Thread-Index: AQHZFEb31FRUO605hEClPnSd5PClL66kFoAAgBQ89hCAAcr5AIACINJggAAxXICAAFq6kA== Date: Fri, 3 Feb 2023 02:56:46 +0000 Message-ID: References: <20221212085923.2314350-1-yongquanx@nvidia.com> <6520018.4vTCxPXJkl@thomas> <1991609.QkHrqEjB74@thomas> In-Reply-To: <1991609.QkHrqEjB74@thomas> Accept-Language: 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: CH2PR12MB4248:EE_|PH0PR12MB5497:EE_ x-ms-office365-filtering-correlation-id: 351b8c18-2c69-4c75-837b-08db05924a1a 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: o7Y/kx2Jmltf/3URpAZP3aDVYjtrDp2uvcfW6kigiOAk17AypIucglSA7Jn4I5U4rFOTmxmNuFvaoEzAK1Jbwh74rL30lgfPSljry4dC7KBdTGYxZBbD5pbo3AtzpZv5zfF8h5vOqGQBDP60ZMMgAkwbwlr7v/vCgSNdVbfim8/O6m/86tXrXHszYiixM4CIsfBPbpCvL0wWvdBbN48ZZGnR84jIvHZEJKeQsEr5PhO4NNr1FcmspUVhZRJvXWG/JLBPlJ7E5sUfnN1GA5OU1w5RtuMj5lSJ/rbtvzaw0lbuZV9Q/lqXozsAFqmujJImsuggj5x/Ho3k6KUUIL3/LN3OnthMwvZ+J64rxMjDOVJQSGVNEC0vC+FutthltQsJEToEjH1FLmE4PEV28sGSERdfuK96KNxNsyfjy3IM7ahdoGP/eSpqXewrewWIUHOD2P0EAkxBIlcRkdUHK4CPEfakxPUW9Y3r1Kt2GLDCo5/JYr0aZKNwnXEurRBmxq53/F0Tg1oOeoApkIsFqg3sl3EEbCqyK9ZTfp7323AF4RvtnJKfZAvfk4hiLkOE+e1kMGlR7487BTu5JX0NnWaUnYpKuhCs8C5YTsWMZPSzcpAiWPAIOlrE3GnnAWEqT5bLwpFlMgjXVK8on7ffUjqe4M07RBlYwQpeAUwIIIbXuzyNNEQy+mnorIrXZBZFHCQqxRdF3eT+j1dHL6yate3B8Q== x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:CH2PR12MB4248.namprd12.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230025)(4636009)(366004)(39860400002)(396003)(136003)(346002)(376002)(451199018)(38100700002)(122000001)(41300700001)(7696005)(38070700005)(8676002)(52536014)(86362001)(316002)(54906003)(6506007)(5660300002)(83380400001)(66946007)(71200400001)(64756008)(66446008)(66476007)(6916009)(33656002)(66556008)(478600001)(4326008)(76116006)(55016003)(8936002)(9686003)(26005)(2906002)(186003); DIR:OUT; SFP:1101; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?v7Gv5oxVcdpoqoME/jYNqEAK/cpmgev+vp4HKxILpVDLUUQKBFHWwzjPTTL6?= =?us-ascii?Q?sAPFMIb7nf7OYpX/PR71oxsAydiZ1SKgmGnW6fnpwfWeHRbgJ2Gtsso9GLGG?= =?us-ascii?Q?IcEIE4L/oUTuTzMUhtYUKvZ+BqIJACKireB078GasSFHc4pqY40zttBTAqNG?= =?us-ascii?Q?5qaaAv5/tiitIWPuysVmvn2zkfpYsOF0dFCVkW9GsHNDDkHGHgl2Yk47BnKV?= =?us-ascii?Q?YkFTL4WHtGAGrSxCJfdfQ5HKlnDANebsIJQ29mfpG36xZTDjz7hveEkPUcNG?= =?us-ascii?Q?sqTSSaK1ZWq6Hveoq3/vtE2d6BHLdKwYa9O14NFbLlpUCSoWwDCi5Vif1bHd?= =?us-ascii?Q?MpCAqQme525ACEZVLawuw+UPz4AWzv5miM+mBDA+LOcFrwpEL/5c1wDdl7tF?= =?us-ascii?Q?Q9NDy3kt+qqe6DAA+lBe5K0M7TP5zBKCeyM2EyrEwZEChzYeNswmpcf+A0uv?= =?us-ascii?Q?eSfHMnxiMiQqhizg4K/Q9fNmEDMYpwbMNMpEfTzruxWJT65gqaP5FcEVD5no?= =?us-ascii?Q?qf7G3ECT3Bz+NOK1MdrUNVop5Ra/FY4hAjHe8Ly9OPQFrAmKZW3xO2BPX+To?= =?us-ascii?Q?8rPmksJPo5ZtTPh5cxWQrXI5Bi0kb38QRqm74NmPcu1+yjSVFhoPOfQTNixl?= =?us-ascii?Q?c+X9kY7+5VP2oSfDt9HqEPzIh8/K3artbEkqia1mebwmTyQ+PigJindGWVMK?= =?us-ascii?Q?xHsiTrKCUwCZCWh1Zx2nE2AkPQ9XRRNN9ebiNhzBtHxAsVcImTw8OZsInT5h?= =?us-ascii?Q?mw5ESQuQOGvmpno3mVXQEb6S2LkahNmYY0KlsXiDBkM/+pw8BMm//X27hm2U?= =?us-ascii?Q?/x4YC+XdwgMs4iJ1l8KN2JNO9564qSpNxrQLNqSzdUWkxcCyZ4aw7kx7JsAR?= =?us-ascii?Q?BwlqMTPaKOW5dVOpLtghnzzr+yoCgzorWTIYKLJXvFd7WvHav2gWs5xUZCfD?= =?us-ascii?Q?ZM0E+RUdCg5isWq3ENz8SH/BhjO4taB64Gq49iNt2tImUEz5qLge1r6UxOz6?= =?us-ascii?Q?b27x6uQy/I/F3wdFXOF48iCiK/B82FdW52gRPMusfVHK13YA8CsmnCG+ACHL?= =?us-ascii?Q?Vo19rhdUNS6TQcqpj7BbvIpvM/HfpBCxgDt6fLHx4kMvbYShOjgxS7JU+nRa?= =?us-ascii?Q?iinSuPUun4z/FpeuYfUCgamwd/gEY0EFDmDXx3XF5PwhM14+nhoj7k+VGiZd?= =?us-ascii?Q?D7SzECY0RjNisVnGdl7vvKfZWT16htQVtV3u/Xq0Y+2Enc7cXc0YJMOzMw9D?= =?us-ascii?Q?93SVibB89bjaxMLNpybGjWny8D1gKqGyw1XjzWiwRjV5RseU/gd2LWObvYeS?= =?us-ascii?Q?WNj0koJIZzMbwXKaaRDDA+5YC68RD1Rt2kfUAhAvEJeOD+TGmM/NntLiq2Fd?= =?us-ascii?Q?j/yz5ji3VIbvDMtkzvK+E92ftx04l/mEcJbF5akNgifPGi0Fm2157hk+AUbB?= =?us-ascii?Q?Y/Es/fOJuacrKSrw+Oz7h8BTviPAxkQyiPebMKrMHoaoliZzoBrYVwXau0Gs?= =?us-ascii?Q?FC0adU/5IR5+LnLATzFdIjrH6+a+ST/+k/SrvUJ3bQD7YCRMoCll44YcmEjB?= =?us-ascii?Q?TTiuJ3FvZ63NkNJvB7F7v+rvKhn9t2bVEq589WBu?= 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: CH2PR12MB4248.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 351b8c18-2c69-4c75-837b-08db05924a1a X-MS-Exchange-CrossTenant-originalarrivaltime: 03 Feb 2023 02:56:46.9204 (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: goUw+jsqurTiSYxlVwhJypD2gjG3GypCZB6zArQ9JpmOmhFRFj/y0KHDE3PWZqQl5LZgLoIw5jzJmLX3IjdObg== X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR12MB5497 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 > 02/02/2023 19:33, Leo Xu (Networking SW): > > > 31/01/2023 07:53, Leo Xu (Networking SW): > > > > From: Thomas Monjalon > > > > > 20/12/2022 08:44, Leo Xu: > > > > > > +/** > > > > > > + * ICMP6 header > > > > > > + */ > > > > > > +struct rte_icmp6_hdr { > > > > > > + uint8_t type; > > > > > > + uint8_t code; > > > > > > + rte_be16_t checksum; > > > > > > +} __rte_packed; > > > > > > + > > > > > > +/** > > > > > > + * ICMP6 echo > > > > > > + */ > > > > > > +struct rte_icmp6_echo { > > > > > > + struct rte_icmp6_hdr hdr; > > > > > > + rte_be16_t identifier; > > > > > > + rte_be16_t sequence; > > > > > > +} __rte_packed; > > > > > > > > > > It is exactly the same as struct rte_icmp_hdr. > > > > > Why not reuse it? > > > > > Maybe introduce struct rte_icmp_base_hdr and define > > > > > rte_icmp_echo_hdr as rte_icmp_hdr? > > > > > > > > Hi Thomas, > > > > Looks like, using rte_icmp_hdr as base header for both icmp and > > > > icmpv6 is > > > not that good. > > > > since, rte_icmp_hdr default their headers always having id and > > > > sequence > > > fields, which is not applicable for most other icmp6/icmp types packe= ts. > > > > > > > > I may suggest to keep icmp and icmp6 structures independent > > > > against each > > > other, because, looks like these two protocols definitions do not > > > share common base. > > > > > > What about introducing rte_icmp_base_hdr? > > > We should try to avoid duplicating things. > > > > > > > You mean introduce rte_icmp_base_hdr like following? > > struct rte_icmp_base_hdr { > > uint8_t icmp_type; > > uint8_t icmp_code; > > rte_be16_t icmp_cksum; > > } __rte_packed; > > > > And change the existing rte_icmp_hdr to be: > > struct rte_icmp_hdr { > > rte_icmp_base_hdr bash_hdr; > > rte_be16_t icmp_ident; > > rte_be16_t icmp_seq_nb; > > } __rte_packed; > > #define rte_icmp6_echo struct rte_icmp_hdr; > > > > If it is, there will be some compatibilities issues, since we changed e= xisting > structure. > > > > Or, maybe I'm missing something. > > Would you help to give more details about above comment? >=20 > Currently we have this: > struct rte_icmp_hdr { > uint8_t icmp_type; /* ICMP packet type. */ > uint8_t icmp_code; /* ICMP packet code. */ > rte_be16_t icmp_cksum; /* ICMP packet checksum. */ > rte_be16_t icmp_ident; /* ICMP packet identifier. */ > rte_be16_t icmp_seq_nb; /* ICMP packet sequence number. */ } > __rte_packed; >=20 > I agree we can move some fields in a base struct, it would change the API= . > We could manage with a union, but we would lose the benefit. > It looks like we need to keep rte_icmp_hdr as is. > So we need to duplicate and define new structs. >=20 > What about removing the "6" from the new structs, so it would apply both = to > IPv4 and IPv6? >=20 > struct rte_icmp_base_hdr { > uint8_t type; > uint8_t code; > rte_be16_t checksum; > } __rte_packed; >=20 > struct rte_icmp_echo_hdr { > struct rte_icmp_base_hdr base; > rte_be16_t identifier; > rte_be16_t sequence; > } __rte_packed; >=20 >=20 I agree with that proposal. Then, we can deem existing struct rte_icmp_hdr as old one, which should not= be used in new app. And looks like, these new defined structures can cover all ICMP4/6 formats. Good idea! I will update accordingly, in next patch.