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

Wrong data size when reading from TUN device\wrong size of data buffers #26

Closed
rstanislav opened this issue Jan 4, 2013 · 7 comments
Closed

Comments

@rstanislav
Copy link

Hello! I have found small bug in code - when reading from TUN you need to add 4 bytes ip overhead (because when client for example send 1500 bytes packet, thats means that tun will add its 4 bytes IP header and real data size will be 1504 bytes).
so pktdata_t data size has to be (DEFAULT_MTU + 4) and when you reading from TUN

read(tuntap->fd, pkt->pktdata.data, (DEFAULT_MTU + 4));

or data will be corrupted.

don't know how much TAP is adding tho...

sorry for my bad english

ps thanks for this great project!

@zehome
Copy link
Owner

zehome commented Jan 4, 2013

Hi, thank you very much for the bug report.

I'll look into it as soon as possible to fix this.

I noticed yesterday you're working in the same area. Maybe we can share our results, I think this would be interresting :-)

ps: my english is bad too :-)

@rstanislav
Copy link
Author

Yeah vtrunkd is also great project but i don't really like how its "done" -
its using only tcp connections and shared memory and based on old vpn
project...way too overloaded and complicated, but it has some great algo's
in it..for speed control and other things
I like your project more, because i think that for aggregation udp is way
more better (yeah its more complicated but at same time it allows to get
max of all avail bw on any channel, where tcp will not allow this because
of sliding window thing...)
I'm working on 3g\slow speed\unstable connections aggregation solution, the
only thing that your project is missing atm is automatic speed control per
channel\ACK of data (so for example when packet lost on 1 channel server
reports that using other channels and client resends it using other
channel) - i'm currently working on this algo and when it will be ready i
will send details with source code to you.

btw also i have found 1 more bug - if REORDER_MAX_SEQ sets low and in
reorder.c code its notices that it segfaults on fprintf

fprintf("[reorder] buffer size can't be more t....

....very strange, i think somewhere in reorder code there is problem with
memory allocation

2013/1/4 Laurent Coustet notifications@github.com

Hi, thank you very much for the bug report.

I'll look into it as soon as possible to fix this.

I noticed yesterday you're working in the same area. Maybe we can share
our results, I think this would be interresting :-)

ps: my english is bad too :-)

ps(bis): thank you for vtrunkd I'm going to look into it.


Reply to this email directly or view it on GitHubhttps://github.com//issues/26#issuecomment-11882927.

@zehome
Copy link
Owner

zehome commented Jan 4, 2013

Yeah ok.

MLVPN was designer for "high speed" lowjitter like ADSL,SDSL as a first goal.

I tried working for 3G/slowspeed/unstable connections with the reorder branch, but without success. There are probably many mistakes on the reorder branch. It was just an unstable experiment.

If you need help debugging or understanding the think, don't hesitate, I'll write as much documentation as needed.

For example, keep in mind MLVPN is using privsep, and the first process launched is the privileged one.
So when you're using gdb, don't forget to "set follow-fork-mode child".

@zehome
Copy link
Owner

zehome commented Jan 4, 2013

There is a system to dynamically change most parameters (speed included) by modifying the configuration, and reloading mlvpn (SIGHUP).

It is planned to add this feature through the control socket (control.c). Should be trivial to do.

@rstanislav
Copy link
Author

I have tried reorder branch with last fixes from master branch and fix for
flush (on timeout do not flush buffer but instead find next packet in seq
and write to tundev) - i'm working now on this part but so far it works
perfectly :) I will commit when finish it.
Also reorder means no need for delay of packets because if packet with high
ms but lower seq not yet arrived next packet will be delayed automaticaly
by reorder logic.

For 3g and unstable connections retransmission of lost packets, reordering
and on the fly weight calculations for channels is key features.

2013/1/4 Laurent Coustet notifications@github.com

There is a system to dynamically change most parameters (speed included)
by modifying the configuration, and reloading mlvpn (SIGHUP).

It is planned to add this feature through the control socket (control.c).
Should be trivial to do.


Reply to this email directly or view it on GitHubhttps://github.com//issues/26#issuecomment-11885036.

@zehome
Copy link
Owner

zehome commented Jan 24, 2013

Sorry for the delay, still have not checked this.. Will try tonight

@zehome
Copy link
Owner

zehome commented Feb 5, 2013

I've checked the code and I don't think there is anything wrong. As I am using IF_NO_PI I should never have more than standard 1500 bytes MTU of data.

Let me know if I'm wrong

@zehome zehome closed this as completed Feb 5, 2013
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

No branches or pull requests

2 participants