2021-04-27 05:22:49

by Patrick Menschel

[permalink] [raw]
Subject: [PATCH v2 0/3] can-isotp: Add more comprehensive error messages

This patch series adds more comprehensive error messages to isotp.c by
using error pointers instead of decimal error numbers.

https://www.kernel.org/doc/html/latest/core-api/printk-formats.html#error-pointers

It also adds a more comprehensive error message in case txqueue is full
due to a burst transfer, telling the user to increase txqueuelen to
prevent this from happening.


Patrick Menschel (3):
can-isotp: Change error format from decimal to symbolic error names
can-isotp: Add symbolic error message to isotp_module_init()
can-isotp: Add error message if txqueuelen is too small

net/can/isotp.c | 20 +++++++++++---------
1 file changed, 11 insertions(+), 9 deletions(-)

--
2.17.1


2021-04-27 05:23:09

by Patrick Menschel

[permalink] [raw]
Subject: [PATCH v2 1/3] can-isotp: Change error format from decimal to symbolic error names

This patch changes the format string for errors from
decimal %d to symbolic error names %pe to achieve
more comprehensive log messages.

Signed-off-by: Patrick Menschel <[email protected]>
---
net/can/isotp.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/net/can/isotp.c b/net/can/isotp.c
index 9f94ad3ca..2c4f84fac 100644
--- a/net/can/isotp.c
+++ b/net/can/isotp.c
@@ -221,8 +221,8 @@ static int isotp_send_fc(struct sock *sk, int ae, u8 flowstatus)

can_send_ret = can_send(nskb, 1);
if (can_send_ret)
- pr_notice_once("can-isotp: %s: can_send_ret %d\n",
- __func__, can_send_ret);
+ pr_notice_once("can-isotp: %s: can_send_ret %pe\n",
+ __func__, ERR_PTR(can_send_ret));

dev_put(dev);

@@ -798,8 +798,8 @@ static enum hrtimer_restart isotp_tx_timer_handler(struct hrtimer *hrtimer)

can_send_ret = can_send(skb, 1);
if (can_send_ret)
- pr_notice_once("can-isotp: %s: can_send_ret %d\n",
- __func__, can_send_ret);
+ pr_notice_once("can-isotp: %s: can_send_ret %pe\n",
+ __func__, ERR_PTR(can_send_ret));

if (so->tx.idx >= so->tx.len) {
/* we are done */
@@ -946,8 +946,8 @@ 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);
+ pr_notice_once("can-isotp: %s: can_send_ret %pe\n",
+ __func__, ERR_PTR(err));
return err;
}

--
2.17.1

2021-04-27 05:23:56

by Patrick Menschel

[permalink] [raw]
Subject: [PATCH v2 3/3] can-isotp: Add error message if txqueuelen is too small

This patch adds an additional error message in
case that txqueuelen is set too small and
advices the user to increase txqueuelen.

This is likely to happen even with small transfers if
txqueuelen is at default value 10 frames.

Signed-off-by: Patrick Menschel <[email protected]>
---
net/can/isotp.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/can/isotp.c b/net/can/isotp.c
index 2075d8d9e..d08f95bfd 100644
--- a/net/can/isotp.c
+++ b/net/can/isotp.c
@@ -797,10 +797,12 @@ static enum hrtimer_restart isotp_tx_timer_handler(struct hrtimer *hrtimer)
can_skb_set_owner(skb, sk);

can_send_ret = can_send(skb, 1);
- if (can_send_ret)
+ if (can_send_ret) {
pr_notice_once("can-isotp: %s: can_send_ret %pe\n",
__func__, ERR_PTR(can_send_ret));
-
+ if (can_send_ret == -ENOBUFS)
+ pr_notice_once("can-isotp: tx queue is full, increasing txqueuelen may prevent this error");
+ }
if (so->tx.idx >= so->tx.len) {
/* we are done */
so->tx.state = ISOTP_IDLE;
--
2.17.1

2021-04-27 05:24:20

by Patrick Menschel

[permalink] [raw]
Subject: [PATCH v2 2/3] can-isotp: Add symbolic error message to isotp_module_init()

This patch adds the value of err with format %pe
to the already existing error message.

Signed-off-by: Patrick Menschel <[email protected]>
---
net/can/isotp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/can/isotp.c b/net/can/isotp.c
index 2c4f84fac..2075d8d9e 100644
--- a/net/can/isotp.c
+++ b/net/can/isotp.c
@@ -1433,7 +1433,7 @@ static __init int isotp_module_init(void)

err = can_proto_register(&isotp_can_proto);
if (err < 0)
- pr_err("can: registration of isotp protocol failed\n");
+ pr_err("can: registration of isotp protocol failed %pe\n", ERR_PTR(err));

return err;
}
--
2.17.1

2021-04-27 07:18:06

by Marc Kleine-Budde

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] can-isotp: Add error message if txqueuelen is too small

On 27.04.2021 05:21:49, Patrick Menschel wrote:
> This patch adds an additional error message in
> case that txqueuelen is set too small and
> advices the user to increase txqueuelen.
>
> This is likely to happen even with small transfers if
> txqueuelen is at default value 10 frames.
>
> Signed-off-by: Patrick Menschel <[email protected]>
> ---
> net/can/isotp.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/net/can/isotp.c b/net/can/isotp.c
> index 2075d8d9e..d08f95bfd 100644
> --- a/net/can/isotp.c
> +++ b/net/can/isotp.c
> @@ -797,10 +797,12 @@ static enum hrtimer_restart isotp_tx_timer_handler(struct hrtimer *hrtimer)
> can_skb_set_owner(skb, sk);
>
> can_send_ret = can_send(skb, 1);
> - if (can_send_ret)
> + if (can_send_ret) {
> pr_notice_once("can-isotp: %s: can_send_ret %pe\n",
> __func__, ERR_PTR(can_send_ret));
> -
> + if (can_send_ret == -ENOBUFS)
> + pr_notice_once("can-isotp: tx queue is full, increasing txqueuelen may prevent this error");

I've added the missing "\n" at the end while applying the patch to
linux-can-next/testing.

regards,
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 |


Attachments:
(No filename) (1.42 kB)
signature.asc (499.00 B)
Download all attachments

2021-04-27 13:14:35

by Patrick Menschel

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] can-isotp: Add error message if txqueuelen is too small

Am 27.04.21 um 09:16 schrieb Marc Kleine-Budde:
> On 27.04.2021 05:21:49, Patrick Menschel wrote:
>> This patch adds an additional error message in
>> case that txqueuelen is set too small and
>> advices the user to increase txqueuelen.
>>
>> This is likely to happen even with small transfers if
>> txqueuelen is at default value 10 frames.
>>
>> Signed-off-by: Patrick Menschel <[email protected]>
>> ---
>> net/can/isotp.c | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/can/isotp.c b/net/can/isotp.c
>> index 2075d8d9e..d08f95bfd 100644
>> --- a/net/can/isotp.c
>> +++ b/net/can/isotp.c
>> @@ -797,10 +797,12 @@ static enum hrtimer_restart isotp_tx_timer_handler(struct hrtimer *hrtimer)
>> can_skb_set_owner(skb, sk);
>>
>> can_send_ret = can_send(skb, 1);
>> - if (can_send_ret)
>> + if (can_send_ret) {
>> pr_notice_once("can-isotp: %s: can_send_ret %pe\n",
>> __func__, ERR_PTR(can_send_ret));
>> -
>> + if (can_send_ret == -ENOBUFS)
>> + pr_notice_once("can-isotp: tx queue is full, increasing txqueuelen may prevent this error");
>
> I've added the missing "\n" at the end while applying the patch to
> linux-can-next/testing.
>
> regards,
> Marc
>
Thanks, that was sloppy of me.

Regards,
Patrick