|
Bugzilla – Full Text Bug Listing |
| Summary: | Adding WifiHeader destroys data in packet. | ||
|---|---|---|---|
| Product: | ns-3 | Reporter: | Laurynas Riliskis <laurynas.riliskis> |
| Component: | network | Assignee: | ns-bugs <ns-bugs> |
| Status: | RESOLVED INVALID | ||
| Severity: | critical | CC: | tommaso.pecorella |
| Priority: | P5 | ||
| Version: | ns-3.11 | ||
| Hardware: | PC | ||
| OS: | Linux | ||
after looking into the code I realize that calling pkt1->RemoveHeader(hdr) will restore original data. However, I think that the set data should be immutable. And getting it should not interfere with headers. This is the correct behavior. A packet is "just" a data container, and the Payload is dependent on the layer it's looking at the packet. What is a header + payload for a layer is just a payload for the next layer, and so on. Hence, there's no point (even from an architectural point of view) to have a function extracting the "payload", as the very concept of payload is not defined uniquely. The CopyData function does its job, as is copying what's in the packet **including** headers and trailers. It's the layer's responsibility to extract the relevant headers or trailers if present. So, basically, you found the correct behavior. BTW, malloc ?!? Found another couple of things, before closing this:
pkt->CopyData(reinterpret_cast< uint8_t*>(buf1), sizeof(Foo));
You had to copy the data up to the packet size ( SerializedSize() ).
If you do that and you print the buffer, you'll see the header and the payload.
The packet's size will be, as a matter of fact, equal to:
sizeof(Foo) + hdr.GetSerializedSize().
T
(In reply to comment #2) > This is the correct behavior. > > A packet is "just" a data container, and the Payload is dependent on the layer > it's looking at the packet. > > What is a header + payload for a layer is just a payload for the next layer, > and so on. > Hence, there's no point (even from an architectural point of view) to have a > function extracting the "payload", as the very concept of payload is not > defined uniquely. > > The CopyData function does its job, as is copying what's in the packet > **including** headers and trailers. It's the layer's responsibility to extract > the relevant headers or trailers if present. > > So, basically, you found the correct behavior. > > BTW, malloc ?!? I understand that point of you. However, the payload is the same for the tx/rx node on the same layer. If layer under forwards up packet and header, the data in packet should be only aligned as data. Currently "data" is the returned header + data. That maybe would complicate the implementation of the packet. However, I think for the clarity, the packet should be hierarchical, the same way as the rest of simulator is e.g BasePacket, WifiMacPacket, EthernetPacket, NetworkPacket etc. Maybe I did not read carefully enough to see the point. BTW, whats from with malloc in test purpose :) ? I see your point. Then the answer is: if the lower layer is accepting from the upper layer separately the header + the payload and it's returning to the upper layer a packet containing both, then there's a design error in the layer API. This, however, can happen for a number of reasons, namely: 1) cross-layer issues and optimizations 2) bad API design in "real" systems with ns-3 replicating the interface, 3) etc. In any case the implementer should know what's the expected packet/header content at any layer, so this is usually not an big issue. About the hierarchical idea, I think that it would definitely kill the performances. Moreover the use case for such a functionality are extremely limited and (in my opinion) heavily wrong. If somebody add an header, the receiver shall do the right preprocessing before extracting the payload. Any method to drop all the headers and jump to the payload is NOT what happens for real in a network and shall be avoided as much as possible. T |
After adding header to the Packet, the previous data is destroyed. The test case shows whats happens #include <memory> #include "ns3/tos-module.h" #include "ns3/core-module.h" #include "ns3/network-module.h" using namespace ns3; using namespace std; typedef struct { int a; int b; int c; }Foo; void withoutheader(){ Foo * buf; buf = (Foo*) malloc(sizeof(Foo)); buf->a=1; buf->b=2; buf->c=3; cout << "before inserting"<<endl; cout << "a: " << buf->a << "b: " << buf->b << "c: " << buf->c << endl; Ptr<Packet> pkt = Create <Packet> (Packet(reinterpret_cast< uint8_t*>(buf), sizeof(Foo))); Foo * buf1; buf1 = (Foo*) malloc(sizeof(Foo)); pkt->CopyData(reinterpret_cast< uint8_t*>(buf1), sizeof(Foo)); cout << "After insert inserting"<<endl; cout << "a: " << buf1->a << "b: " << buf1->b << "c: " << buf1->c << endl; Ptr<Packet> pkt1 = pkt->Copy(); Foo * buf2; buf2 = (Foo*) malloc(sizeof(Foo)); buf2 = buf1; cout << "reasign buffers"<<endl; cout << " a: " << buf2->a << " b: " << buf2->b << " c: " << buf2->c << endl; pkt1->CopyData(reinterpret_cast< uint8_t*>(buf2), sizeof(Foo)); cout << "After copy"<<endl; cout << "a: " << buf2->a << " b: " << buf2->b << " c: " << buf2->c << endl; Ptr<Packet> pkt2 = pkt1->Copy(); Foo * buf3; buf3 = (Foo*) malloc(sizeof(Foo)); pkt2->CopyData(reinterpret_cast< uint8_t*>(buf3), sizeof(Foo)); cout << "After copy"<<endl; cout << "a: " << buf3->a << " b: " << buf3->b << " c: " << buf3->c << endl; } void withHeader(){ WifiMacHeader hdr; hdr.SetTypeData(); hdr.SetAddr1(Mac48Address::GetBroadcast()); hdr.SetAddr2 (Mac48Address::GetBroadcast()); hdr.SetDsNotFrom(); hdr.SetDsNotTo(); Foo * buf; buf = (Foo*) malloc(sizeof(Foo)); buf->a=1; buf->b=2; buf->c=3; cout << "before inserting"<<endl; cout << "a: " << buf->a << "b: " << buf->b << "c: " << buf->c << endl; Ptr<Packet> pkt = Create <Packet> (Packet(reinterpret_cast< uint8_t*>(buf), sizeof(Foo))); //XXX: here get data destroyed pkt->AddHeader(hdr); cout<<"Header added, where is data?"<<endl; Foo * buf1; buf1 = (Foo*) malloc(sizeof(Foo)); pkt->CopyData(reinterpret_cast< uint8_t*>(buf1), sizeof(Foo)); cout << "After insert inserting"<<endl; cout << "a: " << buf1->a << "b: " << buf1->b << "c: " << buf1->c << endl; Ptr<Packet> pkt1 = pkt->Copy(); Foo * buf2; buf2 = (Foo*) malloc(sizeof(Foo)); buf2 = buf1; cout << "reasign buffers"<<endl; cout << " a: " << buf2->a << " b: " << buf2->b << " c: " << buf2->c << endl; pkt1->CopyData(reinterpret_cast< uint8_t*>(buf2), sizeof(Foo)); cout << "After copy"<<endl; cout << "a: " << buf2->a << " b: " << buf2->b << " c: " << buf2->c << endl; Ptr<Packet> pkt2 = pkt1->Copy(); Foo * buf3; buf3 = (Foo*) malloc(sizeof(Foo)); pkt2->CopyData(reinterpret_cast< uint8_t*>(buf3), sizeof(Foo)); cout << "After copy"<<endl; cout << "a: " << buf3->a << " b: " << buf3->b << " c: " << buf3->c << endl; } int main(void) { withoutheader(); cout << "\n\nNow lets repeat with header...\\"<<endl; withHeader(); return 0; }