[PATCH] candump: add option to ignore ENOBUFS

[PATCH] candump: add option to ignore ENOBUFS

[yegorslists]

From: Yegor Yefremov <yegorslists@googlemail.com>

-f option replicated -i option from cangen and allows to
ignore ENOBUFS error in bridge mode

Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
---
 candump.c |   19 ++++++++++++++++---
 1 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/candump.c b/candump.c
index ac51192..3a6be62 100644
--- a/candump.c
+++ b/candump.c
@@ -53,6 +53,7 @@
 #include <ctype.h>
 #include <libgen.h>
 #include <time.h>
+#include <errno.h>
 
 #include <sys/time.h>
 #include <sys/types.h>
@@ -122,6 +123,7 @@ void print_usage(char *prg)
 	fprintf(stderr, "         -r <size>   (set socket receive buffer to <size>)\n");
 	fprintf(stderr, "         -d          (monitor dropped CAN frames)\n");
 	fprintf(stderr, "         -e          (dump CAN error frames in human-readable format)\n");
+	fprintf(stderr, "         -f          (ignore -ENOBUFS return values on write() syscalls)\n");
 	fprintf(stderr, "\n");
 	fprintf(stderr, "Up to %d CAN interfaces with optional filter sets can be specified\n", MAXSOCK);
 	fprintf(stderr, "on the commandline in the form: <ifname>[,filter]*\n");
@@ -211,6 +213,7 @@ int main(int argc, char **argv)
 	unsigned char view = 0;
 	unsigned char log = 0;
 	unsigned char logfrmt = 0;
+	unsigned char ignore_enobufs = 0;
 	int count = 0;
 	int rcvbuf_size = 0;
 	int opt, ret;
@@ -236,7 +239,7 @@ int main(int argc, char **argv)
 	last_tv.tv_sec  = 0;
 	last_tv.tv_usec = 0;
 
-	while ((opt = getopt(argc, argv, "t:ciaSs:b:B:u:ldLn:r:he?")) != -1) {
+	while ((opt = getopt(argc, argv, "ft:ciaSs:b:B:u:ldLn:r:he?")) != -1) {
 		switch (opt) {
 		case 't':
 			timestamp = optarg[0];
@@ -347,6 +350,10 @@ int main(int argc, char **argv)
 			}
 			break;
 
+		case 'f':
+			ignore_enobufs = 1;
+			break;
+
 		default:
 			print_usage(basename(argv[0]));
 			exit(1);
@@ -625,8 +632,14 @@ int main(int argc, char **argv)
 
 					nbytes = write(bridge, &frame, sizeof(struct can_frame));
 					if (nbytes < 0) {
-						perror("bridge write");
-						return 1;
+						if (errno != ENOBUFS) {
+							perror("bridge write");
+							return 1;
+						}
+						if (!ignore_enobufs) {
+							perror("bridge write");
+							return 1;
+						}
 					} else if ((size_t)nbytes < sizeof(struct can_frame)) {
 						fprintf(stderr,"bridge write: incomplete CAN frame\n");
 						return 1;
-- 
1.7.7

Re: [PATCH] candump: add option to ignore ENOBUFS

[Marc Kleine-Budde]

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

On 10/26/2012 11:42 AM, yegorslists@googlemail.com wrote:
> From: Yegor Yefremov <yegorslists@googlemail.com>
> 
> -f option replicated -i option from cangen and allows to
> ignore ENOBUFS error in bridge mode

Better not ignore, but handle the error. If socket would support, wait
with select/poll that the socket get writable and try again. For now
sleep and try again. (or fix the poll/select bug in the kernel).

Marc

> Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
> ---
>  candump.c |   19 ++++++++++++++++---
>  1 files changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/candump.c b/candump.c
> index ac51192..3a6be62 100644
> --- a/candump.c
> +++ b/candump.c
> @@ -53,6 +53,7 @@
>  #include <ctype.h>
>  #include <libgen.h>
>  #include <time.h>
> +#include <errno.h>
>  
>  #include <sys/time.h>
>  #include <sys/types.h>
> @@ -122,6 +123,7 @@ void print_usage(char *prg)
>  	fprintf(stderr, "         -r <size>   (set socket receive buffer to <size>)\n");
>  	fprintf(stderr, "         -d          (monitor dropped CAN frames)\n");
>  	fprintf(stderr, "         -e          (dump CAN error frames in human-readable format)\n");
> +	fprintf(stderr, "         -f          (ignore -ENOBUFS return values on write() syscalls)\n");
>  	fprintf(stderr, "\n");
>  	fprintf(stderr, "Up to %d CAN interfaces with optional filter sets can be specified\n", MAXSOCK);
>  	fprintf(stderr, "on the commandline in the form: <ifname>[,filter]*\n");
> @@ -211,6 +213,7 @@ int main(int argc, char **argv)
>  	unsigned char view = 0;
>  	unsigned char log = 0;
>  	unsigned char logfrmt = 0;
> +	unsigned char ignore_enobufs = 0;
>  	int count = 0;
>  	int rcvbuf_size = 0;
>  	int opt, ret;
> @@ -236,7 +239,7 @@ int main(int argc, char **argv)
>  	last_tv.tv_sec  = 0;
>  	last_tv.tv_usec = 0;
>  
> -	while ((opt = getopt(argc, argv, "t:ciaSs:b:B:u:ldLn:r:he?")) != -1) {
> +	while ((opt = getopt(argc, argv, "ft:ciaSs:b:B:u:ldLn:r:he?")) != -1) {
>  		switch (opt) {
>  		case 't':
>  			timestamp = optarg[0];
> @@ -347,6 +350,10 @@ int main(int argc, char **argv)
>  			}
>  			break;
>  
> +		case 'f':
> +			ignore_enobufs = 1;
> +			break;
> +
>  		default:
>  			print_usage(basename(argv[0]));
>  			exit(1);
> @@ -625,8 +632,14 @@ int main(int argc, char **argv)
>  
>  					nbytes = write(bridge, &frame, sizeof(struct can_frame));
>  					if (nbytes < 0) {
> -						perror("bridge write");
> -						return 1;
> +						if (errno != ENOBUFS) {
> +							perror("bridge write");
> +							return 1;
> +						}
> +						if (!ignore_enobufs) {
> +							perror("bridge write");
> +							return 1;
> +						}
>  					} else if ((size_t)nbytes < sizeof(struct can_frame)) {
>  						fprintf(stderr,"bridge write: incomplete CAN frame\n");
>  						return 1;
> 


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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

Re: [PATCH] candump: add option to ignore ENOBUFS

[Oliver Hartkopp]

On 26.10.2012 11:46, Marc Kleine-Budde wrote:

> On 10/26/2012 11:42 AM, yegorslists@googlemail.com wrote:
>> From: Yegor Yefremov <yegorslists@googlemail.com>
>>
>> -f option replicated -i option from cangen and allows to
>> ignore ENOBUFS error in bridge mode
> 
> Better not ignore, but handle the error. If socket would support, wait
> with select/poll that the socket get writable and try again. For now
> sleep and try again. (or fix the poll/select bug in the kernel).


ACK.

IIRC this was a problem of socket write queue length that are too long to
handle short MTUs like we have with CAN frames.
Therefore the socket itself does not stop the traffic.

Definitely something to be fixed ... i'll take a look into the mails from
Michal Sojka.

Regards,
Oliver

Re: [PATCH] candump: add option to ignore ENOBUFS

[Yegor Yefremov]

On Sun, Oct 28, 2012 at 9:02 PM, Oliver Hartkopp <socketcan@hartkopp.net> wrote:
> On 26.10.2012 11:46, Marc Kleine-Budde wrote:
>
>> On 10/26/2012 11:42 AM, yegorslists@googlemail.com wrote:
>>> From: Yegor Yefremov <yegorslists@googlemail.com>
>>>
>>> -f option replicated -i option from cangen and allows to
>>> ignore ENOBUFS error in bridge mode
>>
>> Better not ignore, but handle the error. If socket would support, wait
>> with select/poll that the socket get writable and try again. For now
>> sleep and try again. (or fix the poll/select bug in the kernel).
>
>
> ACK.
>
> IIRC this was a problem of socket write queue length that are too long to
> handle short MTUs like we have with CAN frames.
> Therefore the socket itself does not stop the traffic.
>
> Definitely something to be fixed ... i'll take a look into the mails from
> Michal Sojka.

Thanks. So the whole changes then would be related to all can-utils,
that can send data (candump, cangen etc.)?

Yegor

Re: [PATCH] candump: add option to ignore ENOBUFS

[Oliver Hartkopp]

Hi Yegor,

the question about a blocking write was still not addressed :-(

Btw. the routing in userspace became obsolete with can-gw which does the
routing in kernel space.

See can-utils 

$> cangw -?

Usage: cangw [options]

Commands:  -A (add a new rule)
           -D (delete a rule)
           -F (flush / delete all rules)
           -L (list all rules)
Mandatory: -s <src_dev>  (source netdevice)
           -d <dst_dev>  (destination netdevice)
Options:   -t (preserve src_dev rx timestamp)
           -e (echo sent frames - recommended on vcanx)
           -i (allow to route to incoming interface)
           -l <hops> (limit the number of frame hops / routings)
           -f <filter> (set CAN filter)
           -m <mod> (set frame modifications)
           -x <from_idx>:<to_idx>:<result_idx>:<init_xor_val> (XOR checksum)
           -c <from>:<to>:<result>:<init_val>:<xor_val>:<crctab[256]> (CRC8 cs)
           -p <profile>:[<profile_data>] (CRC8 checksum profile & parameters)

Values are given and expected in hexadecimal values. Leading 0s can be omitted.

<filter> is a <value><mask> CAN identifier filter
   <can_id>:<can_mask> (matches when <received_can_id> & mask == can_id & mask)
   <can_id>~<can_mask> (matches when <received_can_id> & mask != can_id & mask)

<mod> is a CAN frame modification instruction consisting of
<instruction>:<can_frame-elements>:<can_id>.<can_dlc>.<can_data>
 - <instruction> is one of 'AND' 'OR' 'XOR' 'SET'
 - <can_frame-elements> is _one_ or _more_ of 'I'dentifier 'L'ength 'D'ata
 - <can_id> is an u32 value containing the CAN Identifier
 - <can_dlc> is an u8 value containing the data length code (0 .. 8)
 - <can_data> is always eight(!) u8 values containing the CAN frames data
The max. four modifications are performed in the order AND -> OR -> XOR -> SET

Example:
cangw -A -s can0 -d vcan3 -e -f 123:C00007FF -m SET:IL:333.4.1122334455667788

Supported CRC 8 profiles:
Profile '1' (1U8)       - add one additional u8 value
Profile '2' (16U8)      - add u8 value from table[16] indexed by (data[1] & 0xF)
Profile '3' (SFFID_XOR) - add u8 value (can_id & 0xFF) ^ (can_id >> 8 & 0xFF)

$>

Does that fit your needs?
You can also create these routing jobs with netlink.

Regards,
Oliver


On 28.10.2012 21:02, Oliver Hartkopp wrote:
> On 26.10.2012 11:46, Marc Kleine-Budde wrote:
> 
>> On 10/26/2012 11:42 AM, yegorslists@googlemail.com wrote:
>>> From: Yegor Yefremov <yegorslists@googlemail.com>
>>>
>>> -f option replicated -i option from cangen and allows to
>>> ignore ENOBUFS error in bridge mode
>>
>> Better not ignore, but handle the error. If socket would support, wait
>> with select/poll that the socket get writable and try again. For now
>> sleep and try again. (or fix the poll/select bug in the kernel).
> 
> 
> ACK.
> 
> IIRC this was a problem of socket write queue length that are too long to
> handle short MTUs like we have with CAN frames.
> Therefore the socket itself does not stop the traffic.
> 
> Definitely something to be fixed ... i'll take a look into the mails from
> Michal Sojka.
> 
> Regards,
> Oliver
> 
> 

Re: [PATCH] candump: add option to ignore ENOBUFS

[Yegor Yefremov]

On Fri, Jan 10, 2014 at 11:02 PM, Oliver Hartkopp
<socketcan@hartkopp.net> wrote:
> Hi Yegor,
>
> the question about a blocking write was still not addressed :-(

Is this something to change in the generic networking stack or just
SocketCAN core?

> Btw. the routing in userspace became obsolete with can-gw which does the
> routing in kernel space.
>
> See can-utils
>
> $> cangw -?
>
> Usage: cangw [options]
>
> Commands:  -A (add a new rule)
>            -D (delete a rule)
>            -F (flush / delete all rules)
>            -L (list all rules)
> Mandatory: -s <src_dev>  (source netdevice)
>            -d <dst_dev>  (destination netdevice)
> Options:   -t (preserve src_dev rx timestamp)
>            -e (echo sent frames - recommended on vcanx)
>            -i (allow to route to incoming interface)
>            -l <hops> (limit the number of frame hops / routings)
>            -f <filter> (set CAN filter)
>            -m <mod> (set frame modifications)
>            -x <from_idx>:<to_idx>:<result_idx>:<init_xor_val> (XOR checksum)
>            -c <from>:<to>:<result>:<init_val>:<xor_val>:<crctab[256]> (CRC8 cs)
>            -p <profile>:[<profile_data>] (CRC8 checksum profile & parameters)
>
> Values are given and expected in hexadecimal values. Leading 0s can be omitted.
>
> <filter> is a <value><mask> CAN identifier filter
>    <can_id>:<can_mask> (matches when <received_can_id> & mask == can_id & mask)
>    <can_id>~<can_mask> (matches when <received_can_id> & mask != can_id & mask)
>
> <mod> is a CAN frame modification instruction consisting of
> <instruction>:<can_frame-elements>:<can_id>.<can_dlc>.<can_data>
>  - <instruction> is one of 'AND' 'OR' 'XOR' 'SET'
>  - <can_frame-elements> is _one_ or _more_ of 'I'dentifier 'L'ength 'D'ata
>  - <can_id> is an u32 value containing the CAN Identifier
>  - <can_dlc> is an u8 value containing the data length code (0 .. 8)
>  - <can_data> is always eight(!) u8 values containing the CAN frames data
> The max. four modifications are performed in the order AND -> OR -> XOR -> SET
>
> Example:
> cangw -A -s can0 -d vcan3 -e -f 123:C00007FF -m SET:IL:333.4.1122334455667788
>
> Supported CRC 8 profiles:
> Profile '1' (1U8)       - add one additional u8 value
> Profile '2' (16U8)      - add u8 value from table[16] indexed by (data[1] & 0xF)
> Profile '3' (SFFID_XOR) - add u8 value (can_id & 0xFF) ^ (can_id >> 8 & 0xFF)
>
> $>
>
> Does that fit your needs?
> You can also create these routing jobs with netlink.

I'm trying to add network functionality to slcanpty,c Reading from
socket or pty doesn't make a big difference. But as soon as you try to
send a lot of frames, you get ENOBUFS at once. I'm trying to
understand, what is the best way to handle this situation. In cangen
the situation is rather trivial, as one just sends. In slcanpty it
should support simultaneous read/write, so I'll have to store unsent
CAN frames somewhere, return to select handling and handle read stuff,
before retrying send.

Yegor

> On 28.10.2012 21:02, Oliver Hartkopp wrote:
>> On 26.10.2012 11:46, Marc Kleine-Budde wrote:
>>
>>> On 10/26/2012 11:42 AM, yegorslists@googlemail.com wrote:
>>>> From: Yegor Yefremov <yegorslists@googlemail.com>
>>>>
>>>> -f option replicated -i option from cangen and allows to
>>>> ignore ENOBUFS error in bridge mode
>>>
>>> Better not ignore, but handle the error. If socket would support, wait
>>> with select/poll that the socket get writable and try again. For now
>>> sleep and try again. (or fix the poll/select bug in the kernel).
>>
>>
>> ACK.
>>
>> IIRC this was a problem of socket write queue length that are too long to
>> handle short MTUs like we have with CAN frames.
>> Therefore the socket itself does not stop the traffic.
>>
>> Definitely something to be fixed ... i'll take a look into the mails from
>> Michal Sojka.
>>
>> Regards,
>> Oliver
>>
>>

Re: [PATCH] candump: add option to ignore ENOBUFS

[Oliver Hartkopp]

On 13.01.2014 11:00, Yegor Yefremov wrote:
> On Fri, Jan 10, 2014 at 11:02 PM, Oliver Hartkopp
> <socketcan@hartkopp.net> wrote:
>> Hi Yegor,
>>
>> the question about a blocking write was still not addressed :-(
> 
> Is this something to change in the generic networking stack or just
> SocketCAN core?

AFAICR it has something to do with socket buffer sizes and the poll function
which has to be implemented in CAN_RAW.

> I'm trying to add network functionality to slcanpty,c Reading from
> socket or pty doesn't make a big difference. But as soon as you try to
> send a lot of frames, you get ENOBUFS at once. I'm trying to
> understand, what is the best way to handle this situation. In cangen
> the situation is rather trivial, as one just sends. In slcanpty it
> should support simultaneous read/write, so I'll have to store unsent
> CAN frames somewhere, return to select handling and handle read stuff,
> before retrying send.

When you use a blocking write this may also slow down the original ASCII
source. You should probably think of two separate threads for rx/tx.

Regards,
Oliver