linux-can.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] can-isotp: Add error message if txqueuelen is too small.
@ 2021-04-25 15:10 Patrick Menschel
  2021-04-25 15:35 ` Vincent MAILHOL
  0 siblings, 1 reply; 6+ messages in thread
From: Patrick Menschel @ 2021-04-25 15:10 UTC (permalink / raw)
  To: linux-can

This patch adds a comprehensive error message in
case that txqueuelen is too small and the isotp
driver encounters ENOBUFS (105), e.g.
"no buffer space available" while it does
enqueue its generated frames.

Signed-off-by: Patrick Menschel <menschel.p@posteo.de>
---
 net/can/isotp.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/net/can/isotp.c b/net/can/isotp.c
index 9f94ad3ca..c50e238b0 100644
--- a/net/can/isotp.c
+++ b/net/can/isotp.c
@@ -946,8 +946,17 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
 	err = can_send(skb, 1);
 	dev_put(dev);
 	if (err) {
-		pr_notice_once("can-isotp: %s: can_send_ret %d\n",
-			       __func__, err);
+		if (err == -ENOBUFS) {
+			/* if txqueuelen is not of sufficient length
+			 * for this transfer
+			 */
+			pr_notice_once("can-isotp: %s: can_send_ret %d : No buffer space available in tx queue\n",
+				       __func__, err);
+		} else {
+			/* general error */
+			pr_notice_once("can-isotp: %s: can_send_ret %d\n",
+				       __func__, err);
+		}
 		return err;
 	}
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] can-isotp: Add error message if txqueuelen is too small.
  2021-04-25 15:10 [PATCH] can-isotp: Add error message if txqueuelen is too small Patrick Menschel
@ 2021-04-25 15:35 ` Vincent MAILHOL
  2021-04-25 16:45   ` Patrick Menschel
  0 siblings, 1 reply; 6+ messages in thread
From: Vincent MAILHOL @ 2021-04-25 15:35 UTC (permalink / raw)
  To: Patrick Menschel; +Cc: linux-can

On Sun. 26 Apr 2021 at 00:10, Patrick Menschel <menschel.p@posteo.de> wrote:
>
> This patch adds a comprehensive error message in
> case that txqueuelen is too small and the isotp
> driver encounters ENOBUFS (105), e.g.
> "no buffer space available" while it does
> enqueue its generated frames.
>
> Signed-off-by: Patrick Menschel <menschel.p@posteo.de>
> ---
>  net/can/isotp.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/net/can/isotp.c b/net/can/isotp.c
> index 9f94ad3ca..c50e238b0 100644
> --- a/net/can/isotp.c
> +++ b/net/can/isotp.c
> @@ -946,8 +946,17 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
>         err = can_send(skb, 1);
>         dev_put(dev);
>         if (err) {
> -               pr_notice_once("can-isotp: %s: can_send_ret %d\n",
> -                              __func__, err);
> +               if (err == -ENOBUFS) {
> +                       /* if txqueuelen is not of sufficient length
> +                        * for this transfer
> +                        */
> +                       pr_notice_once("can-isotp: %s: can_send_ret %d : No buffer space available in tx queue\n",
> +                                      __func__, err);

Speaking of comprehensive error messages, it would be great to
print the mnemotechnic of the error code instead of its value:
|                       pr_notice_once("can-isotp: %s: can_send_ret
%pe : tx queue is full\n",
|                                      __func__, ERR_PTR(err));

> +               } else {
> +                       /* general error */
> +                       pr_notice_once("can-isotp: %s: can_send_ret %d\n",
> +                                      __func__, err);

Same here:
|                       pr_notice_once("can-isotp: %s: can_send_ret %pe\n",
|                                      __func__, ERR_PTR(err));

Yours sincerely,
Vincent

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] can-isotp: Add error message if txqueuelen is too small.
  2021-04-25 15:35 ` Vincent MAILHOL
@ 2021-04-25 16:45   ` Patrick Menschel
  2021-04-25 23:59     ` Vincent MAILHOL
  0 siblings, 1 reply; 6+ messages in thread
From: Patrick Menschel @ 2021-04-25 16:45 UTC (permalink / raw)
  To: Vincent MAILHOL; +Cc: linux-can

Am 25.04.21 um 17:35 schrieb Vincent MAILHOL:
> On Sun. 26 Apr 2021 at 00:10, Patrick Menschel <menschel.p@posteo.de> wrote:
> Speaking of comprehensive error messages, it would be great to
> print the mnemotechnic of the error code instead of its value:
> |                       pr_notice_once("can-isotp: %s: can_send_ret
> %pe : tx queue is full\n",
> |                                      __func__, ERR_PTR(err));
> 
Thanks Vincent,

it's the first time I see this format string %pe , is it new or kernel
specific?

Regards,
Patrick Menschel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] can-isotp: Add error message if txqueuelen is too small.
  2021-04-25 16:45   ` Patrick Menschel
@ 2021-04-25 23:59     ` Vincent MAILHOL
  2021-04-26 17:10       ` Patrick Menschel
  0 siblings, 1 reply; 6+ messages in thread
From: Vincent MAILHOL @ 2021-04-25 23:59 UTC (permalink / raw)
  To: Patrick Menschel; +Cc: linux-can

On Mon. 26 Apr 2021 at 01:45, Patrick Menschel <menschel.p@posteo.de> wrote:
>
> Am 25.04.21 um 17:35 schrieb Vincent MAILHOL:
> > On Sun. 26 Apr 2021 at 00:10, Patrick Menschel <menschel.p@posteo.de> wrote:
> > Speaking of comprehensive error messages, it would be great to
> > print the mnemotechnic of the error code instead of its value:
> > |                       pr_notice_once("can-isotp: %s: can_send_ret
> > %pe : tx queue is full\n",
> > |                                      __func__, ERR_PTR(err));
> >
> Thanks Vincent,
>
> it's the first time I see this format string %pe , is it new or kernel
> specific?

Yes, this is fairly recent and it is kernel specific (and I love it).

It was added in commit 57f5677e535ba ("printf: add support for
printing symbolic error names").
Ref: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=57f5677e535ba24b8926a7125be2ef8d7f09323c


Yours sincerely,
Vincent

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] can-isotp: Add error message if txqueuelen is too small.
  2021-04-25 23:59     ` Vincent MAILHOL
@ 2021-04-26 17:10       ` Patrick Menschel
  2021-04-26 18:39         ` Marc Kleine-Budde
  0 siblings, 1 reply; 6+ messages in thread
From: Patrick Menschel @ 2021-04-26 17:10 UTC (permalink / raw)
  To: Vincent MAILHOL; +Cc: linux-can

Am 26.04.21 um 01:59 schrieb Vincent MAILHOL:
> On Mon. 26 Apr 2021 at 01:45, Patrick Menschel <menschel.p@posteo.de> wrote:
>>
>> Am 25.04.21 um 17:35 schrieb Vincent MAILHOL:
>>> On Sun. 26 Apr 2021 at 00:10, Patrick Menschel <menschel.p@posteo.de> wrote:
>>> Speaking of comprehensive error messages, it would be great to
>>> print the mnemotechnic of the error code instead of its value:
>>> |                       pr_notice_once("can-isotp: %s: can_send_ret
>>> %pe : tx queue is full\n",
>>> |                                      __func__, ERR_PTR(err));
>>>
>> Thanks Vincent,
>>
>> it's the first time I see this format string %pe , is it new or kernel
>> specific?
> 
> Yes, this is fairly recent and it is kernel specific (and I love it).
> 
> It was added in commit 57f5677e535ba ("printf: add support for
> printing symbolic error names").
> Ref: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=57f5677e535ba24b8926a7125be2ef8d7f09323c

Now that is really neat, sort of like python's __member__.name attribute
of the Enum type.
https://docs.python.org/3/library/enum.html#allowed-members-and-attributes-of-enumerations
I use it all the time since I moved away from ctypes.
Saves at least 2 lines per log message.

Proves that the very nature of best practice is convenience.

I'll do a v2 and substitute for every occurrence of err in that file
while I'm at it.

Regards,
Patrick

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] can-isotp: Add error message if txqueuelen is too small.
  2021-04-26 17:10       ` Patrick Menschel
@ 2021-04-26 18:39         ` Marc Kleine-Budde
  0 siblings, 0 replies; 6+ messages in thread
From: Marc Kleine-Budde @ 2021-04-26 18:39 UTC (permalink / raw)
  To: Patrick Menschel; +Cc: Vincent MAILHOL, linux-can

[-- Attachment #1: Type: text/plain, Size: 1894 bytes --]

On 26.04.2021 17:10:18, Patrick Menschel wrote:
> Am 26.04.21 um 01:59 schrieb Vincent MAILHOL:
> > On Mon. 26 Apr 2021 at 01:45, Patrick Menschel <menschel.p@posteo.de> wrote:
> >>
> >> Am 25.04.21 um 17:35 schrieb Vincent MAILHOL:
> >>> On Sun. 26 Apr 2021 at 00:10, Patrick Menschel <menschel.p@posteo.de> wrote:
> >>> Speaking of comprehensive error messages, it would be great to
> >>> print the mnemotechnic of the error code instead of its value:
> >>> |                       pr_notice_once("can-isotp: %s: can_send_ret
> >>> %pe : tx queue is full\n",
> >>> |                                      __func__, ERR_PTR(err));
> >>>
> >> Thanks Vincent,
> >>
> >> it's the first time I see this format string %pe , is it new or kernel
> >> specific?
> > 
> > Yes, this is fairly recent and it is kernel specific (and I love it).
> > 
> > It was added in commit 57f5677e535ba ("printf: add support for
> > printing symbolic error names").
> > Ref: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=57f5677e535ba24b8926a7125be2ef8d7f09323c
> 
> Now that is really neat, sort of like python's __member__.name attribute
> of the Enum type.
> https://docs.python.org/3/library/enum.html#allowed-members-and-attributes-of-enumerations
> I use it all the time since I moved away from ctypes.
> Saves at least 2 lines per log message.
> 
> Proves that the very nature of best practice is convenience.
> 
> I'll do a v2 and substitute for every occurrence of err in that file
> while I'm at it.

Make it two patches....what will increase you patch count :)

Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde           |
Embedded Linux                   | https://www.pengutronix.de  |
Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-04-26 18:39 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-25 15:10 [PATCH] can-isotp: Add error message if txqueuelen is too small Patrick Menschel
2021-04-25 15:35 ` Vincent MAILHOL
2021-04-25 16:45   ` Patrick Menschel
2021-04-25 23:59     ` Vincent MAILHOL
2021-04-26 17:10       ` Patrick Menschel
2021-04-26 18:39         ` Marc Kleine-Budde

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).