Do I need a FIFO queue?

[Andrea Crotti]

I have to write data on a tun device, and I thought that for writing I
needed a queue to avoid losing packets.

But maybe it's not needed, I could just wait that it's free (assuming
that it WILL be free, there's only one process accessing).

What I did is a while loop where I check (with a select) when it's
writable and then write it to the queue.
Every time I also try to write the whol remaining of the queue.

It's something like that below (all code is
http://github.com/AndreaCrotti/Network-mote/blob/master/driver/tunnel.c):
--8<---------------cut here---------------start------------->8---
int tun_write(int fd, char *buf, int length){
int nwrite;

// should not exit directly here maybe?
//TODO: Maybe send is better here
if((nwrite = write(fd, buf, length)) < 0){
perror("Writing data");
exit(1);
}

return nwrite;
}

void tunWriteNoQueue(int client_no, char *buf, int len) {
int fd = *get_fd(client_no);
// use some simple error checking here instead
assert(tun_write(fd, buf, len) == len);
}

void addToWriteQueue(int client_no, char *buf, int len) {
int fd = *get_fd(client_no);
// add the message to the queue
write_queue *queue = &(tun_devices[client_no].queue);
add_to_queue(queue, buf);
/* printf("now queue %d <-> %d\n", queue->first, queue->last); */
// now use a select to try to send out everything

// try to send out as many messages as possible
char *message;
// quit immediately the loop if we sent everything or is not writable
while (1) {
message = fetch_from_queue(queue);
if (!message)
break;

if (is_writable(fd)) {
int nwrite = tun_write(fd, buf, len);
/* printf("wrote %d bytes\n", nwrite); */
if (nwrite) {
// otherwise means partially written data
assert(nwrite == len);
// only now we can remove it from the queue
delete_last(queue);
}
}
else
break;
}
}

/**
* Check if the device is ready for writing
*
* @param fd file descriptor to check
*/
int is_writable(int fd) {
fd_set fds;
struct timeval timeout = {.tv_sec = 3, .tv_usec = 0};
int rc;
FD_ZERO(&fds);
FD_SET(fd, &fds);

// is this fd+1 exactly what we need here?
rc = select(fd + 1, NULL, &fds, NULL, &timeout);
return FD_ISSET(fd,&fds) ? 1 : 0;
}
--8<---------------cut here---------------end--------------->8---

 

[Ben Bacarisse]

I have to write data on a tun device, and I thought that for writing I
needed a queue to avoid losing packets.

If that is the "tun" as I understand it, you need to post in
comp.unix.programmer. Please re-post there.

This is a C language group and there are a couple of C-related issue
with your code:

int tun_write(int fd, char *buf, int length){
int nwrite;

// should not exit directly here maybe?
//TODO: Maybe send is better here
if((nwrite = write(fd, buf, length)) < 0){
perror("Writing data");
exit(1);
}
return nwrite;
}

void tunWriteNoQueue(int client_no, char *buf, int len) {
int fd = *get_fd(client_no);
// use some simple error checking here instead
assert(tun_write(fd, buf, len) == len);
}

It's odd (to say the least!) to use assert for such a purpose. Simply
compiling with NDEBUG set will stop all the writes (though this function
at least). You should probably do this instead:

int sent = tun_write(fd, buf, len);
assert(sent == len);

but if the purpose is to stop the program when sent != len, it's better
to do that yourself.

void addToWriteQueue(int client_no, char *buf, int len) {
int fd = *get_fd(client_no);
// add the message to the queue
write_queue *queue = &(tun_devices[client_no].queue);
add_to_queue(queue, buf);
/* printf("now queue %d <-> %d\n", queue->first, queue->last); */
// now use a select to try to send out everything

// try to send out as many messages as possible
char *message;
// quit immediately the loop if we sent everything or is not writable
while (1) {
message = fetch_from_queue(queue);
if (!message)
break;

if (is_writable(fd)) {
int nwrite = tun_write(fd, buf, len);
/* printf("wrote %d bytes\n", nwrite); */
if (nwrite) {
// otherwise means partially written data
assert(nwrite == len);
// only now we can remove it from the queue
delete_last(queue);
}
}
else
break;
}

Some people love break, but I'd write this as follows:

while ((message = fetch_from_queue(queue)) && is_writable(fd)) {
int nwrite = tun_write(fd, buf, len);
if (nwrite) {
// otherwise means partially written data
assert(nwrite == len);
// only now we can remove it from the queue
delete_last(queue);
}
}

BTW, that looks like another dodgy assert. Is it OK to delete_last when
nwrite != len, simply because of how the program is compiled?

<snip>

 

[Andrea Crotti]

If that is the "tun" as I understand it, you need to post in

comp.unix.programmer. Please re-post there.

This is a C language group and there are a couple of C-related issue
with your code:

Ok sorry, yes tun device is what you think I guess, but the problem is
more general.
I was just wondering if I really need to queue something on a device
that should always be writable or not...

It's odd (to say the least!) to use assert for such a purpose. Simply
compiling with NDEBUG set will stop all the writes (though this function
at least). You should probably do this instead:

int sent = tun_write(fd, buf, len);
assert(sent == len);

but if the purpose is to stop the program when sent != len, it's better
to do that yourself.

Yes it's funny because just yesterday I was reading some stuff on how to
not use assert, and then I copied exactly one of the bad examples

Some people love break, but I'd write this as follows:

while ((message = fetch_from_queue(queue)) && is_writable(fd)) {
int nwrite = tun_write(fd, buf, len);
if (nwrite) {
// otherwise means partially written data
assert(nwrite == len);
// only now we can remove it from the queue
delete_last(queue);
}
}

BTW, that looks like another dodgy assert. Is it OK to delete_last when
nwrite != len, simply because of how the program is compiled?

<snip>

Yes much more clean, well I planned to remove asserts to something more
reliable for when we finish the hard debug phase, but you're correct,
it's not a good idea..

I also didn't like break but with python (my main language) I got used
to them, if not misused it can still be quite clear...

 

[Jorgen Grahn]

Some people love break, but I'd write this as follows:

I love break, but I wouldn't use it assymetrically like that,
when I can instead use it to get rid of a whole block indentation:

while (1) {
message = fetch_from_queue(queue);
if (!message) break;
if (!is_writable(fd)) break;

int nwrite = tun_write(fd, buf, len);
/* printf("wrote %d bytes\n", nwrite); */
if (nwrite) {
// otherwise means partially written data
assert(nwrite == len);
// only now we can remove it from the queue
delete_last(queue);
}
}

Of course I could spend 30s more trying to get rid of the ugly
while(1) too.

BTW, on a multitasking system constructs like "if(is_writable(fd))
write(...)" look suspect. The state of the file may have changed
during the millisecond -- or hour -- between the two calls. Just try
to write and be prepared that it may fail.

/Jorgen

 

[Ben Bacarisse]

I love break, but I wouldn't use it assymetrically like that,
when I can instead use it to get rid of a whole block indentation:

while (1) {
message = fetch_from_queue(queue);
if (!message) break;
if (!is_writable(fd)) break;

int nwrite = tun_write(fd, buf, len);
/* printf("wrote %d bytes\n", nwrite); */
if (nwrite) {
// otherwise means partially written data
assert(nwrite == len);
// only now we can remove it from the queue
delete_last(queue);
}
}

Of course I could spend 30s more trying to get rid of the ugly
while(1) too.

Then, surely, you'd end up with the version I posted?

<snip>

 

[Nick Keighley]

a TUN
"... provides low level [Linux] kernel support for IP tunneling"

Ok sorry, yes tun device is what you think I guess, but the problem is
more general.
I was just wondering if I really need to queue something on a device
that should always be writable or not...

yes but to answer the question you'd need to know something about TUNs
or Linux devices at least.

Yes it's funny because just yesterday I was reading some stuff on how to
not use assert, and then I copied exactly one of the bad examples

I too love break but your code was of the form

while (1)
{
if (blah)
do_bler();
else
break;
}

which simply screams "restructure me!!" at you

Yes much more clean, well I planned to remove asserts to something more
reliable for when we finish the hard debug phase, but you're correct,
it's not a good idea..

I also didn't like break but with python (my main language) I got used
to them, if not misused it can still be quite clear...

strange I've pythoned a bit and that didn't turn me into a heavy user
of break. If anything I'd have said python was *more* structured than
C!


--

Justifying space exploration because we get non-stick frying pans is
like justifying music because it is good exercise for the violinist's
right arm.
--Richard Dawkins

 

[Andrea Crotti]

strange I've pythoned a bit and that didn't turn me into a heavy user
of break. If anything I'd have said python was *more* structured than
C!

You're right it is of course.
With list comprehension and some functional programming I almost never
use loops, but when I do is or
for x in iterable:

or
while True:
if ..
break

....

 

[Ben Bacarisse]

I was Andrea Crotti who replied without attribution lines but even so...

I too love break but your code was of the form

while (1)
{
if (blah)
do_bler();
else
break;
}

which simply screams "restructure me!!" at you

Why make this remark after quoting an example that does not have the
form you mention? Who is the "your" in "your code" and the "you" who is
being "screamed at"? Your reply is to Andrea Crotti, but the quote is
my re-write.

<snip>

 

[Jorgen Grahn]

Then, surely, you'd end up with the version I posted?

Possibly; I didn't read your version. My point was that if you're
going to use break, you should put it to good use. (And I suppose I
wanted there to be a decent break example in the thread, in case it
turned into a break flamewar later.)

/Jorgen