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 7AD81A0096 for ; Wed, 5 Jun 2019 09:28:29 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 4396B1B970; Wed, 5 Jun 2019 09:28:29 +0200 (CEST) Received: from dispatch1-us1.ppe-hosted.com (dispatch1-us1.ppe-hosted.com [67.231.154.164]) by dpdk.org (Postfix) with ESMTP id 6868D1B96B; Wed, 5 Jun 2019 09:28:27 +0200 (CEST) X-Virus-Scanned: Proofpoint Essentials engine Received: from webmail.solarflare.com (uk.solarflare.com [193.34.186.16]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by mx1-us3.ppe-hosted.com (PPE Hosted ESMTP Server) with ESMTPS id 268AB9C007E; Wed, 5 Jun 2019 07:28:26 +0000 (UTC) Received: from [192.168.38.17] (91.220.146.112) by ukex01.SolarFlarecom.com (10.17.10.4) with Microsoft SMTP Server (TLS) id 15.0.1395.4; Wed, 5 Jun 2019 08:28:19 +0100 To: Tiwei Bie CC: Maxime Coquelin , Zhihong Wang , , Dilshod Urazov , References: <1559587805-1637-1-git-send-email-arybchenko@solarflare.com> <20190605014117.GA20728@___> From: Andrew Rybchenko Message-ID: <8fb39bf5-2bd2-7faa-9ab7-551e234d77c5@solarflare.com> Date: Wed, 5 Jun 2019 10:28:14 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.7.0 MIME-Version: 1.0 In-Reply-To: <20190605014117.GA20728@___> Content-Language: en-GB X-Originating-IP: [91.220.146.112] X-ClientProxiedBy: ocex03.SolarFlarecom.com (10.20.40.36) To ukex01.SolarFlarecom.com (10.17.10.4) X-TM-AS-Product-Ver: SMEX-12.5.0.1300-8.5.1010-24658.003 X-TM-AS-Result: No-19.544800-8.000000-10 X-TMASE-MatchedRID: Jm7Yxmmj9OnmLzc6AOD8DfHkpkyUphL9ekMgTOQbVFv5+tteD5RzhTbl ER2H1U0/AxrzcNQAPjejfAWRMb+BPTLH/QvGXdfJogGd8wIUGILB1oTlNT01WFTFKQBe714npXO 0hnl5LjzqC5Z1AChT59ZZVqi+HeLhLI7MnpqzzTpkPwYFI6/mFEyQ5fRSh2659C4tEnGKigk0Tq ZEsK6k+OXd2pzF3g6i2FTfVSZnASCKaZfugyCyqPSG/+sPtZVk+KgiyLtJrSBWm0JlHAu9AdMIV mK5pwqYPLa1JP0J/Bl1iSVN1nuFqbxqYUFhPRapAZ0lncqeHqEugUeZzHeSvGnbLXx5NP9tl26E +8sVLpPZEhrS7mh5pusdStsJf8jVTueDjSHi1PdoOA9kFf9sy66IBbSnfz+3x4gtHcG+Se9R18Q pzXStTv2Fq3oxveV/7qL4IUbQ95ZhyMOhUPZ7nrzK9RGVgYIhH3yyYiehccvZhqPLU/1bVdJRpJ 7c9LEB92grUwQgYZd5OPD8XJFfpE1+zyfzlN7ygxsfzkNRlfL9b0xgYrma7dRnEQCUU+jzb2CjU /es002lDOeZ8yieawI4pJ3k1TMhrt9ikCacZnU7x0YDc03J8EooybO1oQu6zQ6zablequAL6Dwd nFVT+Ll+tAX1h+8Tc1dtjrDFB8A= X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No X-TMASE-Result: 10--19.544800-8.000000 X-TMASE-Version: SMEX-12.5.0.1300-8.5.1010-24658.003 X-MDID: 1559719706-3jHx_aIqJsf6 Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH v2] net/virtio: add Tx preparation 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" Hi, On 6/5/19 4:41 AM, Tiwei Bie wrote: > Hi, > > Thanks for the patch! More will follow. At least Tx checksum offload is broken when used together with VLAN insertion since the later prepend to mbuf, but do nothing with l2_len/outer_l2_len. We'll send a patch to move rte_vlan_insert() to Tx prepare and suggest separate patch to update l2_len or outer_l2_len when software VLAN insertion is done. > On Mon, Jun 03, 2019 at 07:50:05PM +0100, Andrew Rybchenko wrote: > [...] >> uint16_t >> +virtio_xmit_pkts_prepare(void *tx_queue __rte_unused, struct rte_mbuf **tx_pkts, >> + uint16_t nb_pkts) >> +{ >> + uint16_t nb_tx; >> + int error; >> + >> + for (nb_tx = 0; nb_tx < nb_pkts; nb_tx++) { >> + struct rte_mbuf *m = tx_pkts[nb_tx]; >> + >> +#ifdef RTE_LIBRTE_ETHDEV_DEBUG >> + error = rte_validate_tx_offload(m); >> + if (unlikely(error)) { >> + rte_errno = -error; >> + break; >> + } >> +#endif >> + >> + error = rte_net_intel_cksum_prepare(m); >> + if (unlikely(error)) { >> + rte_errno = -error; > It's a bit confusing here. > Based on the API doc of rte_eth_tx_prepare(): > > https://github.com/DPDK/dpdk/blob/7f9f46d6cef5b03681a3935b9a18378e08ca6f62/lib/librte_ethdev/rte_ethdev.h#L4360-L4362 > > It should set negative value to rte_errno when error happens, I'm pretty sure that it is a bug in documentation. rte_errno must be positive. I'll send a patch to fix it. Even the code just below sets positive rte_errno. Simple cases are very easy to find: $ git grep 'rte_errno = E' | wc -l 557 $ git grep 'rte_errno = -E' | wc -l 50 A bit more complex cases which require careful review: $ git grep -e 'rte_errno = -[a-z]' | wc -l 37 $ git grep -e 'rte_errno = [a-z]' | wc -l 150 Cases which look right from the first sight overweight wrong 3 times. But it is still too many cases which are potentially wrong. Andrew.