On 3/18/21 11:29 PM, Alex Elder wrote:
There is sanity checking code in the IPA driver that's meant to be
enabled only during development. This allows the driver to make
certain assumptions, but not have to verify those assumptions are
true at (operational) runtime. This code is built conditional on
IPA_VALIDATION, set (if desired) inside the IPA makefile.
Given the pushback on the ipa_assert() patch I will send
out version 2 of this series, omitting the two patches
that involve assertions.
I still think there's a case for my proposal, but I'm
going to move on for now and try to find other ways
to do what I want. In some cases BUILD_BUG_ON() or
WARN_ON_DEV() could be used. In other spots, I might
be able to use dev_dbg() for checking things only
while developing. But there remain a few cases where
none of these options is quite right.
If I ever want to suggest an assertion again I'll do
it as an RFC and will copy Leon and Andrew, to make
sure they can provide input.
Thanks.
-Alex
Unfortunately, this validation code has some errors. First, there
are some mismatched arguments supplied to some dev_err() calls in
ipa_cmd_table_valid() and ipa_cmd_header_valid(), and these are
exposed if validation is enabled. Second, the tag that enables
this conditional code isn't used consistently (it's IPA_VALIDATE
in some spots and IPA_VALIDATION in others).
This series fixes those two problems with the conditional validation
code.
In addition, this series introduces some new assertion macros. I
have been meaning to add this for a long time. There are comments
indicating places where assertions could be checked throughout the
code.
The macros are designed so that any asserted condition will be
checked at compile time if possible. Otherwise, the condition
will be checked at runtime *only* if IPA_VALIDATION is enabled,
and ignored otherwise.
NOTE: The third patch produces two bogus (but understandable)
warnings from checkpatch.pl. It does not recognize that the "expr"
argument passed to those macros aren't actually evaluated more than
once. In both cases, all but one reference is consumed by the
preprocessor or compiler.
A final patch converts a handful of commented assertions into
"real" ones. Some existing validation code can done more simply
with assertions, so over time such cases will be converted. For
now though, this series adds this assertion capability.
-Alex
Alex Elder (4):
net: ipa: fix init header command validation
net: ipa: fix IPA validation
net: ipa: introduce ipa_assert()
net: ipa: activate some commented assertions
drivers/net/ipa/Makefile | 2 +-
drivers/net/ipa/gsi_trans.c | 8 ++---
drivers/net/ipa/ipa_assert.h | 50 ++++++++++++++++++++++++++++++++
drivers/net/ipa/ipa_cmd.c | 53 ++++++++++++++++++++++------------
drivers/net/ipa/ipa_cmd.h | 6 ++--
drivers/net/ipa/ipa_endpoint.c | 6 ++--
drivers/net/ipa/ipa_main.c | 6 ++--
drivers/net/ipa/ipa_mem.c | 6 ++--
drivers/net/ipa/ipa_reg.h | 7 +++--
drivers/net/ipa/ipa_table.c | 11 ++++---
drivers/net/ipa/ipa_table.h | 6 ++--
11 files changed, 115 insertions(+), 46 deletions(-)
create mode 100644 drivers/net/ipa/ipa_assert.h
