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.

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

Include interface ID in the PeerAddress that gets handed up from UDP/TCP code #7389

Merged
merged 2 commits into from
Jun 4, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
61 changes: 61 additions & 0 deletions src/inet/TCPEndPoint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -696,6 +696,67 @@ INET_ERROR TCPEndPoint::GetLocalInfo(IPAddress * retAddr, uint16_t * retPort)
return res;
}

INET_ERROR TCPEndPoint::GetInterfaceId(InterfaceId * retInterface)
{
if (!IsConnected())
return INET_ERROR_INCORRECT_STATE;

#if CHIP_SYSTEM_CONFIG_USE_LWIP
// TODO: Does netif_get_by_index(mTCP->netif_idx) do the right thing? I
// can't quite tell whether LwIP supports a specific interface id for TCP at
// all. For now just claim no particular interface id.
*retInterface = INET_NULL_INTERFACEID;
return INET_NO_ERROR;
#endif // CHIP_SYSTEM_CONFIG_USE_LWIP

#if CHIP_SYSTEM_CONFIG_USE_SOCKETS
union
{
sockaddr any;
sockaddr_in6 in6;
#if INET_CONFIG_ENABLE_IPV4
sockaddr_in in;
#endif // INET_CONFIG_ENABLE_IPV4
} sa;

memset(&sa, 0, sizeof(sa));
socklen_t saLen = sizeof(sa);

if (getpeername(mSocket, &sa.any, &saLen) != 0)
{
return chip::System::MapErrorPOSIX(errno);
}

if (sa.any.sa_family == AF_INET6)
{
if (IPAddress::FromIPv6(sa.in6.sin6_addr).IsIPv6LinkLocal())
{
*retInterface = sa.in6.sin6_scope_id;
}
else
{
// TODO: Is there still a meaningful interface id in this case?
*retInterface = INET_NULL_INTERFACEID;
Copy link
Contributor

Choose a reason for hiding this comment

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

As far as I know, sin6_scope_id will only be an interface ID for link local addresses. So in short, no, I don't believe an interface ID can be extracted for addresses other than link-local. And I can't think of a reason why we would need this for non-link-local addresses. In theory we could bind to a specific interface at which we know a device may reach us. But I don't know why we would want to or need to.

As to the other possible overloaded meanings of sin6_scope_id (zones or site indices?), it's hard to imagine these would be useful to us, or could even be dealt with in a portable way. I don't know much about those though, except that they are not interface indices.

TLDR: It looks to me like the code is doing the correct thing here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for looking into this! This part I was really not sure about, as you can tell from the comments. ;)

}
return INET_NO_ERROR;
}

#if INET_CONFIG_ENABLE_IPV4
if (sa.any.sa_family == AF_INET)
{
// No interface id available for IPv4 sockets.
*retInterface = INET_NULL_INTERFACEID;
}
#endif // INET_CONFIG_ENABLE_IPV4

return INET_ERROR_INCORRECT_STATE;

#endif // CHIP_SYSTEM_CONFIG_USE_SOCKETS

*retInterface = INET_NULL_INTERFACEID;
return INET_NO_ERROR;
}

INET_ERROR TCPEndPoint::Send(System::PacketBufferHandle && data, bool push)
{
INET_ERROR res = INET_NO_ERROR;
Expand Down
11 changes: 11 additions & 0 deletions src/inet/TCPEndPoint.h
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,17 @@ class DLL_EXPORT TCPEndPoint : public EndPointBasis
*/
INET_ERROR GetLocalInfo(IPAddress * retAddr, uint16_t * retPort);

/**
* @brief Extract the interface id of the TCP endpoint.
*
* @param[out] retInterface The interface id.
*
* @retval INET_NO_ERROR success: address and port extracted.
* @retval INET_ERROR_INCORRECT_STATE TCP connection not established.
* @retval INET_ERROR_CONNECTION_ABORTED TCP connection no longer open.
*/
INET_ERROR GetInterfaceId(InterfaceId * retInterface);

/**
* @brief Send message text on TCP connection.
*
Expand Down
4 changes: 4 additions & 0 deletions src/transport/raw/PeerAddress.h
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,10 @@ class PeerAddress
}
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); }
static PeerAddress TCP(const Inet::IPAddress & addr, uint16_t port, Inet::InterfaceId interface)
{
return TCP(addr).SetPort(port).SetInterface(interface);
}

private:
Inet::IPAddress mIPAddress;
Expand Down
12 changes: 9 additions & 3 deletions src/transport/raw/TCP.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -356,9 +356,11 @@ INET_ERROR TCPBase::OnTcpReceive(Inet::TCPEndPoint * endPoint, System::PacketBuf
{
Inet::IPAddress ipAddress;
uint16_t port;
Inet::InterfaceId interfaceId = INET_NULL_INTERFACEID;

endPoint->GetPeerInfo(&ipAddress, &port);
PeerAddress peerAddress = PeerAddress::TCP(ipAddress, port);
endPoint->GetInterfaceId(&interfaceId);
PeerAddress peerAddress = PeerAddress::TCP(ipAddress, port, interfaceId);

TCPBase * tcp = reinterpret_cast<TCPBase *>(endPoint->AppState);
CHIP_ERROR err = tcp->ProcessReceivedBuffer(endPoint, peerAddress, std::move(buffer));
Expand All @@ -379,9 +381,11 @@ void TCPBase::OnConnectionComplete(Inet::TCPEndPoint * endPoint, INET_ERROR inet
TCPBase * tcp = reinterpret_cast<TCPBase *>(endPoint->AppState);
Inet::IPAddress ipAddress;
uint16_t port;
Inet::InterfaceId interfaceId = INET_NULL_INTERFACEID;

endPoint->GetPeerInfo(&ipAddress, &port);
PeerAddress addr = PeerAddress::TCP(ipAddress, port);
endPoint->GetInterfaceId(&interfaceId);
PeerAddress addr = PeerAddress::TCP(ipAddress, port, interfaceId);

// Send any pending packets
for (size_t i = 0; i < tcp->mPendingPacketsSize; i++)
Expand Down Expand Up @@ -507,9 +511,11 @@ void TCPBase::Disconnect(const PeerAddress & address)
{
Inet::IPAddress ipAddress;
uint16_t port;
Inet::InterfaceId interfaceId = INET_NULL_INTERFACEID;

mActiveConnections[i].mEndPoint->GetPeerInfo(&ipAddress, &port);
if (address == PeerAddress::TCP(ipAddress, port))
mActiveConnections[i].mEndPoint->GetInterfaceId(&interfaceId);
if (address == PeerAddress::TCP(ipAddress, port, interfaceId))
{
// NOTE: this leaves the socket in TIME_WAIT.
// Calling Abort() would clean it since SO_LINGER would be set to 0,
Expand Down
2 changes: 1 addition & 1 deletion src/transport/raw/UDP.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ void UDP::OnUdpReceive(Inet::IPEndPointBasis * endPoint, System::PacketBufferHan
{
CHIP_ERROR err = CHIP_NO_ERROR;
UDP * udp = reinterpret_cast<UDP *>(endPoint->AppState);
PeerAddress peerAddress = PeerAddress::UDP(pktInfo->SrcAddress, pktInfo->SrcPort);
PeerAddress peerAddress = PeerAddress::UDP(pktInfo->SrcAddress, pktInfo->SrcPort, pktInfo->Interface);

udp->HandleMessageReceived(peerAddress, std::move(buffer));

Expand Down