Code review - Issue 369810043: ns-3 mptcphttps://codereview.appspot.com/2020-03-26T05:10:50+00:00rietveld
Message from unknown
2018-08-11T20:06:17+00:00kashif nadeemurn:md5:8285130cb9519c5b35e583aa5f615e52
Message from vivelepourriel@gmail.com
2018-09-04T07:14:58+00:00vivelepourrielurn:md5:d84e26d84d120ff1b4a69c26854114bb
some comments
https://codereview.appspot.com/369810043/diff/1/src/internet/model/mptcp-crypto.cc
File src/internet/model/mptcp-crypto.cc (right):
https://codereview.appspot.com/369810043/diff/1/src/internet/model/mptcp-crypto.cc#newcode61
src/internet/model/mptcp-crypto.cc:61: /*
remove
https://codereview.appspot.com/369810043/diff/1/src/internet/model/tcp-header.h
File src/internet/model/tcp-header.h (right):
https://codereview.appspot.com/369810043/diff/1/src/internet/model/tcp-header.h#newcode375
src/internet/model/tcp-header.h:375: for (TcpHeader::TcpOptionList::const_iterator it = l.begin (); it != l.end (); ++it)
this is not mandatory but now that ns3 supports c++11, you may be able to update the code to c++11 standards with for(auto it. ...)
https://codereview.appspot.com/369810043/diff/1/src/internet/model/tcp-header.h#newcode378
src/internet/model/tcp-header.h:378: // << " with " << T::GetTypeId().GetName()
remove comments !
https://codereview.appspot.com/369810043/diff/1/src/internet/model/tcp-l4-protocol.cc
File src/internet/model/tcp-l4-protocol.cc (right):
https://codereview.appspot.com/369810043/diff/1/src/internet/model/tcp-l4-protocol.cc#newcode808
src/internet/model/tcp-l4-protocol.cc:808: )
this function is just for debug. Either remove it or call it only within #ifdef DEBUG #endif
https://codereview.appspot.com/369810043/diff/1/src/internet/model/tcp-option.h
File src/internet/model/tcp-option.h (right):
https://codereview.appspot.com/369810043/diff/1/src/internet/model/tcp-option.h#newcode64
src/internet/model/tcp-option.h:64: MPTCP = 30, //! Multipath TCP options share the same Kind
align comment
Message from unknown
2018-09-05T21:57:08+00:00kashif nadeemurn:md5:a3207ae22cc3136c0630fd96ef406b8b
Message from trucanh524@gmail.com
2018-10-27T08:51:46+00:00trucanh524urn:md5:852208cac455eb34bb821fd2a7308661
At a high level, I think the current patch misses the following:
1. Some examples to show how to simulate the code
2. Unit tests
3. Documentation
Could you please add them?
Message from kshfnadeem@gmail.com
2018-10-27T13:32:34+00:00kashif nadeemurn:md5:4ee37750b2a615963cfef2ac9e7ce5ac
On 2018/10/27 08:51:46, trucanh524 wrote:
> At a high level, I think the current patch misses the following:
>
> 1. Some examples to show how to simulate the code
> 2. Unit tests
> 3. Documentation
>
> Could you please add them?
Hi, truncanh524
Thanks for reviewing my patch. I actually have example scripts but didn't yet created tests and documentation. I will try to create them and upload them here.
Regards,
Message from trucanh524@gmail.com
2018-10-27T14:14:16+00:00trucanh524urn:md5:6ff58c4b03d9e3a159dbbd1309066ae4
On 2018/10/27 13:32:34, kashif nadeem wrote:
> On 2018/10/27 08:51:46, trucanh524 wrote:
> > At a high level, I think the current patch misses the following:
> >
> > 1. Some examples to show how to simulate the code
> > 2. Unit tests
> > 3. Documentation
> >
> > Could you please add them?
>
> Hi, truncanh524
>
> Thanks for reviewing my patch. I actually have example scripts
Do you want to update the patch with your example scripts first so that I could run and review the code in more details given that I have some time this week?
> but didn't yet
> created tests and documentation. I will try to create them and upload them here.
I suggest the documentation also includes all the features that you have implemented, features that are still missing (in comparison with the RFC), and the limitations (if any)of the code. How many subflows the current patch can create?
Best,
Anh
>
> Regards,
Message from kshfnadeem@gmail.com
2018-10-27T14:23:44+00:00kashif nadeemurn:md5:7a5f2c21fe1d131198fc247290a518f2
On 2018/10/27 14:14:16, trucanh524 wrote:
> On 2018/10/27 13:32:34, kashif nadeem wrote:
> > On 2018/10/27 08:51:46, trucanh524 wrote:
> > > At a high level, I think the current patch misses the following:
> > >
> > > 1. Some examples to show how to simulate the code
> > > 2. Unit tests
> > > 3. Documentation
> > >
> > > Could you please add them?
> >
> > Hi, truncanh524
> >
> > Thanks for reviewing my patch. I actually have example scripts
>
> Do you want to update the patch with your example scripts first so that I could
> run and review the code in more details given that I have some time this week?
>
> > but didn't yet
> > created tests and documentation. I will try to create them and upload them
> here.
>
> I suggest the documentation also includes all the features that you have
> implemented, features that are still missing (in comparison with the RFC), and
> the limitations (if any)of the code. How many subflows the current patch can
> create?
>
> Best,
>
> Anh
> >
> > Regards,
Hi You can have example script from below link. I posted it in google groups because some people requested me for it. My code generates any number of flows. Normally 8 flows are recommended in literature per MPTCP connection. I will consider your advice while preparing documentation.
https://groups.google.com/forum/#!searchin/ns-3-reviews/MPTCP$20patch$20is$20on$20codereview$20for$20Review$20process%7Csort:date/ns-3-reviews/wtgm2E5yIU8/emHDjv9OBAAJ
Message from annguyen@ittc.ku.edu
2018-10-27T14:49:51+00:00annguyen_ittc.ku.eduurn:md5:4e29b712c78b2aff85f6f5320f5a1020
I don't see the example after clicking on the link.
Best,
Anh
On Saturday, October 27, 2018, <kshfnadeem@gmail.com> wrote:
> On 2018/10/27 14:14:16, trucanh524 wrote:
>
>> On 2018/10/27 13:32:34, kashif nadeem wrote:
>> > On 2018/10/27 08:51:46, trucanh524 wrote:
>> > > At a high level, I think the current patch misses the following:
>> > >
>> > > 1. Some examples to show how to simulate the code
>> > > 2. Unit tests
>> > > 3. Documentation
>> > >
>> > > Could you please add them?
>> >
>> > Hi, truncanh524
>> >
>> > Thanks for reviewing my patch. I actually have example scripts
>>
>
> Do you want to update the patch with your example scripts first so
>>
> that I could
>
>> run and review the code in more details given that I have some time
>>
> this week?
>
> > but didn't yet
>> > created tests and documentation. I will try to create them and
>>
> upload them
>
>> here.
>>
>
> I suggest the documentation also includes all the features that you
>>
> have
>
>> implemented, features that are still missing (in comparison with the
>>
> RFC), and
>
>> the limitations (if any)of the code. How many subflows the current
>>
> patch can
>
>> create?
>>
>
> Best,
>>
>
> Anh
>> >
>> > Regards,
>>
>
> Hi You can have example script from below link. I posted it in google
> groups because some people requested me for it. My code generates any
> number of flows. Normally 8 flows are recommended in literature per
> MPTCP connection. I will consider your advice while preparing
> documentation.
>
> https://groups.google.com/forum/#!searchin/ns-3-reviews/MPTC
> P$20patch$20is$20on$20codereview$20for$20Review$20process%
> 7Csort:date/ns-3-reviews/wtgm2E5yIU8/emHDjv9OBAAJ
>
> https://codereview.appspot.com/369810043/
>
--
----------------------------------------------------------------------------
*Truc Anh N. Nguyen*
annguyen@ittc.ku.edu http://www.ittc.ku.edu/~annguyen
<http://www.ittc.ku.edu/~annguyen>
Information & Telecommunication Technology Center
Department of Electrical Engineering and Computer Science
The University of Kansas, Lawrence, KS 66045, USA
Message from natale.patriciello@gmail.com
2018-12-06T18:22:48+00:00n.purn:md5:2f6d8ac71d49be04390371908205f9a3
Hi,
That's a huge work. It is impossible to review it in one time, and from the little I have seen, you did not follow any of the TCP latest trend (modularization and C++11 above all).
Let me put in another way: I hate inheritance, it is a pain in the ass to maintain. If you want it to be merged, you have to take the maintenance of the TCP in its entirely. Or, let's put a set of small patches that can be applied incrementally so we can try to fix things.
Message from kshfnadeem@gmail.com
2018-12-11T21:03:06+00:00kashif nadeemurn:md5:6aa1340bb2b8aee290a282a0662a27b8
On 2018/12/06 18:22:48, n.p wrote:
> Hi,
>
> That's a huge work. It is impossible to review it in one time, and from the
> little I have seen, you did not follow any of the TCP latest trend
> (modularization and C++11 above all).
>
> Let me put in another way: I hate inheritance, it is a pain in the ass to
> maintain. If you want it to be merged, you have to take the maintenance of the
> TCP in its entirely. Or, let's put a set of small patches that can be applied
> incrementally so we can try to fix things.
Hi Natale,
Thanks for your reviews. You are right it's a huge work and I spent lot of time on this model. May be, I've missed some of TCP latest trends. If you give me directions and point out things in my code which you think should be modified according to latest TCP trends, I will work on it and will revise this code.
I guess it's a big responsibility to maintain TCP along with MPTCP and I'm not sure if I'm ready for this. You also mentioned we can work with small patches to fix the things. I will work on it and will collaborate with development team to merge this model.
Message from francisleely@gmail.com
2018-12-19T14:05:11+00:00Francis Leeurn:md5:365eb41b459b793c2a86fbd90e44897b
Hi,
I have downloaded the patch and compiled MPTCP successfully in ns-3.28. MPTCP works well when I run the example "mptcp-example.cc" but when I try to test MPTCP in a spine-leaf topology, the simulation ends with:
...
==== Dumping list of mappings ====
==== End of dump ====
msg="Could not find mapping associated to ssn", file=../src/internet/model/mptcp-subflow.cc, line=289
terminate called without an active exception
Sorry I don't spend much time to check the code. I wonder what are the possible reasons for this failure?
Thank you so much.
Best,
Francis
Message from kshfnadeem@gmail.com
2018-12-19T15:51:39+00:00kashif nadeemurn:md5:f57f18489a30801e60c3884437b065f5
Hi,
I guess the problem is with your script and topology. MPTCP connection/flow isn’t getting mapping as expected. It may be the problem with TCP Sequence Numbers. You need to figure it out by debugging.
Sent from Mail for Windows 10
From: francisleely@gmail.com
Sent: 19 December 2018 19:05
To: kshfnadeem@gmail.com; vivelepourriel@gmail.com; trucanh524@gmail.com; annguyen@ittc.ku.edu; natale.patriciello@gmail.com
Cc: ns-3-reviews@googlegroups.com; reply@codereview-hr.appspotmail.com
Subject: Re: ns-3 mptcp (issue 369810043 by kshfnadeem@gmail.com)
Hi,
I have downloaded the patch and compiled MPTCP successfully in ns-3.28.
MPTCP works well when I run the example "mptcp-example.cc" but when I
try to test MPTCP in a spine-leaf topology, the simulation ends with:
...
==== Dumping list of mappings ====
==== End of dump ====
msg="Could not find mapping associated to ssn",
file=../src/internet/model/mptcp-subflow.cc, line=289
terminate called without an active exception
Sorry I don't spend much time to check the code. I wonder what are the
possible reasons for this failure?
Thank you so much.
Best,
Francis
https://codereview.appspot.com/369810043/
Message from rqzhangxd@gmail.com
2019-01-06T01:51:30+00:00rqzhangxdurn:md5:053dd919d751349e20b417c81ae246ae
Hi kashif:
These days, I have to compare the performance of the tcp and mptcp, I find that when several onoffapplications send flows to a same sink application, mptcp have a bug, when I using TCP, the sink application can receive all the bytes, but, when I using mptcp, the sink application just receive only one onoffapplication's packets, whenever other applications start or stop, the sink application just recieve the earliest start onoffapplication. I feel happy to discuss the problems with you, thank you.
Best,
Zhang Ruiqing
Message from rqzhangxd@gmail.com
2019-01-06T01:52:34+00:00rqzhangxdurn:md5:a6fa50d5a45c91e89bbbb3ddf7cbe164
Hi kashif:
These days, I have to compare the performance of the tcp and mptcp, I find that when several onoffapplications send flows to a same sink application, mptcp have a bug, when I using TCP, the sink application can receive all the bytes, but, when I using mptcp, the sink application just receive only one onoffapplication's packets, whenever other applications start or stop, the sink application just recieve the earliest start onoffapplication. I feel happy to discuss the problems with you, thank you.
Best,
Zhang Ruiqing
Message from 1129190026@qq.com
2019-04-16T07:27:00+00:001129190026urn:md5:a36ee4c18f06c95b0943cfe4cdb61af0
On 2019/01/06 01:52:34, rqzhangxd wrote:
> Hi kashif:
> These days, I have to compare the performance of the tcp and mptcp, I find
> that when several onoffapplications send flows to a same sink application, mptcp
> have a bug, when I using TCP, the sink application can receive all the bytes,
> but, when I using mptcp, the sink application just receive only one
> onoffapplication's packets, whenever other applications start or stop, the sink
> application just recieve the earliest start onoffapplication. I feel happy to
> discuss the problems with you, thank you.
> Best,
> Zhang Ruiqing
Hello,
I am writing a topology of mptcp-wifi recently, but this problem also appears, I would like to ask you how to solve this problem.
Message from 1129190026@qq.com
2019-04-16T07:27:33+00:001129190026urn:md5:835812960377ad2b8a67d5bba830f988
On 2019/01/06 01:52:34, rqzhangxd wrote:
> Hi kashif:
> These days, I have to compare the performance of the tcp and mptcp, I find
> that when several onoffapplications send flows to a same sink application, mptcp
> have a bug, when I using TCP, the sink application can receive all the bytes,
> but, when I using mptcp, the sink application just receive only one
> onoffapplication's packets, whenever other applications start or stop, the sink
> application just recieve the earliest start onoffapplication. I feel happy to
> discuss the problems with you, thank you.
> Best,
> Zhang Ruiqing
Hello,
I am writing a topology of mptcp-wifi recently, but this problem also appears, I would like to ask you how to solve this problem.
Message from ytxing96@gmail.com
2020-03-22T16:20:03+00:00ytxing96urn:md5:66296487503e8fe52a7c4cf9c3199615
On 2018/12/19 14:05:11, Francis Lee wrote:
> Hi,
>
> I have downloaded the patch and compiled MPTCP successfully in ns-3.28. MPTCP
> works well when I run the example "mptcp-example.cc" but when I try to test
> MPTCP in a spine-leaf topology, the simulation ends with:
> ...
> ==== Dumping list of mappings ====
> ==== End of dump ====
>
> msg="Could not find mapping associated to ssn",
> file=../src/internet/model/mptcp-subflow.cc, line=289
> terminate called without an active exception
>
> Sorry I don't spend much time to check the code. I wonder what are the possible
> reasons for this failure?
>
> Thank you so much.
>
> Best,
> Francis
Hi
I encounter the same issue. Do you have any idea how this could be solved?
Thanks.
Message from er.lpverma1986@gmail.com
2020-03-26T05:10:50+00:00L.P. Vermaurn:md5:df8ee0f3db4ff698ad354b683781a4cd
On 2020/03/22 16:20:03, ytxing96 wrote:
> On 2018/12/19 14:05:11, Francis Lee wrote:
> > Hi,
> >
> > I have downloaded the patch and compiled MPTCP successfully in ns-3.28. MPTCP
> > works well when I run the example "mptcp-example.cc" but when I try to test
> > MPTCP in a spine-leaf topology, the simulation ends with:
> > ...
> > ==== Dumping list of mappings ====
> > ==== End of dump ====
> >
> > msg="Could not find mapping associated to ssn",
> > file=../src/internet/model/mptcp-subflow.cc, line=289
> > terminate called without an active exception
> >
> > Sorry I don't spend much time to check the code. I wonder what are the
> possible
> > reasons for this failure?
> >
> > Thank you so much.
> >
> > Best,
> > Francis
>
> Hi
>
> I encounter the same issue. Do you have any idea how this could be solved?
>
> Thanks.
Hello Francis,
I am also facing same issue, when I tried to run mptcp-example.cc, the simulation ends with:
> ...
> > ==== Dumping list of mappings ====
> > ==== End of dump ====
are you have solution for same. If yes, kindly help me remove this error.
With Best Regard
L. P Verma