On 2014/04/11 13:51:28, Tom Henderson wrote:
> My main comment is that it isn't clear to me whether this will work on a node
> with multiple net devices and how multiple packet flows are successfully
> demultiplexed at the receiver. Probably more API is needed. Here is a
> challenge test for the test suite: create a client node with two
> SimpleNetDevices A and B, and two send apps As and Bs. Create a server node
> with two SimpleNetDevices C and D, and two server apps Cr and Dr. Connect
A<->C
> and B<->D. Have app As send some number of packets to Cr, and Bs to Dr.
Verify
> that Cr gets only As packets, and Dr only Bs packets.
It will not work as you think. The point is: there's no L4 protocol in between a
PacketSocket
and a SimpleNetDevice. As a consequence, there's no mux/demux. The only
mux/demux is
done by using the protocol field (if the NetDevice supports it).
> Regarding the class naming, elsewhere in ns-3 we spell out "Application" in
the
> class name, so I'd prefer to either spell it out here or find different names.
> How about "SimpleClient" and "SimpleServer" names instead?
I renamed them.
> Finally, the class doesn't seem to easily allow for a user to send packets at
> arbitrary times, only regular intervals starting at time zero. For instance,
> what if I just wanted to send packets only at times 3, 10, and 30 seconds--
how
> could I do that?
You can't :)
About the staring time, just change the Application::Start() time. About sending
packets at specific times, it's not supported.
Mind that the apps are meant to be used in tests, and usually a single packet is
more
than enough.
> Other comments are interleaved in the code.
>
>
https://codereview.appspot.com/45320043/diff/20001/src/network/examples/simpl...
> File src/network/examples/simple-apps.cc (right):
>
>
https://codereview.appspot.com/45320043/diff/20001/src/network/examples/simpl...
> src/network/examples/simple-apps.cc:24: using namespace ns3;
> needs documentation about what this example tries to demonstrate
Done.
>
https://codereview.appspot.com/45320043/diff/20001/src/network/examples/simpl...
> src/network/examples/simple-apps.cc:70: socketAddr.SetProtocol (1);
> document where the value "1" comes from
It's arbitrary, Documented.
>
https://codereview.appspot.com/45320043/diff/20001/src/network/utils/simple-r...
> File src/network/utils/simple-recv-app.cc (right):
>
>
https://codereview.appspot.com/45320043/diff/20001/src/network/utils/simple-r...
> src/network/utils/simple-recv-app.cc:107: { //EOF
> add debug log statement here
To be honest, it was a totally useless check. Removed.
>
https://codereview.appspot.com/45320043/diff/20001/src/network/utils/simple-r...
> src/network/utils/simple-recv-app.cc:112: if
> (PacketSocketAddress::IsMatchingType (from))
> You are incrementing the counters and firing the trace source even if the
packet
> fails this test?
Fixed.
>
https://codereview.appspot.com/45320043/diff/20001/src/network/utils/simple-s...
> File src/network/utils/simple-send-app.cc (right):
>
>
https://codereview.appspot.com/45320043/diff/20001/src/network/utils/simple-s...
> src/network/utils/simple-send-app.cc:52: MakeUintegerAccessor
> (&SimpleSendApp::m_count),
> this variable name would be better named m_maxpackets because it is not a
count
> (m_sent is the count)
Renamed.
>
https://codereview.appspot.com/45320043/diff/20001/src/network/utils/simple-s...
> src/network/utils/simple-send-app.cc:64: "Size of packets generated.",
> specify "(bytes)"
Done.
>
https://codereview.appspot.com/45320043/diff/20001/src/network/utils/simple-s...
> src/network/utils/simple-send-app.cc:68: ;
> suggest to add Tx trace source
Done.
>
https://codereview.appspot.com/45320043/diff/20001/src/network/utils/simple-s...
> src/network/utils/simple-send-app.cc:127: m_sendEvent = Simulator::Schedule
> (Seconds (0.0), &SimpleSendApp::Send, this);
> do you think users will always want to start this at time zero? Should this
> value be configurable?
See above.
>
https://codereview.appspot.com/45320043/diff/20001/src/network/utils/simple-s...
> File src/network/utils/simple-send-app.h (right):
>
>
https://codereview.appspot.com/45320043/diff/20001/src/network/utils/simple-s...
> src/network/utils/simple-send-app.h:39: * Sends packets using PacketSocket. It
> does not require (or use) IP.
> more details about the attributes (and trace sources) here, please.
Done.
> Document also that it is intended to be used for SimpleNetDevice or devices
that
> support the PacketSocket interface (which assumes Mac48Address).
Done
> Document what will happen if user sets packet size to exceed the device's MTU.
Done.
On 2014/04/12 12:58:32, Tommaso Pecorella wrote:
> On 2014/04/11 13:51:28, Tom Henderson wrote:
> > My main comment is that it isn't clear to me whether this will work on a
node
> > with multiple net devices and how multiple packet flows are successfully
> > demultiplexed at the receiver. Probably more API is needed. Here is a
> > challenge test for the test suite: create a client node with two
> > SimpleNetDevices A and B, and two send apps As and Bs. Create a server node
> > with two SimpleNetDevices C and D, and two server apps Cr and Dr. Connect
> A<->C
> > and B<->D. Have app As send some number of packets to Cr, and Bs to Dr.
> Verify
> > that Cr gets only As packets, and Dr only Bs packets.
>
> It will not work as you think. The point is: there's no L4 protocol in between
a
> PacketSocket
> and a SimpleNetDevice. As a consequence, there's no mux/demux. The only
> mux/demux is
> done by using the protocol field (if the NetDevice supports it).
This reply made me realize that I'm unsure about what this application is
supposed to be used for. It seems limited to only providing a packet generator
to sit directly on top of SimpleNetDevices, in a single interface/channel
configuration, which doesn't seem to be of much practical use.
Could you clarify existing tests and examples that you would insert this
application into, to replace any existing packet generation code?
I had thought that the main issue being addressed was that there were no
applications written that didn't require dependency on the internet module
(which is pulled in by the src/applications module), so this application could
be used to directly test devices such as Csma.
On 2014/04/12 23:30:50, Tom Henderson wrote:
> On 2014/04/12 12:58:32, Tommaso Pecorella wrote:
> > On 2014/04/11 13:51:28, Tom Henderson wrote:
> > > My main comment is that it isn't clear to me whether this will work on a
> node
> > > with multiple net devices and how multiple packet flows are successfully
> > > demultiplexed at the receiver. Probably more API is needed. Here is a
> > > challenge test for the test suite: create a client node with two
> > > SimpleNetDevices A and B, and two send apps As and Bs. Create a server
node
> > > with two SimpleNetDevices C and D, and two server apps Cr and Dr. Connect
> > A<->C
> > > and B<->D. Have app As send some number of packets to Cr, and Bs to Dr.
> > Verify
> > > that Cr gets only As packets, and Dr only Bs packets.
> >
> > It will not work as you think. The point is: there's no L4 protocol in
between
> a
> > PacketSocket
> > and a SimpleNetDevice. As a consequence, there's no mux/demux. The only
> > mux/demux is
> > done by using the protocol field (if the NetDevice supports it).
>
>
> This reply made me realize that I'm unsure about what this application is
> supposed to be used for. It seems limited to only providing a packet
generator
> to sit directly on top of SimpleNetDevices, in a single interface/channel
> configuration, which doesn't seem to be of much practical use.
>
> Could you clarify existing tests and examples that you would insert this
> application into, to replace any existing packet generation code?
>
> I had thought that the main issue being addressed was that there were no
> applications written that didn't require dependency on the internet module
> (which is pulled in by the src/applications module), so this application could
> be used to directly test devices such as Csma.
Not really only on top of SimpleNetDevices. On top of anything that can be used
with a PacketSocket.
The real reason I did this is to remove the dependency between the wave module
and the application module, which was needed because the wave test used OnOff
application....
Limited ? Yes.
Still useful ? I think so.
On 2014/04/13 11:26:46, Tommaso Pecorella wrote:
> On 2014/04/12 23:30:50, Tom Henderson wrote:
> > On 2014/04/12 12:58:32, Tommaso Pecorella wrote:
> > > On 2014/04/11 13:51:28, Tom Henderson wrote:
> > > > My main comment is that it isn't clear to me whether this will work on a
> > node
> > > > with multiple net devices and how multiple packet flows are successfully
> > > > demultiplexed at the receiver. Probably more API is needed. Here is a
> > > > challenge test for the test suite: create a client node with two
> > > > SimpleNetDevices A and B, and two send apps As and Bs. Create a server
> node
> > > > with two SimpleNetDevices C and D, and two server apps Cr and Dr.
Connect
> > > A<->C
> > > > and B<->D. Have app As send some number of packets to Cr, and Bs to Dr.
> > > Verify
> > > > that Cr gets only As packets, and Dr only Bs packets.
> > >
> > > It will not work as you think. The point is: there's no L4 protocol in
> between
> > a
> > > PacketSocket
> > > and a SimpleNetDevice. As a consequence, there's no mux/demux. The only
> > > mux/demux is
> > > done by using the protocol field (if the NetDevice supports it).
> >
> >
> > This reply made me realize that I'm unsure about what this application is
> > supposed to be used for. It seems limited to only providing a packet
> generator
> > to sit directly on top of SimpleNetDevices, in a single interface/channel
> > configuration, which doesn't seem to be of much practical use.
> >
> > Could you clarify existing tests and examples that you would insert this
> > application into, to replace any existing packet generation code?
> >
> > I had thought that the main issue being addressed was that there were no
> > applications written that didn't require dependency on the internet module
> > (which is pulled in by the src/applications module), so this application
could
> > be used to directly test devices such as Csma.
>
> Not really only on top of SimpleNetDevices. On top of anything that can be
used
> with a PacketSocket.
>
> The real reason I did this is to remove the dependency between the wave module
> and the application module, which was needed because the wave test used OnOff
> application....
> Limited ? Yes.
> Still useful ? I think so.
I agree with the use case you suggest, and there are more of these. But in this
case, should it be instead called something like "PacketSocketClient" and
"PacketSocketServer" since it is restricted in scope to PacketSocket (i.e.
naming of "Simple" seems to associate it with "SimpleNetDevice"). And if so, do
my previous comments apply (that it would be useful to be able to bind the
sender and receiver when the underlying device is not the SimpleNetDevice)?
On 2014/04/13 18:19:32, Tom Henderson wrote:
> >
> > The real reason I did this is to remove the dependency between the wave
module
> > and the application module, which was needed because the wave test used
OnOff
> > application....
> > Limited ? Yes.
> > Still useful ? I think so.
>
>
> I agree with the use case you suggest, and there are more of these. But in
this
> case, should it be instead called something like "PacketSocketClient" and
> "PacketSocketServer" since it is restricted in scope to PacketSocket (i.e.
> naming of "Simple" seems to associate it with "SimpleNetDevice"). And if so,
do
> my previous comments apply (that it would be useful to be able to bind the
> sender and receiver when the underlying device is not the SimpleNetDevice)?
I've always been bad at naming stuff... consider the renaming done.
Cheers,
T.
Issue 45320043: New Simple applications
(Closed)
Created 10 years, 4 months ago by Tommaso Pecorella
Modified 9 years, 7 months ago
Reviewers: Tom Henderson
Base URL:
Comments: 12
