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 8165DA00C3; Wed, 20 Apr 2022 19:14:26 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 2389040C35; Wed, 20 Apr 2022 19:14:26 +0200 (CEST) Received: from NAM10-DM6-obe.outbound.protection.outlook.com (mail-dm6nam10on2052.outbound.protection.outlook.com [40.107.93.52]) by mails.dpdk.org (Postfix) with ESMTP id 957E7406A2 for ; Wed, 20 Apr 2022 19:14:24 +0200 (CEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=YgNp5pEv/KGSJWhEpInubCPduD6NSi+P+cJWuhlFbgajTkqXlULl8B/SqhFqxYZcnwUDT73oHI1zB4QdPo+zA7Q5wQSk5wNzG9vRws+siqQBZwU9rgoFeRYeozfIMmi867vUpX7gIczFWzodKhChKRDNAfFFk7yXlvhI6knWQv897d2GHzVKpgQvwwdyrb/XhHJQteZpbpVrIG40Zi0c2wwKd78haVFg3OA0mrHa2IqVjVp02WoTlj+tbPAgaIa8+qOEL/FbRZs5LWwh9zEwGxHNjchUx4GskuZ4SE9UV3J7xzuexGb/adz2L5GRW8ZmNhjF71TqvHPVLo0W2fyn5A== 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=2v8hD5Y7Lo83dj5lLlJ1B3xTCmYSM3MFjjlYgDgOm+M=; b=XC6m6Ktka2xUCoB2uTjrSL1xZpSdXnqJbjW++MVBHafohm11668tmHcHW1L3qI6c1B2dxpXmfFmNYAQJ+ggUrzrOKRnbAbnr8oe1HqLofNUzC6Iz31871K/TYkUgcevnB78c21Wh1kvieMDPYit/gseJjEEyau/OdVQe/A4uaxsiodoOaejhHGi6KknTX2LcRJHzd67SWQOVPUwn8BSoTslZq2rWd3dSWOilQHncCRvXhT11FTeW4Yei/0qSo2xdGTQZ5FhA1Re/dQ/Fkmdnx4fn6uW6iUWpk+471jqF0N2Oxlaov/zE+PEf+y2ppVFRUFWWo1HNY2ryblV5ZvjtJw== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass (sender ip is 149.199.80.198) smtp.rcpttodomain=tom.com smtp.mailfrom=xilinx.com; dmarc=pass (p=none sp=none pct=100) action=none header.from=xilinx.com; dkim=none (message not signed); arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=xilinx.onmicrosoft.com; s=selector2-xilinx-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=2v8hD5Y7Lo83dj5lLlJ1B3xTCmYSM3MFjjlYgDgOm+M=; b=B+4ccEqJXPe76lrnHDAFkMfYY7FQnvsKtX3LKOpW5W3AeDrg37WRZscZ549BCMYS8+6FZhButZfUBL5On9ac6suAazjXiVOnFwQDrk13nidBHpmAeKCupG9AK18053zKhnTioI10427QM7ibnvhHw8I8t3dZ1y9U15uVfjQY3pI= Received: from BN8PR12CA0013.namprd12.prod.outlook.com (2603:10b6:408:60::26) by BL0PR02MB4371.namprd02.prod.outlook.com (2603:10b6:208:44::26) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5186.13; Wed, 20 Apr 2022 17:14:21 +0000 Received: from BN1NAM02FT030.eop-nam02.prod.protection.outlook.com (2603:10b6:408:60:cafe::16) by BN8PR12CA0013.outlook.office365.com (2603:10b6:408:60::26) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5186.13 via Frontend Transport; Wed, 20 Apr 2022 17:14:21 +0000 X-MS-Exchange-Authentication-Results: spf=pass (sender IP is 149.199.80.198) smtp.mailfrom=xilinx.com; dkim=none (message not signed) header.d=none;dmarc=pass action=none header.from=xilinx.com; Received-SPF: Pass (protection.outlook.com: domain of xilinx.com designates 149.199.80.198 as permitted sender) receiver=protection.outlook.com; client-ip=149.199.80.198; helo=xir-pvapexch01.xlnx.xilinx.com; Received: from xir-pvapexch01.xlnx.xilinx.com (149.199.80.198) by BN1NAM02FT030.mail.protection.outlook.com (10.13.2.144) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.20.5186.14 via Frontend Transport; Wed, 20 Apr 2022 17:14:20 +0000 Received: from xir-pvapexch01.xlnx.xilinx.com (172.21.17.15) by xir-pvapexch01.xlnx.xilinx.com (172.21.17.15) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2176.14; Wed, 20 Apr 2022 18:14:19 +0100 Received: from smtp.xilinx.com (172.21.105.197) by xir-pvapexch01.xlnx.xilinx.com (172.21.17.15) with Microsoft SMTP Server id 15.1.2176.14 via Frontend Transport; Wed, 20 Apr 2022 18:14:19 +0100 Envelope-to: laitianli@tom.com, dev@dpdk.org, bruce.richardson@intel.com Received: from [10.71.118.43] (port=22092) by smtp.xilinx.com with esmtp (Exim 4.90) (envelope-from ) id 1nhDuF-0003J4-Nr; Wed, 20 Apr 2022 18:14:19 +0100 Message-ID: Date: Wed, 20 Apr 2022 18:14:18 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.8.1 Subject: Re: [PATCH] net/pcap: add snaplen argument Content-Language: en-US To: Tianli Lai , CC: Bruce Richardson References: <20220313112638.3945-1-laitianli@tom.com> From: Ferruh Yigit In-Reply-To: <20220313112638.3945-1-laitianli@tom.com> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 7bit X-EOPAttributedMessage: 0 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: b9a52faa-66fc-4dba-949b-08da22f135c0 X-MS-TrafficTypeDiagnostic: BL0PR02MB4371:EE_ X-Microsoft-Antispam-PRVS: X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: XWRZ0a9gG6zKoxzb3cp2OIGuYu1OW7DOWUus9EToaObX70WOMOMGLqh2uJYcnlakf/sh8rjch5OW8Om6WWWR6hY68kxM+UwmDaxyWbvcc/mYpugCdVWmzkU1ScP6wGjj/0N9jb+o0zMpM7vvvXQdc0HfpcaAm9aiuSpWOz6T3USNa/h2ES1O+/iE5be7kcJuFMQG9Eut8w0FUsKO+yR7aL4X3LRSJY/B/vhfOf4aNf19C1mHWLOX/yETMbCE6h5t1ZecHLW0AK0aK+CjbLW14BCEPUcI8fp5m9Ew7CTAy7WTDYQEd9Lzn4R+01SRij0zeGlgUFac7JZQUZ5pUEriL3HNOK9Yka/bBr0TyRpvLDfRsOiRoj7WMIhaLwxKWEzcVGsu7fRua0rHdBpktSbNv+3Vt1Ofqy0vLx3GoBoJw3KK5EhfgAH2RFzCG177lHyJJz7bRSX/gWBmmYzWRDcDzoXITQphJaJpAxMF27SJDwE5VYWcs54dvArmtvQvdLZ9iYXOwMj53Ii4PpAHWH3Do1ghewj1l0Bvrp4uWYCYgfSM9Zsh3gwUOnPPQGsRCJ6xb2YNdbQp2ECIALuvbD/byRIMX2+oErRyOXcDr68wC8W6nLokr2ZFCZyRzhZKZ9pUnsc68lcx51Ea0KYzB8oa4A8Xih3xRTsIhwuqKghhcXrsO886neslnwiwyfTdacDpUPqB+SiuHkhp/g7hWebzIL4/N1jbhSEarnQyiGYFqDM= X-Forefront-Antispam-Report: CIP:149.199.80.198; CTRY:IE; LANG:en; SCL:1; SRV:; IPV:CAL; SFV:NSPM; H:xir-pvapexch01.xlnx.xilinx.com; PTR:unknown-80-198.xilinx.com; CAT:NONE; SFS:(13230001)(4636009)(36840700001)(46966006)(40470700004)(7636003)(70586007)(426003)(47076005)(110136005)(82310400005)(70206006)(336012)(36860700001)(316002)(356005)(36756003)(4326008)(31686004)(8676002)(508600001)(8936002)(83380400001)(26005)(9786002)(186003)(5660300002)(53546011)(40460700003)(31696002)(2616005)(44832011)(2906002)(50156003)(43740500002); DIR:OUT; SFP:1101; X-OriginatorOrg: xilinx.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Apr 2022 17:14:20.8904 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: b9a52faa-66fc-4dba-949b-08da22f135c0 X-MS-Exchange-CrossTenant-Id: 657af505-d5df-48d0-8300-c31994686c5c X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=657af505-d5df-48d0-8300-c31994686c5c; Ip=[149.199.80.198]; Helo=[xir-pvapexch01.xlnx.xilinx.com] X-MS-Exchange-CrossTenant-AuthSource: BN1NAM02FT030.eop-nam02.prod.protection.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: BL0PR02MB4371 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 3/13/2022 11:26 AM, Tianli Lai wrote: > snaplen argument would set the length of each packet > that will save to pcap file. > > Signed-off-by: Tianli Lai > --- > doc/guides/nics/pcap_ring.rst | 16 +++++++++- > drivers/net/pcap/pcap_ethdev.c | 58 +++++++++++++++++++++++++--------- > 2 files changed, 58 insertions(+), 16 deletions(-) > > diff --git a/doc/guides/nics/pcap_ring.rst b/doc/guides/nics/pcap_ring.rst > index acb1f00e30..8fb309212f 100644 > --- a/doc/guides/nics/pcap_ring.rst > +++ b/doc/guides/nics/pcap_ring.rst > @@ -94,6 +94,13 @@ The different stream types are: > > iface=eth0 > > +* snaplen: Defines the length of one packet that will save pcap file. > + the length of each packets can be snap by set this value. > + this value between 64 and 65535. > + > + snaplen= > + > + Hi Tianli, This section is "Device Streams", it documents main Rx/Tx streams, 'snaplen' can be a detail for this section, what do you think to drop it form here, it is already documented below? > Runtime Config Options > ^^^^^^^^^^^^^^^^^^^^^^ > > @@ -132,6 +139,13 @@ Runtime Config Options > > In this case, one dummy rx queue is created for each tx queue argument passed > > + - Receive packets on Tx and set the length of packet, for example:: > + What about focusing on the argument description, something like: - `snaplen`: Set the length of the saved packet size, for example:: > + --vdev 'net_pcap0,tx_pcap=file_tx.pcap,snaplen=100' > + > +In this case, one dummy rx queue is created for each tx queue argument passed and the length of each packet would not over 100 byte. > + Don't focus on other parameters (like dummy Rx), please only explain what 'snaplen' does. > + > Examples of Usage > ^^^^^^^^^^^^^^^^^ > > @@ -140,7 +154,7 @@ Read packets from one pcap file and write them to another: > .. code-block:: console > > .//app/dpdk-testpmd -l 0-3 -n 4 \ > - --vdev 'net_pcap0,rx_pcap=file_rx.pcap,tx_pcap=file_tx.pcap' \ > + --vdev 'net_pcap0,rx_pcap=file_rx.pcap,tx_pcap=file_tx.pcap,snaplen=100' \ > -- --port-topology=chained > Can you add a new sample with 'snaplen', instead of attaching to existing one? > Read packets from a network interface and write them to a pcap file: > diff --git a/drivers/net/pcap/pcap_ethdev.c b/drivers/net/pcap/pcap_ethdev.c > index ec29fd6bc5..6e111518e7 100644 > --- a/drivers/net/pcap/pcap_ethdev.c > +++ b/drivers/net/pcap/pcap_ethdev.c > @@ -33,10 +33,12 @@ > #define ETH_PCAP_IFACE_ARG "iface" > #define ETH_PCAP_PHY_MAC_ARG "phy_mac" > #define ETH_PCAP_INFINITE_RX_ARG "infinite_rx" > +#define ETH_PCAP_SNAPLEN_ARG "snaplen" > > #define ETH_PCAP_ARG_MAXLEN 64 > > #define RTE_PMD_PCAP_MAX_QUEUES 16 > +#define RTE_PCAP_MIN_SNAPLEN 64 > > static char errbuf[PCAP_ERRBUF_SIZE]; > static struct timespec start_time; > @@ -93,6 +95,7 @@ struct pmd_internals { > int single_iface; > int phy_mac; > unsigned int infinite_rx; > + int snaplen; > }; > > struct pmd_process_private { > @@ -121,6 +124,14 @@ struct pmd_devargs_all { > unsigned int is_rx_pcap; > unsigned int is_rx_iface; > unsigned int infinite_rx; > + int snaplen; > +}; > +struct pmd_devargs_all devargs_all = { > + .single_iface = 0, > + .is_tx_pcap = 0, > + .is_tx_iface = 0, > + .infinite_rx = 0, > + .snaplen = RTE_ETH_PCAP_SNAPSHOT_LEN, > }; > 'devargs_all' is to store user provided devargs per port, making it a global variable is wrong, it causes problems where there are multiple ports, please leave it where it is. > static const char *valid_arguments[] = { > @@ -132,6 +143,7 @@ static const char *valid_arguments[] = { > ETH_PCAP_IFACE_ARG, > ETH_PCAP_PHY_MAC_ARG, > ETH_PCAP_INFINITE_RX_ARG, > + ETH_PCAP_SNAPLEN_ARG, > NULL > }; > > @@ -384,8 +396,10 @@ eth_pcap_tx_dumper(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts) > pcap_dumper_t *dumper; > unsigned char temp_data[RTE_ETH_PCAP_SNAPLEN]; > size_t len, caplen; > - > - pp = rte_eth_devices[dumper_q->port_id].process_private; > + struct pmd_internals *internals; > + struct rte_eth_dev *dev = &rte_eth_devices[dumper_q->port_id]; > + internals = dev->data->dev_private; > + pp = dev->process_private; > dumper = pp->tx_dumper[dumper_q->queue_id]; > > if (dumper == NULL || nb_pkts == 0) > @@ -402,6 +416,8 @@ eth_pcap_tx_dumper(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts) > } > > calculate_timestamp(&header.ts); > + if ((typeof(internals->snaplen))caplen > internals->snaplen) > + caplen = internals->snaplen; When opening pcap interface 'snaplen' is already provided, what happens if 'caplen' is not set correctly here, won't pcap library limit the packets to 'snaplen' already? > header.len = len; > header.caplen = caplen; > /* rte_pktmbuf_read() returns a pointer to the data directly > @@ -536,7 +552,7 @@ open_single_iface(const char *iface, pcap_t **pcap) > } > > static int > -open_single_tx_pcap(const char *pcap_filename, pcap_dumper_t **dumper) > +open_single_tx_pcap(const char *pcap_filename, pcap_dumper_t **dumper, int snaplen) > { > pcap_t *tx_pcap; > > @@ -546,7 +562,7 @@ open_single_tx_pcap(const char *pcap_filename, pcap_dumper_t **dumper) > * pcap holder. > */ > tx_pcap = pcap_open_dead_with_tstamp_precision(DLT_EN10MB, > - RTE_ETH_PCAP_SNAPSHOT_LEN, PCAP_TSTAMP_PRECISION_NANO); > + snaplen, PCAP_TSTAMP_PRECISION_NANO); > if (tx_pcap == NULL) { > PMD_LOG(ERR, "Couldn't create dead pcap"); > return -1; > @@ -627,7 +643,7 @@ eth_dev_start(struct rte_eth_dev *dev) > if (!pp->tx_dumper[i] && > strcmp(tx->type, ETH_PCAP_TX_PCAP_ARG) == 0) { > if (open_single_tx_pcap(tx->name, > - &pp->tx_dumper[i]) < 0) > + &pp->tx_dumper[i], internals->snaplen) < 0) > return -1; > } else if (!pp->tx_pcap[i] && > strcmp(tx->type, ETH_PCAP_TX_IFACE_ARG) == 0) { > @@ -1055,7 +1071,7 @@ open_tx_pcap(const char *key, const char *value, void *extra_args) > struct pmd_devargs *dumpers = extra_args; > pcap_dumper_t *dumper; > > - if (open_single_tx_pcap(pcap_filename, &dumper) < 0) > + if (open_single_tx_pcap(pcap_filename, &dumper, devargs_all.snaplen) < 0) Here 'devargs_all' is used directly since patch makes it global, but when it is not global anymore, perhaps you can consider to add 'snaplen' to "struct pmd_devargs" and get it to this function via it. > return -1; > > if (add_queue(dumpers, pcap_filename, key, NULL, dumper) < 0) { > @@ -1066,6 +1082,15 @@ open_tx_pcap(const char *key, const char *value, void *extra_args) > return 0; > } > > +static int > +parse_uint_value(const char *key __rte_unused, const char *value, void *extra_args) > +{ > + char *end; > + int *val = extra_args; > + *val = strtoul(value, &end, 10); Should check 'strtoul' return value and return error accordingly. > + return 0; > +} > + > /* > * Opens an interface for reading and writing > */ > @@ -1325,10 +1350,10 @@ eth_from_pcaps(struct rte_vdev_device *vdev, > int ret; > > ret = eth_from_pcaps_common(vdev, devargs_all, &internals, ð_dev); > - unrelated change > if (ret < 0) > return ret; > > + internals->snaplen = devargs_all->snaplen; > /* store weather we are using a single interface for rx/tx or not */ > internals->single_iface = single_iface; > > @@ -1404,13 +1429,6 @@ pmd_pcap_probe(struct rte_vdev_device *dev) > struct pmd_internals *internal; > int ret = 0; > > - struct pmd_devargs_all devargs_all = { > - .single_iface = 0, > - .is_tx_pcap = 0, > - .is_tx_iface = 0, > - .infinite_rx = 0, > - }; > - > name = rte_vdev_device_name(dev); > PMD_LOG(INFO, "Initializing pmd_pcap for %s", name); > > @@ -1444,6 +1462,16 @@ pmd_pcap_probe(struct rte_vdev_device *dev) > return -1; > } > > + if (rte_kvargs_count(kvlist, ETH_PCAP_SNAPLEN_ARG) == 1) { > + ret = rte_kvargs_process(kvlist, ETH_PCAP_SNAPLEN_ARG, > + &parse_uint_value, &devargs_all.snaplen); > + if (ret < 0) > + goto free_kvlist; Can you print an error log here? > + if (devargs_all.snaplen < RTE_PCAP_MIN_SNAPLEN || > + devargs_all.snaplen >= RTE_ETH_PCAP_SNAPSHOT_LEN) > + devargs_all.snaplen = RTE_PCAP_MIN_SNAPLEN; I think better to return an error here, with a log. > + } Can you please move parsing 'ETH_PCAP_SNAPLEN_ARG' argument below in the function, where Tx related parameters parsed. After 'devargs_all.is_tx_pcap' checks. And can you please define (and document in above documentation), when 'snaplen' argument is valid? Like can I use it with only 'iface=' argument? Or does it requires 'tx_pcap' to work? If so this should be checked in the code. > + > /* > * If iface argument is passed we open the NICs and use them for > * reading / writing > @@ -1593,7 +1621,7 @@ pmd_pcap_probe(struct rte_vdev_device *dev) > pp->tx_dumper[i] = dumpers.queue[i].dumper; > pp->tx_pcap[i] = dumpers.queue[i].pcap; > } > - > + internal->snaplen = devargs_all.snaplen; This is not required, 'internals' is shared between primary and secondary, if primary has this parameter, secondary can use it already. > eth_dev->process_private = pp; > eth_dev->rx_pkt_burst = eth_pcap_rx; > if (devargs_all.is_tx_pcap)