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 6E540A00C2; Thu, 3 Nov 2022 13:43:39 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id F2DD54069B; Thu, 3 Nov 2022 13:43:38 +0100 (CET) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by mails.dpdk.org (Postfix) with ESMTP id 4B69740694 for ; Thu, 3 Nov 2022 13:43:37 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1667479416; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=2fWF/C332c/DaLaax1D47BLF8RKZ8cMEBj54kDV6mSs=; b=FyeS/Oi7brIGq/RcXFE2ocehAkgXUvEyOSsLgAqwlkJaYNUf9uwsJ6gmEF3+VrYmdxUPmQ oD+VR0mFPuQITbgVQ4bRx/+ESjoxNZt70JTA8n5rr4aay9FuaO4l3vl0oRiD9Otwj5BN+o I4Roi5elg29OhlHj5d+E4Ya3HGxX9Ow= Received: from mail-wm1-f72.google.com (mail-wm1-f72.google.com [209.85.128.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-403-DJ0LKpvePqKjpJoW4ZCZ6g-1; Thu, 03 Nov 2022 08:43:35 -0400 X-MC-Unique: DJ0LKpvePqKjpJoW4ZCZ6g-1 Received: by mail-wm1-f72.google.com with SMTP id f62-20020a1c3841000000b003cf6d9aacbbso796128wma.8 for ; Thu, 03 Nov 2022 05:43:35 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:subject:references:cc:to:from :content-language:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=2fWF/C332c/DaLaax1D47BLF8RKZ8cMEBj54kDV6mSs=; b=v4gJAXIQdYQXILRxz6ecny86LUjyx/2sNoE2RKsS1nNOl2oMBfy7pWPcZl7dX8jqdO KZRUluVG4rOWuNnE580jGWyml9yddX9UvGhPhAa1XD6CVlSDlSY5ftLFYsm1iS6geHWQ iG7LPHBwZ/0Jk2VoYfcws8BYIdfEuyfJUi0N6PO+lhnQHMItCZ8dI8ioF2OmLhkYX3Rf OhWvIjctvwXBUWO+0M3sWL0qRSOBVCQNMw5LEtgfIxcr1qDnqo1f0uj4H2u+X5+KX5np 7DTBiVgHYtGZhH/YDzSZxu3paxXxVP5GmU+Xd4wRERcr7roCYskm790nFemozE5Es2AC owlA== X-Gm-Message-State: ACrzQf0WSrnlUaLut9vEz5js23+tRCPXh232wEq9pPzn0YghNu3TowVo 4EMZrna+87Zy+QbGNLAQQiElAEAHBa/MLaZbSsiGi86Yeh91DQRHlk0+4yleZbgNDgHuu3ba5rh 5LJA= X-Received: by 2002:adf:fcc3:0:b0:236:a9c2:ad20 with SMTP id f3-20020adffcc3000000b00236a9c2ad20mr18610335wrs.365.1667479414403; Thu, 03 Nov 2022 05:43:34 -0700 (PDT) X-Google-Smtp-Source: AMsMyM6UpOh/e7CMaKe4wZdLb8qEB3zcRhidWd8E1zuaNYaB0CgzNqMxPwr000SNqorRrL8KFYdosg== X-Received: by 2002:adf:fcc3:0:b0:236:a9c2:ad20 with SMTP id f3-20020adffcc3000000b00236a9c2ad20mr18610322wrs.365.1667479414141; Thu, 03 Nov 2022 05:43:34 -0700 (PDT) Received: from [192.168.0.36] ([78.17.177.122]) by smtp.gmail.com with ESMTPSA id d18-20020a5d6dd2000000b00236863c02f5sm812078wrz.96.2022.11.03.05.43.33 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 03 Nov 2022 05:43:33 -0700 (PDT) Message-ID: Date: Thu, 3 Nov 2022 12:43:32 +0000 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.4.0 From: Kevin Traynor To: "Zhou, YidingX" , "dev@dpdk.org" Cc: "Yang, Qiming" , "Zhang, Qi Z" References: <20221018102602.217673-1-yidingx.zhou@intel.com> <20221019075432.9698-1-yidingx.zhou@intel.com> <30c5f90a-acd5-eacb-7ad9-8e8e6819d67d@redhat.com> <322c348e-3461-c7ab-a845-2782ffce5ef9@redhat.com> Subject: Re: [PATCH v2] net/iavf: revert fix VLAN insertion In-Reply-To: <322c348e-3461-c7ab-a845-2782ffce5ef9@redhat.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit 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 re-adding dev mailing list On 03/11/2022 10:51, Kevin Traynor wrote: > On 03/11/2022 10:38, Zhou, YidingX wrote: >> Hi, Kevin >> >> According to suggestion, we did many deep investigation and various attempts, unfortunately that the performance drop(about 40%) caused by >> the previous commit: >> 0d58caa7d6d1 ("net/iavf: fix VLAN insertion") >> Still can not be resolved in this cycle due to tight schedule. >> >> Because the performance drop is too serious and the scope of impact is relatively large, we think the previous commit is a mistake. > > ok, I'm trying to understand why a performance drop is worse than some > corrupt packets. > > What is the scope of the performance drop? Does the scope impact more > cases than just when the L2TAG2 is used? > > IOW, is it a functional issue in a small use case, and performance issue > in more use cases? If so, I can understand you wanting to revert. > >> For the original bug, we plan to fix it in the next cycle (by supporting L2TAG2 on the vector path). >> > > With the revert, is there a way to disable use of L2TAG2 being used > while it is incorrect? At very least, the issue should be > documented/bugzilla so a user can know what doesn't work correctly. > >> So we are expecting to revert the above commit from main branch and it should not be merged to stable branch. > > It has already been merged but if the decision for main branch is to > revert, then I can revert on stable also. > > thanks, > Kevin. > >> Sorry I'm a little late in explaining the situation. Your understanding would be appreciated. >> >> /Yiding >> >>> -----Original Message----- >>> From: Zhou, YidingX >>> Sent: Friday, October 21, 2022 10:43 AM >>> To: Kevin Traynor ; dev@dpdk.org >>> Subject: RE: [PATCH v2] net/iavf: revert fix VLAN insertion >>> >>> >>> >>>>>> -----Original Message----- >>>>>> From: Kevin Traynor >>>>>> Sent: Wednesday, October 19, 2022 4:53 PM >>>>>> To: Zhou, YidingX ; dev@dpdk.org >>>>>> Subject: Re: [PATCH v2] net/iavf: revert fix VLAN insertion >>>>>> >>>>>> On 19/10/2022 08:54, Yiding Zhou wrote: >>>>>>> When the kernel driver tells to use the L2TAG2 field for VLAN >>>>>>> insertion, the context descriptor needs to be used. There is an >>>>>>> issue on the vector Tx path, because it does not support the >>>>>>> context >>>> descriptor. >>>>>>> >>>>>>> The previous commit forces to select normal path to avoid the >>>>>>> above issue, but it results in a performance loss of around 40%. >>>>>>> So it needs to be reverted and the original issue needed to be fixed by >>> rework. >>>>>>> >>>>>> >>>>>> Thank you, that is a much clearer explanation. >>>>>> >>>>>> Now on the approach, the commit being reverted says: >>>>>> "When the driver tells the VF to insert VLAN tag using the L2TAG2 >>>>>> field, vector Tx path does not use Tx context descriptor and would >>>>>> cause VLAN tag inserted into the wrong location." >>>>>> >>>>>> So it means this revert is solving a performance regression, but >>>>>> re-introducing the functional issue above. >>>>>> >>>>>> Is there a correct fix for the original issue sent that can be >>>>>> applied too? If not, wouldn't it be better to wait until it is >>>>>> before doing the >>>> revert? >>>>>> >>>>> >>>>> Sorry, there is no correct fix yet. >>>>> We plan to support context descriptor on vector path to fix the >>>>> original issue, It may take more time and cannot be completed within >>>>> this >>>> cycle. >>>>> >>>> >>>> ok, but you didn't answer the second question. >>>> >>>> "When the driver tells the VF to insert VLAN tag using the L2TAG2 >>>> field, vector Tx path does not use Tx context descriptor and would >>>> cause VLAN tag inserted into the wrong location." >>>> >>>> Please explain your justification for (re-)introducing this bug? >>>> >>>> Why is better to get (corrupt?) packets with incorrect VLAN tags than >>>> lose performance for this case? Or have I mis-interpreted the patches. >>>> >>>> >>> Thanks for your review. >>> I agree with you. It should not re-introduce functional issue . >>> This revert is not needed. I will resubmit a new patch for the performance loss. >>> >>>>>>> To reverts >>>>>>> commit 0d58caa7d6d1 ("net/iavf: fix VLAN insertion") >>>>>>> >>>>>>> Fixes: 0d58caa7d6d1 ("net/iavf: fix VLAN insertion") >>>>>>> >>>>>>> Signed-off-by: Yiding Zhou >>>>>>> --- >>>>>>> drivers/net/iavf/iavf_rxtx_vec_common.h | 3 --- >>>>>>> 1 file changed, 3 deletions(-) >>>>>>> >>>>>>> diff --git a/drivers/net/iavf/iavf_rxtx_vec_common.h >>>>>>> b/drivers/net/iavf/iavf_rxtx_vec_common.h >>>>>>> index 4ab22c6b2b..a59cb2ceee 100644 >>>>>>> --- a/drivers/net/iavf/iavf_rxtx_vec_common.h >>>>>>> +++ b/drivers/net/iavf/iavf_rxtx_vec_common.h >>>>>>> @@ -253,9 +253,6 @@ iavf_tx_vec_queue_default(struct iavf_tx_queue >>>> *txq) >>>>>>> if (txq->offloads & IAVF_TX_NO_VECTOR_FLAGS) >>>>>>> return -1; >>>>>>> >>>>>>> - if (txq->vlan_flag == IAVF_TX_FLAGS_VLAN_TAG_LOC_L2TAG2) >>>>>>> - return -1; >>>>>>> - >>>>>>> if (txq->offloads & IAVF_TX_VECTOR_OFFLOAD) >>>>>>> return IAVF_VECTOR_OFFLOAD_PATH; >>>>>>> >>>>> >> >