Skip to content

Commit

Permalink
linux/tcp: stop async connecting when any error from socket gets
Browse files Browse the repository at this point in the history
eg for ECONNREFUSED it wakes for each iteration
  • Loading branch information
jkralik authored and Daniel Adam committed Apr 26, 2023
1 parent e24b224 commit bf6292d
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 6 deletions.
4 changes: 4 additions & 0 deletions .github/workflows/unit-test-with-cfg.yml
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@ jobs:
cmake -DCMAKE_VERBOSE_MAKEFILE=ON -DCMAKE_BUILD_TYPE=${{ inputs.build_type }} ${{ steps.cmake_flags.outputs.compiler }} ${{ inputs.build_args }} ${{ inputs.install_mbedtls && '-DBUILD_MBEDTLS=OFF' || '' }} ${{ inputs.install_tinycbor && '-DBUILD_TINYCBOR=OFF' || '' }} -DBUILD_TESTING=ON ..
make oc-unittests
- name: Setup firewall to drop TCP IPV6 with destination port 12345 (for timeout tests)
run: |
sudo ip6tables -I OUTPUT -p tcp --dport 12345 -j DROP
- name: Run unit tests
run: |
cd build
Expand Down
22 changes: 19 additions & 3 deletions port/linux/tcpsession.c
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ typedef struct tcp_waiting_session_t
oc_clock_time_t start;
uint8_t count;
uint8_t force;
int error;
} retry;
OC_LIST_STRUCT(messages);
on_tcp_connect_t on_tcp_connect;
Expand Down Expand Up @@ -1115,7 +1116,7 @@ tcp_cleanup_connected_waiting_session_locked(tcp_waiting_session_t *ws,
}

static bool
tcp_try_connect_waiting_session_locked(tcp_waiting_session_t *ws)
tcp_try_connect_waiting_session_locked(tcp_waiting_session_t *ws, int *err)
{
assert(ws != NULL && ws->sock != -1);
int error = 0;
Expand All @@ -1125,6 +1126,7 @@ tcp_try_connect_waiting_session_locked(tcp_waiting_session_t *ws)
return false; /* Solaris pending error */
}
if (error != 0) {
*err = error;
OC_ERR("socket error: %d", error);
return false;
}
Expand Down Expand Up @@ -1154,13 +1156,17 @@ enum {
TCP_WAITING_SESSION_VALID,
TCP_WAITING_SESSION_RETRY,
TCP_WAITING_SESSION_EXPIRED,
TCP_WAITING_SESSION_ERROR,
};

static int
tcp_waiting_session_check(const tcp_waiting_session_t *session,
oc_clock_time_t now,
oc_tcp_connect_retry_t connect_retry)
{
if (session->retry.error != 0) {
return TCP_WAITING_SESSION_ERROR;
}
bool retry = session->retry.force != 0;
if (!retry) {
oc_clock_time_t expires_in = connect_retry.timeout * OC_CLOCK_SECOND;
Expand Down Expand Up @@ -1247,6 +1253,9 @@ tcp_check_expiring_sessions(oc_clock_time_t now)
}
break;
}
case TCP_WAITING_SESSION_ERROR:
free_waiting_session_locked(ws, false, false);
break;
case TCP_WAITING_SESSION_EXPIRED:
free_waiting_session_locked(ws, true, false);
break;
Expand All @@ -1262,14 +1271,21 @@ tcp_check_expiring_sessions(oc_clock_time_t now)
static void
tcp_process_waiting_session_locked(tcp_waiting_session_t *ws)
{
if (!tcp_try_connect_waiting_session_locked(ws)) {
int error = 0;
if (!tcp_try_connect_waiting_session_locked(ws, &error)) {
OC_DBG("failed to connect session(%p, fd=%d)", (void *)ws, ws->sock);
if (ws->sock >= 0) {
tcp_context_cfds_fd_clr(&ws->dev->tcp, ws->sock);
close(ws->sock);
ws->sock = -1;
}
ws->retry.force = 1;
if (error == 0) {
ws->retry.force = 1;
ws->retry.error = 0;
} else {
// close the connection
ws->retry.error = 1;
}
}
}

Expand Down
5 changes: 2 additions & 3 deletions port/unittest/connectivitytest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,7 @@ TEST_F(TestConnectivityWithServer, oc_tcp_connect_timeout)
oc_endpoint_t ep = oc::endpoint::FromString(
"coaps+tcp://[::1]:12345"); // reachable address, but inactive port
// enough retries so they will run the whole duration of this test
oc_tcp_set_connect_retry(UINT8_MAX, 5);
oc_tcp_set_connect_retry(0, 5);
auto restore_defaults = []() {
oc_tcp_set_connect_retry(OC_TCP_CONNECT_RETRY_MAX_COUNT,
OC_TCP_CONNECT_RETRY_TIMEOUT);
Expand All @@ -456,8 +456,7 @@ TEST_F(TestConnectivityWithServer, oc_tcp_connect_timeout)
EXPECT_EQ(OC_SEND_MESSAGE_QUEUED, oc_send_buffer2(msg, true));

OC_DBG("oc_tcp_connect_timeout wait");
oc_tcp_set_connect_retry(0, 2);
oc::TestDevice::PoolEvents(2);
oc::TestDevice::PoolEvents(10);

EXPECT_EQ(-1, oc_tcp_connection_state(&ep));
oc_message_unref(msg);
Expand Down
3 changes: 3 additions & 0 deletions port/unittest/dnstest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ TEST_F(TestDNS, oc_dns_lookup_invalid)
static_cast<transport_flags>(0)));

oc_string_t addr;
memset(&addr, 0, sizeof(oc_string_t));
EXPECT_EQ(-1, oc_dns_lookup(nullptr, &addr, static_cast<transport_flags>(0)));

EXPECT_NE(0, oc_dns_lookup("openconnectivity", &addr,
Expand All @@ -61,6 +62,7 @@ TEST_F(TestDNS, oc_dns_lookup_invalid)
TEST_F(TestDNS, oc_dns_lookup)
{
oc_string_t addr;
memset(&addr, 0, sizeof(oc_string_t));
EXPECT_EQ(0, oc_dns_lookup("localhost", &addr, IPV6));
oc_free_string(&addr);
}
Expand All @@ -70,6 +72,7 @@ TEST_F(TestDNS, oc_dns_lookup)
TEST_F(TestDNS, oc_dns_lookup_ipv4)
{
oc_string_t addr;
memset(&addr, 0, sizeof(oc_string_t));
EXPECT_EQ(0, oc_dns_lookup("localhost", &addr, IPV4));
oc_free_string(&addr);
}
Expand Down

0 comments on commit bf6292d

Please sign in to comment.