Submitted by: Andy Moreton <amoreton at solarflare.com>
Sponsored by: Solarflare Communications, Inc.
MFC after: 2 days
Details
Details
- Reviewers
philip np gnn bz - Commits
- rS291677: sfxge: add MCDI logging support to common code
Driver build tested. Module loaded/unloaded.
The patch with support in FreeBSD-specific part will follow.
Diff Detail
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
No Lint Coverage - Unit
No Test Coverage - Build Status
Buildable 1359 Build 1364: arc lint + arc unit
Event Timeline
Comment Actions
A couple of very minor style nits. Looks good otherwise.
| head/sys/dev/sfxge/common/efx_mcdi.c | ||
|---|---|---|
| 468 | Is this correct? Previously you weren't doing the copyout if errcode != 0. From the comment, the copyout was originally intended. Just making sure. | |
| head/sys/dev/sfxge/common/hunt_mcdi.c | ||
| 199 | Maybe just &hdr instead of &hdr[0] for consistency with other calls to emtp->emt_logger() (e.g. line 271 and elsewhere). | |
| 365–366 | Maybe just &hdr and &err here too? | |
Comment Actions
Thanks for review. Style fixes done in committed patch.
Feedback from Andy on copyout:
The copyout is needed to ensure that the logging is always performed (even if errcode != 0). This does mean that the error response is copied to the client buffer even if the client will not make use of it, but is a pragmatic way of ensuring that the logging always happens.
Fixing this properly requires some refactoring of the MCDI code and the client callbacks. The MCDI event handler performs the copyout for event-completed commands, but this should really be done by the background thread that issued the request (that is how the linux net driver does things). That complicates handling of errors and logging.
<<<