From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from NAM02-BL2-obe.outbound.protection.outlook.com (mail-bl2nam02on0060.outbound.protection.outlook.com [104.47.38.60]) by dpdk.org (Postfix) with ESMTP id 7A9B8A48C for ; Tue, 16 Jan 2018 10:32:27 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=CAVIUMNETWORKS.onmicrosoft.com; s=selector1-cavium-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=HScOUhIwj0NaPqdr6KMpEUqoEc14PnnDe70FdFUVvXM=; b=gF4LRHOkuAmjSmJQfDoBlb5bEaADi837IDtqkQsmY34ZMHw37EU7Z7W8zZXbOwnrGa6yGjMOdaQB5qDlh2huOkudxDpkKz9o1txZgcLwNvxDnEtw8QguP4fa7JWrwFGYgDUVQeVGMTb9FD4rewuoIm0+it0M9tC2WJZpiS084JA= Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=Maciej.Czekaj@cavium.com; Received: from [10.0.0.58] (31.172.191.173) by DM5PR07MB2890.namprd07.prod.outlook.com (10.168.102.144) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id 15.20.407.7; Tue, 16 Jan 2018 09:32:24 +0000 To: Ferruh Yigit , dev@dpdk.org Cc: shahafs@mellanox.com, thomas@monjalon.net References: <1514985137-2653-1-git-send-email-maciej.czekaj@caviumnetworks.com> <5795a502-77cf-2b33-a723-bb7b43ec6d82@intel.com> From: Maciej Czekaj Message-ID: Date: Tue, 16 Jan 2018 10:32:15 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US X-Originating-IP: [31.172.191.173] X-ClientProxiedBy: DB6PR0402CA0018.eurprd04.prod.outlook.com (10.172.243.156) To DM5PR07MB2890.namprd07.prod.outlook.com (10.168.102.144) X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 92d7fd8d-7787-458e-11a8-08d55cc40d6b X-Microsoft-Antispam: UriScan:; BCL:0; PCL:0; RULEID:(7020095)(4652020)(5600026)(4604075)(4534125)(4602075)(4627221)(201703031133081)(201702281549075)(2017052603307)(7153060)(7193020); SRVR:DM5PR07MB2890; X-Microsoft-Exchange-Diagnostics: 1; DM5PR07MB2890; 3:ZQM1Myi/kOFsBXn3Y83N21WBYSppKzU2O3ZKjD9Ewa/zBuS0RQNL1Sf4+LBEmDWJ2CzSWF+iocKSCeZQU8AzShQSXCIHLyOwGoL8pOl+kmYb6Coxr4ge4GZpyqGaC8pKNiw5l2XW2YKb6RF3V1pCwDGRIjqdDzRv78cqMpbChq9fCeeabq9ToGJ4kwSLWL2s+d0d/eCunYD3YBcV9t40vQpWVy9m7f6L7k8aWpqvV3KL8c4w7t0aJYJxhEwNOVoD; 25:emaRap0jzqCxtQdiJsmaaVK+wEFJzucszmEDUUShv2CRPa2QzUX1WeWPbNtHSI1f7vyZwBf4CeIusV5pG+ynjhL9StoACLdNPAdXxeV5hrsni1rMdSJ3Fhk3RX72vyY7RzMgPrSLjLgtWRKMS1db0ozYjdY6g2dgZ/5cxvnDIvS8Zr2zGUwo9Xzeu/0vhFWHQQoC326c70VEoOWjIAtCJLHSD4QkjkM6xUsQiZORz0lKMNek4ijjh9yZaiHvo8YAjCEpl5bhRPZnBiUbb7dRkOOS8ruY1qe4x9gP4E6UVJhQdHT+yb3cdqWCgYeCIh8tfnIJNuv8fyszAfQY4F2JqQ==; 31:RiZlY0wMcBnh8o1Jaqd0Ljf1UzvhOyv1/2mey4nox0P3hYF3aWIkDUc9Q2FogTXyvlmC707cS/n0oyyRmDtGZgjzAY6m0r4MpeJO/Nc3nlF/gx9vE9dbMtiUjeT5zFKrwmtWbr/HpByOtZ8ZQyWGOBeA/qrGsJTxnP0cgZO1KRAmhQfPS6k8knp8ZcyqWY5b14aEejxcGwJ+RmkfYZP3d1SwvhToh7YT90rNFlGdF3Y= X-MS-TrafficTypeDiagnostic: DM5PR07MB2890: X-Microsoft-Exchange-Diagnostics: 1; DM5PR07MB2890; 20:9RXfhevAzUxuimhp1pHK5wOxDMOBSGJorI5oqu4PcStcWW64gEZ3z91sfQBpGgLFtignBRr3xU9UA2Y9jnjMJ/YyPXQ6FgPfQOaRutsW7ohahwLt+7iI/zH9sVfFubQdHBHoW0+1sZhbHncInUtJJ2+g5qXN0DutE0UlnLzlbKcAbukeQ6TntJGbfryG+Vzl7or5diytPD3itUuMkQ8i6VAFzVh72tH6flu+Aqvgycr2PT9G1Ya0xPJjwqUSG2+bUG5uYuv6XeD5SHPASIrP5TEgNtJrdmfMQxy/jlyixT5AfwcvQQ67Ky0PALYQIVmCWyRIR7hltTRL4+lkjvnawxFf+gVI2tp25Ctmpjo+sEJDIWCcZpcxpKiSJBmDtGL/c0cTUhazxIWipHhDolzuC7GfMXK0Yt87BRGnlioJ2W8cATx/fqk82i1NSHLbz4t1qBShj5M9TsnyHghHpDnzLqnYFCOwkx50Fa0DfDlsQBK0tqiBG/6tl80iadNmHqscwuqRR528u29ZhOEdFAFgNo2RMh/+4BDpA6iOC2NHVRuHi4oJWjc/lDSR28rHvrD4d6iEsFv2HwinGlGV8DaOk+8D99arW+E3AEXJcjDFMRY=; 4:JjrWklyZUSppECveQ0s6LRN1lRkDBN5Btd49y9vegRJ0qK0KeEeggVSWC3kSiNklPF3nvSa6X7DWMmuk5gXRbfimnoCEiFNwS9SuwTGDQu79UHAFsGV2FVPHrrbJhp2mo3j656fvDESyhI5IW2GKeF3/3VZm574jpmKZX+FMWvt0ZvViRB/mDT+rnjmVBSZT5Qzr4/8Ex/AnkUxj3624n15FlRH7zzjFUWb9BFL9H6JEzjasD9Slnal55bsStQdIq9DVicn4hga3thX/jVPgqzMu1QavZk453bASEYs8QCSbjTipga6ryJtbV2jOhitZ X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:(278428928389397); X-Exchange-Antispam-Report-CFA-Test: BCL:0; PCL:0; RULEID:(6040470)(2401047)(8121501046)(5005006)(10201501046)(3231023)(944501161)(3002001)(93006095)(6041268)(20161123560045)(20161123564045)(20161123562045)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(20161123558120)(6072148)(201708071742011); SRVR:DM5PR07MB2890; BCL:0; PCL:0; RULEID:(100000803101)(100110400095); SRVR:DM5PR07MB2890; X-Forefront-PRVS: 0554B1F54F X-Forefront-Antispam-Report: SFV:NSPM; SFS:(10009020)(6049001)(376002)(396003)(39380400002)(366004)(346002)(39860400002)(199004)(189003)(24454002)(8676002)(36756003)(7736002)(81156014)(53936002)(16526018)(68736007)(53546011)(386003)(59450400001)(6116002)(81166006)(76176011)(52146003)(26005)(8936002)(65806001)(2486003)(23676004)(65956001)(66066001)(47776003)(52116002)(67846002)(2870700001)(3846002)(83506002)(305945005)(2950100002)(72206003)(58126008)(90366009)(64126003)(6246003)(65826007)(97736004)(42882006)(77096006)(4326008)(478600001)(5660300001)(105586002)(6486002)(6666003)(31686004)(2906002)(316002)(106356001)(16576012)(93886005)(229853002)(31696002)(25786009)(50466002)(217873001); DIR:OUT; SFP:1101; SCL:1; SRVR:DM5PR07MB2890; H:[10.0.0.58]; FPR:; SPF:None; PTR:InfoNoRecords; A:1; MX:1; LANG:en; Received-SPF: None (protection.outlook.com: cavium.com does not designate permitted sender hosts) X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtETTVQUjA3TUIyODkwOzIzOll0YkNhSTNReEplQkdGSjVqYmJNZlVpVmNw?= =?utf-8?B?QTFyeEZVV0RhcyttbE9BaXFycWtHdlFxdy9aWjJOMVd3cGRLNDJFSFFrTVBG?= =?utf-8?B?Z0xFOG5mWkluU2FubEVvbi94dWJGMWlLb2g3OE1vM3NMbUdaUTROZXpBNTIx?= =?utf-8?B?d3dROVowVWxmL282QytwUVU0RFdjSmhwNjdVbGowN0J6ZmJ3cSsrWkNhNzRq?= =?utf-8?B?dm1ma2lYdFBSRys5bDFXb0RjbFJmeXgzdXFvcVM0ZW96azU4QkNKOU1acjdE?= =?utf-8?B?anNPYWdXa0F6YnZkd2cvL1FpemRtYWI0blV5aXNrYW9yQU9TakVMK3hHdk5W?= =?utf-8?B?K2hKTHd5ZTRBSmtoZlJnc056Qzd2bTBSeStUdUE1Sjgva2MxQXhWR09nd2M2?= =?utf-8?B?ZHo2MElGazFXV2VLaDYva2VUeVc3S3J3dHpCMCt4ZWZrT0dXQmFQVWJPOG9O?= =?utf-8?B?YVQ4VHFPTW1jWXFuLzJzMlJmdThCRm1sME54MEh0MnUxZSthSEdmVFp0eG96?= =?utf-8?B?NDJaSUdkU1VCUmhsRE9nYVV3NktZbmNHOVdIdDJOT1hXUnMvL0tmSzJycER0?= =?utf-8?B?a2RBY1ZDRmc0MTdtQ3BTR25BVk9DdHhpRTNNYkU2a2lTdGQ3V0lEb2NSK0h2?= =?utf-8?B?RVY5V3gyRlFTditjNTdnaXNhakdtd09iMWtQTFhzaGd0TTJuQXZYcDRkTWVD?= =?utf-8?B?R3QzN21xdjNyTlFEZVV6dVF1U2FHWFRBQllRU2hmcEU2c1pvS3o2aUFmeC9Y?= =?utf-8?B?aVphZkNySkFFK3diWEM1VnlWdTJ0L3o4dWNUbklYYnpMekF4T2p0VnR1c1J5?= =?utf-8?B?WU5wWW40Rmw5RkNma0cydTJmMUJvMTlQbklvT2xEbkorTWIyZHEzaEVxWDdS?= =?utf-8?B?WjVqTjFvUHErUXozVzBDUzYxcnowTThnZUlvVWptazMzcktzZnBxMmtndE5X?= =?utf-8?B?dkE1VTZmN0dKR2RFdTc5bUdhN1UxbmJtdFMvSjF5ZXB2cDkrVE9peENkTEF1?= =?utf-8?B?YndUbE1McWZBaHFUZmN4K2NyaXlvRUg2WThLaGNlLzZ5c1BjZGZrc0dIU3lu?= =?utf-8?B?cDlZZncvZUJUSkx0SEJveW1nemRhbHB5SHFJZ1lPVldCSysvMUlqWlJyUWFz?= =?utf-8?B?bXo1bUdIMk05T05mSkliQ2QyVVBoN3JKbCtERzFZK2FBZzNIUHQ5aWZyMWNH?= =?utf-8?B?T2o0K2xLZXF4UHJkb1NRRGhJMmVLTklSOEFVNlU3dFRaWU4zSFE0TkFwWnFL?= =?utf-8?B?OWM4VkUxT1EwVU9rSTBFTEVCTmU3bGpueFVlYlJDTXduQzJrcXVidEVQMXVR?= =?utf-8?B?RkpXV3VtZ2M2SDFPYmE2UGFYOXhsTnQ3ODFhTTZHR3hvUUNUY2ZpYWlNYXYx?= =?utf-8?B?ZVNzS3FqbjdwMUhkV1QzTUtUdUZLbkt3a2dCREl0K1hxOTRMYmk0K3Z2b3ls?= =?utf-8?B?Z3Q3cUowTG1KNHNEN0c5UFpJWmtJTkVONXhXN3Uxak1WVmF6ckFoNUUyQjRh?= =?utf-8?B?YVptVWVIMVBBVG8zVzJvUE1pWVB4Z3NrUlcxOGk1S0xqbDU2b0VwcW5wRW1r?= =?utf-8?B?OEJYbVBRSjBWbU51UW9TSWNja25pOHdibTJZR3kzdVNvdzA2UTJkRkdjZHRi?= =?utf-8?B?b3ZHWGMxemsrYzhVMHhLOG9iaE9rQnFMWGtpZmlEV3hQaUJZbjFnZWJwendz?= =?utf-8?B?eE9rRWxQRGVtOVhDK1NBaHRsamtyNHVsa3M1VmsxQnJhU2wrbWJENElqL1Nm?= =?utf-8?B?SEJMa2NZckFzSGMxZWRVK0tGbi9UcHd2Mm12MnhpK3kwYlYyV3dhbjJZTUtG?= =?utf-8?B?dDFXSnIvdXVCVWQzbUpISHVvRTZQeTNScG5RaU1HWGxMeGc2MTk3UTRRb2xU?= =?utf-8?B?emN6VmxESStPL0IwM0hINkRVR25ERlNqRmRGYjlkZythczdqcHBIbG5kaGdk?= =?utf-8?B?aHY1Ny9PQnZkL2pMeHdsTDVLb0F0NjZpSHZrREdIb1NkcG91RHI5VVdCeXBZ?= =?utf-8?B?Y2JsMDlZTjZtM2NYREtSemFyejg5cGpoaG5wZz09?= X-Microsoft-Exchange-Diagnostics: 1; DM5PR07MB2890; 6:mE7w4R7htUXX7Eb32M7HKEPXtNWa6ogsvYxtQfwJ/4Fw0Mfb4wGBpoc/BVlUfGbzwfpus7ApritVfOR0Eau4NJ/TRrY1L8yTIwa9VBAvaHoilf3ZQzTwtMT8qbw7YSVMLq4N6KQHvvJ6IBCJ7zynNCuurQeUA5BEF5M8x//cRCF+JYi0OvKa7sxGqZiK+MXozil9CK9B87uT9iK/98P/g6zbwcbU/EsAoq4L1SrL1XbVtByf17peezdYwAvwHmpjc3Z9FM57iEwXrIwF0J+75njjxJG9QJf6Xy4Sc9gXiOZ1O4kf4ixnLEZ0AOpaGmVQnO9Vt1Kv+dtNK4vJBr6mEM2yFSMuC3pJfm8QO7b2X84=; 5:u9+WAiH43gl7Vb1w7hiwJiFF1j0EB61RLqxsqoAdlabhA+CAP/tWBtZkjphQ4wjNCsk7rNs4LZd8eO8j1BPwqASEscsXAoy5wTsxK0EicNSDg1F9fnrJECJRv7yJLNByIfT1ISmofR74ZVF+ssxgkVS4j5Sqy4OcJdebKNMIA/8=; 24:En5MNhOzj81sCxepzgwe0qbdr2yguNbTISgrz+qxfKlDUEtDh03aGVuXt9N78MBKbEkxDprJY28sd3TtqcMPTQFqfIylVLbGgmA0LF3/was=; 7:zoK6iZI9p7AOypZ0Dhl6LtKNgFmuRaNrox3ShG6NFtUEMlpZEwiIoyRAd2RDMoMYP7VpViPncRn7gYB1F2r1YVp6ZrEZi8ghfwlxGMgxS9GhJeAKPWHtjb1F15r4+LAJJTMSJ05doG1bAD9K50m1ThAsHS09izuTU02Cby6mbigAuPKm528Qym8flLzvbX5kEuDMo4cpZidZOyfOqv6XK0RSmyuiX2l50vX/ncBoClt+XeZqhgshSELYRSGuzVUj SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: caviumnetworks.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Jan 2018 09:32:24.2352 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 92d7fd8d-7787-458e-11a8-08d55cc40d6b X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 711e4ccf-2e9b-4bcf-a551-4094005b6194 X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM5PR07MB2890 Subject: Re: [dpdk-dev] [PATCH] net/thunderx: Convert ThunderX VNIC PMD to new offload API 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 Jan 2018 09:32:27 -0000 -- Oryginal message -- > On 1/15/2018 2:49 PM, Maciej Czekaj wrote: >> >> >> >> -- Oryginal message -- >>> On 1/3/2018 1:12 PM, maciej.czekaj@caviumnetworks.com wrote: >>>> From: Maciej Czekaj >>>> >>>> This patch removes all references to old-style offload API >>>> replacing them with new offload flags. >>>> >>>> Signed-off-by: Maciej Czekaj >>> <...> >>> >>>> >>>> dev_info->default_txconf = (struct rte_eth_txconf) { >>>> .tx_free_thresh = NICVF_DEFAULT_TX_FREE_THRESH, >>>> - .txq_flags = >>>> - ETH_TXQ_FLAGS_NOMULTSEGS | >>>> - ETH_TXQ_FLAGS_NOREFCOUNT | >>>> - ETH_TXQ_FLAGS_NOMULTMEMP | >>>> - ETH_TXQ_FLAGS_NOVLANOFFL | >>>> - ETH_TXQ_FLAGS_NOXSUMSCTP, >>>> + .txq_flags = ETH_TXQ_FLAGS_IGNORE, >>> I am not sure about this, Shahafs may comment better, shouldn't application >>> decide to set "c" or not, instead of having this in default >>> configuration? >> I think of it as a safeguard against a legacy application that is using >> old-style txq_flags. >> >> There is an assymetry in API, since >>  - rte_eth_tx_queue setup() and rte_eth_rx_queue_setup() convert flags >>  - rte_eth_dev_info_get() - does not convert them. >> >> The scenario leading to issues is as follows: >> >> 1.Application reads default txq_flags from the rte_eth_dev_info_get(). >>  - dev_info.txconf.offloads contains flags but it is ignored by legacy code >>  - dev_info.txq_flags may be: >>       a) txq_flags == 0 >>       b) txq_flags ==  ETH_TXQ_FLAGS_IGNORE >>       c) txq_flags == new-style flags converted to old-style flags >> >> >> 2. Application uses default txq_flags field to rte_eth_tx_queue_setup(). >> Now, depending on txq_flags: >> >>   a) txq_flags == 0, ethdev layer __clears__ all offloads, see * >>   b) txq_flags ==  ETH_TXQ_FLAGS_IGNORE, ethdev layer converts offloads >> to txq_flags, but leaves orignal offloads, so PMD can still use them >>   c) txq_flags == old-style flags, ethdev layer converts txq_flags to >> offloads, destroying default offloads >> >> >> * relevant code snippet from rte_eth_tx_queue_setup() with comments: >> >>     if (tx_conf->txq_flags & ETH_TXQ_FLAGS_IGNORE) { >>         // ==> converts offloads to txq_flags but LEAVES offloads, too >>         rte_eth_convert_txq_offloads(tx_conf->offloads, >>                          &local_conf.txq_flags); >>         /* Keep the ignore flag. */ >>         local_conf.txq_flags |= ETH_TXQ_FLAGS_IGNORE; >>     } else { >>         // ==> converts txq_flags to offloads but DESTROYS original >> offloads >> rte_eth_convert_txq_flags(tx_conf->txq_flags, >>                       &local_conf.offloads); >>     } >> >> >> So, out of 3 options: >> >> a) does not work for legacy code >> b) will work for legacy code >> c) will work but defeats the purpose of removing old-style flags, since >> dev_info() callback has to setup both old-style and new-style default flags >> >> I chose b) as the simplest way to work-around the issue. I could post a >> patch to ethdev API if you think it is important. > What I understand is txq_flags should be supported because of legacy apps. That > is why ethdev layer converts old txq_flags to offloads when new > ETH_TXQ_FLAGS_IGNORE flag is missing. So that PMD can only use "offloads" > variable for both legacy and new applications. > > To support the case application uses defaults from PMD, the one you mentioned > above, I think we should go with option c). For implementation you can use only > new "offloads", so it is not both old-style and new-style, just new-style, but > as default yes should keep both. > > And new applications will set ETH_TXQ_FLAGS_IGNORE to say "offloads" has valid > information. Currently this also converted to the txq_flags but this will go > away when all PMDs support new method. > > Think about a case where a legacy application gets defaults from PMD and sets a > few more values in txq_flags and configure the device. When you set > ETH_TXQ_FLAGS_IGNORE as part of defaults, ethdev layer will think only > "offloads" is valid and it will overwrite the txq_flags value with offloads one > and the updates to the txq_flags will be lost. > > At one point ETH_TXQ_FLAGS_IGNORE will also go away and applications also will > need to support new method. When it is removed than we can get rid of the > txq_flags defaults from PMDs until than I guess we need to live with them. So you suggest that dev_info callback should provide default txq_flags for a moment? E.g. .txq_flags = ETH_TXQ_FLAGS_NOMULTSEGS | ETH_TXQ_FLAGS_NOREFCOUNT | ETH_TXQ_FLAGS_NOMULTMEMP | ETH_TXQ_FLAGS_NOVLANOFFL | ETH_TXQ_FLAGS_NOXSUMSCTP, That is OK with me. We'll wipe it out later whet it all go away. Will fix in v2. >>> <...> >>> >>>> + if ((conf_tx_offloads & tx_offload_capa) != conf_tx_offloads) { >>>> + PMD_INIT_LOG(ERR, "Some Tx offloads are not supported " >>>> + "requested 0x%lx supported 0x%lx\n", >>>> + conf_tx_offloads, tx_offload_capa); >>> This is broken for 32bits, using PRIx64 instead of "lx" makes your code more >>> portable. >> Will fix in v2. >> >> >>