netdev - Re: [PATCH V1 net-next 2/8] net: ena: Add device distinct log prefix to files

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]

Date:   Thu, 17 Sep 2020 12:38:28 -0700
From:   Saeed Mahameed <saeed@...nel.org>
To:     Shay Agroskin <shayagr@...zon.com>,
        David Miller <davem@...emloft.net>
Cc:     netdev@...r.kernel.org, dwmw@...zon.com, zorik@...zon.com,
        matua@...zon.com, saeedb@...zon.com, msw@...zon.com,
        aliguori@...zon.com, nafea@...zon.com, gtzalik@...zon.com,
        netanel@...zon.com, alisaidi@...zon.com, benh@...zon.com,
        akiyano@...zon.com, sameehj@...zon.com, ndagan@...zon.com,
        amitbern@...zon.com
Subject: Re: [PATCH V1 net-next 2/8] net: ena: Add device distinct log
 prefix to files

On Thu, 2020-09-17 at 21:45 +0300, Shay Agroskin wrote:
> David Miller <davem@...emloft.net> writes:
> 
> > From: Shay Agroskin <shayagr@...zon.com>
> > Date: Sun, 13 Sep 2020 11:16:34 +0300
> > 
> > > ENA logs are adjusted to display the full ENA representation to
> > > distinct each ENA device in case of multiple interfaces.
> > > Using dev_err/warn/info function family for logging provides 
> > > uniform
> > > printing with clear distinction of the driver and device.
> > > 
> > > This patch changes all printing in ena_com files to use dev_* 
> > > logging
> > > messages. It also adds some log messages to make driver 
> > > debugging
> > > easier.
> > > 
> > > Signed-off-by: Amit Bernstein <amitbern@...zon.com>
> > > Signed-off-by: Shay Agroskin <shayagr@...zon.com>
> > 
> > This device prefix is so much less useful than printing the 
> > actual
> > networking adapter that the ena_com operations are for.
> > 
> > So if you are going to do this, go all the way and pass the 
> > ena_adapter
> > or the netdev down into these ena_com routines so that you can 
> > use
> > the netdev_*() message helpers.
> > 
> > Thank you.
> 
> Hi David, I researched the possibility to use netdev_* functions 
> in this patch. Currently our driver initializes the net_device 
> only after calling some functions in ena_com files.
> Although netdev_* log family functions can be used before 
> allocating a net_device struct, the print it produces in such a 
> case is less informative than the dev_* log print (which at least 
> specifies what pcie device made the print).
> 
> I would rather change the allocation order for the net_device 
> struct in our driver, so that when calling ena_com device it would 
> always be allocated (and this way all ena_com prints could be 
> transformed into netdev/netif_* log family).
> This change seems doable, but requires us to do some internal 
> testing before sending it. I could remove this whole patch, but 
> then our driver would be left with pr_err() functions in it which 
> is even less informative than dev_err().
> 

allocated but unregistered netdevices also do not help much as the name
of the netdev is not assigned yet.

why don't use dev_info(pci_dev) macors  for low level functions when
netdev is not available or not allocated yet.

> Can we go through with this patch, and send a future patch which 
> changes all dev_* functions into netif/netdev_* along with the 
> change in the allocation order of the net_device struct ? I know 
> it sounds like a procrastination attempt, but I would really 
> prefer not to drop the patch and leave the driver with pr_* log 
> prints.
> 
> Thanks,
> Shay