Re: Firmware debugging patches?

Re: Firmware debugging patches?

[Ben Greear]

[Good stuff snipped, adding linux-wireless as this is a more
general issue if we are going to consider general framework]


Maybe we should start with goals before getting to implementation
details.  Here's my wish list that is ath10k specific, but probably
similar to other firmware users:

1)  We need the firmware crash text currently printed to
/var/log/messages.

2)  It would be nice to get the firmware RAM and stack dumps at time of
crash to debug more interesting crashes.

3)  It would be nice to know about firmware debug messages for
the period of time directly before the crash (maybe 2-5 minutes?)

4)  It would be nice to have this interleaved with kernel, supplicant,
and related logs.


We need a solution for different types of users.  I suspect the number
of crashes seen in the wild will be more for users nearer the top
of this list.

a) Normal Fedora/Ubuntu/etc default-installed distribution user
with ath10k NIC has wifi issues, firmware crashes, they don't
really know what firmware means or that it crashed, but some automated crash-log
tool notices and gathers debug info for automated bug reporting.

b) Slightly more advanced user actually notices the problem at coffee shop
earlier today, posts about it when they get home, and we ask for
debug info.

c) Experienced and determined user has similar issues, but is able to
reproduce the problem and/or turn on more advanced debugging efforts.

d)  Even more determined user that can and will recompile kernels and/or
try patches.


Anything that has to be enabled before-hand will not help a) and b) above.

If support is not compiled into default kernels, c) will not help you either.

If it is difficult or requires acquiring cutting edge tools not in their
distribution by default, many of c) and some of d) will just ignore the problem or use
different hardware.

If we are storing crashes for something like ethtool to report, we need
RAM and/or disk storage so the firmware RAM dumps and such can be stored until
the user and/or automated tools ask for them.  We need some way to automatically
clean up old crashes so disk/ram is not overly utilized.  For APs,
they are low on both RAM and 'disk', so storing crash logs for any
length of time may be problematic.


I am indeed biased towards using printk, but lets agree on some goals and
see if we can find a set of tools and methods that meets them.


Thanks,
Ben


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: Firmware debugging patches?

[Emmanuel Grumbach]

> [Good stuff snipped, adding linux-wireless as this is a more
> general issue if we are going to consider general framework]
> 
> 
> Maybe we should start with goals before getting to implementation
> details.  Here's my wish list that is ath10k specific, but probably
> similar to other firmware users:
> 
> 1)  We need the firmware crash text currently printed to
> /var/log/messages.
> 
> 2)  It would be nice to get the firmware RAM and stack dumps at time of
> crash to debug more interesting crashes.

Right - but typically you'll have closed source / IP / whatever there..

> 
> 3)  It would be nice to know about firmware debug messages for
> the period of time directly before the crash (maybe 2-5 minutes?)
> 
> 4)  It would be nice to have this interleaved with kernel, supplicant,
> and related logs.
> 
> 
> We need a solution for different types of users.  I suspect the number
> of crashes seen in the wild will be more for users nearer the top
> of this list.
> 
> a) Normal Fedora/Ubuntu/etc default-installed distribution user
> with ath10k NIC has wifi issues, firmware crashes, they don't
> really know what firmware means or that it crashed, but some automated crash-log
> tool notices and gathers debug info for automated bug reporting.

I am working on that for our firmware. I recently added such capability relying on udev to notify the userspace that something bad happens. I gather all the data and prepare a binary file that is sent through debugfs (pulled by a script triggered by udev). I remember the first crash only.

> 
> b) Slightly more advanced user actually notices the problem at coffee shop
> earlier today, posts about it when they get home, and we ask for
> debug info.
> 
> c) Experienced and determined user has similar issues, but is able to
> reproduce the problem and/or turn on more advanced debugging efforts.
> 
> d)  Even more determined user that can and will recompile kernels and/or
> try patches.
> 
> 
> Anything that has to be enabled before-hand will not help a) and b) above.
> 
> If support is not compiled into default kernels, c) will not help you either.
> 
> If it is difficult or requires acquiring cutting edge tools not in their
> distribution by default, many of c) and some of d) will just ignore the problem or use
> different hardware.
> 
> If we are storing crashes for something like ethtool to report, we need
> RAM and/or disk storage so the firmware RAM dumps and such can be stored until
> the user and/or automated tools ask for them.  We need some way to automatically
> clean up old crashes so disk/ram is not overly utilized.  For APs,
> they are low on both RAM and 'disk', so storing crash logs for any
> length of time may be problematic.
 
I did something simpler - but it works. I don't really know the ethtool infrastructure though.

Re: Firmware debugging patches?

[Ben Greear]

On 06/02/2014 11:46 AM, Emmanuel Grumbach wrote:
>> [Good stuff snipped, adding linux-wireless as this is a more
>> general issue if we are going to consider general framework]
>>
>>
>> Maybe we should start with goals before getting to implementation
>> details.  Here's my wish list that is ath10k specific, but probably
>> similar to other firmware users:
>>
>> 1)  We need the firmware crash text currently printed to
>> /var/log/messages.
>>
>> 2)  It would be nice to get the firmware RAM and stack dumps at time of
>> crash to debug more interesting crashes.
> 
> Right - but typically you'll have closed source / IP / whatever there..

I mean that we need the raw data (ie, binary dump, something printed
in ascii-hex, etc).  I understand it will take proprietary tools to
decode it to something a developer can actually debug.

>> 3)  It would be nice to know about firmware debug messages for
>> the period of time directly before the crash (maybe 2-5 minutes?)
>>
>> 4)  It would be nice to have this interleaved with kernel, supplicant,
>> and related logs.
>>
>>
>> We need a solution for different types of users.  I suspect the number
>> of crashes seen in the wild will be more for users nearer the top
>> of this list.
>>
>> a) Normal Fedora/Ubuntu/etc default-installed distribution user
>> with ath10k NIC has wifi issues, firmware crashes, they don't
>> really know what firmware means or that it crashed, but some automated crash-log
>> tool notices and gathers debug info for automated bug reporting.
> 
> I am working on that for our firmware. I recently added such capability relying on udev to notify the userspace that something bad happens. I gather all the data and prepare a binary file that is sent through debugfs (pulled by a script triggered by udev). I remember the first crash only.

How is this binary blob encoded?

At least for drivers that can recover from firmware crashes, I think
we should continue to report crashes, not just the first.

Maybe could store another one after initial crash has been read
and 1 minute has elapsed, or if initial crash has not been read
in 1 day, or something like that.

Also, if we use debugfs then we require upstream kernels to have this
compiled in and mounted if we want to handle this class of user.

I am not sure this is really the case currently.  But, once the
blob is generated and stored in RAM, it would be easily enough to
add ethtool option to dump it w/out debugfs support.  This will
still not really address my concerns because it may take a year
or two for the latest ethtool binary to make it to normal-ish users.

>>
>> b) Slightly more advanced user actually notices the problem at coffee shop
>> earlier today, posts about it when they get home, and we ask for
>> debug info.
>>
>> c) Experienced and determined user has similar issues, but is able to
>> reproduce the problem and/or turn on more advanced debugging efforts.
>>
>> d)  Even more determined user that can and will recompile kernels and/or
>> try patches.
>>
>>
>> Anything that has to be enabled before-hand will not help a) and b) above.
>>
>> If support is not compiled into default kernels, c) will not help you either.
>>
>> If it is difficult or requires acquiring cutting edge tools not in their
>> distribution by default, many of c) and some of d) will just ignore the problem or use
>> different hardware.
>>
>> If we are storing crashes for something like ethtool to report, we need
>> RAM and/or disk storage so the firmware RAM dumps and such can be stored until
>> the user and/or automated tools ask for them.  We need some way to automatically
>> clean up old crashes so disk/ram is not overly utilized.  For APs,
>> they are low on both RAM and 'disk', so storing crash logs for any
>> length of time may be problematic.
>  
> I did something simpler - but it works. I don't really know the ethtool infrastructure though.

I think ethtool would not be overly hard to implement...basic framework is already
in the wifi stack.

Thanks,
Ben


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: Firmware debugging patches?

[Emmanuel Grumbach]

Emmanuel Grumbach
egrumbach@gmail.com


On Mon, Jun 2, 2014 at 9:58 PM, Ben Greear <greearb@candelatech.com> wrote:
> On 06/02/2014 11:46 AM, Emmanuel Grumbach wrote:
>>> [Good stuff snipped, adding linux-wireless as this is a more
>>> general issue if we are going to consider general framework]
>>>
>>>
>>> Maybe we should start with goals before getting to implementation
>>> details.  Here's my wish list that is ath10k specific, but probably
>>> similar to other firmware users:
>>>
>>> 1)  We need the firmware crash text currently printed to
>>> /var/log/messages.
>>>
>>> 2)  It would be nice to get the firmware RAM and stack dumps at time of
>>> crash to debug more interesting crashes.
>>
>> Right - but typically you'll have closed source / IP / whatever there..
>
> I mean that we need the raw data (ie, binary dump, something printed
> in ascii-hex, etc).  I understand it will take proprietary tools to
> decode it to something a developer can actually debug.
>
>>> 3)  It would be nice to know about firmware debug messages for
>>> the period of time directly before the crash (maybe 2-5 minutes?)
>>>
>>> 4)  It would be nice to have this interleaved with kernel, supplicant,
>>> and related logs.
>>>
>>>
>>> We need a solution for different types of users.  I suspect the number
>>> of crashes seen in the wild will be more for users nearer the top
>>> of this list.
>>>
>>> a) Normal Fedora/Ubuntu/etc default-installed distribution user
>>> with ath10k NIC has wifi issues, firmware crashes, they don't
>>> really know what firmware means or that it crashed, but some automated crash-log
>>> tool notices and gathers debug info for automated bug reporting.
>>
>> I am working on that for our firmware. I recently added such capability relying on udev to notify the userspace that something bad happens. I gather all the data and prepare a binary file that is sent through debugfs (pulled by a script triggered by udev). I remember the first crash only.
>
> How is this binary blob encoded?

Different TLV based binary blobs concatenated. The actual encoding of
each of them is another story.

>
> At least for drivers that can recover from firmware crashes, I think
> we should continue to report crashes, not just the first.
>

I remember the first until udev kicks the script that will empty the
buffer. Then I take the second crash's log.

> Maybe could store another one after initial crash has been read
> and 1 minute has elapsed, or if initial crash has not been read
> in 1 day, or something like that.
>
> Also, if we use debugfs then we require upstream kernels to have this
> compiled in and mounted if we want to handle this class of user.

Agreed. I rely on debugfs. But this is "just" the way to reach the filesystem.
Give me another way and I am fine with it.
FWIW Ubuntu which is not exactly the distribution of the super
advanced users has it mounted by default.

>
> I am not sure this is really the case currently.  But, once the
> blob is generated and stored in RAM, it would be easily enough to
> add ethtool option to dump it w/out debugfs support.  This will
> still not really address my concerns because it may take a year
> or two for the latest ethtool binary to make it to normal-ish users.

I understand.

>
>>>
>>> b) Slightly more advanced user actually notices the problem at coffee shop
>>> earlier today, posts about it when they get home, and we ask for
>>> debug info.
>>>
>>> c) Experienced and determined user has similar issues, but is able to
>>> reproduce the problem and/or turn on more advanced debugging efforts.
>>>
>>> d)  Even more determined user that can and will recompile kernels and/or
>>> try patches.
>>>
>>>
>>> Anything that has to be enabled before-hand will not help a) and b) above.
>>>
>>> If support is not compiled into default kernels, c) will not help you either.
>>>
>>> If it is difficult or requires acquiring cutting edge tools not in their
>>> distribution by default, many of c) and some of d) will just ignore the problem or use
>>> different hardware.
>>>
>>> If we are storing crashes for something like ethtool to report, we need
>>> RAM and/or disk storage so the firmware RAM dumps and such can be stored until
>>> the user and/or automated tools ask for them.  We need some way to automatically
>>> clean up old crashes so disk/ram is not overly utilized.  For APs,
>>> they are low on both RAM and 'disk', so storing crash logs for any
>>> length of time may be problematic.
>>
>> I did something simpler - but it works. I don't really know the ethtool infrastructure though.
>
> I think ethtool would not be overly hard to implement...basic framework is already
> in the wifi stack.
>
> Thanks,
> Ben
>
>
> --
> Ben Greear <greearb@candelatech.com>
> Candela Technologies Inc  http://www.candelatech.com
>

Re: Firmware debugging patches?

[Ben Greear]

On 06/02/2014 12:29 PM, Emmanuel Grumbach wrote:

>>>> a) Normal Fedora/Ubuntu/etc default-installed distribution user
>>>> with ath10k NIC has wifi issues, firmware crashes, they don't
>>>> really know what firmware means or that it crashed, but some automated crash-log
>>>> tool notices and gathers debug info for automated bug reporting.
>>>
>>> I am working on that for our firmware. I recently added such capability relying on udev to notify the userspace that something bad happens. I gather all the data and prepare a binary file that is sent through debugfs (pulled by a script triggered by udev). I remember the first crash only.
>>
>> How is this binary blob encoded?
> 
> Different TLV based binary blobs concatenated. The actual encoding of
> each of them is another story.

Should we try to make the 'Type' in TLV be consistent as convenient
across different drivers?  That might someday help auto-reporting tools?

Do you have a link to your patch that defines the types you used?

>> At least for drivers that can recover from firmware crashes, I think
>> we should continue to report crashes, not just the first.
>>
> 
> I remember the first until udev kicks the script that will empty the
> buffer. Then I take the second crash's log.

Sounds good enough to me.

>> Maybe could store another one after initial crash has been read
>> and 1 minute has elapsed, or if initial crash has not been read
>> in 1 day, or something like that.
>>
>> Also, if we use debugfs then we require upstream kernels to have this
>> compiled in and mounted if we want to handle this class of user.
> 
> Agreed. I rely on debugfs. But this is "just" the way to reach the filesystem.
> Give me another way and I am fine with it.
> FWIW Ubuntu which is not exactly the distribution of the super
> advanced users has it mounted by default.

That sounds promising...Looks like Fedora 20 does as well, so maybe
debugfs will be good enough for crash dumps.


It does not resolve my interest in firmware logs interleaved with
kernel logs and possibly supplicant, however.

Looks like trace-cmds could do that, but it will not be
running for normal users when they experience crashes.

Any suggestions other than printk for this feature?

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: Firmware debugging patches?

[Emmanuel Grumbach]

>
>>>>> a) Normal Fedora/Ubuntu/etc default-installed distribution user
>>>>> with ath10k NIC has wifi issues, firmware crashes, they don't
>>>>> really know what firmware means or that it crashed, but some automated crash-log
>>>>> tool notices and gathers debug info for automated bug reporting.
>>>>
>>>> I am working on that for our firmware. I recently added such capability relying on udev to notify the userspace that something bad happens. I gather all the data and prepare a binary file that is sent through debugfs (pulled by a script triggered by udev). I remember the first crash only.
>>>
>>> How is this binary blob encoded?
>>
>> Different TLV based binary blobs concatenated. The actual encoding of
>> each of them is another story.
>
> Should we try to make the 'Type' in TLV be consistent as convenient
> across different drivers?  That might someday help auto-reporting tools?
>
> Do you have a link to your patch that defines the types you used?
>

Take a look at 1bd3cbc1a0e9ed977a6bd470c5bc7bd36fd87e26.
But I am adding more and more content to this file.

>>> At least for drivers that can recover from firmware crashes, I think
>>> we should continue to report crashes, not just the first.
>>>
>>
>> I remember the first until udev kicks the script that will empty the
>> buffer. Then I take the second crash's log.
>
> Sounds good enough to me.
>
>>> Maybe could store another one after initial crash has been read
>>> and 1 minute has elapsed, or if initial crash has not been read
>>> in 1 day, or something like that.
>>>
>>> Also, if we use debugfs then we require upstream kernels to have this
>>> compiled in and mounted if we want to handle this class of user.
>>
>> Agreed. I rely on debugfs. But this is "just" the way to reach the filesystem.
>> Give me another way and I am fine with it.
>> FWIW Ubuntu which is not exactly the distribution of the super
>> advanced users has it mounted by default.
>
> That sounds promising...Looks like Fedora 20 does as well, so maybe
> debugfs will be good enough for crash dumps.
>
>
> It does not resolve my interest in firmware logs interleaved with
> kernel logs and possibly supplicant, however.
>
> Looks like trace-cmds could do that, but it will not be
> running for normal users when they experience crashes.
>
> Any suggestions other than printk for this feature?

I seems you found a way already :)

>
> Thanks,
> Ben
>
> --
> Ben Greear <greearb@candelatech.com>
> Candela Technologies Inc  http://www.candelatech.com
>

Re: Firmware debugging patches?

[Ben Greear]

On 06/04/2014 12:23 PM, Emmanuel Grumbach wrote:

>> Do you have a link to your patch that defines the types you used?
>>
> 
> Take a look at 1bd3cbc1a0e9ed977a6bd470c5bc7bd36fd87e26.
> But I am adding more and more content to this file.

Yeah, after implementing it in ath10k it seems there would
be very little over-lap, so probably each driver should just
do their own.

We can keep the debugfs file names the same, and the udev
event types the same, which should help user-space a bit.


>> It does not resolve my interest in firmware logs interleaved with
>> kernel logs and possibly supplicant, however.
>>
>> Looks like trace-cmds could do that, but it will not be
>> running for normal users when they experience crashes.
>>
>> Any suggestions other than printk for this feature?
> 
> I seems you found a way already :)

I can get firmware logs, but they are not interleaved.
Probably good enough for now..and we can always add
printk or similar in the future if we all decide it's
useful.

Being able to look at your iwlwifi work made my patches lot easier,
so thanks for taking the initiative!

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: Firmware debugging patches?

[Kalle Valo]

Ben Greear <greearb@candelatech.com> writes:

> On 06/04/2014 12:23 PM, Emmanuel Grumbach wrote:
>
>>> Do you have a link to your patch that defines the types you used?
>>>
>> 
>> Take a look at 1bd3cbc1a0e9ed977a6bd470c5bc7bd36fd87e26.
>> But I am adding more and more content to this file.
>
> Yeah, after implementing it in ath10k it seems there would
> be very little over-lap, so probably each driver should just
> do their own.
>
> We can keep the debugfs file names the same, and the udev
> event types the same, which should help user-space a bit.

In the future I would actually want to take this a step further and put
all that to cfg80211. That way the driver would only need to call
cfg80211_firmware_crashed(wiphy, dump) and cfg80211 will handle the
rest. This would make sure that all drivers use the exactly same user
space interfaces and hopefully also motivate other drivers to use it.

-- 
Kalle Valo

Re: Firmware debugging patches?

[Ben Greear]

On 06/05/2014 04:10 AM, Kalle Valo wrote:
> Ben Greear <greearb@candelatech.com> writes:
> 
>> On 06/04/2014 12:23 PM, Emmanuel Grumbach wrote:
>>
>>>> Do you have a link to your patch that defines the types you used?
>>>>
>>>
>>> Take a look at 1bd3cbc1a0e9ed977a6bd470c5bc7bd36fd87e26.
>>> But I am adding more and more content to this file.
>>
>> Yeah, after implementing it in ath10k it seems there would
>> be very little over-lap, so probably each driver should just
>> do their own.
>>
>> We can keep the debugfs file names the same, and the udev
>> event types the same, which should help user-space a bit.
> 
> In the future I would actually want to take this a step further and put
> all that to cfg80211. That way the driver would only need to call
> cfg80211_firmware_crashed(wiphy, dump) and cfg80211 will handle the
> rest. This would make sure that all drivers use the exactly same user
> space interfaces and hopefully also motivate other drivers to use it.

Sounds like a good goal, but let's get the drivers well implemented first
while we have flexibility to change the API.  When things are solid, can
move some common functionality upwards in the stack.

For instance, the ath10k can get wedged where even cold resets fail
over and over...I'm thinking that could have it's own uevent so
user-space can know we need a reboot to recover the NIC...

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: Firmware debugging patches?

[Kalle Valo]

Ben Greear <greearb@candelatech.com> writes:

> On 06/02/2014 12:29 PM, Emmanuel Grumbach wrote:
>
>> Different TLV based binary blobs concatenated. The actual encoding of
>> each of them is another story.
>
> Should we try to make the 'Type' in TLV be consistent as convenient
> across different drivers?  That might someday help auto-reporting
> tools?

I don't see the need for that as all firmwares are so different anyway.
As long as there's some magic string to recognize/verify for which
firmware it is we should be ok.

> It does not resolve my interest in firmware logs interleaved with
> kernel logs and possibly supplicant, however.
>
> Looks like trace-cmds could do that, but it will not be
> running for normal users when they experience crashes.
>
> Any suggestions other than printk for this feature?

Why do you want to put the crash dump in kernel log, can you describe
your "use case" here? For me it would be enough to have a UUID for each
crash dump and then have the driver print that to kernel log:

ath10k: firmware crashed (uuid 1234567890-4321)

And then you just need to find the correct dump from the file system and
start debugging. Would that be enough for you?

-- 
Kalle Valo

Re: Firmware debugging patches?

[Ben Greear]

On 06/05/2014 04:06 AM, Kalle Valo wrote:
> Ben Greear <greearb@candelatech.com> writes:
> 
>> On 06/02/2014 12:29 PM, Emmanuel Grumbach wrote:
>>
>>> Different TLV based binary blobs concatenated. The actual encoding of
>>> each of them is another story.
>>
>> Should we try to make the 'Type' in TLV be consistent as convenient
>> across different drivers?  That might someday help auto-reporting
>> tools?
> 
> I don't see the need for that as all firmwares are so different anyway.
> As long as there's some magic string to recognize/verify for which
> firmware it is we should be ok.
> 
>> It does not resolve my interest in firmware logs interleaved with
>> kernel logs and possibly supplicant, however.
>>
>> Looks like trace-cmds could do that, but it will not be
>> running for normal users when they experience crashes.
>>
>> Any suggestions other than printk for this feature?
> 
> Why do you want to put the crash dump in kernel log, can you describe
> your "use case" here? For me it would be enough to have a UUID for each
> crash dump and then have the driver print that to kernel log:
> 
> ath10k: firmware crashed (uuid 1234567890-4321)
> 
> And then you just need to find the correct dump from the file system and
> start debugging. Would that be enough for you?

Not all systems will have fancy user-space able to deal with this.

At the very least, please leave in the current firmware crash
dump text.  That is enough to get a backtrace out of the firmware
most of the time, and almost every system that has any ability
to be debugged at all will be able to capture that text from
a kernel log file.

I like the printk for the same reason you like the trace:  It shows
things interleaved with other messages, and it is 'always on' unlike
trace, which has to be manually enabled and will not be running by default
on user's systems.

That said, I think there is no need to argue further on this.  Let's
get the debugfs dump stuff working, and maybe it will be good
enough.  It will certainly be better that what is currently
upstream.

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: Firmware debugging patches?

[Kalle Valo]

Ben Greear <greearb@candelatech.com> writes:

>> Why do you want to put the crash dump in kernel log, can you describe
>> your "use case" here? For me it would be enough to have a UUID for each
>> crash dump and then have the driver print that to kernel log:
>> 
>> ath10k: firmware crashed (uuid 1234567890-4321)
>> 
>> And then you just need to find the correct dump from the file system and
>> start debugging. Would that be enough for you?
>
> Not all systems will have fancy user-space able to deal with this.
>
> At the very least, please leave in the current firmware crash
> dump text.

I'm not removing anything. That was just an example how we can identify
crashes.

-- 
Kalle Valo

Re: Firmware debugging patches?

[Ben Greear]

On 06/05/2014 11:51 PM, Kalle Valo wrote:
> Ben Greear <greearb@candelatech.com> writes:
> 
>>> Why do you want to put the crash dump in kernel log, can you describe
>>> your "use case" here? For me it would be enough to have a UUID for each
>>> crash dump and then have the driver print that to kernel log:
>>>
>>> ath10k: firmware crashed (uuid 1234567890-4321)
>>>
>>> And then you just need to find the correct dump from the file system and
>>> start debugging. Would that be enough for you?
>>
>> Not all systems will have fancy user-space able to deal with this.
>>
>> At the very least, please leave in the current firmware crash
>> dump text.
> 
> I'm not removing anything. That was just an example how we can identify
> crashes.

Perhaps the time-stamp is good enough?  I don't see a need for
a uuid, but perhaps I am missing something?

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: Firmware debugging patches?

[Kalle Valo]

Ben Greear <greearb@candelatech.com> writes:

> On 06/05/2014 11:51 PM, Kalle Valo wrote:
>> Ben Greear <greearb@candelatech.com> writes:
>> 
>>>> Why do you want to put the crash dump in kernel log, can you describe
>>>> your "use case" here? For me it would be enough to have a UUID for each
>>>> crash dump and then have the driver print that to kernel log:
>>>>
>>>> ath10k: firmware crashed (uuid 1234567890-4321)
>>>>
>>>> And then you just need to find the correct dump from the file system and
>>>> start debugging. Would that be enough for you?
>>>
>>> Not all systems will have fancy user-space able to deal with this.
>>>
>>> At the very least, please leave in the current firmware crash
>>> dump text.
>> 
>> I'm not removing anything. That was just an example how we can identify
>> crashes.
>
> Perhaps the time-stamp is good enough?  I don't see a need for
> a uuid, but perhaps I am missing something?

UUID is supposed to be unique. If we use walltime there's no guarantee
that the clock is correct and if we use local_clock() (my preference) it
will be reset after every boot.

I just think using something like UUID is more robust. Especially if one
implements an automatic crash dump collector from thousands of deployed
APs, having an UUID makes it a lot easier to manage.

-- 
Kalle Valo

Re: Firmware debugging patches?

[Ben Greear]

On 06/07/2014 06:03 AM, Kalle Valo wrote:
> Ben Greear <greearb@candelatech.com> writes:
>
>> On 06/05/2014 11:51 PM, Kalle Valo wrote:
>>> Ben Greear <greearb@candelatech.com> writes:
>>>
>>>>> Why do you want to put the crash dump in kernel log, can you describe
>>>>> your "use case" here? For me it would be enough to have a UUID for each
>>>>> crash dump and then have the driver print that to kernel log:
>>>>>
>>>>> ath10k: firmware crashed (uuid 1234567890-4321)
>>>>>
>>>>> And then you just need to find the correct dump from the file system and
>>>>> start debugging. Would that be enough for you?
>>>>
>>>> Not all systems will have fancy user-space able to deal with this.
>>>>
>>>> At the very least, please leave in the current firmware crash
>>>> dump text.
>>>
>>> I'm not removing anything. That was just an example how we can identify
>>> crashes.
>>
>> Perhaps the time-stamp is good enough?  I don't see a need for
>> a uuid, but perhaps I am missing something?
>
> UUID is supposed to be unique. If we use walltime there's no guarantee
> that the clock is correct and if we use local_clock() (my preference) it
> will be reset after every boot.
>
> I just think using something like UUID is more robust. Especially if one
> implements an automatic crash dump collector from thousands of deployed
> APs, having an UUID makes it a lot easier to manage.

I can add since-boot timestamp as well.  Time-since-boot is less likely
to be unique than wall-time, and for systems that do have proper wall-time
clock configured, I think that provides some useful info.  (Would be interesting
if all APs in a stadium crashed near the same time, for instance.)

I was thinking we should not add a MAC to the dump, for privacy concerns,
but whatever user-space tools gather the dump could add MAC if user perfers.

With time-of-day, time-since-boot, and MAC, each dump should be unique.

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: Firmware debugging patches?

[Kalle Valo]

Ben Greear <greearb@candelatech.com> writes:

>>> Perhaps the time-stamp is good enough?  I don't see a need for
>>> a uuid, but perhaps I am missing something?
>>
>> UUID is supposed to be unique. If we use walltime there's no guarantee
>> that the clock is correct and if we use local_clock() (my preference) it
>> will be reset after every boot.
>>
>> I just think using something like UUID is more robust. Especially if one
>> implements an automatic crash dump collector from thousands of deployed
>> APs, having an UUID makes it a lot easier to manage.
>
> I can add since-boot timestamp as well.  Time-since-boot is less likely
> to be unique than wall-time, and for systems that do have proper wall-time
> clock configured, I think that provides some useful info.  (Would be interesting
> if all APs in a stadium crashed near the same time, for instance.)
>
> I was thinking we should not add a MAC to the dump, for privacy concerns,
> but whatever user-space tools gather the dump could add MAC if user perfers.

The MAC addresses can be extracted from the target memory anyway so I
don't see harm from including that in the dump. Is it even possible to
address all privacy issues when dealing with firmware dumps?

> With time-of-day, time-since-boot, and MAC, each dump should be unique.

But I would like to easily match from kernel log that the crash dump
matches with log. uuid would provide that in a simple way (check that
the uuid in the log matches with the uuid in the dump). What's so bad
from using uuid?

-- 
Kalle Valo

Re: Firmware debugging patches?

[Johannes Berg]

On Sun, 2014-06-08 at 11:35 +0300, Kalle Valo wrote:

> > I was thinking we should not add a MAC to the dump, for privacy concerns,
> > but whatever user-space tools gather the dump could add MAC if user perfers.
> 
> The MAC addresses can be extracted from the target memory anyway so I
> don't see harm from including that in the dump. Is it even possible to
> address all privacy issues when dealing with firmware dumps?

We had something of a discussion about this - it's usually not just
BSSIDs/MAC addresses, but also encryption keys (say WEP keys, or at
least temporal keys, sometimes more), SSIDs, etc. In short, there's a
whole bunch of private data that can be included in such dumps.

However, we haven't really found a good solution. Popping up big scary
warnings to the user is unlikely to be productive, trying to keep the
data private and only accessible to (the right) developers is not going
to work ... encryption might work, but who manages that?

Certainly the data shouldn't be attached to a public bug tracker
automatically...

johannes

Re: Firmware debugging patches?

[Emmanuel Grumbach]

On 06/08/2014 12:13 PM, Johannes Berg wrote:
> On Sun, 2014-06-08 at 11:35 +0300, Kalle Valo wrote:
> 
>>> I was thinking we should not add a MAC to the dump, for privacy concerns,
>>> but whatever user-space tools gather the dump could add MAC if user perfers.
>>
>> The MAC addresses can be extracted from the target memory anyway so I
>> don't see harm from including that in the dump. Is it even possible to
>> address all privacy issues when dealing with firmware dumps?
> 
> We had something of a discussion about this - it's usually not just
> BSSIDs/MAC addresses, but also encryption keys (say WEP keys, or at
> least temporal keys, sometimes more), SSIDs, etc. In short, there's a
> whole bunch of private data that can be included in such dumps.
> 
> However, we haven't really found a good solution. Popping up big scary
> warnings to the user is unlikely to be productive, trying to keep the
> data private and only accessible to (the right) developers is not going
> to work ... encryption might work, but who manages that?
> 

I disagree here. We found something that met the requirements of the organization we work for.
Note that each company has its requirements when it comes to this kind of things.
If we wanted to come up with a community statement of user privacy, the first step would be to stop printing the BSSID of the AP we associate to the kernel which is typically captured automatically by bug trackers...
So we need to separate between different things: we have the kernel / driver on one side, and we have the distribution on the other. We don't ship kernels to user. The distribution does.
The way I see it, we need to keep the distribution informed on what information this dump might include and how easy it is parse / hack. If the distribution wants to enable it or not is another problem.
Loading iwlwifi with debug=0xffffffff or even tracing can record pretty much everything. Should we stop allowing usage of these?
This doesn't make sense to me.

> Certainly the data shouldn't be attached to a public bug tracker
> automatically...
> 

Sure - but again, I don't think that "good" or "bad" will help here. We need to come up with a real statement of what we information might be enclosed and guarantee that nothing else is included in the data. Then we can make a declaration somewhere.
Note that the distribution is shipping the whole package and that the distribution typically have a "legal department" that handles this kind of things - including a privacy statement that is agreed by the user when he installs the distribution.

Re: Firmware debugging patches?

[Ben Greear]

On 06/08/2014 01:35 AM, Kalle Valo wrote:
> Ben Greear <greearb@candelatech.com> writes:
>
>>>> Perhaps the time-stamp is good enough?  I don't see a need for
>>>> a uuid, but perhaps I am missing something?
>>>
>>> UUID is supposed to be unique. If we use walltime there's no guarantee
>>> that the clock is correct and if we use local_clock() (my preference) it
>>> will be reset after every boot.
>>>
>>> I just think using something like UUID is more robust. Especially if one
>>> implements an automatic crash dump collector from thousands of deployed
>>> APs, having an UUID makes it a lot easier to manage.
>>
>> I can add since-boot timestamp as well.  Time-since-boot is less likely
>> to be unique than wall-time, and for systems that do have proper wall-time
>> clock configured, I think that provides some useful info.  (Would be interesting
>> if all APs in a stadium crashed near the same time, for instance.)
>>
>> I was thinking we should not add a MAC to the dump, for privacy concerns,
>> but whatever user-space tools gather the dump could add MAC if user perfers.
>
> The MAC addresses can be extracted from the target memory anyway so I
> don't see harm from including that in the dump. Is it even possible to
> address all privacy issues when dealing with firmware dumps?

It would be more difficult to find it in debug-log messages (and in most cases,
it is probably not the debug-log at all).  Probably not in the stack dump
most of the time either.

I'm not sure it will be in the BSS regions or not.  I have no plans to dump the
non-bss RAM on the firmware because I have no tools that can decode it anyway,
so probably the patches I've posted so far are relatively OK as far as privacy
is concerned.  Someone who is very paranoid should not be using this feature
anyway.


>> With time-of-day, time-since-boot, and MAC, each dump should be unique.
>
> But I would like to easily match from kernel log that the crash dump
> matches with log. uuid would provide that in a simple way (check that
> the uuid in the log matches with the uuid in the dump). What's so bad
> from using uuid?

Just tell me how you want me to create the uuid and I'll do so.

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: Firmware debugging patches?

[Kalle Valo]

Ben Greear <greearb@candelatech.com> writes:

>>> With time-of-day, time-since-boot, and MAC, each dump should be unique.
>>
>> But I would like to easily match from kernel log that the crash dump
>> matches with log. uuid would provide that in a simple way (check that
>> the uuid in the log matches with the uuid in the dump). What's so bad
>> from using uuid?
>
> Just tell me how you want me to create the uuid and I'll do so.

As kbuild found some compilation problems, I think it would be easier if
I make the final changes to your patches and send that for review. Would
that be ok for you?

-- 
Kalle Valo

Re: Firmware debugging patches?

[Ben Greear]

On 06/09/2014 01:17 AM, Kalle Valo wrote:
> Ben Greear <greearb@candelatech.com> writes:
>
>>>> With time-of-day, time-since-boot, and MAC, each dump should be unique.
>>>
>>> But I would like to easily match from kernel log that the crash dump
>>> matches with log. uuid would provide that in a simple way (check that
>>> the uuid in the log matches with the uuid in the dump). What's so bad
>>> from using uuid?
>>
>> Just tell me how you want me to create the uuid and I'll do so.
>
> As kbuild found some compilation problems, I think it would be easier if
> I make the final changes to your patches and send that for review. Would
> that be ok for you?

Yes, that would be great.

I think we can continue to tweak the header format for a bit,
as I haven't updated the decode tool to deal with the new binary blob
anyway, and the version field gives us an out if we need to make
more serious changes.

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: Firmware debugging patches?

[Ben Greear]

On 06/09/2014 08:09 AM, Ben Greear wrote:
> 
> 
> On 06/09/2014 01:17 AM, Kalle Valo wrote:
>> Ben Greear <greearb@candelatech.com> writes:
>>
>>>>> With time-of-day, time-since-boot, and MAC, each dump should be unique.
>>>>
>>>> But I would like to easily match from kernel log that the crash dump
>>>> matches with log. uuid would provide that in a simple way (check that
>>>> the uuid in the log matches with the uuid in the dump). What's so bad
>>>> from using uuid?
>>>
>>> Just tell me how you want me to create the uuid and I'll do so.
>>
>> As kbuild found some compilation problems, I think it would be easier if
>> I make the final changes to your patches and send that for review. Would
>> that be ok for you?
> 
> Yes, that would be great.

In case it helps, let me post my latest patches...will get those up
in an hour or two.

Thanks,
Ben

> 
> I think we can continue to tweak the header format for a bit,
> as I haven't updated the decode tool to deal with the new binary blob
> anyway, and the version field gives us an out if we need to make
> more serious changes.
> 
> Thanks,
> Ben
> 


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: Firmware debugging patches?

[Ben Greear]

On 06/09/2014 08:47 AM, Ben Greear wrote:
> On 06/09/2014 08:09 AM, Ben Greear wrote:
>>
>>
>> On 06/09/2014 01:17 AM, Kalle Valo wrote:
>>> Ben Greear <greearb@candelatech.com> writes:
>>>
>>>>>> With time-of-day, time-since-boot, and MAC, each dump should be unique.
>>>>>
>>>>> But I would like to easily match from kernel log that the crash dump
>>>>> matches with log. uuid would provide that in a simple way (check that
>>>>> the uuid in the log matches with the uuid in the dump). What's so bad
>>>>> from using uuid?
>>>>
>>>> Just tell me how you want me to create the uuid and I'll do so.
>>>
>>> As kbuild found some compilation problems, I think it would be easier if
>>> I make the final changes to your patches and send that for review. Would
>>> that be ok for you?
>>
>> Yes, that would be great.
> 
> In case it helps, let me post my latest patches...will get those up
> in an hour or two.

Actually, I seem to have just forgotten everything over the weekend...the
v2 patches I posted last Friday are current with my own tree.  So,
if you can tweak them to your satisfaction, that should work fine for
me.

Thanks,
Ben


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: Firmware debugging patches?

[Kalle Valo]

Ben Greear <greearb@candelatech.com> writes:

>>>> As kbuild found some compilation problems, I think it would be easier if
>>>> I make the final changes to your patches and send that for review. Would
>>>> that be ok for you?
>>>
>>> Yes, that would be great.
>> 
>> In case it helps, let me post my latest patches...will get those up
>> in an hour or two.
>
> Actually, I seem to have just forgotten everything over the weekend...the
> v2 patches I posted last Friday are current with my own tree.  So,
> if you can tweak them to your satisfaction, that should work fine for
> me.

Great, thanks. I'll try to send a new version today. If you could then
review them carefully and test as well, that would be really good.

-- 
Kalle Valo

Re: Firmware debugging patches?

[Ben Greear]

On 06/09/2014 11:05 PM, Kalle Valo wrote:
> Ben Greear <greearb@candelatech.com> writes:
>
>>>>> As kbuild found some compilation problems, I think it would be easier if
>>>>> I make the final changes to your patches and send that for review. Would
>>>>> that be ok for you?
>>>>
>>>> Yes, that would be great.
>>>
>>> In case it helps, let me post my latest patches...will get those up
>>> in an hour or two.
>>
>> Actually, I seem to have just forgotten everything over the weekend...the
>> v2 patches I posted last Friday are current with my own tree.  So,
>> if you can tweak them to your satisfaction, that should work fine for
>> me.
>
> Great, thanks. I'll try to send a new version today. If you could then
> review them carefully and test as well, that would be really good.

I'll update my decode tool to support this as soon as it is pushed to your
tree, and will put it into our development tree for testing.  I have some
more patches related to this that I can post after the current 4 make it
upstream...these patches dump BSS data areas and send uevent, etc.

Thanks,
Ben


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: Firmware debugging patches?

[Ben Greear]

On 06/09/2014 11:05 PM, Kalle Valo wrote:
> Ben Greear <greearb@candelatech.com> writes:
> 
>>>>> As kbuild found some compilation problems, I think it would be easier if
>>>>> I make the final changes to your patches and send that for review. Would
>>>>> that be ok for you?
>>>>
>>>> Yes, that would be great.
>>>
>>> In case it helps, let me post my latest patches...will get those up
>>> in an hour or two.
>>
>> Actually, I seem to have just forgotten everything over the weekend...the
>> v2 patches I posted last Friday are current with my own tree.  So,
>> if you can tweak them to your satisfaction, that should work fine for
>> me.
> 
> Great, thanks. I'll try to send a new version today. If you could then
> review them carefully and test as well, that would be really good.

Any chance we could get at least some version of this in soon?

It would be nice to have a good way for users to report more than
just the basic crash dump registers...

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: Firmware debugging patches?

[Kalle Valo]

Ben Greear <greearb@candelatech.com> writes:

> On 06/09/2014 11:05 PM, Kalle Valo wrote:
>> Ben Greear <greearb@candelatech.com> writes:
>> 
>>>>>> As kbuild found some compilation problems, I think it would be easier if
>>>>>> I make the final changes to your patches and send that for review. Would
>>>>>> that be ok for you?
>>>>>
>>>>> Yes, that would be great.
>>>>
>>>> In case it helps, let me post my latest patches...will get those up
>>>> in an hour or two.
>>>
>>> Actually, I seem to have just forgotten everything over the weekend...the
>>> v2 patches I posted last Friday are current with my own tree.  So,
>>> if you can tweak them to your satisfaction, that should work fine for
>>> me.
>> 
>> Great, thanks. I'll try to send a new version today. If you could then
>> review them carefully and test as well, that would be really good.
>
> Any chance we could get at least some version of this in soon?

I'll should be do it soon. Something else came up and I wasn't able to
work on this. Sorry about that.

-- 
Kalle Valo

Re: Firmware debugging patches?

[Kalle Valo]

Ben Greear <greearb@candelatech.com> writes:

> I am not sure this is really the case currently.  But, once the
> blob is generated and stored in RAM, it would be easily enough to
> add ethtool option to dump it w/out debugfs support.  This will
> still not really address my concerns because it may take a year
> or two for the latest ethtool binary to make it to normal-ish users.

[...]

> I think ethtool would not be overly hard to implement...basic framework is already
> in the wifi stack.

Few weeks back John Linville sent ethtool related patches to support
something like this. Did you notice that?

-- 
Kalle Valo

Re: Firmware debugging patches?

[Ben Greear]

On 06/05/2014 03:58 AM, Kalle Valo wrote:
> Ben Greear <greearb@candelatech.com> writes:
> 
>> I am not sure this is really the case currently.  But, once the
>> blob is generated and stored in RAM, it would be easily enough to
>> add ethtool option to dump it w/out debugfs support.  This will
>> still not really address my concerns because it may take a year
>> or two for the latest ethtool binary to make it to normal-ish users.
> 
> [...]
> 
>> I think ethtool would not be overly hard to implement...basic framework is already
>> in the wifi stack.
> 
> Few weeks back John Linville sent ethtool related patches to support
> something like this. Did you notice that?


I didn't...but will take a look once I get my pending patches
dealt with.

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: Firmware debugging patches?

[Kalle Valo]

Hi,

sorry for jumping in late, I get too much email as usual :/

Ben Greear <greearb@candelatech.com> writes:

> [Good stuff snipped, adding linux-wireless as this is a more
> general issue if we are going to consider general framework]
>
>
> Maybe we should start with goals before getting to implementation
> details.  Here's my wish list that is ath10k specific, but probably
> similar to other firmware users:
>
> 1)  We need the firmware crash text currently printed to
> /var/log/messages.
>
> 2)  It would be nice to get the firmware RAM and stack dumps at time of
> crash to debug more interesting crashes.
>
> 3)  It would be nice to know about firmware debug messages for
> the period of time directly before the crash (maybe 2-5 minutes?)
>
> 4)  It would be nice to have this interleaved with kernel, supplicant,
> and related logs.

I would add:

5) A generic user space interface so that the same user space tool can
   store firmware crash dumps from all wireless drivers and no need to
   reinvent the wheel.

> If we are storing crashes for something like ethtool to report, we need
> RAM and/or disk storage so the firmware RAM dumps and such can be stored until
> the user and/or automated tools ask for them.  We need some way to automatically
> clean up old crashes so disk/ram is not overly utilized.  For APs,
> they are low on both RAM and 'disk', so storing crash logs for any
> length of time may be problematic.

I think most, if not all, wireless drivers store the firmware binary in
RAM forever. I'm having hard time to believe that storing one instance
(the latest one) of firmware crash dump would make any difference.
That's why I'm not worried about RAM usage.

-- 
Kalle Valo

Re: Firmware debugging patches?

[Ben Greear]

On 06/05/2014 03:51 AM, Kalle Valo wrote:
> Hi,
> 
> sorry for jumping in late, I get too much email as usual :/
> 
> Ben Greear <greearb@candelatech.com> writes:
> 
>> [Good stuff snipped, adding linux-wireless as this is a more
>> general issue if we are going to consider general framework]
>>
>>
>> Maybe we should start with goals before getting to implementation
>> details.  Here's my wish list that is ath10k specific, but probably
>> similar to other firmware users:
>>
>> 1)  We need the firmware crash text currently printed to
>> /var/log/messages.
>>
>> 2)  It would be nice to get the firmware RAM and stack dumps at time of
>> crash to debug more interesting crashes.
>>
>> 3)  It would be nice to know about firmware debug messages for
>> the period of time directly before the crash (maybe 2-5 minutes?)
>>
>> 4)  It would be nice to have this interleaved with kernel, supplicant,
>> and related logs.
> 
> I would add:
> 
> 5) A generic user space interface so that the same user space tool can
>    store firmware crash dumps from all wireless drivers and no need to
>    reinvent the wheel.

Looks like the plan to move the feature up into mac80211 once
we get the lower-level details sorted would take care of this.

> 
>> If we are storing crashes for something like ethtool to report, we need
>> RAM and/or disk storage so the firmware RAM dumps and such can be stored until
>> the user and/or automated tools ask for them.  We need some way to automatically
>> clean up old crashes so disk/ram is not overly utilized.  For APs,
>> they are low on both RAM and 'disk', so storing crash logs for any
>> length of time may be problematic.
> 
> I think most, if not all, wireless drivers store the firmware binary in
> RAM forever. I'm having hard time to believe that storing one instance
> (the latest one) of firmware crash dump would make any difference.
> That's why I'm not worried about RAM usage.

I'm concerned about the contents of the target's RAM, not the firmware
binary image.  Each crash would want to store a snapshot of the target's
RAM, including bss regions, stack(s), etc.  Basically, a poor man's
re-implementation of a core file.  In practice, this works out to
less that 100k bytes I think, so likely anything that can run ath10k
has that much memory to spare for debugging efforts...

I think this is resolved well enough in my patch (and iwlwifi does
it slightly differently, but should work too).

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com