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

Add a Transport::TCP class for tcp-based communication. #2689

Merged
merged 46 commits into from
Sep 17, 2020
Merged

Add a Transport::TCP class for tcp-based communication. #2689

Show file tree
Hide file tree
Changes from 34 commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
a3ece7d
Add Disconnect call support in transport base and tuple, since connec…
andy31415 Sep 15, 2020
5a64e0f
Remove message send port from UDP - peer address already contains a port
andy31415 Sep 15, 2020
80c881b
Add tcp transport type to peer addresses
andy31415 Sep 15, 2020
aace8a8
Create a TCP header and cpp files that compile with gn. Not yet funct…
andy31415 Sep 15, 2020
1d6c6b9
Added TCP tests. They fail since no implementation available, however…
andy31415 Sep 15, 2020
57cdb9c
Started to rename more stuff from UDP to TCP in tests
andy31415 Sep 15, 2020
5c98783
Add a buffer of active connections within tcp connection. Use templat…
andy31415 Sep 15, 2020
4fefbb2
Added an array of "Pending packets" for the TCP interface
andy31415 Sep 15, 2020
a6c8f9e
Free pending packet buffers in the TCP destructor
andy31415 Sep 15, 2020
a46519e
Start adding ability to enqueu and start connections over TCP
andy31415 Sep 15, 2020
bc911d6
More work on connections
andy31415 Sep 15, 2020
443add7
Setup for data receive
andy31415 Sep 15, 2020
8e7f6ac
Setup for data receive
andy31415 Sep 15, 2020
be7ad2c
Seeing TCP data being received
andy31415 Sep 15, 2020
1a1425f
Prepare for message receiving. CHIPConnection logic can be used as a …
andy31415 Sep 16, 2020
ef4a42c
Added logic for message receiving and parsing
andy31415 Sep 16, 2020
10fc27e
Unit tests now pass, however connections are not properly closed yet
andy31415 Sep 16, 2020
3d7a4c7
Unit tests pass
andy31415 Sep 16, 2020
517235a
Add ability to close active connections and wait for them to actually…
andy31415 Sep 16, 2020
ed3cb92
Work towards being able to close active connections
andy31415 Sep 16, 2020
6b66de4
Add close notification handling and peer close callback
andy31415 Sep 16, 2020
8b9d993
One end of the connection now is cleared. Not both ends though, which…
andy31415 Sep 16, 2020
be3d830
Update accept error handling: nothing we can do except log
andy31415 Sep 16, 2020
0e4819f
Slight comment update
andy31415 Sep 16, 2020
628359f
Update comment on what causes time wait in connections. Frustrating, …
andy31415 Sep 16, 2020
14cdba1
Merge branch 'master' into 01_implement_tcp_transport
andy31415 Sep 16, 2020
53d8fd9
Use interface from the peer address rather than from the listen socket
andy31415 Sep 16, 2020
6278737
Add interface comparison logic into peer address objects
andy31415 Sep 16, 2020
9958256
Update typo for interface id
andy31415 Sep 16, 2020
e3662fb
GN format files
andy31415 Sep 16, 2020
975c34b
Update timeouts to 5s for TCP tests, hoping qemu tests fail because o…
andy31415 Sep 16, 2020
acabcc3
Update namespacing in unit tests
andy31415 Sep 16, 2020
7c019c7
Add TCP endpoint include - seems missing as nrf complains
andy31415 Sep 16, 2020
6086423
Add check for TCP endpoint to exist within the TCP transport. This if…
andy31415 Sep 16, 2020
65b37fd
Code review updates
andy31415 Sep 17, 2020
c67cb08
Fix typo
andy31415 Sep 17, 2020
8083d21
More code review updates: add a free on error and update file descrip…
andy31415 Sep 17, 2020
287b5cd
More code review updates: ensure endpoint count is updated, add log f…
andy31415 Sep 17, 2020
a8637f0
Update one more place for used endpoint count decreasing
andy31415 Sep 17, 2020
1b60cbc
Call disconnect on session expiry
andy31415 Sep 17, 2020
3b27f33
Updated comment regarding expiry. Expiry is especially critical for T…
andy31415 Sep 17, 2020
7f5dfa4
Fix typo
andy31415 Sep 17, 2020
ab3202a
Code review updates
andy31415 Sep 17, 2020
9d9e445
More comment updates
andy31415 Sep 17, 2020
73e12e8
More comment updates
andy31415 Sep 17, 2020
da64988
Make peer address equality strict on interface id
andy31415 Sep 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/transport/BUILD.gn
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ static_library("transport") {
"SecureSession.h",
"SecureSessionMgr.cpp",
"SecureSessionMgr.h",
"TCP.cpp",
"TCP.h",
"Tuple.h",
"UDP.cpp",
"UDP.h",
Expand Down
9 changes: 8 additions & 1 deletion src/transport/Base.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ class Base : public ReferenceCounted<Base>
/**
* @brief Send a message to the specified target.
*
* On connection-oriented transports, sending a message implies connecting to it first.
andy31415 marked this conversation as resolved.
Show resolved Hide resolved
*
* @details
* This method calls <tt>chip::System::PacketBuffer::Free</tt> on
* behalf of the caller regardless of the return status.
Expand All @@ -72,7 +74,12 @@ class Base : public ReferenceCounted<Base>
*
* Generally it is expected that a transport can send to any peer from which it receives a message.
*/
virtual bool CanSendToPeer(const Transport::PeerAddress & address) = 0;
virtual bool CanSendToPeer(const PeerAddress & address) = 0;

/**
* Handle disconnection from the specified peer if currently connected to it.
*/
virtual void Disconnect(const PeerAddress & address) {}

protected:
/**
Expand Down
25 changes: 20 additions & 5 deletions src/transport/PeerAddress.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ enum class Type
kUndefined,
kUdp,
kBle,
// More constants to be added later, such as TCP
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we maintain this comment or is TCP the last constant we plan to add?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not aware of anything else we could add as chip spec. I believe a comment would be 'add more stuff here if needed' however that seems redundant as any enum would be 'add more if needed'.

kTcp,
};

/**
Expand All @@ -57,6 +57,7 @@ enum class Type
class PeerAddress
{
public:
PeerAddress() : mIPAddress(Inet::IPAddress::Any), mTransportType(Type::kUndefined), mInterface(INET_NULL_INTERFACEID) {}
PeerAddress(const Inet::IPAddress & addr, Type type) : mIPAddress(addr), mTransportType(type), mInterface(INET_NULL_INTERFACEID)
{}
PeerAddress(Type type) : mTransportType(type) {}
Expand Down Expand Up @@ -96,11 +97,15 @@ class PeerAddress

bool IsInitialized() const { return mTransportType != Type::kUndefined; }

bool operator==(const PeerAddress & other)
bool operator==(const PeerAddress & other) const
{
return (mTransportType == other.mTransportType) && (mIPAddress == other.mIPAddress) && (mPort == other.mPort);
return (mTransportType == other.mTransportType) && (mIPAddress == other.mIPAddress) && (mPort == other.mPort) &&
((mInterface == INET_NULL_INTERFACEID) || (other.mInterface == INET_NULL_INTERFACEID) ||
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we had missed the inteface compare before.

But it seems to make sense to me to have null be treated as 'any' for a compare. If we do not like that, we need more thought into how TCP determines 'an active connection already exists'.

Likely peer communication still needs some thought - we can likely compare IP address however port can be anything since it is random on connection establishment on one side.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this field should be matched exactly.

We shouldn't understand equality of addresses to mean "could these addresses send to the same destination" and we couldn't answer that question in general.

It should just mean "these addresses are identical", including zone identifier.

Weaker matches can be separate functions, not equality.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, updated.
Our unit tests currently use null interface id, so some bugs may creep up.

(mInterface == other.mInterface));
}

bool operator!=(const PeerAddress & other) const { return !(*this == other); }

/// Maximum size of an Inet address ToString format, that can hold both IPV6 and IPV4 addresses.
#ifdef INET6_ADDRSTRLEN
static constexpr size_t kInetMaxAddrLen = INET6_ADDRSTRLEN;
Expand Down Expand Up @@ -129,6 +134,14 @@ class PeerAddress
mIPAddress.ToString(ip_addr, sizeof(ip_addr));
snprintf(buf, bufSize, "UDP:%s:%d", ip_addr, mPort);
break;
case Type::kTcp:
mIPAddress.ToString(ip_addr, sizeof(ip_addr));
snprintf(buf, bufSize, "TCP:%s:%d", ip_addr, mPort);
break;
case Type::kBle:
// Note that BLE does not currently use any specific address.
snprintf(buf, bufSize, "BLE");
break;
default:
snprintf(buf, bufSize, "ERROR");
break;
Expand All @@ -146,12 +159,14 @@ class PeerAddress
{
return UDP(addr).SetPort(port).SetInterface(interface);
}
static PeerAddress TCP(const Inet::IPAddress & addr) { return PeerAddress(addr, Type::kTcp); }
static PeerAddress TCP(const Inet::IPAddress & addr, uint16_t port) { return TCP(addr).SetPort(port); }

private:
Inet::IPAddress mIPAddress;
Type mTransportType;
uint16_t mPort = CHIP_PORT; ///< Relevant for UDP data sending.
Inet::InterfaceId mInterface;
uint16_t mPort = CHIP_PORT; ///< Relevant for UDP data sending.
Inet::InterfaceId mInterface = INET_NULL_INTERFACEID;
};

} // namespace Transport
Expand Down
Loading