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]

Message-ID: <pj41zlk0wsdyy7.fsf@u68c7b5b1d2d758.ant.amazon.com>
Date:   Thu, 17 Sep 2020 21:45:36 +0300
From:   Shay Agroskin <shayagr@...zon.com>
To:     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

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

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