zehome / MLVPN

Multi-link VPN (ADSL/SDSL/xDSL/Network aggregation / bonding)
http://www.mlvpn.fr/
BSD 2-Clause "Simplified" License
521 stars 129 forks source link

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

Closed rstanislav closed 11 years ago

rstanislav commented 11 years ago

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 commented 11 years ago

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 commented 11 years ago

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/zehome/MLVPN/issues/26#issuecomment-11882927.

zehome commented 11 years ago

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 commented 11 years ago

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 commented 11 years ago

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/zehome/MLVPN/issues/26#issuecomment-11885036.

zehome commented 11 years ago

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

zehome commented 11 years ago

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