From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <3chas3@gmail.com> Received: from mail-qk1-f194.google.com (mail-qk1-f194.google.com [209.85.222.194]) by dpdk.org (Postfix) with ESMTP id AD6181B54B for ; Tue, 16 Apr 2019 20:32:20 +0200 (CEST) Received: by mail-qk1-f194.google.com with SMTP id a71so12794238qkg.2 for ; Tue, 16 Apr 2019 11:32:20 -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=RsBl8q2pTg+KJoMxchaABAUYb2YnNnFRYpdx0VMHGtY=; b=NHkUPj9vffAyvA55IcPvZK5JPRK7bjKb33egMnlF1N3zzaI9M0goHOTveL7Fiw2fLa gJSdtXdQ8yDewqXlxPFd2Dx5sKoqnZmLzqw/3lqI5IaH+eEwrrWXC9lDtJbvyQFBdp9U WkO7VQ7U7kHPce+0XS1dh7nzESopL0BDN0TtVkzzWeml1ZKfYl9iKzMhb0JGfS8+82AL ps7TlGMtf7fLwwD3GqPbiuX+dpzfCKNs1E9OJFXIeIW/pznYNlReQ+hBG0wVfZ6HrG6Y YOGBJv3gqxx9OBOt9Qan9HUmMqZDB0OaQh0C0bqJaxV7m+1vUaFFdQuBoTniOG6eYJ9h j7LQ== 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=RsBl8q2pTg+KJoMxchaABAUYb2YnNnFRYpdx0VMHGtY=; b=Hdd04nrwr255exgsAZlrhwFA2fLF5X3UUaCdOD91YZv0SnmJdsM2evrzGu1HGqg3kW 9YU0l+FOyeGoLLETrtFpQGn2whLlhssJ9s3nnVxI9IdA8XYvtNx1rPkw8YNIp2TMFQR5 i/adjheTqwDiQM8DzW4s3Vcz+m2G9ErzogqUXBielzWi+Zg2VI9Zgr4XapOirgzeax0H BXCBVmzvEI1SVgOpAIcHCdtfhJFvcv1ICWRCgLjHt5cQGZQlpSV27jv0/YjkpdCd9tP+ blz3IUo4TX1vxk+ZmYUfXDZVZyWhDnRBKFBViL3/HKJCkP2zIXxW1m9DqeSnZb3Li4SV mEiA== X-Gm-Message-State: APjAAAVMRotvpoRlc+CHOU27Cb8sttRkuRvlB2qk8VNpH5rrILmagiUu 3Q0rD7pDa6N1CSwliBmGs+A= X-Google-Smtp-Source: APXvYqz7kD6IuqmsflWkXUjj3ehLB7avIjt4txB2AXuO3/m0t/vW19PJtVrFrXGotxNnrRHPosTRtw== X-Received: by 2002:a37:654e:: with SMTP id z75mr63036412qkb.314.1555439539824; Tue, 16 Apr 2019 11:32:19 -0700 (PDT) Received: from [192.168.1.10] (pool-96-255-82-34.washdc.fios.verizon.net. [96.255.82.34]) by smtp.gmail.com with ESMTPSA id n10sm42414903qta.86.2019.04.16.11.32.18 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 16 Apr 2019 11:32:18 -0700 (PDT) To: Bruce Richardson , Ferruh Yigit Cc: Olivier Matz , dev@dpdk.org, Stephen Hemminger , Chas Williams References: <20190416155126.26438-1-ferruh.yigit@intel.com> <20190416162824.GA1886@bricha3-MOBL.ger.corp.intel.com> From: Chas Williams <3chas3@gmail.com> Message-ID: <267fabc6-ce13-a91c-8a34-401252d3b277@gmail.com> Date: Tue, 16 Apr 2019 14:32:18 -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: <20190416162824.GA1886@bricha3-MOBL.ger.corp.intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH] net: do not insert VLAN tag to shared mbufs 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: Tue, 16 Apr 2019 18:32:20 -0000 On 4/16/19 12:28 PM, Bruce Richardson wrote: > On Tue, Apr 16, 2019 at 04:51:26PM +0100, Ferruh Yigit wrote: >> The vlan_insert() is buggy when it tires to handle the shared mbufs, >> instead don't support inserting VLAN tag into shared mbufs and return >> an error for that case. >> >> Signed-off-by: Ferruh Yigit >> --- >> Cc: Stephen Hemminger >> Cc: Chas Williams >> >> This is another approach to RFC to fix the vlan_insert: >> https://patches.dpdk.org/patch/51870/ >> >> vlan_insert() mostly used by drivers to insert VLAN tag into packet >> data in Tx path, drivers creating new copies of mbufs in Tx path may >> result unexpected behavior, like not freed or double freed mbufs. >> --- >> lib/librte_net/rte_ether.h | 11 ++--------- >> 1 file changed, 2 insertions(+), 9 deletions(-) >> > So what is the API to be used if one does want to insert a vlan tag into a > shared mbuf? It's unlikely you would ever want to do that. Have one thread perform some operation on the mbuf and other threads would expect this to have happened? It seems counter to the way that packets might flow through an application. Typically, you would insert the vlan and then share the mbuf. Modifying a shared mbuf should make you ask, what are the other copies expecting? > Also, why is it such a problem to create new copies of data inside the > driver if that is necessary? You create a copy and use that, freeing the > original (i.e. in all likelyhood decrememting the ref-count since you no > longer use it). You already have the pointer to the mbuf pool from the > original buffer so you can get a copy from the same place. I'm curious to > know why it would be impossible to do a functionally correct > implementation? It is not an issue to do this correctly. Hemminger did submit a patch that appeared to do this correctly (I haven't tested it). As mentioned earlier the tricky part is returning the buffer to the application. If you create a copy and transmit fails, you need to free that buffer or return it to the application for it to free. If you free the buffer when making a buffer, you certainly can't return it to the application for it to be freed a second time. > /Bruce > 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 74BB2A00E6 for ; Tue, 16 Apr 2019 20:32:23 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 46C151B557; Tue, 16 Apr 2019 20:32:22 +0200 (CEST) Received: from mail-qk1-f194.google.com (mail-qk1-f194.google.com [209.85.222.194]) by dpdk.org (Postfix) with ESMTP id AD6181B54B for ; Tue, 16 Apr 2019 20:32:20 +0200 (CEST) Received: by mail-qk1-f194.google.com with SMTP id a71so12794238qkg.2 for ; Tue, 16 Apr 2019 11:32:20 -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=RsBl8q2pTg+KJoMxchaABAUYb2YnNnFRYpdx0VMHGtY=; b=NHkUPj9vffAyvA55IcPvZK5JPRK7bjKb33egMnlF1N3zzaI9M0goHOTveL7Fiw2fLa gJSdtXdQ8yDewqXlxPFd2Dx5sKoqnZmLzqw/3lqI5IaH+eEwrrWXC9lDtJbvyQFBdp9U WkO7VQ7U7kHPce+0XS1dh7nzESopL0BDN0TtVkzzWeml1ZKfYl9iKzMhb0JGfS8+82AL ps7TlGMtf7fLwwD3GqPbiuX+dpzfCKNs1E9OJFXIeIW/pznYNlReQ+hBG0wVfZ6HrG6Y YOGBJv3gqxx9OBOt9Qan9HUmMqZDB0OaQh0C0bqJaxV7m+1vUaFFdQuBoTniOG6eYJ9h j7LQ== 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=RsBl8q2pTg+KJoMxchaABAUYb2YnNnFRYpdx0VMHGtY=; b=Hdd04nrwr255exgsAZlrhwFA2fLF5X3UUaCdOD91YZv0SnmJdsM2evrzGu1HGqg3kW 9YU0l+FOyeGoLLETrtFpQGn2whLlhssJ9s3nnVxI9IdA8XYvtNx1rPkw8YNIp2TMFQR5 i/adjheTqwDiQM8DzW4s3Vcz+m2G9ErzogqUXBielzWi+Zg2VI9Zgr4XapOirgzeax0H BXCBVmzvEI1SVgOpAIcHCdtfhJFvcv1ICWRCgLjHt5cQGZQlpSV27jv0/YjkpdCd9tP+ blz3IUo4TX1vxk+ZmYUfXDZVZyWhDnRBKFBViL3/HKJCkP2zIXxW1m9DqeSnZb3Li4SV mEiA== X-Gm-Message-State: APjAAAVMRotvpoRlc+CHOU27Cb8sttRkuRvlB2qk8VNpH5rrILmagiUu 3Q0rD7pDa6N1CSwliBmGs+A= X-Google-Smtp-Source: APXvYqz7kD6IuqmsflWkXUjj3ehLB7avIjt4txB2AXuO3/m0t/vW19PJtVrFrXGotxNnrRHPosTRtw== X-Received: by 2002:a37:654e:: with SMTP id z75mr63036412qkb.314.1555439539824; Tue, 16 Apr 2019 11:32:19 -0700 (PDT) Received: from [192.168.1.10] (pool-96-255-82-34.washdc.fios.verizon.net. [96.255.82.34]) by smtp.gmail.com with ESMTPSA id n10sm42414903qta.86.2019.04.16.11.32.18 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 16 Apr 2019 11:32:18 -0700 (PDT) To: Bruce Richardson , Ferruh Yigit Cc: Olivier Matz , dev@dpdk.org, Stephen Hemminger , Chas Williams References: <20190416155126.26438-1-ferruh.yigit@intel.com> <20190416162824.GA1886@bricha3-MOBL.ger.corp.intel.com> From: Chas Williams <3chas3@gmail.com> Message-ID: <267fabc6-ce13-a91c-8a34-401252d3b277@gmail.com> Date: Tue, 16 Apr 2019 14:32:18 -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: <20190416162824.GA1886@bricha3-MOBL.ger.corp.intel.com> Content-Type: text/plain; charset="UTF-8"; format="flowed" Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH] net: do not insert VLAN tag to shared mbufs 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: <20190416183218.sVVvB1uzT81C4eJlVZUrghYA7115OovkhvmPpEMVXkI@z> On 4/16/19 12:28 PM, Bruce Richardson wrote: > On Tue, Apr 16, 2019 at 04:51:26PM +0100, Ferruh Yigit wrote: >> The vlan_insert() is buggy when it tires to handle the shared mbufs, >> instead don't support inserting VLAN tag into shared mbufs and return >> an error for that case. >> >> Signed-off-by: Ferruh Yigit >> --- >> Cc: Stephen Hemminger >> Cc: Chas Williams >> >> This is another approach to RFC to fix the vlan_insert: >> https://patches.dpdk.org/patch/51870/ >> >> vlan_insert() mostly used by drivers to insert VLAN tag into packet >> data in Tx path, drivers creating new copies of mbufs in Tx path may >> result unexpected behavior, like not freed or double freed mbufs. >> --- >> lib/librte_net/rte_ether.h | 11 ++--------- >> 1 file changed, 2 insertions(+), 9 deletions(-) >> > So what is the API to be used if one does want to insert a vlan tag into a > shared mbuf? It's unlikely you would ever want to do that. Have one thread perform some operation on the mbuf and other threads would expect this to have happened? It seems counter to the way that packets might flow through an application. Typically, you would insert the vlan and then share the mbuf. Modifying a shared mbuf should make you ask, what are the other copies expecting? > Also, why is it such a problem to create new copies of data inside the > driver if that is necessary? You create a copy and use that, freeing the > original (i.e. in all likelyhood decrememting the ref-count since you no > longer use it). You already have the pointer to the mbuf pool from the > original buffer so you can get a copy from the same place. I'm curious to > know why it would be impossible to do a functionally correct > implementation? It is not an issue to do this correctly. Hemminger did submit a patch that appeared to do this correctly (I haven't tested it). As mentioned earlier the tricky part is returning the buffer to the application. If you create a copy and transmit fails, you need to free that buffer or return it to the application for it to free. If you free the buffer when making a buffer, you certainly can't return it to the application for it to be freed a second time. > /Bruce >