From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-f67.google.com (mail-wr1-f67.google.com [209.85.221.67]) by dpdk.org (Postfix) with ESMTP id 900A64CBB for ; Wed, 14 Nov 2018 16:16:23 +0100 (CET) Received: by mail-wr1-f67.google.com with SMTP id v18-v6so17699130wrt.8 for ; Wed, 14 Nov 2018 07:16:23 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=LGSby+Q8kySdpTW3mdUKmzFsk6aSfp4IjceDVUKwVLU=; b=cUll+CYjoA0u0eByBAGLoogrUQAM5CvVsCl3r0ETiuAiwgyog5/SCaVnSXOkFT37fZ aPyLVqTdUi6duih5y6kENQFbRhpPiXxu1aFVgzCLVQAhFpD5RIBAU4IfhqTcRI9L2Fyp 3QrQyFYB/AY9t7nosxA7dlVqNcDSItkfdnywSwWw8ExOea5vkHf70QZfFHwZkK2zslT/ hhW115hcu/cPsQXWeJ71NppnAqb07mMjcmr5/9mT9BTeoG+qJHSl4PItAVRh9NWvV05v /CAA1IOWsy1KCeDqsAdN8gEudp7EPtaDo9TpGcn5AKe8ADDvFciBn5C86acjZZzMrvN5 mc0A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=LGSby+Q8kySdpTW3mdUKmzFsk6aSfp4IjceDVUKwVLU=; b=Oc161F+0Lp6LkHv93M1HVCz2tG91zgmXR2xHDP7rwsFMhd2XngkG9gkFI5N9NORTjR DLMjaLz2JclDo4ntGPfmtcz9YA+p4fNS2DCR62dojCdxugHx9HuJbDhMxUSRfJ54W8/x OLqe1u/Wv3wCY4OaJptunpMeWowjU3nZEt+pUt4wl102k7Y6S7bTaR7kLJR1kQW+LWcw IwC/JraZxsnAfRKuNeBf5fEzhkV09kWys/sW7+5d1S9MDtbUSD+ijNF5S+KQkvk+3d4j BrqMqyiKVLqBw2W0K0lXIFzqTM0ckQPWFdmFwOTGWST4GQI2H6mkGyYgBvlfp1hRm/c8 us/w== X-Gm-Message-State: AGRZ1gJ09XuvsNqvEw8K6ebb4slRTdNk0lUjPDZ90cgQFIPQnBbQpUd0 Fk0ezPsqU1JaRPBKl2ngIXA0vg== X-Google-Smtp-Source: AJdET5foO5we+TlrNGqTuA9+ZeUNm3VQPFNvqTXHhad+EvVpWyuXR9HbXYVjcJVVM2XFlSkGH8U9Og== X-Received: by 2002:adf:bc0c:: with SMTP id s12-v6mr2377261wrg.255.1542208583241; Wed, 14 Nov 2018 07:16:23 -0800 (PST) Received: from 6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id b22sm2464462wmj.9.2018.11.14.07.16.20 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 14 Nov 2018 07:16:20 -0800 (PST) Date: Wed, 14 Nov 2018 16:16:01 +0100 From: Adrien Mazarguil To: Shahaf Shuler Cc: Ori Kam , Ophir Munk , Yongseok Koh , Andrew Rybchenko , Ferruh Yigit , "dev@dpdk.org" , Thomas Monjalon , Asaf Penso , Olga Shern Message-ID: <20181114151601.GH17131@6wind.com> References: <20181107164118.GM4638@6wind.com> <20181113171519.GF17131@6wind.com> <20181114094040.GG17131@6wind.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [dpdk-dev] [PATCH v2] ethdev: document RSS default key and types 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: Wed, 14 Nov 2018 15:16:23 -0000 On Wed, Nov 14, 2018 at 01:51:19PM +0000, Shahaf Shuler wrote: > IMO, it will make it more clear if the key will *have* to be null, because there is no single good reason to have it otherwise. > > However it looks like an endless debate between strict and relaxed API. there are points to both sides, yet we are failing to converge. > Mlx5 already implements according to the rss_key_len and rss_key approach. What are other PMD doing? > > Assuming there is a consensus among the PMDs, maybe we can follow it in order to avoid the extra work. > is it that critical for you to enforce only the key_len w/o the rss_key? Not at all, I don't mind extra checks in PMDs actually. To be clear, here's a list of what I consider valid PMD checks: - if (!key_len) use_default_key(); - if (key_len) { assert(key); use_app_key(); } - if (key_len) { if (!key) complain(); else use_app_key(); } - if (key_len) { if (!key) { complain(); use_default_key(); } else use_app_key(); } - if (key && key_len) use_app_key(); else use_default_key(); /* it's OK since the alternative is a crash */ - if (!key_len || !key) use_default_key(); /* ditto */ While those are invalid: - if (!key_len && !key) use_default_key(); /* err, else what? */ - if (!key_len) { assert(!key); use_default_key(); } /* unless you hate users */ - if (!key_len) { if (key) complain(); use_default_key(); } /* extra noise can be annoying */ What I'm most concerned with is rte_flow API documentation, that is, what we ask users to do in order to achieve something. A default behavior for a zero value is currently documented on "level" and "types" fields. "key_len" and "queue_num" are currently lacking this information. Just like "key", requesting RSS without providing a list of queues should default to all configured Rx queues for convenience. In that case the "queue" pointer can be undefined, not necessarily NULL because the PMD won't look for queues if that list anyway, its value doesn't matter. So documentation will describe that if "key_len" or "queue_num" are nonzero, non-default behavior is requested and the related "key"/"queue" fields must be set and valid. Otherwise they can remain undefined. > > > To this doc issue, > > > I don't understand on what cases it makes sense for application to have > > rss_key_len = 0 while rss_key != NULL. This is obviously in-consist input, and > > of course all PMD will just ignore the key. > > > I think enforcing rss_key and rss_key_len to be NULL is a fair requirement > > from application, and it makes no confusion in the API inputs. > > > > Then you need to define what happens when key_len != 0 and key == NULL, > > also when key_len == 0 and key != NULL, none of which make sense > > currently. > > Of course all are in-consist and the PMD is free to reject such rules. This is not related though to how we define the default RSS right? If we force users to set both key_len = 0 *and* key = NULL like you suggest in order to get the default behavior and key_len is not enough, I think it makes sense to also describe these two cases for consistency. Users are going to wonder why is the damn PMD reading key at all if its length is zero, so we have to explain that no, the PMD doesn't do that but the pointer still has to be NULL anyway. Likewise users shouldn't be tempted to enter a nonzero key length without a valid key. It also has to be described should we choose this approach. -- Adrien Mazarguil 6WIND