From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ferruh Yigit Subject: Re: [PATCH v4 1/3] app/testpmd: move dumping packets to a separate function Date: Tue, 16 Oct 2018 14:11:29 +0100 Message-ID: <5d5af971-e919-cbd8-18dc-05a14fc08aa6@intel.com> References: <1537793304-27883-1-git-send-email-rasland@mellanox.com> <1538897848-1693-1-git-send-email-rasland@mellanox.com> <860e6ca0-fc54-2ecf-538b-56a96fbf8176@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: Thomas Monjalon , "dev@dpdk.org" , Shahaf Shuler , "Xueming(Steven) Li" , Ori Kam , "jerin.jacob@caviumnetworks.com" , "david.marchand@6wind.com" , "bernard.iremonger@intel.com" To: Raslan Darawsheh , "jingjing.wu@intel.com" Return-path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id DBB7A5F3B for ; Tue, 16 Oct 2018 15:11:33 +0200 (CEST) In-Reply-To: Content-Language: en-US List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 10/16/2018 11:10 AM, Raslan Darawsheh wrote: > Hi Ferruh, > > PSB. > > Kindest regards, > Raslan Darawsheh > >> -----Original Message----- >> From: Ferruh Yigit >> Sent: Thursday, October 11, 2018 5:56 PM >> To: Raslan Darawsheh ; jingjing.wu@intel.com >> Cc: Thomas Monjalon ; dev@dpdk.org; Shahaf >> Shuler ; Xueming(Steven) Li >> ; Ori Kam ; >> jerin.jacob@caviumnetworks.com; david.marchand@6wind.com; >> bernard.iremonger@intel.com >> Subject: Re: [dpdk-dev] [PATCH v4 1/3] app/testpmd: move dumping >> packets to a separate function >> >> On 10/7/2018 8:38 AM, Raslan Darawsheh wrote: >>> verbosity for the received/sent packets is needed in all of the >>> forwarding engines so moving it to be in a separate function >> >> +1, this is good idea. >> >>> --- >>> changes in v3: >>> - add util.c in the mason.build file >>> - restore missing check for ol_flags & PKT_RX_RSS_HASH. >>> - add local variables for rte_be_to_cpu to avoid long >>> lines. >>> >>> changes in v4: >>> - add missing l3 and l4 checks >>> --- >>> >>> Signed-off-by: Raslan Darawsheh >> >> <...> >> >>> @@ -0,0 +1,150 @@ >>> +/* SPDX-License-Identifie.r: BSD-3-Clause >>> + * Copyri.ght(c) 2010-2018 Mellanox technology. >> >> It can be good to keep original Copyright owner when moving some code >> from one file another. > I added a new copy right since it contains some new functionality as well not only the original one. What is the new functionality added to "dump_pkt_burst()" ? Overall there is a new feature but not into this file. I think better to keep the original copyright when you are copying something from one file to another and append your copyright when added a significant contribution to that file. >> >> <...> >> >>> + sw_packet_type = rte_net_get_ptype(mb, &hdr_lens, >>> + RTE_PTYPE_ALL_MASK); >> >> DPDK coding convention requires a single tab in next line, instead of alligning >> to the parenthesis. > Will send a new version with these fixed. >> >> <...> >> >>> + /* Do not support ipv4 option field */ >>> + if (RTE_ETH_IS_IPV4_HDR(packet_type)) { >>> + l3_len = sizeof(struct ipv4_hdr); >>> + ipv4_hdr = rte_pktmbuf_mtod_offset(mb, >>> + struct ipv4_hdr *, >>> + l2_len); >>> + l4_proto = ipv4_hdr->next_proto_id; >> >> The syntax broken here. > This is the same as present in rxonly file nothing changed >> >> <...> >> >>> + } >>> + printf(" - %s queue=0x%x", is_rx ? "Receive" : "Send", >>> + (unsigned int) queue); >> >> Isn't this same information with initial header line, does this add any >> extra/new information? > > This is as of per packet, since you can get more than one packet printed when you have RSS the first one will be a print for the entire burst, > and then you'll get this print for each packet printed. >