Re: [dpdk-dev] [RFC PATCH 0/5] graph: introduce graph subsystem
On Fri, Feb 21, 2020 at 9:44 PM Thomas Monjalon wrote: > > 21/02/2020 16:56, Jerin Jacob: > > On Fri, Feb 21, 2020 at 4:40 PM Thomas Monjalon wrote: > > > 21/02/2020 11:30, Jerin Jacob: > > > > On Mon, Feb 17, 2020 at 4:28 PM Jerin Jacob > > > > wrote: > > > > > On Mon, Feb 17, 2020 at 2:08 PM Thomas Monjalon > > > > > wrote: > > > > > > If we add rte_graph to DPDK, we will have 2 similar libraries. > > > > > > > > > > > > I already proposed several times to move rte_pipeline in a separate > > > > > > repository for two reasons: > > > > > > 1/ it is acting at a higher API layer level > > > > > > > > > > We need to define what is the higher layer API. Is it processing > > > > > beyond L2? > > > > > > My opinion is that any API which is implemented differently > > > for different hardware should be in DPDK. > > > > We need to define SIMD optimization(not HW specific to but > > architecture-specific) > > treatment as well, as the graph and node library will have SIMD > > optimization as well. > > I think SIMD optimization is generic to any performance-related project, > not specific to DPDK. > > > > In general, by the above policy enforced, we need to split DPDK like below, > > dpdk.git > > -- > > librte_compressdev > > librte_bbdev > > librte_eventdev > > librte_pci > > librte_rawdev > > librte_eal > > librte_security > > librte_mempool > > librte_mbuf > > librte_cryptodev > > librte_ethdev > > > > other repo(s). > > > > librte_cmdline > > librte_cfgfile > > librte_bitratestats > > librte_efd > > librte_latencystats > > librte_kvargs > > librte_jobstats > > librte_gso > > librte_gro > > librte_flow_classify > > librte_pipeline > > librte_net > > librte_metrics > > librte_meter > > librte_member > > librte_table > > librte_stack > > librte_sched > > librte_rib > > librte_reorder > > librte_rcu > > librte_power > > librte_distributor > > librte_bpf > > librte_ip_frag > > librte_hash > > librte_fib > > librte_timer > > librte_telemetry > > librte_port > > librte_pdump > > librte_kni > > librte_acl > > librte_vhost > > librte_ring > > librte_lpm > > librte_ipsec > > I think it is a fair conclusion of the scope I am arguing, yes. OK. See below. > > > What is expected to be maintained, tested, etc. > > > > We need to maintain and test other code in OTHER dpdk repo as well. > > Yes but the ones responsible are not the same. I see your point. Can I interpret it as you would like to NOT take responsibility of SW libraries(Items enumerated in the second list)? I think, the main question would be, how it will deliver to distros and/or end-users and what will be part of the dpdk release? I can think of two options. Maybe distro folks have better view on this. options 1: - Split dpdk to dpdk-core.git, dpdk-algo.git etc based on the functionalities and maintainer's availability. - Follow existing release cadence and deliver single release tarball with content from the above repos. options 2: - Introduce more subtrees(dpdk-next-algo.git etc) based on the functionalities and maintainer's availability. - Follow existing release cadence and have a pull request to main dpdk.git just like Linux kernel or existing scheme of things. I am for option 2. NOTE: This new graph and node library, I would like to make its new subtree in the existing scheme of things so that it will NOT be a burden for you to manage.
Re: [dpdk-dev] [RFC PATCH 0/5] graph: introduce graph subsystem
22/02/2020 10:05, Jerin Jacob: > On Fri, Feb 21, 2020 at 9:44 PM Thomas Monjalon wrote: > > 21/02/2020 16:56, Jerin Jacob: > > > On Fri, Feb 21, 2020 at 4:40 PM Thomas Monjalon > > > wrote: > > > > 21/02/2020 11:30, Jerin Jacob: > > > > > On Mon, Feb 17, 2020 at 4:28 PM Jerin Jacob > > > > > wrote: > > > > > > On Mon, Feb 17, 2020 at 2:08 PM Thomas Monjalon > > > > > > wrote: > > > > > > > If we add rte_graph to DPDK, we will have 2 similar libraries. > > > > > > > > > > > > > > I already proposed several times to move rte_pipeline in a > > > > > > > separate > > > > > > > repository for two reasons: > > > > > > > 1/ it is acting at a higher API layer level > > > > > > > > > > > > We need to define what is the higher layer API. Is it processing > > > > > > beyond L2? > > > > > > > > My opinion is that any API which is implemented differently > > > > for different hardware should be in DPDK. > > > > > > We need to define SIMD optimization(not HW specific to but > > > architecture-specific) > > > treatment as well, as the graph and node library will have SIMD > > > optimization as well. > > > > I think SIMD optimization is generic to any performance-related project, > > not specific to DPDK. > > > > > > > In general, by the above policy enforced, we need to split DPDK like > > > below, > > > dpdk.git > > > -- > > > librte_compressdev > > > librte_bbdev > > > librte_eventdev > > > librte_pci > > > librte_rawdev > > > librte_eal > > > librte_security > > > librte_mempool > > > librte_mbuf > > > librte_cryptodev > > > librte_ethdev > > > > > > other repo(s). > > > > > > librte_cmdline > > > librte_cfgfile > > > librte_bitratestats > > > librte_efd > > > librte_latencystats > > > librte_kvargs > > > librte_jobstats > > > librte_gso > > > librte_gro > > > librte_flow_classify > > > librte_pipeline > > > librte_net > > > librte_metrics > > > librte_meter > > > librte_member > > > librte_table > > > librte_stack > > > librte_sched > > > librte_rib > > > librte_reorder > > > librte_rcu > > > librte_power > > > librte_distributor > > > librte_bpf > > > librte_ip_frag > > > librte_hash > > > librte_fib > > > librte_timer > > > librte_telemetry > > > librte_port > > > librte_pdump > > > librte_kni > > > librte_acl > > > librte_vhost > > > librte_ring > > > librte_lpm > > > librte_ipsec > > > > I think it is a fair conclusion of the scope I am arguing, yes. > > OK. See below. > > > > > What is expected to be maintained, tested, etc. > > > > > > We need to maintain and test other code in OTHER dpdk repo as well. > > > > Yes but the ones responsible are not the same. > > I see your point. Can I interpret it as you would like to NOT take > responsibility > of SW libraries(Items enumerated in the second list)? It's not only about me. This is a community decision. > I think, the main question would be, how it will deliver to distros > and/or end-users > and what will be part of the dpdk release? > > I can think of two options. Maybe distro folks have better view on this. > > options 1: > - Split dpdk to dpdk-core.git, dpdk-algo.git etc based on the > functionalities and maintainer's availability. > - Follow existing release cadence and deliver single release tarball > with content from the above repos. > > options 2: > - Introduce more subtrees(dpdk-next-algo.git etc) based on the > functionalities and maintainer's availability. > - Follow existing release cadence and have a pull request to main > dpdk.git just like Linux kernel or existing scheme of things. > > I am for option 2. > > NOTE: This new graph and node library, I would like to make its new > subtree in the existing scheme of > things so that it will NOT be a burden for you to manage. The option 2 is to make maintainers life easier. Keeping all libraries in the same repository allows to have an unique release and a central place for the apps and docs. The option 1 may make contributors life easier if we consider adding new libraries can make contributions harder in case of dependencies. The option 1 makes also repositories smaller, so maybe easier to approach. It makes easier to fully validate testing and quality of a repository. Having separate packages makes easier to select what is distributed and supported. After years thinking about the scope of DPDK repository, I am still not sure which solution is best. I really would like to see more opinions, thanks.
Re: [dpdk-dev] [RFC PATCH 0/5] graph: introduce graph subsystem
On Sat, Feb 22, 2020 at 3:23 PM Thomas Monjalon wrote: > > 22/02/2020 10:05, Jerin Jacob: > > On Fri, Feb 21, 2020 at 9:44 PM Thomas Monjalon wrote: > > > 21/02/2020 16:56, Jerin Jacob: > > > > On Fri, Feb 21, 2020 at 4:40 PM Thomas Monjalon > > > > wrote: > > > > > 21/02/2020 11:30, Jerin Jacob: > > > > > > On Mon, Feb 17, 2020 at 4:28 PM Jerin Jacob > > > > > > wrote: > > > > > > > On Mon, Feb 17, 2020 at 2:08 PM Thomas Monjalon > > > > > > > wrote: > > > > > > > > If we add rte_graph to DPDK, we will have 2 similar libraries. > > > > > > > > > > > > > > > > I already proposed several times to move rte_pipeline in a > > > > > > > > separate > > > > > > > > repository for two reasons: > > > > > > > > 1/ it is acting at a higher API layer level > > > > > > > > > > > > > > We need to define what is the higher layer API. Is it processing > > > > > > > beyond L2? > > > > > > > > > > My opinion is that any API which is implemented differently > > > > > for different hardware should be in DPDK. > > > > > > > > We need to define SIMD optimization(not HW specific to but > > > > architecture-specific) > > > > treatment as well, as the graph and node library will have SIMD > > > > optimization as well. > > > > > > I think SIMD optimization is generic to any performance-related project, > > > not specific to DPDK. > > > > > > > > > > In general, by the above policy enforced, we need to split DPDK like > > > > below, > > > > dpdk.git > > > > -- > > > > librte_compressdev > > > > librte_bbdev > > > > librte_eventdev > > > > librte_pci > > > > librte_rawdev > > > > librte_eal > > > > librte_security > > > > librte_mempool > > > > librte_mbuf > > > > librte_cryptodev > > > > librte_ethdev > > > > > > > > other repo(s). > > > > > > > > librte_cmdline > > > > librte_cfgfile > > > > librte_bitratestats > > > > librte_efd > > > > librte_latencystats > > > > librte_kvargs > > > > librte_jobstats > > > > librte_gso > > > > librte_gro > > > > librte_flow_classify > > > > librte_pipeline > > > > librte_net > > > > librte_metrics > > > > librte_meter > > > > librte_member > > > > librte_table > > > > librte_stack > > > > librte_sched > > > > librte_rib > > > > librte_reorder > > > > librte_rcu > > > > librte_power > > > > librte_distributor > > > > librte_bpf > > > > librte_ip_frag > > > > librte_hash > > > > librte_fib > > > > librte_timer > > > > librte_telemetry > > > > librte_port > > > > librte_pdump > > > > librte_kni > > > > librte_acl > > > > librte_vhost > > > > librte_ring > > > > librte_lpm > > > > librte_ipsec > > > > > > I think it is a fair conclusion of the scope I am arguing, yes. > > > > OK. See below. > > > > > > > What is expected to be maintained, tested, etc. > > > > > > > > We need to maintain and test other code in OTHER dpdk repo as well. > > > > > > Yes but the ones responsible are not the same. > > > > I see your point. Can I interpret it as you would like to NOT take > > responsibility > > of SW libraries(Items enumerated in the second list)? > > It's not only about me. This is a community decision. OK. Let wait for community feedback. Probably we discuss more in public TB meeting in 26th Feb. > > > > I think, the main question would be, how it will deliver to distros > > and/or end-users > > and what will be part of the dpdk release? > > > > I can think of two options. Maybe distro folks have better view on this. > > > > options 1: > > - Split dpdk to dpdk-core.git, dpdk-algo.git etc based on the > > functionalities and maintainer's availability. > > - Follow existing release cadence and deliver single release tarball > > with content from the above repos. > > > > options 2: > > - Introduce more subtrees(dpdk-next-algo.git etc) based on the > > functionalities and maintainer's availability. > > - Follow existing release cadence and have a pull request to main > > dpdk.git just like Linux kernel or existing scheme of things. > > > > I am for option 2. > > > > NOTE: This new graph and node library, I would like to make its new > > subtree in the existing scheme of > > things so that it will NOT be a burden for you to manage. > > The option 2 is to make maintainers life easier. > Keeping all libraries in the same repository allows to have > an unique release and a central place for the apps and docs. > > The option 1 may make contributors life easier if we consider > adding new libraries can make contributions harder in case of dependencies. > The option 1 makes also repositories smaller, so maybe easier to approach. > It makes easier to fully validate testing and quality of a repository. > Having separate packages makes easier to select what is distributed and > supported. If the final dpdk release tarball looks same for option1 and option2 then I think, option 1 is overhead to manage intra repo dependency. I agree with Thomas, it is better to decide as a community what direction we need to take and align existing and new libraries with that scheme.
Re: [dpdk-dev] [PATCH v2] pmdinfogen: add SPDX license tag
27/09/2019 10:39, Hemant Agrawal: > This patch adds SPDX license tag to pmdinfogen files. > > These files are originally drived from kernel. > They are being used as binary tool to support internal > build. > > This patch requires license exception approval from > DPDK Technical Board and Governing Board. > > Signed-off-by: Hemant Agrawal > Acked-by: Neil Horman > --- > --- a/license/exceptions.txt > +++ b/license/exceptions.txt > @@ -7,12 +7,12 @@ Note that following licenses are not exceptions:- > - BSD-3-Clause > - Dual BSD-3-Clause OR GPL-2.0 > - Dual BSD-3-Clause OR LGPL-2.1 > - - GPL-2.0 (*Only for kernel code*) > + - GPL-2.0 (*Only for kernel code and tools*) GPL tools remain exceptions according to the Governing Board. I will drop the addition of tools in "not exceptions" list. > - > SPDX Identifier TB Approval Date GB Approval Date File name > - > -1. > +1.GPL-2.025-Sep-2019 TBD buildtools/pmdinfogen/pmdinfogen.* > > - The pmdinfogen exception was approved by the Governing Board on 12/18/2019. Applied with above changes, thanks.
Re: [dpdk-dev] [PATCH 3/5] devtools: add SPDX tag to load-devel-config
04/12/2019 16:55, Stephen Hemminger: > Trivial file was missing any license. > > Signed-off-by: Stephen Hemminger Acked-by: Thomas Monjalon Applied, thanks
Re: [dpdk-dev] [PATCH v2] add top level SPDX license identifier.
28/11/2019 08:05, Hemant Agrawal: > This patch adds top level SPDX license identifiers for some of the dpdk > source and scripts, where the copyright owners have not yet agreed to > replace the full BSD-3 license plate. > > This patch also add SPDX license tag for some of files with no > previous license plates. (DPDK is BSD-3) > > Signed-off-by: Hemant Agrawal > --- > app/test-pmd/flowgen.c| 7 ++- > app/test-pmd/macswap.c| 6 ++ > app/test/test_compressdev_test_buffer.h | 2 ++ > app/test/test_timer_racecond.c| 8 +++- > devtools/cocci.sh | 3 +-- > .../test/trs_aesgcm_inline_crypto_fallback_defs.sh| 2 +- > .../test/tun_aesgcm_inline_crypto_fallback_defs.sh| 2 +- > examples/performance-thread/l3fwd-thread/test.sh | 1 + > lib/librte_ethdev/rte_ethdev_pci.h| 6 ++ > lib/librte_ethdev/rte_ethdev_vdev.h | 6 ++ > 10 files changed, 17 insertions(+), 26 deletions(-) Some of these files are already fixed. Applied this patch for the remaining gap, thanks
Re: [dpdk-dev] [PATCH v3] cmdline: increase maximum line length
This patch is flagged as an ABI breakage: https://travis-ci.com/ovsrobot/dpdk/jobs/289313318#L2273 On Thu, Feb 20, 2020 at 3:53 PM Wisam Jaddo wrote: > > This increase due to the usage of cmdline in dpdk applications > as config commands such as testpmd do for rte_flow rules creation. > > The current size of buffer is not enough to fill > many cases of rte_flow commands validation/creation. > > rte_flow now can have outer items, inner items, modify > actions, meta data actions, duplicate action, fate action and > more in one single rte flow, thus 512 char will not be enough > to validate such rte flow rules. > > Such change shouldn't affect the memory since the cmdline > reading again using the same buffer. I don't get your point here. > Cc: sta...@dpdk.org This is not a fix. -- David Marchand
Re: [dpdk-dev] [PATCH v3] devtools: add new SPDX license compliance checker
07/02/2020 18:52, Stephen Hemminger: > Simple script to look for drivers and scripts that > are missing requires SPDX header. > > Signed-off-by: Stephen Hemminger > --- > devtools/spdx-check.sh | 24 For consistency with other scripts, it should be named check-spdx.sh Please add the new file in MAINTAINERS file in the section "Developers and Maintainers Tools". I think it should be mentioned also in the contributors guide. > --- /dev/null > +++ b/devtools/spdx-check.sh > @@ -0,0 +1,24 @@ > +#! /bin/sh > +# SPDX-License-Identifier: BSD-3-Clause > +# Copyright (c) 2019 Microsoft Corporation While talking about license, are you sure the (c) is required? > +# > +# Produce a list of files with incorrect license > +# information In order to add this script in a CI, it should return an error code. Based on the error code, the CI can report it as success, warning or failure to patchwork. > + > +echo "Files without SPDX License" > +echo "--" > + > +git grep -L SPDX-License-Identifier -- \ > +':^.git*' ':^.ci/*' ':^.travis.yml' \ > +':^README' ':^MAINTAINERS' ':^VERSION' ':^ABI_VERSION' \ > +':^*/Kbuild' ':^*/README' \ > +':^license/' ':^doc/' ':^config/' ':^buildtools/' \ > +':^*.cocci' ':^*.abignore' \ > +':^*.def' ':^*.map' ':^*.ini' ':^*.data' ':^*.cfg' ':^*.txt' This should be considered a critical error. > + > +echo > +echo "Files with additional license text" > +echo "--" > + > +git grep -l Redistribution -- \ > +':^license/' ':^/devtools/spdx-check.sh' This should be considered as a warning.
Re: [dpdk-dev] [PATCH v3] devtools: add new SPDX license compliance checker
07/02/2020 18:52, Stephen Hemminger: > Simple script to look for drivers and scripts that > are missing requires SPDX header. > > Signed-off-by: Stephen Hemminger [...] > +git grep -L SPDX-License-Identifier -- \ > +':^.git*' ':^.ci/*' ':^.travis.yml' \ > +':^README' ':^MAINTAINERS' ':^VERSION' ':^ABI_VERSION' \ > +':^*/Kbuild' ':^*/README' \ > +':^license/' ':^doc/' ':^config/' ':^buildtools/' \ > +':^*.cocci' ':^*.abignore' \ > +':^*.def' ':^*.map' ':^*.ini' ':^*.data' ':^*.cfg' ':^*.txt' I think we should plan to add SPDX tag to the doc (rst and svg files).
Re: [dpdk-dev] [PATCH v3] regexdev: introduce regexdev subsystem
> diff --git a/lib/librte_regexdev/rte_regexdev.h
> b/lib/librte_regexdev/rte_regexdev.h
> new file mode 100644
> index 000..c42128b
> --- /dev/null
> +++ b/lib/librte_regexdev/rte_regexdev.h
> @@ -0,0 +1,1411 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(C) 2019 Marvell International Ltd.
> + * Copyright(C) 2020 Mellanox International Ltd.
There are a few comments from Xiang as well. So let's add Intel also
to the list.
> + */
> +
> +#ifndef _RTE_REGEXDEV_H_
> +#define _RTE_REGEXDEV_H_
> +
> +/**
> + * RegEx device information
> + */
> +struct rte_regex_dev_info {
> + const char *driver_name; /**< RegEx driver name. */
> + struct rte_device *dev; /**< Device information. */
> + uint16_t max_matches;
> + /**< Maximum matches per scan supported by this device. */
> + uint16_t max_queue_pairs;
> + /**< Maximum queue pairs supported by this device. */
> + uint16_t max_payload_size;
> + /**< Maximum payload size for a pattern match request or scan.
> +* @see RTE_REGEX_DEV_CFG_CROSS_BUFFER_SCAN_F
> +*/
> + uint32_t max_rules_per_group;
> + /**< Maximum rules supported per group by this device.
> +* This number can't be larger then 20 bits.
s/then/than
I think, we don't need to say this " This number can't be larger than 20 bits."
It may help SW drivers.
> +*/
> + uint16_t max_groups;
> + /**< Maximum group supported by this device.
> +* This number can't be larger then 12 bits.
s/then/than
I think, we don't need to say this " This number can't be larger than 12 bits."
It may help SW drivers.
> +*/
> + uint32_t regex_dev_capa;
> + /**< RegEx device capabilities. @see RTE_REGEX_DEV_CAPA_* */
> + uint64_t rule_flags;
> + /**< Supported compiler rule flags.
> +* @see RTE_REGEX_PCRE_RULE_*, struct rte_regex_rule::rule_flags
> +*/
> + uint8_t max_scatter_gather;
> + /**< The max supported number of buffers that can
> +* be used in a single ops. The total size of all elements
> +* must be less then max_payload_size.
> +*/
> +};
> +int
> +rte_regex_rule_db_compile(uint8_t dev_id);
> +
I think your "rte_regex_rule_db_compile_activate() - compile and
activate the new rule set"
API name looks good. I am for rte_regex_rule_db_compile_activate().
> +/* Fast path APIs */
> +
> +/**
> + * The generic *rte_regex_match* structure to hold the RegEx match
> attributes.
> + * @see struct rte_regex_ops::matches
> + */
> +struct rte_regex_match {
> + RTE_STD_C11
> + union {
> + uint64_t u64;
> + struct {
> + uint32_t rule_id:20;
> + /**< Rule identifier to which the pattern matched.
> +* @see struct rte_regex_rule::rule_id
> +*/
> + uint32_t group_id:12;
> + /**< Group identifier of the rule which the pattern
> +* matched. @see struct rte_regex_rule::group_id
> +*/
> + uint16_t offset;
Since we have end_offset now, IMO, it is better to change this offset
to "start_offset".
> + /**< Starting Byte Position for matched rule. */
> + RTE_STD_C11
> + union {
> + uint16_t len;
> + /**< Length of match in bytes */
> + uint16_t end_offset;
> + /**< The end offset of the match. In case
> +* MATCH_AS_START configuration is disabled.
> +* @see RTE_REGEX_DEV_CFG_MATCH_AS_START
> +*/
We have not concluded on this scheme. Have one field which has
different meaning will be difficult
for application. i.e fast path we need to have a check for this.
I think, Based on the majority of HW/SW implementation, we need to
either go with len or
end_offset. What Mellanox HW returns? len or end_offset?
or We can keep it as len or end_offset based on which drivers upstream first,
other drivers when it comes, we can see how to abstract it?
> + };
> + };
> + };
> +};
> +/**
> + * The generic *rte_regex_ops* structure to hold the RegEx attributes
> + * for enqueue and dequeue operation.
> + */
> +struct rte_regex_ops {
> + /* W0 */
> + uint16_t req_flags;
> + /**< Request flags for the RegEx ops.
> +* @see RTE_REGEX_OPS_REQ_*
> +*/
> + uint16_t rsp_flags;
> + /**< Response flags for the RegEx ops.
> +* @see RTE_REGEX_OPS_RSP_*
> +*/
> + uint16_t nb_actual_matches;
> + /**< The total number of actual matches detected by the Regex
> device.*/
> + uint16_t nb_matches;
> + /**< The total
Re: [dpdk-dev] [PATCH v2] devtools: check c99 comment use in checkpatches.sh
15/02/2020 18:09, Lance Richardson: > C99-style comments are not permitted according to DPDK coding > style guidelines, enable checking for these by checkpatch.pl. > > Signed-off-by: Lance Richardson Applied, thanks
Re: [dpdk-dev] [PATCH v2] devtools: add EditorConfig file
25/10/2019 16:04, Robin Jarry: > EditorConfig is a file format and collection of text editor plugins for > maintaining consistent coding styles between different editors and IDEs. > > Initialize the file following the coding rules in > doc/guides/contributing/coding_style.rst, > doc/guides/contributing/documentation.rst and > doc/guides/contributing/patches.rst. > > In order for this file to be taken into account (unless they use an > editor with built-in EditorConfig support), developers will have to > install a plugin. > > Note: The max_line_length property is only supported by a limited number > of EditorConfig plugins. It will be ignored if unsupported. > > Add this new file in MAINTAINERS in the "Developers and Maintainers > Tools" section. > > Link: https://editorconfig.org/ > Link: https://github.com/editorconfig/editorconfig-emacs > Link: https://github.com/editorconfig/editorconfig-vim > Link: > https://github.com/editorconfig/editorconfig/wiki/EditorConfig-Properties#max_line_length > Cc: Thomas Monjalon > Cc: Stephen Hemminger > Cc: Anatoly Burakov > Cc: Keith Wiles > Cc: Ray Kinsella > Cc: Andrew Rybchenko > Signed-off-by: Robin Jarry > --- > Changelog: > > v2: > > - Add link to editorconfig.org in file for syntax reference. > - Use [*.EXT] syntax for section headers (as shown on home page). > - Add trim_trailing_whitespace option. This patch was forgotten, sorry. Applied, thanks it looks to be a good addition.
Re: [dpdk-dev] [PATCH] devtools: export title syntax for check-git-log
Hi, Sorry for not looking at this patch before. 25/10/2019 11:59, Sean Morrissey: > Moved title syntax to a separate file so that it improves code readability > and allows for easy addition of new correct title syntax in future cases. I think it is a good idea for a reason which is not said here: the logic is switched from checking bad patterns to checking good wording. > Signed-off-by: Sean Morrissey [...] > +data="$selfdir/commit-title-syntax.txt" The file is processed to check specifically the case of words, so it should be renamed to something like words-case.txt The file need to be added to MAINTAINERS file as well. The variable name "data" is too vague. I suggest "words". > +while IFS= read -r line We should avoid "while" loop because it is a sub-shell, so it forbids setting global variables as it is done in another patch collecting stats: https://patches.dpdk.org/patch/65243/ "for" loop should be fine. The variable name should be "word" I think. > +do > + regex=":.*\<$line\>" > + bad=$(echo "$headlines" | grep -i $regex | grep \ If using -o option of grep, we can capture the word and compare directly with the correct word. It will be also better when printing the error. > + -v ':.*\' ) I don't like having this exception but I have no better idea. We can move it before the first grep so it will still work with grep -o. > + if ! [ -z "$bad" ] > + then > + bad=$(echo "$headlines" | grep --color=always -v $regex \ > + | grep --color=always -i $regex \ > + | sed 's,^,\t,') > + [ -z "$bad" ] || printf "Wrong headline case:\n$bad\n" As said above, the word can be printed alone if using grep -o. > + fi > +done < "$data" > > # special case check for VMDq to give good error message > bad=$(echo "$headlines" | grep -E --color=always \ The VMDq case should be moved as well. > --- /dev/null > +++ b/devtools/commit-title-syntax.txt > @@ -0,0 +1,45 @@ > +Rx > +Tx > +PF > +VF > +HW > +SW > +FW > +L2 > +L3 > +L4 > +API > +arm The right syntax is Arm. > +aarch64 aarch32 is missing. > +armv7 > +armv8 > +CRC > +DCB > +DMA > +EEPROM > +FreeBSD > +IOVA > +LACP > +Linux > +LRO > +LSC > +MAC > +MSS > +MTU > +NIC > +NVM > +NUMA > +PCI > +PHY > +PMD > +RETA > +RSS > +SCTP > +TOS > +TPID > +TSO > +TTL > +UDP > +VLAN > +VDPA The right syntax is vDPA > +VSI
Re: [dpdk-dev] [PATCH 1/2] devtools: standardize script arguments
Hi,
Thanks for improving tooling.
28/01/2020 16:02, Ciara Power:
> range=${1:-origin/master..}
If doing a real option management, range should be the remaining argument
after option parsing.
> +if [ "$range" = '--help' ] ; then
> + print_usage
Missing "exit 0" after usage.
> # convert -N to HEAD~N.. in order to comply with git-log-fixes.sh getopts
> -if printf -- $range | grep -q '^-[0-9]\+' ; then
> - range="HEAD$(printf -- $range | sed 's,^-,~,').."
> +elif printf -- "$range" | grep -q '^-[0-9]\+' ; then
> + range="HEAD$(printf -- "$range" | sed 's,^-,~,').."
getopts won't be called if $1 starts with -N.
I think it would be cleaner to handle this in "?" case below.
> +else
> + while getopts hr:n: ARG ; do
> + case $ARG in
> + n ) range="HEAD~$OPTARG.." ;;
> + r ) range=$OPTARG ;;
-r is not a git-log option.
Please handle it without the need for -r.
> + h ) print_usage ; exit 0 ;;
> + ? ) print_usage ; exit 1 ;;
> + esac
> + done
> + shift $(($OPTIND - 1))
Re: [dpdk-dev] [PATCH 2/2] devtools: added stats print
28/01/2020 16:02, Ciara Power: > When all checks are completed on the specified commit logs, the script > indicates if all are valid, or if there were some failures. I think the most important would be to have an error code. Please could you exit with 0 only if no issue is found? [...] > +total=$(echo "$commits" | wc -l) > +if $failure ; then > + printf "\nInvalid patch(es) found - checked $total patch" > +else > + printf "\n$total/$total valid patch" > +fi > +[ $total -le 1 ] || printf 'es' > +printf '\n'
[dpdk-dev] [dpdk-announce] release candidate 20.02-rc4
A new DPDK release candidate is ready for testing: https://git.dpdk.org/dpdk/tag/?id=v20.02-rc4 76 patches were integrated. This is small enough to be confident about closing 20.02 soon. The release notes so far: http://doc.dpdk.org/guides/rel_notes/release_20_02.html Please add your tested platforms to the list: http://doc.dpdk.org/guides/rel_notes/release_20_02.html#tested-platforms Some deprecation notices are pending for review. This is the last release candidate for DPDK 20.02. Please share some release validation results by replying to this message (at dev@dpdk.org). If no critical issue is reported, 20.02 will be closed on Tuesday 25th. If you are preparing the next release cycle, please send your v1 patches before the 20.05 proposal deadline, which will happen on March 18th. It is also time to build an estimated roadmap for the next cycles. Thank you everyone
[dpdk-dev] Question about vhost user interrupt mode
Hi all,
Right now on OVS, dpdkvhostuser can only run in polling mode (please
correct me if I am wrong). We are trying to enable interrupt mode of
dpdkvhostuser type port on OVS. We found that, with changes below, OVS
can poll&block on exposed kickfd and vhostuser is working under
interrupt mode without consuming 2 CPUs.
My question is, is this the correct direction to do so, or is there a
better way? Thanks!
--- a/lib/librte_vhost/vhost.h
+++ b/lib/librte_vhost/vhost.h
@@ -318,7 +318,6 @@ struct vring_packed_desc_event {
(1ULL << VIRTIO_NET_F_GUEST_UFO) | \
(1ULL << VIRTIO_NET_F_GUEST_ECN) | \
(1ULL << VIRTIO_RING_F_INDIRECT_DESC) | \
- (1ULL << VIRTIO_RING_F_EVENT_IDX) | \
(1ULL << VIRTIO_NET_F_MTU) | \
(1ULL << VIRTIO_F_IN_ORDER) | \
(1ULL << VIRTIO_F_IOMMU_PLATFORM) | \
+int rte_vhost_get_kickfd(int vid, uint16_t queue_id)
+{
+ struct virtio_net *dev;
+ struct vhost_virtqueue *vq;
+
+ dev = get_device(vid);
+ if (!dev)
+ return -1;
+
+ if (unlikely(!(dev->flags & VIRTIO_DEV_BUILTIN_VIRTIO_NET))) {
+ // vhost net backend is disabled.
+ return -1;
+ }
+
+ if (unlikely(!is_valid_virt_queue_idx(queue_id, 1, dev->nr_vring))) {
+ return -1;
+ }
+
+ vq = dev->virtqueue[queue_id];
+ // XXX lock?
+ return vq->kickfd;
+}
Best,
Yifeng Sun
