From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id F118BA00E6 for ; Tue, 16 Apr 2019 17:03:47 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id C5F431B4F2; Tue, 16 Apr 2019 17:03:47 +0200 (CEST) Received: from mail-pg1-f193.google.com (mail-pg1-f193.google.com [209.85.215.193]) by dpdk.org (Postfix) with ESMTP id CC7751B45A for ; Tue, 16 Apr 2019 17:03:45 +0200 (CEST) Received: by mail-pg1-f193.google.com with SMTP id i2so10469749pgj.11 for ; Tue, 16 Apr 2019 08:03:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=REKd0GdoiGGM8bsPo+yORwhq3rRv5nK+Ds4HvJENkA4=; b=eabtrgap7Cx12SBRqkkNbCnCPFYbv9EZzw2YV282IGeeR+nUM/Izhckl+ZrCKfaajh VIMWw1S/Mz2m/7jEgZPBegBQZDU9lksk0bwCSCEfOwgOHCCel9MS3zFomrp1W4GznlQb ePrhQPQe5rK7KLzviBTJYZwM5gd1477bqThrlba8obw8+y116cQmJ2NfY9nD88nwzeWj 89nAz4YVNs66tFfgQaAjs4TJtv3jG7Ju/Wmcb6ykePhtu76GKMVZFc/4xFbsPEoVCNqX HpNBqbJaLrqXRnlWoIzD8or1TtVVQeWyD9w3NBgHS1kXaxb0YHQNgaSmv9sid0unt9bm VZ6A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=REKd0GdoiGGM8bsPo+yORwhq3rRv5nK+Ds4HvJENkA4=; b=PTF5SYAL+TeAffRnnMTb1VdIoubMWAghWe50JxRo0Ti8Cy/Fw+CniL1AucOgCdAnfu /xJm45lXp8GRJpRjiWFoBno6+esEMpdopJu3qUXB57u3Y5S2JOOXt18Om4vKBfzcXkhh hQVLLzSps2ifAkECRNEuE8zXPVc4UWgBMexATb7ZMZJrKlSrfbqLDiCR7b7KaXF6khtO Moe+0VSfMfrfwXynguNAyrRPM8lnpChd7ewzqfSR+o8gLgmcScPnkpuTuhrkF9DziWBM 5NOtkFm3/XIWs66XT7D/2GKFrqmLm3VxFk8lup8W9dZiHvcscsoD264YjjSjDkoTlYSz 6pGQ== X-Gm-Message-State: APjAAAWnpJu96E7PQgMQGiRRDpbV3vFvF0fXawjyEZZu64R30KbA/HkY mwK0JcEHlw4A3uDWsR2QlCstgA== X-Google-Smtp-Source: APXvYqwwFLRyPIF9MvGmG/m4n28py/oPO3D9lU3eVz3DdaC9LSUR0tHscq1AjvaRHaLjgDEd7Eca2g== X-Received: by 2002:a63:e556:: with SMTP id z22mr75059506pgj.290.1555427020272; Tue, 16 Apr 2019 08:03:40 -0700 (PDT) Received: from xps13.lan (204-195-22-127.wavecable.com. [204.195.22.127]) by smtp.gmail.com with ESMTPSA id e1sm64132935pfn.187.2019.04.16.08.03.39 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 16 Apr 2019 08:03:40 -0700 (PDT) Date: Tue, 16 Apr 2019 08:03:36 -0700 From: Stephen Hemminger To: Bruce Richardson Cc: Ferruh Yigit , dev@dpdk.org, Chas Williams , "John W. Linville" Message-ID: <20190416080336.3b8e16a6@xps13.lan> In-Reply-To: <20190416094212.GA1865@bricha3-MOBL.ger.corp.intel.com> References: <20190408160419.7409-1-stephen@networkplumber.org> <20190408164112.12471-1-stephen@networkplumber.org> <7e03b000-f4d4-71c3-5978-8a8623d7ace5@intel.com> <20190412150833.63c41806@shemminger-XPS-13-9360> <3dff5ba9-3965-be8d-7bd5-d8504a66c130@intel.com> <20190416094212.GA1865@bricha3-MOBL.ger.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v2] net/af_packet: fix vlan_insert corruption X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 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" Message-ID: <20190416150336.ngqrI2htPiT6fscmYHKXgl4X2LEhQxJQ6RP1wq-xP5A@z> On Tue, 16 Apr 2019 10:42:13 +0100 Bruce Richardson wrote: > On Tue, Apr 16, 2019 at 10:37:07AM +0100, Ferruh Yigit wrote: > > On 4/12/2019 11:08 PM, Stephen Hemminger wrote: > > > On Fri, 12 Apr 2019 17:28:17 +0100 > > > Ferruh Yigit wrote: > > > > > >> On 4/8/2019 5:41 PM, Stephen Hemminger wrote: > > >>> If the af_packet transmit is sending a VLAN packet, > > >>> and the transmit path to the kernel os full, then it would > > >>> mismanage the outgoing mbuf. The original mbuf would end up > > >>> being freed twice, once by AF_PACKET PMD and once by caller. > > >> > > >> This comment is valid with your new patch [1] that updates 'rte_vlan_insert()' > > >> to duplicate the mbuf, right? > > >> > > >> That patch looks like won't make the release, so I suggest this one wait that > > >> patch, although this is harmless on its own, commit log is misleading. > > >> > > >> [1] > > >> https://patches.dpdk.org/patch/51870/ > > > > > > It was always true, even with existing vlan_insert. > > > Existing vlan_insert has issues if it ever creates a clone packet. > > > Existing vlan_insert can duplicate mbuf through clone > > > > > > > Right, existing vlan_insert has same issue on af_packet. > > > > But, should vlan_insert try to duplicate the mbuf when it is shared, does it > > worth the complexity it brings? And when that support removed this patch won't > > be needed. > > I don't think vlan insert or other mbuf manipulation APIs should be > checking for shared state or not - that's the job of the app. We could have > cases where the user does want to modify a shared mbuf. > > Regards, > /Bruce The vlan_insert code is called on transmit, and there are lots of cases where a transmit mbuf might be shared (like TCP stack). And in that case inserting the vlan must be non-destructive to the original mbuf. Whether you want to push the problem to the driver or do it in the library, it is still a problem.