RE: [PATCH v2] graph: fix head move when graph walk in mcore dispatch
> -Original Message-
> From: Wu, Jingjing
> Sent: Friday, March 22, 2024 11:47 PM
> To: dev@dpdk.org
> Cc: Wu, Jingjing ; jer...@marvell.com;
> pbhagavat...@marvell.com; Yan, Zhirun
> Subject: [PATCH v2] graph: fix head move when graph walk in mcore dispatch
>
> Head move happens before the core id check, which will cause the last source
> node be executed even core id is not correct. This patch changes head check to
> less than 1 instead of 0 to fix this issue.
>
> Fixes: 35dfd9b9fd85 ("graph: introduce graph walk by cross-core dispatch")
>
> Signed-off-by: Jingjing Wu
> ---
> lib/graph/rte_graph_model_mcore_dispatch.h | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/lib/graph/rte_graph_model_mcore_dispatch.h
> b/lib/graph/rte_graph_model_mcore_dispatch.h
> index 75ec388cad..1cc75b7ac4 100644
> --- a/lib/graph/rte_graph_model_mcore_dispatch.h
> +++ b/lib/graph/rte_graph_model_mcore_dispatch.h
> @@ -100,9 +100,8 @@ rte_graph_walk_mcore_dispatch(struct rte_graph
> *graph)
> node = (struct rte_node *)RTE_PTR_ADD(graph,
> cir_start[(int32_t)head++]);
>
> /* skip the src nodes which not bind with current worker */
> - if ((int32_t)head < 0 && node->dispatch.lcore_id != graph-
> >dispatch.lcore_id)
> + if ((int32_t)head < 1 && node->dispatch.lcore_id !=
> +graph->dispatch.lcore_id)
> continue;
> -
No need for this line.
> /* Schedule the node until all task/objs are done */
> if (node->dispatch.lcore_id != RTE_MAX_LCORE &&
> graph->dispatch.lcore_id != node->dispatch.lcore_id &&
> --
> 2.34.1
With small change,
Acked-by: Zhirun Yan
RE: release candidate 24.03-rc4
> -Original Message- > From: Thomas Monjalon > Sent: Monday, March 25, 2024 11:01 AM > To: annou...@dpdk.org > Subject: release candidate 24.03-rc4 > > A new DPDK release candidate is ready for testing: > https://git.dpdk.org/dpdk/tag/?id=v24.03-rc4 > > There are 31 new patches in this snapshot. > > Release notes: > https://doc.dpdk.org/guides/rel_notes/release_24_03.html > > As usual, you can report any issue on https://bugs.dpdk.org You may share > some release validation results by replying to this message at dev@dpdk.org > and by adding tested hardware in the release notes. > > The final release should happen in a couple of days. > > Please think about sharing your roadmap now for DPDK 24.07. > > Thank you everyone > Update the test status for Intel part. dpdk24.03-rc4 all test is done. not found new issue. # Basic Intel(R) NIC testing * Build or compile: *Build: cover the build test combination with latest GCC/Clang version and the popular OS revision such as Ubuntu23.10, Ubuntu22.04.3, Fedora39, RHEL8.9/9.2, Centos7.9, FreeBSD14.0, SUSE15, OpenAnolis8.8, CBL-Mariner2.0 etc. - All test passed. *Compile: cover the CFLAGES(O0/O1/O2/O3) with popular OS such as Ubuntu22.04.3 and RHEL9.2. - All test passed with latest dpdk. * PF/VF(i40e, ixgbe): test scenarios including PF/VF-RTE_FLOW/TSO/Jumboframe/checksum offload/VLAN/VXLAN, etc. - All test case is done. No new issue is found. * PF/VF(igc): test scenarios including PF/VF-RTE_FLOW/TSO/Jumboframe/checksum offload/VLAN/VXLAN, etc. - All test case is done. No new issue is found. * PF/VF(ice): test scenarios including Switch features/Package Management/Flow Director/Advanced Tx/Advanced RSS/ACL/DCF/Flexible Descriptor, etc. - Execution rate is done. No new issue is found. * Intel NIC single core/NIC performance: test scenarios including PF/VF single core performance test, RFC2544 Zero packet loss performance test, etc. - Execution rate is done. No new issue is found. * Power; IPsec; DLB; DSA dmadev: * Power: test scenarios including bi-direction/Telemetry/Empty Poll Lib/Priority Base Frequency, etc. - Execution rate is done. No new issue is found. * IPsec: test scenarios including ipsec/ipsec-gw/ipsec library basic test - QAT&SW/FIB library, etc. - Execution rate is done. No new issue is found. * DLB: test scenarios including DLB2/DLB2.5 - Execution rate is done. No new issue is found. * DSA dmadev: - Execution rate is done. found the second issue. # Basic cryptodev and virtio testing * Virtio: both function and performance test are covered. Such as PVP/Virtio_loopback/virtio-user loopback/virtio-net VM2VM perf testing/VMAWARE ESXI 8.0U1, etc. - Execution rate is done. No new issue is found. * Cryptodev: *Function test: test scenarios including Cryptodev API testing/CompressDev ISA-L/QAT/ZLIB PMD Testing/FIPS, etc. - Execution rate is done. No new issue is found. *Performance test: test scenarios including Throughput Performance /Cryptodev Latency, etc. - Execution rate is done. No performance drop. Regards, Xu, Hailin
Re: [PATCH v2] doc: update LTS maintenance to 3 years
On Mon, 25 Mar 2024 at 10:02, Morten Brørup wrote: > > > From: Thomas Monjalon [mailto:tho...@monjalon.net] > > Sent: Monday, 25 March 2024 00.14 > > > > 17/01/2024 17:24, Kevin Traynor: > > > The existing official policy was to maintain LTS releases for 2 years. > > > > > > 19.11 and 20.11 LTS releases were maintained for 3 years and there was > > > not significant issues caused by code divergence from main etc. > > > > > > Update the policy to indicate 3 years maintenance for LTS releases, but > > > note that it depends on community support. > > > > > > Signed-off-by: Kevin Traynor > > > > More opinions, comments or acks? > > It is an improvement. > > Acked-by: Morten Brørup Acked-by: Luca Boccassi
Re: [PATCH v2] doc: update LTS maintenance to 3 years
On 28/03/2024 10:01, Luca Boccassi wrote: > On Mon, 25 Mar 2024 at 10:02, Morten Brørup > wrote: >> >>> From: Thomas Monjalon [mailto:tho...@monjalon.net] >>> Sent: Monday, 25 March 2024 00.14 >>> >>> 17/01/2024 17:24, Kevin Traynor: The existing official policy was to maintain LTS releases for 2 years. 19.11 and 20.11 LTS releases were maintained for 3 years and there was not significant issues caused by code divergence from main etc. Update the policy to indicate 3 years maintenance for LTS releases, but note that it depends on community support. Signed-off-by: Kevin Traynor >>> >>> More opinions, comments or acks? >> >> It is an improvement. >> >> Acked-by: Morten Brørup > > Acked-by: Luca Boccassi > Xueming/Christian - any comments on this ? If it sounds ok, can you Ack ? thanks, Kevin.
Re: [PATCH] net/ice: fix vlan stripping in double VLAN mode
On Wed, Mar 27, 2024 at 7:44 PM Vladimir Medvedkin
wrote:
>
> The ICE hardware can operate in two modes - single vlan mode
> or double vlan mode. Depending on the operating mode the hardware
> handles vlan header with single vlan tag differently.
> When double vlan enabled, a packet with a single VLAN is treated
> as a packet with outer VLAN only. Otherwise, a single VLAN in a
> packet is treated as inner VLAN.
>
> This patch fixes the logic of how vlan stripping is programmed.
>
> Bugzilla ID: 1402
Nit: no need for an empty line here, the Bugzilla ID: tag goes with
the Fixes: and Cc: block.
>
> Fixes: de5da9d16430 ("net/ice: support double VLAN")
> Cc: mingjinx...@intel.com
> Cc: sta...@dpdk.org
>
> Signed-off-by: Vladimir Medvedkin
Thanks Vladimir.
It looks to fix the issue I observed.
I'll let Carlos confirm the fix is good for him too.
--
David Marchand
[DPDK/DTS Bug 1408] Check dependencies on remote target
https://bugs.dpdk.org/show_bug.cgi?id=1408 Bug ID: 1408 Summary: Check dependencies on remote target Product: DPDK Version: unspecified Hardware: All OS: All Status: UNCONFIRMED Severity: normal Priority: Normal Component: DTS Assignee: dev@dpdk.org Reporter: luca.vizza...@arm.com CC: juraj.lin...@pantheon.tech, pr...@iol.unh.edu Target Milestone: --- When running DTS it is assumed that all the required dependencies are already installed and DTS can fail without a very obvious reason. Checking DPDK and Scapy dependencies before setting up may help the tester. These checks could be done as part of the smoke tests, but that'd require to execute them before any setup begins. -- You are receiving this mail because: You are the assignee for the bug.
DPDK Release Status Meeting 2024-03-28
Release status meeting minutes 2024-03-28 = Agenda: * Release Dates * Subtrees * Roadmaps * LTS * Defects * Opens Participants: * AMD * ARM * Debian/Microsoft * Intel * Marvell * Nvidia * Red Hat Release Dates - The following are the current/updated working dates for 24.03: * V1: 29 December 2023 * RC1: 21 February 2024 * RC2: 8 March2024 * RC3: 15 March2024 * Release: 27 March2024 * 24.07 Proposed dates: - Proposal deadline (RFC/v1 patches): 26 April 2024 - API freeze (-rc1): 7 June 2024 - PMD features freeze (-rc2): 21 June 2024 - Builtin applications features freeze (-rc3): 28 June 2024 - Release: 10 July 2023 https://core.dpdk.org/roadmap/#dates Subtrees * next-net * Complete for RC4/Release. * next-net-intel * Complete for RC4/Release. * next-net-mlx * Complete for RC4/Release. * next-net-mvl * Complete for RC4/Release. * next-eventdev * Complete for RC4/Release. * next-baseband * Complete for RC4/Release. * next-virtio * Complete for RC4/Release. * next-crypto * Complete for RC4/Release. * main * Working on release notes and final documentation fixes. * Target release March 28th. * 24.07 Proposed dates: - Proposal deadline (RFC/v1 patches): 26 April 2024 - API freeze (-rc1): 7 June 2024 - PMD features freeze (-rc2): 21 June 2024 - Builtin applications features freeze (-rc3): 28 June 2024 - Release: 10 July 2023 LTS --- Please add acks to confirm validation support for a 3 year LTS window: http://inbox.dpdk.org/dev/20240117161804.223582-1-ktray...@redhat.com/ * 23.11.1 - In progress. * 22.11.5 - In progress. * 21.11.7 - In progress. * 20.11.10 - Will only be updated with CVE and critical fixes. * 19.11.15 - Will only be updated with CVE and critical fixes. * Distros * Debian 12 contains DPDK v22.11 * Ubuntu 24.04-LTS will contain DPDK v23.11 * Ubuntu 23.04 contains DPDK v22.11 Defects --- * Bugzilla links, 'Bugs', added for hosted projects * https://www.dpdk.org/hosted-projects/ DPDK Release Status Meetings The DPDK Release Status Meeting is intended for DPDK Committers to discuss the status of the master tree and sub-trees, and for project managers to track progress or milestone dates. The meeting occurs on every Thursday at 9:30 UTC over Jitsi on https://meet.jit.si/DPDK You don't need an invite to join the meeting but if you want a calendar reminder just send an email to "John McNamara john.mcnam...@intel.com" for the invite.
[PATCH v1] doc: update release notes for 24.03
Fix grammar, spelling and formatting of DPDK 24.03 release notes. Signed-off-by: John McNamara --- Note: template sections should be removed. doc/guides/rel_notes/release_24_03.rst | 27 ++ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/doc/guides/rel_notes/release_24_03.rst b/doc/guides/rel_notes/release_24_03.rst index 34d7badde9..8e7ad8f99f 100644 --- a/doc/guides/rel_notes/release_24_03.rst +++ b/doc/guides/rel_notes/release_24_03.rst @@ -57,13 +57,13 @@ New Features * **Added HiSilicon UACCE bus support.** - UACCE (Unified/User-space-access-intended Accelerator Framework) bus driver - has been added, so that the accelerator devices could be seen in DPDK and could - be further registered such as a compress, crypto, DMA and ethernet devices. + Added UACCE (Unified/User-space-access-intended Accelerator Framework) bus + driver so that the accelerator devices such as a compress, crypto, + DMA and ethernet devices could be seen and registered in DPDK. * **Introduced argument parsing library.** - The argparse library was added to ease writing user-friendly applications, + The argparse library was added to help writing user-friendly applications, replacing ``getopt()`` usage. * **Improved RSS hash algorithm support.** @@ -73,8 +73,8 @@ New Features * **Added query of used descriptors number in Tx queue.** - * Added a fath path function ``rte_eth_tx_queue_count`` -to get the number of used descriptors of a Tx queue. + * Added a fast path function ``rte_eth_tx_queue_count`` +to get the number of used descriptors for a Tx queue. * **Added hash calculation of an encapsulated packet as done by the HW.** @@ -111,7 +111,7 @@ New Features * Added ``normal_llq_hdr`` devarg that enforces normal LLQ header policy. * Added support for LLQ header size recommendation from the device. * Allowed large LLQ with 1024 entries when the device supports enlarged memory BAR. - * Added `control_poll_interval` devarg that configure control-path to work in poll-mode. + * Added `control_poll_interval` devarg that configures the control-path to work in poll-mode. * Added support for binding ports to `uio_pci_generic` kernel module. * **Updated Atomic Rules' Arkville driver.** @@ -184,7 +184,7 @@ New Features * **Added Marvell Nitrox compression driver.** Added a new compression driver for Marvell Nitrox devices to support - deflate compression and decompression algorithm. + the deflate compression and decompression algorithm. * **Updated Marvell cnxk eventdev driver.** @@ -240,7 +240,7 @@ API Changes * eal: Improved ``RTE_BUILD_BUG_ON`` by using C11 ``static_assert``. Non-constant expressions are now rejected instead of being silently ignored. -* gso: ``rte_gso_segment`` now returns -ENOTSUP for unknown protocols. +* gso: ``rte_gso_segment`` now returns ``-ENOTSUP`` for unknown protocols. * ethdev: Renamed structure ``rte_flow_action_modify_data`` to be ``rte_flow_field_data`` for more generic usage. @@ -352,7 +352,8 @@ Tested Platforms * Firmware version: 4.40 0x8001c982 1.3534.0 * Device id (pf/vf): 8086:1593 / 8086:1889 * Driver version(out-tree): 1.13.7 (ice) - * Driver version(in-tree): 5.15.0-82-generic (Ubuntu22.04.3)/ 5.14.0-284.11.1.rt14.296.el9_2.x86_64 (RHEL9.2) (ice) + * Driver version(in-tree): 5.15.0-82-generic (Ubuntu22.04.3) / +5.14.0-284.11.1.rt14.296.el9_2.x86_64 (RHEL9.2) (ice) * OS Default DDP: 1.3.35.0 * COMMS DDP: 1.3.45.0 * Wireless Edge DDP: 1.3.13.0 @@ -407,7 +408,8 @@ Tested Platforms * Firmware version: 0x000161bf * Device id (pf/vf): 8086:10fb / 8086:10ed * Driver version(out-tree): 5.19.9 (ixgbe) - * Driver version(in-tree): 5.15.0-82-generic (Ubuntu22.04.3)/ 5.14.0-284.11.1.el9_2.x86_64 (RHEL9.2)(ixgbe) + * Driver version(in-tree): 5.15.0-82-generic (Ubuntu22.04.3) / +5.14.0-284.11.1.el9_2.x86_64 (RHEL9.2)(ixgbe) * Intel\ |reg| Ethernet Converged Network Adapter X710-DA4 (4x10G) @@ -427,7 +429,8 @@ Tested Platforms * Firmware version: 9.40 0x8000ed12 1.3429.0 * Device id (pf/vf): 8086:158b / 8086:154c * Driver version(out-tree): 2.24.6 (i40e) - * Driver version(in-tree): 5.15.0-82-generic (Ubuntu22.04.3)/5.14.0-284.11.1.el9_2.x86_64 (RHEL9.2)(i40e) + * Driver version(in-tree): 5.15.0-82-generic (Ubuntu22.04.3) / +5.14.0-284.11.1.el9_2.x86_64 (RHEL9.2)(i40e) * Intel\ |reg| Ethernet Converged Network Adapter XL710-QDA2 (2X40G) -- 2.34.1
Re: [PATCH v1] doc: update release notes for 24.03
On 2024/3/28 20:05, John McNamara wrote: > Fix grammar, spelling and formatting of DPDK 24.03 release notes. > > Signed-off-by: John McNamara Acked-by: Chengwen Feng
[PATCH v2 0/4] Virtio-user queues setup fixes
This series aims at fixing several issues found in Virtio-user PMD related to queues setup and cleanup. It has been tested with Vhost-vDPA backend using Nvidia Cx6-Dx vDPA VF. First patch in the series renames the queues iterator helper, so it is not a fix. But I would suggest to have it backported to ease backporting of the fixes. Changes in v2: -- - Fix regression in patch 4 reported by CI - Prefix titles with net/virtio-user (David) - Reword patch 2 title (David) - Apply David acks on patches 1-3 Maxime Coquelin (4): net/virtio-user: rename queue iterator net/virtio-user: fix control queue destruction net/virtio-user: fix shadow control queue notification init net/virtio-user: fix control queue allocation .../net/virtio/virtio_user/virtio_user_dev.c | 133 +- 1 file changed, 68 insertions(+), 65 deletions(-) -- 2.44.0
[PATCH v2 2/4] net/virtio-user: fix control queue destruction
This patch uses the freshly renamed iterator to destroy
queues at stop time. Doing this, we fix the missing
control queue destruction.
Fixes: 90966e8e5b67 ("net/virtio-user: send shadow virtqueue info to the
backend")
Cc: sta...@dpdk.org
Acked-by: David Marchand
Signed-off-by: Maxime Coquelin
---
.../net/virtio/virtio_user/virtio_user_dev.c | 27 ---
1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index c3d44880f5..0776c54deb 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -33,6 +33,22 @@ const char * const virtio_user_backend_strings[] = {
[VIRTIO_USER_BACKEND_VHOST_VDPA] = "VHOST_VDPA",
};
+static int
+virtio_user_destroy_queue(struct virtio_user_dev *dev, uint32_t queue_sel)
+{
+ struct vhost_vring_state state;
+ int ret;
+
+ state.index = queue_sel;
+ ret = dev->ops->get_vring_base(dev, &state);
+ if (ret < 0) {
+ PMD_DRV_LOG(ERR, "(%s) Failed to destroy queue %u", dev->path,
queue_sel);
+ return -1;
+ }
+
+ return 0;
+}
+
static int
virtio_user_create_queue(struct virtio_user_dev *dev, uint32_t queue_sel)
{
@@ -237,7 +253,6 @@ virtio_user_start_device(struct virtio_user_dev *dev)
int virtio_user_stop_device(struct virtio_user_dev *dev)
{
- struct vhost_vring_state state;
uint32_t i;
int ret;
@@ -258,14 +273,8 @@ int virtio_user_stop_device(struct virtio_user_dev *dev)
}
/* Stop the backend. */
- for (i = 0; i < dev->max_queue_pairs * 2; ++i) {
- state.index = i;
- ret = dev->ops->get_vring_base(dev, &state);
- if (ret < 0) {
- PMD_DRV_LOG(ERR, "(%s) get_vring_base failed,
index=%u", dev->path, i);
- goto err;
- }
- }
+ if (virtio_user_foreach_queue(dev, virtio_user_destroy_queue) < 0)
+ goto err;
dev->started = false;
--
2.44.0
[PATCH v2 3/4] net/virtio-user: fix shadow control queue notification init
The Virtio-user control queue kick and call FDs were not
uninitialized at device stop time.
This patch fixes this using the queues iterator helper for
both initialization and uninitialization.
Fixes: 90966e8e5b67 ("net/virtio-user: send shadow virtqueue info to the
backend")
Cc: sta...@dpdk.org
Acked-by: David Marchand
Signed-off-by: Maxime Coquelin
---
.../net/virtio/virtio_user/virtio_user_dev.c | 90 +--
1 file changed, 43 insertions(+), 47 deletions(-)
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index 0776c54deb..912e87fecf 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -33,6 +33,45 @@ const char * const virtio_user_backend_strings[] = {
[VIRTIO_USER_BACKEND_VHOST_VDPA] = "VHOST_VDPA",
};
+static int
+virtio_user_uninit_notify_queue(struct virtio_user_dev *dev, uint32_t
queue_sel)
+{
+ if (dev->kickfds[queue_sel] >= 0) {
+ close(dev->kickfds[queue_sel]);
+ dev->kickfds[queue_sel] = -1;
+ }
+
+ if (dev->callfds[queue_sel] >= 0) {
+ close(dev->callfds[queue_sel]);
+ dev->callfds[queue_sel] = -1;
+ }
+
+ return 0;
+}
+
+static int
+virtio_user_init_notify_queue(struct virtio_user_dev *dev, uint32_t queue_sel)
+{
+ /* May use invalid flag, but some backend uses kickfd and
+* callfd as criteria to judge if dev is alive. so finally we
+* use real event_fd.
+*/
+ dev->callfds[queue_sel] = eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK);
+ if (dev->callfds[queue_sel] < 0) {
+ PMD_DRV_LOG(ERR, "(%s) Failed to setup callfd for queue %u: %s",
+ dev->path, queue_sel, strerror(errno));
+ return -1;
+ }
+ dev->kickfds[queue_sel] = eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK);
+ if (dev->kickfds[queue_sel] < 0) {
+ PMD_DRV_LOG(ERR, "(%s) Failed to setup kickfd for queue %u: %s",
+ dev->path, queue_sel, strerror(errno));
+ return -1;
+ }
+
+ return 0;
+}
+
static int
virtio_user_destroy_queue(struct virtio_user_dev *dev, uint32_t queue_sel)
{
@@ -423,33 +462,9 @@ virtio_user_dev_init_mac(struct virtio_user_dev *dev,
const char *mac)
static int
virtio_user_dev_init_notify(struct virtio_user_dev *dev)
{
- uint32_t i, j, nr_vq;
- int callfd;
- int kickfd;
-
- nr_vq = dev->max_queue_pairs * 2;
- if (dev->hw_cvq)
- nr_vq++;
- for (i = 0; i < nr_vq; i++) {
- /* May use invalid flag, but some backend uses kickfd and
-* callfd as criteria to judge if dev is alive. so finally we
-* use real event_fd.
-*/
- callfd = eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK);
- if (callfd < 0) {
- PMD_DRV_LOG(ERR, "(%s) callfd error, %s", dev->path,
strerror(errno));
- goto err;
- }
- kickfd = eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK);
- if (kickfd < 0) {
- close(callfd);
- PMD_DRV_LOG(ERR, "(%s) kickfd error, %s", dev->path,
strerror(errno));
- goto err;
- }
- dev->callfds[i] = callfd;
- dev->kickfds[i] = kickfd;
- }
+ if (virtio_user_foreach_queue(dev, virtio_user_init_notify_queue) < 0)
+ goto err;
if (dev->device_features & (1ULL << VIRTIO_F_NOTIFICATION_DATA))
if (dev->ops->map_notification_area &&
@@ -458,16 +473,7 @@ virtio_user_dev_init_notify(struct virtio_user_dev *dev)
return 0;
err:
- for (j = 0; j < i; j++) {
- if (dev->kickfds[j] >= 0) {
- close(dev->kickfds[j]);
- dev->kickfds[j] = -1;
- }
- if (dev->callfds[j] >= 0) {
- close(dev->callfds[j]);
- dev->callfds[j] = -1;
- }
- }
+ virtio_user_foreach_queue(dev, virtio_user_uninit_notify_queue);
return -1;
}
@@ -475,18 +481,8 @@ virtio_user_dev_init_notify(struct virtio_user_dev *dev)
static void
virtio_user_dev_uninit_notify(struct virtio_user_dev *dev)
{
- uint32_t i;
+ virtio_user_foreach_queue(dev, virtio_user_uninit_notify_queue);
- for (i = 0; i < dev->max_queue_pairs * 2; ++i) {
- if (dev->kickfds[i] >= 0) {
- close(dev->kickfds[i]);
- dev->kickfds[i] = -1;
- }
- if (dev->callfds[i] >= 0) {
- close(dev->callfds[i]);
- dev->callfds[i] = -1;
- }
- }
if (dev->ops->unmap_notification_area && dev->notify_area)
[PATCH v2 1/4] net/virtio-user: rename queue iterator
This is a preliminary rework to prepare for iterating
over queues for non-setup operations.
Also, remove the error log that does not provide much
information given the callbacks already provide one.
Acked-by: David Marchand
Signed-off-by: Maxime Coquelin
---
drivers/net/virtio/virtio_user/virtio_user_dev.c | 13 +
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index 4fdfe70e7c..c3d44880f5 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -129,7 +129,7 @@ virtio_user_kick_queue(struct virtio_user_dev *dev,
uint32_t queue_sel)
}
static int
-virtio_user_queue_setup(struct virtio_user_dev *dev,
+virtio_user_foreach_queue(struct virtio_user_dev *dev,
int (*fn)(struct virtio_user_dev *, uint32_t))
{
uint32_t i, nr_vq;
@@ -138,12 +138,9 @@ virtio_user_queue_setup(struct virtio_user_dev *dev,
if (dev->hw_cvq)
nr_vq++;
- for (i = 0; i < nr_vq; i++) {
- if (fn(dev, i) < 0) {
- PMD_DRV_LOG(ERR, "(%s) setup VQ %u failed", dev->path,
i);
+ for (i = 0; i < nr_vq; i++)
+ if (fn(dev, i) < 0)
return -1;
- }
- }
return 0;
}
@@ -157,7 +154,7 @@ virtio_user_dev_set_features(struct virtio_user_dev *dev)
pthread_mutex_lock(&dev->mutex);
/* Step 0: tell vhost to create queues */
- if (virtio_user_queue_setup(dev, virtio_user_create_queue) < 0)
+ if (virtio_user_foreach_queue(dev, virtio_user_create_queue) < 0)
goto error;
features = dev->features;
@@ -205,7 +202,7 @@ virtio_user_start_device(struct virtio_user_dev *dev)
goto error;
/* Step 3: kick queues */
- ret = virtio_user_queue_setup(dev, virtio_user_kick_queue);
+ ret = virtio_user_foreach_queue(dev, virtio_user_kick_queue);
if (ret < 0)
goto error;
--
2.44.0
[PATCH v2 4/4] net/virtio-user: fix control queue allocation
It is possible to have the control queue without the
device advertising VIRTIO_NET_F_MQ.
Rely on the VIRTIO_NET_F_CTRL_VQ feature being advertised
instead.
Fixes: 6fdf32d1e318 ("net/virtio-user: remove max queues limitation")
Cc: sta...@dpdk.org
Signed-off-by: Maxime Coquelin
---
drivers/net/virtio/virtio_user/virtio_user_dev.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index 912e87fecf..b3aed48452 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -21,6 +21,7 @@
#include
#include "vhost.h"
+#include "virtio.h"
#include "virtio_user_dev.h"
#include "../virtio_ethdev.h"
@@ -615,7 +616,7 @@ virtio_user_alloc_vrings(struct virtio_user_dev *dev)
bool packed_ring = !!(dev->device_features & (1ull <<
VIRTIO_F_RING_PACKED));
nr_vrings = dev->max_queue_pairs * 2;
- if (dev->device_features & (1ull << VIRTIO_NET_F_MQ))
+ if (dev->device_features & (1ull << VIRTIO_NET_F_CTRL_VQ))
nr_vrings++;
dev->callfds = rte_zmalloc("virtio_user_dev", nr_vrings *
sizeof(*dev->callfds), 0);
--
2.44.0
Re: [PATCH v1] doc: update release notes for 24.03
28/03/2024 13:24, fengchengwen: > On 2024/3/28 20:05, John McNamara wrote: > > Fix grammar, spelling and formatting of DPDK 24.03 release notes. > > > > Signed-off-by: John McNamara > > Acked-by: Chengwen Feng Applied, thanks.
[PATCH 0/1] net/ena/base bug fix for 23.11 stable only
From: Shai Brandes
Hi, the fix is for a bug that was introduced in 23.11.
The fix was already merged into 24.03 indirectly as part of c8a1898f82f8
("net/ena: improve style and readability")
and the entire function was later restructured in patch bcb1753156ac
("net/ena/base: modify customer metrics memory management")
Meaning, this issue is indirectly fixed going forward, but we need to introduce
a dedicated fix for 23.11 stable only.
I CC'ed also dev mailing list since this involves Bugzilla bug fix.
Sorry in advance in case this is not the correct procedure.
All the best,
Shai
Shai Brandes (1):
net/ena: fix metrics excessive memory consumption
drivers/net/ena/base/ena_com.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
--
2.17.1
[PATCH 1/1] net/ena: fix metrics excessive memory consumption
From: Shai Brandes
The driver accidentally allocates a huge memory
buffer for the customer metrics because it uses
an uninitialized variable for the buffer length.
This can lead to excessive memory footprint for
the driver which can even fail to initialize in
case of insufficient memory.
Signed-off-by: Shai Brandes
Reviewed-by: Amit Bernstein
Fixes: f73f53f7dc7a ("net/ena: upgrade HAL")
Bugzilla ID: 14001
---
drivers/net/ena/base/ena_com.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ena/base/ena_com.c b/drivers/net/ena/base/ena_com.c
index 6953a1fa33..8ae7dcf48e 100644
--- a/drivers/net/ena/base/ena_com.c
+++ b/drivers/net/ena/base/ena_com.c
@@ -3134,16 +3134,18 @@ int ena_com_allocate_debug_area(struct ena_com_dev
*ena_dev,
int ena_com_allocate_customer_metrics_buffer(struct ena_com_dev *ena_dev)
{
struct ena_customer_metrics *customer_metrics =
&ena_dev->customer_metrics;
+ customer_metrics->buffer_len = ENA_CUSTOMER_METRICS_BUFFER_SIZE;
+ customer_metrics->buffer_virt_addr = NULL;
ENA_MEM_ALLOC_COHERENT(ena_dev->dmadev,
customer_metrics->buffer_len,
customer_metrics->buffer_virt_addr,
customer_metrics->buffer_dma_addr,
customer_metrics->buffer_dma_handle);
- if (unlikely(customer_metrics->buffer_virt_addr == NULL))
+ if (unlikely(customer_metrics->buffer_virt_addr == NULL)) {
+ customer_metrics->buffer_len = 0;
return ENA_COM_NO_MEM;
-
- customer_metrics->buffer_len = ENA_CUSTOMER_METRICS_BUFFER_SIZE;
+ }
return 0;
}
--
2.17.1
[PATCH v2 0/1] net/ena/base: bug fix for 23.11 stable only
From: Shai Brandes
Hi, the fix is for a bug that was introduced in 23.11.
The fix was already merged into 24.03 indirectly as part of c8a1898f82f8
("net/ena: improve style and readability")
and the entire function was later restructured in patch bcb1753156ac
("net/ena/base: modify customer metrics memory management")
Meaning, this issue is indirectly fixed going forward, but we need to introduce
a dedicated fix for 23.11 stable only.
I CC'ed also dev mailing list since this involves Bugzilla bug fix.
Sorry in advance in case this is not the correct procedure.
All the best,
Shai
Shai Brandes (1):
net/ena/base: fix metrics excessive memory consumption
drivers/net/ena/base/ena_com.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
--
2.17.1
[PATCH v2 1/1] net/ena/base: fix metrics excessive memory consumption
From: Shai Brandes
The driver accidentally allocates a huge memory
buffer for the customer metrics because it uses
an uninitialized variable for the buffer length.
This can lead to excessive memory footprint for
the driver which can even fail to initialize in
case of insufficient memory.
Signed-off-by: Shai Brandes
Reviewed-by: Amit Bernstein
Fixes: f73f53f7dc7a ("net/ena: upgrade HAL")
Bugzilla ID: 1400
---
drivers/net/ena/base/ena_com.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ena/base/ena_com.c b/drivers/net/ena/base/ena_com.c
index 6953a1fa33..8ae7dcf48e 100644
--- a/drivers/net/ena/base/ena_com.c
+++ b/drivers/net/ena/base/ena_com.c
@@ -3134,16 +3134,18 @@ int ena_com_allocate_debug_area(struct ena_com_dev
*ena_dev,
int ena_com_allocate_customer_metrics_buffer(struct ena_com_dev *ena_dev)
{
struct ena_customer_metrics *customer_metrics =
&ena_dev->customer_metrics;
+ customer_metrics->buffer_len = ENA_CUSTOMER_METRICS_BUFFER_SIZE;
+ customer_metrics->buffer_virt_addr = NULL;
ENA_MEM_ALLOC_COHERENT(ena_dev->dmadev,
customer_metrics->buffer_len,
customer_metrics->buffer_virt_addr,
customer_metrics->buffer_dma_addr,
customer_metrics->buffer_dma_handle);
- if (unlikely(customer_metrics->buffer_virt_addr == NULL))
+ if (unlikely(customer_metrics->buffer_virt_addr == NULL)) {
+ customer_metrics->buffer_len = 0;
return ENA_COM_NO_MEM;
-
- customer_metrics->buffer_len = ENA_CUSTOMER_METRICS_BUFFER_SIZE;
+ }
return 0;
}
--
2.17.1
RE: [PATCH v2 0/1] net/ena/base: bug fix for 23.11 stable only
Sorry, for some reason this appears on the wrong branch
I will fix this and upload a new patch
> -Original Message-
> From: shaib...@amazon.com
> Sent: Thursday, March 28, 2024 4:03 PM
> To: ferruh.yi...@amd.com; bl...@debian.org;
> christian.ehrha...@canonical.com; xuemi...@nvidia.com;
> ktray...@redhat.com
> Cc: sta...@dpdk.org; dev@dpdk.org; Brandes, Shai
>
> Subject: [PATCH v2 0/1] net/ena/base: bug fix for 23.11 stable only
>
> From: Shai Brandes
>
> Hi, the fix is for a bug that was introduced in 23.11.
> The fix was already merged into 24.03 indirectly as part of c8a1898f82f8
> ("net/ena: improve style and readability") and the entire function was later
> restructured in patch bcb1753156ac ("net/ena/base: modify customer
> metrics memory management") Meaning, this issue is indirectly fixed going
> forward, but we need to introduce a dedicated fix for 23.11 stable only.
> I CC'ed also dev mailing list since this involves Bugzilla bug fix.
> Sorry in advance in case this is not the correct procedure.
>
> All the best,
> Shai
>
> Shai Brandes (1):
> net/ena/base: fix metrics excessive memory consumption
>
> drivers/net/ena/base/ena_com.c | 8 +---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> --
> 2.17.1
Re: [PATCH v2 0/1] net/ena/base: bug fix for 23.11 stable only
Please see https://core.dpdk.org/contribute/ for instructions on
sending patches for stable releases, otherwise they will be missed
On Thu, 28 Mar 2024 at 14:22, Brandes, Shai wrote:
>
> Sorry, for some reason this appears on the wrong branch
> I will fix this and upload a new patch
>
> > -Original Message-
> > From: shaib...@amazon.com
> > Sent: Thursday, March 28, 2024 4:03 PM
> > To: ferruh.yi...@amd.com; bl...@debian.org;
> > christian.ehrha...@canonical.com; xuemi...@nvidia.com;
> > ktray...@redhat.com
> > Cc: sta...@dpdk.org; dev@dpdk.org; Brandes, Shai
> >
> > Subject: [PATCH v2 0/1] net/ena/base: bug fix for 23.11 stable only
> >
> > From: Shai Brandes
> >
> > Hi, the fix is for a bug that was introduced in 23.11.
> > The fix was already merged into 24.03 indirectly as part of c8a1898f82f8
> > ("net/ena: improve style and readability") and the entire function was later
> > restructured in patch bcb1753156ac ("net/ena/base: modify customer
> > metrics memory management") Meaning, this issue is indirectly fixed going
> > forward, but we need to introduce a dedicated fix for 23.11 stable only.
> > I CC'ed also dev mailing list since this involves Bugzilla bug fix.
> > Sorry in advance in case this is not the correct procedure.
> >
> > All the best,
> > Shai
> >
> > Shai Brandes (1):
> > net/ena/base: fix metrics excessive memory consumption
> >
> > drivers/net/ena/base/ena_com.c | 8 +---
> > 1 file changed, 5 insertions(+), 3 deletions(-)
> >
> > --
> > 2.17.1
>
[DPDK] maintainers: update for cpfl driver
From: Yuying Zhang Delete self from cpfl driver. Signed-off-by: Yuying Zhang --- MAINTAINERS | 1 - 1 file changed, 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index 0d1c8126e3..6a7d0293c6 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -800,7 +800,6 @@ F: doc/guides/nics/idpf.rst F: doc/guides/nics/features/idpf.ini Intel cpfl - EXPERIMENTAL -M: Yuying Zhang M: Beilei Xing T: git://dpdk.org/next/dpdk-next-net-intel F: drivers/net/cpfl/ -- 2.34.1
[PATCH] maintainers: update for testpmd
From: Yuying Zhang Delete self from testpmd. Signed-off-by: Yuying Zhang --- MAINTAINERS | 1 - 1 file changed, 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index 6a7d0293c6..969b70eabd 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1781,7 +1781,6 @@ F: app/test/sample_packet_forward.h Networking drivers testing tool M: Aman Singh -M: Yuying Zhang T: git://dpdk.org/next/dpdk-next-net F: app/test-pmd/ F: doc/guides/testpmd_app_ug/ -- 2.34.1
[PATCH] maintainers: update for i40e
From: Yuying Zhang Delete self from i40e driver. Signed-off-by: Yuying Zhang --- MAINTAINERS | 1 - 1 file changed, 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index 969b70eabd..1338d3d70e 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -758,7 +758,6 @@ F: doc/guides/nics/intel_vf.rst F: doc/guides/nics/features/ixgbe*.ini Intel i40e -M: Yuying Zhang M: Beilei Xing T: git://dpdk.org/next/dpdk-next-net-intel F: drivers/net/i40e/ -- 2.34.1
[PATCH v2 0/2] stop using mmx intrinsics
MSVC does not support older MMX intrinsics use SSE/AVX instead. v2: * move conditional #include into rte_vect.h and include rte_vect.h into net_crc_avx512.c net_crc_sse.c instead of duplicating conditional compile of include in each file. Tyler Retzlaff (2): eal: include header for MSVC SIMD intrinsics net: stop using mmx intrinsics lib/eal/include/generic/rte_vect.h | 6 +- lib/net/net_crc_avx512.c | 27 +++ lib/net/net_crc_sse.c | 27 +++ 3 files changed, 19 insertions(+), 41 deletions(-) -- 1.8.3.1
[PATCH v2 1/2] eal: include header for MSVC SIMD intrinsics
MSVC documents that you use the monolithic intrin.h for all intrinsics (including SIMD intrinsics) include intrin.h into rte_vec.h when building with MSVC so we don't have to duplicate conditionally compile include it across the DPDK source. Signed-off-by: Tyler Retzlaff --- lib/eal/include/generic/rte_vect.h | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/eal/include/generic/rte_vect.h b/lib/eal/include/generic/rte_vect.h index 6540419..1f84292 100644 --- a/lib/eal/include/generic/rte_vect.h +++ b/lib/eal/include/generic/rte_vect.h @@ -15,7 +15,11 @@ #include -#ifndef RTE_TOOLCHAIN_MSVC +#ifdef RTE_TOOLCHAIN_MSVC + +#include + +#else /* Unsigned vector types */ -- 1.8.3.1
[PATCH v2 2/2] net: stop using mmx intrinsics
Update code to use only avx/sse intrinsics as mmx is not supported on
MSVC.
Signed-off-by: Tyler Retzlaff
---
lib/net/net_crc_avx512.c | 27 +++
lib/net/net_crc_sse.c| 27 +++
2 files changed, 14 insertions(+), 40 deletions(-)
diff --git a/lib/net/net_crc_avx512.c b/lib/net/net_crc_avx512.c
index 0f0dee4..d18eb96 100644
--- a/lib/net/net_crc_avx512.c
+++ b/lib/net/net_crc_avx512.c
@@ -5,11 +5,10 @@
#include
#include
+#include
#include "net_crc.h"
-#include
-
/* VPCLMULQDQ CRC computation context structure */
struct crc_vpclmulqdq_ctx {
__m512i rk1_rk2;
@@ -331,13 +330,10 @@ static const alignas(16) uint32_t mask2[4] = {
c9, c10, c11);
crc32_eth.fold_3x128b = _mm512_setr_epi64(c12, c13, c14, c15,
c16, c17, 0, 0);
- crc32_eth.fold_1x128b = _mm_setr_epi64(_mm_cvtsi64_m64(c16),
- _mm_cvtsi64_m64(c17));
+ crc32_eth.fold_1x128b = _mm_set_epi64x(c17, c16);
- crc32_eth.rk5_rk6 = _mm_setr_epi64(_mm_cvtsi64_m64(c18),
- _mm_cvtsi64_m64(c19));
- crc32_eth.rk7_rk8 = _mm_setr_epi64(_mm_cvtsi64_m64(c20),
- _mm_cvtsi64_m64(c21));
+ crc32_eth.rk5_rk6 = _mm_set_epi64x(c19, c18);
+ crc32_eth.rk7_rk8 = _mm_set_epi64x(c21, c20);
}
static void
@@ -378,13 +374,10 @@ static const alignas(16) uint32_t mask2[4] = {
c9, c10, c11);
crc16_ccitt.fold_3x128b = _mm512_setr_epi64(c12, c13, c14, c15,
c16, c17, 0, 0);
- crc16_ccitt.fold_1x128b = _mm_setr_epi64(_mm_cvtsi64_m64(c16),
- _mm_cvtsi64_m64(c17));
+ crc16_ccitt.fold_1x128b = _mm_set_epi64x(c17, c16);
- crc16_ccitt.rk5_rk6 = _mm_setr_epi64(_mm_cvtsi64_m64(c18),
- _mm_cvtsi64_m64(c19));
- crc16_ccitt.rk7_rk8 = _mm_setr_epi64(_mm_cvtsi64_m64(c20),
- _mm_cvtsi64_m64(c21));
+ crc16_ccitt.rk5_rk6 = _mm_set_epi64x(c19, c18);
+ crc16_ccitt.rk7_rk8 = _mm_set_epi64x(c21, c20);
}
void
@@ -392,12 +385,6 @@ static const alignas(16) uint32_t mask2[4] = {
{
crc32_load_init_constants();
crc16_load_init_constants();
-
- /*
-* Reset the register as following calculation may
-* use other data types such as float, double, etc.
-*/
- _mm_empty();
}
uint32_t
diff --git a/lib/net/net_crc_sse.c b/lib/net/net_crc_sse.c
index d673ae3..112dc94 100644
--- a/lib/net/net_crc_sse.c
+++ b/lib/net/net_crc_sse.c
@@ -6,12 +6,11 @@
#include
#include
+#include
#include
#include "net_crc.h"
-#include
-
/** PCLMULQDQ CRC computation context structure */
struct crc_pclmulqdq_ctx {
__m128i rk1_rk2;
@@ -272,12 +271,9 @@ static const alignas(16) uint8_t crc_xmm_shift_tab[48] = {
p = 0x10811LLU;
/** Save the params in context structure */
- crc16_ccitt_pclmulqdq.rk1_rk2 =
- _mm_setr_epi64(_mm_cvtsi64_m64(k1), _mm_cvtsi64_m64(k2));
- crc16_ccitt_pclmulqdq.rk5_rk6 =
- _mm_setr_epi64(_mm_cvtsi64_m64(k5), _mm_cvtsi64_m64(k6));
- crc16_ccitt_pclmulqdq.rk7_rk8 =
- _mm_setr_epi64(_mm_cvtsi64_m64(q), _mm_cvtsi64_m64(p));
+ crc16_ccitt_pclmulqdq.rk1_rk2 = _mm_set_epi64x(k2, k1);
+ crc16_ccitt_pclmulqdq.rk5_rk6 = _mm_set_epi64x(k6, k5);
+ crc16_ccitt_pclmulqdq.rk7_rk8 = _mm_set_epi64x(p, q);
/** Initialize CRC32 data */
k1 = 0xccaa009eLLU;
@@ -288,18 +284,9 @@ static const alignas(16) uint8_t crc_xmm_shift_tab[48] = {
p = 0x1db710641LLU;
/** Save the params in context structure */
- crc32_eth_pclmulqdq.rk1_rk2 =
- _mm_setr_epi64(_mm_cvtsi64_m64(k1), _mm_cvtsi64_m64(k2));
- crc32_eth_pclmulqdq.rk5_rk6 =
- _mm_setr_epi64(_mm_cvtsi64_m64(k5), _mm_cvtsi64_m64(k6));
- crc32_eth_pclmulqdq.rk7_rk8 =
- _mm_setr_epi64(_mm_cvtsi64_m64(q), _mm_cvtsi64_m64(p));
-
- /**
-* Reset the register as following calculation may
-* use other data types such as float, double, etc.
-*/
- _mm_empty();
+ crc32_eth_pclmulqdq.rk1_rk2 = _mm_set_epi64x(k2, k1);
+ crc32_eth_pclmulqdq.rk5_rk6 = _mm_set_epi64x(k6, k5);
+ crc32_eth_pclmulqdq.rk7_rk8 = _mm_set_epi64x(p, q);
}
uint32_t
--
1.8.3.1
Re: [PATCH] net: stop using mmx intrinsics
On Thu, Mar 21, 2024 at 07:01:17PM +0100, Thomas Monjalon wrote: > 21/03/2024 18:27, Tyler Retzlaff: > > On Thu, Mar 21, 2024 at 06:09:01PM +0100, Thomas Monjalon wrote: > > > 20/03/2024 22:12, Tyler Retzlaff: > > > > +#ifdef RTE_TOOLCHAIN_MSVC > > > > +#include > > > > +#else > > > > #include > > > > +#endif > > > > > > It is not the same include in MSVC? > > > > unfortunately intrin.h is vestigial in the monolithic approach. to use > > any intrinsic you're supposed to include only the one and only true > > header instead of vendor/arch feature specific headers. > > > > > Is it something we want to wrap in a DPDK header file? > > > > do you mean create a monolithic rte_intrinsic.h header that is > > essentially > > > > #ifdef MSVC > > #include > > #else > > #include > > #include > > #include > > ... > > #endif > > > > i assumed that doing something like this might be unpopular due to the > > unnecessary namespace pollution. > > We already have such a file. > It is rte_vect.h. > I suppose we should just make sure it is included consistently > instead of x86intrin.h or immintrin.h > > This command will show where changes are required: > git grep intrin.h thanks! i saw none of the problems i had before so this worked great. there is only one other include of intrin.h in eal now and it is not for vector intrinsics so it should be cleaner to just include rte_vect.h whenever SIMD / vector intrinsics are required for windows and !windows. > >
Re: [PATCH 1/6] dts: add parameters data structure
Overall I like the idea of having a structured way of passing
command-line arguments to applications as strings and I think that
this is a well-abstracted approach. I also like that this approach
still supports the ability to pass strings "as-is" and use them as
parameters as well. That opens the door for potentially creating
dataclasses which only detail key-parameters that we assume you will
use, without blocking you from inputting whatever you want.
On Tue, Mar 26, 2024 at 3:04 PM Luca Vizzarro wrote:
> +META_VALUE_ONLY = "value_only"
> +META_OPTIONS_END = "options_end"
> +META_SHORT_NAME = "short_name"
> +META_LONG_NAME = "long_name"
> +META_MULTIPLE = "multiple"
> +META_MIXINS = "mixins"
> +
> +
I might add some kind of block comment here as a separator that
delimits that metadata modifiers start here. Something like what is
written in scapy.py which creates sections for XML-RPC method vs ones
that are run by the docker container. This isn't something strictly
necessary, but it might help break things up and add a little more
explanation.
> +def value_only(metadata: dict[str, Any] = {}) -> dict[str, Any]:
> +"""Injects the value of the attribute as-is without flag. Metadata
> modifier for :func:`dataclasses.field`."""
> +return {**metadata, META_VALUE_ONLY: True}
> +
> +
You could do the same thing here for mixins, but again, I'm not sure
it's really necessary.
> +def field_mixins(*mixins: Mixin, metadata: dict[str, Any] = {}) -> dict[str,
> Any]:
> +"""Takes in a variable number of mixins to manipulate the value's
> rendering. Metadata modifier for :func:`dataclasses.field`.
> +
> +The ``metadata`` keyword argument can be used to chain metadata
> modifiers together.
> +
> +Mixins can be chained together, executed from right to left in the
> arguments list order.
> +
> +Example:
> +
> +.. code:: python
> +
> +hex_bitmask: int | None = field(default=0b1101,
> metadata=field_mixins(hex, metadata=param_name("mask")))
> +
> +will render as ``--mask=0xd``. The :func:`hex` built-in can be used as a
> mixin turning a valid integer into a hexadecimal representation.
> +"""
> +return {**metadata, META_MIXINS: mixins}
> 2.34.1
>
Re: [PATCH 2/6] dts: use Params for interactive shells
On Tue, Mar 26, 2024 at 3:04 PM Luca Vizzarro wrote:
>
> Make it so that interactive shells accept an implementation of `Params`
> for app arguments. Convert EalParameters to use `Params` instead.
>
> String command line parameters can still be supplied by using the
> `StrParams` implementation.
>
> Signed-off-by: Luca Vizzarro
> Reviewed-by: Jack Bond-Preston
> Reviewed-by: Honnappa Nagarahalli
> ---
> @@ -40,7 +42,7 @@ class InteractiveShell(ABC):
> _ssh_channel: Channel
> _logger: DTSLogger
> _timeout: float
> -_app_args: str
> +_app_args: Params | None
>
I'm not sure if allowing None should be the solution for these shells
as opposed to just supplying an empty parameter object. Maybe
something that could be done is the factory method in sut_node allows
it to be None, but when it comes time to make the abstract shell it
creates an empty one if it doesn't exist, or the init method makes
this an optional parameter but creates it if it doesn't exist.
I suppose this logic would have to exist somewhere because the
parameters aren't always required, it's just a question of where we
should put it and if we should just assume that the interactive shell
class just knows how to accept some parameters and put them into the
shell. I would maybe leave this as something that cannot be None and
handle it outside of the shell, but I'm not sure it's something really
required and I don't have a super strong opinion on it.
> #: Prompt to expect at the end of output when sending a command.
> #: This is often overridden by subclasses.
> @@ -118,8 +119,15 @@ def _start_application(self, get_privileged_command:
> Callable[[str], str] | None
> Also find the number of pci addresses which were allowed on the
> command line when the app
> was started.
> """
> -self._app_args += " -i --mask-event intr_lsc"
> -self.number_of_ports = self._app_args.count("-a ")
> +from framework.testbed_model.sut_node import EalParameters
> +
> +assert isinstance(self._app_args, EalParameters)
> +
> +if isinstance(self._app_args.app_params, StrParams):
> +self._app_args.app_params.value += " -i --mask-event intr_lsc"
> +
> +self.number_of_ports = len(self._app_args.ports) if
> self._app_args.ports is not None else 0
I we should override the _app_args parameter in the testpmd shell to
always be EalParameters instead of doing this import and assertion.
It's a DPDK app, so we will always need EalParameters anyway, so we
might as well put that as our typehint to start off as what we expect
instead.
The checking of an instance of StrParams also feels a little strange
here, it might be more ideal if we could just add the parameters
without this check. Maybe something we can do, just because these
parameters are meant to be CLI commands anyway and will be rendered as
a string, is replace the StrParams object with a method on the base
Params dataclass that allows you to just add any string as a value
only field. Then, we don't have to bother validating anything about
the app parameters and we don't care what they are, we can just add a
string to them of new parameters.
I think this is something that likely also gets solved when you
replace this with testpmd parameters, but it might be a good way to
make the Params object more versatile in general so that people can
diverge and add their own flags to it if needed.
> +
> super()._start_application(get_privileged_command)
>
> def start(self, verify: bool = True) -> None:
> @@ -134,7 +136,7 @@ def create_interactive_shell(
> shell_cls: Type[InteractiveShellType],
> timeout: float,
> privileged: bool,
> -app_args: str,
> +app_args: Params | None,
This also falls in line with what I was saying before about where this
logic is handled. This was made to always be a string originally
because by this point it being optional was already handled by the
sut_node.create_interactive_shell() and we should have some kind of
value here (even if that value is an empty parameters dataclass) to
pass into the application. It sort of semantically doesn't really
change much, but at this point it might as well not be None, so we can
simplify this type.
> ) -> InteractiveShellType:
> """Factory for interactive session handlers.
>
> +@dataclass(kw_only=True)
> +class EalParameters(Params):
> """The environment abstraction layer parameters.
>
> The string representation can be created by converting the instance to a
> string.
> """
>
> -def __init__(
> -self,
> -lcore_list: LogicalCoreList,
> -memory_channels: int,
> -prefix: str,
> -no_pci: bool,
> -vdevs: list[VirtualDevice],
> -ports: list[Port],
> -other_eal_param: str,
> -):
> -"""Initialize the parameters according to inputs.
> -
> -Process the parameters into the form
Re: [PATCH 3/6] dts: add testpmd shell params
We talked about this in DTS meeting, looking at this some more, we already use default parameters for Eal and structure those, so we already have sort of tied ourselves into a situation of if those ever change (unlikely) we would need to change as well, so maybe this could be something we use, I'd like to hear more of peoples thoughts on this and what Juraj thinks when he is back. Just because this is fairly large and bloats the testpmd file a little bit, it might be more worth it to move this into a separate file and import it so this file doesn't get too large. Especially because this file will likely already grow quite a bit just from the amount of testpmd commands we are going to have to handle in the future. On Tue, Mar 26, 2024 at 3:04 PM Luca Vizzarro wrote: > > Implement all the testpmd shell parameters into a data structure. > > Signed-off-by: Luca Vizzarro > Reviewed-by: Jack Bond-Preston > Reviewed-by: Honnappa Nagarahalli > --- > 2.34.1 >
Re: [PATCH 5/6] dts: add statefulness to InteractiveShell
On Tue, Mar 26, 2024 at 3:04 PM Luca Vizzarro wrote: > diff --git a/dts/framework/remote_session/interactive_shell.py > b/dts/framework/remote_session/interactive_shell.py > index a2c7b30d9f..5d80061e8d 100644 > --- a/dts/framework/remote_session/interactive_shell.py > +++ b/dts/framework/remote_session/interactive_shell.py > @@ -41,8 +41,10 @@ class InteractiveShell(ABC): > _stdout: channel.ChannelFile > _ssh_channel: Channel > _logger: DTSLogger > +__default_timeout: float Only single underscores are used for other private variables, probably better to keep that consistent with this one. > _timeout: float > _app_args: Params | None > +_is_privileged: bool = False > 2.34.1 >
Re: [PATCH 6/6] dts: add statefulness to TestPmdShell
On Tue, Mar 26, 2024 at 3:04 PM Luca Vizzarro wrote: > > This commit provides a state container for TestPmdShell. It currently > only indicates whether the packet forwarding has started > or not, and the number of ports which were given to the shell. > > This also fixes the behaviour of `wait_link_status_up` to use the > command timeout as inherited from InteractiveShell. > > Signed-off-by: Luca Vizzarro > Reviewed-by: Jack Bond-Preston > Reviewed-by: Honnappa Nagarahalli > --- > @@ -723,7 +731,13 @@ def _start_application(self, get_privileged_command: > Callable[[str], str] | None > if self._app_args.app_params is None: > self._app_args.app_params = TestPmdParameters() > > -self.number_of_ports = len(self._app_args.ports) if > self._app_args.ports is not None else 0 > +assert isinstance(self._app_args.app_params, TestPmdParameters) > + This is tricky because ideally we wouldn't have the assertion here, but I understand why it is needed because Eal parameters have app args which can be any instance of params. I'm not sure of the best way to solve this, because making testpmd parameters extend from eal would break the general scheme that you have in place, and having an extension of EalParameters that enforces this app_args is TestPmdParameters would solve the issues, but might be a little clunky. Is there a way we can use a generic to get python to just understand that, in this case, this will always be TestPmdParameters? If not I might prefer making a private class where this is TestPmdParameters, just because there aren't really any other assertions that we use elsewhere and an unexpected exception from this (even though I don't think that can happen) could cause people some issues. It might be the case that an assertion is the easiest way to deal with it though, what do you think? > +if self._app_args.app_params.auto_start: > +self.state.packet_forwarding_started = True > + > +if self._app_args.ports is not None: > +self.state.number_of_ports = len(self._app_args.ports) > > super()._start_application(get_privileged_command) > > 2.34.1 >
Re: [PATCH v2 1/2] eal: include header for MSVC SIMD intrinsics
On Thu, Mar 28, 2024 at 09:14:05AM -0700, Tyler Retzlaff wrote: > MSVC documents that you use the monolithic intrin.h for all intrinsics > (including SIMD intrinsics) include intrin.h into rte_vec.h when > building with MSVC so we don't have to duplicate conditionally compile > include it across the DPDK source. > > Signed-off-by: Tyler Retzlaff > --- Acked-by: Bruce Richardson
Re: [PATCH v2 2/2] net: stop using mmx intrinsics
On Thu, Mar 28, 2024 at 09:14:06AM -0700, Tyler Retzlaff wrote:
> Update code to use only avx/sse intrinsics as mmx is not supported on
> MSVC.
>
> Signed-off-by: Tyler Retzlaff
> ---
One comment inline below. With or without that suggestion:
Acked-by: Bruce Richardson
> lib/net/net_crc_avx512.c | 27 +++
> lib/net/net_crc_sse.c| 27 +++
> 2 files changed, 14 insertions(+), 40 deletions(-)
>
> diff --git a/lib/net/net_crc_avx512.c b/lib/net/net_crc_avx512.c
> index 0f0dee4..d18eb96 100644
> --- a/lib/net/net_crc_avx512.c
> +++ b/lib/net/net_crc_avx512.c
> @@ -5,11 +5,10 @@
> #include
>
> #include
> +#include
>
> #include "net_crc.h"
>
> -#include
> -
> /* VPCLMULQDQ CRC computation context structure */
> struct crc_vpclmulqdq_ctx {
> __m512i rk1_rk2;
> @@ -331,13 +330,10 @@ static const alignas(16) uint32_t mask2[4] = {
> c9, c10, c11);
> crc32_eth.fold_3x128b = _mm512_setr_epi64(c12, c13, c14, c15,
> c16, c17, 0, 0);
Since the setr's below are being replaced, it would be nice to change these
ones above too. Long term I think it's going to be confusing having some
assignments set up as L->R, while others are R->L.
> - crc32_eth.fold_1x128b = _mm_setr_epi64(_mm_cvtsi64_m64(c16),
> - _mm_cvtsi64_m64(c17));
> + crc32_eth.fold_1x128b = _mm_set_epi64x(c17, c16);
>
> - crc32_eth.rk5_rk6 = _mm_setr_epi64(_mm_cvtsi64_m64(c18),
> - _mm_cvtsi64_m64(c19));
> - crc32_eth.rk7_rk8 = _mm_setr_epi64(_mm_cvtsi64_m64(c20),
> - _mm_cvtsi64_m64(c21));
> + crc32_eth.rk5_rk6 = _mm_set_epi64x(c19, c18);
> + crc32_eth.rk7_rk8 = _mm_set_epi64x(c21, c20);
> }
Re: [DPDK] maintainers: update for cpfl driver
On Thu, Mar 28, 2024 at 04:06:38PM +, yuying.zh...@intel.com wrote: > From: Yuying Zhang > > Delete self from cpfl driver. > > Signed-off-by: Yuying Zhang > --- Sorry to see you go, Acked-by: Bruce Richardson
Re: [PATCH] maintainers: update for testpmd
On Thu, Mar 28, 2024 at 04:10:21PM +, yuying.zh...@intel.com wrote: > From: Yuying Zhang > > Delete self from testpmd. > > Signed-off-by: Yuying Zhang > --- Acked-by: Bruce Richardson
Re: [PATCH] maintainers: update for i40e
On Thu, Mar 28, 2024 at 04:13:07PM +, yuying.zh...@intel.com wrote: > From: Yuying Zhang > > Delete self from i40e driver. > > Signed-off-by: Yuying Zhang > --- Acked-by: Bruce Richardson
Re: [PATCH v5 0/2] eal: initialize shared plugins on Windows
Recheck-request: github-robot
DPDK 24.03 released
A new major release is available: https://fast.dpdk.org/rel/dpdk-24.03.tar.xz This is the work we did during the last months: 987 commits from 154 authors 1334 files changed, 79260 insertions(+), 22824 deletions(-) It is not planned to start a maintenance branch for 24.03. This version is ABI-compatible with 23.11. Below are some new features: - argument parsing library - dynamic logging standardized - HiSilicon UACCE bus - Tx queue query - flow matching with random and field comparison - flow action NAT64 - flow template table resizing - more cleanups to prepare MSVC build - more DTS tests and cleanups More details in the release notes: https://doc.dpdk.org/guides/rel_notes/release_24_03.html There are 31 new contributors (including authors, reviewers and testers). Welcome to Akshay Dorwat, Alan Elder, Bhuvan Mital, Brad Larson, Christian Koue Muf, Chuanyu Xue, Emi Aoki, Fidel Castro, Flore Norceide, Gavin Li, Holly Nichols, Jack Bond-Preston, Lewis Donzis, Liangxing Wang, Luca Vizzarro, Masoumeh Farhadi Nia, Mykola Kostenok, Nicholas Pratte, Nishikant Nayak, Oleksandr Kolomeiets, Parthakumar Roy, Qian Hao, Shani Peretz, Shaowei Sun, Ting-Kai Ku, Tingting Liao, Tom Jones, Vamsi Krishna Atluri, Venkat Kumar Ande, Vinh Tran, and Wathsala Vithanage. Below is the number of commits per employer (with authors count): 202 Marvell (26) 166 NVIDIA (23) 125 Intel (31) 80 networkplumber.org (1) 77 Corigine (6) 64 Red Hat (5) 56 Huawei (7) 52 Broadcom (6) 33 AMD (9) 32 Amazon (1) 27 Microsoft (4) 14 PANTHEON.tech (1) 14 Arm (5) 7 Google (2) 6 UNH (1) ... A big thank to all courageous people who reviewed other's work. Based on Reviewed-by and Acked-by tags, the top non-PMD reviewers are: 50 Akhil Goyal 44 Ferruh Yigit 40 Chengwen Feng 36 Anoob Joseph 32 Morten Brørup 26 Tyler Retzlaff 21 Dariusz Sosnowski 18 Ori Kam 18 Bruce Richardson The next challenge is to reduce open bugs drastically. The next version will be 24.07 in July. The new features for 24.07 can be submitted during the next 4 weeks: http://core.dpdk.org/roadmap#dates Please share your roadmap. Don't forget to register for the webinar about DPDK in the cloud: https://zoom.us/webinar/register/WN_IG21wHwlTEGTv3sAXqcoFg Thanks everyone
[PATCH] vhost: optimize mbuf allocation in virtio Tx packed path
Currently virtio_dev_tx_packed() always allocates requested @count of
packets, no matter how many packets are really available on the virtio
Tx ring. Later it has to free all packets it didn't use and if, for
example, there were zero available packets on the ring, then all @count
mbufs would be allocated just to be freed afterwards.
This wastes CPU cycles since rte_pktmbuf_alloc_bulk() /
rte_pktmbuf_free_bulk() do quite a lot of work.
Optimize it by using the same idea as the virtio_dev_tx_split() uses on
the Tx split path: estimate the number of available entries on the ring
and allocate only that number of mbufs.
On the split path it's pretty easy to estimate.
On the packed path it's more work since it requires checking flags for
up to @count of descriptors. Still it's much less expensive than the
alloc/free pair.
The new get_nb_avail_entries_packed() function doesn't change how
virtio_dev_tx_packed() works with regard to memory barriers since the
barrier between checking flags and other descriptor fields is still in
place later in virtio_dev_tx_batch_packed() and
virtio_dev_tx_single_packed().
The difference for a guest transmitting ~17Gbps with MTU 1500 on a `perf
record` / `perf report` (on lower pps the savings will be bigger):
* Before the change:
Samples: 18K of event 'cycles:P', Event count (approx.): 19206831288
Children Self Pid:Command
- 100.00% 100.00% 798808:dpdk-worker1
<... skip ...>
- 99.09% pkt_burst_io_forward
- 90.26% common_fwd_stream_receive
- 90.04% rte_eth_rx_burst
- 75.53% eth_vhost_rx
- 74.29% rte_vhost_dequeue_burst
- 71.48% virtio_dev_tx_packed_compliant
+ 17.11% rte_pktmbuf_alloc_bulk
+ 11.80% rte_pktmbuf_free_bulk
+ 2.11% vhost_user_inject_irq
0.75% rte_pktmbuf_reset
0.53% __rte_pktmbuf_free_seg_via_array
0.88% vhost_queue_stats_update
+ 13.66% mlx5_rx_burst_vec
+ 8.69% common_fwd_stream_transmit
* After:
Samples: 18K of event 'cycles:P', Event count (approx.): 19225310840
Children Self Pid:Command
- 100.00% 100.00% 859754:dpdk-worker1
<... skip ...>
- 98.61% pkt_burst_io_forward
- 86.29% common_fwd_stream_receive
- 85.84% rte_eth_rx_burst
- 61.94% eth_vhost_rx
- 60.05% rte_vhost_dequeue_burst
- 55.98% virtio_dev_tx_packed_compliant
+ 3.43% rte_pktmbuf_alloc_bulk
+ 2.50% vhost_user_inject_irq
1.17% vhost_queue_stats_update
0.76% rte_rwlock_read_unlock
0.54% rte_rwlock_read_trylock
+ 22.21% mlx5_rx_burst_vec
+ 12.00% common_fwd_stream_transmit
It can be seen that virtio_dev_tx_packed_compliant() goes from 71.48% to
55.98% with rte_pktmbuf_alloc_bulk() going from 17.11% to 3.43% and
rte_pktmbuf_free_bulk() going away completely.
Signed-off-by: Andrey Ignatov
---
lib/vhost/virtio_net.c | 33 +
1 file changed, 33 insertions(+)
diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c
index 1359c5fb1f..b406b5d7d9 100644
--- a/lib/vhost/virtio_net.c
+++ b/lib/vhost/virtio_net.c
@@ -3484,6 +3484,35 @@ virtio_dev_tx_single_packed(struct virtio_net *dev,
return ret;
}
+static __rte_always_inline uint16_t
+get_nb_avail_entries_packed(const struct vhost_virtqueue *__rte_restrict vq,
+ uint16_t max_nb_avail_entries)
+{
+ const struct vring_packed_desc *descs = vq->desc_packed;
+ bool avail_wrap = vq->avail_wrap_counter;
+ uint16_t avail_idx = vq->last_avail_idx;
+ uint16_t nb_avail_entries = 0;
+ uint16_t flags;
+
+ while (nb_avail_entries < max_nb_avail_entries) {
+ flags = descs[avail_idx].flags;
+
+ if ((avail_wrap != !!(flags & VRING_DESC_F_AVAIL)) ||
+ (avail_wrap == !!(flags & VRING_DESC_F_USED)))
+ return nb_avail_entries;
+
+ if (!(flags & VRING_DESC_F_NEXT))
+ ++nb_avail_entries;
+
+ if (unlikely(++avail_idx >= vq->size)) {
+ avail_idx -= vq->size;
+ avail_wrap = !avail_wrap;
+ }
+ }
+
+ return nb_avail_entries;
+}
+
__rte_always_inline
static uint16_t
virtio_dev_tx_packed(struct virtio_net *dev,
@@ -3497,6 +3526,10 @@ virtio_dev_tx_packed(struct virtio_
[PATCH v18 00/15] Logging unification and improvements
Improvements and unification of logging library. This version works on all platforms: Linux, Windows and FreeBSD. This is update to rework patch set. It adds several new features to the console log output. * Putting a timestamp on console output which is useful for analyzing performance of startup codes. Timestamp is optional and must be enabled on command line. * Displaying console output with colors. It uses the standard conventions used by many other Linux commands for colorized display. The default is to enable color if the console output is going to a terminal. But it can be always on or disabled by command line flag. This default was chosen based on what dmesg(1) command does. I find color helpful because DPDK drivers and libraries print lots of not very useful messages. And having error messages highlighted in bold face helps. This might also get users to pay more attention to error messages. Many bug reports have earlier messages that are lost because there are so many info messages. * Add support for automatic detection of systemd journal protocol. If running as systemd service will get enhanced logging. * Use of syslog is optional and the meaning of the --syslog flag has changed. The default is *not* to use syslog. Add myself as maintainer for log because by now have added more than previous authors... v18 - handle more Windows MSVC incompatabilities. Stephen Hemminger (15): maintainers: add for log library windows: make getopt functions have const properties windows: add os shim for localtime_r windows: common wrapper for vasprintf and asprintf eal: make eal_log_level_parse common eal: do not duplicate rte_init_alert() messages eal: change rte_exit() output to match rte_log() log: move handling of syslog facility out of eal eal: initialize log before everything else log: drop syslog support, and make code common log: add hook for printing log messages log: add timestamp option log: add optional support of syslog log: add support for systemd journal log: colorize log output MAINTAINERS | 1 + app/test/test_eal_flags.c | 64 +- doc/guides/linux_gsg/linux_eal_parameters.rst | 27 - doc/guides/prog_guide/log_lib.rst | 57 ++ drivers/bus/pci/pci_common.c | 32 - lib/eal/common/eal_common_debug.c | 11 +- lib/eal/common/eal_common_options.c | 126 ++-- lib/eal/common/eal_options.h | 5 + lib/eal/common/eal_private.h | 10 - lib/eal/freebsd/eal.c | 64 +- lib/eal/linux/eal.c | 68 +- lib/eal/windows/eal.c | 77 +-- lib/eal/windows/getopt.c | 23 +- lib/eal/windows/include/getopt.h | 8 +- lib/eal/windows/include/rte_os_shim.h | 58 ++ lib/log/log.c | 652 +- lib/log/log_freebsd.c | 5 +- lib/log/log_internal.h| 25 +- lib/log/log_linux.c | 61 -- lib/log/log_windows.c | 18 - lib/log/meson.build | 5 +- lib/log/version.map | 4 +- 22 files changed, 973 insertions(+), 428 deletions(-) delete mode 100644 lib/log/log_linux.c delete mode 100644 lib/log/log_windows.c -- 2.43.0
[PATCH v18 01/15] maintainers: add for log library
"You touch it you own it" Add myself as maintainer for log library. Signed-off-by: Stephen Hemminger Acked-by: Tyler Retzlaff --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 7abb3aee49..54c28a601d 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -180,6 +180,7 @@ F: app/test/test_threads.c F: app/test/test_version.c Logging +M: Stephen Hemminger F: lib/log/ F: doc/guides/prog_guide/log_lib.rst F: app/test/test_logs.c -- 2.43.0
[PATCH v18 02/15] windows: make getopt functions have const properties
Having different prototypes on different platforms can lead
to lots of unnecessary workarounds. Looks like the version of
getopt used from windows was based on an older out of date
version from FreeBSD.
This patch changes getopt, getopt_long, etc to have the same const
attributes as Linux and FreeBSD. The changes are derived from
the current FreeBSD version of getopt_long.
Signed-off-by: Stephen Hemminger
Acked-by: Tyler Retzlaff
Acked-by: Dmitry Kozlyuk
---
lib/eal/windows/getopt.c | 23 ---
lib/eal/windows/include/getopt.h | 8
2 files changed, 16 insertions(+), 15 deletions(-)
diff --git a/lib/eal/windows/getopt.c b/lib/eal/windows/getopt.c
index a1f51c6c23..50ff71b930 100644
--- a/lib/eal/windows/getopt.c
+++ b/lib/eal/windows/getopt.c
@@ -20,7 +20,7 @@
#include
#include
-const char*optarg; /* argument associated with option */
+char*optarg; /* argument associated with option */
intopterr = 1; /* if error message should be printed */
intoptind = 1; /* index into parent argv vector */
intoptopt = '?'; /* character checked for validity */
@@ -39,9 +39,9 @@ static void pass(const char *a) {(void) a; }
#defineBADARG ((*options == ':') ? (int)':' : (int)'?')
#defineINORDER 1
-#defineEMSG""
+static char EMSG[] = "";
-static const char *place = EMSG; /* option letter processing */
+static char *place = EMSG; /* option letter processing */
/* XXX: set optreset to 1 rather than these two */
static int nonopt_start = -1; /* first non option argument (for permute) */
@@ -80,7 +80,7 @@ gcd(int a, int b)
*/
static void
permute_args(int panonopt_start, int panonopt_end, int opt_end,
- char **nargv)
+ char * const *nargv)
{
int cstart, cyclelen, i, j, ncycle, nnonopts, nopts, pos;
char *swap;
@@ -101,11 +101,12 @@ permute_args(int panonopt_start, int panonopt_end, int
opt_end,
pos -= nnonopts;
else
pos += nopts;
+
swap = nargv[pos];
/* LINTED const cast */
- ((char **) nargv)[pos] = nargv[cstart];
+ ((char **)(uintptr_t)nargv)[pos] = nargv[cstart];
/* LINTED const cast */
- ((char **)nargv)[cstart] = swap;
+ ((char **)(uintptr_t)nargv)[cstart] = swap;
}
}
}
@@ -116,7 +117,7 @@ permute_args(int panonopt_start, int panonopt_end, int
opt_end,
* Returns -1 if short_too is set and the option does not match long_options.
*/
static int
-parse_long_options(char **nargv, const char *options,
+parse_long_options(char * const *nargv, const char *options,
const struct option *long_options, int *idx, int short_too)
{
const char *current_argv;
@@ -236,7 +237,7 @@ parse_long_options(char **nargv, const char *options,
* Parse argc/argv argument vector. Called by user level routines.
*/
static int
-getopt_internal(int nargc, char **nargv, const char *options,
+getopt_internal(int nargc, char *const nargv[], const char *options,
const struct option *long_options, int *idx, int flags)
{
char *oli; /* option letter list index */
@@ -434,7 +435,7 @@ getopt_internal(int nargc, char **nargv, const char
*options,
* Parse argc/argv argument vector.
*/
int
-getopt(int nargc, char *nargv[], const char *options)
+getopt(int nargc, char *const nargv[], const char *options)
{
return getopt_internal(nargc, nargv, options, NULL, NULL,
FLAG_PERMUTE);
@@ -445,7 +446,7 @@ getopt(int nargc, char *nargv[], const char *options)
* Parse argc/argv argument vector.
*/
int
-getopt_long(int nargc, char *nargv[], const char *options,
+getopt_long(int nargc, char *const nargv[], const char *options,
const struct option *long_options, int *idx)
{
@@ -458,7 +459,7 @@ getopt_long(int nargc, char *nargv[], const char *options,
* Parse argc/argv argument vector.
*/
int
-getopt_long_only(int nargc, char *nargv[], const char *options,
+getopt_long_only(int nargc, char *const nargv[], const char *options,
const struct option *long_options, int *idx)
{
diff --git a/lib/eal/windows/include/getopt.h b/lib/eal/windows/include/getopt.h
index 6f57af454b..e4cf6873cb 100644
--- a/lib/eal/windows/include/getopt.h
+++ b/lib/eal/windows/include/getopt.h
@@ -44,7 +44,7 @@
/** argument to current option, or NULL if it has none */
-extern const char *optarg;
+extern char *optarg;
/** Current position in arg string. Starts from 1.
* Setting to 0 resets state.
*/
@@ -80,14 +80,14 @@ struct option {
};
/** Compat: getopt */
-int getopt(int argc, char *argv[], const char *options);
+int getopt(int argc, char *const
[PATCH v18 03/15] windows: add os shim for localtime_r
Windows does not have localtime_r but it does have a similar
function that can be used instead.
Signed-off-by: Stephen Hemminger
Acked-by: Tyler Retzlaff
---
lib/eal/windows/include/rte_os_shim.h | 10 ++
1 file changed, 10 insertions(+)
diff --git a/lib/eal/windows/include/rte_os_shim.h
b/lib/eal/windows/include/rte_os_shim.h
index eda8113662..e9741a9df2 100644
--- a/lib/eal/windows/include/rte_os_shim.h
+++ b/lib/eal/windows/include/rte_os_shim.h
@@ -110,4 +110,14 @@ rte_clock_gettime(clockid_t clock_id, struct timespec *tp)
}
#define clock_gettime(clock_id, tp) rte_clock_gettime(clock_id, tp)
+static inline struct tm *
+rte_localtime_r(const time_t *timer, struct tm *buf)
+{
+ if (localtime_s(buf, timer) == 0)
+ return buf;
+ else
+ return NULL;
+}
+#define localtime_r(timer, buf) rte_localtime_r(timer, buf)
+
#endif /* _RTE_OS_SHIM_ */
--
2.43.0
[PATCH v18 04/15] windows: common wrapper for vasprintf and asprintf
Replace the windows version of asprintf() that was only usable
in eal. With a more generic one that supports both vasprintf()
and asprintf(). This also eliminates duplicate code.
Fixes: 8f4de2dba9b9 ("bus/pci: fill bus specific information")
Fixes: 9ec521006db0 ("eal/windows: hide asprintf shim")
Signed-off-by: Stephen Hemminger
Acked-by: Tyler Retzlaff
---
drivers/bus/pci/pci_common.c | 32 --
lib/eal/common/eal_private.h | 10 --
lib/eal/windows/eal.c | 28
lib/eal/windows/include/rte_os_shim.h | 48 +++
4 files changed, 48 insertions(+), 70 deletions(-)
diff --git a/drivers/bus/pci/pci_common.c b/drivers/bus/pci/pci_common.c
index 889a48d2af..80691c75a3 100644
--- a/drivers/bus/pci/pci_common.c
+++ b/drivers/bus/pci/pci_common.c
@@ -45,38 +45,6 @@ const char *rte_pci_get_sysfs_path(void)
return path;
}
-#ifdef RTE_EXEC_ENV_WINDOWS
-#define asprintf pci_asprintf
-
-static int
-__rte_format_printf(2, 3)
-pci_asprintf(char **buffer, const char *format, ...)
-{
- int size, ret;
- va_list arg;
-
- va_start(arg, format);
- size = vsnprintf(NULL, 0, format, arg);
- va_end(arg);
- if (size < 0)
- return -1;
- size++;
-
- *buffer = malloc(size);
- if (*buffer == NULL)
- return -1;
-
- va_start(arg, format);
- ret = vsnprintf(*buffer, size, format, arg);
- va_end(arg);
- if (ret != size - 1) {
- free(*buffer);
- return -1;
- }
- return ret;
-}
-#endif /* RTE_EXEC_ENV_WINDOWS */
-
static struct rte_devargs *
pci_devargs_lookup(const struct rte_pci_addr *pci_addr)
{
diff --git a/lib/eal/common/eal_private.h b/lib/eal/common/eal_private.h
index 71523cfdb8..da8d77a134 100644
--- a/lib/eal/common/eal_private.h
+++ b/lib/eal/common/eal_private.h
@@ -737,16 +737,6 @@ void __rte_thread_init(unsigned int lcore_id, rte_cpuset_t
*cpuset);
*/
void __rte_thread_uninit(void);
-/**
- * asprintf(3) replacement for Windows.
- */
-#ifdef RTE_EXEC_ENV_WINDOWS
-__rte_format_printf(2, 3)
-int eal_asprintf(char **buffer, const char *format, ...);
-
-#define asprintf(buffer, format, ...) \
- eal_asprintf(buffer, format, ##__VA_ARGS__)
-#endif
#define EAL_LOG(level, ...) \
RTE_LOG_LINE(level, EAL, "" __VA_ARGS__)
diff --git a/lib/eal/windows/eal.c b/lib/eal/windows/eal.c
index 52f0e7462d..8ca00c0f95 100644
--- a/lib/eal/windows/eal.c
+++ b/lib/eal/windows/eal.c
@@ -503,34 +503,6 @@ rte_eal_init(int argc, char **argv)
return fctret;
}
-/* Don't use MinGW asprintf() to have identical code with all toolchains. */
-int
-eal_asprintf(char **buffer, const char *format, ...)
-{
- int size, ret;
- va_list arg;
-
- va_start(arg, format);
- size = vsnprintf(NULL, 0, format, arg);
- va_end(arg);
- if (size < 0)
- return -1;
- size++;
-
- *buffer = malloc(size);
- if (*buffer == NULL)
- return -1;
-
- va_start(arg, format);
- ret = vsnprintf(*buffer, size, format, arg);
- va_end(arg);
- if (ret != size - 1) {
- free(*buffer);
- return -1;
- }
- return ret;
-}
-
int
rte_vfio_container_dma_map(__rte_unused int container_fd,
__rte_unused uint64_t vaddr,
diff --git a/lib/eal/windows/include/rte_os_shim.h
b/lib/eal/windows/include/rte_os_shim.h
index e9741a9df2..65153fdb38 100644
--- a/lib/eal/windows/include/rte_os_shim.h
+++ b/lib/eal/windows/include/rte_os_shim.h
@@ -3,6 +3,7 @@
#ifndef _RTE_OS_SHIM_
#define _RTE_OS_SHIM_
+#include
#include
#include
@@ -120,4 +121,51 @@ rte_localtime_r(const time_t *timer, struct tm *buf)
}
#define localtime_r(timer, buf) rte_localtime_r(timer, buf)
+/* print to allocated string */
+__rte_format_printf(2, 0)
+static inline int
+rte_vasprintf(char **strp, const char *fmt, va_list ap)
+{
+ char *str;
+ int len, ret;
+
+ *strp = NULL;
+
+ /* determine size of buffer needed */
+ len = _vscprintf(fmt, ap);
+ if (len < 0)
+ return -1;
+
+ len += 1; /* for nul termination */
+ str = malloc(len);
+ if (str == NULL)
+ return -1;
+
+ ret = vsnprintf(str, len, fmt, ap);
+ if (ret < 0) {
+ free(str);
+ return -1;
+ } else {
+ *strp = str;
+ return ret;
+ }
+}
+#define vasprintf(strp, fmt, ap) rte_vasprintf(strp, fmt, ap)
+
+__rte_format_printf(2, 3)
+static inline int
+rte_asprintf(char **strp, const char *fmt, ...)
+{
+ int ret;
+
+ va_list ap;
+
+ va_start(ap, fmt);
+ ret = rte_vasprintf(strp, fmt, ap);
+ va_end(ap);
+
+ return ret;
+}
+
+#define asprintf(strp, fmt, ...) rte_asprintf(strp, fmt, __VA_ARGS__)
#endif /* _RTE_OS_SHIM_ */
--
2.43.
[PATCH v18 05/15] eal: make eal_log_level_parse common
The code to parse for log-level option should be same on
all OS variants.
Signed-off-by: Stephen Hemminger
Acked-by: Tyler Retzlaff
---
lib/eal/common/eal_common_options.c | 46 +
lib/eal/common/eal_options.h| 1 +
lib/eal/freebsd/eal.c | 42 --
lib/eal/linux/eal.c | 39
lib/eal/windows/eal.c | 35 --
5 files changed, 47 insertions(+), 116 deletions(-)
diff --git a/lib/eal/common/eal_common_options.c
b/lib/eal/common/eal_common_options.c
index e541f07939..5435399b85 100644
--- a/lib/eal/common/eal_common_options.c
+++ b/lib/eal/common/eal_common_options.c
@@ -1640,6 +1640,51 @@ eal_parse_huge_unlink(const char *arg, struct
hugepage_file_discipline *out)
return -1;
}
+/* Parse the all arguments looking for log related ones */
+int
+eal_log_level_parse(int argc, char * const argv[])
+{
+ struct internal_config *internal_conf =
eal_get_internal_configuration();
+ int option_index, opt;
+ const int old_optind = optind;
+ const int old_optopt = optopt;
+ const int old_opterr = opterr;
+ char *old_optarg = optarg;
+#ifdef RTE_EXEC_ENV_FREEBSD
+ const int old_optreset = optreset;
+ optreset = 1;
+#endif
+
+ optind = 1;
+ opterr = 0;
+
+ while ((opt = getopt_long(argc, argv, eal_short_options,
+ eal_long_options, &option_index)) != EOF) {
+
+ switch (opt) {
+ case OPT_LOG_LEVEL_NUM:
+ if (eal_parse_common_option(opt, optarg, internal_conf)
< 0)
+ return -1;
+ break;
+ case '?':
+ /* getopt is not happy, stop right now */
+ goto out;
+ default:
+ continue;
+ }
+ }
+out:
+ /* restore getopt lib */
+ optind = old_optind;
+ optopt = old_optopt;
+ optarg = old_optarg;
+ opterr = old_opterr;
+#ifdef RTE_EXEC_ENV_FREEBSD
+ optreset = old_optreset;
+#endif
+ return 0;
+}
+
int
eal_parse_common_option(int opt, const char *optarg,
struct internal_config *conf)
@@ -2173,6 +2218,7 @@ rte_vect_set_max_simd_bitwidth(uint16_t bitwidth)
return 0;
}
+
void
eal_common_usage(void)
{
diff --git a/lib/eal/common/eal_options.h b/lib/eal/common/eal_options.h
index 3cc9cb6412..f3f2e104f6 100644
--- a/lib/eal/common/eal_options.h
+++ b/lib/eal/common/eal_options.h
@@ -96,6 +96,7 @@ enum {
extern const char eal_short_options[];
extern const struct option eal_long_options[];
+int eal_log_level_parse(int argc, char * const argv[]);
int eal_parse_common_option(int opt, const char *argv,
struct internal_config *conf);
int eal_option_device_parse(void);
diff --git a/lib/eal/freebsd/eal.c b/lib/eal/freebsd/eal.c
index bab77118e9..9825bcea0b 100644
--- a/lib/eal/freebsd/eal.c
+++ b/lib/eal/freebsd/eal.c
@@ -363,48 +363,6 @@ eal_get_hugepage_mem_size(void)
return (size < SIZE_MAX) ? (size_t)(size) : SIZE_MAX;
}
-/* Parse the arguments for --log-level only */
-static void
-eal_log_level_parse(int argc, char **argv)
-{
- int opt;
- char **argvopt;
- int option_index;
- const int old_optind = optind;
- const int old_optopt = optopt;
- const int old_optreset = optreset;
- char * const old_optarg = optarg;
- struct internal_config *internal_conf =
- eal_get_internal_configuration();
-
- argvopt = argv;
- optind = 1;
- optreset = 1;
-
- while ((opt = getopt_long(argc, argvopt, eal_short_options,
- eal_long_options, &option_index)) != EOF) {
-
- int ret;
-
- /* getopt is not happy, stop right now */
- if (opt == '?')
- break;
-
- ret = (opt == OPT_LOG_LEVEL_NUM) ?
- eal_parse_common_option(opt, optarg, internal_conf) : 0;
-
- /* common parser is not happy */
- if (ret < 0)
- break;
- }
-
- /* restore getopt lib */
- optind = old_optind;
- optopt = old_optopt;
- optreset = old_optreset;
- optarg = old_optarg;
-}
-
/* Parse the argument given in the command line of the application */
static int
eal_parse_args(int argc, char **argv)
diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index fd422f1f62..bffeb1f34e 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -546,45 +546,6 @@ eal_parse_vfio_vf_token(const char *vf_token)
return -1;
}
-/* Parse the arguments for --log-level only */
-static void
-eal_log_level_parse(int argc, char **argv)
-{
- int opt;
- char **argvopt;
- int option_index;
- const int old_optin
[PATCH v18 06/15] eal: do not duplicate rte_init_alert() messages
The message already goes through logging, and does not need
to be printed on stderr. Message level should be ALERT
to match function name.
Signed-off-by: Stephen Hemminger
Acked-by: Tyler Retzlaff
---
lib/eal/freebsd/eal.c | 3 +--
lib/eal/linux/eal.c | 3 +--
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/lib/eal/freebsd/eal.c b/lib/eal/freebsd/eal.c
index 9825bcea0b..17b56f38aa 100644
--- a/lib/eal/freebsd/eal.c
+++ b/lib/eal/freebsd/eal.c
@@ -529,8 +529,7 @@ rte_eal_iopl_init(void)
static void rte_eal_init_alert(const char *msg)
{
- fprintf(stderr, "EAL: FATAL: %s\n", msg);
- EAL_LOG(ERR, "%s", msg);
+ EAL_LOG(ALERT, "%s", msg);
}
/* Launch threads, called at application init(). */
diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index bffeb1f34e..23dc26b124 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -840,8 +840,7 @@ static int rte_eal_vfio_setup(void)
static void rte_eal_init_alert(const char *msg)
{
- fprintf(stderr, "EAL: FATAL: %s\n", msg);
- EAL_LOG(ERR, "%s", msg);
+ EAL_LOG(ALERT, "%s", msg);
}
/*
--
2.43.0
[PATCH v18 07/15] eal: change rte_exit() output to match rte_log()
The rte_exit() output format confuses the timestamp and coloring
options. Change it to use be a single line with proper prefix.
Before:
[ 0.006481] EAL: Error - exiting with code: 1
Cause: [ 0.006489] Cannot init EAL: Permission denied
After:
[ 0.006238] EAL: Error - exiting with code: 1
[ 0.006250] EAL: Cause - Cannot init EAL: Permission denied
Signed-off-by: Stephen Hemminger
Acked-by: Tyler Retzlaff
---
lib/eal/common/eal_common_debug.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/lib/eal/common/eal_common_debug.c
b/lib/eal/common/eal_common_debug.c
index 3e77995896..ad2be63cbb 100644
--- a/lib/eal/common/eal_common_debug.c
+++ b/lib/eal/common/eal_common_debug.c
@@ -34,17 +34,18 @@ void
rte_exit(int exit_code, const char *format, ...)
{
va_list ap;
+ char msg[256];
if (exit_code != 0)
- RTE_LOG(CRIT, EAL, "Error - exiting with code: %d\n"
- " Cause: ", exit_code);
+ EAL_LOG(CRIT, "Error - exiting with code: %d", exit_code);
va_start(ap, format);
- rte_vlog(RTE_LOG_CRIT, RTE_LOGTYPE_EAL, format, ap);
+ vsnprintf(msg, sizeof(msg), format, ap);
va_end(ap);
+ rte_log(RTE_LOG_CRIT, RTE_LOGTYPE_EAL, "EAL: Cause - %s", msg);
+
if (rte_eal_cleanup() != 0 && rte_errno != EALREADY)
- EAL_LOG(CRIT,
- "EAL could not release all resources");
+ EAL_LOG(CRIT, "EAL could not release all resources");
exit(exit_code);
}
--
2.43.0
[PATCH v18 08/15] log: move handling of syslog facility out of eal
The syslog facility property is better handled in lib/log
rather than in eal. This also allows for changes to what
syslog flag means in later steps.
Signed-off-by: Stephen Hemminger
---
lib/eal/common/eal_common_options.c | 51 ++---
lib/eal/freebsd/eal.c | 5 ++-
lib/eal/linux/eal.c | 7 ++--
lib/eal/windows/eal.c | 6 ++--
lib/log/log_freebsd.c | 2 +-
lib/log/log_internal.h | 5 ++-
lib/log/log_linux.c | 47 --
lib/log/log_windows.c | 8 -
lib/log/version.map | 1 +
9 files changed, 68 insertions(+), 64 deletions(-)
diff --git a/lib/eal/common/eal_common_options.c
b/lib/eal/common/eal_common_options.c
index 5435399b85..661b2db211 100644
--- a/lib/eal/common/eal_common_options.c
+++ b/lib/eal/common/eal_common_options.c
@@ -6,9 +6,6 @@
#include
#include
#include
-#ifndef RTE_EXEC_ENV_WINDOWS
-#include
-#endif
#include
#include
#include
@@ -349,10 +346,6 @@ eal_reset_internal_config(struct internal_config
*internal_cfg)
}
internal_cfg->base_virtaddr = 0;
-#ifdef LOG_DAEMON
- internal_cfg->syslog_facility = LOG_DAEMON;
-#endif
-
/* if set to NONE, interrupt mode is determined automatically */
internal_cfg->vfio_intr_mode = RTE_INTR_MODE_NONE;
memset(internal_cfg->vfio_vf_token, 0,
@@ -1297,47 +1290,6 @@ eal_parse_lcores(const char *lcores)
return ret;
}
-#ifndef RTE_EXEC_ENV_WINDOWS
-static int
-eal_parse_syslog(const char *facility, struct internal_config *conf)
-{
- int i;
- static const struct {
- const char *name;
- int value;
- } map[] = {
- { "auth", LOG_AUTH },
- { "cron", LOG_CRON },
- { "daemon", LOG_DAEMON },
- { "ftp", LOG_FTP },
- { "kern", LOG_KERN },
- { "lpr", LOG_LPR },
- { "mail", LOG_MAIL },
- { "news", LOG_NEWS },
- { "syslog", LOG_SYSLOG },
- { "user", LOG_USER },
- { "uucp", LOG_UUCP },
- { "local0", LOG_LOCAL0 },
- { "local1", LOG_LOCAL1 },
- { "local2", LOG_LOCAL2 },
- { "local3", LOG_LOCAL3 },
- { "local4", LOG_LOCAL4 },
- { "local5", LOG_LOCAL5 },
- { "local6", LOG_LOCAL6 },
- { "local7", LOG_LOCAL7 },
- { NULL, 0 }
- };
-
- for (i = 0; map[i].name; i++) {
- if (!strcmp(facility, map[i].name)) {
- conf->syslog_facility = map[i].value;
- return 0;
- }
- }
- return -1;
-}
-#endif
-
static void
eal_log_usage(void)
{
@@ -1663,6 +1615,7 @@ eal_log_level_parse(int argc, char * const argv[])
switch (opt) {
case OPT_LOG_LEVEL_NUM:
+ case OPT_SYSLOG_NUM:
if (eal_parse_common_option(opt, optarg, internal_conf)
< 0)
return -1;
break;
@@ -1882,7 +1835,7 @@ eal_parse_common_option(int opt, const char *optarg,
#ifndef RTE_EXEC_ENV_WINDOWS
case OPT_SYSLOG_NUM:
- if (eal_parse_syslog(optarg, conf) < 0) {
+ if (eal_log_syslog(optarg) < 0) {
EAL_LOG(ERR, "invalid parameters for --"
OPT_SYSLOG);
return -1;
diff --git a/lib/eal/freebsd/eal.c b/lib/eal/freebsd/eal.c
index 17b56f38aa..6552f9c138 100644
--- a/lib/eal/freebsd/eal.c
+++ b/lib/eal/freebsd/eal.c
@@ -11,7 +11,6 @@
#include
#include
#include
-#include
#include
#include
#include
@@ -392,8 +391,8 @@ eal_parse_args(int argc, char **argv)
goto out;
}
- /* eal_log_level_parse() already handled this option */
- if (opt == OPT_LOG_LEVEL_NUM)
+ /* eal_log_level_parse() already handled these */
+ if (opt == OPT_LOG_LEVEL_NUM || opt == OPT_LOG_SYSLOG_NUM)
continue;
ret = eal_parse_common_option(opt, optarg, internal_conf);
diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index 23dc26b124..3d0c34063e 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -610,8 +610,8 @@ eal_parse_args(int argc, char **argv)
goto out;
}
- /* eal_log_level_parse() already handled this option */
- if (opt == OPT_LOG_LEVEL_NUM)
+ /* eal_log_level_parse() already handled these options */
+ if (opt == OPT_LOG_LEVEL_NUM || opt == OPT_SYSLOG_NUM)
continue;
ret = eal_parse_common_option(opt, optarg, internal_conf);
@@ -1106,8 +1106,7 @@ rte_eal_init(int argc, ch
[PATCH v18 09/15] eal: initialize log before everything else
In order for all log messages (including CPU mismatch) to
come out through the logging library, it must be initialized
as early in rte_eal_init() as possible on all platforms.
Where it was done before was likely historical based on
the support of non-OS isolated CPU's which required a shared
memory buffer; that support was dropped before DPDK was
publicly released.
Signed-off-by: Stephen Hemminger
Acked-by: Tyler Retzlaff
---
lib/eal/freebsd/eal.c | 12 +---
lib/eal/linux/eal.c| 19 +--
lib/eal/windows/eal.c | 8 ++--
lib/log/log_freebsd.c | 3 +--
lib/log/log_internal.h | 2 +-
lib/log/log_linux.c| 14 ++
lib/log/log_windows.c | 4 +---
7 files changed, 33 insertions(+), 29 deletions(-)
diff --git a/lib/eal/freebsd/eal.c b/lib/eal/freebsd/eal.c
index 6552f9c138..55ff27a4da 100644
--- a/lib/eal/freebsd/eal.c
+++ b/lib/eal/freebsd/eal.c
@@ -52,6 +52,7 @@
#include "eal_options.h"
#include "eal_memcfg.h"
#include "eal_trace.h"
+#include "log_internal.h"
#define MEMSIZE_IF_NO_HUGE_PAGE (64ULL * 1024ULL * 1024ULL)
@@ -546,6 +547,14 @@ rte_eal_init(int argc, char **argv)
bool has_phys_addr;
enum rte_iova_mode iova_mode;
+ /* setup log as early as possible */
+ if (eal_log_level_parse(argc, argv) < 0) {
+ rte_eal_init_alert("invalid log arguments.");
+ rte_errno = EINVAL;
+ return -1;
+ }
+ eal_log_init(getprogname());
+
/* checks if the machine is adequate */
if (!rte_cpu_is_supported()) {
rte_eal_init_alert("unsupported cpu type.");
@@ -565,9 +574,6 @@ rte_eal_init(int argc, char **argv)
/* clone argv to report out later in telemetry */
eal_save_args(argc, argv);
- /* set log level as early as possible */
- eal_log_level_parse(argc, argv);
-
if (rte_eal_cpu_init() < 0) {
rte_eal_init_alert("Cannot detect lcores.");
rte_errno = ENOTSUP;
diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index 3d0c34063e..b9a0fb1742 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -936,6 +936,15 @@ rte_eal_init(int argc, char **argv)
struct internal_config *internal_conf =
eal_get_internal_configuration();
+ /* setup log as early as possible */
+ if (eal_log_level_parse(argc, argv) < 0) {
+ rte_eal_init_alert("invalid log arguments.");
+ rte_errno = EINVAL;
+ return -1;
+ }
+
+ eal_log_init(program_invocation_short_name);
+
/* checks if the machine is adequate */
if (!rte_cpu_is_supported()) {
rte_eal_init_alert("unsupported cpu type.");
@@ -952,9 +961,6 @@ rte_eal_init(int argc, char **argv)
eal_reset_internal_config(internal_conf);
- /* set log level as early as possible */
- eal_log_level_parse(argc, argv);
-
/* clone argv to report out later in telemetry */
eal_save_args(argc, argv);
@@ -1106,13 +1112,6 @@ rte_eal_init(int argc, char **argv)
#endif
}
- if (eal_log_init(program_invocation_short_name) < 0) {
- rte_eal_init_alert("Cannot init logging.");
- rte_errno = ENOMEM;
- rte_atomic_store_explicit(&run_once, 0,
rte_memory_order_relaxed);
- return -1;
- }
-
#ifdef VFIO_PRESENT
if (rte_eal_vfio_setup() < 0) {
rte_eal_init_alert("Cannot init VFIO");
diff --git a/lib/eal/windows/eal.c b/lib/eal/windows/eal.c
index 14e498a643..e59aba954e 100644
--- a/lib/eal/windows/eal.c
+++ b/lib/eal/windows/eal.c
@@ -250,9 +250,13 @@ rte_eal_init(int argc, char **argv)
char cpuset[RTE_CPU_AFFINITY_STR_LEN];
char thread_name[RTE_THREAD_NAME_SIZE];
- eal_log_init(NULL);
+ if (eal_log_level_parse(argc, argv) < 0) {
+ rte_eal_init_alert("invalid log arguments.");
+ rte_errno = EINVAL;
+ return -1;
+ }
- eal_log_level_parse(argc, argv);
+ eal_log_init(NULL);
if (eal_create_cpu_map() < 0) {
rte_eal_init_alert("Cannot discover CPU and NUMA.");
diff --git a/lib/log/log_freebsd.c b/lib/log/log_freebsd.c
index 953e371bee..33a0925c43 100644
--- a/lib/log/log_freebsd.c
+++ b/lib/log/log_freebsd.c
@@ -5,8 +5,7 @@
#include
#include "log_internal.h"
-int
+void
eal_log_init(__rte_unused const char *id)
{
- return 0;
}
diff --git a/lib/log/log_internal.h b/lib/log/log_internal.h
index cb15cdff08..d5fabd7ef7 100644
--- a/lib/log/log_internal.h
+++ b/lib/log/log_internal.h
@@ -14,7 +14,7 @@
* Initialize the default log stream.
*/
__rte_internal
-int eal_log_init(const char *id);
+void eal_log_init(const char *id);
/*
* Determine where log data is written when no call to rte_openlog_stream.
diff --git a/lib/log/log_linux.c b/lib/log/log_linux.c
index 47aa074da2..6d7dc8f3ab 100644
--- a/li
[PATCH v18 10/15] log: drop syslog support, and make code common
This patch makes the log setup code common across all platforms.
Drops syslog support for now, will come back in later patch.
Signed-off-by: Stephen Hemminger
---
app/test/test_eal_flags.c | 11 ++-
lib/eal/common/eal_common_options.c | 3 -
lib/log/log.c | 29 +---
lib/log/log_internal.h | 6 --
lib/log/log_linux.c | 102
lib/log/log_windows.c | 22 --
lib/log/meson.build | 5 +-
lib/log/version.map | 1 -
8 files changed, 26 insertions(+), 153 deletions(-)
delete mode 100644 lib/log/log_linux.c
delete mode 100644 lib/log/log_windows.c
diff --git a/app/test/test_eal_flags.c b/app/test/test_eal_flags.c
index 6cb4b06757..36e3185a10 100644
--- a/app/test/test_eal_flags.c
+++ b/app/test/test_eal_flags.c
@@ -984,11 +984,10 @@ test_misc_flags(void)
const char *argv1[] = {prgname, prefix, mp_flag, "--no-pci"};
/* With -v */
const char *argv2[] = {prgname, prefix, mp_flag, "-v"};
+ /* With empty --syslog */
+ const char *argv3[] = {prgname, prefix, mp_flag, "--syslog"};
/* With valid --syslog */
- const char *argv3[] = {prgname, prefix, mp_flag,
- "--syslog", "syslog"};
- /* With empty --syslog (should fail) */
- const char *argv4[] = {prgname, prefix, mp_flag, "--syslog"};
+ const char *argv4[] = {prgname, prefix, mp_flag, "--syslog", "always"};
/* With invalid --syslog */
const char *argv5[] = {prgname, prefix, mp_flag, "--syslog", "error"};
/* With no-sh-conf, also use no-huge to ensure this test runs on BSD */
@@ -1083,8 +1082,8 @@ test_misc_flags(void)
printf("Error - process did not run ok with --syslog flag\n");
goto fail;
}
- if (launch_proc(argv4) == 0) {
- printf("Error - process run ok with empty --syslog flag\n");
+ if (launch_proc(argv4) != 0) {
+ printf("Error - process did not with --syslog always flag\n");
goto fail;
}
if (launch_proc(argv5) == 0) {
diff --git a/lib/eal/common/eal_common_options.c
b/lib/eal/common/eal_common_options.c
index 661b2db211..9ab512e8a1 100644
--- a/lib/eal/common/eal_common_options.c
+++ b/lib/eal/common/eal_common_options.c
@@ -2212,9 +2212,6 @@ eal_common_usage(void)
" (can be used multiple times)\n"
" --"OPT_VMWARE_TSC_MAP"Use VMware TSC map instead of
native RDTSC\n"
" --"OPT_PROC_TYPE" Type of this process
(primary|secondary|auto)\n"
-#ifndef RTE_EXEC_ENV_WINDOWS
- " --"OPT_SYSLOG"Set syslog facility\n"
-#endif
" --"OPT_LOG_LEVEL"= Set global log level\n"
" --"OPT_LOG_LEVEL"=:\n"
" Set specific log level\n"
diff --git a/lib/log/log.c b/lib/log/log.c
index 255f757d94..f597da2e39 100644
--- a/lib/log/log.c
+++ b/lib/log/log.c
@@ -70,12 +70,13 @@ struct log_cur_msg {
/* per core log */
static RTE_DEFINE_PER_LCORE(struct log_cur_msg, log_cur_msg);
-/* default logs */
-
/* Change the stream that will be used by logging system */
int
rte_openlog_stream(FILE *f)
{
+ if (rte_logs.file != NULL)
+ fclose(rte_logs.file);
+
rte_logs.file = f;
return 0;
}
@@ -505,13 +506,20 @@ rte_log(uint32_t level, uint32_t logtype, const char
*format, ...)
return ret;
}
+/* Placeholder */
+int
+eal_log_syslog(const char *mode __rte_unused)
+{
+ return -1;
+}
+
/*
- * Called by environment-specific initialization functions.
+ * Called by rte_eal_init
*/
void
-eal_log_set_default(FILE *default_log)
+eal_log_init(const char *id __rte_unused)
{
- default_log_stream = default_log;
+ default_log_stream = stderr;
#if RTE_LOG_DP_LEVEL >= RTE_LOG_DEBUG
RTE_LOG(NOTICE, EAL,
@@ -525,8 +533,11 @@ eal_log_set_default(FILE *default_log)
void
rte_eal_log_cleanup(void)
{
- if (default_log_stream) {
- fclose(default_log_stream);
- default_log_stream = NULL;
- }
+ FILE *log_stream = rte_log_get_stream();
+
+ /* don't close stderr on the application */
+ if (log_stream != stderr)
+ fclose(log_stream);
+
+ rte_logs.file = NULL;
}
diff --git a/lib/log/log_internal.h b/lib/log/log_internal.h
index d5fabd7ef7..3c46328e7b 100644
--- a/lib/log/log_internal.h
+++ b/lib/log/log_internal.h
@@ -16,12 +16,6 @@
__rte_internal
void eal_log_init(const char *id);
-/*
- * Determine where log data is written when no call to rte_openlog_stream.
- */
-__rte_internal
-void eal_log_set_default(FILE *default_log);
-
/*
* Save a log option for later.
*/
diff --git a/lib/log/log_linux.c b/lib/log/log_linux.c
deleted file mode 100644
index 6d7dc8f3ab..00
--- a/lib/log/log_linux.c
+++ /dev/null
[PATCH v18 11/15] log: add hook for printing log messages
This is useful for when decorating log output for console
or journal. Provide basic version in this patch.
Signed-off-by: Stephen Hemminger
---
lib/log/log.c | 17 -
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/lib/log/log.c b/lib/log/log.c
index f597da2e39..acd4c320b6 100644
--- a/lib/log/log.c
+++ b/lib/log/log.c
@@ -26,16 +26,21 @@ struct rte_log_dynamic_type {
uint32_t loglevel;
};
+typedef int (*log_print_t)(FILE *f, uint32_t level, const char *fmt, va_list
ap);
+static int log_print(FILE *f, uint32_t level, const char *format, va_list ap);
+
/** The rte_log structure. */
static struct rte_logs {
uint32_t type; /**< Bitfield with enabled logs. */
uint32_t level; /**< Log level. */
FILE *file; /**< Output file set by rte_openlog_stream, or NULL. */
+ log_print_t print_func;
size_t dynamic_types_len;
struct rte_log_dynamic_type *dynamic_types;
} rte_logs = {
.type = UINT32_MAX,
.level = RTE_LOG_DEBUG,
+ .print_func = log_print,
};
struct rte_eal_opt_loglevel {
@@ -78,6 +83,7 @@ rte_openlog_stream(FILE *f)
fclose(rte_logs.file);
rte_logs.file = f;
+ rte_logs.print_func = log_print;
return 0;
}
@@ -484,7 +490,7 @@ rte_vlog(uint32_t level, uint32_t logtype, const char
*format, va_list ap)
RTE_PER_LCORE(log_cur_msg).loglevel = level;
RTE_PER_LCORE(log_cur_msg).logtype = logtype;
- ret = vfprintf(f, format, ap);
+ ret = (*rte_logs.print_func)(f, level, format, ap);
fflush(f);
return ret;
}
@@ -513,6 +519,15 @@ eal_log_syslog(const char *mode __rte_unused)
return -1;
}
+/* default log print function */
+__rte_format_printf(3, 0)
+static int
+log_print(FILE *f, uint32_t level __rte_unused,
+ const char *format, va_list ap)
+{
+ return vfprintf(f, format, ap);
+}
+
/*
* Called by rte_eal_init
*/
--
2.43.0
[PATCH v18 12/15] log: add timestamp option
When debugging driver or startup issues, it is useful to have
a timestamp on each message printed. The messages in syslog
already have a timestamp, but often syslog is not available
during testing.
There are multiple timestamp formats similar to Linux dmesg.
The default is time relative since startup (when first
step of logging initialization is done by constructor).
Other alternative formats are delta, ctime, reltime and iso formats.
Example:
$ dpdk-testpmd --log-timestamp -- -i
[ 0.008610] EAL: Detected CPU lcores: 8
[ 0.008634] EAL: Detected NUMA nodes: 1
[ 0.008792] EAL: Detected static linkage of DPDK
[ 0.010620] EAL: Multi-process socket /var/run/dpdk/rte/mp_socket
[ 0.012618] EAL: Selected IOVA mode 'VA'
[ 0.016675] testpmd: No probed ethernet devices
Interactive-mode selected
Signed-off-by: Stephen Hemminger
---
app/test/test_eal_flags.c | 26
doc/guides/prog_guide/log_lib.rst | 26
lib/eal/common/eal_common_options.c | 14 ++-
lib/eal/common/eal_options.h| 2 +
lib/eal/freebsd/eal.c | 6 +-
lib/eal/linux/eal.c | 4 +-
lib/eal/windows/eal.c | 4 +-
lib/log/log.c | 183 +++-
lib/log/log_internal.h | 9 ++
lib/log/version.map | 1 +
10 files changed, 268 insertions(+), 7 deletions(-)
diff --git a/app/test/test_eal_flags.c b/app/test/test_eal_flags.c
index 36e3185a10..e54f6e8b7f 100644
--- a/app/test/test_eal_flags.c
+++ b/app/test/test_eal_flags.c
@@ -1054,6 +1054,19 @@ test_misc_flags(void)
const char * const argv22[] = {prgname, prefix, mp_flag,
"--huge-worker-stack=512"};
+ /* Try running with --log-timestamp */
+ const char * const argv23[] = {prgname, prefix, mp_flag,
+ "--log-timestamp" };
+
+ /* Try running with --log-timestamp=iso */
+ const char * const argv24[] = {prgname, prefix, mp_flag,
+ "--log-timestamp=iso" };
+
+ /* Try running with invalid timestamp */
+ const char * const argv25[] = {prgname, prefix, mp_flag,
+ "--log-timestamp=invalid" };
+
+
/* run all tests also applicable to FreeBSD first */
if (launch_proc(argv0) == 0) {
@@ -1161,6 +1174,19 @@ test_misc_flags(void)
printf("Error - process did not run ok with
--huge-worker-stack=size parameter\n");
goto fail;
}
+ if (launch_proc(argv23) != 0) {
+ printf("Error - process did not run ok with --log-timestamp
parameter\n");
+ goto fail;
+ }
+ if (launch_proc(argv24) != 0) {
+ printf("Error - process did not run ok with --log-timestamp=iso
parameter\n");
+ goto fail;
+ }
+ if (launch_proc(argv25) == 0) {
+ printf("Error - process did run ok with --log-timestamp=invalid
parameter\n");
+ goto fail;
+ }
+
rmdir(hugepath_dir3);
rmdir(hugepath_dir2);
diff --git a/doc/guides/prog_guide/log_lib.rst
b/doc/guides/prog_guide/log_lib.rst
index ff9d1b54a2..504eefe1d2 100644
--- a/doc/guides/prog_guide/log_lib.rst
+++ b/doc/guides/prog_guide/log_lib.rst
@@ -59,6 +59,32 @@ For example::
Within an application, the same result can be got using the
``rte_log_set_level_pattern()`` or ``rte_log_set_level_regex()`` APIs.
+Log timestamp
+~
+
+An optional timestamp can be added before each message
+by adding the ``--log-timestamp`` option.
+For example::
+
+ /path/to/app --log-level=lib.*:debug --log-timestamp
+
+Multiple timestamp alternative timestamp formats are available:
+
+.. csv-table:: Log time stamp format
+ :header: "Format", "Description", "Example"
+ :widths: 6, 30, 32
+
+ "ctime", "Unix ctime", "``[Wed Mar 20 07:26:12 2024]``"
+ "delta", "Offset since last", "``[<3.162373>]``"
+ "reltime", "Seconds since last or time if minute changed", "``[
+3.001791]`` or ``[Mar20 07:26:12]``"
+ "iso", "ISO-8601", "``[2024-03-20T07:26:12−07:00]``"
+
+To prefix all console messages with ISO format time the syntax is::
+
+ /path/to/app --log-timestamp=iso
+
+
+
Using Logging APIs to Generate Log Messages
---
diff --git a/lib/eal/common/eal_common_options.c
b/lib/eal/common/eal_common_options.c
index 9ab512e8a1..5173835c2c 100644
--- a/lib/eal/common/eal_common_options.c
+++ b/lib/eal/common/eal_common_options.c
@@ -74,6 +74,7 @@ eal_long_options[] = {
{OPT_IOVA_MODE, 1, NULL, OPT_IOVA_MODE_NUM},
{OPT_LCORES,1, NULL, OPT_LCORES_NUM },
{OPT_LOG_LEVEL, 1, NULL, OPT_LOG_LEVEL_NUM},
+ {OPT_LOG_TIMESTAMP, 2, NULL, OPT_LOG_TIMESTAMP_NUM},
{OPT_TRACE, 1, NULL, OPT_TRACE_NUM
[PATCH v18 14/15] log: add support for systemd journal
If DPDK application is being run as a systemd service, then
it can use the journal protocol which allows putting more information
in the log such as priority and other information.
The use of journal protocol is automatically detected and
handled. Rather than having a dependency on libsystemd,
just use the protocol directly as defined in:
https://systemd.io/JOURNAL_NATIVE_PROTOCOL/
Signed-off-by: Stephen Hemminger
---
lib/log/log.c | 154 +-
1 file changed, 152 insertions(+), 2 deletions(-)
diff --git a/lib/log/log.c b/lib/log/log.c
index ec0d55273e..650d294120 100644
--- a/lib/log/log.c
+++ b/lib/log/log.c
@@ -17,6 +17,10 @@
#include
#else
#include
+#include
+#include
+#include
+#include
#endif
#include
@@ -56,6 +60,7 @@ static struct rte_logs {
FILE *file; /**< Output file set by rte_openlog_stream, or NULL. */
#ifndef RTE_EXEC_ENV_WINDOWS
enum eal_log_syslog syslog_opt;
+ int journal_fd;
#endif
log_print_t print_func;
@@ -775,6 +780,138 @@ static cookie_io_functions_t syslog_log_func = {
.close = syslog_log_close,
};
+/*
+ * send message using journal protocol to journald
+ */
+static int
+journal_send(uint32_t level, const char *buf, size_t len)
+{
+ struct iovec iov[3];
+ char msg[] = "MESSAGE=";
+ char prio[32];
+ int ret;
+
+ iov[0].iov_base = msg;
+ iov[0].iov_len = strlen(msg);
+
+ iov[1].iov_base = (char *)(uintptr_t)buf;
+ iov[1].iov_len = len;
+
+ /* priority value between 0 ("emerg") and 7 ("debug") */
+ iov[2].iov_base = prio;
+ iov[2].iov_len = snprintf(prio, sizeof(prio),
+ "PRIORITY=%i\n", level - 1);
+
+ ret = writev(rte_logs.journal_fd, iov, 3);
+ return ret;
+}
+
+__rte_format_printf(3, 0)
+static int
+journal_print(FILE *f __rte_unused, uint32_t level, const char *format,
va_list ap)
+{
+ char buf[BUFSIZ];
+ size_t len;
+
+ len = vsnprintf(buf, sizeof(buf), format, ap);
+ if (len == 0)
+ return 0;
+
+ /* check for truncation */
+ if (len >= sizeof(buf) - 1)
+ len = sizeof(buf) - 1;
+
+ /* check that message ends with newline, if not add one */
+ if (buf[len - 1] != '\n')
+ buf[len++] = '\n';
+
+ return journal_send(level, buf, len);
+}
+
+/* wrapper for log stream to put messages into journal */
+static ssize_t
+journal_log_write(__rte_unused void *c, const char *buf, size_t size)
+{
+ return journal_send(rte_log_cur_msg_loglevel(), buf, size);
+}
+
+static cookie_io_functions_t journal_log_func = {
+ .write = journal_log_write,
+};
+
+/*
+ * Check if stderr is going to system journal.
+ * This is the documented way to handle systemd journal
+ *
+ * See: https://systemd.io/JOURNAL_NATIVE_PROTOCOL/
+ *
+ */
+static bool
+is_journal(int fd)
+{
+ char *jenv, *endp = NULL;
+ struct stat st;
+ unsigned long dev, ino;
+
+ jenv = getenv("JOURNAL_STREAM");
+ if (jenv == NULL)
+ return false;
+
+ if (fstat(fd, &st) < 0)
+ return false;
+
+ /* systemd sets colon-separated list of device and inode number */
+ dev = strtoul(jenv, &endp, 10);
+ if (endp == NULL || *endp != ':')
+ return false; /* missing colon */
+
+ ino = strtoul(endp + 1, NULL, 10);
+
+ return dev == st.st_dev && ino == st.st_ino;
+}
+
+/* Connect to systemd's journal service */
+static int
+open_journal(const char *id)
+{
+ char *syslog_id = NULL;
+ struct sockaddr_un sun = {
+ .sun_family = AF_UNIX,
+ .sun_path = "/run/systemd/journal/socket",
+ };
+ ssize_t len;
+ int s;
+
+ s = socket(AF_UNIX, SOCK_DGRAM, 0);
+ if (s < 0)
+ return -1;
+
+ if (connect(s, (struct sockaddr *)&sun, sizeof(sun)) < 0)
+ goto error;
+
+ /* Send syslog identifier as first message */
+ len = asprintf(&syslog_id, "SYSLOG_IDENTIFIER=%s\n", id);
+ if (len == 0)
+ goto error;
+
+ if (write(s, syslog_id, len) != len)
+ goto error;
+
+ free(syslog_id);
+
+ /* redirect other log messages to journal */
+ FILE *log_stream = fopencookie(NULL, "w", journal_log_func);
+ if (log_stream != NULL)
+ default_log_stream = log_stream;
+
+ return s;
+
+error:
+ free(syslog_id);
+ close(s);
+ return -1;
+}
+
static void
log_open_syslog(const char *id, bool is_terminal)
{
@@ -797,11 +934,24 @@ log_open_syslog(const char *id, bool is_terminal)
static void
log_output_selection(const char *id)
{
+#ifdef RTE_EXEC_ENV_WINDOWS
RTE_SET_USED(id);
-
-#ifndef RTE_EXEC_ENV_WINDOWS
+#else
bool is_terminal = isatty(STDERR_FILENO);
+ /* If stderr is redirected to systemd journal then upgrade */
+
[PATCH v18 15/15] log: colorize log output
Like dmesg, colorize the log output (unless redirected to file).
Timestamp is green, the subsystem is in yellow and the message
is red if urgent, boldface if an error, and normal for info and
debug messages.
Signed-off-by: Stephen Hemminger
---
app/test/test_eal_flags.c | 24
doc/guides/prog_guide/log_lib.rst | 16 ++-
lib/eal/common/eal_common_options.c | 11 ++
lib/eal/common/eal_options.h| 2 +
lib/log/log.c | 168 +++-
lib/log/log_internal.h | 5 +
lib/log/version.map | 1 +
7 files changed, 223 insertions(+), 4 deletions(-)
diff --git a/app/test/test_eal_flags.c b/app/test/test_eal_flags.c
index 08f4866461..c6c05e2e1d 100644
--- a/app/test/test_eal_flags.c
+++ b/app/test/test_eal_flags.c
@@ -1067,6 +1067,18 @@ test_misc_flags(void)
const char * const argv25[] = {prgname, prefix, mp_flag,
"--log-timestamp=invalid" };
+ /* Try running with --log-color */
+ const char * const argv26[] = {prgname, prefix, mp_flag,
+ "--log-color" };
+
+ /* Try running with --log-color=never */
+ const char * const argv27[] = {prgname, prefix, mp_flag,
+ "--log-color=never" };
+
+ /* Try running with --log-color=invalid */
+ const char * const argv28[] = {prgname, prefix, mp_flag,
+ "--log-color=invalid" };
+
/* run all tests also applicable to FreeBSD first */
@@ -1187,6 +1199,18 @@ test_misc_flags(void)
printf("Error - process did run ok with --log-timestamp=invalid
parameter\n");
goto fail;
}
+ if (launch_proc(argv26) != 0) {
+ printf("Error - process did not run ok with --log-color
parameter\n");
+ goto fail;
+ }
+ if (launch_proc(argv27) != 0) {
+ printf("Error - process did not run ok with --log-color=none
parameter\n");
+ goto fail;
+ }
+ if (launch_proc(argv28) == 0) {
+ printf("Error - process did run ok with --log-timestamp=invalid
parameter\n");
+ goto fail;
+ }
rmdir(hugepath_dir3);
diff --git a/doc/guides/prog_guide/log_lib.rst
b/doc/guides/prog_guide/log_lib.rst
index abaedc7212..40727ebaae 100644
--- a/doc/guides/prog_guide/log_lib.rst
+++ b/doc/guides/prog_guide/log_lib.rst
@@ -59,6 +59,21 @@ For example::
Within an application, the same result can be got using the
``rte_log_set_level_pattern()`` or ``rte_log_set_level_regex()`` APIs.
+Color output
+
+
+The log library will highlight important messages.
+This is controlled by the ``--log-color`` option.
+he optional argument ``when`` can be ``auto``, ``never``, or ``always``.
+The default setting is ``auto`` which enables color when the output to
+``stderr`` is a terminal.
+If the ``when`` argument is omitted, it defaults to ``always``.
+
+For example to turn off all coloring::
+
+ /path/to/app --log-color=none
+
+
Log timestamp
~
@@ -101,7 +116,6 @@ option. There are three possible settings for this option:
If ``--syslog`` option is not specified, then only console (stderr) will be
used.
-
Using Logging APIs to Generate Log Messages
---
diff --git a/lib/eal/common/eal_common_options.c
b/lib/eal/common/eal_common_options.c
index 9ca7db04aa..5e7ab29ae3 100644
--- a/lib/eal/common/eal_common_options.c
+++ b/lib/eal/common/eal_common_options.c
@@ -75,6 +75,7 @@ eal_long_options[] = {
{OPT_LCORES,1, NULL, OPT_LCORES_NUM },
{OPT_LOG_LEVEL, 1, NULL, OPT_LOG_LEVEL_NUM},
{OPT_LOG_TIMESTAMP, 2, NULL, OPT_LOG_TIMESTAMP_NUM},
+ {OPT_LOG_COLOR, 2, NULL, OPT_LOG_COLOR_NUM},
{OPT_TRACE, 1, NULL, OPT_TRACE_NUM},
{OPT_TRACE_DIR, 1, NULL, OPT_TRACE_DIR_NUM},
{OPT_TRACE_BUF_SIZE,1, NULL, OPT_TRACE_BUF_SIZE_NUM },
@@ -1618,6 +1619,7 @@ eal_log_level_parse(int argc, char * const argv[])
case OPT_LOG_LEVEL_NUM:
case OPT_SYSLOG_NUM:
case OPT_LOG_TIMESTAMP_NUM:
+ case OPT_LOG_COLOR_NUM:
if (eal_parse_common_option(opt, optarg, internal_conf)
< 0)
return -1;
break;
@@ -1862,6 +1864,14 @@ eal_parse_common_option(int opt, const char *optarg,
}
break;
+ case OPT_LOG_COLOR_NUM:
+ if (eal_log_color(optarg) < 0) {
+ EAL_LOG(ERR, "invalid parameters for --"
+ OPT_LOG_COLOR);
+ return -1;
+ }
+ break;
+
#ifndef RTE_EXEC_ENV_WINDOWS
case OPT_TRACE_NUM
[PATCH v18 13/15] log: add optional support of syslog
Log to syslog only if option is specified. And if syslog is used
then normally only log to syslog, don't duplicate output.
Also enables syslog support on FreeBSD.
Signed-off-by: Stephen Hemminger
---
app/test/test_eal_flags.c | 5 +-
doc/guides/linux_gsg/linux_eal_parameters.rst | 27
doc/guides/prog_guide/log_lib.rst | 17 +++
lib/eal/common/eal_common_options.c | 5 +-
lib/log/log.c | 121 --
5 files changed, 137 insertions(+), 38 deletions(-)
diff --git a/app/test/test_eal_flags.c b/app/test/test_eal_flags.c
index e54f6e8b7f..08f4866461 100644
--- a/app/test/test_eal_flags.c
+++ b/app/test/test_eal_flags.c
@@ -987,9 +987,10 @@ test_misc_flags(void)
/* With empty --syslog */
const char *argv3[] = {prgname, prefix, mp_flag, "--syslog"};
/* With valid --syslog */
- const char *argv4[] = {prgname, prefix, mp_flag, "--syslog", "always"};
+ const char *argv4[] = {prgname, prefix, mp_flag, "--syslog=both"};
/* With invalid --syslog */
- const char *argv5[] = {prgname, prefix, mp_flag, "--syslog", "error"};
+ const char *argv5[] = {prgname, prefix, mp_flag, "--syslog=invalid"};
+
/* With no-sh-conf, also use no-huge to ensure this test runs on BSD */
const char *argv6[] = {prgname, "-m", DEFAULT_MEM_SIZE,
no_shconf, nosh_prefix, no_huge};
diff --git a/doc/guides/linux_gsg/linux_eal_parameters.rst
b/doc/guides/linux_gsg/linux_eal_parameters.rst
index ea8f381391..d86f94d8a8 100644
--- a/doc/guides/linux_gsg/linux_eal_parameters.rst
+++ b/doc/guides/linux_gsg/linux_eal_parameters.rst
@@ -108,30 +108,3 @@ Memory-related options
* ``--match-allocations``
Free hugepages back to system exactly as they were originally allocated.
-
-Other options
-~
-
-* ``--syslog ``
-
-Set syslog facility. Valid syslog facilities are::
-
-auth
-cron
-daemon
-ftp
-kern
-lpr
-mail
-news
-syslog
-user
-uucp
-local0
-local1
-local2
-local3
-local4
-local5
-local6
-local7
diff --git a/doc/guides/prog_guide/log_lib.rst
b/doc/guides/prog_guide/log_lib.rst
index 504eefe1d2..abaedc7212 100644
--- a/doc/guides/prog_guide/log_lib.rst
+++ b/doc/guides/prog_guide/log_lib.rst
@@ -83,6 +83,23 @@ To prefix all console messages with ISO format time the
syntax is::
/path/to/app --log-timestamp=iso
+Log output
+~~
+
+If desired, messages can be redirected to syslog (on Linux and FreeBSD) with
the ``--syslog``
+option. There are three possible settings for this option:
+
+*always*
+Redirect all log output to syslog.
+
+*auto*
+Use console if it is a terminal, and use syslog if is not.
+
+*both*
+Print to both console and syslog.
+
+If ``--syslog`` option is not specified, then only console (stderr) will be
used.
+
Using Logging APIs to Generate Log Messages
diff --git a/lib/eal/common/eal_common_options.c
b/lib/eal/common/eal_common_options.c
index 5173835c2c..9ca7db04aa 100644
--- a/lib/eal/common/eal_common_options.c
+++ b/lib/eal/common/eal_common_options.c
@@ -91,7 +91,7 @@ eal_long_options[] = {
{OPT_PROC_TYPE, 1, NULL, OPT_PROC_TYPE_NUM},
{OPT_SOCKET_MEM,1, NULL, OPT_SOCKET_MEM_NUM },
{OPT_SOCKET_LIMIT, 1, NULL, OPT_SOCKET_LIMIT_NUM },
- {OPT_SYSLOG,1, NULL, OPT_SYSLOG_NUM },
+ {OPT_SYSLOG,2, NULL, OPT_SYSLOG_NUM },
{OPT_VDEV, 1, NULL, OPT_VDEV_NUM },
{OPT_VFIO_INTR, 1, NULL, OPT_VFIO_INTR_NUM},
{OPT_VFIO_VF_TOKEN, 1, NULL, OPT_VFIO_VF_TOKEN_NUM},
@@ -2221,6 +2221,9 @@ eal_common_usage(void)
" (can be used multiple times)\n"
" --"OPT_VMWARE_TSC_MAP"Use VMware TSC map instead of
native RDTSC\n"
" --"OPT_PROC_TYPE" Type of this process
(primary|secondary|auto)\n"
+#ifndef RTE_EXEC_ENV_WINDOWS
+ " --"OPT_SYSLOG"[=] Enable use of syslog\n"
+#endif
" --"OPT_LOG_LEVEL"= Set global log level\n"
" --"OPT_LOG_LEVEL"=:\n"
" Set specific log level\n"
diff --git a/lib/log/log.c b/lib/log/log.c
index 2dca91306e..ec0d55273e 100644
--- a/lib/log/log.c
+++ b/lib/log/log.c
@@ -13,15 +13,17 @@
#include
#include
+#ifdef RTE_EXEC_ENV_WINDOWS
+#include
+#else
+#include
+#endif
+
#include
#include
#include "log_internal.h"
-#ifdef RTE_EXEC_ENV_WINDOWS
-#include
-#endif
-
struct rte_log_dynamic_type {
const char *name;
uint32_t loglevel;
@@ -36,14 +38,25 @@ enum eal_log_time_format {
EAL_LOG_TIMESTAMP_ISO,
};
+enum eal_log_syslog {
+ EAL_LOG_SYSLOG_NONE
Re: [PATCH] vhost: optimize mbuf allocation in virtio Tx packed path
On Thu, Mar 28, 2024 at 04:44:26PM -0700, Stephen Hemminger wrote:
> On Thu, 28 Mar 2024 16:33:38 -0700
> Andrey Ignatov wrote:
>
> >
> > +static __rte_always_inline uint16_t
> > +get_nb_avail_entries_packed(const struct vhost_virtqueue *__rte_restrict
> > vq,
> > + uint16_t max_nb_avail_entries)
> > +{
>
> You don't need always inline, the compiler will do it anyway.
I can remove it in v2, but it's not completely obvious to me how is it
decided when to specify it explicitly and when not?
I see plenty of __rte_always_inline in this file:
% git grep -c '^static __rte_always_inline' lib/vhost/virtio_net.c
lib/vhost/virtio_net.c:66
Re: [PATCH 0/2] introduce PM QoS interface
在 2024/3/26 20:46, Morten Brørup 写道: From: lihuisong (C) [mailto:lihuis...@huawei.com] Sent: Tuesday, 26 March 2024 13.15 在 2024/3/26 16:27, Morten Brørup 写道: From: lihuisong (C) [mailto:lihuis...@huawei.com] Sent: Tuesday, 26 March 2024 03.12 在 2024/3/22 20:35, Morten Brørup 写道: From: lihuisong (C) [mailto:lihuis...@huawei.com] Sent: Friday, 22 March 2024 09.54 [...] For the case need PM QoS in DPDK, I think, it is better to set cpu latency to zero to prevent service thread from the deeper the idle state. It would defeat the purpose (i.e. not saving sufficient amounts of power) if the CPU cannot enter a deeper idle state. Yes, it is not good for power. AFAIS, PM QoS is just to decrease the influence for performance. Anyway, if we set to zero, system can be into Cstates-0 at least. Personally, I would think a wake-up latency of up to 10 microseconds should be fine for must purposes. Default Linux timerslack is 50 microseconds, so you could also use that value. How much CPU latency is ok. Maybe, we can give the decision to the application. Yes, the application should decide the acceptable worst-case latency. Linux will collect all these QoS request and use the minimum latency. what do you think, Morten? For the example application, you could use a value of 50 microseconds and refer to this value also being the default timerslack in Linux. There is a description for "/proc//timerslack_ns" in Linux document [1] " This file provides the value of the task’s timerslack value in nanoseconds. This value specifies an amount of time that normal timers may be deferred in order to coalesce timers and avoid unnecessary wakeups. This allows a task’s interactivity vs power consumption tradeoff to be adjusted. " I cannot understand what the relationship is between the timerslack in Linux and cpu latency to wake up. It seems that timerslack is just to defer the timer in order to coalesce timers and avoid unnecessary wakeups. And it has not a lot to do with the CPU latency which is aimed to avoid task to enter deeper idle state and satify application request. Correct. They control two different things. However, both can cause latency for the application, so my rationale for the relationship was: If the application accepts X us of latency caused by kernel scheduling delays (caused by timerslack), the application should accept the same amount of latency caused by CPU wake-up latency. Understand, thanks for explain. This also means that if you want lower latency than 50 us, you should not only set cpu wake-up latency, you should also set timerslack. Obviously, if the application is only affected by one of the two, the application only needs to adjust that one of them. Yes, I think it is. As for the 50 us value, someone in the Linux kernel team decided that 50 us was an acceptable amount of latency for the kernel; we could use the same value, referring to that. Or we could choose some other value, and describe how we came up with our own value. And if necessary, also adjust timerslack accordingly. So how about use the default 50us of timerslack in l3fwd-power? And we add some description about this in code or document, like, suggest user also need to modify this process's timerslack if want a more little latency.
Re: [PATCH v3 00/45] use stdatomic API
Recheck-request: iol-unit-amd64-testing
Re: [PATCH v2 1/6] ethdev: support setting lanes
在 2024/3/26 21:47, Ajit Khaparde 写道: On Tue, Mar 26, 2024 at 4:15 AM lihuisong (C) wrote: 在 2024/3/26 18:30, Thomas Monjalon 写道: 26/03/2024 02:42, lihuisong (C): 在 2024/3/25 17:30, Thomas Monjalon 写道: 25/03/2024 07:24, huangdengdui: On 2024/3/22 21:58, Thomas Monjalon wrote: 22/03/2024 08:09, Dengdui Huang: -#define RTE_ETH_LINK_SPEED_10G RTE_BIT32(8) /**< 10 Gbps */ -#define RTE_ETH_LINK_SPEED_20G RTE_BIT32(9) /**< 20 Gbps */ -#define RTE_ETH_LINK_SPEED_25G RTE_BIT32(10) /**< 25 Gbps */ -#define RTE_ETH_LINK_SPEED_40G RTE_BIT32(11) /**< 40 Gbps */ -#define RTE_ETH_LINK_SPEED_50G RTE_BIT32(12) /**< 50 Gbps */ -#define RTE_ETH_LINK_SPEED_56G RTE_BIT32(13) /**< 56 Gbps */ -#define RTE_ETH_LINK_SPEED_100GRTE_BIT32(14) /**< 100 Gbps */ -#define RTE_ETH_LINK_SPEED_200GRTE_BIT32(15) /**< 200 Gbps */ -#define RTE_ETH_LINK_SPEED_400GRTE_BIT32(16) /**< 400 Gbps */ +#define RTE_ETH_LINK_SPEED_10GRTE_BIT32(8) /**< 10 Gbps */ +#define RTE_ETH_LINK_SPEED_20GRTE_BIT32(9) /**< 20 Gbps 2lanes */ +#define RTE_ETH_LINK_SPEED_25GRTE_BIT32(10) /**< 25 Gbps */ +#define RTE_ETH_LINK_SPEED_40GRTE_BIT32(11) /**< 40 Gbps 4lanes */ +#define RTE_ETH_LINK_SPEED_50GRTE_BIT32(12) /**< 50 Gbps */ +#define RTE_ETH_LINK_SPEED_56GRTE_BIT32(13) /**< 56 Gbps 4lanes */ +#define RTE_ETH_LINK_SPEED_100G RTE_BIT32(14) /**< 100 Gbps */ +#define RTE_ETH_LINK_SPEED_200G RTE_BIT32(15) /**< 200 Gbps 4lanes */ +#define RTE_ETH_LINK_SPEED_400G RTE_BIT32(16) /**< 400 Gbps 4lanes */ +#define RTE_ETH_LINK_SPEED_10G_4LANES RTE_BIT32(17) /**< 10 Gbps 4lanes */ +#define RTE_ETH_LINK_SPEED_50G_2LANES RTE_BIT32(18) /**< 50 Gbps 2 lanes */ +#define RTE_ETH_LINK_SPEED_100G_2LANESRTE_BIT32(19) /**< 100 Gbps 2 lanes */ +#define RTE_ETH_LINK_SPEED_100G_4LANESRTE_BIT32(20) /**< 100 Gbps 4lanes */ +#define RTE_ETH_LINK_SPEED_200G_2LANESRTE_BIT32(21) /**< 200 Gbps 2lanes */ +#define RTE_ETH_LINK_SPEED_400G_8LANESRTE_BIT32(22) /**< 400 Gbps 8lanes */ I don't think it is a good idea to make this more complex. It brings nothing as far as I can see, compared to having speed and lanes separated. Can we have lanes information a separate value? no need for bitmask. Hi,Thomas, Ajit, roretzla, damodharam I also considered the option at the beginning of the design. But this option is not used due to the following reasons: 1. For the user, ethtool couples speed and lanes. The result of querying the NIC capability is as follows: Supported link modes: 10baseSR4/Full 10baseSR2/Full The NIC capability is configured as follows: ethtool -s eth1 speed 10 lanes 4 autoneg off ethtool -s eth1 speed 10 lanes 2 autoneg off Therefore, users are more accustomed to the coupling of speed and lanes. 2. For the PHY, When the physical layer capability is configured through the MDIO, the speed and lanes are also coupled. For example: Table 45–7—PMA/PMD control 2 register bit definitions[1] PMA/PMD type selection 1 0 0 1 0 1 0 = 100GBASE-SR2 PMA/PMD 0 1 0 1 1 1 1 = 100GBASE-SR4 PMA/PMD Therefore, coupling speeds and lanes is easier to understand. And it is easier for the driver to report the support lanes. In addition, the code implementation is compatible with the old version. When the driver does not support the lanes setting, the code does not need to be modified. So I think the speed and lanes coupling is better. I don't think so. You are mixing hardware implementation, user tool, and API. Having a separate and simple API is cleaner and not more difficult to handle in some get/set style functions. Having a separate and simple API is cleaner. It's good. But supported lane capabilities have a lot to do with the specified speed. This is determined by releated specification. If we add a separate API for speed lanes, it probably is hard to check the validity of the configuration for speed and lanes. And the setting lane API sepparated from speed is not good for uniforming all PMD's behavior in ethdev layer. Please let's be more specific. There are 3 needs: - set PHY lane config - get PHY lane config - get PHY lane capabilities IMO, this lane capabilities should be reported based on supported speed capabilities. There is no problem providing a function to get the number of PHY lanes. It is possible to set PHY lanes number after defining a fixed speed. yes it's ok. The patch[1] is an example for this separate API. I think it is not very good. It cannot tell user and PMD the follow points: 1) user don't know what lanes should or can be set for a specified speed on one NIC. This is about capabilities. Can we say a HW will support a maximum number of PHY lanes in general? We may need to associate the maximum speed per lane? Do we really need to associate PHY lane and PHY speed numbers for capabili
