RE: [EXTERNAL] [PATCH] graph: avoid accessing graph list when getting stats
> -Original Message-
> From: Robin Jarry
> Sent: Monday, March 25, 2024 9:23 PM
> To: dev@dpdk.org; Jerin Jacob ; Kiran Kumar
> Kokkilagadda ; Nithin Kumar Dabilpuram
> ; Zhirun Yan
> Subject: [EXTERNAL] [PATCH] graph: avoid accessing graph list when getting
> stats
>
> Prioritize security for external emails: Confirm sender and content safety
> before clicking links or opening attachments
>
> --
> In rte_graph_cluster_stats_get, the walk model of the first graph is checked
> to determine if multi-core dispatch specific counters should be updated or
> not. This global list is accessed without any locks.
>
> If the global list is modified by another thread while
> rte_graph_cluster_stats_get is called, it can result in undefined behaviour.
>
> Adding a lock would make it impossible to call rte_graph_cluster_stats_get in
> packet processing code paths. Avoid accessing the global list instead by
> storing a bool field in the private rte_graph_cluster_stats structure.
>
> Also update the default callback to avoid accessing the global list and use a
> different default callback depending on the graph model.
>
> Signed-off-by: Robin Jarry
> ---
> lib/graph/graph_stats.c | 60 ++---
> 1 file changed, 39 insertions(+), 21 deletions(-)
>
> diff --git a/lib/graph/graph_stats.c b/lib/graph/graph_stats.c index
> 2fb808b21ec5..f8048e08be8a 100644
> --- a/lib/graph/graph_stats.c
> +++ b/lib/graph/graph_stats.c
> @@ -34,6 +34,7 @@ struct __rte_cache_aligned rte_graph_cluster_stats {
> uint32_t cluster_node_size; /* Size of struct cluster_node */
> rte_node_t max_nodes;
> int socket_id;
> + bool dispatch;
> void *cookie;
> size_t sz;
>
> @@ -74,17 +75,16 @@ print_banner_dispatch(FILE *f) }
>
> static inline void
> -print_banner(FILE *f)
> +print_banner(FILE *f, bool dispatch)
> {
> - if
> (rte_graph_worker_model_get(STAILQ_FIRST(graph_list_head_get())->graph)
> ==
> - RTE_GRAPH_MODEL_MCORE_DISPATCH)
> + if (dispatch)
> print_banner_dispatch(f);
> else
> print_banner_default(f);
> }
>
> static inline void
> -print_node(FILE *f, const struct rte_graph_cluster_node_stats *stat)
> +print_node(FILE *f, const struct rte_graph_cluster_node_stats *stat,
> +bool dispatch)
> {
> double objs_per_call, objs_per_sec, cycles_per_call, ts_per_hz;
> const uint64_t prev_calls = stat->prev_calls; @@ -104,8 +104,7 @@
> print_node(FILE *f, const struct rte_graph_cluster_node_stats *stat)
> objs_per_sec = ts_per_hz ? (objs - prev_objs) / ts_per_hz : 0;
> objs_per_sec /= 100;
>
> - if
> (rte_graph_worker_model_get(STAILQ_FIRST(graph_list_head_get())->graph)
> ==
> - RTE_GRAPH_MODEL_MCORE_DISPATCH) {
> + if (dispatch) {
> fprintf(f,
> "|%-31s|%-15" PRIu64 "|%-15" PRIu64 "|%-15" PRIu64
> "|%-15" PRIu64 "|%-15" PRIu64
> @@ -123,20 +122,17 @@ print_node(FILE *f, const struct
> rte_graph_cluster_node_stats *stat) }
>
> static int
> -graph_cluster_stats_cb(bool is_first, bool is_last, void *cookie,
> +graph_cluster_stats_cb(bool dispatch, bool is_first, bool is_last, void
> +*cookie,
> const struct rte_graph_cluster_node_stats *stat) {
> FILE *f = cookie;
> - int model;
> -
> - model =
> rte_graph_worker_model_get(STAILQ_FIRST(graph_list_head_get())->graph);
>
> if (unlikely(is_first))
> - print_banner(f);
> + print_banner(f, dispatch);
> if (stat->objs)
> - print_node(f, stat);
> + print_node(f, stat, dispatch);
> if (unlikely(is_last)) {
> - if (model == RTE_GRAPH_MODEL_MCORE_DISPATCH)
> + if (dispatch)
> boarder_model_dispatch();
> else
> boarder();
> @@ -145,6 +141,20 @@ graph_cluster_stats_cb(bool is_first, bool is_last,
> void *cookie,
> return 0;
> };
>
> +static int
> +graph_cluster_stats_cb_rtc(bool is_first, bool is_last, void *cookie,
> +const struct rte_graph_cluster_node_stats *stat) {
> + return graph_cluster_stats_cb(false, is_first, is_last, cookie, stat);
> +};
> +
> +static int
> +graph_cluster_stats_cb_dispatch(bool is_first, bool is_last, void *cookie,
> + const struct rte_graph_cluster_node_stats
> *stat) {
> + return graph_cluster_stats_cb(true, is_first, is_last, cookie, stat);
> +};
> +
> static struct rte_graph_cluster_stats * stats_mem_init(struct cluster
> *cluster,
> const struct rte_graph_cluster_stats_param *prm) @@ -157,8
> +167,16 @@ stats_mem_init(struct cluster *cluster,
>
> /* Fix up callback */
> fn = prm->fn;
> - if (fn == NULL)
> - fn = graph_cluster_stats_cb;
> + if (fn == NULL) {
> +
Re: [dpdk-dev] [PATCH v2] net/i40e: fix forward outer IPv6 VXLAN packets
Hello Bruce, John,
On Fri, Nov 5, 2021 at 4:39 AM Jie Wang wrote:
>
> Testpmd forwards packets in checksum mode that it need to calculate
> the checksum of each layer's protocol. Then it will fill flags and
> header length into mbuf.
>
> In process_outer_cksums, HW calculates the outer checksum if
> tx_offloads contains outer UDP checksum otherwise SW calculates
> the outer checksum.
>
> When tx_offloads contains outer UDP checksum or outer IPv4 checksum,
> mbuf will be filled with correct header length.
>
> This patch added outer UDP checksum in tx_offload_capa and
> I40E_TX_OFFLOAD_MASK, when we set csum hw outer-udp on that the
> engine can forward outer IPv6 VXLAN packets.
>
> Fixes: 7497d3e2f777 ("net/i40e: convert to new Tx offloads API")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Jie Wang
- There is a bz opened by a OVS user trying to offload geneve checksum.
https://bugs.dpdk.org/show_bug.cgi?id=1406
Reading the X7xx datasheet, parsing i40e_parse_tunneling_params() and
looking at the packets reported by the user, I understand that outer
udp checksum is actually *not* supported by net/i40e.
And so the change from this mail thread should be reverted as the
driver falsely claims support for this feature.
- I found some bits about X722 (5535087e6c56 ("i40e/base: add outer
UDP checksum for X722")) supporting this feature, but I did not find a
definition in the datasheet.
Besides, this I40E_TXD_CTX_QW0_L4T_CS_MASK is not used in the net/i40e tx path.
We need Intel to clear state what is supported or not, and send fixes
accordingly.
Thanks.
--
David Marchand
Re: [dpdk-dev] [PATCH] net/ice: fix outer UDP Tx offload checksum error
Hello,
On Mon, Nov 23, 2020 at 8:08 AM Murphy Yang wrote:
>
> If enable hardware outer UDP TX offload checksum, it doesn't take effect
> when send 'IPv6/UDP/VXLAN' packet with error outer UDP checksum.
>
> In order to take effect, set the 'L4T_CS' flag valid only when 'L4TUNT'
> equals one and 'EIPT' is not zero. If 'L4T_CS' flag marked, the hardware
> can calculate the outer tunneling UDP checksum.
>
> Fixes: bd70c451532c ("net/ice: support Tx checksum offload for tunnel")
>
> Signed-off-by: Murphy Yang
> ---
> drivers/net/ice/ice_rxtx.c | 7 +--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c
> index 5fbd68eafc..9769e216bf 100644
> --- a/drivers/net/ice/ice_rxtx.c
> +++ b/drivers/net/ice/ice_rxtx.c
> @@ -2319,8 +2319,11 @@ ice_parse_tunneling_params(uint64_t ol_flags,
> *cd_tunneling |= (tx_offload.l2_len >> 1) <<
> ICE_TXD_CTX_QW0_NATLEN_S;
>
> - if ((ol_flags & PKT_TX_OUTER_UDP_CKSUM) &&
> - (ol_flags & PKT_TX_OUTER_IP_CKSUM) &&
> + /**
> +* Calculate the tunneling UDP checksum.
> +* Shall be set only if L4TUNT = 01b and EIPT is not zero
> +*/
> + if (!(*cd_tunneling & ICE_TX_CTX_EIPT_NONE) &&
drivers/net/ice/base/ice_lan_tx_rx.h: ICE_TX_CTX_EIPT_NONE= 0x0,
So this check above is always true...
I suspect the intent was:
if ((*cd_tunneling & ICE_TXD_CTX_QW0_EIPT_M) != ICE_TX_CTX_EIPT_NONE)
> (*cd_tunneling & ICE_TXD_CTX_UDP_TUNNELING))
> *cd_tunneling |= ICE_TXD_CTX_QW0_L4T_CS_M;
> }
> --
> 2.17.1
>
--
David Marchand
Re: [EXTERNAL] [PATCH] graph: avoid accessing graph list when getting stats
Kiran Kumar Kokkilagadda, Mar 29, 2024 at 08:44:
> +167,16 @@ stats_mem_init(struct cluster *cluster,
>
> /* Fix up callback */
>fn = prm->fn;
> - if (fn == NULL)
> - fn = graph_cluster_stats_cb;
> + if (fn == NULL) {
> + for (int i = 0; i < cluster->nb_graphs; i++) {
> + const struct rte_graph *graph = cluster->graphs[i]- graph;
> + if (graph->model == RTE_GRAPH_MODEL_MCORE_DISPATCH)
> + fn = graph_cluster_stats_cb_dispatch;
> + else
> + fn = graph_cluster_stats_cb_rtc;
> + break;
> + }
> + }
>
Do we need loop here? Just take cluster->graphs[0] and remove break?
I wasn't sure if the cluster could be empty at this point. If it is
guaranteed to contain at least one graph, I'll send a v2 without this
loop.
Thanks!
Re: [PATCH v16 1/8] net/ntnic: initial commit which adds register defines
On 10/9/2023 8:57 AM, Christian Koue Muf wrote: > On 9/29/2023 12:24 PM, Thomas Monjalon wrote: >> 29/09/2023 11:46, Ferruh Yigit: >>> On 9/29/2023 10:21 AM, Christian Koue Muf wrote: On 9/21/2023 4:05 PM, Ferruh Yigit wrote: > On 9/20/2023 2:17 PM, Thomas Monjalon wrote: >> Hello, >> >> 19/09/2023 11:06, Christian Koue Muf: >>> On 9/18/23 10:34 AM, Ferruh Yigit wrote: On 9/15/2023 7:37 PM, Morten Brørup wrote: >> From: Ferruh Yigit [mailto:ferruh.yi...@amd.com] >> Sent: Friday, 15 September 2023 17.55 >> >> On 9/8/2023 5:07 PM, Mykola Kostenok wrote: >>> From: Christian Koue Muf >>> >>> The NTNIC PMD does not rely on a kernel space Napatech >>> driver, thus all defines related to the register layout is >>> part of the PMD code, which will be added in later commits. >>> >>> Signed-off-by: Christian Koue Muf >>> Reviewed-by: Mykola Kostenok >>> >> >> Hi Mykola, Christiam, >> >> This PMD scares me, overall it is a big drop: >> "249 files changed, 87128 insertions(+)" >> >> I think it is not possible to review all in one release cycle, >> and it is not even possible to say if all code used or not. >> >> I can see code is already developed, and it is difficult to >> restructure developed code, but restructure it into small >> pieces really helps for reviews. >> >> >> Driver supports good list of features, can it be possible to >> distribute upstream effort into multiple release. >> Starting from basic functionality and add features gradually. >> Target for this release can be providing datapath, and add >> more if we have time in the release, what do you think? >> >> I was expecting to get only Rx/Tx in this release, not really more. >> >> I agree it may be interesting to discuss some design and check >> whether we need more features in ethdev as part of the driver >> upstreaming process. >> >> >> Also there are large amount of base code (HAL / FPGA code), >> instead of adding them as a bulk, relevant ones with a feature >> can be added with the feature patch, this eliminates dead code >> in the base code layer, also helps user/review to understand >> the link between driver code and base code. >> >> Yes it would be interesting to see what is really needed for the >> basic initialization and what is linked to a specific offload or >> configuration feature. >> >> As a maintainer, I have to do some changes across all drivers >> sometimes, and I use git blame a lot to understand why something was >> added. >> >> > Jumping in here with an opinion about welcoming new NIC vendors to > the community: > > Generally, if a NIC vendor supplies a PMD for their NIC, I expect the > vendor to take responsibility for the quality of the PMD, including > providing a maintainer and support backporting of fixes to the PMD in > LTS releases. This should align with the vendor's business case for > upstreaming their driver. > > If the vendor provides one big patch series, which may be difficult > to understand/review, the fallout mainly hits the vendor's customers > (and thus the vendor's support organization), not the community as a > whole. > Hi Morten, I was thinking same before making my above comment, what happens if vendors submit as one big patch and when a problem occurs we can ask owner to fix. Probably this makes vendor happy and makes my life (or any other maintainer's life) easier, it is always easier to say yes. But I come up with two main reasons to ask for a rework: 1- Technically any vendor can deliver their software to their customers via a public git repository, they don't have to upstream to https://linkprotect.cudasvc.com/url?a=https%3a%2f%2fdpdk.org&c=E ,1,N poJejuuvPdOPfcFJYtsmkQF6PVrDjGsZ8x_gi5xDrTyZokK_nM11u4ZpzHgM10J9 bOLl nhoR6fFAzWtCzOhRCzVruYj520zZORv6-MjJeSC5TrGnIFL&typo=1, but upstreaming has many benefits. One of those benefits is upstreaming provides a quality assurance for vendor's customers (that is why customer can be asking for this, as we are having in many cases), and this quality assurance comes from additional eyes reviewing the code and guiding vendors for the DPDK quality standards (some vendors already doing pretty good, but new ones sometimes requires hand-holding). If driver is one big patch series, it is practically
Re: [PATCH v6 1/8] net/rnp: add skeleton
On 9/1/2023 3:30 AM, Wenbo Cao wrote: > Add Basic PMD library and doc build infrastructure > Update maintainers file to claim responsibility. > > Signed-off-by: Wenbo Cao > Hi Wenbo, What is the status of the 'rnp' driver, v7 was expected but not received, will upstreaming continue for v24.03?
Re: [PATCH v5 00/32] Introduce sssnic PMD for 3SNIC's 9x0 serials Ethernet adapters
On 9/26/2023 2:13 PM, Ferruh Yigit wrote: > On 9/4/2023 5:56 AM, wa...@3snic.com wrote: >> From: Renyong Wan >> >> The sssnic PMD (**librte_pmd_sssnic**) provides poll mode driver support >> for 3SNIC 9x0 serials family of Ethernet adapters. >> >> Supported NICs are: >> >> - 3S910 Dual Port SFP28 10/25GbE Ethernet adapter >> - 3S920 Quad Port SFP28 10/25GbE Ethernet adapter >> - 3S920 Quad Port QSFP28 100GbE Ethernet adapter >> >> Features of sssnic PMD are: >> >> - Link status >> - Link status event >> - Queue start/stop >> - Rx interrupt >> - Scattered Rx >> - TSO >> - LRO >> - Promiscuous mode >> - Allmulticast mode >> - Unicast MAC filter >> - Multicast MAC filte >> - RSS hash >> - RSS key update >> - RSS reta update >> - Inner RSS >> - VLAN filter >> - VLAN offload >> - L3 checksum offload >> - L4 checksum offload >> - Inner L3 checksum >> - Inner L4 checksum >> - Basic stats >> - Extended stats >> - Stats per queue >> - Flow control >> - FW version >> - Generic flow API >> >> --- >> v2: >> * Fixed 'Title underline too short' in doc/guides/nics/sssnic.rst. >> * Removed error.h from including files. >> * Fixed variable 'cmd_len' is uninitialized when used. >> * Fixed 'EINVAL' undeclared. >> * Fixed wrong format of printing uint64_t. >> * Fixed 'mask->hdr.src_addr' will always evaluate to 'true'. >> >> v3: >> * Fixed dereferencing type-punned pointer in base/sssnic_mbox.c. >> >> v4: >> * Fixed dereferencing type-punned pointer in base/sssnic_eventq.c. >> * Fixed coding style issue of COMPLEX_MACRO. >> * Fixed coding style issue of REPEATED_WORD. >> >> v5: >> * Fixed rebase mistake. >> * Fixed incorrect path in MAINTAINERS file. >> --- >> >> Renyong Wan (32): >> net/sssnic: add build and doc infrastructure >> net/sssnic: add log type and log macros >> net/sssnic: support probe and remove >> net/sssnic: initialize hardware base >> net/sssnic: add event queue >> net/sssnic/base: add message definition and utility >> net/sssnic/base: add mailbox support >> net/sssnic/base: add work queue >> net/sssnic/base: add control queue >> net/sssnic: add dev configure and infos get >> net/sssnic: add dev MAC ops >> net/sssnic: support dev link status >> net/sssnic: support link status event >> net/sssnic: support Rx queue setup and release >> net/sssnic: support Tx queue setup and release >> net/sssnic: support Rx queue start and stop >> net/sssnic: support Tx queue start and stop >> net/sssnic: add Rx interrupt support >> net/sssnic: support dev start and stop >> net/sssnic: support dev close and reset >> net/sssnic: add allmulticast and promiscuous ops >> net/sssnic: add basic and extended stats ops >> net/sssnic: support Rx packet burst >> net/sssnic: support Tx packet burst >> net/sssnic: add RSS support >> net/sssnic: support dev MTU set >> net/sssnic: support dev queue info get >> net/sssnic: support dev firmware version get >> net/sssnic: add dev flow control ops >> net/sssnic: support VLAN offload and filter >> net/sssnic: add generic flow ops >> net/sssnic: add VF dev support >> > > Hi Renyong, > > Driver mostly looks good, I did put some minor comments, I guess driver > can be merged soon with those issues addressed. > > Also agree on Stephen's comments, if you apply them in next version, > please feel free to keep Stephen's ack in next version. > > Hi Renyong, v5 was almost there, will there be a new version for the v24.07?
Re: [PATCH] vhost: optimize mbuf allocation in virtio Tx packed path
Hi Stephen, On 3/29/24 03:53, Stephen Hemminger wrote: On Thu, 28 Mar 2024 17:10:42 -0700 Andrey Ignatov wrote: 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 Cargo cult really. Cargo cult... really? Well, I just did a quick test by comparing IO forwarding with testpmd between main branch and with adding a patch that removes all the inline/noinline in lib/vhost/virtio_net.c [0]. main branch: 14.63Mpps main branch - inline/noinline: 10.24Mpps Andrey, thanks for the patch, I'll have a look at it next week. Maxime [0]: https://pastebin.com/72P2npZ0
The effect of inlining
+CC techboard > From: Maxime Coquelin [mailto:maxime.coque...@redhat.com] > Sent: Friday, 29 March 2024 14.05 > > Hi Stephen, > > On 3/29/24 03:53, Stephen Hemminger wrote: > > On Thu, 28 Mar 2024 17:10:42 -0700 > > Andrey Ignatov wrote: > > > >>> > >>> 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 > > > > > > Cargo cult really. > > > > Cargo cult... really? > > Well, I just did a quick test by comparing IO forwarding with testpmd > between main branch and with adding a patch that removes all the > inline/noinline in lib/vhost/virtio_net.c [0]. > > main branch: 14.63Mpps > main branch - inline/noinline: 10.24Mpps Thank you for testing this, Maxime. Very interesting! It is sometimes suggested on techboard meetings that we should convert more inline functions to non-inline for improved API/ABI stability, with the argument that the performance of inlining is negligible. I think this test proves that the sum of many small (negligible) performance differences it not negligible! > > Andrey, thanks for the patch, I'll have a look at it next week. > > Maxime > > [0]: https://pastebin.com/72P2npZ0
RE: [PATCH v6 1/8] net/rnp: add skeleton
Hi Ferruh, Thanks for your reminder, I'm sorry for that I had been work on anothing before. Recendly, I have been reworked on this work. It will miss on release of v24.03. For another thing, I'm always confused for the secondary process call like hw->mac_ops this function pointer. is this method can work normally on secondary process ? For example,ixgbe on secondary process call ixgbe_get_module_eeprom I fond that it will cause process core-dump. because of secondary process use primary process function-point address. Is dpdk plaform allowed user call the eth_dev_ops on secondary process? I don't find any limit and don't know wether or not to protect this function call work normally. My driver eth_dev_ops achieve will used some function-pointer, so I'm confued about this. Wish your kind guidance. Regadrs Wenbo > -Original Message- > From: ferruh.yi...@amd.com > Sent: 2024年3月29日 19:28 > To: Wenbo Cao ; Thomas Monjalon > > Cc: dev@dpdk.org; andrew.rybche...@oktetlabs.ru; yao...@mucse.com > Subject: Re: [PATCH v6 1/8] net/rnp: add skeleton > > On 9/1/2023 3:30 AM, Wenbo Cao wrote: > > Add Basic PMD library and doc build infrastructure Update maintainers > > file to claim responsibility. > > > > Signed-off-by: Wenbo Cao > > > > Hi Wenbo, > > What is the status of the 'rnp' driver, v7 was expected but not received, will > upstreaming continue for v24.03?
Re: The effect of inlining
On Fri, Mar 29, 2024 at 02:42:49PM +0100, Morten Brørup wrote: > +CC techboard > > > From: Maxime Coquelin [mailto:maxime.coque...@redhat.com] > > Sent: Friday, 29 March 2024 14.05 > > > > Hi Stephen, > > > > On 3/29/24 03:53, Stephen Hemminger wrote: > > > On Thu, 28 Mar 2024 17:10:42 -0700 > > > Andrey Ignatov wrote: > > > > > >>> > > >>> 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 > > > > > > > > > Cargo cult really. > > > > > > > Cargo cult... really? > > > > Well, I just did a quick test by comparing IO forwarding with testpmd > > between main branch and with adding a patch that removes all the > > inline/noinline in lib/vhost/virtio_net.c [0]. > > > > main branch: 14.63Mpps > > main branch - inline/noinline: 10.24Mpps > > Thank you for testing this, Maxime. Very interesting! > > It is sometimes suggested on techboard meetings that we should convert more > inline functions to non-inline for improved API/ABI stability, with the > argument that the performance of inlining is negligible. removing inline functions probably has an even more profound negative impact when using dynamic linking. for all the value of msvc's dll scoped security features they do have overheads per-call that can't be wished away i imagine equivalents in gcc are the same. > > I think this test proves that the sum of many small (negligible) performance > differences it not negligible! sure looks that way, though i think there is some distinction to be made between inline and *forced* inline. force inline may be losing us some opportunity for the compiler to optimize better than is obvious to us. > > > > > Andrey, thanks for the patch, I'll have a look at it next week. > > > > Maxime > > > > [0]: https://pastebin.com/72P2npZ0 >
[DPDK/core Bug 1409] arparse library assumes enum are 64 bit
https://bugs.dpdk.org/show_bug.cgi?id=1409 Bug ID: 1409 Summary: arparse library assumes enum are 64 bit Product: DPDK Version: 24.03 Hardware: All OS: All Status: UNCONFIRMED Severity: normal Priority: Normal Component: core Assignee: dev@dpdk.org Reporter: step...@networkplumber.org Target Milestone: --- MSVC correctly flags that this line in rte_argparse.h is incorrect: RTE_ARGPARSE_ARG_RESERVED_FIELD = RTE_GENMASK64(63, 48), The problem is that enum values are just an alias for int, and it can be 32 bits. Taken from the current C Standard (C99): http://www.open std.org/JTC1/SC22/WG14/www/docs/n1256.pdf 6.7.2.2 Enumeration specifiers [...] Constraints The expression that defines the value of an enumeration constant shall be an integer constant expression that has a value representable as an int. [...] Each enumerated type shall be compatible with char, a signed integer type, or an unsigned integer type. The choice of type is implementation-defined, but shall be capable of representing the values of all the members of the enumeration. Since rte_argparse only uses 10 bits now. The suggested fix here is to: 1. Assume 32 bits 2. Get rid of the reserved field - reserved fields are bad idea -- You are receiving this mail because: You are the assignee for the bug.
[PATCH v19 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 v19 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. Color is used by many tools (vi, iproute2, git) because it is helpful; 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... v19 - split log outputs into seperate files, which makes it more OO and cleaner. - make journal log configurable (mostly for testing) 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 | 71 ++ drivers/bus/pci/pci_common.c | 32 --- lib/eal/common/eal_common_debug.c | 11 +- lib/eal/common/eal_common_options.c | 137 ++- lib/eal/common/eal_options.h | 7 + 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 | 87 --- lib/log/log_color.c | 160 + lib/log/log_freebsd.c | 12 - lib/log/log_internal.h| 28 ++- lib/log/log_journal.c | 216 ++ lib/log/log_linux.c | 61 - lib/log/log_private.h | 34 +++ lib/log/log_stubs.c | 34 +++ lib/log/log_syslog.c | 89 lib/log/log_timestamp.c | 204 + lib/log/log_windows.c | 18 -- lib/log/meson.build | 14 +- lib/log/version.map | 5 +- 28 files changed, 1172 insertions(+), 448 deletions(-) create mode 100644 lib/log/log_color.c delete mode 100644 lib/log/log_freebsd.c create mode 100644 lib/log/log_journal.c delete mode 100644 lib/log/log_linux.c create mode 100644 lib/log/log_private.h create mode 100644 lib/log/log_stubs.c create mode 100644 lib/log/log_syslog.c create mode 100644 lib/log/log_timestamp.c delete mode 100644 lib/log/log_windows.c -- 2.43.0
[PATCH v19 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 v19 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 v19 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 v19 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 v19 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 v19 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 v19 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.c | 2 ++
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 +
10 files changed, 70 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);
@
[PATCH v19 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 v19 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 | 41 +--
lib/log/log_freebsd.c | 11 ---
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 -
9 files changed, 23 insertions(+), 179 deletions(-)
delete mode 100644 lib/log/log_freebsd.c
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 4b24e145b6..3fe86ddcd7 100644
--- a/lib/log/log.c
+++ b/lib/log/log.c
@@ -57,9 +57,6 @@ TAILQ_HEAD(rte_eal_opt_loglevel_list, rte_eal_opt_loglevel);
static struct rte_eal_opt_loglevel_list opt_loglevel_list =
TAILQ_HEAD_INITIALIZER(opt_loglevel_list);
-/* Stream to use for logging if rte_logs.file is NULL */
-static FILE *default_log_stream;
-
/**
* This global structure stores some information about the message
* that is currently being processed by one lcore
@@ -72,8 +69,6 @@ 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)
@@ -87,17 +82,7 @@ rte_log_get_stream(void)
{
FILE *f = rte_logs.file;
- if (f == NULL) {
- /*
-* Grab the current value of stderr here, rather than
-* just initializing default_log_stream to stderr. This
-* ensures that we will always use the current value
-* of stderr, even if the application closes and
-* reopens it.
-*/
- return default_log_stream != NULL ? default_log_stream : stderr;
- }
- return f;
+ return (f == NULL) ? stderr : f;
}
/* Set global log level */
@@ -507,14 +492,19 @@ 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;
-
#if RTE_LOG_DP_LEVEL >= RTE_LOG_DEBUG
RTE_LOG(NOTICE, EAL,
"Debug datapl
[PATCH v19 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 | 14 +-
lib/log/log_private.h | 11 +++
2 files changed, 24 insertions(+), 1 deletion(-)
create mode 100644 lib/log/log_private.h
diff --git a/lib/log/log.c b/lib/log/log.c
index 3fe86ddcd7..71b00528d0 100644
--- a/lib/log/log.c
+++ b/lib/log/log.c
@@ -18,6 +18,7 @@
#include
#include "log_internal.h"
+#include "log_private.h"
#ifdef RTE_EXEC_ENV_WINDOWS
#define strdup _strdup
@@ -28,16 +29,19 @@ struct rte_log_dynamic_type {
uint32_t loglevel;
};
+
/** 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 {
@@ -74,6 +78,7 @@ int
rte_openlog_stream(FILE *f)
{
rte_logs.file = f;
+ rte_logs.print_func = log_print;
return 0;
}
@@ -470,7 +475,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;
}
@@ -499,6 +504,13 @@ eal_log_syslog(const char *mode __rte_unused)
return -1;
}
+/* default log print function */
+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
*/
diff --git a/lib/log/log_private.h b/lib/log/log_private.h
new file mode 100644
index 00..afd833c3bd
--- /dev/null
+++ b/lib/log/log_private.h
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
+
+#ifndef LOG_PRIVATE_H
+#define LOG_PRIVATE_H
+
+typedef int (*log_print_t)(FILE *f, uint32_t level, const char *fmt, va_list
ap);
+
+__rte_format_printf(3, 0)
+int log_print(FILE *f, uint32_t level, const char *format, va_list ap);
+
+#endif /* LOG_PRIVATE_H */
--
2.43.0
[PATCH v19 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 | 10 +-
lib/log/log_internal.h | 9 ++
lib/log/log_private.h | 7 +
lib/log/log_timestamp.c | 204
lib/log/meson.build | 6 +-
lib/log/version.map | 1 +
13 files changed, 309 insertions(+), 10 deletions(-)
create mode 100644 lib/log/log_timestamp.c
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,
[PATCH v19 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 | 25 +++---
lib/log/log_private.h | 4 +
lib/log/log_stubs.c | 28 ++
lib/log/log_syslog.c | 89 +++
lib/log/meson.build | 6 ++
9 files changed, 165 insertions(+), 41 deletions(-)
create mode 100644 lib/log/log_stubs.c
create mode 100644 lib/log/log_syslog.c
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 3c15f2a560..ddb8400d73 100644
--- a/lib/log/log.c
+++ b/lib/log/log.c
@@ -12,18 +12,19 @@
#include
#include
#include
+#include
#include
#include
#include
-#include "log_internal.h
[PATCH v19 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
---
doc/guides/prog_guide/log_lib.rst | 14 ++
lib/eal/common/eal_common_options.c | 11 ++
lib/eal/common/eal_options.h| 2 +
lib/log/log.c | 5 +
lib/log/log_internal.h | 3 +
lib/log/log_journal.c | 216
lib/log/log_private.h | 4 +
lib/log/log_stubs.c | 6 +
lib/log/meson.build | 5 +
lib/log/version.map | 1 +
10 files changed, 267 insertions(+)
create mode 100644 lib/log/log_journal.c
diff --git a/doc/guides/prog_guide/log_lib.rst
b/doc/guides/prog_guide/log_lib.rst
index abaedc7212..476dedb097 100644
--- a/doc/guides/prog_guide/log_lib.rst
+++ b/doc/guides/prog_guide/log_lib.rst
@@ -100,6 +100,20 @@ option. There are three possible settings for this option:
If ``--syslog`` option is not specified, then only console (stderr) will be
used.
+Messages can be redirected to systemd journal which is an enhanced version of
syslog with the ``--log-journal`` option.
+
+There are three possible settings for this option:
+
+*auto*
+If stderr is redirected to journal by ``systemd`` service
+then use journal socket to instead of stderr for log.
+This is the default.
+
+*never*
+Do not try to use journal.
+
+*always*
+Always try to direct messages to journal socket.
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..9a82118184 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_JOURNAL, 2, NULL, OPT_LOG_JOURNAL_NUM },
{OPT_LOG_TIMESTAMP, 2, NULL, OPT_LOG_TIMESTAMP_NUM},
{OPT_TRACE, 1, NULL, OPT_TRACE_NUM},
{OPT_TRACE_DIR, 1, NULL, OPT_TRACE_DIR_NUM},
@@ -1617,6 +1618,7 @@ eal_log_level_parse(int argc, char * const argv[])
switch (opt) {
case OPT_LOG_LEVEL_NUM:
case OPT_SYSLOG_NUM:
+ case OPT_LOG_JOURNAL_NUM:
case OPT_LOG_TIMESTAMP_NUM:
if (eal_parse_common_option(opt, optarg, internal_conf)
< 0)
return -1;
@@ -1843,6 +1845,14 @@ eal_parse_common_option(int opt, const char *optarg,
return -1;
}
break;
+
+ case OPT_LOG_JOURNAL_NUM:
+ if (eal_log_journal(optarg) < 0) {
+ EAL_LOG(ERR, "invalid parameters for --"
+ OPT_LOG_JOURNAL);
+ return -1;
+ }
+ break;
#endif
case OPT_LOG_LEVEL_NUM:
@@ -2223,6 +2233,7 @@ eal_common_usage(void)
" --"OPT_PROC_TYPE" Type of this process
(primary|secondary|auto)\n"
#ifndef RTE_EXEC_ENV_WINDOWS
" --"OPT_SYSLOG"[=] Enable use of syslog\n"
+ " --"OPT_LOG_JOURNAL"[=] Enable use of systemd journal\n"
#endif
" --"OPT_LOG_LEVEL"= Set global log level\n"
" --"OPT_LOG_LEVEL"=:\n"
diff --git a/lib/eal/common/eal_options.h b/lib/eal/common/eal_options.h
index e24c9eca53..c5a1c70288 100644
--- a/lib/eal/common/eal_options.h
+++ b/lib/eal/common/eal_options.h
@@ -35,6 +35,8 @@ enum {
OPT_LCORES_NUM,
#define OPT_LOG_LEVEL "log-level"
OPT_LOG_LEVEL_NUM,
+#define OPT_LOG_JOURNAL "log-journal"
+ OPT_LOG_JOURNAL_NUM,
#define OPT_LOG_TIMESTAMP "log-timestamp"
OPT_LOG_TIMESTAMP_NUM,
#define OPT_TRACE "trace"
diff --git a/lib/log/log.c b/lib/log/log.c
index ddb8400d73..d97bde984e 100644
--- a/lib/log/log.c
+++ b/lib/log/log.c
@@ -515,6 +515,11 @@ eal_log_init(const char *id)
#else
bool is_terminal = isatty(STDERR_FILENO);
+#ifdef RTE_EXEC_ENV_LINUX
+ if (log_journal_enabled(id))
+ rte_logs.print_func = journal_print;
+ else
+#endif
if (log_syslog_enabled(is_terminal))
log_syslog_open(id, is_terminal);
else
diff --git a/lib/log/log_internal.h b/lib/log/log_internal.h
index 7c7d44eed2..82fdc21ac2 100644
--- a/lib/log/log_internal.h
+++ b/lib/log/log_internal.h
@@ -29,6 +29
[PATCH v19 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| 6 +-
lib/log/log.c | 24 +++--
lib/log/log_color.c | 160
lib/log/log_internal.h | 5 +
lib/log/log_private.h | 8 ++
lib/log/meson.build | 2 +-
lib/log/version.map | 1 +
10 files changed, 247 insertions(+), 10 deletions(-)
create mode 100644 lib/log/log_color.c
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 476dedb097..f46720fe34 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
~
@@ -115,7 +130,6 @@ There are three possible settings for this option:
*always*
Always try to direct messages to journal socket.
-
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 9a82118184..70fdf3f5a1 100644
--- a/lib/eal/common/eal_common_options.c
+++ b/lib/eal/common/eal_common_options.c
@@ -73,6 +73,7 @@ eal_long_options[] = {
{OPT_HUGE_UNLINK, 2, NULL, OPT_HUGE_UNLINK_NUM },
{OPT_IOVA_MODE, 1, NULL, OPT_IOVA_MODE_NUM},
{OPT_LCORES,1, NULL, OPT_LCORES_NUM },
+ {OPT_LOG_COLOR, 2, NULL, OPT_LOG_COLOR_NUM},
{OPT_LOG_LEVEL, 1, NULL, OPT_LOG_LEVEL_NUM},
{OPT_LOG_JOURNAL, 2, NULL, OPT_LOG_JOURNAL_NUM },
{OPT_LOG_TIMESTAMP, 2, NULL, OPT_LOG_TIMESTAMP_NUM},
@@ -1620,6 +1621,7 @@ eal_log_level_parse(int argc, char * const argv[])
case OPT_SYSLOG_NUM:
case OPT_LOG_JOURNAL_NUM:
case OPT_LOG_TIMESTAMP_NUM:
+ case OPT_LOG_COLOR_NUM:
if (eal_parse_common_option(opt, optarg, internal_conf)
< 0)
return -1;
break;
@@ -1872,6 +1874,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 --"
+
