passt: Add cppcheck target, test, and address resulting warnings

...mostly false positives, but a number of very relevant ones too,
in tcp_get_sndbuf(), tcp_conn_from_tap(), and siphash PREAMBLE().

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
This commit is contained in:
Stefano Brivio 2021-10-21 09:41:13 +02:00
parent c3f8e4d2cd
commit 627e18fa8a
17 changed files with 159 additions and 118 deletions

View file

@ -141,7 +141,7 @@ pkgs:
# #
# - bugprone-suspicious-string-compare # - bugprone-suspicious-string-compare
# Return value of memcmp(), not really suspicious # Return value of memcmp(), not really suspicious
clang-tidy: $(wildcard *.c) clang-tidy: $(wildcard *.c) $(wildcard *.h)
clang-tidy -checks=*,-modernize-*,\ clang-tidy -checks=*,-modernize-*,\
-clang-analyzer-valist.Uninitialized,\ -clang-analyzer-valist.Uninitialized,\
-cppcoreguidelines-init-variables,\ -cppcoreguidelines-init-variables,\
@ -163,3 +163,33 @@ clang-tidy: $(wildcard *.c)
-cppcoreguidelines-avoid-non-const-global-variables,\ -cppcoreguidelines-avoid-non-const-global-variables,\
-bugprone-suspicious-string-compare \ -bugprone-suspicious-string-compare \
--warnings-as-errors=* $(wildcard *.c) -- $(CFLAGS) --warnings-as-errors=* $(wildcard *.c) -- $(CFLAGS)
ifeq ($(shell $(CC) -v 2>&1 | grep -c "gcc version"),1)
TARGET := $(shell ${CC} -v 2>&1 | sed -n 's/Target: \(.*\)/\1/p')
VER := $(shell $(CC) -dumpversion)
EXTRA_INCLUDES := /usr/lib/gcc/$(TARGET)/$(VER)/include
EXTRA_INCLUDES_OPT := -I$(EXTRA_INCLUDES)
else
EXTRA_INCLUDES_OPT :=
endif
cppcheck: $(wildcard *.c) $(wildcard *.h)
cppcheck --std=c99 --error-exitcode=1 --enable=all --force \
--inconclusive --library=posix \
-I/usr/include $(EXTRA_INCLUDES_OPT) \
--suppress=missingIncludeSystem \
--suppress="*:$(EXTRA_INCLUDES)/avx512fintrin.h" \
--suppress="*:$(EXTRA_INCLUDES)/xmmintrin.h" \
--suppress="*:$(EXTRA_INCLUDES)/emmintrin.h" \
--suppress="*:$(EXTRA_INCLUDES)/avxintrin.h" \
--suppress="*:$(EXTRA_INCLUDES)/bmiintrin.h" \
--suppress=objectIndex:tcp.c --suppress=objectIndex:udp.c \
--suppress=va_list_usedBeforeStarted:util.c \
--suppress=unusedFunction:igmp.c \
--suppress=unusedFunction:siphash.c \
--suppress=knownConditionTrueFalse:conf.c \
--suppress=strtokCalled:conf.c --suppress=strtokCalled:qrap.c \
--suppress=getpwnamCalled:passt.c \
--suppress=localtimeCalled:pcap.c \
--suppress=unusedStructMember:pcap.c \
--suppress=funcArgNamesDifferent:util.h \
.

View file

@ -167,8 +167,8 @@ static uint32_t csum_avx2(const void *buf, size_t len, uint32_t init)
{ {
__m256i a, b, sum256, sum_a_hi, sum_a_lo, sum_b_hi, sum_b_lo, c, d; __m256i a, b, sum256, sum_a_hi, sum_a_lo, sum_b_hi, sum_b_lo, c, d;
__m256i __sum_a_hi, __sum_a_lo, __sum_b_hi, __sum_b_lo; __m256i __sum_a_hi, __sum_a_lo, __sum_b_hi, __sum_b_lo;
const uint64_t *buf64 = (const uint64_t *)buf; const __m256i *buf256 = (const __m256i *)buf;
const __m256i *buf256; const uint64_t *buf64;
const uint16_t *buf16; const uint16_t *buf16;
uint64_t sum64 = init; uint64_t sum64 = init;
int odd = len & 1; int odd = len & 1;
@ -176,7 +176,6 @@ static uint32_t csum_avx2(const void *buf, size_t len, uint32_t init)
__m256i zero; __m256i zero;
zero = _mm256_setzero_si256(); zero = _mm256_setzero_si256();
buf256 = (const __m256i *)buf64;
if (len < sizeof(__m256i) * 4) if (len < sizeof(__m256i) * 4)
goto less_than_128_bytes; goto less_than_128_bytes;
@ -267,7 +266,6 @@ static uint32_t csum_avx2(const void *buf, size_t len, uint32_t init)
/* Fold 128-bit sum into 64 bits. */ /* Fold 128-bit sum into 64 bits. */
sum64 += _mm_extract_epi64(sum128, 0) + _mm_extract_epi64(sum128, 1); sum64 += _mm_extract_epi64(sum128, 0) + _mm_extract_epi64(sum128, 1);
buf64 = (const uint64_t *)buf256;
less_than_128_bytes: less_than_128_bytes:
for (; len >= sizeof(a); len -= sizeof(a), buf256++) { for (; len >= sizeof(a); len -= sizeof(a), buf256++) {

6
conf.c
View file

@ -678,7 +678,7 @@ pasta_opts:
void conf_print(struct ctx *c) void conf_print(struct ctx *c)
{ {
char buf6[INET6_ADDRSTRLEN], buf4[INET_ADDRSTRLEN], ifn[IFNAMSIZ]; char buf4[INET_ADDRSTRLEN], ifn[IFNAMSIZ];
int i; int i;
if (c->mode == MODE_PASTA) { if (c->mode == MODE_PASTA) {
@ -723,6 +723,8 @@ void conf_print(struct ctx *c)
} }
if (c->v6) { if (c->v6) {
char buf6[INET6_ADDRSTRLEN];
if (!c->no_ndp && !c->no_dhcpv6) if (!c->no_ndp && !c->no_dhcpv6)
info("NDP/DHCPv6:"); info("NDP/DHCPv6:");
else if (!c->no_ndp) else if (!c->no_ndp)
@ -1013,7 +1015,7 @@ void conf(struct ctx *c, int argc, char **argv)
errno = 0; errno = 0;
mask = strtol(optarg, NULL, 0); mask = strtol(optarg, NULL, 0);
if (mask >= 0 && mask <= 32 && !errno) { if (mask > 0 && mask <= 32 && !errno) {
c->mask4 = htonl(0xffffffff << (32 - mask)); c->mask4 = htonl(0xffffffff << (32 - mask));
break; break;
} }

13
dhcp.c
View file

@ -212,19 +212,20 @@ static void opt_set_dns_search(struct ctx *c, size_t max_len)
for (i = 0; *c->dns_search[i].n; i++) { for (i = 0; *c->dns_search[i].n; i++) {
unsigned int n; unsigned int n;
int dup = -1; int count = -1;
char *p; char *p;
buf[0] = 0; buf[0] = 0;
for (p = c->dns_search[i].n, n = 1; *p; p++) { for (p = c->dns_search[i].n, n = 1; *p; p++) {
if (*p == '.') { if (*p == '.') {
/* RFC 1035 4.1.4 Message compression */ /* RFC 1035 4.1.4 Message compression */
dup = opt_dns_search_dup_ptr(opts[119].s, p + 1, count = opt_dns_search_dup_ptr(opts[119].s,
opts[119].slen); p + 1,
opts[119].slen);
if (dup >= 0) { if (count >= 0) {
buf[n++] = '\xc0'; buf[n++] = '\xc0';
buf[n++] = dup; buf[n++] = count;
break; break;
} }
buf[n++] = '.'; buf[n++] = '.';
@ -234,7 +235,7 @@ static void opt_set_dns_search(struct ctx *c, size_t max_len)
} }
/* The compression pointer is also an end of label */ /* The compression pointer is also an end of label */
if (dup < 0) if (count < 0)
buf[n++] = 0; buf[n++] = 0;
if (n >= max_len) if (n >= max_len)

8
ndp.c
View file

@ -91,7 +91,7 @@ int ndp(struct ctx *c, struct ethhdr *eh, size_t len)
memcpy(p, c->mac, ETH_ALEN); memcpy(p, c->mac, ETH_ALEN);
p += 6; p += 6;
} else if (ih->icmp6_type == RS) { } else if (ih->icmp6_type == RS) {
size_t len = 0; size_t dns_s_len = 0;
int i, n; int i, n;
if (c->no_ra) if (c->no_ra)
@ -139,7 +139,7 @@ int ndp(struct ctx *c, struct ethhdr *eh, size_t len)
} }
for (n = 0; *c->dns_search[n].n; n++) for (n = 0; *c->dns_search[n].n; n++)
len += strlen(c->dns_search[n].n) + 2; dns_s_len += strlen(c->dns_search[n].n) + 2;
if (len) { if (len) {
*p++ = 31; /* DNSSL */ *p++ = 31; /* DNSSL */
*p++ = (len + 8 - 1) / 8 + 1; /* length */ *p++ = (len + 8 - 1) / 8 + 1; /* length */
@ -163,8 +163,8 @@ int ndp(struct ctx *c, struct ethhdr *eh, size_t len)
*(p++) = 0; *(p++) = 0;
} }
memset(p, 0, 8 - len % 8); /* padding */ memset(p, 0, 8 - dns_s_len % 8); /* padding */
p += 8 - len % 8; p += 8 - dns_s_len % 8;
} }
*p++ = 1; /* source ll */ *p++ = 1; /* source ll */

View file

@ -47,7 +47,6 @@ static int nl_seq;
static int nl_sock_init_do(void *arg) static int nl_sock_init_do(void *arg)
{ {
struct sockaddr_nl addr = { .nl_family = AF_NETLINK, }; struct sockaddr_nl addr = { .nl_family = AF_NETLINK, };
struct ctx *c = (struct ctx *)arg;
int *s = &nl_sock, v = 1; int *s = &nl_sock, v = 1;
ns: ns:
@ -55,7 +54,7 @@ ns:
bind(*s, (struct sockaddr *)&addr, sizeof(addr))) bind(*s, (struct sockaddr *)&addr, sizeof(addr)))
*s = -1; *s = -1;
if (*s == -1 || !c || s == &nl_sock_ns) if (*s == -1 || !arg || s == &nl_sock_ns)
return 0; return 0;
setsockopt(*s, SOL_NETLINK, NETLINK_GET_STRICT_CHK, &v, sizeof(v)); setsockopt(*s, SOL_NETLINK, NETLINK_GET_STRICT_CHK, &v, sizeof(v));
@ -206,11 +205,10 @@ v6:
word = (long *)has_v4; word = (long *)has_v4;
for (i = 0; i < ARRAY_SIZE(has_v4) / sizeof(long); i++, word++) { for (i = 0; i < ARRAY_SIZE(has_v4) / sizeof(long); i++, word++) {
int ifi;
tmp = *word; tmp = *word;
while ((n = ffsl(tmp))) { while ((n = ffsl(tmp))) {
ifi = i * sizeof(long) * 8 + n - 1; int ifi = i * sizeof(long) * 8 + n - 1;
if (!first_v4) if (!first_v4)
first_v4 = ifi; first_v4 = ifi;

View file

@ -272,7 +272,7 @@ static void pid_file(struct ctx *c) {
* @argc: Argument count * @argc: Argument count
* @argv: Options, plus optional target PID for pasta mode * @argv: Options, plus optional target PID for pasta mode
* *
* Return: 0 once interrupted, non-zero on failure * Return: non-zero on failure
* *
* #syscalls read write open close fork dup2 exit chdir ioctl writev syslog * #syscalls read write open close fork dup2 exit chdir ioctl writev syslog
* #syscalls prlimit64 epoll_ctl epoll_create1 epoll_wait accept4 accept listen * #syscalls prlimit64 epoll_ctl epoll_create1 epoll_wait accept4 accept listen
@ -394,6 +394,4 @@ loop:
post_handler(&c, &now); post_handler(&c, &now);
goto loop; goto loop;
return 0;
} }

View file

@ -44,7 +44,7 @@ union epoll_ref;
*/ */
union epoll_ref { union epoll_ref {
struct { struct {
uint32_t proto:8, int32_t proto:8,
s:24; s:24;
union { union {
union tcp_epoll_ref tcp; union tcp_epoll_ref tcp;

11
pasta.c
View file

@ -167,9 +167,8 @@ netns:
*/ */
void pasta_start_ns(struct ctx *c) void pasta_start_ns(struct ctx *c)
{ {
char buf[BUFSIZ], *shell, proc_path[PATH_MAX]; int euid = geteuid(), fd;
int euid = geteuid(); char *shell;
int fd;
c->foreground = 1; c->foreground = 1;
if (!c->debug) if (!c->debug)
@ -181,6 +180,8 @@ void pasta_start_ns(struct ctx *c)
} }
if (pasta_child_pid) { if (pasta_child_pid) {
char proc_path[PATH_MAX];
NS_CALL(pasta_wait_for_ns, c); NS_CALL(pasta_wait_for_ns, c);
snprintf(proc_path, PATH_MAX, "/proc/%i/ns/net", snprintf(proc_path, PATH_MAX, "/proc/%i/ns/net",
@ -197,7 +198,9 @@ void pasta_start_ns(struct ctx *c)
} }
if (!c->netns_only) { if (!c->netns_only) {
snprintf(buf, BUFSIZ, "%u %u %u", 0, euid, 1); char buf[BUFSIZ];
snprintf(buf, BUFSIZ, "%i %i %i", 0, euid, 1);
fd = open("/proc/self/uid_map", O_WRONLY); fd = open("/proc/self/uid_map", O_WRONLY);
if (write(fd, buf, strlen(buf)) < 0) if (write(fd, buf, strlen(buf)) < 0)

6
pcap.c
View file

@ -172,9 +172,7 @@ fail:
*/ */
void pcap_init(struct ctx *c, int index) void pcap_init(struct ctx *c, int index)
{ {
char name[] = PCAP_PREFIX PCAP_ISO8601_STR STR(UINT_MAX) ".pcap";
struct timeval tv; struct timeval tv;
struct tm *tm;
if (pcap_fd != -1) if (pcap_fd != -1)
return; return;
@ -183,6 +181,10 @@ void pcap_init(struct ctx *c, int index)
return; return;
if (*c->pcap == 1) { if (*c->pcap == 1) {
char name[] = PCAP_PREFIX PCAP_ISO8601_STR STR(UINT_MAX)
".pcap";
struct tm *tm;
if (c->mode == MODE_PASTA) if (c->mode == MODE_PASTA)
memcpy(name, PCAP_PREFIX_PASTA, memcpy(name, PCAP_PREFIX_PASTA,
sizeof(PCAP_PREFIX_PASTA)); sizeof(PCAP_PREFIX_PASTA));

5
qrap.c
View file

@ -127,7 +127,7 @@ int main(int argc, char **argv)
struct arphdr ah; struct arphdr ah;
struct arpmsg am; struct arpmsg am;
} probe = { } probe = {
htonl(42), .vnet_len = htonl(42),
{ {
.h_dest = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff }, .h_dest = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff },
.h_source = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff }, .h_source = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff },
@ -198,11 +198,12 @@ int main(int argc, char **argv)
if (!strcmp(argv[i], "-device") && i + 1 < argc) { if (!strcmp(argv[i], "-device") && i + 1 < argc) {
char *p; char *p;
long n;
has_dev = 1; has_dev = 1;
if ((p = strstr(argv[i + 1], dev->template))) { if ((p = strstr(argv[i + 1], dev->template))) {
long n;
n = strtol(p + strlen(dev->template), NULL, 16); n = strtol(p + strlen(dev->template), NULL, 16);
if (!errno) if (!errno)
addr_map |= (1 << n); addr_map |= (1 << n);

View file

@ -65,7 +65,7 @@
int __i; \ int __i; \
\ \
do { \ do { \
for (__i = sizeof(v) / sizeof(v[0]); __i >= 0; __i--) \ for (__i = sizeof(v) / sizeof(v[0]) - 1; __i >= 0; __i--) \
v[__i] = k[__i % 2]; \ v[__i] = k[__i % 2]; \
} while (0) } while (0)
@ -152,13 +152,13 @@ __attribute__((__noinline__)) /* See comment in Makefile */
uint64_t siphash_20b(const uint8_t *in, const uint64_t *k) uint64_t siphash_20b(const uint8_t *in, const uint64_t *k)
{ {
uint32_t *in32 = (uint32_t *)in; uint32_t *in32 = (uint32_t *)in;
uint64_t combined;
int i; int i;
PREAMBLE(20); PREAMBLE(20);
for (i = 0; i < 2; i++, in32 += 2) { for (i = 0; i < 2; i++, in32 += 2) {
combined = (uint64_t)(*(in32 + 1)) << 32 | *in32; uint64_t combined = (uint64_t)(*(in32 + 1)) << 32 | *in32;
v[3] ^= combined; v[3] ^= combined;
SIPROUND(2); SIPROUND(2);
v[0] ^= combined; v[0] ^= combined;
@ -205,13 +205,13 @@ uint32_t siphash_32b(const uint8_t *in, const uint64_t *k)
uint32_t siphash_36b(const uint8_t *in, const uint64_t *k) uint32_t siphash_36b(const uint8_t *in, const uint64_t *k)
{ {
uint32_t *in32 = (uint32_t *)in; uint32_t *in32 = (uint32_t *)in;
uint64_t combined;
int i; int i;
PREAMBLE(36); PREAMBLE(36);
for (i = 0; i < 4; i++, in32 += 2) { for (i = 0; i < 4; i++, in32 += 2) {
combined = (uint64_t)(*(in32 + 1)) << 32 | *in32; uint64_t combined = (uint64_t)(*(in32 + 1)) << 32 | *in32;
v[3] ^= combined; v[3] ^= combined;
SIPROUND(2); SIPROUND(2);
v[0] ^= combined; v[0] ^= combined;

29
tap.c
View file

@ -265,19 +265,22 @@ static void tap_packet_debug(struct iphdr *iph, struct ipv6hdr *ip6h,
{ {
char buf6s[INET6_ADDRSTRLEN], buf6d[INET6_ADDRSTRLEN]; char buf6s[INET6_ADDRSTRLEN], buf6d[INET6_ADDRSTRLEN];
char buf4s[INET_ADDRSTRLEN], buf4d[INET_ADDRSTRLEN]; char buf4s[INET_ADDRSTRLEN], buf4d[INET_ADDRSTRLEN];
uint8_t proto; uint8_t proto = 0;
if (iph || seq4) { if (iph || seq4) {
inet_ntop(AF_INET, iph ? &iph->saddr : &seq4->saddr, inet_ntop(AF_INET, iph ? &iph->saddr : &seq4->saddr,
buf4s, sizeof(buf4s)), buf4s, sizeof(buf4s));
inet_ntop(AF_INET, iph ? &iph->daddr : &seq4->daddr, inet_ntop(AF_INET, iph ? &iph->daddr : &seq4->daddr,
buf4d, sizeof(buf4d)), buf4d, sizeof(buf4d));
proto = iph ? iph->protocol : seq4->protocol; if (iph)
proto = iph->protocol;
else if (seq4)
proto = seq4->protocol;
} else { } else {
inet_ntop(AF_INET6, ip6h ? &ip6h->saddr : &seq6->saddr, inet_ntop(AF_INET6, ip6h ? &ip6h->saddr : &seq6->saddr,
buf6s, sizeof(buf6s)), buf6s, sizeof(buf6s));
inet_ntop(AF_INET6, ip6h ? &ip6h->daddr : &seq6->daddr, inet_ntop(AF_INET6, ip6h ? &ip6h->daddr : &seq6->daddr,
buf6d, sizeof(buf6d)), buf6d, sizeof(buf6d));
proto = proto6; proto = proto6;
} }
@ -397,12 +400,12 @@ resume:
for (seq = l4_seq4 + seq_count - 1; seq >= l4_seq4; seq--) { for (seq = l4_seq4 + seq_count - 1; seq >= l4_seq4; seq--) {
if (L4_MATCH(iph, uh, seq)) { if (L4_MATCH(iph, uh, seq)) {
if (seq->msgs >= UIO_MAXIOV) if (seq->msgs >= UIO_MAXIOV)
seq = l4_seq4 - 1; seq = NULL;
break; break;
} }
} }
if (seq < l4_seq4) { if (!seq || seq < l4_seq4) {
seq = l4_seq4 + seq_count++; seq = l4_seq4 + seq_count++;
L4_SET(iph, uh, seq); L4_SET(iph, uh, seq);
seq->msgs = 0; seq->msgs = 0;
@ -560,12 +563,12 @@ resume:
for (seq = l4_seq6 + seq_count - 1; seq >= l4_seq6; seq--) { for (seq = l4_seq6 + seq_count - 1; seq >= l4_seq6; seq--) {
if (L4_MATCH(ip6h, proto, uh, seq)) { if (L4_MATCH(ip6h, proto, uh, seq)) {
if (seq->msgs >= UIO_MAXIOV) if (seq->msgs >= UIO_MAXIOV)
seq = l4_seq6 - 1; seq = NULL;
break; break;
} }
} }
if (seq < l4_seq6) { if (!seq || seq < l4_seq6) {
seq = l4_seq6 + seq_count++; seq = l4_seq6 + seq_count++;
L4_SET(ip6h, proto, uh, seq); L4_SET(ip6h, proto, uh, seq);
seq->msgs = 0; seq->msgs = 0;
@ -711,7 +714,7 @@ next:
static int tap_handler_pasta(struct ctx *c, struct timespec *now) static int tap_handler_pasta(struct ctx *c, struct timespec *now)
{ {
ssize_t n = 0, len; ssize_t n = 0, len;
int err, seq4_i = 0, seq6_i = 0; int ret, seq4_i = 0, seq6_i = 0;
restart: restart:
while ((len = read(c->fd_tap, pkt_buf + n, TAP_BUF_BYTES - n)) > 0) { while ((len = read(c->fd_tap, pkt_buf + n, TAP_BUF_BYTES - n)) > 0) {
@ -749,7 +752,7 @@ restart:
if (len < 0 && errno == EINTR) if (len < 0 && errno == EINTR)
goto restart; goto restart;
err = errno; ret = errno;
if (seq4_i) if (seq4_i)
tap4_handler(c, seq4, seq4_i, now); tap4_handler(c, seq4, seq4_i, now);
@ -757,7 +760,7 @@ restart:
if (seq6_i) if (seq6_i)
tap6_handler(c, seq6, seq6_i, now); tap6_handler(c, seq6, seq6_i, now);
if (len > 0 || err == EAGAIN) if (len > 0 || ret == EAGAIN)
return 0; return 0;
epoll_ctl(c->epollfd, EPOLL_CTL_DEL, c->fd_tap, NULL); epoll_ctl(c->epollfd, EPOLL_CTL_DEL, c->fd_tap, NULL);

109
tcp.c
View file

@ -757,13 +757,14 @@ static int tcp_rtt_dst_low(struct tcp_tap_conn *conn)
/** /**
* tcp_rtt_dst_check() - Check tcpi_min_rtt, insert endpoint in table if low * tcp_rtt_dst_check() - Check tcpi_min_rtt, insert endpoint in table if low
* @conn: Connection pointer * @conn: Connection pointer
* @info: Pointer to struct tcp_info for socket * @tinfo: Pointer to struct tcp_info for socket
*/ */
static void tcp_rtt_dst_check(struct tcp_tap_conn *conn, struct tcp_info *info) static void tcp_rtt_dst_check(struct tcp_tap_conn *conn, struct tcp_info *tinfo)
{ {
int i, hole = -1; int i, hole = -1;
if (!info->tcpi_min_rtt || (int)info->tcpi_min_rtt > LOW_RTT_THRESHOLD) if (!tinfo->tcpi_min_rtt ||
(int)tinfo->tcpi_min_rtt > LOW_RTT_THRESHOLD)
return; return;
for (i = 0; i < LOW_RTT_TABLE_SIZE; i++) { for (i = 0; i < LOW_RTT_TABLE_SIZE; i++) {
@ -809,21 +810,23 @@ static void tcp_splice_state(struct tcp_splice_conn *conn, enum tcp_state state)
*/ */
static void tcp_get_sndbuf(struct tcp_tap_conn *conn) static void tcp_get_sndbuf(struct tcp_tap_conn *conn)
{ {
int s = conn->sock, v; int s = conn->sock, sndbuf;
socklen_t sl; socklen_t sl;
uint64_t v;
sl = sizeof(v); sl = sizeof(sndbuf);
if (getsockopt(s, SOL_SOCKET, SO_SNDBUF, &v, &sl)) { if (getsockopt(s, SOL_SOCKET, SO_SNDBUF, &sndbuf, &sl)) {
conn->snd_buf = WINDOW_DEFAULT; conn->snd_buf = WINDOW_DEFAULT;
return; return;
} }
v = sndbuf;
if (v >= SNDBUF_BIG) if (v >= SNDBUF_BIG)
v /= 2; v /= 2;
else if (v > SNDBUF_SMALL) else if (v > SNDBUF_SMALL)
v -= v * (v - SNDBUF_SMALL) / (SNDBUF_BIG - SNDBUF_SMALL) / 2; v -= v * (v - SNDBUF_SMALL) / (SNDBUF_BIG - SNDBUF_SMALL) / 2;
conn->snd_buf = v; conn->snd_buf = MIN(INT_MAX, v);
} }
/** /**
@ -1537,17 +1540,17 @@ static size_t tcp_l2_buf_fill_headers(struct ctx *c, struct tcp_tap_conn *conn,
* @c: Execution context * @c: Execution context
* @conn: Connection pointer * @conn: Connection pointer
* @flags: TCP header flags we are about to send, if any * @flags: TCP header flags we are about to send, if any
* @info: tcp_info from kernel, can be NULL if not pre-fetched * @tinfo: tcp_info from kernel, can be NULL if not pre-fetched
* *
* Return: 1 if sequence or window were updated, 0 otherwise * Return: 1 if sequence or window were updated, 0 otherwise
*/ */
static int tcp_update_seqack_wnd(struct ctx *c, struct tcp_tap_conn *conn, static int tcp_update_seqack_wnd(struct ctx *c, struct tcp_tap_conn *conn,
int flags, struct tcp_info *info) int flags, struct tcp_info *tinfo)
{ {
uint32_t prev_ack_to_tap = conn->seq_ack_to_tap; uint32_t prev_ack_to_tap = conn->seq_ack_to_tap;
uint32_t prev_wnd_to_tap = conn->wnd_to_tap; uint32_t prev_wnd_to_tap = conn->wnd_to_tap;
socklen_t sl = sizeof(*info); socklen_t sl = sizeof(*tinfo);
struct tcp_info info_new; struct tcp_info tinfo_new;
int s = conn->sock; int s = conn->sock;
if (conn->state > ESTABLISHED || (flags & (DUP_ACK | FORCE_ACK)) || if (conn->state > ESTABLISHED || (flags & (DUP_ACK | FORCE_ACK)) ||
@ -1555,13 +1558,13 @@ static int tcp_update_seqack_wnd(struct ctx *c, struct tcp_tap_conn *conn,
conn->snd_buf < SNDBUF_SMALL) { conn->snd_buf < SNDBUF_SMALL) {
conn->seq_ack_to_tap = conn->seq_from_tap; conn->seq_ack_to_tap = conn->seq_from_tap;
} else if (conn->seq_ack_to_tap != conn->seq_from_tap) { } else if (conn->seq_ack_to_tap != conn->seq_from_tap) {
if (!info) { if (!tinfo) {
info = &info_new; tinfo = &tinfo_new;
if (getsockopt(s, SOL_TCP, TCP_INFO, info, &sl)) if (getsockopt(s, SOL_TCP, TCP_INFO, tinfo, &sl))
return 0; return 0;
} }
conn->seq_ack_to_tap = info->tcpi_bytes_acked + conn->seq_ack_to_tap = tinfo->tcpi_bytes_acked +
conn->seq_init_from_tap; conn->seq_init_from_tap;
if (SEQ_LT(conn->seq_ack_to_tap, prev_ack_to_tap)) if (SEQ_LT(conn->seq_ack_to_tap, prev_ack_to_tap))
@ -1574,20 +1577,20 @@ static int tcp_update_seqack_wnd(struct ctx *c, struct tcp_tap_conn *conn,
goto out; goto out;
} }
if (!info) { if (!tinfo) {
if (conn->wnd_to_tap > WINDOW_DEFAULT) if (conn->wnd_to_tap > WINDOW_DEFAULT)
goto out; goto out;
info = &info_new; tinfo = &tinfo_new;
if (getsockopt(s, SOL_TCP, TCP_INFO, info, &sl)) if (getsockopt(s, SOL_TCP, TCP_INFO, tinfo, &sl))
goto out; goto out;
} }
if (conn->local || tcp_rtt_dst_low(conn)) { if (conn->local || tcp_rtt_dst_low(conn)) {
conn->wnd_to_tap = info->tcpi_snd_wnd; conn->wnd_to_tap = tinfo->tcpi_snd_wnd;
} else { } else {
tcp_get_sndbuf(conn); tcp_get_sndbuf(conn);
conn->wnd_to_tap = MIN((int)info->tcpi_snd_wnd, conn->snd_buf); conn->wnd_to_tap = MIN((int)tinfo->tcpi_snd_wnd, conn->snd_buf);
} }
conn->wnd_to_tap = MIN(conn->wnd_to_tap, MAX_WINDOW); conn->wnd_to_tap = MIN(conn->wnd_to_tap, MAX_WINDOW);
@ -1613,8 +1616,8 @@ static int tcp_send_to_tap(struct ctx *c, struct tcp_tap_conn *conn, int flags,
uint32_t prev_wnd_to_tap = conn->wnd_to_tap; uint32_t prev_wnd_to_tap = conn->wnd_to_tap;
struct tcp4_l2_flags_buf_t *b4 = NULL; struct tcp4_l2_flags_buf_t *b4 = NULL;
struct tcp6_l2_flags_buf_t *b6 = NULL; struct tcp6_l2_flags_buf_t *b6 = NULL;
struct tcp_info info = { 0 }; struct tcp_info tinfo = { 0 };
socklen_t sl = sizeof(info); socklen_t sl = sizeof(tinfo);
size_t optlen = 0, eth_len; size_t optlen = 0, eth_len;
int s = conn->sock; int s = conn->sock;
struct iovec *iov; struct iovec *iov;
@ -1626,15 +1629,15 @@ static int tcp_send_to_tap(struct ctx *c, struct tcp_tap_conn *conn, int flags,
!flags && conn->wnd_to_tap) !flags && conn->wnd_to_tap)
return 0; return 0;
if (getsockopt(s, SOL_TCP, TCP_INFO, &info, &sl)) { if (getsockopt(s, SOL_TCP, TCP_INFO, &tinfo, &sl)) {
tcp_tap_destroy(c, conn); tcp_tap_destroy(c, conn);
return -ECONNRESET; return -ECONNRESET;
} }
if (!conn->local) if (!conn->local)
tcp_rtt_dst_check(conn, &info); tcp_rtt_dst_check(conn, &tinfo);
if (!tcp_update_seqack_wnd(c, conn, flags, &info) && !flags) if (!tcp_update_seqack_wnd(c, conn, flags, &tinfo) && !flags)
return 0; return 0;
if (CONN_V4(conn)) { if (CONN_V4(conn)) {
@ -1661,7 +1664,7 @@ static int tcp_send_to_tap(struct ctx *c, struct tcp_tap_conn *conn, int flags,
*data++ = OPT_MSS_LEN; *data++ = OPT_MSS_LEN;
if (c->mtu == -1) { if (c->mtu == -1) {
mss = info.tcpi_snd_mss; mss = tinfo.tcpi_snd_mss;
} else { } else {
mss = c->mtu - sizeof(struct tcphdr); mss = c->mtu - sizeof(struct tcphdr);
if (CONN_V4(conn)) if (CONN_V4(conn))
@ -1681,11 +1684,11 @@ static int tcp_send_to_tap(struct ctx *c, struct tcp_tap_conn *conn, int flags,
th->doff += OPT_MSS_LEN / 4; th->doff += OPT_MSS_LEN / 4;
#ifdef HAS_SND_WND #ifdef HAS_SND_WND
if (!c->tcp.kernel_snd_wnd && info.tcpi_snd_wnd) if (!c->tcp.kernel_snd_wnd && tinfo.tcpi_snd_wnd)
c->tcp.kernel_snd_wnd = 1; c->tcp.kernel_snd_wnd = 1;
#endif #endif
conn->ws = MIN(MAX_WS, info.tcpi_snd_wscale); conn->ws = MIN(MAX_WS, tinfo.tcpi_snd_wscale);
*data++ = OPT_NOP; *data++ = OPT_NOP;
*data++ = OPT_WS; *data++ = OPT_WS;
@ -1768,7 +1771,7 @@ static void tcp_rst(struct ctx *c, struct tcp_tap_conn *conn)
static void tcp_clamp_window(struct tcp_tap_conn *conn, struct tcphdr *th, static void tcp_clamp_window(struct tcp_tap_conn *conn, struct tcphdr *th,
int len, unsigned int window, int init) int len, unsigned int window, int init)
{ {
if (init) { if (init && th) {
int ws = tcp_opt_get(th, len, OPT_WS, NULL, NULL); int ws = tcp_opt_get(th, len, OPT_WS, NULL, NULL);
conn->ws_tap = ws; conn->ws_tap = ws;
@ -1901,7 +1904,7 @@ static void tcp_conn_from_tap(struct ctx *c, int af, void *addr,
sock_pool_p = &init_sock_pool6[i]; sock_pool_p = &init_sock_pool6[i];
else else
sock_pool_p = &init_sock_pool4[i]; sock_pool_p = &init_sock_pool4[i];
if ((ref.r.s = s = *sock_pool_p) >= 0) { if ((ref.r.s = s = (*sock_pool_p)) >= 0) {
*sock_pool_p = -1; *sock_pool_p = -1;
break; break;
} }
@ -2164,7 +2167,7 @@ static int tcp_data_from_sock(struct ctx *c, struct tcp_tap_conn *conn,
struct timespec *now) struct timespec *now)
{ {
int fill_bufs, send_bufs = 0, last_len, iov_rem = 0; int fill_bufs, send_bufs = 0, last_len, iov_rem = 0;
int send, len, plen, v4 = CONN_V4(conn); int sendlen, len, plen, v4 = CONN_V4(conn);
uint32_t seq_to_tap = conn->seq_to_tap; uint32_t seq_to_tap = conn->seq_to_tap;
int s = conn->sock, i, ret = 0; int s = conn->sock, i, ret = 0;
struct msghdr mh_sock = { 0 }; struct msghdr mh_sock = { 0 };
@ -2226,16 +2229,16 @@ recvmsg:
if (!len) if (!len)
goto zero_len; goto zero_len;
send = len - already_sent; sendlen = len - already_sent;
if (send <= 0) { if (sendlen <= 0) {
tcp_tap_epoll_mask(c, conn, conn->events | EPOLLET); tcp_tap_epoll_mask(c, conn, conn->events | EPOLLET);
return 0; return 0;
} }
tcp_tap_epoll_mask(c, conn, conn->events & ~EPOLLET); tcp_tap_epoll_mask(c, conn, conn->events & ~EPOLLET);
send_bufs = DIV_ROUND_UP(send, conn->mss_guest); send_bufs = DIV_ROUND_UP(sendlen, conn->mss_guest);
last_len = send - (send_bufs - 1) * conn->mss_guest; last_len = sendlen - (send_bufs - 1) * conn->mss_guest;
/* Likely, some new data was acked too. */ /* Likely, some new data was acked too. */
tcp_update_seqack_wnd(c, conn, 0, NULL); tcp_update_seqack_wnd(c, conn, 0, NULL);
@ -2594,7 +2597,7 @@ int tcp_tap_handler(struct ctx *c, int af, void *addr,
conn->mss_guest = MIN(MSS6, conn->mss_guest); conn->mss_guest = MIN(MSS6, conn->mss_guest);
} }
/* info.tcpi_bytes_acked already includes one byte for SYN, but /* tinfo.tcpi_bytes_acked already includes one byte for SYN, but
* not for incoming connections. * not for incoming connections.
*/ */
conn->seq_init_from_tap = ntohl(th->seq) + 1; conn->seq_init_from_tap = ntohl(th->seq) + 1;
@ -2787,8 +2790,8 @@ static int tcp_splice_connect(struct ctx *c, struct tcp_splice_conn *conn,
.sin_addr = { .s_addr = htonl(INADDR_LOOPBACK) }, .sin_addr = { .s_addr = htonl(INADDR_LOOPBACK) },
}; };
const struct sockaddr *sa; const struct sockaddr *sa;
int ret, one = 1;
socklen_t sl; socklen_t sl;
int one = 1;
conn->to = sock_conn; conn->to = sock_conn;
@ -2807,7 +2810,8 @@ static int tcp_splice_connect(struct ctx *c, struct tcp_splice_conn *conn,
if (connect(conn->to, sa, sl)) { if (connect(conn->to, sa, sl)) {
if (errno != EINPROGRESS) { if (errno != EINPROGRESS) {
ret = -errno; int ret = -errno;
close(sock_conn); close(sock_conn);
return ret; return ret;
} }
@ -3049,10 +3053,8 @@ void tcp_sock_handler_splice(struct ctx *c, union epoll_ref ref,
goto close; goto close;
if (events & EPOLLOUT) { if (events & EPOLLOUT) {
struct epoll_event ev = { ev.events = EPOLLIN | EPOLLRDHUP;
.events = EPOLLIN | EPOLLRDHUP, ev.data.u64 = ref.u64;
.data.u64 = ref.u64,
};
if (conn->state == SPLICE_CONNECT) if (conn->state == SPLICE_CONNECT)
tcp_splice_connect_finish(c, conn, ref.r.p.tcp.tcp.v6); tcp_splice_connect_finish(c, conn, ref.r.p.tcp.tcp.v6);
@ -3111,12 +3113,13 @@ swap:
while (1) { while (1) {
int retry_write = 0, more = 0; int retry_write = 0, more = 0;
ssize_t read, to_write = 0, written; ssize_t readlen, to_write = 0, written;
retry: retry:
read = splice(move_from, NULL, pipes[1], NULL, c->tcp.pipe_size, readlen = splice(move_from, NULL, pipes[1], NULL,
SPLICE_F_MOVE | SPLICE_F_NONBLOCK); c->tcp.pipe_size,
if (read < 0) { SPLICE_F_MOVE | SPLICE_F_NONBLOCK);
if (readlen < 0) {
if (errno == EINTR) if (errno == EINTR)
goto retry; goto retry;
@ -3124,13 +3127,13 @@ retry:
goto close; goto close;
to_write = c->tcp.pipe_size; to_write = c->tcp.pipe_size;
} else if (!read) { } else if (!readlen) {
eof = 1; eof = 1;
to_write = c->tcp.pipe_size; to_write = c->tcp.pipe_size;
} else { } else {
never_read = 0; never_read = 0;
to_write += read; to_write += readlen;
if (read >= (long)c->tcp.pipe_size * 90 / 100) if (readlen >= (long)c->tcp.pipe_size * 90 / 100)
more = SPLICE_F_MORE; more = SPLICE_F_MORE;
if (bitmap_isset(rcvlowat_set, conn - ts)) if (bitmap_isset(rcvlowat_set, conn - ts))
@ -3142,12 +3145,12 @@ eintr:
SPLICE_F_MOVE | more | SPLICE_F_NONBLOCK); SPLICE_F_MOVE | more | SPLICE_F_NONBLOCK);
/* Most common case: skip updating counters. */ /* Most common case: skip updating counters. */
if (read > 0 && read == written) { if (readlen > 0 && readlen == written) {
if (read >= (long)c->tcp.pipe_size * 10 / 100) if (readlen >= (long)c->tcp.pipe_size * 10 / 100)
continue; continue;
if (!bitmap_isset(rcvlowat_set, conn - ts) && if (!bitmap_isset(rcvlowat_set, conn - ts) &&
read > (long)c->tcp.pipe_size / 10) { readlen > (long)c->tcp.pipe_size / 10) {
int lowat = c->tcp.pipe_size / 4; int lowat = c->tcp.pipe_size / 4;
setsockopt(move_from, SOL_SOCKET, SO_RCVLOWAT, setsockopt(move_from, SOL_SOCKET, SO_RCVLOWAT,
@ -3160,7 +3163,7 @@ eintr:
break; break;
} }
*seq_read += read > 0 ? read : 0; *seq_read += readlen > 0 ? readlen : 0;
*seq_write += written > 0 ? written : 0; *seq_write += written > 0 ? written : 0;
if (written < 0) { if (written < 0) {

View file

@ -11,8 +11,12 @@
# Copyright (c) 2021 Red Hat GmbH # Copyright (c) 2021 Red Hat GmbH
# Author: Stefano Brivio <sbrivio@redhat.com> # Author: Stefano Brivio <sbrivio@redhat.com>
htools clang-tidy htools clang-tidy cppcheck
test Run clang-tidy test Run clang-tidy
hout RET make clang-tidy; echo $? hout RET make clang-tidy; echo $?
check [ __RET__ -eq 0 ] check [ __RET__ -eq 0 ]
test Run cppcheck
hout RET make cppcheck; echo $?
check [ __RET__ -eq 0 ]

18
udp.c
View file

@ -616,9 +616,9 @@ static void udp_sock_handler_splice(struct ctx *c, union epoll_ref ref,
if (ref.r.p.udp.udp.splice == UDP_TO_NS || if (ref.r.p.udp.udp.splice == UDP_TO_NS ||
ref.r.p.udp.udp.splice == UDP_TO_INIT) { ref.r.p.udp.udp.splice == UDP_TO_INIT) {
for (i = 0; i < n; i++) { for (i = 0; i < n; i++) {
struct msghdr *mh = &udp_splice_mmh_send[i].msg_hdr; struct msghdr *mh_s = &udp_splice_mmh_send[i].msg_hdr;
mh->msg_iov->iov_len = udp_splice_mmh_recv[i].msg_len; mh_s->msg_iov->iov_len = udp_splice_mmh_recv[i].msg_len;
} }
sendmmsg(s, udp_splice_mmh_send, n, MSG_NOSIGNAL); sendmmsg(s, udp_splice_mmh_send, n, MSG_NOSIGNAL);
@ -626,9 +626,9 @@ static void udp_sock_handler_splice(struct ctx *c, union epoll_ref ref,
} }
for (i = 0; i < n; i++) { for (i = 0; i < n; i++) {
struct msghdr *mh = &udp_splice_mmh_sendto[i].msg_hdr; struct msghdr *mh_s = &udp_splice_mmh_sendto[i].msg_hdr;
mh->msg_iov->iov_len = udp_splice_mmh_recv[i].msg_len; mh_s->msg_iov->iov_len = udp_splice_mmh_recv[i].msg_len;
} }
if (v6) { if (v6) {
@ -710,8 +710,6 @@ void udp_sock_handler(struct ctx *c, union epoll_ref ref, uint32_t events,
else else
b->ip6h.saddr = c->addr6_ll; b->ip6h.saddr = c->addr6_ll;
b->ip6h.saddr = c->gw6;
udp_tap_map[V6][src].ts_local = now->tv_sec; udp_tap_map[V6][src].ts_local = now->tv_sec;
if (IN6_IS_ADDR_LOOPBACK(&b->s_in6.sin6_addr)) if (IN6_IS_ADDR_LOOPBACK(&b->s_in6.sin6_addr))
@ -1000,11 +998,11 @@ int udp_tap_handler(struct ctx *c, int af, void *addr,
} }
for (i = 0; i < count; i++) { for (i = 0; i < count; i++) {
struct udphdr *uh; struct udphdr *uh_send;
uh = (struct udphdr *)(msg[i].pkt_buf_offset + pkt_buf); uh_send = (struct udphdr *)(msg[i].pkt_buf_offset + pkt_buf);
m[i].iov_base = (char *)(uh + 1); m[i].iov_base = (char *)(uh_send + 1);
m[i].iov_len = msg[i].l4_len - sizeof(*uh); m[i].iov_len = msg[i].l4_len - sizeof(*uh_send);
mm[i].msg_hdr.msg_name = sa; mm[i].msg_hdr.msg_name = sa;
mm[i].msg_hdr.msg_namelen = sl; mm[i].msg_hdr.msg_namelen = sl;

4
util.c
View file

@ -51,7 +51,7 @@ void name(const char *format, ...) { \
\ \
if (setlogmask(0) & LOG_MASK(LOG_DEBUG)) { \ if (setlogmask(0) & LOG_MASK(LOG_DEBUG)) { \
clock_gettime(CLOCK_REALTIME, &tp); \ clock_gettime(CLOCK_REALTIME, &tp); \
fprintf(stderr, "%lu.%04lu: ", \ fprintf(stderr, "%li.%04li: ", \
tp.tv_sec - log_debug_start, \ tp.tv_sec - log_debug_start, \
tp.tv_nsec / (100 * 1000)); \ tp.tv_nsec / (100 * 1000)); \
} else { \ } else { \
@ -142,7 +142,7 @@ void passt_vsyslog(int pri, const char *format, va_list ap)
if (format[strlen(format)] != '\n') if (format[strlen(format)] != '\n')
n += snprintf(buf + n, BUFSIZ - n, "\n"); n += snprintf(buf + n, BUFSIZ - n, "\n");
if (log_opt | LOG_PERROR) if (log_opt & LOG_PERROR)
fprintf(stderr, "%s", buf + sizeof("<0>")); fprintf(stderr, "%s", buf + sizeof("<0>"));
send(log_sock, buf, n, 0); send(log_sock, buf, n, 0);