[2/2] net/mlx5: improve log file path

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(-)
  

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?

>  /**
>   * Dump debug information to log file.
>   *
> --
> 2.34.1
  

On 2025/3/4 14:23, 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,
>
>> -----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?
>
>>   /**
>>    * Dump debug information to log file.
>>    *
>> --
>> 2.34.1

Hi Bing,

Thanks for your comments.

Yes. Read-write (RW) permissions must be applied to this
directory because DPDK needs to write runtime information
to this directory such as config file, socket file etc.
Additionally, within the function stack, the
`eal_create_runtime_dir()` function includes the command
`mkdir(run_dir, 0700)`, indicating that the owner should
have read, write, and execute permissions.

Brs,
Yang Ming
  

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
  

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.

Brs,
Yang Ming
  

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