More changes to enable Shenango integration. #10
Conversation
…ation
Details:
- Disable DpdkDriver in CMakeLists.txt temporarily
- Remove unused method Driver::Packet::getMaxPayloadSize()
- Remove param `driver` in handleXXXPacket
- Change Driver::Packet to become a POD struct
- Change Driver::Packet::{address,priority} into params in Driver::sendPacket
- Remove the opaque Driver::Address
- Use IP packets as the common interface between transport and driver
- Extend Homa packet headers to include L4 src/dst port numbers
- Use SocketAddress (i.e., ip + port) as opposed to Driver::Address to identify
the src/dst address of a message
- Reenable DPDK in CMakeLists.txt - Initialize an ARP table at driver startup using the content of /proc/net/arp - Select eth port via the symbolic name of the network interface (e.g., eno1d1) (the current implementation uses ioctl to obtain the IP and MAC addresses of a network interface) - Add a system test for DPDK driver: test/dpdk_test.cc
- Change IpAddress from typedef to a POD type to provide some type safety - Add a third argument to Driver::receivePackets() to hold source addresses of ingress packets when the method returns - Eliminate Driver::Packet (use Homa::PacketSpec instead) - Move L4 header fields sport/dport into header prefix
The biggest challenge is to move away from the poll-based execution model previously embedded in the implementation. For example, Shenango can't afford to drive the execution of the transport by calling Transport::poll in a busy loop. Also, Shenango needs to allow users to block on a socket waiting for incoming messages. Another refactoring that results in small code changes all over the place is the Driver::Packet interface. The old interface cannot prevent the driver from modifying the payload (e.g., prepend L3 headers). This will lead to corrupted message when the packet needs to be retransmitted. List of major changes: - CHoma: provide C-bindings of the Homa APIs (Shenango is written in C) - Shenango: implement Shenango Driver, MailboxDir, and Mailbox - TransportPoller: extract poll-based execution model out of the Transport - SimpleMailboxDir: a simple reference implementation for Homa::MailboxDir - Driver::Packet: a new packet interface to eliminate the awkward PacketSpec; this is used to provide an immutable view of the packet to the transport (driver can prepend headers to the payload w/o affecting the transport) - Sender: add a couple of user-defined callbacks (Shenango currently relies on them to wake up blocking threads) - Finally, bring unit tests up-to-date.
... by combining Mailbox::close(), Mailbox::deliver() and MailboxDir::open() into MailboxDir::deliver().
yilongli
force-pushed
the
shenango-integration-rebase
branch
3 times, most recently
from
c6a886c
to
60f0dd3
Compare
yilongli
force-pushed
the
shenango-integration-rebase
branch
from
60f0dd3
to
a0838b3
Compare
There was a problem hiding this comment.
Overall, it is definitely much improved and has the separation between high-level (basic) and low-level (advanced) APIs we talked about.
The only major issue is surrounding "sockets" and "port numbers." The API has dropped the notion of sockets (compared to a previous iteration) but retains port numbers. To me it seems that without sockets, there is no reason for the transport to be aware of port numbers. Port numbers are simply a way to label a socket; without sockets it seems that there is no reason to have such a label. This is something we should discuss.
There are also a number of minor issues that I've noted but those should be easy to fix.
|
|
||
| /** | ||
| * Allocate Message that can be sent with this Transport. | ||
| * | ||
| * @param sourcePort | ||
| * @param port |
There was a problem hiding this comment.
There doesn't seem to be a notion of a "Socket" anymore. If the Tranport doesn't have sockets, does an OutMessage need to associated to a port?
There was a problem hiding this comment.
Yes, if Homa is intended to be a L4 protocol. Recall that the first four bytes of (almost) all L4 headers store the source and destination ports.
| /** | ||
| * Return the remote address from which this Message is sent. | ||
| */ | ||
| virtual SocketAddress getSourceAddress() const = 0; |
There was a problem hiding this comment.
It's odd having a "SocketAddress" when the Transport doesn't have "Sockets."
There was a problem hiding this comment.
I don't think so; a L4 transport should have the concept of socket address but that doesn't mean a transport library should implement its own socket data structure. Let's discuss.
There was a problem hiding this comment.
Thinking about it some more I think there a two separate issues surrounding port numbers and sockets.
- The port number is exposed as part of the "send" interface but not as part of the "receive" interface. If we allow sending from and to a port number, we should also allow receiving based on a port number.
- It feels strange to name things based on a "Socket" if there isn't a concept of a socket. We should either explicitly used the idea of a socket in our interfaces or we should remove the use of the term socket from our interfaces.
The biggest challenge is to move away from the poll-based execution model
previously embedded in the implementation. For example, Shenango can't
afford to drive the execution of the transport by calling Transport::poll
in a busy loop. Also, Shenango needs to allow users to block on a socket
waiting for incoming messages.
Another refactoring that results in small code changes all over the place
is the Driver::Packet interface. The old interface cannot prevent the
driver from modifying the payload (e.g., prepend L3 headers). This will
lead to corrupted message when the packet needs to be retransmitted.
List of major changes:
this is used to provide an immutable view of the packet to the transport
(driver can prepend headers to the payload w/o affecting the transport)
on them to wake up blocking threads)