Re: ttymalloc(int baud)
The decision to allocate more storage is decided by an unexpected usage pattern we encountered after the API was designed, which isn't immediately obvious from the device name... Some of these devices are designed for high-speed PPP. Some of them (in particular on USB) actually move bytes faster than their advertised baud rate. They were not working well. So the buffers were grown. That also adjusted the watermarks, and then things worked better. tty.c 1.128 But was the same good for all ttys? Do consider the calculated watermarks. At the time -- cautiously -- we didn't want to grow the buffers for all the ttys. Maybe that decision can change? This one leads me to think we can: tty_pty.c: pti->pt_tty = ttymalloc(100); 992 maximum ptys, that's almost 8MB of memory. But we come back to the watermark calculation... which is why the expected baud rate (maximum baud rate?) is the argument.. Wow. I wrote this code earlier than 1993 when replacing the broken Jolitz rings and creating a proper clist-compat layer... wrote: > Another trivial code readability comment: > > I see 24 references to this in the code, 20 of which pass 0, and 4 of > which pass 1,000,000 as the parameter. > > Passing 0 defaults to a baud of 115200. The baud determines the qlen, > which is 4096 for 115200 and 8192 for anything larger, so all it is > doing now is selecting between two buffer sizes via magic number > parameters. > > It would probably be appropriate today to just always use 8192 and not > even take a parameter; it doesn't look like the selection between the > two is done in a rigorous way. > > There is also a comment in umodem.c that has an outdated assumption: > /* > * These are the maximum number of bytes transferred per frame. > * Buffers are this large to deal with high speed wireless devices. > * Capped at 1024 as ttymalloc() is limited to this amount. > */ > #define UMODEMIBUFSIZE 1024 > #define UMODEMOBUFSIZE 1024 Yes, that is weird and wrong.
Re: libpcap: allow breaking out of loop when using savefile
I just committed this, thank you :) dlg > On 16 May 2020, at 05:14, Caspar Schutijser wrote: > > Hi, > > Below is a patch that makes breaking out of the loop work when using > a savefile. > > The pcap_breakloop() function was backported from tcpdump.org libpcap > to OpenBSD libpcap by djm@ on Nov 18, 2005. The bits to make > pcap_breakloop() work were backported to pcap-bpf.c [1] but not to > savefile.c even though tcpdump.org implemented support there too [2]. > > The diff below backports this piece of code to savefile.c after all. > > Thanks, > Caspar Schutijser > > [1] > https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/lib/libpcap/pcap-bpf.c.diff?r1=1.16=1.17 > [2] > https://github.com/the-tcpdump-group/libpcap/commit/991d444f7116bef16893826b46f3950f62281507#diff-95d4d29d0f11145ff40b850860667a97R745 > > > Index: savefile.c > === > RCS file: /cvs/src/lib/libpcap/savefile.c,v > retrieving revision 1.16 > diff -u -p -r1.16 savefile.c > --- savefile.c22 Dec 2015 19:51:04 - 1.16 > +++ savefile.c15 May 2020 19:03:44 - > @@ -307,6 +307,23 @@ pcap_offline_read(pcap_t *p, int cnt, pc > while (status == 0) { > struct pcap_pkthdr h; > > + /* > + * Has "pcap_breakloop()" been called? > + * If so, return immediately - if we haven't read any > + * packets, clear the flag and return -2 to indicate > + * that we were told to break out of the loop, otherwise > + * leave the flag set, so that the *next* call will break > + * out of the loop without having read any packets, and > + * return the number of packets we've processed so far. > + */ > + if (p->break_loop) { > + if (n == 0) { > + p->break_loop = 0; > + return (PCAP_ERROR_BREAK); > + } else > + return (n); > + } > + > status = sf_next_packet(p, , p->buffer, p->bufsize); > if (status) { > if (status == 1) >
Re: [PATCH] pipex(4): rework PPP input
> On 27 May 2020, at 01:29, Sergey Ryazanov wrote: > > On Tue, May 26, 2020 at 12:07 PM Vitaliy Makkoveev > wrote: >>> On 25 May 2020, at 22:04, Sergey Ryazanov wrote: >>> On Sat, May 23, 2020 at 3:07 PM Vitaliy Makkoveev >>> wrote: For example, each pipex session should have unique pair of `protocol’ and `session_id’. These values are passed from userland. While the only instance of npppd(8) uses pipex(4) this is not the problem. But you introduce the case while pipex(4) will be used by multiple independent userland programs. At least, I have interest how you handle this. >>> >>> This should not be a problem here. npppd(8) support server mode only. >>> While my work is to implement acceleration for client side of L2TP >>> connection. >> >> I guess they can coexist. Also you can have multiple connections to >> ppp servers simultaneously. > > With 16 bits long session id field, according to birthday problem to > reach 0.9 collision probability I need 549 simultaneous sessions. > Should I still be worried or I have a time to complete integration > work and then update UDP filter for love of the game? Why do you ask me? Do what you want. > > -- > Sergey >
Re: WireGuard patchset for OpenBSD, rev. 2
On Tue, May 26, 2020 at 4:52 PM Jason A. Donenfeld wrote: > With regards to your crash, though, that's a bit more puzzling, and > I'd be interested to learn more details. Because these structs are > already naturally aligned, the __packed attribute, even with the odd > nesting Matt had prior, should have produced all entirely aligned > accesses. That makes me think your kaboom was coming from someplace > else. One possibility is that you were running the git tree on the two > days that I was playing with uint128_t, only to find out that some of > openbsd's patches to clang miscalculate stack sizes when they're in > use, so that work was shelved for another day and the commits removed; > perhaps you were just unlucky? Or you hit some other bug that's > lurking. Either way, output from ddb's `bt` would at least be useful. Do you know off hand if we're able to assume any type of alignment with mbuf->m_data? mtod just casts without any address fixup, which means if mbuf->m_data isn't aligned by some other mechanism, we're in trouble. But I would assume there _is_ some alignment imposed, since the rest of the stack appears to parse tcp headers and such directly without byte-by-byte copies being made.
Re: [PATCH] pipex(4): rework PPP input
On Wed, May 27, 2020 at 2:12 AM Vitaliy Makkoveev wrote: > > On 27 May 2020, at 01:29, Sergey Ryazanov wrote: > > On Tue, May 26, 2020 at 12:07 PM Vitaliy Makkoveev > > wrote: > >>> On 25 May 2020, at 22:04, Sergey Ryazanov wrote: > >>> On Sat, May 23, 2020 at 3:07 PM Vitaliy Makkoveev > >>> wrote: > For example, each pipex session should have unique pair of `protocol’ and > `session_id’. These values are passed from userland. While the only > instance of npppd(8) uses pipex(4) this is not the problem. But you > introduce the case while pipex(4) will be used by multiple independent > userland programs. At least, I have interest how you handle this. > >>> > >>> This should not be a problem here. npppd(8) support server mode only. > >>> While my work is to implement acceleration for client side of L2TP > >>> connection. > >> > >> I guess they can coexist. Also you can have multiple connections to > >> ppp servers simultaneously. > > > > With 16 bits long session id field, according to birthday problem to > > reach 0.9 collision probability I need 549 simultaneous sessions. > > Should I still be worried or I have a time to complete integration > > work and then update UDP filter for love of the game? > > Why do you ask me? Do what you want. I want to prioritize my todo list, and I in need of advice, is possible collision is non-critical at the moment, or I missed something and should solve it first? -- Serge
Re: [PATCH] tcpdump: add ppp address/protocol compression support
Hello! On Mon, May 4, 2020 at 9:37 PM Sergey Ryazanov wrote: > Add support for parsing ppp frames with compressed address and(or) > protocol fields. Since we have no apriory information than try to > guess such frames by inability to parse a frame in a regular way. > > ok? Does someone have any objections on this diff? -- Sergey
Re: [RFC] pppd: add pipex(4) L2TP control support
On Tue, May 26, 2020 at 11:31 AM Claudio Jeker wrote: > On Tue, May 26, 2020 at 09:22:28AM +0200, Martin Pieuchot wrote: > > On 25/05/20(Mon) 21:42, Sergey Ryazanov wrote: > > > Add dedicated option to activate kernel L2TP acceleration via > > > the pipex(4). The options should be passed by a L2TP tunnel > > > management daemon (e.g. xl2tpd). > > > > What is the difference between npppd(8) and pppd(8)? Aren't those two > > redundant? Why did you choose to modify pppd(8) and not npppd(8)? > > npppd(8) is server only it can not establish a connection. pppd(8) on the > other hand is more client side (but I think it can do both). True. > I think the split in userland makes sense (using npppd as LNS/LAC and pppd > to connect to a ISP via PPP (even though we also use sppp/pppoe(4) for > that). Now for the kernel side it would be great if the interface used > would be the same for all ppp users. In this regard this is a step in the > right direction. Comparing addition of client support to npppd and reworking pppd to support pppx(4), I will choose the npppd reworking. The pppd code is the mess that is too tightly connected with TTY. Moreover, pppd is designed to establish the PPP connection over any type of serial line, including RS232 connected Dial-Up modem. So introducing the pppx(4) support will require preserving of all existing code *and* creation of a big module that is responsible pppx(4) handling in case of L2TP connection. And such feature will require a hard xl2tpd daemon reworking too. It is easier to rework npppd or ever create a new L2TP client daemon from the scratch. So I found a 3rd solution - add a pipex(4) support for the good old-fashioned ppp(4) and a tiny option for pppd. And this my plan if someone do not have a better solution. > > > diff --git usr.sbin/pppd/ipcp.c usr.sbin/pppd/ipcp.c > > > index 1296d897b14..e7a6dbd6ee7 100644 > > > --- usr.sbin/pppd/ipcp.c > > > +++ usr.sbin/pppd/ipcp.c > > > @@ -1251,6 +1251,10 @@ ipcp_up(f) > > > return; > > > } > > > > > > + /* enable pipex(4), keep working if failed */ > > > + if (pipex_conf.pr_protocol && !apipex(go->ouraddr, ho->hisaddr, mask)) > > > + IPCPDEBUG((LOG_WARNING, "apipex failed")); > > > + > > > #if (defined(SVR4) && (defined(SNI) || defined(__USLC__))) > > > if (!sifaddr(f->unit, go->ouraddr, ho->hisaddr, mask)) { > > > IPCPDEBUG((LOG_WARNING, "sifaddr failed")); > > > @@ -1304,6 +1308,7 @@ ipcp_down(f) > > > if (demand) { > > > sifnpmode(f->unit, PPP_IP, NPMODE_QUEUE); > > > } else { > > > + dpipex(); > > > sifdown(f->unit); > > > ipcp_clear_addrs(f->unit); > > > } > > > diff --git usr.sbin/pppd/options.c usr.sbin/pppd/options.c > > > index 828f7cdce65..fe0e2e6b54b 100644 > > > --- usr.sbin/pppd/options.c > > > +++ usr.sbin/pppd/options.c > > > @@ -144,6 +144,9 @@ struct bpf_program pass_filter;/* Filter program for > > > packets to pass */ > > > struct bpf_program active_filter; /* Filter program for link-active > > > pkts */ > > > pcap_t pc;/* Fake struct pcap so we can compile > > > expr */ > > > #endif > > > +struct pipex_session_req pipex_conf = {/* pipex(4) session > > > configuration */ > > > + .pr_protocol = 0, > > > +}; > > > > > > /* > > > * Prototypes > > > @@ -253,6 +256,8 @@ static int setactivefilter(char **); > > > static int setmslanman(char **); > > > #endif > > > > > > +static int setpipexl2tp(char **); > > > + > > > static int number_option(char *, u_int32_t *, int); > > > static int int_option(char *, int *); > > > static int readable(int fd); > > > @@ -391,6 +396,8 @@ static struct cmd { > > > {"ms-lanman", 0, setmslanman}, /* Use LanMan psswd when using > > > MS-CHAP */ > > > #endif > > > > > > +{"pipex-l2tp", 6, setpipexl2tp}, /* set pipex(4) L2TP > > > parameters */ > > > + > > > {NULL, 0, NULL} > > > }; > > > > > > @@ -2283,3 +2290,78 @@ setmslanman(argv) > > > return (1); > > > } > > > #endif > > > + > > > +static int > > > +inet_atoss(cp, ss) > > > +char *cp; > > > +struct sockaddr_storage *ss; > > > +{ > > > +struct sockaddr_in *sin = (struct sockaddr_in *)ss; > > > +char *c, *p; > > > +unsigned port; > > > + > > > +if ((c = strchr(cp, ':')) == NULL) > > > + return 0; > > > +*c = '\0'; > > > +if (!inet_aton(cp, >sin_addr)) > > > + return 0; > > > +if (!(port = strtoul(c + 1, , 10)) || *p != '\0') > > > + return 0; > > > +sin->sin_port = htons(port); > > > +sin->sin_family = AF_INET; > > > +ss->ss_len = sizeof(*sin); > > > + > > > +return 1; > > > +} > > > + > > > +static int > > > +strtou16(str, valp) > > > +char *str; > > > +uint16_t *valp; > > > +{ > > > +char *ptr; > > > + > > > +*valp = strtoul(str, , 0); > > > + > > > +return *ptr == '\0' ? 1 : 0; > > > +} > > > + > > > +static int > > > +setpipexl2tp(argv) > > > +char **argv; > > > +{ > > > +BZERO((char *)_conf,
Re: WireGuard patchset for OpenBSD, rev. 2
Hey Klemens, Theo, On Tue, May 26, 2020 at 2:38 PM Klemens Nanni wrote: > > On Tue, May 26, 2020 at 02:23:06PM -0600, Jason A. Donenfeld wrote: > > That's good news that it's working for you now, but I didn't change > > anything within the last 24 hours (you mentioned "yesterday") that > > would seem related to this. So perhaps don't get too excited just yet. > I was surprised, too. Perhaps something went wrong applying all those > single patches from the wireguard-openbsd resository, but if so, I'd > doubt that it would build in the first place... > > So far the interface is stable, but I keep testing. When I find the > time, I'll also take a look at the pieces of code which blew up earlier. My audit is complete, and every single usage of __packed has been removed. Those never should have been there in the first place. WireGuard was specifically designed for kernels, and I distinctly remember making sure things would be high performance on little mips devices, which necessitated natural struct alignment. And indeed, all of the structs that wireguard uses on the wire have fields that are always aligned to each field's size, so no __packed is ever necessary. This all has been fixed up in the git repo now. With regards to your crash, though, that's a bit more puzzling, and I'd be interested to learn more details. Because these structs are already naturally aligned, the __packed attribute, even with the odd nesting Matt had prior, should have produced all entirely aligned accesses. That makes me think your kaboom was coming from someplace else. One possibility is that you were running the git tree on the two days that I was playing with uint128_t, only to find out that some of openbsd's patches to clang miscalculate stack sizes when they're in use, so that work was shelved for another day and the commits removed; perhaps you were just unlucky? Or you hit some other bug that's lurking. Either way, output from ddb's `bt` would at least be useful. Jason
Re: packets to bridged interfaces bypass input filter
On Tue, May 26, 2020 at 09:26:07PM +0200, Sven M. Hallberg wrote: > hi all, > > i sent the following question to misc@ on march 29th but received no > response. i hope you don't mind me retrying on tech@. > > while playing around with pf, i noticed that some connections that i > thought should be blocked, were in fact not. here is my fairly standard > bridge setup between a wlan interface and a wired ethernet. > ># cat /etc/hostname.em0 >group lan >up > ># cat /etc/hostname.athn0 >mediaopt hostap >nwid xxx wpakey abcd1234 >group lan >up > ># cat /etc/hostname.vether0 >group lan >inet 192.168.22.145/28 > ># cat /etc/hostname.bridge0 >add em0 add athn0 add vether0 >blocknonip athn0 >up > > i then set up pf rules that should let the (w)lan interfaces talk to > the world. > > # cat /etc/pf.conf > set skip on lo > block return > pass out# ingress filter > pass in on lan from lan:network to any > > now this all works as intended. but, when i remove athn0 from the lan > group, i would expect the above ruleset to block any incoming > connections from the wlan. > > # ifconfig athn0 -group lan Did you flush states between your tests (pfctl -F states) ? -> After you removed athn0 from lan group ? > > and indeed, i could not ping a wired host from the wlan. however, i > could still access the router (192.168.22.145) itself from the wireless. > as far as i can tell, this is due to the following code in if_bridge.c, > bridge_process() -- comments by me: > > if (bridge_ourether(bif0->ifp, eh->ether_dhost)) { > /* addressed to the iface it came in on */ > bif = bif0; > } else { > SMR_SLIST_FOREACH_LOCKED(bif, >sc_iflist, bif_next) { > if (bif->ifp == ifp) > continue; > if (bridge_ourether(bif->ifp, eh->ether_dhost)) > /* addressed to some other bridge member */ > break; > } > } > if (bif != NULL) { > /* ... */ > if (bridge_filterrule(>bif_brlin, eh, m) == > BRL_ACTION_BLOCK) { > goto bad; > } > /* ... */ > } > > a packet that comes in on a member of a bridge and is addressed to _any_ > bridge member interface, is processed as input to (only) the destination > interface. if that destination interface differs from the one that the > packet actually came in on, pf rules for the actual input interface are > not evaluated. > > this seems to contradict the following statement in the NOTES section of > bridge(4) -- in any case, it certainly contradicted my intuition: > > Bridged packets pass through pf(4) filters once as input on the receiving > interface and once as output on all interfaces on which they are > forwarded. In order to pass through the bridge packets must pass any in > rules on the input and any out rules on the output interface. Packets > may be blocked either entering or leaving the bridge. > > now, to be clear, i don't mind that the packet passes through 'in' rules for > vether0 in my example, but i wonder if it should not also pass through > the 'in' rules for 'athn0'. > > i would appreciate enlightenment on this. note that (contrary to misc) i > am subscribed to tech@, so no cc needed. > > > thanks, > pesco > > PS: at the time, i was using OpenBSD 6.6, but the code in question > appears unchanged in current. >
Re: [PATCH] pipex(4): rework PPP input
On Tue, May 26, 2020 at 12:07 PM Vitaliy Makkoveev wrote: >> On 25 May 2020, at 22:04, Sergey Ryazanov wrote: >> On Sat, May 23, 2020 at 3:07 PM Vitaliy Makkoveev >> wrote: >>> For example, each pipex session should have unique pair of `protocol’ and >>> `session_id’. These values are passed from userland. While the only >>> instance of npppd(8) uses pipex(4) this is not the problem. But you >>> introduce the case while pipex(4) will be used by multiple independent >>> userland programs. At least, I have interest how you handle this. >> >> This should not be a problem here. npppd(8) support server mode only. >> While my work is to implement acceleration for client side of L2TP >> connection. > > I guess they can coexist. Also you can have multiple connections to > ppp servers simultaneously. With 16 bits long session id field, according to birthday problem to reach 0.9 collision probability I need 549 simultaneous sessions. Should I still be worried or I have a time to complete integration work and then update UDP filter for love of the game? -- Sergey
[patch] correct return type in pcap_open_live.3
Please see attached diff. Index: pcap_open_live.3 === RCS file: /cvs/src/lib/libpcap/pcap_open_live.3,v retrieving revision 1.3 diff -u -p -u -r1.3 pcap_open_live.3 --- pcap_open_live.325 Sep 2019 17:02:00 - 1.3 +++ pcap_open_live.326 May 2020 22:09:24 - @@ -95,7 +95,7 @@ .Fn pcap_dump_fopen "pcap_t *p" "FILE *f" .Ft "char *" .Fn pcap_lookupdev "char *errbuf" -.Ft uint +.Ft int .Fn pcap_lookupnet "const char *device" "bpf_u_int32 *netp" "bpf_u_int32 *maskp" "char *errbuf" .Ft int .Fn pcap_dispatch "pcap_t *p" "int cnt" "pcap_handler callback" "u_char *user"
ttymalloc(int baud)
Another trivial code readability comment: I see 24 references to this in the code, 20 of which pass 0, and 4 of which pass 1,000,000 as the parameter. Passing 0 defaults to a baud of 115200. The baud determines the qlen, which is 4096 for 115200 and 8192 for anything larger, so all it is doing now is selecting between two buffer sizes via magic number parameters. It would probably be appropriate today to just always use 8192 and not even take a parameter; it doesn't look like the selection between the two is done in a rigorous way. There is also a comment in umodem.c that has an outdated assumption: /* * These are the maximum number of bytes transferred per frame. * Buffers are this large to deal with high speed wireless devices. * Capped at 1024 as ttymalloc() is limited to this amount. */ #define UMODEMIBUFSIZE 1024 #define UMODEMOBUFSIZE 1024
Re: WireGuard patchset for OpenBSD, rev. 2
On Tue, May 26, 2020 at 2:33 PM Theo de Raadt wrote: > > Jason A. Donenfeld wrote: > > > Hey Klemens, > > > > On Tue, May 26, 2020 at 9:13 AM Klemens Nanni wrote: > > > I worked with the patches from the wireguard-openbsd repository after > > > version one of this diff on tech@ became a bit old. > > > > > > That was until yesterday; the kernel would panic due to memory > > > alignment issues in various spots, `amd64# ping6 tunnel-ip.sparc64' > > > would receive a single ICMP6 echo reply from the sparc64 peer before > > > panicing its kernel very early in noise init code. > > > > > > I've had fixed a few bugs and ran into different panics during later > > > code paths. > > > > > > Fortunately, using "rev. 2" of your diff on top of a clean checkout just > > > works so far on sparc64 without any additional patches required - this > > > is a very pleasnt suprise and I can start looking at it under production > > > use cases more thoroughly now. > > > > That's good news that it's working for you now, but I didn't change > > anything within the last 24 hours (you mentioned "yesterday") that > > would seem related to this. So perhaps don't get too excited just yet. > > I've got a decent MIPS setup here and a hacked up qemu for having a > > bit more detail on alignment traps. I'll see if I can reproduce any > > issues. > > I suspect problems around forcing __packed. > > __packed is very dangerous. > > If you know the sizes of the objects are correctly sized and placed to > not have gap-pads, then not using __packed might be better. Right, most times, uses of packed are wrong and can be avoided. I'll work through that all and see what can be minimized. In a cursory look I did see code to the effect of `struct { u32 a; u64 b; } __packed`, which could trap. Jason
Re: WireGuard patchset for OpenBSD, rev. 2
Jason A. Donenfeld wrote: > Hey Klemens, > > On Tue, May 26, 2020 at 9:13 AM Klemens Nanni wrote: > > I worked with the patches from the wireguard-openbsd repository after > > version one of this diff on tech@ became a bit old. > > > > That was until yesterday; the kernel would panic due to memory > > alignment issues in various spots, `amd64# ping6 tunnel-ip.sparc64' > > would receive a single ICMP6 echo reply from the sparc64 peer before > > panicing its kernel very early in noise init code. > > > > I've had fixed a few bugs and ran into different panics during later > > code paths. > > > > Fortunately, using "rev. 2" of your diff on top of a clean checkout just > > works so far on sparc64 without any additional patches required - this > > is a very pleasnt suprise and I can start looking at it under production > > use cases more thoroughly now. > > That's good news that it's working for you now, but I didn't change > anything within the last 24 hours (you mentioned "yesterday") that > would seem related to this. So perhaps don't get too excited just yet. > I've got a decent MIPS setup here and a hacked up qemu for having a > bit more detail on alignment traps. I'll see if I can reproduce any > issues. I suspect problems around forcing __packed. __packed is very dangerous. If you know the sizes of the objects are correctly sized and placed to not have gap-pads, then not using __packed might be better. __packed won't sae you from leaking gap bytes, either. We only run on ILP32 and I32LP64 ABIs, and all of them pack in the classic way...
packets to bridged interfaces bypass input filter
hi all, i sent the following question to misc@ on march 29th but received no response. i hope you don't mind me retrying on tech@. while playing around with pf, i noticed that some connections that i thought should be blocked, were in fact not. here is my fairly standard bridge setup between a wlan interface and a wired ethernet. # cat /etc/hostname.em0 group lan up # cat /etc/hostname.athn0 mediaopt hostap nwid xxx wpakey abcd1234 group lan up # cat /etc/hostname.vether0 group lan inet 192.168.22.145/28 # cat /etc/hostname.bridge0 add em0 add athn0 add vether0 blocknonip athn0 up i then set up pf rules that should let the (w)lan interfaces talk to the world. # cat /etc/pf.conf set skip on lo block return pass out# ingress filter pass in on lan from lan:network to any now this all works as intended. but, when i remove athn0 from the lan group, i would expect the above ruleset to block any incoming connections from the wlan. # ifconfig athn0 -group lan and indeed, i could not ping a wired host from the wlan. however, i could still access the router (192.168.22.145) itself from the wireless. as far as i can tell, this is due to the following code in if_bridge.c, bridge_process() -- comments by me: if (bridge_ourether(bif0->ifp, eh->ether_dhost)) { /* addressed to the iface it came in on */ bif = bif0; } else { SMR_SLIST_FOREACH_LOCKED(bif, >sc_iflist, bif_next) { if (bif->ifp == ifp) continue; if (bridge_ourether(bif->ifp, eh->ether_dhost)) /* addressed to some other bridge member */ break; } } if (bif != NULL) { /* ... */ if (bridge_filterrule(>bif_brlin, eh, m) == BRL_ACTION_BLOCK) { goto bad; } /* ... */ } a packet that comes in on a member of a bridge and is addressed to _any_ bridge member interface, is processed as input to (only) the destination interface. if that destination interface differs from the one that the packet actually came in on, pf rules for the actual input interface are not evaluated. this seems to contradict the following statement in the NOTES section of bridge(4) -- in any case, it certainly contradicted my intuition: Bridged packets pass through pf(4) filters once as input on the receiving interface and once as output on all interfaces on which they are forwarded. In order to pass through the bridge packets must pass any in rules on the input and any out rules on the output interface. Packets may be blocked either entering or leaving the bridge. now, to be clear, i don't mind that the packet passes through 'in' rules for vether0 in my example, but i wonder if it should not also pass through the 'in' rules for 'athn0'. i would appreciate enlightenment on this. note that (contrary to misc) i am subscribed to tech@, so no cc needed. thanks, pesco PS: at the time, i was using OpenBSD 6.6, but the code in question appears unchanged in current.
Re: WireGuard patchset for OpenBSD, rev. 2
Hey Klemens, On Tue, May 26, 2020 at 9:13 AM Klemens Nanni wrote: > I worked with the patches from the wireguard-openbsd repository after > version one of this diff on tech@ became a bit old. > > That was until yesterday; the kernel would panic due to memory > alignment issues in various spots, `amd64# ping6 tunnel-ip.sparc64' > would receive a single ICMP6 echo reply from the sparc64 peer before > panicing its kernel very early in noise init code. > > I've had fixed a few bugs and ran into different panics during later > code paths. > > Fortunately, using "rev. 2" of your diff on top of a clean checkout just > works so far on sparc64 without any additional patches required - this > is a very pleasnt suprise and I can start looking at it under production > use cases more thoroughly now. That's good news that it's working for you now, but I didn't change anything within the last 24 hours (you mentioned "yesterday") that would seem related to this. So perhaps don't get too excited just yet. I've got a decent MIPS setup here and a hacked up qemu for having a bit more detail on alignment traps. I'll see if I can reproduce any issues. Jason
Re: WireGuard patchset for OpenBSD, rev. 2
Hey Tobias, On Tue, May 26, 2020 at 5:28 AM Tobias Heider wrote: > > + if (((SIZE_MAX - size) / sizeof(struct wg_aip_io)) < sc->sc_aip_num) > > + goto error; > > I still think those two should return an error. 'goto error' is misleading as > it doesn't actually set ret != 0. 'error' should probably be renamed > to 'cleanup' to avoid confusion and ret should be set to ERANGE . I'll change the label name to be more clear; I agree "error" is misleading. Returning ERANGE wouldn't be correct though; we still want the struct to be copied back to the user so that they can use the information in it to optionally try again (optionally, because maybe they don't care about info) with a different buffer size.
pppx(4): prevent access to `pxi' being destroyed
`pppx_if' has `pxi_ready' field used to prevent access to incomplete `pxi'. But we don't prevent access to `pxi' which we destroy. pppx_if_destroy() can sleep so we can grab `pxi' which we already destroying by concurrent thread and cause use-after-free issue. I guess to use `pxi_ready' to prevent access to `pxi' at destroy stage too. Index: sys/net/if_pppx.c === RCS file: /cvs/src/sys/net/if_pppx.c,v retrieving revision 1.84 diff -u -p -r1.84 if_pppx.c --- sys/net/if_pppx.c 18 Apr 2020 04:03:56 - 1.84 +++ sys/net/if_pppx.c 26 May 2020 12:59:56 - @@ -594,8 +594,10 @@ pppxclose(dev_t dev, int flags, int mode /* XXX */ NET_LOCK(); - while ((pxi = LIST_FIRST(>pxd_pxis))) + while ((pxi = LIST_FIRST(>pxd_pxis))) { + pxi->pxi_ready = 0; pppx_if_destroy(pxd, pxi); + } NET_UNLOCK(); LIST_REMOVE(pxd, pxd_entry); @@ -951,6 +953,7 @@ pppx_del_session(struct pppx_dev *pxd, s pxi = pppx_if_find(pxd, req->pcr_session_id, req->pcr_protocol); if (pxi == NULL) return (EINVAL); + pxi->pxi_ready = 0; req->pcr_stat = pxi->pxi_session.stat;
Error reporting in ikev2_ike_sa_alive (was: Improve error reporting in pfkey_sa_last_used)
During childsa last use checks, iked debug logs results, per SA, after a successful pfkey_sa_last_used call. This patch makes logging behavior more closely match that, on error. I chose log_warn instead of log_debug since iked will complain about the nonzero errno after pfkey_reply: pfkey_sa_last_used: message: No such process With this patch an operator can at least troubleshoot which SAs are causing the trouble. Comments? Make sense? thank you, all matthew weaver --- Index: ikev2.c === RCS file: /cvs/src/sbin/iked/ikev2.c,v retrieving revision 1.223 diff -u -p -u -r1.223 ikev2.c --- ikev2.c 2 May 2020 13:01:37 - 1.223 +++ ikev2.c 26 May 2020 15:53:42 - @@ -4347,8 +4347,15 @@ ikev2_ike_sa_alive(struct iked *env, voi TAILQ_FOREACH(csa, >sa_childsas, csa_entry) { if (!csa->csa_loaded) continue; - if (pfkey_sa_last_used(env->sc_pfkey, csa, _used) != 0) + if (pfkey_sa_last_used(env->sc_pfkey, csa, _used) != 0) { + log_warn( + "%s: %s CHILD SA spi %s failed to determine " + "last use", __func__, + csa->csa_dir == IPSP_DIRECTION_IN ? + "incoming" : "outgoing", + print_spi(csa->csa_spi.spi, csa->csa_spi.spi_size)); continue; + } diff = (uint32_t)(gettime() - last_used); log_debug("%s: %s CHILD SA spi %s last used %llu second(s) ago", __func__,
Re: WireGuard patchset for OpenBSD, rev. 2
On Tue, May 26, 2020 at 08:09:48AM -0600, Theo de Raadt wrote: > I'll let you know who has sparc64 machines to help out: > > kn was the developer who saw the problem. jca is also adept > enough to look at this with you. I worked with the patches from the wireguard-openbsd repository after version one of this diff on tech@ became a bit old. That was until yesterday; the kernel would panic due to memory alignment issues in various spots, `amd64# ping6 tunnel-ip.sparc64' would receive a single ICMP6 echo reply from the sparc64 peer before panicing its kernel very early in noise init code. I've had fixed a few bugs and ran into different panics during later code paths. Fortunately, using "rev. 2" of your diff on top of a clean checkout just works so far on sparc64 without any additional patches required - this is a very pleasnt suprise and I can start looking at it under production use cases more thoroughly now. playground$ arch -s ; doas ifconfig wg0 sparc64 wg0: flags=8043 mtu 1420 index 5 priority 0 llprio 3 wgport 2345 wgpubkey mrtNB07tzEJKyJDvhaov7QYt487BXLK3hnnZB+pDIhM= wgpeer 9xuyga6Z/W7l/vnLIl67PiRcf57VujfoqpIH5VArpR4= wgendpoint 2001:xxx 2345 tx: 701892, rx: 13283708 last handshake: 29 seconds ago wgaip ::/0 groups: wg inet6 2001:db8::1 prefixlen 126
Re: filesystem code integer and many inodes
On Tue, May 26, 2020 at 03:54:15PM +0200, Otto Moerbeek wrote: > On Tue, May 26, 2020 at 07:51:28AM -0600, Todd C. Miller wrote: > > > On Tue, 26 May 2020 12:07:21 +0200, Otto Moerbeek wrote: > > > > > Apart from the noting the strange Subject: I also like to mention one > > > change in the way cylinder groups are scanned. The current code scans > > > forward and backward, which causes an uneven distribution of full cgs > > > (the upper end of the cgs will get full first). Fix that by always > > > scanning forward, wrapping to cg 0 if needed. > > > > Should that be a separate commit? I can't find any problems > > with the diff but I haven't tried running with it yet. > > > > - todd > > Yeah, I can do that. Note that it must be comitted first, since the > loop condition is always true if I change the loop var to unsigned. > > -Otto > And a new diff. I accidentally capitalized a letter just before sending. Thanks to naddy for spotting that. -Otto Index: sbin/clri/clri.c === RCS file: /cvs/src/sbin/clri/clri.c,v retrieving revision 1.20 diff -u -p -r1.20 clri.c --- sbin/clri/clri.c28 Jun 2019 13:32:43 - 1.20 +++ sbin/clri/clri.c26 May 2020 09:41:18 - @@ -68,7 +68,8 @@ main(int argc, char *argv[]) char *fs, sblock[SBLOCKSIZE]; size_t bsize; off_t offset; - int i, fd, imax, inonum; + int i, fd; + ino_t imax, inonum; if (argc < 3) usage(); Index: sbin/dumpfs/dumpfs.c === RCS file: /cvs/src/sbin/dumpfs/dumpfs.c,v retrieving revision 1.35 diff -u -p -r1.35 dumpfs.c --- sbin/dumpfs/dumpfs.c17 Feb 2020 16:11:25 - 1.35 +++ sbin/dumpfs/dumpfs.c26 May 2020 09:41:18 - @@ -69,7 +69,7 @@ union { #define acgcgun.cg intdumpfs(int, const char *); -intdumpcg(const char *, int, int); +intdumpcg(const char *, int, u_int); intmarshal(const char *); intopen_disk(const char *); void pbits(void *, int); @@ -163,6 +163,7 @@ dumpfs(int fd, const char *name) size_t size; off_t off; int i, j; + u_int cg; switch (afs.fs_magic) { case FS_UFS2_MAGIC: @@ -172,7 +173,7 @@ dumpfs(int fd, const char *name) afs.fs_magic, ctime()); printf("superblock location\t%jd\tid\t[ %x %x ]\n", (intmax_t)afs.fs_sblockloc, afs.fs_id[0], afs.fs_id[1]); - printf("ncg\t%d\tsize\t%jd\tblocks\t%jd\n", + printf("ncg\t%u\tsize\t%jd\tblocks\t%jd\n", afs.fs_ncg, (intmax_t)fssize, (intmax_t)afs.fs_dsize); break; case FS_UFS1_MAGIC: @@ -198,7 +199,7 @@ dumpfs(int fd, const char *name) printf("cylgrp\t%s\tinodes\t%s\tfslevel %d\n", i < 1 ? "static" : "dynamic", i < 2 ? "4.2/4.3BSD" : "4.4BSD", i); - printf("ncg\t%d\tncyl\t%d\tsize\t%d\tblocks\t%d\n", + printf("ncg\t%u\tncyl\t%d\tsize\t%d\tblocks\t%d\n", afs.fs_ncg, afs.fs_ncyl, afs.fs_ffs1_size, afs.fs_ffs1_dsize); break; default: @@ -223,9 +224,9 @@ dumpfs(int fd, const char *name) (intmax_t)afs.fs_cstotal.cs_ndir, (intmax_t)afs.fs_cstotal.cs_nifree, (intmax_t)afs.fs_cstotal.cs_nffree); - printf("bpg\t%d\tfpg\t%d\tipg\t%d\n", + printf("bpg\t%d\tfpg\t%d\tipg\t%u\n", afs.fs_fpg / afs.fs_frag, afs.fs_fpg, afs.fs_ipg); - printf("nindir\t%d\tinopb\t%d\tmaxfilesize\t%ju\n", + printf("nindir\t%d\tinopb\t%u\tmaxfilesize\t%ju\n", afs.fs_nindir, afs.fs_inopb, (uintmax_t)afs.fs_maxfilesize); printf("sbsize\t%d\tcgsize\t%d\tcsaddr\t%jd\tcssize\t%d\n", @@ -238,10 +239,10 @@ dumpfs(int fd, const char *name) printf("nbfree\t%d\tndir\t%d\tnifree\t%d\tnffree\t%d\n", afs.fs_ffs1_cstotal.cs_nbfree, afs.fs_ffs1_cstotal.cs_ndir, afs.fs_ffs1_cstotal.cs_nifree, afs.fs_ffs1_cstotal.cs_nffree); - printf("cpg\t%d\tbpg\t%d\tfpg\t%d\tipg\t%d\n", + printf("cpg\t%d\tbpg\t%d\tfpg\t%d\tipg\t%u\n", afs.fs_cpg, afs.fs_fpg / afs.fs_frag, afs.fs_fpg, afs.fs_ipg); - printf("nindir\t%d\tinopb\t%d\tnspf\t%d\tmaxfilesize\t%ju\n", + printf("nindir\t%d\tinopb\t%u\tnspf\t%d\tmaxfilesize\t%ju\n", afs.fs_nindir, afs.fs_inopb, afs.fs_nspf, (uintmax_t)afs.fs_maxfilesize); printf("sbsize\t%d\tcgsize\t%d\tcgoffset %d\tcgmask\t0x%08x\n", @@ -261,7 +262,7 @@ dumpfs(int fd, const char *name) afs.fs_sblkno, afs.fs_cblkno, afs.fs_iblkno, afs.fs_dblkno);
Re: WireGuard patchset for OpenBSD, rev. 2
I'll let you know who has sparc64 machines to help out: kn was the developer who saw the problem. jca is also adept enough to look at this with you.
Re: filesystem code integer and many inodes
On Tue, May 26, 2020 at 07:51:28AM -0600, Todd C. Miller wrote: > On Tue, 26 May 2020 12:07:21 +0200, Otto Moerbeek wrote: > > > Apart from the noting the strange Subject: I also like to mention one > > change in the way cylinder groups are scanned. The current code scans > > forward and backward, which causes an uneven distribution of full cgs > > (the upper end of the cgs will get full first). Fix that by always > > scanning forward, wrapping to cg 0 if needed. > > Should that be a separate commit? I can't find any problems > with the diff but I haven't tried running with it yet. > > - todd Yeah, I can do that. Note that it must be comitted first, since the loop condition is always true if I change the loop var to unsigned. -Otto
Re: filesystem code integer and many inodes
On Tue, 26 May 2020 12:07:21 +0200, Otto Moerbeek wrote: > Apart from the noting the strange Subject: I also like to mention one > change in the way cylinder groups are scanned. The current code scans > forward and backward, which causes an uneven distribution of full cgs > (the upper end of the cgs will get full first). Fix that by always > scanning forward, wrapping to cg 0 if needed. Should that be a separate commit? I can't find any problems with the diff but I haven't tried running with it yet. - todd
Re: [PATCH] pipex(4): rework PPP input
> On 25 May 2020, at 22:04, Sergey Ryazanov wrote: > > Hello Vitaliy, > > On Sat, May 23, 2020 at 3:07 PM Vitaliy Makkoveev > wrote: >>> On 23 May 2020, at 13:11, Sergey Ryazanov wrote: >>> On Wed, May 20, 2020 at 10:13 PM Vitaliy Makkoveev >>> wrote: On Wed, May 20, 2020 at 04:08:01AM +0300, Sergey Ryazanov wrote: > On Tue, May 19, 2020 at 12:11 PM Vitaliy Makkoveev > wrote: >> On Mon, May 04, 2020 at 10:03:40PM +0300, Sergey Ryazanov wrote: >>> Split checks from frame accepting with header removing in the common >>> PPP input function. This should fix packet capture on a PPP interfaces. >> >> Can you describe the problem you fix? As mpi@ pointed to me, reviewers >> are stupid and have no telepathy skills :) > > When I tried to capture packets on a ppp (4) interface (with pipex > activated), I noticed that all the PPP CCP frames were ok, but all the > ingress PPP IP frames were mangled, and they did not contain the PPP > header at all. This time only pppx(4) and pppac(4) have pipex(4) support. >>> >>> Yes, and as I wrote in the first mail, now I am working on ppp(4) & >>> pipex(4) integration to speed up client side of L2TP. >> >> May be you can share you work? Not for commit, but for feedback. >> > > I send a couple of diffs in separate mails. First change is for ppp(4) > to support pipex(4) acceleration in DL path. Second diff adds a new > option for pppd to control pipex activation. > > We will need also to update xl2tpd package to teach it how to > configure pppd for pipex usage. This is quite simple change, but it > still require some cleanup so I do not publish it yet. > >> For example, each pipex session should have unique pair of `protocol’ and >> `session_id’. These values are passed from userland. While the only >> instance of npppd(8) uses pipex(4) this is not the problem. But you >> introduce the case while pipex(4) will be used by multiple independent >> userland programs. At least, I have interest how you handle this. > > This should not be a problem here. npppd(8) support server mode only. > While my work is to implement acceleration for client side of L2TP > connection. I guess they can coexist. Also you can have multiple connections to ppp servers simultaneously.
Re: iked(8): AES_GCM ciphers for IKE
On Thu, May 14, 2020 at 10:07:30PM +0200, Tobias Heider wrote: > Hi, > > currently iked(8) supports AES-GCM only for ESP. > The diff below adds the ENCR_AES_GCM_16 and ENCR_AES_GCM_12 variants for IKE. > (for more information see [1] and [2]). > Both variants support the 128, 196, and 256 bit key lengths. > > The new new ciphers can be configured with: > - aes-128-gcm, aes-196-gcm and aes-256-gcm for ENCR_AES_GCM_16 > - aes-128-gcm-12, aes-196-gcm-12 and aes-256-gcm-12 for ENCR_AES_GCM_12 > > It would be nice if we could get some interop testing with different IKEv2 > implementations. I have so far successfully tested strongswan <-> iked and > of course iked <-> iked. > > Feedback welcome ;) > > [1] https://tools.ietf.org/html/rfc5282 > [2] > https://www.iana.org/assignments/ikev2-parameters/ikev2-parameters.xhtml#ikev2-parameters-5 Here is the latest revision of the diff with a few more comments and without aes-192-gcm. We have so far successfully tested against iked, strongswan and juniper srx. ok? Index: crypto.c === RCS file: /cvs/src/sbin/iked/crypto.c,v retrieving revision 1.27 diff -u -p -r1.27 crypto.c --- crypto.c14 May 2020 15:08:30 - 1.27 +++ crypto.c26 May 2020 12:58:28 - @@ -92,7 +92,7 @@ hash_new(uint8_t type, uint16_t id) struct iked_hash*hash; const EVP_MD*md = NULL; HMAC_CTX*ctx = NULL; - int length = 0, fixedkey = 0, trunc = 0; + int length = 0, fixedkey = 0, trunc = 0, isaead = 0; switch (type) { case IKEV2_XFORMTYPE_PRF: @@ -156,6 +156,14 @@ hash_new(uint8_t type, uint16_t id) length = SHA512_DIGEST_LENGTH; trunc = 32; break; + case IKEV2_XFORMAUTH_AES_GCM_12: + length = 12; + isaead = 1; + break; + case IKEV2_XFORMAUTH_AES_GCM_16: + length = 16; + isaead = 1; + break; case IKEV2_XFORMAUTH_NONE: case IKEV2_XFORMAUTH_DES_MAC: case IKEV2_XFORMAUTH_KPDK_MD5: @@ -177,7 +185,7 @@ hash_new(uint8_t type, uint16_t id) print_map(id, ikev2_xformtype_map)); break; } - if (md == NULL) + if (!isaead && md == NULL) return (NULL); if ((hash = calloc(1, sizeof(*hash))) == NULL) { @@ -192,6 +200,10 @@ hash_new(uint8_t type, uint16_t id) hash->hash_trunc = trunc; hash->hash_length = length; hash->hash_fixedkey = fixedkey; + hash->hash_isaead = isaead; + + if (isaead) + return (hash); if ((ctx = calloc(1, sizeof(*ctx))) == NULL) { log_debug("%s: alloc hash ctx", __func__); @@ -276,6 +288,7 @@ cipher_new(uint8_t type, uint16_t id, ui const EVP_CIPHER*cipher = NULL; EVP_CIPHER_CTX *ctx = NULL; int length = 0, fixedkey = 0, ivlength = 0; + int saltlength = 0, authid = 0; switch (type) { case IKEV2_XFORMTYPE_ENCR: @@ -309,6 +322,36 @@ cipher_new(uint8_t type, uint16_t id, ui ivlength = EVP_CIPHER_iv_length(cipher); fixedkey = EVP_CIPHER_key_length(cipher); break; + case IKEV2_XFORMENCR_AES_GCM_16: + case IKEV2_XFORMENCR_AES_GCM_12: + switch (id_length) { + case 128: + cipher = EVP_aes_128_gcm(); + break; + case 256: + cipher = EVP_aes_256_gcm(); + break; + default: + log_debug("%s: invalid key length %d" + " for cipher %s", __func__, id_length, + print_map(id, ikev2_xformencr_map)); + break; + } + if (cipher == NULL) + break; + switch(id) { + case IKEV2_XFORMENCR_AES_GCM_16: + authid = IKEV2_XFORMAUTH_AES_GCM_16; + break; + case IKEV2_XFORMENCR_AES_GCM_12: + authid = IKEV2_XFORMAUTH_AES_GCM_12; + break; + } + length = EVP_CIPHER_block_size(cipher); + ivlength = 8; + saltlength = 4; + fixedkey = EVP_CIPHER_key_length(cipher) + saltlength; +
Re: [RFC] pppd: add pipex(4) L2TP control support
> On 26 May 2020, at 11:31, Claudio Jeker wrote: > > [skip] > > Is pppd(8) still using K function declarations? Can we please add new > functions with ANSI declarations instead and convert the rest as well. > Also it looks like something strange is going on with indentation (just > look at the last function above, it seems lines start with 4 spaces > instead of 1 tab). Again this could be bad pppd code. pppd(8) is not KNF compliant.
iwx: fix tx queue index in iwx_tx()
When enabling Tx queues in iwx_enable_data_tx_queues() the driver computes the Tx queue index as: int qid = ac + IWX_DQA_AUX_QUEUE + 1; with: #define IWX_DQA_AUX_QUEUE 1 In iwx_tx(), we use a different way of computing the Tx queue index, which is a leftover from iwm(4): ring = >txq[IWX_DQA_MIN_MGMT_QUEUE + ac]; with: #define IWX_DQA_MIN_MGMT_QUEUE 5 This kind of bug would usually have severe consequences. But in this case I got very lucky: 'ac' ranges from 0 to 3, and iwx_tx() always ends up picking Tx queue 5 from the range 5-8 because 'ac' is always passed in as 0 by the caller. iwx_enable_data_tx_queues() enables Tx queues 2-5 inclusive. So from the firmware's point of view, Tx queue 5 is enabled and is used to send frames, and it all just works in spite of the bug. With this fix, iwx_tx() uses Tx queue 2 and would work correctly for values of 'ac' other than 0, as intended. ok? diff 6fed087d6e00a5c30641d2658d09ced7cb1e3090 1e58df09264b5ea64bbe8f02e26da8b4e738c867 blob - 2f41c5a7c8a943e38bbd69a81d46cf432d329607 blob + 831f4d766af8fbf4e00f3fc0ad1624838869d92e --- sys/dev/pci/if_iwx.c +++ sys/dev/pci/if_iwx.c @@ -1674,13 +1674,13 @@ iwx_alloc_tx_ring(struct iwx_softc *sc, struct iwx_tx_ * * In DQA mode we use 1 command queue + 4 DQA mgmt/data queues. * The command is queue 0 (sc->txq[0]), and 4 mgmt/data frame queues -* are sc->tqx[IWX_DQA_MIN_MGMT_QUEUE + ac], i.e. sc->txq[5:8], +* are sc->tqx[ac + IWX_DQA_AUX_QUEUE + 1], i.e. sc->txq[2:5], * in order to provide one queue per EDCA category. * * Tx aggregation will require additional queues (one queue per TID * for which aggregation is enabled) but we do not implement this yet. */ - if (qid > IWX_DQA_MAX_MGMT_QUEUE) + if (qid > IWX_DQA_MIN_MGMT_QUEUE) return 0; err = iwx_dma_contig_alloc(sc->sc_dmat, >bc_tbl, @@ -4246,7 +4246,7 @@ iwx_tx(struct iwx_softc *sc, struct mbuf *m, struct ie * Tx aggregation will require additional queues (one queue per TID * for which aggregation is enabled) but we do not implement this yet. */ - ring = >txq[IWX_DQA_MIN_MGMT_QUEUE + ac]; + ring = >txq[ac + IWX_DQA_AUX_QUEUE + 1]; desc = >desc[ring->cur]; memset(desc, 0, sizeof(*desc)); data = >data[ring->cur];
Re: WireGuard patchset for OpenBSD, rev. 2
On Tue, May 26, 2020 at 07:39:01PM +1000, Matt Dunwoodie wrote: > Hi tech, > > After some feedback and comments, we've addressed the concerns, and > fixed a few things from our side too. Overall the structure is familiar > with no major changes, so any prior readings mostly carry over. > > This is a quick summary of the changes. Detailed changes can be found > on the out-of-tree repo or we can elaborate if more detail is needed. > > * Allow non-root to read WireGuard network configuration of the > interface. > - This allows users to help debug issues without > escalating to root. > > * Removal of wgconf option from ifconfig. > - This option was not mature enough for inclusion into base. Upon > reflection, this behaviour probably needs further consideration > about how to integrate into ifconfig. > > * Fix ioctl interface. > - The linked list setup was fragile, complicated, and broken, so > we've gone with something much simpler, and user easy to program > for, using a boring ioctl pattern that's been tried successfully > elsewhere already. > > * Limit the padding of the packet to the MTU. > - Fits the protocol spec and prevents unneccessary fragmentation. > > * Align handshake/identity locks and timer semantics with spec and other > implementations. > - This avoids any subtle edge cases that may occur. > > Here are the changes that shouldn't need any explanation: > > * Updated documentation based on feedback from tech and others. > * Maintain order of peers when adding/deleting . > * Rename noise_alloc to noise_upcall. > * Remove wg_timers_fn indirection. > > Minor formatting changes may also be present. > > Once someone picks this up, we're more than happy to help integrate > this and maintain it in-tree. > > Signed > Matt Dunwoodie Hi Matt, just repeating what I commented yesterday for the new diff to make sure it isn't overlooked. > +int > +wg_ioctl_get(struct wg_softc *sc, struct wg_data_io *data) > +{ > + struct wg_interface_io *iface_p, iface_o; > + struct wg_peer_io *peer_p, peer_o; > + struct wg_aip_io*aip_p; > + > + struct wg_peer *peer; > + struct wg_aip *aip; > + > + size_t size, peer_count, aip_count; > + int ret = 0, is_suser = suser(curproc) == 0; > + > + size = sizeof(struct wg_interface_io); > + if (data->wgd_size < size && !is_suser) > + goto ret_size; > + > + iface_p = data->wgd_interface; > + bzero(_o, sizeof(iface_o)); > + > + rw_enter_read(>sc_lock); > + > + if (sc->sc_udp_port != 0) { > + iface_o.i_port = ntohs(sc->sc_udp_port); > + iface_o.i_flags |= WG_INTERFACE_HAS_PORT; > + } > + > + if (sc->sc_udp_rtable != 0) { > + iface_o.i_rtable = sc->sc_udp_rtable; > + iface_o.i_flags |= WG_INTERFACE_HAS_RTABLE; > + } > + > + if (!is_suser) > + goto out; > + > + if (noise_local_keys(>sc_local, iface_o.i_public, > + iface_o.i_private) == 0) { > + iface_o.i_flags |= WG_INTERFACE_HAS_PUBLIC; > + iface_o.i_flags |= WG_INTERFACE_HAS_PRIVATE; > + } > + > + if (((SIZE_MAX - size) / sizeof(struct wg_peer_io)) < sc->sc_peer_num) > + goto error; > + size += sizeof(struct wg_peer_io) * sc->sc_peer_num; > + if (((SIZE_MAX - size) / sizeof(struct wg_aip_io)) < sc->sc_aip_num) > + goto error; I still think those two should return an error. 'goto error' is misleading as it doesn't actually set ret != 0. 'error' should probably be renamed to 'cleanup' to avoid confusion and ret should be set to ERANGE . > + size += sizeof(struct wg_aip_io) * sc->sc_aip_num; > + if (data->wgd_size < size) > + goto error; > + > + peer_count = 0; > + peer_p = _p->i_peers[0]; > + WG_PEERS_FOREACH(peer, sc) { > + bzero(_o, sizeof(peer_o)); > + peer_o.p_flags = WG_PEER_HAS_PUBLIC; > + peer_o.p_protocol_version = 1; > + > + if (noise_remote_keys(>p_remote, peer_o.p_public, > + peer_o.p_psk) == 0) > + peer_o.p_flags |= WG_PEER_HAS_PSK; > + > + if (wg_timers_get_persistent_keepalive(>p_timers, > + _o.p_pka) == 0) > + peer_o.p_flags |= WG_PEER_HAS_PKA; > + > + if (wg_peer_get_sockaddr(peer, _o.p_sa) == 0) > + peer_o.p_flags |= WG_PEER_HAS_ENDPOINT; > + > + mtx_enter(>p_counters_mtx); > + peer_o.p_txbytes = peer->p_counters_tx; > + peer_o.p_rxbytes = peer->p_counters_rx; > + mtx_leave(>p_counters_mtx); > + > + wg_timers_get_last_handshake(>p_timers, > + _o.p_last_handshake); > + > + aip_count = 0; > + aip_p = _p->p_aips[0]; > + LIST_FOREACH(aip, >p_aip, a_entry) { > + if
Re: filesystem code integer and many inodes
On Tue, May 26, 2020 at 11:58:39AM +0200, Otto Moerbeek wrote: > Hi, > > In theory ffs code support a maximum of UINT_MAX inodes, but in > practice, due to integer overflows in the current code, the limit is > INT_MAX inodes. > > This fixes that, and allows me to create and use filesystems with more > than INT_MAX inodes. This is partly from FreeBSD code. > > Main change is in fs.h, modifying a few fields to unsigned, most > notably fs_ipg (inodes per cylinder group) and fs_ncg (number of > cylinder groups). In various places fs_ipg * fs_ncg is computed, so > both should be unsigned. I also made sure cg indexes are unsigned and > ino_t is used for inode numbers. > > Tested on a 30TB partition with various parameters (notably -f x -b y > and -i z combinations). > > Please test and/or review, Apart from the noting the strange Subject: I also like to mention one change in the way cylinder groups are scanned. The current code scans forward and backward, which causes an uneven distribution of full cgs (the upper end of the cgs will get full first). Fix that by always scanning forward, wrapping to cg 0 if needed. -Otto
filesystem code integer and many inodes
Hi, In theory ffs code support a maximum of UINT_MAX inodes, but in practice, due to integer overflows in the current code, the limit is INT_MAX inodes. This fixes that, and allows me to create and use filesystems with more than INT_MAX inodes. This is partly from FreeBSD code. Main change is in fs.h, modifying a few fields to unsigned, most notably fs_ipg (inodes per cylinder group) and fs_ncg (number of cylinder groups). In various places fs_ipg * fs_ncg is computed, so both should be unsigned. I also made sure cg indexes are unsigned and ino_t is used for inode numbers. Tested on a 30TB partition with various parameters (notably -f x -b y and -i z combinations). Please test and/or review, -Otto Index: sbin/clri/clri.c === RCS file: /cvs/src/sbin/clri/clri.c,v retrieving revision 1.20 diff -u -p -r1.20 clri.c --- sbin/clri/clri.c28 Jun 2019 13:32:43 - 1.20 +++ sbin/clri/clri.c26 May 2020 09:41:18 - @@ -68,7 +68,8 @@ main(int argc, char *argv[]) char *fs, sblock[SBLOCKSIZE]; size_t bsize; off_t offset; - int i, fd, imax, inonum; + int i, fd; + ino_t imax, inonum; if (argc < 3) usage(); Index: sbin/dumpfs/dumpfs.c === RCS file: /cvs/src/sbin/dumpfs/dumpfs.c,v retrieving revision 1.35 diff -u -p -r1.35 dumpfs.c --- sbin/dumpfs/dumpfs.c17 Feb 2020 16:11:25 - 1.35 +++ sbin/dumpfs/dumpfs.c26 May 2020 09:41:18 - @@ -69,7 +69,7 @@ union { #define acgcgun.cg intdumpfs(int, const char *); -intdumpcg(const char *, int, int); +intdumpcg(const char *, int, u_int); intmarshal(const char *); intopen_disk(const char *); void pbits(void *, int); @@ -163,6 +163,7 @@ dumpfs(int fd, const char *name) size_t size; off_t off; int i, j; + u_int cg; switch (afs.fs_magic) { case FS_UFS2_MAGIC: @@ -172,7 +173,7 @@ dumpfs(int fd, const char *name) afs.fs_magic, ctime()); printf("superblock location\t%jd\tid\t[ %x %x ]\n", (intmax_t)afs.fs_sblockloc, afs.fs_id[0], afs.fs_id[1]); - printf("ncg\t%d\tsize\t%jd\tblocks\t%jd\n", + printf("ncg\t%u\tsize\t%jd\tblocks\t%jd\n", afs.fs_ncg, (intmax_t)fssize, (intmax_t)afs.fs_dsize); break; case FS_UFS1_MAGIC: @@ -198,7 +199,7 @@ dumpfs(int fd, const char *name) printf("cylgrp\t%s\tinodes\t%s\tfslevel %d\n", i < 1 ? "static" : "dynamic", i < 2 ? "4.2/4.3BSD" : "4.4BSD", i); - printf("ncg\t%d\tncyl\t%d\tsize\t%d\tblocks\t%d\n", + printf("ncg\t%u\tncyl\t%d\tsize\t%d\tblocks\t%d\n", afs.fs_ncg, afs.fs_ncyl, afs.fs_ffs1_size, afs.fs_ffs1_dsize); break; default: @@ -223,9 +224,9 @@ dumpfs(int fd, const char *name) (intmax_t)afs.fs_cstotal.cs_ndir, (intmax_t)afs.fs_cstotal.cs_nifree, (intmax_t)afs.fs_cstotal.cs_nffree); - printf("bpg\t%d\tfpg\t%d\tipg\t%d\n", + printf("bpg\t%d\tfpg\t%d\tipg\t%u\n", afs.fs_fpg / afs.fs_frag, afs.fs_fpg, afs.fs_ipg); - printf("nindir\t%d\tinopb\t%d\tmaxfilesize\t%ju\n", + printf("nindir\t%d\tinopb\t%u\tmaxfilesize\t%ju\n", afs.fs_nindir, afs.fs_inopb, (uintmax_t)afs.fs_maxfilesize); printf("sbsize\t%d\tcgsize\t%d\tcsaddr\t%jd\tcssize\t%d\n", @@ -238,10 +239,10 @@ dumpfs(int fd, const char *name) printf("nbfree\t%d\tndir\t%d\tnifree\t%d\tnffree\t%d\n", afs.fs_ffs1_cstotal.cs_nbfree, afs.fs_ffs1_cstotal.cs_ndir, afs.fs_ffs1_cstotal.cs_nifree, afs.fs_ffs1_cstotal.cs_nffree); - printf("cpg\t%d\tbpg\t%d\tfpg\t%d\tipg\t%d\n", + printf("cpg\t%d\tbpg\t%d\tfpg\t%d\tipg\t%u\n", afs.fs_cpg, afs.fs_fpg / afs.fs_frag, afs.fs_fpg, afs.fs_ipg); - printf("nindir\t%d\tinopb\t%d\tnspf\t%d\tmaxfilesize\t%ju\n", + printf("nindir\t%d\tinopb\t%u\tnspf\t%d\tmaxfilesize\t%ju\n", afs.fs_nindir, afs.fs_inopb, afs.fs_nspf, (uintmax_t)afs.fs_maxfilesize); printf("sbsize\t%d\tcgsize\t%d\tcgoffset %d\tcgmask\t0x%08x\n", @@ -261,7 +262,7 @@ dumpfs(int fd, const char *name) afs.fs_sblkno, afs.fs_cblkno, afs.fs_iblkno, afs.fs_dblkno); printf("cgrotor\t%d\tfmod\t%d\tronly\t%d\tclean\t%d\n", afs.fs_cgrotor, afs.fs_fmod, afs.fs_ronly, afs.fs_clean); - printf("avgfpdir %d\tavgfilesize %d\n", + printf("avgfpdir %u\tavgfilesize %u\n", afs.fs_avgfpdir,
Re: WireGuard patchset for OpenBSD, rev. 2
Hey tech@, A few things I thought I should add to our v2 revision: First, the improvements we've made in the last few weeks have been pretty substantial, and we've now got a much more faithful protocol implementation. I've been running this on a few high traffic servers, and I'll probably move demo.wireguard.com over to it soonish. The ioctl stuff has also been cleaned up and finalized now. It's this last point that I'm quite happy about: the latest wireguard-tools package in ports now fully supports this patch. So that means after patching your kernel you can easily run: # pkg_add -Dsnap wireguard-tools And it'll interface with this implementation like usual, providing wg(8) and wg-quick(8). Go programmers will also be happy to learn that Matt Layher has added support for the ioctl interface to his wgctrl-go project. As you might have surmised, I'm very interested in getting expanded testing of this patch. We're still very early in the 6.8 cycle. I'm wondering if it would make sense to check this v2 revision into cvs _now_, which will then make it included in -current, so that folks can test and use this easily. Then, Matt and I can continue to develop and refine this in one of two ways: - Send (bi-?)monthly patches to tech@ with fixes we've been working on. - Enable both of our commit bits for that part of the tree, and we'll push patches directly. Either one works, or maybe you have a third idea for that. I'm fairly committed to working on this full time to get it perfect for 6.8. The other thing I've been exploring is re-licensing the wireguard-tools package as MIT or ISC in case we decide at some later point down the road (not now) that maybe wg(8) would fit well in the base system. Anyway, please go forth and test this! And thanks a lot for the feedback from our v1. Looking forward to the same on v2. Jason
Initialize v4l2_requestbuffers struct to avoid invalid mmap
video(1) supports reading frames from a webcam via mmap(). To inform the V4L2 device about the number of desired buffers containing the frames to be memory-mapped, a VIDIOC_REQBUFS ioctl call is used. At the moment the v4l2_requestbuffers struct used for the VIDIOC_REQBUFS ioctl is only partially initialized which leads to an invalid mmap for my webcam: video: mmap: Invalid argument With the following diff applied my camera works (only on EHCI ports with USB 3.0 disabled in BIOS but that is a different story). OK? Index: video.c === RCS file: /cvs/xenocara/app/video/video.c,v retrieving revision 1.29 diff -u -p -r1.29 video.c --- video.c 6 Nov 2019 05:46:51 - 1.29 +++ video.c 26 May 2020 09:14:39 - @@ -1354,6 +1354,8 @@ mmap_init(struct video *vid) /* request buffers */ rb.count = MMAP_NUM_BUFS; + rb.type = V4L2_BUF_TYPE_VIDEO_CAPTURE; + rb.memory = V4L2_MEMORY_MMAP; r = ioctl(vid->dev.fd, VIDIOC_REQBUFS, ); if (r == -1) { warn("ioctl VIDIOC_REQBUFS");
Re: sysupgrade change to allow installing from url
On Tue, May 26, 2020 at 12:03:50AM +0200, Sebastian Benoit wrote: > Solene Rapenne(sol...@perso.pw) on 2020.05.25 15:25:40 +0200: > > Hi, > > > > I don't know if this will be accepted but I propose to add a -u [url] > > parameter to use older snapshots from an archive server for example. > > > > I wanted to add an optional parameter to -s at first but in case of > > sysupgrade -s [installurl] the install url would have been mistaken by > > this. > > > > If you specify a -u url and installurl at the same time, -u url > > superseed installurl. > > > > This would allow easier bisecting using older snapshots. > > I have fixed the way the directories are laid out, so now you can do this > > # echo "http://ftp.hostserver.de/archive/2020-05-14-0105; > /etc/installurl > # sysupgrade -n > # strings /home/_sysupgrade/bsd | grep 6.7-current > @(#)OpenBSD 6.7-current (GENERIC) #181: Wed May 13 12:43:30 MDT 2020 > Just for the archives, one can provide the URL on the command line, which I think is more sensible for this particular usecase: $ doas sysupgrade -n http://ftp.hostserver.de/archive/2020-05-14-0105 [...] $ what /home/_sysupgrade/bsd /home/_sysupgrade/bsd: OpenBSD 6.7-current (GENERIC) #181: Wed May 13 12:43:30 MDT 2020 -- I'm not entirely sure you are real.
Re: [RFC] pppd: add pipex(4) L2TP control support
On Tue, May 26, 2020 at 09:22:28AM +0200, Martin Pieuchot wrote: > On 25/05/20(Mon) 21:42, Sergey Ryazanov wrote: > > Add dedicated option to activate kernel L2TP acceleration via > > the pipex(4). The options should be passed by a L2TP tunnel > > management daemon (e.g. xl2tpd). > > What is the difference between npppd(8) and pppd(8)? Aren't those two > redundant? Why did you choose to modify pppd(8) and not npppd(8)? npppd(8) is server only it can not establish a connection. pppd(8) on the other hand is more client side (but I think it can do both). I think the split in userland makes sense (using npppd as LNS/LAC and pppd to connect to a ISP via PPP (even though we also use sppp/pppoe(4) for that). Now for the kernel side it would be great if the interface used would be the same for all ppp users. In this regard this is a step in the right direction. > > This diff is complete, but kernel support in ppp(4) is not ready. > > --- > > usr.sbin/pppd/ipcp.c| 5 +++ > > usr.sbin/pppd/options.c | 82 + > > usr.sbin/pppd/pppd.8| 18 + > > usr.sbin/pppd/pppd.h| 7 > > usr.sbin/pppd/sys-bsd.c | 56 > > 5 files changed, 168 insertions(+) > > > > diff --git usr.sbin/pppd/ipcp.c usr.sbin/pppd/ipcp.c > > index 1296d897b14..e7a6dbd6ee7 100644 > > --- usr.sbin/pppd/ipcp.c > > +++ usr.sbin/pppd/ipcp.c > > @@ -1251,6 +1251,10 @@ ipcp_up(f) > > return; > > } > > > > + /* enable pipex(4), keep working if failed */ > > + if (pipex_conf.pr_protocol && !apipex(go->ouraddr, ho->hisaddr, mask)) > > + IPCPDEBUG((LOG_WARNING, "apipex failed")); > > + > > #if (defined(SVR4) && (defined(SNI) || defined(__USLC__))) > > if (!sifaddr(f->unit, go->ouraddr, ho->hisaddr, mask)) { > > IPCPDEBUG((LOG_WARNING, "sifaddr failed")); > > @@ -1304,6 +1308,7 @@ ipcp_down(f) > > if (demand) { > > sifnpmode(f->unit, PPP_IP, NPMODE_QUEUE); > > } else { > > + dpipex(); > > sifdown(f->unit); > > ipcp_clear_addrs(f->unit); > > } > > diff --git usr.sbin/pppd/options.c usr.sbin/pppd/options.c > > index 828f7cdce65..fe0e2e6b54b 100644 > > --- usr.sbin/pppd/options.c > > +++ usr.sbin/pppd/options.c > > @@ -144,6 +144,9 @@ struct bpf_program pass_filter;/* Filter program for > > packets to pass */ > > struct bpf_program active_filter; /* Filter program for link-active > > pkts */ > > pcap_t pc;/* Fake struct pcap so we can compile > > expr */ > > #endif > > +struct pipex_session_req pipex_conf = {/* pipex(4) session > > configuration */ > > + .pr_protocol = 0, > > +}; > > > > /* > > * Prototypes > > @@ -253,6 +256,8 @@ static int setactivefilter(char **); > > static int setmslanman(char **); > > #endif > > > > +static int setpipexl2tp(char **); > > + > > static int number_option(char *, u_int32_t *, int); > > static int int_option(char *, int *); > > static int readable(int fd); > > @@ -391,6 +396,8 @@ static struct cmd { > > {"ms-lanman", 0, setmslanman}, /* Use LanMan psswd when using MS-CHAP > > */ > > #endif > > > > +{"pipex-l2tp", 6, setpipexl2tp}, /* set pipex(4) L2TP parameters > > */ > > + > > {NULL, 0, NULL} > > }; > > > > @@ -2283,3 +2290,78 @@ setmslanman(argv) > > return (1); > > } > > #endif > > + > > +static int > > +inet_atoss(cp, ss) > > +char *cp; > > +struct sockaddr_storage *ss; > > +{ > > +struct sockaddr_in *sin = (struct sockaddr_in *)ss; > > +char *c, *p; > > +unsigned port; > > + > > +if ((c = strchr(cp, ':')) == NULL) > > + return 0; > > +*c = '\0'; > > +if (!inet_aton(cp, >sin_addr)) > > + return 0; > > +if (!(port = strtoul(c + 1, , 10)) || *p != '\0') > > + return 0; > > +sin->sin_port = htons(port); > > +sin->sin_family = AF_INET; > > +ss->ss_len = sizeof(*sin); > > + > > +return 1; > > +} > > + > > +static int > > +strtou16(str, valp) > > +char *str; > > +uint16_t *valp; > > +{ > > +char *ptr; > > + > > +*valp = strtoul(str, , 0); > > + > > +return *ptr == '\0' ? 1 : 0; > > +} > > + > > +static int > > +setpipexl2tp(argv) > > +char **argv; > > +{ > > +BZERO((char *)_conf, sizeof(pipex_conf)); > > + > > +if (!inet_atoss(argv[0], _conf.pr_local_address)) { > > + option_error("pipex-l2tp: invalid local address of tunnel: %s", > > argv[0]); > > + return 0; > > +} > > +if (!inet_atoss(argv[1], _conf.pr_peer_address)) { > > + option_error("pipex-l2tp: invalid peer address of tunnel: %s", argv[1]); > > + return 0; > > +} > > +if (!strtou16(argv[2], _conf.pr_proto.l2tp.tunnel_id)) { > > + option_error("pipex-l2tp: invalid local tunnel id: %s", argv[2]); > > + return 0; > > +} > > +if (!strtou16(argv[3], _conf.pr_proto.l2tp.peer_tunnel_id)) { > > + option_error("pipex-l2tp: invalid peer tunnel id: %s", argv[3]); > > + return 0; > > +} > > +if
Re: [RFC] pppd: add pipex(4) L2TP control support
On 25/05/20(Mon) 21:42, Sergey Ryazanov wrote: > Add dedicated option to activate kernel L2TP acceleration via > the pipex(4). The options should be passed by a L2TP tunnel > management daemon (e.g. xl2tpd). What is the difference between npppd(8) and pppd(8)? Aren't those two redundant? Why did you choose to modify pppd(8) and not npppd(8)? > This diff is complete, but kernel support in ppp(4) is not ready. > --- > usr.sbin/pppd/ipcp.c| 5 +++ > usr.sbin/pppd/options.c | 82 + > usr.sbin/pppd/pppd.8| 18 + > usr.sbin/pppd/pppd.h| 7 > usr.sbin/pppd/sys-bsd.c | 56 > 5 files changed, 168 insertions(+) > > diff --git usr.sbin/pppd/ipcp.c usr.sbin/pppd/ipcp.c > index 1296d897b14..e7a6dbd6ee7 100644 > --- usr.sbin/pppd/ipcp.c > +++ usr.sbin/pppd/ipcp.c > @@ -1251,6 +1251,10 @@ ipcp_up(f) > return; > } > > + /* enable pipex(4), keep working if failed */ > + if (pipex_conf.pr_protocol && !apipex(go->ouraddr, ho->hisaddr, mask)) > + IPCPDEBUG((LOG_WARNING, "apipex failed")); > + > #if (defined(SVR4) && (defined(SNI) || defined(__USLC__))) > if (!sifaddr(f->unit, go->ouraddr, ho->hisaddr, mask)) { > IPCPDEBUG((LOG_WARNING, "sifaddr failed")); > @@ -1304,6 +1308,7 @@ ipcp_down(f) > if (demand) { > sifnpmode(f->unit, PPP_IP, NPMODE_QUEUE); > } else { > + dpipex(); > sifdown(f->unit); > ipcp_clear_addrs(f->unit); > } > diff --git usr.sbin/pppd/options.c usr.sbin/pppd/options.c > index 828f7cdce65..fe0e2e6b54b 100644 > --- usr.sbin/pppd/options.c > +++ usr.sbin/pppd/options.c > @@ -144,6 +144,9 @@ structbpf_program pass_filter;/* Filter program for > packets to pass */ > struct bpf_program active_filter; /* Filter program for link-active > pkts */ > pcap_t pc; /* Fake struct pcap so we can compile expr */ > #endif > +struct pipex_session_req pipex_conf = { /* pipex(4) session > configuration */ > + .pr_protocol = 0, > +}; > > /* > * Prototypes > @@ -253,6 +256,8 @@ static int setactivefilter(char **); > static int setmslanman(char **); > #endif > > +static int setpipexl2tp(char **); > + > static int number_option(char *, u_int32_t *, int); > static int int_option(char *, int *); > static int readable(int fd); > @@ -391,6 +396,8 @@ static struct cmd { > {"ms-lanman", 0, setmslanman}, /* Use LanMan psswd when using MS-CHAP > */ > #endif > > +{"pipex-l2tp", 6, setpipexl2tp}, /* set pipex(4) L2TP parameters */ > + > {NULL, 0, NULL} > }; > > @@ -2283,3 +2290,78 @@ setmslanman(argv) > return (1); > } > #endif > + > +static int > +inet_atoss(cp, ss) > +char *cp; > +struct sockaddr_storage *ss; > +{ > +struct sockaddr_in *sin = (struct sockaddr_in *)ss; > +char *c, *p; > +unsigned port; > + > +if ((c = strchr(cp, ':')) == NULL) > + return 0; > +*c = '\0'; > +if (!inet_aton(cp, >sin_addr)) > + return 0; > +if (!(port = strtoul(c + 1, , 10)) || *p != '\0') > + return 0; > +sin->sin_port = htons(port); > +sin->sin_family = AF_INET; > +ss->ss_len = sizeof(*sin); > + > +return 1; > +} > + > +static int > +strtou16(str, valp) > +char *str; > +uint16_t *valp; > +{ > +char *ptr; > + > +*valp = strtoul(str, , 0); > + > +return *ptr == '\0' ? 1 : 0; > +} > + > +static int > +setpipexl2tp(argv) > +char **argv; > +{ > +BZERO((char *)_conf, sizeof(pipex_conf)); > + > +if (!inet_atoss(argv[0], _conf.pr_local_address)) { > + option_error("pipex-l2tp: invalid local address of tunnel: %s", > argv[0]); > + return 0; > +} > +if (!inet_atoss(argv[1], _conf.pr_peer_address)) { > + option_error("pipex-l2tp: invalid peer address of tunnel: %s", argv[1]); > + return 0; > +} > +if (!strtou16(argv[2], _conf.pr_proto.l2tp.tunnel_id)) { > + option_error("pipex-l2tp: invalid local tunnel id: %s", argv[2]); > + return 0; > +} > +if (!strtou16(argv[3], _conf.pr_proto.l2tp.peer_tunnel_id)) { > + option_error("pipex-l2tp: invalid peer tunnel id: %s", argv[3]); > + return 0; > +} > +if (!strtou16(argv[4], _conf.pr_session_id)) { > + option_error("pipex-l2tp: invalid local call id: %s", argv[4]); > + return 0; > +} > +if (!strtou16(argv[5], _conf.pr_peer_session_id)) { > + option_error("pipex-l2tp: invalid peer call id: %s", argv[5]); > + return 0; > +} > + > +/* Indicate address field presense */ > +pipex_conf.pr_ppp_flags = PIPEX_PPP_HAS_ACF; > + > +/* Finally set the protocol type to implicitly indicate config validity > */ > +pipex_conf.pr_protocol = PIPEX_PROTO_L2TP; > + > +return 1; > +} > diff --git usr.sbin/pppd/pppd.8 usr.sbin/pppd/pppd.8 > index 5fba6f1714d..6a7f6e01c09 100644 > --- usr.sbin/pppd/pppd.8 > +++ usr.sbin/pppd/pppd.8 > @@ -829,6 +829,24 @@ option is used. > .It
Re: TP-Link TL-WN822N-EU v5 USB ID
On Mon, May 25, 2020 at 11:27:27PM +0300, Tero Koskinen wrote: > Hi, > > I have TP-Link TL-WN822N-EU v5 USB adapter[1] with > USB VID/PID 2357:0108: > > $ usbdevs > Controller /dev/usb0: > addr 01: 1022: AMD, xHCI root hub > addr 02: 2357:0108 Realtek, 802.11n NIC > Controller /dev/usb1: > addr 01: 1022: AMD, EHCI root hub > addr 02: 0438:7900 Advanced Micro Devices, Hub > $ > > Patch at the end adds it to the usbdevs. I hope I got it right > as I added it between existing 0x0107 and 0x0109 IDs. > > Relevant dmesg bits with the patch: > urtwn0 at uhub0 port 3 configuration 1 interface 0 "Realtek 802.11n NIC" rev > 2.10/2.00 addr 2 > urtwn0: MAC/BB RTL8192EU, RF 6052 2T2R, address 50:3e:aa:24:32:9e > uhub2 at uhub1 port 1 configuration 1 interface 0 "Advanced Micro Devices > Hub" rev 2.00/0.18 addr 2 > > Full dmesg at https://tkoskine.me/dmesg.tplink-TL-WN822N.txt > > Yours, > Tero thanks, committed