diff --git a/dhcpv6.c b/dhcpv6.c index c1a27aba..e4df0db5 100644 --- a/dhcpv6.c +++ b/dhcpv6.c @@ -550,10 +550,18 @@ int dhcpv6(struct ctx *c, struct iov_tail *data, { const struct opt_server_id *server_id = NULL; const struct opt_hdr *client_id = NULL; + /* The _storage variables can't be local to the blocks they're used in, + * because IOV_*_HEADER() may return pointers to them which are + * dereferenced afterwards. Since we don't have Rust-like lifetime + * tracking, cppcheck can't reasonably determine that, so we must + * suppress its warnings. */ + /* cppcheck-suppress [variableScope,unmatchedSuppression] */ struct opt_server_id server_id_storage; struct iov_tail opt, client_id_base; const struct opt_ia_na *ia = NULL; + /* cppcheck-suppress [variableScope,unmatchedSuppression] */ struct opt_hdr client_id_storage; + /* cppcheck-suppress [variableScope,unmatchedSuppression] */ struct opt_ia_na ia_storage; const struct in6_addr *src; struct msg_hdr mh_storage; diff --git a/linux_dep.h b/linux_dep.h index 1d9e1669..89e590c8 100644 --- a/linux_dep.h +++ b/linux_dep.h @@ -142,7 +142,7 @@ struct tcp_info_linux { #endif __attribute__ ((weak)) -/* cppcheck-suppress funcArgNamesDifferent */ +/* cppcheck-suppress [funcArgNamesDifferent,unmatchedSuppression] */ int close_range(unsigned int first, unsigned int last, int flags) { return syscall(SYS_close_range, first, last, flags); } diff --git a/tcp.c b/tcp.c index 48b1ef29..91aa3301 100644 --- a/tcp.c +++ b/tcp.c @@ -1030,6 +1030,11 @@ int tcp_update_seqack_wnd(const struct ctx *c, struct tcp_tap_conn *conn, return 0; } + /* This trips a cppcheck bug in some versions, including + * cppcheck 2.18.3. + * https://sourceforge.net/p/cppcheck/discussion/general/thread/fecde59085/ + */ + /* cppcheck-suppress [uninitvar,unmatchedSuppression] */ conn->seq_ack_to_tap = tinfo->tcpi_bytes_acked + conn->seq_init_from_tap; @@ -1769,7 +1774,7 @@ static int tcp_data_from_tap(const struct ctx *c, struct tcp_tap_conn *conn, } } - if (th->fin) + if (th->fin && seq == seq_from_tap) fin = 1; if (!len) @@ -1876,7 +1881,7 @@ static int tcp_data_from_tap(const struct ctx *c, struct tcp_tap_conn *conn, } out: - if (keep != -1) { + if (keep != -1 || partial_send) { /* We use an 8-bit approximation here: the associated risk is * that we skip a duplicate ACK on 8-bit sequence number * collision. Fast retransmit is a SHOULD in RFC 5681, 3.2. @@ -2130,9 +2135,15 @@ int tcp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af, /* Established connections not accepting data from tap */ if (conn->events & TAP_FIN_RCVD) { + size_t dlen; bool retr; - retr = th->ack && !tcp_packet_data_len(th, l4len) && !th->fin && + if ((dlen = tcp_packet_data_len(th, l4len))) { + flow_dbg(conn, "data segment in CLOSE-WAIT (%zu B)", + dlen); + } + + retr = th->ack && !th->fin && ntohl(th->ack_seq) == conn->seq_ack_from_tap && ntohs(th->window) == conn->wnd_from_tap; diff --git a/tcp_buf.c b/tcp_buf.c index a493b5a0..cc106bc6 100644 --- a/tcp_buf.c +++ b/tcp_buf.c @@ -368,7 +368,19 @@ int tcp_buf_data_from_sock(const struct ctx *c, struct tcp_tap_conn *conn) conn_flag(c, conn, STALLED); } else if ((conn->events & (SOCK_FIN_RCVD | TAP_FIN_SENT)) == SOCK_FIN_RCVD) { - int ret = tcp_buf_send_flag(c, conn, FIN | ACK); + int ret; + + /* On TAP_FIN_SENT, we won't get further data events + * from the socket, and this might be the last ACK + * segment we send to the tap, so update its sequence to + * include everything we received until now. + * + * See also the special handling on CONN_IS_CLOSING() in + * tcp_update_seqack_wnd(). + */ + conn->seq_ack_to_tap = conn->seq_from_tap; + + ret = tcp_buf_send_flag(c, conn, FIN | ACK); if (ret) { tcp_rst(c, conn); return ret; diff --git a/tcp_vu.c b/tcp_vu.c index ebd3a1ed..3ec3538d 100644 --- a/tcp_vu.c +++ b/tcp_vu.c @@ -410,7 +410,12 @@ int tcp_vu_data_from_sock(const struct ctx *c, struct tcp_tap_conn *conn) conn_flag(c, conn, STALLED); } else if ((conn->events & (SOCK_FIN_RCVD | TAP_FIN_SENT)) == SOCK_FIN_RCVD) { - int ret = tcp_vu_send_flag(c, conn, FIN | ACK); + int ret; + + /* See tcp_buf_data_from_sock() */ + conn->seq_ack_to_tap = conn->seq_from_tap; + + ret = tcp_vu_send_flag(c, conn, FIN | ACK); if (ret) { tcp_rst(c, conn); return ret; diff --git a/test/Makefile b/test/Makefile index 49388276..5b5f0fc1 100644 --- a/test/Makefile +++ b/test/Makefile @@ -8,6 +8,8 @@ BATS = bats -j $(shell nproc) EXETOOL = exeter/exetool/exetool WGET = wget -c +FLAKE8 = flake8 +MYPY = mypy --strict DEBIAN_IMGS = debian-8.11.0-openstack-amd64.qcow2 \ debian-10-nocloud-amd64.qcow2 \ @@ -52,24 +54,27 @@ UBUNTU_NEW_IMGS = xenial-server-cloudimg-powerpc-disk1.img \ jammy-server-cloudimg-s390x.img UBUNTU_IMGS = $(UBUNTU_OLD_IMGS) $(UBUNTU_NEW_IMGS) -DOWNLOAD_ASSETS = exeter mbuto podman \ +DOWNLOAD_ASSETS = $(EXETOOL) mbuto podman \ $(DEBIAN_IMGS) $(FEDORA_IMGS) $(OPENSUSE_IMGS) $(UBUNTU_IMGS) TESTDATA_ASSETS = small.bin big.bin medium.bin \ rampstream LOCAL_ASSETS = mbuto.img mbuto.mem.img podman/bin/podman QEMU_EFI.fd \ $(DEBIAN_IMGS:%=prepared-%) $(FEDORA_IMGS:%=prepared-%) \ $(UBUNTU_NEW_IMGS:%=prepared-%) \ - nstool guest-key guest-key.pub $(EXETOOL) \ - $(TESTDATA_ASSETS) + nstool guest-key guest-key.pub $(TESTDATA_ASSETS) ASSETS = $(DOWNLOAD_ASSETS) $(LOCAL_ASSETS) EXETER_PYPATH = exeter/py3 +EXETER_PYTHON = build/build.py EXETER_BATS = smoke/smoke.sh.bats \ - build/build.py.bats build/static_checkers.sh.bats + $(EXETER_PYTHON:%=%.bats) build/static_checkers.sh.bats BATS_FILES = $(EXETER_BATS) \ podman/test/system/505-networking-pasta.bats +# Python test code (for linters) +PYPKGS = $(EXETER_PYTHON) + CFLAGS = -Wall -Werror -Wextra -pedantic -std=c99 assets: $(ASSETS) @@ -128,6 +133,12 @@ medium.bin: big.bin: dd if=/dev/urandom bs=1M count=10 of=$@ +flake8: pull-exeter + PYTHONPATH=$(EXETER_PYPATH) $(FLAKE8) $(PYPKGS) + +mypy: pull-exeter + PYTHONPATH=$(EXETER_PYPATH) $(MYPY) $(PYPKGS) + $(EXETER_BATS): %.bats: % $(EXETOOL) PYTHONPATH=$(EXETER_PYPATH) $(EXETOOL) bats -- $< > $@ @@ -142,6 +153,7 @@ debug: assets clean: rm -f perf.js *~ + rm -rf .mypy_cache rm -f $(LOCAL_ASSETS) rm -f $(EXETER_BATS) rm -rf test_logs diff --git a/test/build/build.py b/test/build/build.py index e49287c9..e3de8305 100755 --- a/test/build/build.py +++ b/test/build/build.py @@ -18,11 +18,12 @@ from pathlib import Path import subprocess import tempfile -from typing import Iterable, Iterator +from typing import Iterator import exeter -def sh(cmd): + +def sh(cmd: str) -> None: """Run given command in a shell""" subprocess.run(cmd, shell=True) diff --git a/test/build/static_checkers.sh b/test/build/static_checkers.sh index 42806e79..228b99ae 100755 --- a/test/build/static_checkers.sh +++ b/test/build/static_checkers.sh @@ -21,6 +21,10 @@ exeter_set_description cppcheck "passt sources pass cppcheck" exeter_register clang_tidy make -C .. clang-tidy exeter_set_description clang_tidy "passt sources pass clang-tidy" -exeter_main "$@" +exeter_register flake8 make flake8 +exeter_set_description flake8 "passt tests in Python pass flake8" +exeter_register mypy make mypy +exeter_set_description mypy "passt tests in Python pass mypy --strict" +exeter_main "$@" diff --git a/vhost_user.c b/vhost_user.c index fa343a86..223332d5 100644 --- a/vhost_user.c +++ b/vhost_user.c @@ -939,6 +939,7 @@ static bool vu_get_queue_num_exec(struct vu_dev *vdev, { (void)vdev; + /* NOLINTNEXTLINE(misc-redundant-expression) */ vmsg_set_reply_u64(vmsg, VHOST_USER_MAX_VQS / 2); debug("VHOST_USER_MAX_VQS %u", VHOST_USER_MAX_VQS / 2);