netdev - Re: [net-next 08/15] iavf: Fix up debug print macro

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <9408eb59ecaa3e245fd71ec0211a34c3fb0e324b.camel@perches.com>
Date:   Sat, 29 Jun 2019 13:42:06 -0700
From:   Joe Perches <joe@...ches.com>
To:     Jeff Kirsher <jeffrey.t.kirsher@...el.com>, davem@...emloft.net
Cc:     netdev@...r.kernel.org, nhorman@...hat.com, sassmann@...hat.com,
        Andrew Bowers <andrewx.bowers@...el.com>
Subject: Re: [net-next 08/15] iavf: Fix up debug print macro

On Fri, 2019-06-28 at 15:49 -0700, Jeff Kirsher wrote:
> This aligns the iavf_debug() macro with the other Intel drivers.
> 
> Add the bus number, bus_id field to i40e_bus_info so output shows
> each physical port(i.e func) in following format:
>   [[[[<domain>]:]<bus>]:][<slot>][.[<func>]]
> domains are numbered from 0 to ffff), bus (0-ff), slot (0-1f) and
> function (0-7).
> 
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@...el.com>
> Tested-by: Andrew Bowers <andrewx.bowers@...el.com>
> ---
>  drivers/net/ethernet/intel/iavf/iavf_osdep.h | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/iavf/iavf_osdep.h b/drivers/net/ethernet/intel/iavf/iavf_osdep.h
> index d39684558597..a452ce90679a 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf_osdep.h
> +++ b/drivers/net/ethernet/intel/iavf/iavf_osdep.h
> @@ -44,8 +44,12 @@ struct iavf_virt_mem {
>  #define iavf_allocate_virt_mem(h, m, s) iavf_allocate_virt_mem_d(h, m, s)
>  #define iavf_free_virt_mem(h, m) iavf_free_virt_mem_d(h, m)
>  
> -#define iavf_debug(h, m, s, ...)  iavf_debug_d(h, m, s, ##__VA_ARGS__)
> -extern void iavf_debug_d(void *hw, u32 mask, char *fmt_str, ...)
> -	__printf(3, 4);
> +#define iavf_debug(h, m, s, ...)				\
> +do {								\
> +	if (((m) & (h)->debug_mask))				\
> +		pr_info("iavf %02x:%02x.%x " s,			\
> +			(h)->bus.bus_id, (h)->bus.device,	\
> +			(h)->bus.func, ##__VA_ARGS__);		\
> +} while (0)

Why not change the function to do this?

And if this is really wanted this particular way
the now unused function should be removed too.

But I suggest emitting at KERN_DEBUG and using
the more typical %pV vsprintf extension.

---

 drivers/net/ethernet/intel/iavf/iavf_main.c  | 25 ++++++++++++++-----------
 drivers/net/ethernet/intel/iavf/iavf_osdep.h |  9 ++++++---
 2 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c
index 881561b36083..8504fd71d398 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_main.c
+++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
@@ -143,25 +143,28 @@ enum iavf_status iavf_free_virt_mem_d(struct iavf_hw *hw,
 }
 
 /**
- * iavf_debug_d - OS dependent version of debug printing
+ * _iavf_debug - OS dependent version of debug printing
  * @hw:  pointer to the HW structure
  * @mask: debug level mask
- * @fmt_str: printf-type format description
+ * @fmt: printf-type format description
  **/
-void iavf_debug_d(void *hw, u32 mask, char *fmt_str, ...)
+void _iavf_debug(const struct iavf_hw *hw, u32 mask, const char *fmt, ...)
 {
-	char buf[512];
-	va_list argptr;
+	struct va_format vaf;
+	va_list args;
 
-	if (!(mask & ((struct iavf_hw *)hw)->debug_mask))
+	if (!(hw->debug_mask & mask))
 		return;
 
-	va_start(argptr, fmt_str);
-	vsnprintf(buf, sizeof(buf), fmt_str, argptr);
-	va_end(argptr);
+	va_start(args, fmt);
 
-	/* the debug string is already formatted with a newline */
-	pr_info("%s", buf);
+	vaf.fmt = fmt;
+	vaf.va = &args;
+
+	pr_debug("iavf %02x:%02x.%x %pV",
+		 hw->bus.bus_id, hw->bus.device, hw->bus.func, &vaf);
+
+	va_end(args);
 }
 
 /**
diff --git a/drivers/net/ethernet/intel/iavf/iavf_osdep.h b/drivers/net/ethernet/intel/iavf/iavf_osdep.h
index d39684558597..0e6ac7d262c8 100644
--- a/drivers/net/ethernet/intel/iavf/iavf_osdep.h
+++ b/drivers/net/ethernet/intel/iavf/iavf_osdep.h
@@ -44,8 +44,11 @@ struct iavf_virt_mem {
 #define iavf_allocate_virt_mem(h, m, s) iavf_allocate_virt_mem_d(h, m, s)
 #define iavf_free_virt_mem(h, m) iavf_free_virt_mem_d(h, m)
 
-#define iavf_debug(h, m, s, ...)  iavf_debug_d(h, m, s, ##__VA_ARGS__)
-extern void iavf_debug_d(void *hw, u32 mask, char *fmt_str, ...)
-	__printf(3, 4);
+struct iavf_hw;
+
+__printf(3, 4)
+void _iavf_debug(const struct iavf_hw *hw, u32 mask, const char *fmt, ...);
+#define iavf_debug(hw, mask, fmt, ...)					\
+	_iavf_debug(hw, mask, fmt, ##__VA_ARGS__)
 
 #endif /* _IAVF_OSDEP_H_ */