Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

set up message queues for the IP and UDP thread #680

Merged
merged 2 commits into from
Feb 24, 2014

Conversation

benpicco
Copy link
Contributor

When receiving many (UDP) packets RIOT crashes both on native and on the msba2.
It turns out that lost messages in the network stack are not handled well: If a queued message gets replaced by a new one, sender in _msg_receive points to invalid data, RIOT crashes.

This pull request works around this issue by hooking up the message queue in ip.c that is defined, but never used. It also introduces a similar queue for the udp thread, so now up to 64 packets can be received at roughly the same time before RIOT crashes.

@@ -51,7 +51,7 @@
volatile int _native_in_isr;
volatile int _native_in_syscall;

static sigset_t _native_sig_set, _native_sig_set_dint;
static sigset_t _native_sig_set = {0}, _native_sig_set_dint = {0};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as far as I know static variables are always initialized to zero. so no need to do it explicitly. http://stackoverflow.com/questions/3373108/why-are-static-variables-auto-initialized-to-zero

@mehlis mehlis self-assigned this Feb 12, 2014
msg_queue is defined but never used, hook it up so IP packets get queued instead of dropped when there is more than one.
change the name to ip_msg_queue to avoid naming conflicts.
All other layers in the network stack use a msg_queue to not drop messages, which in this context represent packages.
This finally fixes the random crashes when UDP network traffic is present. Turns out RIOT is not handling lost messages well.
@OlegHahm
Copy link
Member

64 * sizeof(msg_t) = 512. That's quiet a lot for most platforms. Can we reduce the size of the queue for non-native platforms?

@mehlis
Copy link
Contributor

mehlis commented Feb 16, 2014

@OlegHahm do you think that 8 slots are enough?
Are there other places in the sixlowpan stack with too big queues?
What is a good value for this, in general?

@OlegHahm
Copy link
Member

I think it heavily depends on the expected traffic pattern. In general I would vote for dimensioning these queues MCU dependent and fix/improve the handling of lost messages in 6LoWPAN.

@OlegHahm
Copy link
Member

Maybe @benpicco can do some testing with different queue sizes and determine the best size.

@mehlis
Copy link
Contributor

mehlis commented Feb 19, 2014

@OlegHahm let's merge this now and open up a ticket to keep in mind that the queue sizes in the sixlowpan stack need a refinement.

@mehlis
Copy link
Contributor

mehlis commented Feb 24, 2014

as discussed in dev meeting, ACK, GO

mehlis pushed a commit that referenced this pull request Feb 24, 2014
set up message queues for the IP and UDP thread
@mehlis mehlis merged commit 8556403 into RIOT-OS:master Feb 24, 2014
@OlegHahm
Copy link
Member

Created #773

@OlegHahm OlegHahm mentioned this pull request Jan 13, 2015
@benpicco benpicco deleted the fix_crash branch August 20, 2019 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants