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 BDB6541BBB; Fri, 3 Feb 2023 10:45:12 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 9F6964021E; Fri, 3 Feb 2023 10:45:12 +0100 (CET) Received: from mail-vk1-f170.google.com (mail-vk1-f170.google.com [209.85.221.170]) by mails.dpdk.org (Postfix) with ESMTP id 803614014F for ; Fri, 3 Feb 2023 10:45:11 +0100 (CET) Received: by mail-vk1-f170.google.com with SMTP id 6so2286777vko.7 for ; Fri, 03 Feb 2023 01:45:11 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=iZItQnGMDxiuzENvUec8ASmRXWzvs+4wG/JhKTz/UC4=; b=ciRjDj8N/ETiU7PfbAwX6pqvTdalVqC/1gQXH8I3SGOoi8/E9pPt+VNRNZwWG5HsrK 143YcM7Be1et3J3EAzhR01sNpqotYSGIOEJKKYUUC2YvyfkVepT1zwpt7m1bLdke7Qg1 oFi9ZcT59TxUavm7Vsj1rLmPEcW2rasskYxMzSPX9xPjYGHea93UsVkNCzimAYYo0VYd RZ3YQAXd1idmuYjocGbnOx78Y1ogFz28FhWUbGeBz+pfI7/iA/ToiC++vO2mIOb5Jfyk IPV24VxukSD+DweRCvRQ/i6VwK0yycK2zaqDKZ0hj/GbdFYnxEzvQ1sq7dh+BzUttVEo BpOA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=iZItQnGMDxiuzENvUec8ASmRXWzvs+4wG/JhKTz/UC4=; b=hqVpzFtgVZ7ekHx/xTotm51rfAbpd6ErurIRKLgip/oD9QQNuFtJt00cz9Kjbge11T T6CF8gh7vcAkN80TjvmMnHrGQLdSClIYgMIyLOloYdoYDMmfPhN/C6gA8shXLCy0bq/E QSf88Vb71QduX1zi/zzS5HIwqYNxKg8GuazNXczBct4DAF9LMTQWKdhtw3b3CNZxIxU/ W8TX/kIgypThp8J2K3/SRv7PJQbpvArMRsv9ne9Yf0x/4HHVeiogIjPWisDxIJ0/nHB/ IkgV6/ac2St3diyXFj9ZifL6GYRVQBLiOaChBpsdVrG/+d/atIoA8rlrJXF89JNvh9ZS CkHw== X-Gm-Message-State: AO0yUKVZerC/4cZu1idLpIiM9WqLdq2ykcXTpVtiaCf5vOIJmUwPxpzn 7yMLxphVt7u2xpB4iwMjcaa8DkD7vjXuSdeHVZ8= X-Google-Smtp-Source: AK7set+fwor5wwQRrYMs8SL9vPJn37TEIKvnWSYrBm57piFd9XzVcFldEZS1NNBV7VjDDhOm+uzw/20j9Mz7y69mzF4= X-Received: by 2002:a05:6122:189c:b0:3e1:cd5f:ea24 with SMTP id bi28-20020a056122189c00b003e1cd5fea24mr1472537vkb.29.1675417510860; Fri, 03 Feb 2023 01:45:10 -0800 (PST) MIME-Version: 1.0 References: <20230107161852.3708690-1-s.v.naga.harish.k@intel.com> <20230123180458.486189-1-s.v.naga.harish.k@intel.com> In-Reply-To: From: Jerin Jacob Date: Fri, 3 Feb 2023 15:14:44 +0530 Message-ID: Subject: Re: [PATCH v2 1/3] eventdev/eth_rx: add params set/get APIs To: "Naga Harish K, S V" Cc: "jerinj@marvell.com" , "Carrillo, Erik G" , "Gujjar, Abhinandan S" , "dev@dpdk.org" , "Jayatheerthan, Jay" Content-Type: text/plain; charset="UTF-8" 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 On Thu, Feb 2, 2023 at 9:42 PM Naga Harish K, S V wrote: > > Hi Jerin, > > > -----Original Message----- > > From: Jerin Jacob > > Sent: Monday, January 30, 2023 8:13 PM > > To: Naga Harish K, S V > > Cc: jerinj@marvell.com; Carrillo, Erik G ; Gujjar, > > Abhinandan S ; dev@dpdk.org; > > Jayatheerthan, Jay > > Subject: Re: [PATCH v2 1/3] eventdev/eth_rx: add params set/get APIs > > > > On Mon, Jan 30, 2023 at 3:26 PM Naga Harish K, S V > > wrote: > > > > > > > > > > > > > -----Original Message----- > > > > From: Jerin Jacob > > > > Sent: Saturday, January 28, 2023 4:24 PM > > > > To: Naga Harish K, S V > > > > Cc: jerinj@marvell.com; Carrillo, Erik G > > > > ; Gujjar, Abhinandan S > > > > ; dev@dpdk.org; Jayatheerthan, Jay > > > > > > > > Subject: Re: [PATCH v2 1/3] eventdev/eth_rx: add params set/get APIs > > > > > > > > On Wed, Jan 25, 2023 at 10:02 PM Naga Harish K, S V > > > > wrote: > > > > > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > From: Jerin Jacob > > > > > > > > > > > > > > > > > > > > > > > > + */ > > > > > > > > > > > + uint32_t rsvd[15]; > > > > > > > > > > > + /**< Reserved fields for future use */ > > > > > > > > > > > > > > > > > > > > Introduce rte_event_eth_rx_adapter_runtime_params_init() > > > > > > > > > > to > > > > > > make > > > > > > > > > > sure rsvd is zero. > > > > > > > > > > > > > > > > > > > > > > > > > > > > The reserved fields are not used by the adapter or application. > > > > > > > > > Not sure Is it necessary to Introduce a new API to clear > > > > > > > > > reserved > > > > fields. > > > > > > > > > > > > > > > > When adapter starts using new fileds(when we add new fieds > > > > > > > > in future), the old applicaiton which is not using > > > > > > > > rte_event_eth_rx_adapter_runtime_params_init() may have > > junk > > > > > > > > value and then adapter implementation will behave bad. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > does it mean, the application doesn't re-compile for the new DPDK? > > > > > > > > > > > > Yes. No need recompile if ABI not breaking. > > > > > > > > > > > > > When some of the reserved fields are used in the future, the > > > > > > > application > > > > > > also may need to be recompiled along with DPDK right? > > > > > > > As the application also may need to use the newly consumed > > > > > > > reserved > > > > > > fields? > > > > > > > > > > > > The problematic case is: > > > > > > > > > > > > Adapter implementation of 23.07(Assuming there is change params) > > > > > > field needs to work with application of 23.03. > > > > > > rte_event_eth_rx_adapter_runtime_params_init() will sove that. > > > > > > > > > > > > > > > > As rte_event_eth_rx_adapter_runtime_params_init() initializes only > > > > reserved fields to zero, it may not solve the issue in this case. > > > > > > > > rte_event_eth_rx_adapter_runtime_params_init() needs to zero all > > > > fields, not just reserved field. > > > > The application calling sequence is > > > > > > > > struct my_config c; > > > > rte_event_eth_rx_adapter_runtime_params_init(&c) > > > > c.interseted_filed_to_be_updated = val; > > > > > > > Can it be done like > > > struct my_config c = {0}; > > > c.interseted_filed_to_be_updated = val; and update Doxygen > > > comments to recommend above usage to reset all fields? > > > This way, rte_event_eth_rx_adapter_runtime_params_init() can be > > avoided. > > > > Better to have a function for documentation clarity. Similar scheme already > > there in DPDK. See rte_eth_cman_config_init() > > > > > > > The reference function rte_eth_cman_config_init() is resetting the params struct and initializing the required params with default values in the pmd cb. No need for PMD cb. > The proposed rte_event_eth_rx_adapter_runtime_params_init () API just needs to reset the params struct. There are no pmd CBs involved. > Having an API just to reset the struct seems overkill. What do you think? It is slow path API. Keeping it as function is better. Also, it helps the documentations of config parm in rte_event_eth_rx_adapter_runtime_params_config() like, This structure must be initialized with rte_event_eth_rx_adapter_runtime_params_init() or so. > > > > > > > > Let me share an example and you can tell where is the issue > > > > > > > > 1)Assume parameter structure is 64B and for 22.03 8B are used. > > > > 2)rte_event_eth_rx_adapter_runtime_params_init() will clear all 64B. > > > > 3)There is an application written based on 22.03 which using only 8B > > > > after calling rte_event_eth_rx_adapter_runtime_params_init() > > > > 4)Assume, in 22.07 another 8B added to structure. > > > > 5)Now, the application (3) needs to run on 22.07. Since the > > > > application is calling > > > > rte_event_eth_rx_adapter_runtime_params_init() > > > > and 9 to 15B are zero, the implementation will not go bad. > > > > > > > > > The old application only tries to set/get previous valid fields > > > > > and the newly > > > > used fields may still contain junk value. > > > > > If the application wants to make use of any the newly used params, > > > > > the > > > > application changes are required anyway. > > > > > > > > Yes. If application wants to make use of newly added features. No > > > > need to change if new features are not needed for old application.