From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <3chas3@gmail.com> Received: from mail-qk1-f195.google.com (mail-qk1-f195.google.com [209.85.222.195]) by dpdk.org (Postfix) with ESMTP id ED39E27D for ; Sun, 7 Apr 2019 01:11:45 +0200 (CEST) Received: by mail-qk1-f195.google.com with SMTP id b74so5934333qkg.9 for ; Sat, 06 Apr 2019 16:11:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=6I9Cq6/QZWk/qqqfBRnJbWiinMgXNTszsNjGxklOq4g=; b=huKyBF9e2m1ggNPSxK/0P0HEVTp7UPnxXsmmsD02BBJBWJHKLsErR/NaO1ot4R5egV X43VwpQL7Y8JNu7apd/DET/JFv6ZZxrK7HYnxNb6hFwZ2lUQw2e0SoxDvHo8U/IZU/cj ShsIhJELJ7IG0/zX5kxLmO2mMDzja25/zJPzkuyD/vhH/TIqG8VNOLsutymhte3s5DHy Uyq8246/ouXNnUCaliCRI69GD+HIf6fOEdelL957T5+S9yU1NtDFp9vACRBiAk+T6I0Q T8D6S1gs7Z90P5n/TZIq5WGQ2etRAuMOdQVHEXuenNbKTXs7RdzAiMqH3FAwwMfVOYNd U/+Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=6I9Cq6/QZWk/qqqfBRnJbWiinMgXNTszsNjGxklOq4g=; b=fNxgDLjVE0EVGvmjskhFeVjzlLDohx1RMw5C/Jn1ZdPXWWKjNWCU69QBIgB1/ZDrBJ qaD1c/6e4sINJc2pMTLUFP5IA+3Y9V5/B8sTwDlQ7qbKkCbtizm9t9sk4cILVj2/uTaU Ox5pC8DClPhfEcikcpxhYHL3wxR8z5uZbHbfjHlXJLNrJlw7KPb+K2xiRP7VyvWF+7bq Jw4PCQrzctJ+vSIG2gzKM1PsM73+VmsmcwtYJU9rxj7Y5BA0DeEKu05W03TzA49LHE+j kYzx+9D8429KLal1+gcdYx4l7GqtzgQtN4+GNI1wGQEJuojMrGWHmqsUjCDzAH84wy6s u0Uw== X-Gm-Message-State: APjAAAXqNZaWUorpukBXg+qjFVn1PqR4LztVtZ55Ko9ifQekflIQn8Om zflq9fhq3PK9hPEFyHXtCgc= X-Google-Smtp-Source: APXvYqzu4GLPaQjnqRlmx+RiHvo1UuNbE9NRbu2E3ZcjsnGQvrIbcnQ7cmMmdU9Wgks8uhyc0aLSVQ== X-Received: by 2002:a05:620a:1407:: with SMTP id d7mr17138759qkj.189.1554592305265; Sat, 06 Apr 2019 16:11:45 -0700 (PDT) Received: from [192.168.1.19] (pool-96-255-82-34.washdc.fios.verizon.net. [96.255.82.34]) by smtp.gmail.com with ESMTPSA id i24sm17983664qti.76.2019.04.06.16.11.43 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sat, 06 Apr 2019 16:11:44 -0700 (PDT) To: Stephen Hemminger Cc: dev@dpdk.org, Stephen Hemminger References: <20190328205322.852-1-stephen@networkplumber.org> <20190404165449.0f7ea879@shemminger-XPS-13-9360> From: Chas Williams <3chas3@gmail.com> Message-ID: Date: Sat, 6 Apr 2019 19:11:43 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 MIME-Version: 1.0 In-Reply-To: <20190404165449.0f7ea879@shemminger-XPS-13-9360> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [RFC v2] net: fix rte_vlan_insert with shared mbuf 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: , X-List-Received-Date: Sat, 06 Apr 2019 23:11:46 -0000 On 4/4/19 7:54 PM, Stephen Hemminger wrote: > On Sat, 30 Mar 2019 08:41:33 -0400 > Chas Williams <3chas3@gmail.com> wrote: > >> Unfortunately, I think the complete fix is more complicated than this. >> Drivers that use rte_vlan_insert don't anticipate that the mbuf might >> change and that (hardware) transmit can fail. >> >> They make a copy of the mbuf pointer from the incoming transmit list >> and don't update the original if rte_vlan_insert creates a new mbuf. >> If transmit fails, the application needs to be the given the new mbuf >> for re-transmission or freeing. >> >> On 3/28/19 4:53 PM, Stephen Hemminger wrote: >>> If mbuf is shared then rte_vlan_insert() would clobber the original >>> Ethernet header. The changed version handles this by getting >>> an mbuf that will hold the new Ethernet and VLAN header followed >>> by another mbuf (cloned) for the data. >>> >>> Fixes: c974021a5949 ("ether: add soft vlan encap/decap") >>> Signed-off-by: Stephen Hemminger > > The virtio driver is buggy, it saves original mbuf and doesn't handle > rewrite. Dpaa2 is fine, should never happen since it checks refcnt etc. > Netvsc PMD is using this on rx path and is safe. > AF_packet PMD should work fine as well. af_packet is broken as well. It needs to defer rte_vlan_insert until after it gets the next incoming frame. Otherwise the break can exit the loop early. > So virtio is the one that needs fixing I agree with this. 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 D0AE9A0679 for ; Sun, 7 Apr 2019 01:11:49 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 38C0C2E81; Sun, 7 Apr 2019 01:11:48 +0200 (CEST) Received: from mail-qk1-f195.google.com (mail-qk1-f195.google.com [209.85.222.195]) by dpdk.org (Postfix) with ESMTP id ED39E27D for ; Sun, 7 Apr 2019 01:11:45 +0200 (CEST) Received: by mail-qk1-f195.google.com with SMTP id b74so5934333qkg.9 for ; Sat, 06 Apr 2019 16:11:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=6I9Cq6/QZWk/qqqfBRnJbWiinMgXNTszsNjGxklOq4g=; b=huKyBF9e2m1ggNPSxK/0P0HEVTp7UPnxXsmmsD02BBJBWJHKLsErR/NaO1ot4R5egV X43VwpQL7Y8JNu7apd/DET/JFv6ZZxrK7HYnxNb6hFwZ2lUQw2e0SoxDvHo8U/IZU/cj ShsIhJELJ7IG0/zX5kxLmO2mMDzja25/zJPzkuyD/vhH/TIqG8VNOLsutymhte3s5DHy Uyq8246/ouXNnUCaliCRI69GD+HIf6fOEdelL957T5+S9yU1NtDFp9vACRBiAk+T6I0Q T8D6S1gs7Z90P5n/TZIq5WGQ2etRAuMOdQVHEXuenNbKTXs7RdzAiMqH3FAwwMfVOYNd U/+Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=6I9Cq6/QZWk/qqqfBRnJbWiinMgXNTszsNjGxklOq4g=; b=fNxgDLjVE0EVGvmjskhFeVjzlLDohx1RMw5C/Jn1ZdPXWWKjNWCU69QBIgB1/ZDrBJ qaD1c/6e4sINJc2pMTLUFP5IA+3Y9V5/B8sTwDlQ7qbKkCbtizm9t9sk4cILVj2/uTaU Ox5pC8DClPhfEcikcpxhYHL3wxR8z5uZbHbfjHlXJLNrJlw7KPb+K2xiRP7VyvWF+7bq Jw4PCQrzctJ+vSIG2gzKM1PsM73+VmsmcwtYJU9rxj7Y5BA0DeEKu05W03TzA49LHE+j kYzx+9D8429KLal1+gcdYx4l7GqtzgQtN4+GNI1wGQEJuojMrGWHmqsUjCDzAH84wy6s u0Uw== X-Gm-Message-State: APjAAAXqNZaWUorpukBXg+qjFVn1PqR4LztVtZ55Ko9ifQekflIQn8Om zflq9fhq3PK9hPEFyHXtCgc= X-Google-Smtp-Source: APXvYqzu4GLPaQjnqRlmx+RiHvo1UuNbE9NRbu2E3ZcjsnGQvrIbcnQ7cmMmdU9Wgks8uhyc0aLSVQ== X-Received: by 2002:a05:620a:1407:: with SMTP id d7mr17138759qkj.189.1554592305265; Sat, 06 Apr 2019 16:11:45 -0700 (PDT) Received: from [192.168.1.19] (pool-96-255-82-34.washdc.fios.verizon.net. [96.255.82.34]) by smtp.gmail.com with ESMTPSA id i24sm17983664qti.76.2019.04.06.16.11.43 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sat, 06 Apr 2019 16:11:44 -0700 (PDT) To: Stephen Hemminger Cc: dev@dpdk.org, Stephen Hemminger References: <20190328205322.852-1-stephen@networkplumber.org> <20190404165449.0f7ea879@shemminger-XPS-13-9360> From: Chas Williams <3chas3@gmail.com> Message-ID: Date: Sat, 6 Apr 2019 19:11:43 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 MIME-Version: 1.0 In-Reply-To: <20190404165449.0f7ea879@shemminger-XPS-13-9360> Content-Type: text/plain; charset="UTF-8"; format="flowed" Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [RFC v2] net: fix rte_vlan_insert with shared mbuf 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: <20190406231143.gOaqn-ot813VZ9AC2JVnT3hZg_FMnl71lxUuIgz_qTs@z> On 4/4/19 7:54 PM, Stephen Hemminger wrote: > On Sat, 30 Mar 2019 08:41:33 -0400 > Chas Williams <3chas3@gmail.com> wrote: > >> Unfortunately, I think the complete fix is more complicated than this. >> Drivers that use rte_vlan_insert don't anticipate that the mbuf might >> change and that (hardware) transmit can fail. >> >> They make a copy of the mbuf pointer from the incoming transmit list >> and don't update the original if rte_vlan_insert creates a new mbuf. >> If transmit fails, the application needs to be the given the new mbuf >> for re-transmission or freeing. >> >> On 3/28/19 4:53 PM, Stephen Hemminger wrote: >>> If mbuf is shared then rte_vlan_insert() would clobber the original >>> Ethernet header. The changed version handles this by getting >>> an mbuf that will hold the new Ethernet and VLAN header followed >>> by another mbuf (cloned) for the data. >>> >>> Fixes: c974021a5949 ("ether: add soft vlan encap/decap") >>> Signed-off-by: Stephen Hemminger > > The virtio driver is buggy, it saves original mbuf and doesn't handle > rewrite. Dpaa2 is fine, should never happen since it checks refcnt etc. > Netvsc PMD is using this on rx path and is safe. > AF_packet PMD should work fine as well. af_packet is broken as well. It needs to defer rte_vlan_insert until after it gets the next incoming frame. Otherwise the break can exit the loop early. > So virtio is the one that needs fixing I agree with this.