Re: [External] Re: [PATCH 2/2] net/mlx5: improve log file path

["Ming 1. Yang (NSB)" <ming.1.yang@nokia-sbell.com>]

On 2025/3/18 00:05, Bing Zhao wrote:
> Caution: This is an external email. Please be very careful when clicking links or opening attachments. See http://nok.it/nsb for additional information.
>
> Hi Ming & Stephen,
>
>> -----Original Message-----
>> From: Yang Ming <ming.1.yang@nokia-sbell.com>
>> Sent: Wednesday, March 12, 2025 10:33 AM
>> To: Stephen Hemminger <stephen@networkplumber.org>; Bing Zhao
>> <bingz@nvidia.com>
>> Cc: Dariusz Sosnowski <dsosnowski@nvidia.com>; Slava Ovsiienko
>> <viacheslavo@nvidia.com>; Ori Kam <orika@nvidia.com>; Suanming Mou
>> <suanmingm@nvidia.com>; Matan Azrad <matan@nvidia.com>; dev@dpdk.org
>> Subject: Re: [External] Re: [PATCH 2/2] net/mlx5: improve log file path
>>
>> External email: Use caution opening links or attachments
>>
>>
>> On 2025/3/10 22:59, Stephen Hemminger wrote:
>>> Caution: This is an external email. Please be very careful when clicking
>> links or opening attachments. See http://nok.it/nsb for additional
>> information.
>>> On Tue, 4 Mar 2025 06:23:06 +0000
>>> Bing Zhao <bingz@nvidia.com> wrote:
>>>
>>>> Hi Ming,
>>>>
>>>>> -----Original Message-----
>>>>> From: Yang Ming <ming.1.yang@nokia-sbell.com>
>>>>> Sent: Friday, December 13, 2024 5:25 PM
>>>>> To: Dariusz Sosnowski <dsosnowski@nvidia.com>; Slava Ovsiienko
>>>>> <viacheslavo@nvidia.com>; Bing Zhao <bingz@nvidia.com>; Ori Kam
>>>>> <orika@nvidia.com>; Suanming Mou <suanmingm@nvidia.com>; Matan Azrad
>>>>> <matan@nvidia.com>
>>>>> Cc: dev@dpdk.org; Yang Ming <ming.1.yang@nokia-sbell.com>
>>>>> Subject: [PATCH 2/2] net/mlx5: improve log file path
>>>>>
>>>>> External email: Use caution opening links or attachments
>>>>>
>>>>>
>>>>> 1. /var/log is hard code which is not a good coding style.
>>>>> 2. /var/log may be not allowed to be written via container's
>>>>> read-only mode.
>>>>>
>>>>> Signed-off-by: Yang Ming <ming.1.yang@nokia-sbell.com>
>>>>> ---
>>>>>    drivers/net/mlx5/mlx5_rxtx.c | 3 ++-
>>>>>    1 file changed, 2 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/net/mlx5/mlx5_rxtx.c
>>>>> b/drivers/net/mlx5/mlx5_rxtx.c index eadadcdffb..a0da73c9c3 100644
>>>>> --- a/drivers/net/mlx5/mlx5_rxtx.c
>>>>> +++ b/drivers/net/mlx5/mlx5_rxtx.c
>>>>> @@ -12,6 +12,7 @@
>>>>>    #include <rte_prefetch.h>
>>>>>    #include <rte_common.h>
>>>>>    #include <rte_branch_prediction.h>
>>>>> +#include <rte_eal.h>
>>>>>    #include <rte_ether.h>
>>>>>    #include <rte_cycles.h>
>>>>>    #include <rte_flow.h>
>>>>> @@ -311,7 +312,7 @@ mlx5_set_swp_types_table(void)
>>>>>           }
>>>>>    }
>>>>>
>>>>> -#define MLX5_SYSTEM_LOG_DIR "/var/log"
>>>>> +#define MLX5_SYSTEM_LOG_DIR rte_eal_get_runtime_dir()
>>>> I agree that using the fixed PATH is not a good practice. Can you
>> ensure that the runtime DIR is with RW+ permissions?
>>> Drivers doing any kind of custom logging is bad practice.
>>> This should be handled by EAL logging, not private fprintf's
>>>
>>>
>>>
>> Hi Stephen,
>>
>> Yes, I completely agree with you. The DPDK driver should utilize EAL
>> logging instead of fprintf. We have recently addressed an issue where DPDK
>> was applied in a container with a read-only file system mode. In this
>> mode, the /var/log directory is read-only. However, when DPDK is running,
>> the directory specified by rte_eal_get_runtime_dir() must be configured
>> with read-write permissions. Therefore, we have made this minor
>> improvement.
>>
>> Please note that we are not the developers of the Mellanox CX4/CX5 NIC,
>> nor are we affiliated with the manufacturer of these NICs. As such, we are
>> unable to make the improvements you described.
>>
> I tend to disagree with such comments. Such dump will be generated when some error happens. We want to save the information into some file for offline analysis. I checked the code, current RTE_LOG routines do not have a variant that supports specified output stream but not only stdout/stderr/syslog. Only global PATH can be used. (correct me if I missed something) If there is an API can specify the output stream instead of the global set one, that should satisfy the needs.
>
> Saving it into some file is a must. For example, if the customer is using a monitor with VGA display interface connected to the server directly or a BMC simulated console terminal, sometimes the previous prints will be flushed out of buffer of the screen if there are a lot of logs to be printed. There is even no scroll up/down can be used like via SSH vterm.
>
> After some internal discussion with our experts, we thought it would be better not only to change the PATH blindly.
> But we can firstly check/try to write to the default fixed PATH as today. If failed due to no permission/no space, then trying the fallback way to use the runtime_dir() instead. And then notify the user with a LOG that the file is saved into [PATH xxxx]. WDYT?
>
>> Brs,
>> Yang Ming

Hi Dariusz & Bing,

Yes, I agree that the fallback mechanism is highly suitable for this 
scenario. It not only addresses the issue but also maintains 
compatibility with the original user cases, making it a better solution. 
I will enhance this patch shortly. Your assistance in reviewing it would 
be greatly appreciated.

Brs,
Yang Ming