-
Notifications
You must be signed in to change notification settings - Fork 673
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.
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
infiniband soft_RoCE implement #1603
base: dev
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #1603 +/- ##
==========================================
- Coverage 83.13% 83.03% -0.10%
==========================================
Files 276 279 +3
Lines 47315 47561 +246
Branches 9520 9540 +20
==========================================
+ Hits 39337 39494 +157
- Misses 7091 7177 +86
- Partials 887 890 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution. I take a quick look. Seems that the code is not formatted, and missing documents. Please take a look at https://pcapplusplus.github.io/community#contribute. Also, please try to follow the coding style with the other places in the repo.
@FlashBarryAllen in order to review this PR I'd like to know the protocol better. I did some reading and here is what I found. Can you please confirm these are good sources to understand the protocol?
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you also provide a pcap/pcapng file so it's easier to open in Wireshark?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where is the pcap file? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sry, the file is too big to upload. I provide a small file of the roce.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can extract only the packet in InfiniBandPacket.dat
and put it in a pcap file, then add the pcap file to the repo
|
Thank you @FlashBarryAllen ! Please see the PR comments above |
@FlashBarryAllen there are a few more open comments, can you please take of them before I continue to review the PR? |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where is the pcap file? 🤔
void setSolicitedEvent(bool se) const; | ||
|
||
/** | ||
* @return migreq which used to communicate migration state |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is migreq
? 🤔
bool getMigrationState() const; | ||
|
||
/** | ||
* Set migreq |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto: What is migreq
?
for (auto* curLayer = parsedPacket.getFirstLayer(); curLayer != nullptr; curLayer = curLayer->getNextLayer()) | ||
{ | ||
switch (curLayer->getProtocol()) | ||
{ | ||
case pcpp::Ethernet: | ||
{ | ||
// now let's get the Ethernet layer | ||
auto* ethernetLayer = parsedPacket.getLayerOfType<pcpp::EthLayer>(); | ||
PTF_ASSERT_NOT_NULL(ethernetLayer); | ||
break; | ||
} | ||
case pcpp::VLAN: | ||
{ | ||
// now let's get the Vlan layer | ||
auto* vlanLayer = parsedPacket.getLayerOfType<pcpp::VlanLayer>(); | ||
PTF_ASSERT_NOT_NULL(vlanLayer); | ||
break; | ||
} | ||
case pcpp::IPv4: | ||
{ | ||
// let's get the IPv4 layer | ||
auto* ipLayer = parsedPacket.getLayerOfType<pcpp::IPv4Layer>(); | ||
PTF_ASSERT_NOT_NULL(ipLayer); | ||
break; | ||
} | ||
case pcpp::UDP: | ||
{ | ||
// let's get the UDP layer | ||
auto* udpLayer = parsedPacket.getLayerOfType<pcpp::UdpLayer>(); | ||
PTF_ASSERT_NOT_NULL(udpLayer); | ||
PTF_ASSERT_EQUAL(udpLayer->getSrcPort(), 57236); | ||
PTF_ASSERT_EQUAL(udpLayer->getDstPort(), 4791); | ||
break; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All this is not needed. Please look at other tests. You can only keep the InfiniBand part
PTF_ASSERT_EQUAL(ibLayer->getSoliciteEvent(), false); | ||
PTF_ASSERT_EQUAL(ibLayer->getMigrationState(), false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of PTF_ASSERT_EQUAL
you can use PTF_ASSERT_FALSE
PTF_ASSERT_EQUAL(ibLayer->getFecn(), false); | ||
PTF_ASSERT_EQUAL(ibLayer->getBecn(), false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
pcpp::EthLayer ethLayer1(pcpp::MacAddress("02:7d:fa:01:17:40"), pcpp::MacAddress("02:7d:fa:00:10:01"), | ||
PCPP_ETHERTYPE_IP); | ||
pcpp::IPv4Layer ipLayer1(pcpp::IPv4Address("192.168.0.1"), pcpp::IPv4Address("192.168.0.2")); | ||
pcpp::UdpLayer udpLayer1(30502, 4791); | ||
pcpp::InfiniBandLayer infinibandLayer1(12, 0, 0, 0, 65535, 17, 1, 5557091); | ||
|
||
pcpp::Packet infinibandPacket1(100); | ||
PTF_ASSERT_TRUE(infinibandPacket1.addLayer(ðLayer1)); | ||
PTF_ASSERT_TRUE(infinibandPacket1.addLayer(&ipLayer1)); | ||
PTF_ASSERT_TRUE(infinibandPacket1.addLayer(&udpLayer1)); | ||
PTF_ASSERT_TRUE(infinibandPacket1.addLayer(&infinibandLayer1)); | ||
|
||
PTF_ASSERT_EQUAL(infinibandPacket1.getLayerOfType<pcpp::UdpLayer>()->getDataLen(), 20); | ||
PTF_ASSERT_NOT_NULL(infinibandPacket1.getLayerOfType<pcpp::InfiniBandLayer>()); | ||
PTF_ASSERT_EQUAL(udpLayer1.getDstPort(), 4791); | ||
PTF_ASSERT_EQUAL(infinibandLayer1.getOpcode(), 12); | ||
PTF_ASSERT_EQUAL(infinibandLayer1.getPartitionKey(), 65535); | ||
PTF_ASSERT_EQUAL(infinibandLayer1.getQueuePairNumber(), 17); | ||
PTF_ASSERT_EQUAL(infinibandLayer1.getAck(), 1); | ||
PTF_ASSERT_EQUAL(infinibandLayer1.getPacketSequenceNumber(), 5557091); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can write a test like this: https://github.com/seladb/PcapPlusPlus/blob/master/Tests/Packet%2B%2BTest/Tests/DhcpV6Tests.cpp#L123-L124
Basically what it does is:
- Create the InfiniBand layer
- Load the packet from the
InfiniBand.dat
file and get the InfiniBand layer - Assert that the raw buffer length of the layer in (1) is equal to the raw buffer length of the layer in (2)
- Assert that the raw buffer of the layer in (1) is equal to the raw buffer of the layer in (2) using
PTF_ASSERT_BUF_COMPARE
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add InfiniBandEditTest
to test all of the setter methods
Soft-RoCE is the software implementation of the RDMA transport. It provides a high-performance, low-latency communication mechanism for applications that require direct memory access (DMA) between nodes.
Key features and benefits of Soft-RoCE:
Kernel Integration: Since Red Hat Enterprise Linux 7.4, the Soft-RoCE driver has been merged into the kernel, making it a standard component of the operating system.
User-Space Driver: The user-space driver is also integrated into the rdma-core package, providing a consistent API for applications to interact with the RDMA transport.
Performance: Soft-RoCE offers high performance and low latency, making it suitable for applications that require fast, reliable communication.
Flexibility: It can be used in a variety of network topologies, including Ethernet, Infiniband, and RoCE.
Soft-RoCE is also known as RXE. This is an alternative name for the technology.
And the wireshark has supported rdma, I think our pcapp should syn this feature.