From: Jakub Kicinski <kuba@kernel.org>
To: Marc Kleine-Budde <mkl@pengutronix.de>
Cc: netdev@vger.kernel.org, davem@davemloft.net,
linux-can@vger.kernel.org, kernel@pengutronix.de,
Davide Caratti <dcaratti@redhat.com>
Subject: Re: [PATCH net-next 6/7] can: add drop reasons in the receive path of AF_CAN
Date: Tue, 10 Jun 2025 15:50:39 -0700 [thread overview]
Message-ID: <20250610155039.64ccdbda@kernel.org> (raw)
In-Reply-To: <20250610094933.1593081-7-mkl@pengutronix.de>
On Tue, 10 Jun 2025 11:46:21 +0200 Marc Kleine-Budde wrote:
> Besides the existing pr_warn_once(), use skb drop reasons in case AF_CAN
> layer drops non-conformant CAN{,FD,XL} frames, or conformant frames
> received by "wrong" devices, so that it's possible to debug (and count)
> such events using existing tracepoints:
Hm, I wonder if the protocol is really the most useful way
to categorize. Does it actually help to identify problems on
production systems?
AFAIU we try to categorize by drop condition. So given the condition
is:
if (unlikely(dev->type != ARPHRD_CAN || !can_get_ml_priv(dev) || !can_is_canfd_skb(skb)))
my intuition would be to split this into two: "not a CAN device" and
"invalid CAN frame". The latter not split by proto - user can dig into
the stack traces of the relevant drop for more details.
But drop reasons are not uAPI so we can re-align later.
next prev parent reply other threads:[~2025-06-10 22:50 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-06-10 9:46 [PATCH net-next 0/7] pull-request: can-next 2025-06-10 Marc Kleine-Budde
2025-06-10 9:46 ` [PATCH net-next 1/7] can: netlink: replace tabulation by space in assignment Marc Kleine-Budde
2025-06-10 23:50 ` patchwork-bot+netdevbpf
2025-06-10 9:46 ` [PATCH net-next 2/7] can: bittiming: rename CAN_CTRLMODE_TDC_MASK into CAN_CTRLMODE_FD_TDC_MASK Marc Kleine-Budde
2025-06-10 9:46 ` [PATCH net-next 3/7] can: bittiming: rename can_tdc_is_enabled() into can_fd_tdc_is_enabled() Marc Kleine-Budde
2025-06-10 9:46 ` [PATCH net-next 4/7] can: netlink: can_changelink(): rename tdc_mask into fd_tdc_flag_provided Marc Kleine-Budde
2025-06-10 9:46 ` [PATCH net-next 5/7] documentation: networking: can: Document alloc_candev_mqs() Marc Kleine-Budde
2025-06-10 9:46 ` [PATCH net-next 6/7] can: add drop reasons in the receive path of AF_CAN Marc Kleine-Budde
2025-06-10 22:50 ` Jakub Kicinski [this message]
2025-06-11 11:39 ` Davide Caratti
2025-06-10 9:46 ` [PATCH net-next 7/7] can: add drop reasons in CAN protocols receive path Marc Kleine-Budde
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=20250610155039.64ccdbda@kernel.org \
--to=kuba@kernel.org \
--cc=davem@davemloft.net \
--cc=dcaratti@redhat.com \
--cc=kernel@pengutronix.de \
--cc=linux-can@vger.kernel.org \
--cc=mkl@pengutronix.de \
--cc=netdev@vger.kernel.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).