Skip to content

Commit

Permalink
api_msg: fix tcp_abort thread safety
Browse files Browse the repository at this point in the history
As this tcp_abort could be directly called by application thread without
locking the TCP/TP core.

This commit also reworks and fixes CI:
1) Unit tests build with format-f warnings with GCC-11
2) Reworked to use common shell scripts from both GitLab and GitHub CI
  • Loading branch information
david-cermak committed Jan 9, 2023
1 parent 46227e2 commit 10197b2
Show file tree
Hide file tree
Showing 15 changed files with 152 additions and 118 deletions.
47 changes: 4 additions & 43 deletions .github/workflows/ci-linux.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,19 +26,8 @@ jobs:
python -m pip install lxml junit-xml
wget --no-verbose ${WGET_CHECK2JUNIT_PY}
- name: Build and Run unit tests with make
run: |
export LWIPDIR=../../../../src && cd ${CONTRIB}/ports/unix/check
make -j 4 check
make clean
export EXTRA_CFLAGS="-DESP_LWIP=1" && export CC="${CC} $EXTRA_CFLAGS"
make -j 4 check
make clean
export EXTRA_CFLAGS="-DESP_LWIP=1 -DIP_FORWARD=1" && export CC="${CC} $EXTRA_CFLAGS"
make -j 4 check
make clean
export EXTRA_CFLAGS="-DESP_LWIP=1 -DIP_FORWARD=1 -DIP_NAPT=1 -DLWIP_ARCH_CC_H -include cc_esp_platform.h" && export CC="${CC} $EXTRA_CFLAGS"
make -j 4 check
- name: Build and Run unit tests with make and cmake
run: ./test/ci/unit_tests.sh

- name: Run cmake
run: mkdir build && cd build && cmake .. -G Ninja
Expand All @@ -48,38 +37,10 @@ jobs:
run: cd build && cmake --build . --target lwipdocs

- name: Validate combinations of options
run: |
cp ${CONTRIB}/examples/example_app/lwipcfg.h.example ${CONTRIB}/examples/example_app/lwipcfg.h
cd ${CONTRIB}/ports/unix/example_app
export CFLAGS="-DESP_LWIP=LWIP_NETCONN_FULLDUPLEX -DESP_LWIP_IP4_REASSEMBLY_TIMERS_ONDEMAND=ESP_LWIP -DESP_LWIP_IP6_REASSEMBLY_TIMERS_ONDEMAND=ESP_LWIP -DESP_LWIP_DNS_TIMERS_ONDEMAND=ESP_LWIP -DESP_LWIP_DHCP_FINE_TIMERS_ONDEMAND=ESP_LWIP -DESP_LWIP_IGMP_TIMERS_ONDEMAND=ESP_LWIP -DESP_LWIP_MLD6_TIMERS_ONDEMAND=ESP_LWIP -DESP_DNS=ESP_LWIP -DESP_LWIP_ARP=ESP_LWIP"
export LWIPDIR=../../../../src/
make TESTFLAGS="-Wno-documentation" -j 4
chmod +x iteropts.sh && ./iteropts.sh
- name: Build and run unit tests with cmake
run: |
export LWIP_DIR=`pwd`
cd ${CONTRIB}/ports/unix/check
mkdir build && cd build && cmake -DLWIP_DIR=`pwd`/../../../../.. .. -G Ninja
cmake --build . && ./lwip_unittests
python ${LWIP_DIR}/check2junit.py lwip_unittests.xml > ${LWIP_DIR}/unit_tests.xml
run: ./test/ci/validate_opts.sh

- name: Build and run test apps
run: |
export LWIP_DIR=`pwd` && export LWIP_CONTRIB_DIR=`pwd`/${CONTRIB}
cd test/apps
# Prepare a failing report in case we get stuck (check in no-fork mode)
python socket_linger_stress_test.py failed > ${LWIP_DIR}/socket_linger_stress_test.xml
for cfg in config_no_linger config_linger config_linger_reuse; do
cmake -DCI_BUILD=1 -DTEST_CONFIG=${cfg} -B ${cfg} -G Ninja .
cmake --build ${cfg}/
timeout 10 ./${cfg}/lwip_test_apps
python ${LWIP_DIR}/check2junit.py lwip_test_apps.xml > ${LWIP_DIR}/${cfg}.xml
done
# Run the lingering test multiple times
for run in {1..10000}; do ( timeout 10 ./config_linger/lwip_test_apps ) || exit 1 ; done
# All good, regenerate the stress test-report, since the test succeeded
python socket_linger_stress_test.py > ${LWIP_DIR}/socket_linger_stress_test.xml
run: ./test/ci/test_apps.sh

- name: Upload Test Results
if: always()
Expand Down
47 changes: 3 additions & 44 deletions .gitlab-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,36 +37,7 @@ run_unittests:
dependencies: []
script:
- *get_contrib
- export LWIPDIR=../../../../src && cd ${CONTRIB}/ports/unix/check
# build and run default lwip tests (ESP_LWIP=0!)
- make -j 4 check
# retest with ESP_LWIP patches
- make clean
- export EXTRA_CFLAGS="-DESP_LWIP=1" && export CC="${CC} $EXTRA_CFLAGS"
- make -j 4 check
# retest with IP_FORWARD enabled
- make clean
- export EXTRA_CFLAGS="-DESP_LWIP=1 -DIP_FORWARD=1" && export CC="${CC} $EXTRA_CFLAGS"
- make -j 4 check
# retest with IP_FORWARD and IP_NAPT enabled
- make clean
- export EXTRA_CFLAGS="-DESP_LWIP=1 -DIP_FORWARD=1 -DIP_NAPT=1 -DLWIP_ARCH_CC_H -include cc_esp_platform.h" && export CC="${CC} $EXTRA_CFLAGS"
- make -j 4 check
# Please uncomment the below to test IP_FORWARD/IP_NAPT tests with debug output (only ip4_route test suite will be executed)
#- make clean
#- export EXTRA_CFLAGS="-DESP_LWIP=1 -DIP_FORWARD=1 -DESP_TEST_DEBUG=1 -DIP_NAPT=1 -DLWIP_ARCH_CC_H -include cc_esp_platform.h" && export CC="${CC} $EXTRA_CFLAGS"
#- make -j 4 check

run_unittests_cmake:
stage: host_test
tags:
- host_test
script:
- *get_contrib
- *get_cmake
- cd ${CONTRIB}/ports/unix/check
- mkdir build && cd build && cmake -DLWIP_DIR=`pwd`/../../../../.. .. -G Ninja
- cmake --build . && ./lwip_unittests
- ./test/ci/unit_tests.sh

build_all:
stage: host_test
Expand All @@ -84,11 +55,7 @@ validate_opts:
- host_test
script:
- *get_contrib
- cp ${CONTRIB}/examples/example_app/lwipcfg.h.example ${CONTRIB}/examples/example_app/lwipcfg.h
- cd ${CONTRIB}/ports/unix/example_app
- export CFLAGS="-DESP_LWIP=LWIP_NETCONN_FULLDUPLEX -DESP_LWIP_IP4_REASSEMBLY_TIMERS_ONDEMAND=ESP_LWIP -DESP_LWIP_IP6_REASSEMBLY_TIMERS_ONDEMAND=ESP_LWIP -DESP_LWIP_DNS_TIMERS_ONDEMAND=ESP_LWIP -DESP_LWIP_DHCP_FINE_TIMERS_ONDEMAND=ESP_LWIP -DESP_LWIP_IGMP_TIMERS_ONDEMAND=ESP_LWIP -DESP_LWIP_MLD6_TIMERS_ONDEMAND=ESP_LWIP -DESP_DNS=ESP_LWIP"
- export LWIPDIR=../../../../src/
- chmod +x iteropts.sh && ./iteropts.sh
- ./test/ci/validate_opts.sh

run_test_apps:
stage: host_test
Expand All @@ -97,15 +64,7 @@ run_test_apps:
script:
- *get_contrib
- *get_cmake
- export LWIP_DIR=`pwd` && export LWIP_CONTRIB_DIR=`pwd`/${CONTRIB}
- cd test/apps
- for cfg in config_no_linger config_linger config_linger_reuse; do
- cmake -DCI_BUILD=1 -DTEST_CONFIG=${cfg} -B ${cfg} -G Ninja .
- cmake --build ${cfg}/
- timeout 10 ./${cfg}/lwip_test_apps
- mv lwip_test_apps.xml ${cfg}.xml
- done
- for run in {1..10000}; do ( timeout 10 ./config_linger/lwip_test_apps ) || exit 1 ; done
- ./test/ci/test_apps.sh

.add_gh_key_remote: &add_gh_key_remote |
command -v ssh-agent >/dev/null || exit 1
Expand Down
20 changes: 20 additions & 0 deletions src/api/api_msg.c
Original file line number Diff line number Diff line change
Expand Up @@ -840,6 +840,20 @@ netconn_free(struct netconn *conn)
memp_free(MEMP_NETCONN, conn);
}

#if ESP_LWIP
struct tcp_psb_msg {
struct tcpip_api_call_data call;
struct tcp_pcb *pcb;
};

static err_t tcp_do_abort(struct tcpip_api_call_data *msg)
{
struct tcp_psb_msg *pcb_msg = __containerof(msg, struct tcp_psb_msg, call);
tcp_abort(pcb_msg->pcb);
return ERR_OK;
}
#endif /* ESP_LWIP */

/**
* Delete rcvmbox and acceptmbox of a netconn and free the left-over data in
* these mboxes
Expand Down Expand Up @@ -902,7 +916,13 @@ netconn_drain(struct netconn *conn)
#endif /* ESP_LWIP */
netconn_drain(newconn);
if (newconn->pcb.tcp != NULL) {
#if ESP_LWIP
struct tcp_psb_msg pcb_msg = { 0 };
pcb_msg.pcb = newconn->pcb.tcp;
tcpip_api_call(tcp_do_abort, &pcb_msg.call);
#else
tcp_abort(newconn->pcb.tcp);
#endif /* ESP_LWIP */
newconn->pcb.tcp = NULL;
}
netconn_free(newconn);
Expand Down
1 change: 1 addition & 0 deletions test/apps/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ endif()
set (LWIP_DEFINITIONS -DLWIP_DEBUG -DLWIP_NOASSERT_ON_ERROR -DLWIP_OPTTEST_FILE)
set (LWIP_INCLUDE_DIRS
"${LWIP_DIR}/test/apps"
"${LWIP_DIR}/test/unix"
"${LWIP_CONTRIB_DIR}/ports/unix/port/include"
"${LWIP_DIR}/test/unit"
"${LWIP_DIR}/src/include"
Expand Down
16 changes: 10 additions & 6 deletions test/apps/socket_linger_stress_test.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
import sys
from junit_xml import TestSuite as ts, TestCase as tc
try:
import sys
from junit_xml import TestSuite as ts, TestCase as tc

t=tc("lingering close stress test")
if len(sys.argv) > 1 and sys.argv[1] == "failed":
t.add_failure_info("test got stuck when closing clients socket")
print(ts.to_xml_string([ts("SOCKET SO_LINGER stress test", [t])]))
t=tc("lingering close stress test")
if len(sys.argv) > 1 and sys.argv[1] == "failed":
t.add_failure_info("test got stuck when closing clients socket")
print(ts.to_xml_string([ts("SOCKET SO_LINGER stress test", [t])]))

except ImportError:
print()
20 changes: 20 additions & 0 deletions test/ci/test_apps.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
#!/bin/bash
set -e

export LWIP_DIR=`pwd`
export LWIP_CONTRIB_DIR=`pwd`/${CONTRIB}

cd test/apps
# Prepare a failing report in case we get stuck (check in no-fork mode)
python socket_linger_stress_test.py failed > ${LWIP_DIR}/socket_linger_stress_test.xml
for cfg in config_no_linger config_linger config_linger_reuse; do
cmake -DCI_BUILD=1 -DTEST_CONFIG=${cfg} -B ${cfg} -G Ninja .
cmake --build ${cfg}/
timeout 10 ./${cfg}/lwip_test_apps
[ -f check2junit.py ] &&
python ${LWIP_DIR}/check2junit.py lwip_test_apps.xml > ${LWIP_DIR}/${cfg}.xml
done
# Run the lingering test multiple times
for run in {1..10000}; do ( timeout 10 ./config_linger/lwip_test_apps ) || exit 1 ; done
# All good, regenerate the stress test-report, since the test succeeded
python socket_linger_stress_test.py > ${LWIP_DIR}/socket_linger_stress_test.xml
43 changes: 43 additions & 0 deletions test/ci/unit_tests.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
#!/bin/bash
set -e

export CI_ROOT_DIR=`pwd`
export LWIPDIR=../../../../src
export ORIG_CC=${CC}
pushd `pwd`
cd ${CONTRIB}/ports/unix/check

### with GNU Make

# build and run default lwip tests (ESP_LWIP=0!)
make clean
make -j 4 check
# retest with ESP_LWIP patches
make clean
export EXTRA_CFLAGS="-DESP_LWIP=1" && export CC="${ORIG_CC} ${EXTRA_CFLAGS}"
make -j 4 check
# retest with IP_FORWARD enabled
make clean
export EXTRA_CFLAGS="-DESP_LWIP=1 -DIP_FORWARD=1" && export CC="${ORIG_CC} ${EXTRA_CFLAGS}"
make -j 4 check
# retest with IP_FORWARD and IP_NAPT enabled
make clean
export EXTRA_CFLAGS="-DESP_LWIP=1 -DIP_FORWARD=1 -DIP_NAPT=1" && export CC="${ORIG_CC} ${EXTRA_CFLAGS}"
make -j 4 check
# Please uncomment the below to test IP_FORWARD/IP_NAPT tests with debug output (only ip4_route test suite will be executed)
make clean
export EXTRA_CFLAGS="-DESP_LWIP=1 -DIP_FORWARD=1 -DESP_TEST_DEBUG=1 -DIP_NAPT=1" && export CC="${ORIG_CC} ${EXTRA_CFLAGS}"
make -j 4 check


### with CMake
cd ${CI_ROOT_DIR}/${CONTRIB}/ports/unix/check
rm -rf build
export EXTRA_CFLAGS=""
export CC="${ORIG_CC}"
mkdir build && cd build && cmake -DLWIP_DIR=`pwd`/../../../../.. .. -G Ninja
cmake --build . && ./lwip_unittests
[ -f ${CI_ROOT_DIR}/check2junit.py ] &&
python ${CI_ROOT_DIR}/check2junit.py lwip_unittests.xml > ${CI_ROOT_DIR}/unit_tests.xml

popd
11 changes: 11 additions & 0 deletions test/ci/validate_opts.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#!/bin/bash
set -e

export CFLAGS="-I../../../../test/unix -include esp_lwipopts.h"
export LWIPDIR=../../../../src/
cp ${CONTRIB}/examples/example_app/lwipcfg.h.example ${CONTRIB}/examples/example_app/lwipcfg.h
cd ${CONTRIB}/ports/unix/example_app

make TESTFLAGS="-Wno-documentation" -j 4
chmod +x iteropts.sh
./iteropts.sh
8 changes: 4 additions & 4 deletions test/unit/cc_esp_platform.h → test/unit/arch/cc.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,6 @@
#define LWIP_ERRNO_STDINCLUDE 1
#endif

#ifdef LWIP_RAND
#define LWIP_RAND() ((u32_t)rand())
#endif

/* different handling for unit test, normally not needed */
#ifdef LWIP_NOASSERT_ON_ERROR
#define LWIP_ERROR(message, expression, handler) do { if (!(expression)) { \
Expand All @@ -85,4 +81,8 @@ typedef struct sio_status_s sio_status_t;

typedef unsigned int sys_prot_t;

#ifndef __containerof
#define __containerof(ptr, type, member) ((type *)(void *)((char *)ptr - offsetof(type, member)))
#endif

#endif /* LWIP_ARCH_CC_ESP_H */
12 changes: 6 additions & 6 deletions test/unit/core/test_pbuf.c
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ START_TEST(test_pbuf_take_at_edge)

out = (u8_t*)p->payload;
for (i = 0; i < (int)sizeof(testdata); i++) {
fail_unless(out[i] == testdata[i],
ck_assert_msg(out[i] == testdata[i],
"Bad data at pos %d, was %02X, expected %02X", i, out[i], testdata[i]);
}

Expand All @@ -204,11 +204,11 @@ START_TEST(test_pbuf_take_at_edge)
fail_unless(res == ERR_OK);

out = (u8_t*)p->payload;
fail_unless(out[p->len - 1] == testdata[0],
ck_assert_msg(out[p->len - 1] == testdata[0],
"Bad data at pos %d, was %02X, expected %02X", p->len - 1, out[p->len - 1], testdata[0]);
out = (u8_t*)q->payload;
for (i = 1; i < (int)sizeof(testdata); i++) {
fail_unless(out[i-1] == testdata[i],
ck_assert_msg(out[i-1] == testdata[i],
"Bad data at pos %d, was %02X, expected %02X", p->len - 1 + i, out[i-1], testdata[i]);
}

Expand All @@ -218,7 +218,7 @@ START_TEST(test_pbuf_take_at_edge)

out = (u8_t*)p->payload;
for (i = 0; i < (int)sizeof(testdata); i++) {
fail_unless(out[i] == testdata[i],
ck_assert_msg(out[i] == testdata[i],
"Bad data at pos %d, was %02X, expected %02X", p->len+i, out[i], testdata[i]);
}
pbuf_free(p);
Expand All @@ -245,11 +245,11 @@ START_TEST(test_pbuf_get_put_at_edge)
pbuf_put_at(p, p->len, testdata);

out = (u8_t*)q->payload;
fail_unless(*out == testdata,
ck_assert_msg(*out == testdata,
"Bad data at pos %d, was %02X, expected %02X", p->len, *out, testdata);

getdata = pbuf_get_at(p, p->len);
fail_unless(*out == getdata,
ck_assert_msg(*out == getdata,
"pbuf_get_at() returned bad data at pos %d, was %02X, expected %02X", p->len, getdata, *out);
pbuf_free(p);
}
Expand Down
Loading

0 comments on commit 10197b2

Please sign in to comment.