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 2A351A0C43; Fri, 15 Oct 2021 20:15:03 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id AD10840041; Fri, 15 Oct 2021 20:15:02 +0200 (CEST) Received: from mail-pf1-f177.google.com (mail-pf1-f177.google.com [209.85.210.177]) by mails.dpdk.org (Postfix) with ESMTP id 608D04003C for ; Fri, 15 Oct 2021 20:15:01 +0200 (CEST) Received: by mail-pf1-f177.google.com with SMTP id d9so1854949pfl.6 for ; Fri, 15 Oct 2021 11:15:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20210112.gappssmtp.com; s=20210112; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=fcQUHpEBS7QftltfByFddFuGAuqpAglNRJnjCmNw05o=; b=iGOPL3Cww/py4av6wXxH6qe7uwE8r7e07zr1HCo1lkjUXjjZNh9w4M7RSsKSDqH7fd Qm2xfoFuXMZYLWSLzQvN7rvue+1l+Cx5RH4vYH3auGcS0j9C8rVMRYZypU2uwWDLV3OC UIdvTTHF0vyRHZTSPaWnHtkbx54x2bnn4OBD2+wQhR79d5ZJKmD/k7LyHvDRUyd9RWm4 shR9MpofwmH39zi1y+IaczA3HhHVH3fTvxRqZTOU/EXUheszbQfP1TUjOA5mvaUNW51x WnlPRjYfF2+eoK4OosnIp9mI22gjqDCWRXZ/RxqDkvUA3qqD/L+Bi67wu7b7l5ri6cV3 wRjQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=fcQUHpEBS7QftltfByFddFuGAuqpAglNRJnjCmNw05o=; b=iFHIceIdLb0my/kcoS6yr4fA/CKUAbD6hnA3ar0lUXIFnmkVtndiE9ivnklemW/9o+ tczkMwd7VwSL92nFZRaAPnWsztBeiJcL3CYA51XTYri/zZzxZS8LWpgMIr4L8lZf4W0n pjmIWiXy1i5gtH1MEiznyAUkpksdcgVFq6fav8q9Pl5sno73QT8X25YqnMidy69orwQq jHWSCgI7BrU3pK/uW/fOFzKEoYUUMlrQi9GfqwhG7lMW4LOIy1HDL2RIZhValvrQY8cD wH3G2ppc1RieXSwtUMWF37ooXhQFZGnL9z1KIWqQucrBwUgkFl3mQAPxwS5NjROELPGC LdsA== X-Gm-Message-State: AOAM532aE/+ahvgK4l/i4+LMV9418EWjArI89TffKAnTn+yehgb694HL 0SSko87LetNBT7lZieIovLnrjawceAtUBQ== X-Google-Smtp-Source: ABdhPJyfO8N5DhkCK/+cc3LHa1w+KKr44xYrK0pjOmOU4bhG495ToXFiNS1dw5mePaaqi0rJ84hVbA== X-Received: by 2002:aa7:88cb:0:b0:44d:4b3f:36c1 with SMTP id k11-20020aa788cb000000b0044d4b3f36c1mr12719007pff.76.1634321700361; Fri, 15 Oct 2021 11:15:00 -0700 (PDT) Received: from hermes.local (204-195-33-123.wavecable.com. [204.195.33.123]) by smtp.gmail.com with ESMTPSA id u24sm5455162pfm.81.2021.10.15.11.14.59 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 15 Oct 2021 11:15:00 -0700 (PDT) Date: Fri, 15 Oct 2021 11:14:56 -0700 From: Stephen Hemminger To: "Pattan, Reshma" Cc: "dev@dpdk.org" Message-ID: <20211015111456.1f0c55aa@hermes.local> In-Reply-To: References: <20210903004732.109023-1-stephen@networkplumber.org> <20211001162705.442298-1-stephen@networkplumber.org> <20211001162705.442298-3-stephen@networkplumber.org> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v12 02/12] librte_pcapng: add new library for writing pcapng files 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 Sender: "dev" On Fri, 15 Oct 2021 09:36:00 +0000 "Pattan, Reshma" wrote: > > -----Original Message----- > > From: dev On Behalf Of Stephen Hemminger > > See draft RFC > > https://www.ietf.org/id/draft-tuexen-opsawg-pcapng-03.html > > The page is not found. Might need to add new link I guess > > > +enum pcapng_interface_options { > > + PCAPNG_IFB_NAME = 2, > > + PCAPNG_IFB_DESCRIPTION, > > Can IFB(interface block) be replaced with IF(interface) only? But that's ok, upto u. > > > > + buf = calloc(1, len); > > + if (!buf) > > + return -1; > > How about returning -ENOMEM > > > + > > + hdr = (struct pcapng_section_header *)buf; > > + *hdr = (struct pcapng_section_header) { > > + .block_type = PCAPNG_SECTION_BLOCK, > > + .block_length = len, > > + .byte_order_magic = PCAPNG_BYTE_ORDER_MAGIC, > > + .major_version = PCAPNG_MAJOR_VERS, > > + .minor_version = PCAPNG_MINOR_VERS, > > + .section_length = UINT64_MAX, > > + }; > > + hdr->block_length = len; > > Why to assign block_len with len again? as it is already done few lines above. > > > + opt = pcapng_add_option(opt, PCAPNG_OPT_END, NULL, 0); > > Some comments around this code, about adding end of options at the end of options list would be helpful. Ok, but someone looking at this code should really look at the standard to see what the data format is. > > + > > +/* Write the PCAPNG section header at start of file */ static ssize_t > > :s/section header/ interface header? Good catch, copy/paste of comment. > > > +pcapng_interface_block(rte_pcapng_t *self, const char *if_name, > > + if (mac_addr) > > + len += pcapng_optlen(6); > > How about using RTE_ETHER_ADDR_LEN instead of 6 Fixing now, also merging pcapng_interface_block since only called one place. > > > +struct rte_mbuf * rte_pcapng_copy(uint16_t port_id, uint32_t queue, > > > +fail: > > + rte_pktmbuf_free(mc); > > > Freeing mc , would that take care of freeing up the additional byte prepended after mc creation? Mbuf are allocation unit, so the whole buffer goes. > > > + opt = pcapng_add_option(opt, PCAPNG_EPB_QUEUE, > > + &queue, sizeof(queue)); > > Don't we need to add end of options to the end of option list, like did in Interface block and section header block? It turns out that the reference (wireshark) does not. So did not do that to save space on the output file. > > > diff --git a/lib/pcapng/rte_pcapng.h b/lib/pcapng/rte_pcapng.h new file mode > > + * > > + * Packets to be captured are copied by rte_pcapng_mbuf() > > Do you mean by rte_pcapng_copy()? Good catch, function got renamed and comment not updated.