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 75E1FA0540; Mon, 30 May 2022 20:05:51 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 591DA40689; Mon, 30 May 2022 20:05:51 +0200 (CEST) Received: from NAM11-BN8-obe.outbound.protection.outlook.com (mail-bn8nam11on2048.outbound.protection.outlook.com [40.107.236.48]) by mails.dpdk.org (Postfix) with ESMTP id A2946400D6 for ; Mon, 30 May 2022 20:05:50 +0200 (CEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=NdZOUH2szso6hddgIwENss1mK8OSyAkErP5b6Dby5ZFPzHjQQ0HvModWiUJp438nCS1k4GvigPtziTB2eDt/S5De0QX599m4gC87dSyqFIWV2s6jCiHaEG93DsFmvgRQVGhbavJgIYt4/UuqqwBe9HrkfaEPVX87CY5DFm7DWXj3BmzpZBWXRmuZrVstrqzpZMKr3pE4JjTSThmCcVLlroLGozKcCcIdoFr2cWNaIpXNCuqhnc+bPPX0jtfpyUjwU5c2+0LlePGH4QHmOMwdsuKFehtP5fTCfTpIMnTjMZ/xfcC+KlsczMXovLci2p9EVhfJV3M5nIMAtuLWu/y+Qg== 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=/2+8eemf8p8JJHK2T5+k/JM6ss8qCQnxc5i+IY0310c=; b=cdirYSjWvypqJecMYhTrpLYlFxESE0OkZ1nHc6ENwPav7whRGA6TCzAgiJAITnRpC0KhAlyeYcS5PJ0Z5MvqWiwh2raxIGuSVM+JV2ZdZBOrWw6TJF0GpyV+fBIWtM1dBvt4+EDynPxRDD32LYf4Q8/4Enrti6QUAU08cV1l/ycO/fhMiWuxC0Eq7Ve8/YSC3Ta69ZpN6RMjrJyxo7wnqSGT9VhPuSV/+FtQkH7Qmjr6nZtI8UuWK3/jdN3p3fVPN6lOJSbemkmHWBHJQhjnbqNhiwo0qyCxGWxEhLfwd28cuI7+2StX7yA8snji4M4D1h6xpWEcb1jLAj5NMTDZ2A== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass (sender ip is 149.199.80.198) smtp.rcpttodomain=cgstowernetworks.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=/2+8eemf8p8JJHK2T5+k/JM6ss8qCQnxc5i+IY0310c=; b=QOammILzj1MHt/Laxe6scm3SLa6MXpzYmW4YJzhQ7gxw6Fv/NRQjCi5DDNBOfF/BVT5B8j5joJuVxpU66Uoh1Ft15xEuc0ks23U3yQGBlm1Iz551xDKCign6KtSK6nq3MuffCIwmr3SwZmwWChf7djp6za9DkiLOWehJRNy4PYQ= Received: from BN6PR22CA0027.namprd22.prod.outlook.com (2603:10b6:404:37::13) by DM5PR02MB2857.namprd02.prod.outlook.com (2603:10b6:3:10d::18) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5293.13; Mon, 30 May 2022 18:05:47 +0000 Received: from BN1NAM02FT059.eop-nam02.prod.protection.outlook.com (2603:10b6:404:37:cafe::a9) by BN6PR22CA0027.outlook.office365.com (2603:10b6:404:37::13) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5293.19 via Frontend Transport; Mon, 30 May 2022 18:05:47 +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-pvapexch02.xlnx.xilinx.com; pr=C Received: from xir-pvapexch02.xlnx.xilinx.com (149.199.80.198) by BN1NAM02FT059.mail.protection.outlook.com (10.13.2.167) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.20.5293.13 via Frontend Transport; Mon, 30 May 2022 18:05:47 +0000 Received: from xir-pvapexch02.xlnx.xilinx.com (172.21.17.17) by xir-pvapexch02.xlnx.xilinx.com (172.21.17.17) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2176.14; Mon, 30 May 2022 19:05:46 +0100 Received: from smtp.xilinx.com (172.21.105.197) by xir-pvapexch02.xlnx.xilinx.com (172.21.17.17) with Microsoft SMTP Server id 15.1.2176.14 via Frontend Transport; Mon, 30 May 2022 19:05:46 +0100 Envelope-to: ferruh.yigit@xilinx.com, ido@cgstowernetworks.com, stephen@networkplumber.org, dev@dpdk.org, laitianli@tom.com Received: from [10.71.117.148] (port=62964) by smtp.xilinx.com with esmtp (Exim 4.90) (envelope-from ) id 1nvjlx-0000nm-Ec; Mon, 30 May 2022 19:05:45 +0100 Message-ID: Date: Mon, 30 May 2022 19:05:44 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.9.1 Subject: Re: [PATCH v3] pcap: support MTU set Content-Language: en-US To: Ido Goshen , , CC: , Tianli Lai References: <20220317174347.110909-1-ido@cgstowernetworks.com> <20220530103647.35626-1-ido@cgstowernetworks.com> From: Ferruh Yigit In-Reply-To: <20220530103647.35626-1-ido@cgstowernetworks.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: f257fe5b-04f6-4f03-90ed-08da426705dc X-MS-TrafficTypeDiagnostic: DM5PR02MB2857: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: EzUJ4Sj9G6x0pjFwCkcPoa5GnFhTRuRqf/D0VngPpmL7Mgyt4gnsyQfxDlMZ41Pv+xE8uJj9KkZFYdk3qz3zb+lFpMH/EQs79qHjW0OxA7OwHnYiEhdmZT7TA4vwaDg3FYYK8AOSZWNoFDMS6xWeeylsyuvJpX6Wet/lSQimEYBxc9lPvU6sgyaNXjma3F8bqIS/zYTfutNPU26E9AHATsUTH1Ap1+GgVk7oG4LMhfojv32bWVhrWucvAiMBpHRY4bnlI595fk1iBrryo3BB5mjfbyx2f26esBIAsCZsBNkNxzximH4zJUsU8fW1agixwmKAXYoUuln2eaFlfCVBPifZ3RHPM24te4X8nR+SCOsd5k3QWkbV/tICV4DHy2zeJV2BRaRbC197d2D8NI+MaZJsui/+zLSlCAjrUIo9JVaLObDjqSI5rzeCGnih2AvKZut+/+m0avpGP4nXmkinPOjofjK68CEVtDWS2GnUpaIZG4bxl5dJ9462f0IfJEftnDqmF07dmBUNXwmXq1qb/3XLfJRIo601EtQVKIv19O5/P/Wp3IHnEBmnySizyjKUJwEiX4xK3oCIDE8ztMH7G5JwQKyknyyFAj4V9qYpCJujFMfadvTrLL18k8cEe0tsIXb3Xr9vrMav9Tdt2EzNaB6mamlTMMK+hh43/wCLiCqV9IaTQsn6OQyQChTY9ZgiBhVhzeVRCvUyhaIUvSRE/DrXv083VzjY35UtJ9BLi4HhWoe2sm5UAW6fCWUIgwSOkFgi+GvQyHeu3+zJL6Rd+pNsllyaMmHdKorP9BVEtyvJyrP8ZnrLDiBQGK6nM2iz X-Forefront-Antispam-Report: CIP:149.199.80.198; CTRY:IE; LANG:en; SCL:1; SRV:; IPV:CAL; SFV:NSPM; H:xir-pvapexch02.xlnx.xilinx.com; PTR:unknown-80-198.xilinx.com; CAT:NONE; SFS:(13230001)(4636009)(36840700001)(46966006)(40470700004)(5660300002)(186003)(31686004)(966005)(31696002)(2616005)(508600001)(426003)(336012)(82310400005)(47076005)(36860700001)(83380400001)(70586007)(36756003)(4326008)(70206006)(53546011)(8676002)(110136005)(54906003)(316002)(8936002)(9786002)(44832011)(2906002)(7636003)(356005)(40460700003)(26005)(50156003)(43740500002); DIR:OUT; SFP:1101; X-OriginatorOrg: xilinx.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 May 2022 18:05:47.2033 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: f257fe5b-04f6-4f03-90ed-08da426705dc 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-pvapexch02.xlnx.xilinx.com] X-MS-Exchange-CrossTenant-AuthSource: BN1NAM02FT059.eop-nam02.prod.protection.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM5PR02MB2857 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 5/30/2022 11:36 AM, Ido Goshen wrote: > Support rte_eth_dev_set_mtu by pcap vdevs > Enforce mtu on rx/tx > Still not sure about enforcing MTU on pcap, but please find comments on mechanical issues > Bugzilla ID: 961 > Signed-off-by: Ido Goshen > > --- > v3: > Preserve pcap behavior to support max size packets by default > alternative to v2 in order to limit the code change to pcap only and > avoid abi change. > Enforce mtu only in case rte_eth_dev_set_mtu was explicitly called. > > v2: > Preserve pcap behavior to support max size packets by default. > --- > drivers/net/pcap/pcap_ethdev.c | 44 +++++++++++++++++++++++++++++++--- > 1 file changed, 41 insertions(+), 3 deletions(-) > Is documentation needs to be updated as well? And what do you think to update release notes for this update? > diff --git a/drivers/net/pcap/pcap_ethdev.c b/drivers/net/pcap/pcap_ethdev.c > index ec29fd6bc5..2e7fff9579 100644 > --- a/drivers/net/pcap/pcap_ethdev.c > +++ b/drivers/net/pcap/pcap_ethdev.c > @@ -93,6 +93,7 @@ struct pmd_internals { > int single_iface; > int phy_mac; > unsigned int infinite_rx; > + int is_mtu_set; > }; > > struct pmd_process_private { > @@ -278,11 +279,13 @@ eth_pcap_rx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts) > const u_char *packet; > struct rte_mbuf *mbuf; > struct pcap_rx_queue *pcap_q = queue; > + struct rte_eth_dev *dev = &rte_eth_devices[pcap_q->port_id]; > + struct pmd_internals *internals = dev->data->dev_private; 'rte_eth_devices[]' needs to be used for 'process_private' but lets not tie it to access 'dev_private'. You can add "struct pmd_internals *" to the "struct pcap_rx_queue" & "struct pcap_tx_queue" structs for the access. Please check null PMD for sample. > uint16_t num_rx = 0; > uint32_t rx_bytes = 0; > pcap_t *pcap; > > - pp = rte_eth_devices[pcap_q->port_id].process_private; > + pp = dev->process_private; > pcap = pp->rx_pcap[pcap_q->queue_id]; > > if (unlikely(pcap == NULL || nb_pkts == 0)) > @@ -303,6 +306,13 @@ eth_pcap_rx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts) > break; > } > > + if (unlikely(header.caplen > dev->data->mtu) && > + internals->is_mtu_set) { > + pcap_q->rx_stat.err_pkts++; > + rte_pktmbuf_free(mbuf); > + break; > + } > + > if (header.caplen <= rte_pktmbuf_tailroom(mbuf)) { > /* pcap packet will fit in the mbuf, can copy it */ > rte_memcpy(rte_pktmbuf_mtod(mbuf, void *), packet, > @@ -378,6 +388,8 @@ eth_pcap_tx_dumper(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts) > struct rte_mbuf *mbuf; > struct pmd_process_private *pp; > struct pcap_tx_queue *dumper_q = queue; > + struct rte_eth_dev *dev = &rte_eth_devices[dumper_q->port_id]; > + struct pmd_internals *internals = dev->data->dev_private; > uint16_t num_tx = 0; > uint32_t tx_bytes = 0; > struct pcap_pkthdr header; > @@ -385,7 +397,7 @@ eth_pcap_tx_dumper(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts) > unsigned char temp_data[RTE_ETH_PCAP_SNAPLEN]; > size_t len, caplen; > > - pp = rte_eth_devices[dumper_q->port_id].process_private; > + pp = dev->process_private; > dumper = pp->tx_dumper[dumper_q->queue_id]; > > if (dumper == NULL || nb_pkts == 0) > @@ -396,6 +408,13 @@ eth_pcap_tx_dumper(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts) > for (i = 0; i < nb_pkts; i++) { > mbuf = bufs[i]; > len = caplen = rte_pktmbuf_pkt_len(mbuf); > + > + if (unlikely(len > dev->data->mtu) && > + internals->is_mtu_set) { It is possible to save only some part of the packet to the pcap file, please check snaplen patch [1], how MTU config should work with this feature? [1] https://patchwork.dpdk.org/project/dpdk/patch/20220313112638.3945-1-laitianli@tom.com/ > + rte_pktmbuf_free(mbuf); > + continue; Normally a PMD should not silently free a packet itself, it should return error and application will decide to free the packet or not. > + } > + > if (unlikely(!rte_pktmbuf_is_contiguous(mbuf) && > len > sizeof(temp_data))) { > caplen = sizeof(temp_data); > @@ -464,13 +483,15 @@ eth_pcap_tx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts) > struct rte_mbuf *mbuf; > struct pmd_process_private *pp; > struct pcap_tx_queue *tx_queue = queue; > + struct rte_eth_dev *dev = &rte_eth_devices[tx_queue->port_id]; > + struct pmd_internals *internals = dev->data->dev_private; > uint16_t num_tx = 0; > uint32_t tx_bytes = 0; > pcap_t *pcap; > unsigned char temp_data[RTE_ETH_PCAP_SNAPLEN]; > size_t len; > > - pp = rte_eth_devices[tx_queue->port_id].process_private; > + pp = dev->process_private; > pcap = pp->tx_pcap[tx_queue->queue_id]; > > if (unlikely(nb_pkts == 0 || pcap == NULL)) > @@ -479,6 +500,13 @@ eth_pcap_tx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts) > for (i = 0; i < nb_pkts; i++) { > mbuf = bufs[i]; > len = rte_pktmbuf_pkt_len(mbuf); > + > + if (unlikely(len > dev->data->mtu) && > + internals->is_mtu_set) { > + rte_pktmbuf_free(mbuf); > + continue; ditto > + } > + > if (unlikely(!rte_pktmbuf_is_contiguous(mbuf) && > len > sizeof(temp_data))) { > PMD_LOG(ERR, > @@ -807,6 +835,14 @@ eth_stats_reset(struct rte_eth_dev *dev) > return 0; > } > > +static int eth_mtu_set(struct rte_eth_dev *dev, uint16_t mtu) Please follow coding convention to have return type in a separate line: static int eth_mtu_set(struct rte_eth_dev *dev, uint16_t mtu) { > +{ > + PMD_LOG(INFO, "mtu set %s %u\n", dev->device->name, mtu); Can you please move the log after variable declerations. And can be good to capitilise MTU in the log. > + struct pmd_internals *internals = dev->data->dev_private; > + internals->is_mtu_set = 1; > + return 0; > +} > + > static inline void > infinite_rx_ring_free(struct rte_ring *pkts) > { > @@ -1004,6 +1040,7 @@ static const struct eth_dev_ops ops = { > .link_update = eth_link_update, > .stats_get = eth_stats_get, > .stats_reset = eth_stats_reset, > + .mtu_set = eth_mtu_set, > }; > > static int > @@ -1233,6 +1270,7 @@ pmd_init_internals(struct rte_vdev_device *vdev, > .addr_bytes = { 0x02, 0x70, 0x63, 0x61, 0x70, iface_idx++ } > }; > (*internals)->phy_mac = 0; > + (*internals)->is_mtu_set = 0; > data = (*eth_dev)->data; > data->nb_rx_queues = (uint16_t)nb_rx_queues; > data->nb_tx_queues = (uint16_t)nb_tx_queues;