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 CF125A0547; Tue, 15 Nov 2022 13:33:06 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 7765D40E03; Tue, 15 Nov 2022 13:33:06 +0100 (CET) Received: from NAM11-DM6-obe.outbound.protection.outlook.com (mail-dm6nam11on2064.outbound.protection.outlook.com [40.107.223.64]) by mails.dpdk.org (Postfix) with ESMTP id 73BEE40DFD for ; Tue, 15 Nov 2022 13:33:05 +0100 (CET) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Z8S1x0N5Fq3p//kJAhrq47lIzkSLHLpT6AnoY7mRsJQA7WEpUz0c5d5zklAk5VXclKXxZXwz+dYsiujQxNJywBSnBXY1c9/4v3GfN2arwnk0SUCx2wA98DZdEBZpcTDz+kfStXFPaRPlJ3Usy6keK3NgW5WDCssRiiAiLF/w8ShVEXgWZTwXSquVB4Qy2wPKEKdMLVhSvraafTwc395ro1gHJotwilehX9zSXjbJVsN4nHfr+X7anVz41+IKNvs1GHPyVY6PCd8PdB10v51PGjDzMmJ5ZR6YxyPLyv46iEbve0MJ70TYkAaH6q+HE8fAwiY4rktoA9zbl0JNE04HBQ== 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=WdOtU+6M71uP/Zda2QGtaWcwv4wFqo/5Krxv0oEVmuc=; b=IEWugbFbMFrZjU0wkoxuv7WCRVRDZxLWYjybrJsmh18YWILWGnaAiKj0l4RcSONECIvaeUh0Ip9FH+pnu9FBkZvskxYQRjcJfiKupf+pjgDqimjxGKDquhMCb3+ZIxVTHhVBCKo7uFhcvOt+wvbk3LlWFZ3LlE8j/qwBw74KlA7rtk0lfU9ll7nStZSncYuMcmAed+l7nDC6Z+gv8jgvEjenA5EwQ2kcTG4mF+7rBqaKw419/RHTTETUIfM0fy1YEWjMGVA/sG7t10j1OINQQI+Ut2ZdZMmb25qXyxbnmM2KZp0E2dL4TImic/iD6c9MOVtqEm/YSxsZEG7LP0Qm3Q== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=amd.com; dmarc=pass action=none header.from=amd.com; dkim=pass header.d=amd.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amd.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=WdOtU+6M71uP/Zda2QGtaWcwv4wFqo/5Krxv0oEVmuc=; b=igWR5Ttzup24U8+rQJNizg+87usdWUboEMMC2lRGyCFcVTmisRb0x0qSOHRVAWEoAK0K1ceRrovFDMduDUi5pc8qAP2sajomV7Pz7DtCUWKGR/6OheTKOJyj/9hnT9jDxJ8ujH4vAURIAQymTLbV+ToSIiSh+BrvIWbdidfjajk= Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=amd.com; Received: from CH2PR12MB4294.namprd12.prod.outlook.com (2603:10b6:610:a9::11) by DM6PR12MB4105.namprd12.prod.outlook.com (2603:10b6:5:217::18) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5813.18; Tue, 15 Nov 2022 12:33:03 +0000 Received: from CH2PR12MB4294.namprd12.prod.outlook.com ([fe80::b482:d5bd:c7d0:3842]) by CH2PR12MB4294.namprd12.prod.outlook.com ([fe80::b482:d5bd:c7d0:3842%8]) with mapi id 15.20.5813.018; Tue, 15 Nov 2022 12:33:03 +0000 Message-ID: Date: Tue, 15 Nov 2022 12:32:56 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.4.2 Content-Language: en-US To: Andrew Rybchenko , Aman Singh , Yuying Zhang Cc: dev@dpdk.org, Georgiy Levashov , Ivan Ilchenko References: <20221017144133.1899052-1-andrew.rybchenko@oktetlabs.ru> <20221017144133.1899052-2-andrew.rybchenko@oktetlabs.ru> From: Ferruh Yigit Subject: Re: [PATCH 1/2] app/testpmd: prepare to support TCP in Tx only mode In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-ClientProxiedBy: LO4P265CA0190.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:311::18) To CH2PR12MB4294.namprd12.prod.outlook.com (2603:10b6:610:a9::11) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: CH2PR12MB4294:EE_|DM6PR12MB4105:EE_ X-MS-Office365-Filtering-Correlation-Id: 3bd03010-466b-4401-7370-08dac7058912 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: qhXic834jZ1m4TC1tZQYEFM22d+ZsEj3PShDccqpyljopnovS5vFEK7gl1w9puSFcI/FhaeY3uxJTI6mgCBCNehdv366rUBJcMH1czEg82rPU+SIkgLtskn8S0GYrzbC40jmVRbcw6IO32Ift5cvxSqS/0PstkE29wKKtI1pkTv2xfDCyiYINZWsTxO2bpaL/tq9nowTxDaiI0jClJ4exZGh90wHDdZV41Tf2yLM09hGBddcnB5tnia2m36yR7ZqLMqgFzozbLDH+IU+zSyzFFE+cvOTMlbObgn8EkPxV0oSjfFhkbQC5dHH8v6KEybWa9i8PF+vD2eNtzwaPW9l4xxXQCNyGQsKZLHRvz16ST3Io8DdhQs+Wzlwz6Vbx9JQfk1hrlOVdin9ZyoFNW8Sc4fiCATi2a1nDKDuycJ7gvisdVm1puChrkuvVvWlSsf4GDFz7qXjGaVSxqt7CwY6knPqjSiz5+VtPa0paSAa8zVxNiWFru8Cja6OLKN67B8lfenUx2qxcoq9iVGee9BLdV4o+XuKHWA2v7jaH1w0accba+XYTaPO+j2/i5B91xHarXwJOZvqWV5Nrk4Uavl/dsFsulg+IKwGNtpWu47kyv+/qakq5ucKK+uyvhVhtv+Sp3MOCtEhJ/D+wyosT8Maa4TQV6qhWDuZ/vS80XyJHCAGwZD9jMBIPCf/VfpI3Z+1LUlfWvz03Ni4eCcywysc4saK30sx5Hiv+m5BYMHNo1vQ9MvXbbEeps74ddmNClxuVP8wFJD1kfLDpcmpZY0ZJOKofIq7IYvQ765UuE8a/vw= X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:CH2PR12MB4294.namprd12.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230022)(4636009)(346002)(396003)(376002)(136003)(39860400002)(366004)(451199015)(26005)(6512007)(66556008)(66476007)(66946007)(316002)(8676002)(2616005)(8936002)(5660300002)(186003)(4326008)(36756003)(53546011)(41300700001)(54906003)(110136005)(6506007)(31696002)(83380400001)(2906002)(86362001)(38100700002)(30864003)(44832011)(6486002)(31686004)(6666004)(478600001)(43740500002)(45980500001); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?SHc2RUR6b0sxRFJ3cVhTMS9SWm52dXoxTWs3TWRvZDZjMEdPcFNNTnNXeTNO?= =?utf-8?B?b1BWRzQ4UTZjVlJYUWNBbnVZL0RWejNBTVRUR2h0blNOMmNjTnlsc3J0VGE0?= =?utf-8?B?aHZtbXNRbVhlaE9jK21GMzJDbkVDMGZiTEF0d2NlQjBpeWF2Y3BZZUt6N0VI?= =?utf-8?B?bmowc2trVCtrcVJnYW9MUGF3eFBKZkExWXpMNFYxRkt6Zm9sNkVvazJMVFFQ?= =?utf-8?B?Y3JxTldCVDZJWDdvOERMNFRycExzRVlCeUtlOHpGRThWOEVXdWI5WFVCeWdl?= =?utf-8?B?eWVwMDR4V3lHZ2ZwMlpVZ09xTVZBVHZGUGtLUG9Vb0lLY3hGTGVEYnA5R0h3?= =?utf-8?B?dTBXWjluMGNsUDZZR1JGeEM4ZjlKRi95c2ZWeUpRY3dGTEtRU0NMTm5oSlNL?= =?utf-8?B?dmRJT0JqQ1VoN3NLM0pBOUZ3Y2U0YlJ4WkhLTU9DTHY0UWViTXl3YzRJZ3d4?= =?utf-8?B?K21OUXNGY0FvalVFVVJydFYrN0Z5WEJ0a0F0R1kyR1NhYlRlTGxZL0FMSTJy?= =?utf-8?B?TERINnE1V2RqWGRoUGE2cFRjdDNGMWhEMUVLK2ZlWUhaSkFNRHZSV2RsMjg5?= =?utf-8?B?OWtpczhWNlNnS2tHa1pKSmkzMjJNanhrNEpkNTc2Z2x4WEx6aFR1TEJJY25a?= =?utf-8?B?QWpRaVdGbVordnNYQzlGcmZwWEJ2S2lqeWZTNFhGZ3FqV0JHUjA3T2VGT0gz?= =?utf-8?B?UFR6MjZtQks5bDl2dmczekZ6bGlqVlN5NytSMzJQUWxuMlp1NGY3MDV4aTFI?= =?utf-8?B?SGFjM1ZHUUhnaU94bTJIYXRwVDJoRVV1cXRKeVdteEM4QVBnRUUwMWhuVXdi?= =?utf-8?B?RnJVc2JqSDVWNGRTeWx1NVE4dS9PVHJNdWNOTnV5aGpaZW5IK0xGN1NkbS9X?= =?utf-8?B?dFN1QTI5VERURWlIOFZJamM3RnNDclYzdlViRXBCYlk5WFRYSFVlY2ZpdE5h?= =?utf-8?B?RDUwYW1NdUp1REkzbVkyeExaY2ZMMURDVHUrZmg2NUhVejhnd1hhYWVRdkNC?= =?utf-8?B?Q2FEeGVQejFaWEFwa0puUmFCV09vUWZUclBvWkkyOVFYNEJGeExsZENxOWI3?= =?utf-8?B?b2tudytFYVpBS1BpclpvZmtxU3MzUG5oMWtTbE9TcHNFbXlnLzFqRTdncFVI?= =?utf-8?B?Rnc0Mjhpb3Z3V3cxa2xyS2I1a1U3bDJWNEFxa082bjVRNDVxcTBVem83cXFk?= =?utf-8?B?M0N4djNTNFN0UmFHREdSVkkwWW1mSXUyeURJQzJqUmdRTzhSc1lpYTBsQ1ZD?= =?utf-8?B?dGd0Z3ZCUzBDYXVJV0NGVXJtTlhMV05IWXd2WUd2N0Z2OVN2U0RCQlpBbCts?= =?utf-8?B?UzVJNks3Q3BqbW9qbE5UQU04cHk1RmFrd0pEd09lRFNtcUdBWnlOYzljS3hH?= =?utf-8?B?d05TejBGM3J5dnY4cDZ1a3U2a3FYQVBDb2EwOUtLR0hBMmV1UFFFbWN5d3Rk?= =?utf-8?B?RjJZMFhramJCcU1odmpWU3RXZHBobjd6NmpaWUpOaG9Wc3k1ZWFmNm5pR0dv?= =?utf-8?B?dEFrbnpZdCtPK2tyNVBWVXRJOUNnZ0t5eHRKbUovRk9ORlhjLytWT3BWeTlB?= =?utf-8?B?Zld1M3hjZ1BRcWVZdGQ1Yk01aXVlUElRaGt2QitELy9uN1dHei9zY2M2RC9z?= =?utf-8?B?YWM2dnBiaUpFeUl3dkFRbnJydTJwR2FzR2dHOHhaYUR1Q2Jvck1IaUdkeGEz?= =?utf-8?B?OHlXSUI2VmE4Q25XVmpQdXBLcVY2eWRpYWVHWXIzMVkxZGcvbkxISDd0NW9E?= =?utf-8?B?VDdoNEdQL1FiUkdNNnYrRlJEUFE2bjAvU2tDYzVhTE9sKzcvVUNRMTJZOGtQ?= =?utf-8?B?SjB5b3FPblhHYldvVG55MUhrNk9xOGRqVnpQT2dDSUNPYTNPWHlYVERqNDVl?= =?utf-8?B?U3YxQnFtcXVubzFYWWNjNDAyazAycDIwY3NCaGxkdlRrZitFZGtsejFyM0Ev?= =?utf-8?B?QUZKeko4eTFyQzJBbHYxcG1HU0NiblpHbFRQckxBRDUxcEdtdEZSSlc2Lzdo?= =?utf-8?B?RXpNWWFnVlZ0b2pyb2lWZG1WZzVDNDhyQ2Vhd2pNcE1MQlJ2UVIxZDEwVDBz?= =?utf-8?B?SjEvYTlFNW1kWVU2ckdpTUdoRHZKMlFpdVBrWk9hRmhjTlpZMVY3YUQvMWFL?= =?utf-8?Q?JbrTGEu44T/3nAvPStyINeuv+?= X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: 3bd03010-466b-4401-7370-08dac7058912 X-MS-Exchange-CrossTenant-AuthSource: CH2PR12MB4294.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Nov 2022 12:33:02.9181 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: X9A2O9J3IxX3U5Pl6CQ1qIkviTvcOmywLqjepAlKmMVUAKfewuzbrn60TC5ezLiz X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR12MB4105 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 On 11/11/2022 8:36 AM, Andrew Rybchenko wrote: > On 10/19/22 19:39, Ferruh Yigit wrote: >> On 10/17/2022 3:41 PM, Andrew Rybchenko wrote: >>> This is useful for the incoming support of TCP TSO in Tx only mode. >>> >>> Signed-off-by: Georgiy Levashov >>> Signed-off-by: Ivan Ilchenko >>> Signed-off-by: Andrew Rybchenko >>> --- >>>   app/test-pmd/parameters.c |  6 +-- >>>   app/test-pmd/testpmd.h    |  4 +- >>>   app/test-pmd/txonly.c     | 98 +++++++++++++++++++++++++-------------- >>>   3 files changed, 67 insertions(+), 41 deletions(-) >>> >>> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c >>> index ccb1173d7d..545ebee16b 100644 >>> --- a/app/test-pmd/parameters.c >>> +++ b/app/test-pmd/parameters.c >>> @@ -835,7 +835,7 @@ launch_args_parse(int argc, char** argv) >>>                       rte_exit(EXIT_FAILURE, >>>                            "Invalid UDP port: %s\n", >>>                            optarg); >>> -                tx_udp_src_port = n; >>> +                tx_l4_src_port = n; >>>                   if (*end == ',') { >>>                       char *dst = end + 1; >>> @@ -845,9 +845,9 @@ launch_args_parse(int argc, char** argv) >>>                           rte_exit(EXIT_FAILURE, >>>                                "Invalid destination UDP port: %s\n", >>>                                dst); >>> -                    tx_udp_dst_port = n; >>> +                    tx_l4_dst_port = n; >>>                   } else { >>> -                    tx_udp_dst_port = n; >>> +                    tx_l4_dst_port = n; >>>                   } >>>               } >> >> The argument to set this variable is "tx-udp", so it has explicit >> 'udp' in the name, so I am not sure to reuse it as L4. >> > > The patch changes nothing externally. UDP is an layer 4 > protocol and I see no problems in storage of the the > UDP port in the variable names tx_l4_dst_port. > Hi Andrew, What you said is true for this patch, but the intentions of the patch is preparation for TCP support. So eventually "tx-udp" argument will be used to configure TCP flows, which is confusing. > It is just cosmetics patch with prepares code to > further changes. > >> Also documentation [1] and help output [2] explicitly mentions from >> this as UDP, please check `git grep "tx-udp"` output. >> > > UDP is a layer 4 protocol. The variable could be named > this way from the very beginning. > Do you think we need separate variables for TCP and UDP? > I think no. Not separate variables, separate testpmd parameter for TCP. > >> One option is to update the argument name and documentation, even this >> can break some exitsting test scripts etc.. > > I think it is a bad option. > >> OR >> Can add a new parameter as "tx-tcp", I think this is better because >> right now next patch uses "txonly-tso-mss" to decide protocol is TCP, >> instead of using implied parameter "tx-tcp" can clarify that TCP >> protocol is requested. This also can help to decouple TCP and TSO >> support. >> What do you think? >> > > We can introduce tx-tcp parameter to decouple TCP and TSO > in a separate patch and it should save the port number to > tx_l4_dst_port variable. > I simply skipped it for now since additoin of not used and > not tested functionality is not good. > If you think that we really need tx-tcp, I'll add it in v2. > Please, confirm. > I think better to separate them. But I didn't get why it adds not tested functionality? >> >> [1] >> doc/guides/testpmd_app_ug/run_app.rst >> >> [2] >> printf("  --tx-udp=src[,dst]: UDP ports in Tx-only mode\n"); >> >>> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h >>> index acdb7e855d..30915bd84b 100644 >>> --- a/app/test-pmd/testpmd.h >>> +++ b/app/test-pmd/testpmd.h >>> @@ -619,8 +619,8 @@ extern int8_t tx_pthresh; >>>   extern int8_t tx_hthresh; >>>   extern int8_t tx_wthresh; >>> -extern uint16_t tx_udp_src_port; >>> -extern uint16_t tx_udp_dst_port; >>> +extern uint16_t tx_l4_src_port; >>> +extern uint16_t tx_l4_dst_port; >>>   extern uint32_t tx_ip_src_addr; >>>   extern uint32_t tx_ip_dst_addr; >>> diff --git a/app/test-pmd/txonly.c b/app/test-pmd/txonly.c >>> index 021624952d..44bda752bc 100644 >>> --- a/app/test-pmd/txonly.c >>> +++ b/app/test-pmd/txonly.c >>> @@ -46,8 +46,8 @@ struct tx_timestamp { >>>   }; >>>   /* use RFC863 Discard Protocol */ >>> -uint16_t tx_udp_src_port = 9; >>> -uint16_t tx_udp_dst_port = 9; >>> +uint16_t tx_l4_src_port = 9; >>> +uint16_t tx_l4_dst_port = 9; >>>   /* use RFC5735 / RFC2544 reserved network test addresses */ >>>   uint32_t tx_ip_src_addr = (198U << 24) | (18 << 16) | (0 << 8) | 1; >>> @@ -57,7 +57,10 @@ uint32_t tx_ip_dst_addr = (198U << 24) | (18 << >>> 16) | (0 << 8) | 2; >>>   static struct rte_ipv4_hdr pkt_ip_hdr; /**< IP header of >>> transmitted packets. */ >>>   RTE_DEFINE_PER_LCORE(uint8_t, _ip_var); /**< IP address variation */ >>> -static struct rte_udp_hdr pkt_udp_hdr; /**< UDP header of tx >>> packets. */ >>> + >>> +static union pkt_l4_hdr_t { >>> +    struct rte_udp_hdr udp;    /**< UDP header of tx packets. */ >>> +} pkt_l4_hdr; /**< Layer 4 header of tx packets. */ >>>   static uint64_t timestamp_mask; /**< Timestamp dynamic flag mask */ >>>   static int32_t timestamp_off; /**< Timestamp dynamic field offset */ >>> @@ -102,22 +105,30 @@ copy_buf_to_pkt(void* buf, unsigned len, struct >>> rte_mbuf *pkt, unsigned offset) >>>   } >>>   static void >>> -setup_pkt_udp_ip_headers(struct rte_ipv4_hdr *ip_hdr, >>> -             struct rte_udp_hdr *udp_hdr, >>> -             uint16_t pkt_data_len) >>> +setup_pkt_l4_ip_headers(uint8_t ip_proto, struct rte_ipv4_hdr *ip_hdr, >>> +            union pkt_l4_hdr_t *l4_hdr, uint16_t pkt_data_len) >>>   { >>>       uint16_t *ptr16; >>>       uint32_t ip_cksum; >>>       uint16_t pkt_len; >>> +    struct rte_udp_hdr *udp_hdr; >>> -    /* >>> -     * Initialize UDP header. >>> -     */ >>> -    pkt_len = (uint16_t) (pkt_data_len + sizeof(struct rte_udp_hdr)); >>> -    udp_hdr->src_port = rte_cpu_to_be_16(tx_udp_src_port); >>> -    udp_hdr->dst_port = rte_cpu_to_be_16(tx_udp_dst_port); >>> -    udp_hdr->dgram_len      = RTE_CPU_TO_BE_16(pkt_len); >>> -    udp_hdr->dgram_cksum    = 0; /* No UDP checksum. */ >>> +    switch (ip_proto) { >>> +    case IPPROTO_UDP: >>> +        /* >>> +         * Initialize UDP header. >>> +         */ >>> +        pkt_len = (uint16_t)(pkt_data_len + sizeof(struct >>> rte_udp_hdr)); >>> +        udp_hdr = &l4_hdr->udp; >>> +        udp_hdr->src_port = rte_cpu_to_be_16(tx_l4_src_port); >>> +        udp_hdr->dst_port = rte_cpu_to_be_16(tx_l4_dst_port); >>> +        udp_hdr->dgram_len      = RTE_CPU_TO_BE_16(pkt_len); >>> +        udp_hdr->dgram_cksum    = 0; /* No UDP checksum. */ >>> +        break; >>> +    default: >>> +        pkt_len = pkt_data_len; >>> +        break; >>> +    } >>>       /* >>>        * Initialize IP header. >>> @@ -127,7 +138,7 @@ setup_pkt_udp_ip_headers(struct rte_ipv4_hdr >>> *ip_hdr, >>>       ip_hdr->type_of_service   = 0; >>>       ip_hdr->fragment_offset = 0; >>>       ip_hdr->time_to_live   = IP_DEFTTL; >>> -    ip_hdr->next_proto_id = IPPROTO_UDP; >>> +    ip_hdr->next_proto_id = ip_proto; >>>       ip_hdr->packet_id = 0; >>>       ip_hdr->total_length   = RTE_CPU_TO_BE_16(pkt_len); >>>       ip_hdr->src_addr = rte_cpu_to_be_32(tx_ip_src_addr); >>> @@ -162,27 +173,32 @@ update_pkt_header(struct rte_mbuf *pkt, >>> uint32_t total_pkt_len) >>>   { >>>       struct rte_ipv4_hdr *ip_hdr; >>>       struct rte_udp_hdr *udp_hdr; >>> -    uint16_t pkt_data_len; >>> +    uint16_t ip_data_len; >>>       uint16_t pkt_len; >>> -    pkt_data_len = (uint16_t) (total_pkt_len - ( >>> +    ip_data_len = (uint16_t) (total_pkt_len - ( >>>                       sizeof(struct rte_ether_hdr) + >>> -                    sizeof(struct rte_ipv4_hdr) + >>> -                    sizeof(struct rte_udp_hdr))); >>> -    /* update UDP packet length */ >>> -    udp_hdr = rte_pktmbuf_mtod_offset(pkt, struct rte_udp_hdr *, >>> -                sizeof(struct rte_ether_hdr) + >>> -                sizeof(struct rte_ipv4_hdr)); >>> -    pkt_len = (uint16_t) (pkt_data_len + sizeof(struct rte_udp_hdr)); >>> -    udp_hdr->dgram_len = RTE_CPU_TO_BE_16(pkt_len); >>> +                    sizeof(struct rte_ipv4_hdr))); >>>       /* update IP packet length and checksum */ >>>       ip_hdr = rte_pktmbuf_mtod_offset(pkt, struct rte_ipv4_hdr *, >>>                   sizeof(struct rte_ether_hdr)); >>>       ip_hdr->hdr_checksum = 0; >>> -    pkt_len = (uint16_t) (pkt_len + sizeof(struct rte_ipv4_hdr)); >>> +    pkt_len = (uint16_t) (ip_data_len + sizeof(struct rte_ipv4_hdr)); >>>       ip_hdr->total_length = RTE_CPU_TO_BE_16(pkt_len); >>>       ip_hdr->hdr_checksum = rte_ipv4_cksum(ip_hdr); >>> + >>> +    switch (ip_hdr->next_proto_id) { >>> +    case IPPROTO_UDP: >>> +        /* update UDP packet length */ >>> +        udp_hdr = rte_pktmbuf_mtod_offset(pkt, struct rte_udp_hdr *, >>> +                    sizeof(struct rte_ether_hdr) + >>> +                    sizeof(struct rte_ipv4_hdr)); >>> +        udp_hdr->dgram_len = RTE_CPU_TO_BE_16(ip_data_len); >>> +        break; >>> +    default: >>> +        break; >>> +    } >>>   } >>>   static inline bool >>> @@ -193,8 +209,9 @@ pkt_burst_prepare(struct rte_mbuf *pkt, struct >>> rte_mempool *mbp, >>>   { >>>       struct rte_mbuf *pkt_segs[RTE_MAX_SEGS_PER_PKT]; >>>       struct rte_mbuf *pkt_seg; >>> -    uint32_t nb_segs, pkt_len; >>> +    uint32_t nb_segs, pkt_len, l4_hdr_size; >>>       uint8_t i; >>> +    struct rte_ipv4_hdr *ip_hdr; >>>       if (unlikely(tx_pkt_split == TX_PKT_SPLIT_RND)) >>>           nb_segs = rte_rand() % tx_pkt_nb_segs + 1; >>> @@ -230,14 +247,14 @@ pkt_burst_prepare(struct rte_mbuf *pkt, struct >>> rte_mempool *mbp, >>>       copy_buf_to_pkt(eth_hdr, sizeof(*eth_hdr), pkt, 0); >>>       copy_buf_to_pkt(&pkt_ip_hdr, sizeof(pkt_ip_hdr), pkt, >>>               sizeof(struct rte_ether_hdr)); >>> + >>> +    ip_hdr = rte_pktmbuf_mtod_offset(pkt, >>> +            struct rte_ipv4_hdr *, >>> +            sizeof(struct rte_ether_hdr)); >>>       if (txonly_multi_flow) { >>>           uint8_t  ip_var = RTE_PER_LCORE(_ip_var); >>> -        struct rte_ipv4_hdr *ip_hdr; >>>           uint32_t addr; >>> -        ip_hdr = rte_pktmbuf_mtod_offset(pkt, >>> -                struct rte_ipv4_hdr *, >>> -                sizeof(struct rte_ether_hdr)); >>>           /* >>>            * Generate multiple flows by varying IP src addr. This >>>            * enables packets are well distributed by RSS in >>> @@ -249,9 +266,17 @@ pkt_burst_prepare(struct rte_mbuf *pkt, struct >>> rte_mempool *mbp, >>>           ip_hdr->src_addr = rte_cpu_to_be_32(addr); >>>           RTE_PER_LCORE(_ip_var) = ip_var; >>>       } >>> -    copy_buf_to_pkt(&pkt_udp_hdr, sizeof(pkt_udp_hdr), pkt, >>> -            sizeof(struct rte_ether_hdr) + >>> -            sizeof(struct rte_ipv4_hdr)); >>> +    switch (ip_hdr->next_proto_id) { >>> +    case IPPROTO_UDP: >>> +        copy_buf_to_pkt(&pkt_l4_hdr.udp, sizeof(pkt_l4_hdr.udp), pkt, >>> +                sizeof(struct rte_ether_hdr) + >>> +                sizeof(struct rte_ipv4_hdr)); >>> +        l4_hdr_size = sizeof(pkt_l4_hdr.udp); >>> +        break; >>> +    default: >>> +        l4_hdr_size = 0; >>> +        break; >>> +    } >>>       if (unlikely(tx_pkt_split == TX_PKT_SPLIT_RND) || >>> txonly_multi_flow) >>>           update_pkt_header(pkt, pkt_len); >>> @@ -308,7 +333,7 @@ pkt_burst_prepare(struct rte_mbuf *pkt, struct >>> rte_mempool *mbp, >>>           copy_buf_to_pkt(×tamp_mark, sizeof(timestamp_mark), pkt, >>>               sizeof(struct rte_ether_hdr) + >>>               sizeof(struct rte_ipv4_hdr) + >>> -            sizeof(pkt_udp_hdr)); >>> +            l4_hdr_size); >>>       } >>>       /* >>>        * Complete first mbuf of packet and append it to the >>> @@ -449,7 +474,8 @@ tx_only_begin(portid_t pi) >>>           return -EINVAL; >>>       } >>> -    setup_pkt_udp_ip_headers(&pkt_ip_hdr, &pkt_udp_hdr, pkt_data_len); >>> +    setup_pkt_l4_ip_headers(IPPROTO_UDP, &pkt_ip_hdr, &pkt_l4_hdr, >>> +                pkt_data_len); >> >> 'pkt_data_len' is calculated as following, it is correct for this >> patch, but it will be wrong in next patch because UDP header size is >> used in calculation. >> Need to fix this code, either in this patch and make it protocol >> agnostic, or in next patch with protocol check. > > Again, the goal of the patch is to do cosmetic changes to > prepare to add new functionality in follow up patches. > The patch does not add TCP support. So, I don't understand > how it can be improved here. So, I'll fix the problem in the > next patch when I have TCP support and corresponding branching. > Thanks a lot for the catch. > >> >> ` >>          pkt_hdr_len = (uint16_t)(sizeof(struct rte_ether_hdr) + >>                                   sizeof(struct rte_ipv4_hdr) + >>                                   sizeof(struct rte_udp_hdr)); >>          pkt_data_len = tx_pkt_length - pkt_hdr_len; >> ` >> >