tech@ & krw (since your code in question was imported to vmd),
I found strange behavior running tcpbench(1) to measure the connection
between a vmd guest and my host, as well as guest-to-guest. In short,
it's some bogus logic in how vmd tries to intercept dhcp/bootp on local
interfaces. Diff at the bottom addresses the issue, some background:
Running tcpbench(1) for ~20-30s on my machine, vmd (with -v debug
logging) barfs a bunch of lines like:
5 udp packets in 5 too long - dropped
The tcpbench(1) throughput stalls out at that point and reports 0 Mbps
avg bandwidth measurements.
If anyone wants to reproduce, use an OpenBSD guest and just run:
[host]$ tcpbench -s
[guest]$ tcpbench -t 180 100.64.x.2
Where 'x' is the appropriate value for your guest's local interface.
reyk@ imported packet.c from dhclient(8), but there's no validation that
the packet being inspected is an IP/UDP packet vs. IP/TCP, leading to
bogus logic related to inspecing UDP header attributes. In dhclient(8),
the decode_udp_ip_header function is used in a place where a bpf capture
buffer has already made sure it's a UDP packet (see sbin/dhclient/bpf.c).
In addition, there was a lot of stateful counting and checking we just
don't need in vmd(8), so I've ripped that out as well. It makes no sense
in this context.
OK?
Index: packet.c
===================================================================
RCS file: /cvs/src/usr.sbin/vmd/packet.c,v
retrieving revision 1.1
diff -u -p -r1.1 packet.c
--- packet.c 19 Apr 2017 15:38:32 -0000 1.1
+++ packet.c 22 May 2021 14:15:09 -0000
@@ -220,12 +220,6 @@ decode_udp_ip_header(unsigned char *buf,
unsigned char *data;
u_int32_t ip_len;
u_int32_t sum, usum;
- static unsigned int ip_packets_seen;
- static unsigned int ip_packets_bad_checksum;
- static unsigned int udp_packets_seen;
- static unsigned int udp_packets_bad_checksum;
- static unsigned int udp_packets_length_checked;
- static unsigned int udp_packets_length_overflow;
int len;
/* Assure that an entire IP header is within the buffer. */
@@ -236,17 +230,11 @@ decode_udp_ip_header(unsigned char *buf,
return (-1);
ip = (struct ip *)(buf + offset);
- ip_packets_seen++;
+ if (ip->ip_p != IPPROTO_UDP)
+ return (-1);
/* Check the IP header checksum - it should be zero. */
if (wrapsum(checksum(buf + offset, ip_len, 0)) != 0) {
- ip_packets_bad_checksum++;
- if (ip_packets_seen > 4 && ip_packets_bad_checksum != 0 &&
- (ip_packets_seen / ip_packets_bad_checksum) < 2) {
- log_info("%u bad IP checksums seen in %u packets",
- ip_packets_bad_checksum, ip_packets_seen);
- ip_packets_seen = ip_packets_bad_checksum = 0;
- }
return (-1);
}
@@ -274,7 +262,6 @@ decode_udp_ip_header(unsigned char *buf,
if (buflen < offset + ip_len + sizeof(*udp))
return (-1);
udp = (struct udphdr *)(buf + offset + ip_len);
- udp_packets_seen++;
/* Assure that the entire UDP packet is within the buffer. */
if (buflen < offset + ip_len + ntohs(udp->uh_ulen))
@@ -286,20 +273,8 @@ decode_udp_ip_header(unsigned char *buf,
* UDP header and the data. If the UDP checksum field is zero,
* we're not supposed to do a checksum.
*/
- udp_packets_length_checked++;
len = ntohs(udp->uh_ulen) - sizeof(*udp);
if ((len < 0) || (len + data > buf + buflen)) {
- udp_packets_length_overflow++;
- if (udp_packets_length_checked > 4 &&
- udp_packets_length_overflow != 0 &&
- (udp_packets_length_checked /
- udp_packets_length_overflow) < 2) {
- log_info("%u udp packets in %u too long - dropped",
- udp_packets_length_overflow,
- udp_packets_length_checked);
- udp_packets_length_overflow =
- udp_packets_length_checked = 0;
- }
return (-1);
}
if (len + data != buf + buflen)
@@ -313,15 +288,7 @@ decode_udp_ip_header(unsigned char *buf,
2 * sizeof(ip->ip_src),
IPPROTO_UDP + (u_int32_t)ntohs(udp->uh_ulen)))));
- udp_packets_seen++;
if (usum && usum != sum) {
- udp_packets_bad_checksum++;
- if (udp_packets_seen > 4 && udp_packets_bad_checksum != 0 &&
- (udp_packets_seen / udp_packets_bad_checksum) < 2) {
- log_info("%u bad udp checksums in %u packets",
- udp_packets_bad_checksum, udp_packets_seen);
- udp_packets_seen = udp_packets_bad_checksum = 0;
- }
return (-1);
}
