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

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

[Patrick Menschel]

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

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

[Patrick Menschel]

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 <menschel.p@posteo.de>
---
 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

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

[Patrick Menschel]

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

Signed-off-by: Patrick Menschel <menschel.p@posteo.de>
---
 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

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

[Patrick Menschel]

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 <menschel.p@posteo.de>
---
 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

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

[Marc Kleine-Budde]

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

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 <menschel.p@posteo.de>
> ---
>  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 |

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

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

[Patrick Menschel]

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 <menschel.p@posteo.de>
>> ---
>>  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