Re: tcp tso loopback checksum
On 21.5.2023. 22:41, Alexander Bluhm wrote: > Hi, > > When sending TCP packets with software TSO to the local address of > a physical interface, the TCP checksum was miscalculated. > > This bug was triggered on loopback interface, when sending to the > local interface address of a physical interface. Due to another > bug, the smaller MTU of the physical interface was used. Then the > packet was sent without TSO chopping as it did fit the larger MTU > of the loopback interface. Although the loopback interface does > not support hardware TSO, the modified TCP pseudo header checksum > for hardware TSO checksum offloading was calculated. > > Please test with and without hardware TSO. Hi, in lab i have: ix0: flags=2008843 mtu 1500 inet 192.168.100.14 netmask 0xff00 broadcast 192.168.100.255 inet6 fe80::225:90ff:fe5d:ca38%ix0 prefixlen 64 scopeid 0x1 inet6 192:168:1000:1000::114 prefixlen 64 lo123: flags=8049 mtu 32768 inet 10.156.156.1 netmask 0xff00 inet6 fe80::1%lo123 prefixlen 64 scopeid 0xd inet6 192:168:1560:1560::114 prefixlen 64 iperf3 -s6 -B 192:168:1000:1000::114 <- server iperf3 -6 -B 192:168:1560:1560::114 -c 192:168:1000:1000::114 <- client iperf3 -s -B 192.168.100.14 -p 5202 <- server iperf3 -B 10.156.156.1 -c 192.168.100.14 -p 5202 <- client without this diff and with net.inet.tcp.tso=1 I'm getting few Kbps over ip4 and ip6. with net.inet.tcp.tso=0 i'm getting cca 650Mbps smc4# netstat -sp tcp | grep TSO 0 output TSO packets software chopped 0 output TSO packets hardware processed 0 output TSO packets generated 0 output TSO packets dropped With this diff and with net.inet.tcp.tso=1 i'm getting 2Gbps over ip4 and 2Gbps over ip6. with net.inet.tcp.tso=0 650Mbps. smc4# netstat -sp tcp | grep TSO 157397 output TSO packets software chopped 0 output TSO packets hardware processed 4076665 output TSO packets generated 0 output TSO packets dropped
Re: Fix wrong interface mtu in tcp_mss
On Sat, May 20, 2023 at 02:46:27PM +0200, Claudio Jeker wrote: > On Fri, May 19, 2023 at 07:58:47PM +0200, Jan Klemkow wrote: > > Hi, > > > > We use the wrong interface and mtu in tcp_mss() to calculate the mss if > > the destination address points is a local address. In ip_output() we > > use the correct interface and its mtu. > > > > This limits the mss to 1448 if the mtu of the interface it 1500, > > instead of using a local 32k mss. > > > > The bigger issue is: local bulk traffic with the current TSO > > implementation is broken. tcp_output() creates TSO packets with an mss > > smaller then 32k and ip_output() calls if_output instead of > > tcp_if_output_tso() because it fits into the mtu check of lo0. > > > > This diff takes the same logic to pick the interface in tcp_mss() as its > > done in ip_output() and fixes both issues. > > > > ok? > > I'm fine with this going in since it emulates the same behaviour as > ip_output. For the curious ip6_output() seems to have the same workaround. OK bluhm@ > Now in an ideal world the other issue exposed by this mtu mismatch should > also be fixed. We had similar issues with TCP over loopback on multiple > occasions. So maybe it is time to fix this for good. Please wait before commiting your diff. My "tcp tso loopback checksum" on tech@ should be commited and tested first, so we see whether that fixes the underlying problem. > Note: In an ideal world lo(4) would do TSO and LRO since it bounces the > packet right back at us. I think we should try to implement lo(4) checksum offloading. In an ideal world, packets are marked with correct checksum over loopback instead of calculating it twice. Then sending large packets with TSO and receiving with LSO would also be good. It helps testing the feature. But performance improvemnet might be small as MTU is already 32768 for loopback. bluhm > > Index: netinet/tcp_input.c > > === > > RCS file: /cvs/src/sys/netinet/tcp_input.c,v > > retrieving revision 1.387 > > diff -u -p -r1.387 tcp_input.c > > --- netinet/tcp_input.c 14 Mar 2023 00:24:05 - 1.387 > > +++ netinet/tcp_input.c 19 May 2023 17:22:47 - > > @@ -2805,7 +2805,11 @@ tcp_mss(struct tcpcb *tp, int offer) > > if (rt == NULL) > > goto out; > > > > - ifp = if_get(rt->rt_ifidx); > > + if (ISSET(rt->rt_flags, RTF_LOCAL)) > > + ifp = if_get(rtable_loindex(inp->inp_rtableid)); > > + else > > + ifp = if_get(rt->rt_ifidx); > > + > > if (ifp == NULL) > > goto out; > > > > > > -- > :wq Claudio
tcp tso loopback checksum
Hi,
When sending TCP packets with software TSO to the local address of
a physical interface, the TCP checksum was miscalculated.
This bug was triggered on loopback interface, when sending to the
local interface address of a physical interface. Due to another
bug, the smaller MTU of the physical interface was used. Then the
packet was sent without TSO chopping as it did fit the larger MTU
of the loopback interface. Although the loopback interface does
not support hardware TSO, the modified TCP pseudo header checksum
for hardware TSO checksum offloading was calculated.
Please test with and without hardware TSO.
ok?
bluhm
Index: netinet/ip_output.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_output.c,v
retrieving revision 1.387
diff -u -p -r1.387 ip_output.c
--- netinet/ip_output.c 15 May 2023 16:34:56 - 1.387
+++ netinet/ip_output.c 21 May 2023 20:20:03 -
@@ -1882,7 +1882,8 @@ in_proto_cksum_out(struct mbuf *m, struc
u_int16_t csum = 0, offset;
offset = ip->ip_hl << 2;
- if (ISSET(m->m_pkthdr.csum_flags, M_TCP_TSO)) {
+ if (ISSET(m->m_pkthdr.csum_flags, M_TCP_TSO) && ifp != NULL &&
+ ISSET(ifp->if_capabilities, IFCAP_TSOv4)) {
csum = in_cksum_phdr(ip->ip_src.s_addr,
ip->ip_dst.s_addr, htonl(ip->ip_p));
} else if (ISSET(m->m_pkthdr.csum_flags,
Index: netinet6/ip6_output.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/ip6_output.c,v
retrieving revision 1.276
diff -u -p -r1.276 ip6_output.c
--- netinet6/ip6_output.c 15 May 2023 16:34:57 - 1.276
+++ netinet6/ip6_output.c 21 May 2023 20:20:18 -
@@ -2710,7 +2710,8 @@ in6_proto_cksum_out(struct mbuf *m, stru
u_int16_t csum;
offset = ip6_lasthdr(m, 0, IPPROTO_IPV6, &nxt);
- if (ISSET(m->m_pkthdr.csum_flags, M_TCP_TSO)) {
+ if (ISSET(m->m_pkthdr.csum_flags, M_TCP_TSO) && ifp != NULL &&
+ ISSET(ifp->if_capabilities, IFCAP_TSOv6)) {
csum = in6_cksum_phdr(&ip6->ip6_src, &ip6->ip6_dst,
htonl(0), htonl(nxt));
} else {
Re: patch: atfork unlock
On 07/12/22(Wed) 22:17, Joel Knight wrote:
> Hi. As first mentioned on misc[1], I've identified a deadlock in libc
> when a process forks, the children are multi-threaded, and they set
> one or more atfork callbacks. The diff below causes ATFORK_UNLOCK() to
> release the lock even when the process isn't multi-threaded. This
> avoids the deadlock. With this patch applied, the test case I have for
> this issue succeeds and there are no new failures during a full 'make
> regress'.
>
> Threading is outside my area of expertise so I've no idea if the fix
> proposed here is appropriate. I'm happy to take or test feedback.
>
> The diff is below and a clean copy is here:
> https://www.packetmischief.ca/files/patches/atfork_on_fork.diff.
This sounds legit to me. Anyone some time wants to take a look?
> .joel
>
>
> [1] https://marc.info/?l=openbsd-misc&m=166926508819790&w=2
>
>
>
> Index: lib/libc/include/thread_private.h
> ===
> RCS file: /data/cvs-mirror/OpenBSD/src/lib/libc/include/thread_private.h,v
> retrieving revision 1.36
> diff -p -u -r1.36 thread_private.h
> --- lib/libc/include/thread_private.h 6 Jan 2021 19:54:17 - 1.36
> +++ lib/libc/include/thread_private.h 8 Dec 2022 04:28:45 -
> @@ -228,7 +228,7 @@ __END_HIDDEN_DECLS
> } while (0)
> #define _ATFORK_UNLOCK() \
> do { \
> - if (__isthreaded) \
> + if (_thread_cb.tc_atfork_unlock != NULL) \
> _thread_cb.tc_atfork_unlock(); \
> } while (0)
>
> Index: regress/lib/libpthread/pthread_atfork_on_fork/Makefile
> ===
> RCS file: regress/lib/libpthread/pthread_atfork_on_fork/Makefile
> diff -N regress/lib/libpthread/pthread_atfork_on_fork/Makefile
> --- /dev/null 1 Jan 1970 00:00:00 -
> +++ regress/lib/libpthread/pthread_atfork_on_fork/Makefile 7 Dec 2022
> 04:38:39 -
> @@ -0,0 +1,9 @@
> +# $OpenBSD$
> +
> +PROG= pthread_atfork_on_fork
> +
> +REGRESS_TARGETS= timeout
> +timeout:
> + timeout 10s ./${PROG}
> +
> +.include
> Index: regress/lib/libpthread/pthread_atfork_on_fork/pthread_atfork_on_fork.c
> ===
> RCS file:
> regress/lib/libpthread/pthread_atfork_on_fork/pthread_atfork_on_fork.c
> diff -N regress/lib/libpthread/pthread_atfork_on_fork/pthread_atfork_on_fork.c
> --- /dev/null 1 Jan 1970 00:00:00 -
> +++ regress/lib/libpthread/pthread_atfork_on_fork/pthread_atfork_on_fork.c
> 7 Dec 2022 04:59:10 -
> @@ -0,0 +1,94 @@
> +/* $OpenBSD$ */
> +
> +/*
> + * Copyright (c) 2022 Joel Knight
> + *
> + * Permission to use, copy, modify, and distribute this software for any
> + * purpose with or without fee is hereby granted, provided that the above
> + * copyright notice and this permission notice appear in all copies.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + */
> +
> +/*
> + * This test exercises atfork lock/unlock through multiple generations of
> + * forked child processes where each child also becomes multi-threaded.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +
> +#define NUMBER_OF_GENERATIONS 4
> +#define NUMBER_OF_TEST_THREADS 2
> +
> +#define SAY(...) do { \
> +fprintf(stderr, "pid %5i ", getpid()); \
> +fprintf(stderr, __VA_ARGS__); \
> +} while (0)
> +
> +void
> +prepare(void)
> +{
> +/* Do nothing */
> +}
> +
> +void *
> +thread(void *arg)
> +{
> +return (NULL);
> +}
> +
> +int
> +test(int fork_level)
> +{
> +pid_t proc_pid;
> +size_t thread_index;
> +pthread_t threads[NUMBER_OF_TEST_THREADS];
> +
> +proc_pid = fork();
> +fork_level = fork_level - 1;
> +
> +if (proc_pid == 0) {
> +SAY("generation %i\n", fork_level);
> +pthread_atfork(prepare, NULL, NULL);
> +
> +for (thread_index = 0; thread_index < NUMBER_OF_TEST_THREADS;
> thread_index++) {
> +pthread_create(&threads[thread_index], NULL, thread, NULL);
> +}
> +
> +for (thread_index = 0; thread_index < NUMBER_OF_TEST_THREADS;
> thread_index++) {
> +pthread_join(threads[thread_index], NULL);
> +}
> +
> +if (fork_level > 0) {
> +test(fork_level);
> +}
> +
> +SAY("exiting\n");
> +exit(0);
> +}
> +else {
> +SAY("parent waiting on child %i\n", proc_pid);
> +waitpid(proc_pid, 0, 0);
> +}
> +
> +return (0);
> +}
> +
> +int
> +main(int argc, char *argv[])
>
hack game: fix launch without /usr/games in path
Hello.
After removing /usr/games from PATH, hack failing with error:
Cannot get status of hack.
Hack tries to get information about st_mtime of execuatble, so it
search for hack in all directories from path. Game wants to compare
modification time of hack and saves, so saves from previous version
won't be load.
I think hack is mature enough and has stable format of saves, we can
remove this check and bring back ability to run it without path's
modification.
Index: hack.bones.c
===
RCS file: /cvs/src/games/hack/hack.bones.c,v
retrieving revision 1.11
diff -u -p -u -p -r1.11 hack.bones.c
--- hack.bones.c28 Jun 2019 13:32:52 - 1.11
+++ hack.bones.c21 May 2023 12:18:07 -
@@ -139,17 +139,15 @@ savebones(void)
int
getbones(void)
{
- int fd,x,y,ok;
+ int fd,x,y;
if(rn2(3)) return(0); /* only once in three times do we find bones */
bones[6] = '0' + dlevel/10;
bones[7] = '0' + dlevel%10;
if((fd = open(bones, O_RDONLY)) == -1) return(0);
- if((ok = uptodate(fd)) != 0){
- getlev(fd, 0, dlevel);
- for(x = 0; x < COLNO; x++) for(y = 0; y < ROWNO; y++)
- levl[x][y].seen = levl[x][y].new = 0;
- }
+ getlev(fd, 0, dlevel);
+ for(x = 0; x < COLNO; x++) for(y = 0; y < ROWNO; y++)
+ levl[x][y].seen = levl[x][y].new = 0;
(void) close(fd);
#ifdef WIZARD
if(!wizard) /* duvel!frans: don't remove bones while debugging */
@@ -158,5 +156,5 @@ getbones(void)
pline("Cannot unlink %s .", bones);
return(0);
}
- return(ok);
+ return(1);
}
Index: hack.h
===
RCS file: /cvs/src/games/hack/hack.h,v
retrieving revision 1.13
diff -u -p -u -p -r1.13 hack.h
--- hack.h 9 Jan 2016 18:33:15 - 1.13
+++ hack.h 21 May 2023 12:18:07 -
@@ -681,7 +681,6 @@ int phase_of_the_moon(void);
int night(void);
int midnight(void);
void gethdate(char *);
-int uptodate(int);
void getlock(void);
#ifdef MAIL
void getmailstatus(void);
Index: hack.main.c
===
RCS file: /cvs/src/games/hack/hack.main.c,v
retrieving revision 1.24
diff -u -p -u -p -r1.24 hack.main.c
--- hack.main.c 28 Jun 2019 13:32:52 - 1.24
+++ hack.main.c 21 May 2023 12:18:08 -
@@ -103,7 +103,6 @@ static void chdirx(char *, boolean);
int
main(int argc, char **argv)
{
- extern char *__progname;
int fd;
#ifdef CHDIR
char *dir;
@@ -183,15 +182,6 @@ main(int argc, char **argv)
u.ux = FAR; /* prevent nscr() */
(void) signal(SIGHUP, hackhangup);
- /*
-* Find the creation date of this game,
-* so as to avoid restoring outdated savefiles.
-*/
- gethdate(__progname);
-
- /*
-* We cannot do chdir earlier, otherwise gethdate will fail.
-*/
#ifdef CHDIR
chdirx(dir,1);
#endif
@@ -298,8 +288,7 @@ main(int argc, char **argv)
setftty();
(void) snprintf(SAVEF, sizeof SAVEF, "save/%u%s", getuid(), plname);
regularize(SAVEF+5);/* avoid . or / in name */
- if((fd = open(SAVEF, O_RDONLY)) >= 0 &&
- (uptodate(fd) || unlink(SAVEF) == 666)) {
+ if((fd = open(SAVEF, O_RDONLY)) >= 0) {
(void) signal(SIGINT,done1);
pline("Restoring old save file...");
(void) fflush(stdout);
Index: hack.unix.c
===
RCS file: /cvs/src/games/hack/hack.unix.c,v
retrieving revision 1.21
diff -u -p -u -p -r1.21 hack.unix.c
--- hack.unix.c 15 Dec 2021 16:29:29 - 1.21
+++ hack.unix.c 21 May 2023 12:18:08 -
@@ -154,52 +154,7 @@ midnight(void)
return(getlt()->tm_hour == 0);
}
-struct stat buf, hbuf;
-
-void
-gethdate(char *name)
-{
- char *p, *np, *path;
- char filename[PATH_MAX];
-
- if (strchr(name, '/') != NULL || (p = getenv("PATH")) == NULL)
- p = "";
- np = path = strdup(p); /* Make a copy for strsep. */
- if (path == NULL)
- err(1, NULL);
-
- for (;;) {
- if ((p = strsep(&np, ":")) == NULL)
- break;
- if (*p == '\0') /* :: */
- (void) strlcpy(filename, name, sizeof filename);
- else
- (void) snprintf(filename, sizeof filename,
- "%s/%s", p, name);
-
- if (stat(filename, &hbuf) == 0) {
- free(path);
- return;
- }
- }
- error("Cannot get status of %s.",
- (p = strrchr(name, '/')) ? p+1 : name);
- free(path);
-}
-
-int
-uptodate(int fd)
-{
- if(fstat(fd, &buf)) {
Re: remove net.inet6.ip6.soiikey from userland
On Sat, May 20, 2023 at 07:47:46PM +0200, Florian Obser wrote: > On 2023-05-20 19:37 +02, Paul de Weerd wrote: > > On Sat, May 20, 2023 at 05:33:11PM +0200, Florian Obser wrote: > > | In case this turns out to be useful for unlocking work in the kernel. > > | > > | It's a minimum diff, if we want to go this way we probably want to move > > | init_soiikey() to the engine process and stop bouncing through the main > > | process when an interface changes. > > | > > | This changes behaviour: in -current we can change the sysctl and down/up > > | an interface to read the new value, with this diff that no longer > > | works. slaacd will (and can) only read the file on startup. Having to restart slaacd when changing soii.key at runtime seems fine to me, it's a seed and not really a config file, anyway. > > | > > | This has consequences for the installer: slaacd starts before > > | /mnt/etc/soii.key is available in the upgrade case. Which in turn means > > | that we get a different IPv6 address in the installer than in the > > | running system. I don't know how big of an issue that is. > > > > Can't speak for others, but my use case for soii-addresses is for > > incoming connections - outgoing ones should use the temporary privacy > > addressed. So for the installer it doesn't matter. > > > > My guess is that this goes for many (most? all?) users of > > soii-addresses, so it should be safe to not have those in the > > installer during upgrades. > > I'm worried that someone runs a completely locked down network. > They installed a server, figured out the random but stable IP address > and added it to their firewall and only allow communication from known > IP addresses. > > They will be fine if they use sysupgrade(8). The installed base is > probably smaller than 6. Lots of time still until release to find out for real.
