From: Leon Romanovsky <leon@kernel.org>
To: Alex Elder <elder@linaro.org>
Cc: davem@davemloft.net, kuba@kernel.org, bjorn.andersson@linaro.org,
evgreen@chromium.org, cpratapa@codeaurora.org,
subashab@codeaurora.org, elder@kernel.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next 0/4] net: ipa: kill IPA_VALIDATION
Date: Tue, 27 Jul 2021 14:16:39 +0300 [thread overview]
Message-ID: <YP/rFwvIHOvIwMNO@unreal> (raw)
In-Reply-To: <20210726174010.396765-1-elder@linaro.org>
On Mon, Jul 26, 2021 at 12:40:06PM -0500, Alex Elder wrote:
> A few months ago I proposed cleaning up some code that validates
> certain things conditionally, arguing that doing so once is enough,
> thus doing so always should not be necessary.
> https://lore.kernel.org/netdev/20210320141729.1956732-1-elder@linaro.org/
> Leon Romanovsky felt strongly that this was a mistake, and in the
> end I agreed to change my plans.
<...>
> The second patch fixes a bug that wasn't normally exposed because of
> the conditional compilation (a reason Leon was right about this).
Thanks Alex,
If you want another anti pattern that is very popular in netdev, the following pattern is
wrong by definition :):
if (WARN_ON(...))
return ...
The WARN_*() macros are intended catch impossible flows, something that
shouldn't exist. The idea that printed stack to dmesg and return to the
caller will fix the situation is a very naive one. That stack already
says that something very wrong in the system.
If such flow can be valid use "if(...) return ..", if not use plain
WARN_ON(...).
Thanks
next prev parent reply other threads:[~2021-07-27 11:16 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-26 17:40 [PATCH net-next 0/4] net: ipa: kill IPA_VALIDATION Alex Elder
2021-07-26 17:40 ` [PATCH net-next 1/4] net: ipa: fix ipa_cmd_table_valid() Alex Elder
2021-07-26 17:40 ` [PATCH net-next 2/4] net: ipa: always validate filter and route tables Alex Elder
2021-07-26 17:40 ` [PATCH net-next 3/4] net: ipa: kill the remaining conditional validation code Alex Elder
2021-07-26 17:40 ` [PATCH net-next 4/4] net: ipa: use WARN_ON() rather than assertions Alex Elder
2021-07-26 21:52 ` [PATCH net-next 0/4] net: ipa: kill IPA_VALIDATION patchwork-bot+netdevbpf
2021-07-27 11:16 ` Leon Romanovsky [this message]
2021-07-27 12:34 ` Alex Elder
2021-07-27 12:56 ` Leon Romanovsky
2021-07-27 13:40 ` Alex Elder
2021-07-27 14:03 ` Leon Romanovsky
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YP/rFwvIHOvIwMNO@unreal \
--to=leon@kernel.org \
--cc=bjorn.andersson@linaro.org \
--cc=cpratapa@codeaurora.org \
--cc=davem@davemloft.net \
--cc=elder@kernel.org \
--cc=elder@linaro.org \
--cc=evgreen@chromium.org \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=subashab@codeaurora.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).