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.

Sign up for GitHub

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

Jump to bottom

Issues about doParsimPacking() and doParsimUnpacking() for TcpOption in TcpHeader_m.h #961

Closed
kalsasdf opened this issue Mar 20, 2024 · 2 comments
Closed

Issues about doParsimPacking() and doParsimUnpacking() for TcpOption in TcpHeader_m.h #961

kalsasdf opened this issue Mar 20, 2024 · 2 comments
Assignees
@ZoltanBojthe
Labels
Bug Fixed
Milestone
INET 4.6

Comments

@kalsasdf
Copy link

Issue Description:

In the TcpHeader_m.h file, there exists an inconsistency regarding the definition of the doParsimPacking() and doParsimUnpacking() functions for the TcpOption class, which mismatches the way TcpOption is used in the TcpHeader class.

The issue arises from the fact that the doParsimPacking() and doParsimUnpacking() functions expect TcpOption& as parameters, while in the TcpHeader class, TcpOption objects are defined as a two-level pointer, TcpOption * *headerOption = nullptr.

Consequently, when dereferencing in the doParsimArrayPacking() function in TcpHeader_m.cc,
1710916650656

the TcpOption object cannot locate the correct doParsimPacking() and doParsimUnpacking() functions, resulting in the error "Parsim error: No doParsimPacking() function for type inet::tcp::TcpOption *".
1710916696935

We propose an improvement method as illustrated below.
1710916753579 in TcpHeader::parsimPack(omnetpp::cCommBuffer *b)
1710916813693
in TcpHeader::parsimUnpack(omnetpp::cCommBuffer *b)

However, the issue is that TcpHeader_m.cc and TcpHeader_m.h are generated by TcpHeader.msg and would be cleared during make clean. As a result, our modifications will also be wiped out. Moreover, TcpHeader.msg does not directly relate to the doParsimPacking() and doParsimUnpacking() functions, making it impossible for us to amend TcpHeader.msg to generate these functions correctly.

Therefore, we suggest submitting this issue to the official INET for further enhancement.
@levy levy added the Bug label Mar 27, 2024
@kalsasdf
Copy link
Author

Supplementary:
The above method enables the normal packing and unpacking of the TcpOption field. However, further testing revealed that this modification affects the subsequent fields' ability to pack and unpack correctly, like EthernetPadding in SYN. Therefore, we switched to the following approach, which effectively bypasses the inline packing and unpacking functions (doParsimPacking() and doParsimUnpacking()) of TcpOption. Instead, we directly call the parsimPack and parsimUnpack functions within the TcpOption class.

1711866010254
in TcpHeader::parsimPack(omnetpp::cCommBuffer *b)
1711866021708
in TcpHeader::parsimUnpack(omnetpp::cCommBuffer *b)

@levy levy added this to the INET 4.6 milestone Dec 2, 2024
ZoltanBojthe added a commit that referenced this issue Dec 17, 2024
@ZoltanBojthe
Tcp: fixed TcpHeader parsim/unparsim (issue #961)
5236543 
Verified with next code in Tcp::handleLowerPacket():

{
    cCommBuffer *b = check_and_cast<cCommBuffer *>(createOne("omnetpp::cMemCommBuffer"));
    tcpHeader.get()->parsimPack(b);
    auto h = makeShared<TcpHeader>();
    h->parsimUnpack(b);
    ASSERT(h->getChunkLength() == tcpHeader->getChunkLength());
    ASSERT(h->getHeaderOptionArrayLength() == tcpHeader->getHeaderOptionArrayLength());
    MemoryOutputStream s1, s2;
    Chunk::serialize(s1, tcpHeader);
    Chunk::serialize(s2, h);
    ASSERT(s1.getData() == s2.getData());
}
@ZoltanBojthe
Copy link
Contributor

Fixed in inet master (for INET 4.6)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ZoltanBojthe ZoltanBojthe

Labels
Bug Fixed
Projects
None yet
Milestone
INET 4.6
Development

No branches or pull requests
3 participants
@levy @ZoltanBojthe @kalsasdf