DPDK patches and discussions
 help / color / mirror / Atom feed
From: Ferruh Yigit <ferruh.yigit@intel.com>
To: Hemant Agrawal <hemant.agrawal@nxp.com>, "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH v5 1/8] net/dpaa: add support for fmlib in dpdk
Date: Wed, 26 Aug 2020 22:20:32 +0100
Message-ID: <21c30604-35eb-d116-deb3-5e1e59ee797e@intel.com> (raw)
In-Reply-To: <AM6PR04MB4456FFCC76898721575FD87989540@AM6PR04MB4456.eurprd04.prod.outlook.com>

On 8/26/2020 6:06 PM, Hemant Agrawal wrote:
> HI Ferruh,
> 
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>> Sent: Wednesday, August 26, 2020 8:22 PM
>> To: Hemant Agrawal <hemant.agrawal@nxp.com>; dev@dpdk.org
>> Subject: Re: [PATCH v5 1/8] net/dpaa: add support for fmlib in dpdk
>>
>> On 8/26/2020 2:54 PM, Ferruh Yigit wrote:
>>> On 8/13/2020 7:01 PM, Hemant Agrawal wrote:
>>>> DPAA platorm MAC interface is known as FMAN i.e. Frame Manager.
>>>> There are two ways to control it.
>>>> 1. Statically configure the queues and classification rules before
>>>> the start of the application using FMC tool.
>>>> 2. Dynamically configure it within application by making API calls of
>>>> fmlib.
>>>>
>>>> The fmlib or Frame Manager library provides an API on top of the
>>>> Frame Manager driver ioctl calls, that provides a user space
>>>> application with a simple way to configure driver parameters and PCD
>>>> (parse - classify - distribute) rules.
>>
>>
>> Hi Hemant,
>>
>> This patch is missing some serious documentation, fmlib support depends on
>> some kernel module(s) which doesn't mention anywhere as dependency,
>> and what are those modules, where can we find them, what is the licensing
>> of them etc all missing.
> 
> [Hemant] ok
>>
>> Also there is some code in the patchset that assumes there is a generated
>> binary in "/tmp/fmc.bin", the documentation of how this binary generated,
>> where can we find tool to generate binary is missing.
>> Should we rely on a binary should exist in a tmp folder is something else..
>>
>> There is some licensing concerns, some code is GPL-2 dual licensed, can't
>> they licensed as BSD-3?
>>
> [Hemant] We can change the licensing of this code as we are anyway changing the files. 
>

OK

> 
>> Last thing is there is still some clutter in the code from linux, and there are
>> still formatting/syntax issues...
> 
> [Hemant] ok
>>
>>>>
>>>> This patch integrates the base fmlib so that various queue config,
>>>> RSS and classification related features can be supported on DPAA
>> platform.
>>>>
>>>> Signed-off-by: Sachin Saxena <sachin.saxena@nxp.com>
>>>> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
>>>> ---
>>>> v4: adapting to DPDK style
>>>> v5: removing shared compilation issue and spelling errors
>>>>
>>>>  drivers/net/dpaa/Makefile                 |    4 +-
>>>
>>> For this release, v20.11, all Makefile changes can be dropped, since
>>> make build system will be removed soon.
>>>
>>> <...>
>>>
>>>> +
>>>> +/* #define FM_LIB_DBG */
>>>
>>> What about adding this as a config option, to enable & disable the
>>> debug, instead of having as a commented code?
>>> And overall why not use this as runtime configurable logging, instead
>>> of compile time?
>>>
>>>> +
>>>> +#if defined(FM_LIB_DBG)
>>>> +	#define _fml_dbg(format, arg...) \
>>>> +	printf("fmlib [%s:%u] - " format, \
>>>> +		__func__, __LINE__, ##arg)
>>>
>>> Please use DPDK logging, instead of 'printf'.
>>>
> [Hemant] ok
> 
>>> Btw, this file has dual license, we have dual license for the code
>>> that is shared between kernel and DPDK, if this is shared with kernel
>>> how can have the 'printf'?
>>> Should debug related macros moved to some other header?
>>>
>>> <...>
>>>
>>>> +
>>>> +uint32_t fm_pcd_disable(t_handle h_fm_pcd) {
>>>> +	t_device	*p_dev = (t_device *)h_fm_pcd;
>>>
>>>
>>> Since this is not shared but DPDK only code (that was the outcome of
>>> the techboard discussion), can you pleae follow the DPDK coding
>>> convention to the file. There are multiple samples doesn't match the
>>> convention, I won't comment on other instances.
> [Hemant]  We have changed the Hungarian notation to all small case. 
> Will you please help me with one example here.  How you want to see it?
> 

My comment was on tab between type (t_device) and variable name (p_dev), this
exists in many places.
Also there is an issue on how the function definition syntax should be, return
type in above line etc ...
A reminder of the DPDK coding sytle:
https://doc.dpdk.org/guides/contributing/coding_style.html

> 
>>>
>>> <...>
>>>
>>>> +#if defined(CONFIG_COMPAT)
>>>> +#define FM_PCD_IOC_PRS_LOAD_SW_COMPAT \
>>>> +	_IOW(FM_IOC_TYPE_BASE, FM_PCD_IOC_NUM(3), \
>>>> +	     ioc_compat_fm_pcd_prs_sw_params_t)
>>>> +#endif
>>>
>>> 'CONFIG_COMPAT' is for Linux, do we need it for the DPDK copy?
>>>
> [Hemant] We were trying to avoid changes in common files with Linux. 
> 

For example for the Intel common code, there are #ifdefs that are parsed and
output generated based on project you are upstreaming. So both Linux and DPDK
(and others) code generated from same common source, can something similar be
done here?
We know that 'CONFIG_COMPAT' is not something exists in DPDK, doesn't sound
right to have that piece of code in DPDK.

  reply	other threads:[~2020-08-26 21:20 UTC|newest]

Thread overview: 81+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-10 17:19 [dpdk-dev] [PATCH v2 1/9] net/dpaa: support Rxq and Txq info routines Hemant Agrawal
2020-07-10 17:19 ` [dpdk-dev] [PATCH v2 2/9] net/dpaa: add support for fmlib in dpdk Hemant Agrawal
2020-07-10 17:19 ` [dpdk-dev] [PATCH v2 3/9] net/dpaa: add VSP support in FMLIB Hemant Agrawal
2020-07-10 17:19 ` [dpdk-dev] [PATCH v2 4/9] net/dpaa: add support for fmcless mode Hemant Agrawal
2020-07-10 17:19 ` [dpdk-dev] [PATCH v2 5/9] bus/dpaa: add shared MAC support Hemant Agrawal
2020-07-10 17:19 ` [dpdk-dev] [PATCH v2 6/9] bus/dpaa: add Virtual Storage Profile port init Hemant Agrawal
2020-07-10 17:19 ` [dpdk-dev] [PATCH v2 7/9] net/dpaa: add support for Virtual Storage Profile Hemant Agrawal
2020-07-10 17:19 ` [dpdk-dev] [PATCH v2 8/9] net/dpaa: add fmc parser support for VSP Hemant Agrawal
2020-07-10 17:19 ` [dpdk-dev] [PATCH v2 9/9] net/dpaa: add RSS update func with FMCless Hemant Agrawal
2020-07-11  8:17 ` [dpdk-dev] [PATCH v3 1/8] net/dpaa: add support for fmlib in dpdk Hemant Agrawal
2020-07-11  8:17   ` [dpdk-dev] [PATCH v3 2/8] net/dpaa: add VSP support in FMLIB Hemant Agrawal
2020-07-11  8:17   ` [dpdk-dev] [PATCH v3 3/8] net/dpaa: add support for fmcless mode Hemant Agrawal
2020-07-11  8:17   ` [dpdk-dev] [PATCH v3 4/8] bus/dpaa: add shared MAC support Hemant Agrawal
2020-07-11  8:17   ` [dpdk-dev] [PATCH v3 5/8] bus/dpaa: add Virtual Storage Profile port init Hemant Agrawal
2020-07-11  8:17   ` [dpdk-dev] [PATCH v3 6/8] net/dpaa: add support for Virtual Storage Profile Hemant Agrawal
2020-07-11  8:17   ` [dpdk-dev] [PATCH v3 7/8] net/dpaa: add fmc parser support for VSP Hemant Agrawal
2020-07-11  8:17   ` [dpdk-dev] [PATCH v3 8/8] net/dpaa: add RSS update func with FMCless Hemant Agrawal
2020-07-17 11:36   ` [dpdk-dev] [PATCH v3 1/8] net/dpaa: add support for fmlib in dpdk Ferruh Yigit
2020-07-19 20:10     ` Thomas Monjalon
2020-07-20  4:50       ` Hemant Agrawal
2020-07-20 17:06         ` Thomas Monjalon
2020-07-21  3:26           ` Hemant Agrawal
2020-07-20 18:42         ` Stephen Hemminger
2020-07-28 13:41         ` David Marchand
2020-07-29  6:39           ` Hemant Agrawal
2020-07-29 12:07             ` Thomas Monjalon
2020-07-29 14:33             ` Kevin Traynor
2020-07-20  9:51       ` Ferruh Yigit
2020-08-11 12:29   ` [dpdk-dev] [PATCH v4 " Hemant Agrawal
2020-08-11 12:29     ` [dpdk-dev] [PATCH v4 2/8] net/dpaa: add VSP support in FMLIB Hemant Agrawal
2020-08-11 12:29     ` [dpdk-dev] [PATCH v4 3/8] net/dpaa: add support for fmcless mode Hemant Agrawal
2020-08-11 12:29     ` [dpdk-dev] [PATCH v4 4/8] bus/dpaa: add shared MAC support Hemant Agrawal
2020-08-11 12:29     ` [dpdk-dev] [PATCH v4 5/8] bus/dpaa: add Virtual Storage Profile port init Hemant Agrawal
2020-08-11 12:29     ` [dpdk-dev] [PATCH v4 6/8] net/dpaa: add support for Virtual Storage Profile Hemant Agrawal
2020-08-11 12:30     ` [dpdk-dev] [PATCH v4 7/8] net/dpaa: add fmc parser support for VSP Hemant Agrawal
2020-08-11 12:30     ` [dpdk-dev] [PATCH v4 8/8] net/dpaa: add RSS update func with FMCless Hemant Agrawal
2020-08-13 18:01     ` [dpdk-dev] [PATCH v5 1/8] net/dpaa: add support for fmlib in dpdk Hemant Agrawal
2020-08-13 18:01       ` [dpdk-dev] [PATCH v5 2/8] net/dpaa: add VSP support in FMLIB Hemant Agrawal
2020-08-13 18:01       ` [dpdk-dev] [PATCH v5 3/8] net/dpaa: add support for fmcless mode Hemant Agrawal
2020-08-13 18:01       ` [dpdk-dev] [PATCH v5 4/8] bus/dpaa: add shared MAC support Hemant Agrawal
2020-08-13 18:01       ` [dpdk-dev] [PATCH v5 5/8] bus/dpaa: add Virtual Storage Profile port init Hemant Agrawal
2020-08-13 18:01       ` [dpdk-dev] [PATCH v5 6/8] net/dpaa: add support for Virtual Storage Profile Hemant Agrawal
2020-08-13 18:01       ` [dpdk-dev] [PATCH v5 7/8] net/dpaa: add fmc parser support for VSP Hemant Agrawal
2020-08-13 18:01       ` [dpdk-dev] [PATCH v5 8/8] net/dpaa: add RSS update func with FMCless Hemant Agrawal
2020-08-26 13:54       ` [dpdk-dev] [PATCH v5 1/8] net/dpaa: add support for fmlib in dpdk Ferruh Yigit
2020-08-26 14:52         ` Ferruh Yigit
2020-08-26 17:06           ` Hemant Agrawal
2020-08-26 21:20             ` Ferruh Yigit [this message]
2020-09-01 12:36       ` [dpdk-dev] [PATCH v6 " Hemant Agrawal
2020-09-01 12:36         ` [dpdk-dev] [PATCH v6 2/8] net/dpaa: add VSP support in FMLIB Hemant Agrawal
2020-09-01 12:36         ` [dpdk-dev] [PATCH v6 3/8] net/dpaa: add support for fmcless mode Hemant Agrawal
2020-09-01 12:36         ` [dpdk-dev] [PATCH v6 4/8] bus/dpaa: add shared MAC support Hemant Agrawal
2020-09-01 12:36         ` [dpdk-dev] [PATCH v6 5/8] bus/dpaa: add Virtual Storage Profile port init Hemant Agrawal
2020-09-01 12:36         ` [dpdk-dev] [PATCH v6 6/8] net/dpaa: add support for Virtual Storage Profile Hemant Agrawal
2020-09-01 12:36         ` [dpdk-dev] [PATCH v6 7/8] net/dpaa: add fmc parser support for VSP Hemant Agrawal
2020-09-01 12:36         ` [dpdk-dev] [PATCH v6 8/8] net/dpaa: add RSS update func with FMCless Hemant Agrawal
2020-09-01 15:48         ` [dpdk-dev] [PATCH v6 1/8] net/dpaa: add support for fmlib in dpdk Ferruh Yigit
2020-09-02  5:15           ` Hemant Agrawal
2020-09-02 13:32             ` Ferruh Yigit
2020-09-03  3:24               ` Hemant Agrawal
2020-09-03 19:54                 ` Ferruh Yigit
2020-09-04  8:29         ` [dpdk-dev] [PATCH v7 1/7] net/dpaa: add VSP support in FMLIB Hemant Agrawal
2020-09-04  8:29           ` [dpdk-dev] [PATCH v7 2/7] net/dpaa: add support for fmcless mode Hemant Agrawal
2020-09-04  8:29           ` [dpdk-dev] [PATCH v7 3/7] bus/dpaa: add shared MAC support Hemant Agrawal
2020-09-04  8:29           ` [dpdk-dev] [PATCH v7 4/7] bus/dpaa: add Virtual Storage Profile port init Hemant Agrawal
2020-09-04  8:29           ` [dpdk-dev] [PATCH v7 5/7] net/dpaa: add support for Virtual Storage Profile Hemant Agrawal
2020-09-04  8:29           ` [dpdk-dev] [PATCH v7 6/7] net/dpaa: add fmc parser support for VSP Hemant Agrawal
2020-09-04  8:29           ` [dpdk-dev] [PATCH v7 7/7] net/dpaa: add RSS update func with FMCless Hemant Agrawal
2020-09-04  8:39           ` [dpdk-dev] [PATCH v8 1/8] net/dpaa: add support for fmlib in dpdk Hemant Agrawal
2020-09-04  8:39             ` [dpdk-dev] [PATCH v8 2/8] net/dpaa: add VSP support in FMLIB Hemant Agrawal
2020-09-04  8:39             ` [dpdk-dev] [PATCH v8 3/8] net/dpaa: add support for fmcless mode Hemant Agrawal
2020-09-04  8:39             ` [dpdk-dev] [PATCH v8 4/8] bus/dpaa: add shared MAC support Hemant Agrawal
2020-09-04  8:39             ` [dpdk-dev] [PATCH v8 5/8] bus/dpaa: add Virtual Storage Profile port init Hemant Agrawal
2020-09-04  8:39             ` [dpdk-dev] [PATCH v8 6/8] net/dpaa: add support for Virtual Storage Profile Hemant Agrawal
2020-09-04  8:39             ` [dpdk-dev] [PATCH v8 7/8] net/dpaa: add fmc parser support for VSP Hemant Agrawal
2020-09-04  8:39             ` [dpdk-dev] [PATCH v8 8/8] net/dpaa: add RSS update func with FMCless Hemant Agrawal
2020-09-04 12:51             ` [dpdk-dev] [PATCH v8 1/8] net/dpaa: add support for fmlib in dpdk Ferruh Yigit
2020-09-08  9:55               ` Ferruh Yigit
2020-09-08 10:19                 ` Thomas Monjalon
2020-09-08 12:10                   ` Ferruh Yigit
2020-09-09 11:16                     ` [dpdk-dev] [dpdk-ci] " Ferruh Yigit

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=21c30604-35eb-d116-deb3-5e1e59ee797e@intel.com \
    --to=ferruh.yigit@intel.com \
    --cc=dev@dpdk.org \
    --cc=hemant.agrawal@nxp.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git