knote(9) and knote_locked(9)
Make knote(9) lock the knote list internally, and add knote_locked(9) for the typical situation where the list is already locked. Simplify the kqueue API a bit (and make room for the new function) by dropping the KNOTE(9) macro. Its value is dubious, not least because it is common to use proper non-inline functions even for very minor tasks in the kernel. Index: share/man/man9/knote.9 === RCS file: src/share/man/man9/knote.9,v retrieving revision 1.9 diff -u -p -r1.9 knote.9 --- share/man/man9/knote.9 21 Jan 2014 03:15:46 - 1.9 +++ share/man/man9/knote.9 2 Feb 2023 04:32:53 - @@ -33,18 +33,21 @@ .Os .Sh NAME .Nm knote , -.Nm KNOTE +.Nm knote_locked .Nd raise kernel event .Sh SYNOPSIS .In sys/param.h .In sys/event.h .Ft void .Fn knote "struct klist *list" "long hint" -.Fn KNOTE "struct klist *list" "long hint" +.Ft void +.Fn knote_locked "struct klist *list" "long hint" .Sh DESCRIPTION The .Fn knote -function provides a hook into the kqueue kernel event notification +and +.Fn knote_locked +functions provide a hook into the kqueue kernel event notification mechanism to allow sections of the kernel to raise a kernel event in the form of a .Sq knote , @@ -60,7 +63,7 @@ of knotes, along with a .Fa hint (which is passed to the appropriate filter routine). .Fn knote -then walks the +then locks and walks the .Fa list making calls to the filter routine for each knote. As each knote contains a reference to the data structure that it is @@ -80,17 +83,19 @@ If the knote is already on the active li call to the filter occurs in order to provide an opportunity for the filter to record the activity. .Pp +.Fn knote_locked +is like +.Fn knote +but assumes that the +.Fa list +is already locked. +.Pp .Fn knote +and +.Fn knote_locked must not be called from interrupt contexts running at an interrupt priority level higher than .Fn splsched . -.Pp -.Fn KNOTE -is a macro that calls -.Fn knote list hint -if -.Fa list -is not empty. .\" .Sh ERRORS .Sh SEE ALSO .Xr kqueue 2 @@ -98,8 +103,6 @@ is not empty. .Sh HISTORY The .Fn knote -and -.Fn KNOTE functions first appeared in .Fx 4.1 , and then in Index: sys/arch/arm64/dev/apm.c === RCS file: src/sys/arch/arm64/dev/apm.c,v retrieving revision 1.21 diff -u -p -r1.21 apm.c --- sys/arch/arm64/dev/apm.c22 Jan 2023 13:14:21 - 1.21 +++ sys/arch/arm64/dev/apm.c2 Feb 2023 04:32:53 - @@ -345,7 +345,7 @@ apm_record_event(u_int event) return 1; apm_evindex++; - KNOTE(>sc_note, APM_EVENT_COMPOSE(event, apm_evindex)); + knote_locked(>sc_note, APM_EVENT_COMPOSE(event, apm_evindex)); return 0; } Index: sys/arch/i386/i386/apm.c === RCS file: src/sys/arch/i386/i386/apm.c,v retrieving revision 1.129 diff -u -p -r1.129 apm.c --- sys/arch/i386/i386/apm.c30 Jan 2023 10:49:04 - 1.129 +++ sys/arch/i386/i386/apm.c2 Feb 2023 04:32:54 - @@ -311,7 +311,7 @@ apm_record_event(struct apm_softc *sc, u } apm_evindex++; - KNOTE(>sc_note, APM_EVENT_COMPOSE(type, apm_evindex)); + knote_locked(>sc_note, APM_EVENT_COMPOSE(type, apm_evindex)); return (0); } Index: sys/arch/loongson/dev/apm.c === RCS file: src/sys/arch/loongson/dev/apm.c,v retrieving revision 1.41 diff -u -p -r1.41 apm.c --- sys/arch/loongson/dev/apm.c 19 Nov 2022 16:23:48 - 1.41 +++ sys/arch/loongson/dev/apm.c 2 Feb 2023 04:32:54 - @@ -363,7 +363,7 @@ apm_record_event(u_int event, const char return (1); apm_evindex++; - KNOTE(>sc_note, APM_EVENT_COMPOSE(event, apm_evindex)); + knote_locked(>sc_note, APM_EVENT_COMPOSE(event, apm_evindex)); return (0); } Index: sys/dev/audio.c === RCS file: src/sys/dev/audio.c,v retrieving revision 1.205 diff -u -p -r1.205 audio.c --- sys/dev/audio.c 8 Nov 2022 17:53:01 - 1.205 +++ sys/dev/audio.c 2 Feb 2023 04:32:54 - @@ -285,7 +285,7 @@ audio_mixer_wakeup(struct audio_softc *s wakeup(>mix_blocking); sc->mix_blocking = 0; } - KNOTE(>mix_klist, 0); + knote_locked(>mix_klist, 0); } void @@ -297,7 +297,7 @@ audio_buf_wakeup(struct audio_buf *buf) wakeup(>blocking); buf->blocking = 0; } - KNOTE(>klist, 0); + knote_locked(>klist, 0); } int Index: sys/dev/acpi/acpi.c === RCS file: src/sys/dev/acpi/acpi.c,v retrieving revision 1.418 diff -u -p -r1.418 acpi.c --- sys/dev/acpi/acpi.c 13 Sep 2022 17:14:54 - 1.418 +++ sys/dev/acpi/acpi.c 2 Feb 2023 04:32:54 -
Re: Move duplicating initialization to soalloc()
On Tue, Jan 31, 2023 at 09:50:26PM +0300, Vitaliy Makkoveev wrote:
> On Tue, Jan 31, 2023 at 06:00:45PM +, Visa Hankala wrote:
> > On Tue, Jan 31, 2023 at 12:44:47PM +0300, Vitaliy Makkoveev wrote:
> > > Since we have soalloc() to do common socket initialization, move the
> > > rest within. I mostly need to do this because standalone socket's buffer
> > > locking require to introduce another klistops data for buffers and there
> > > is no reason to add more copypaste to sonewconn().
> > >
> > > Also this makes `socket_klistops' private to kern/uipc_socket.c
> > >
> > > @@ -226,9 +225,6 @@ sonewconn(struct socket *head, int conns
> > > so->so_rcv.sb_lowat = head->so_rcv.sb_lowat;
> > > so->so_rcv.sb_timeo_nsecs = head->so_rcv.sb_timeo_nsecs;
> > >
> > > - klist_init(>so_rcv.sb_klist, _klistops, so);
> > > - klist_init(>so_snd.sb_klist, _klistops, so);
> > > - sigio_init(>so_sigio);
> > > sigio_copy(>so_sigio, >so_sigio);
> >
> > With this change, something should call klist_free() and sigio_free()
> > for 'so' if soreserve() fails in sonewconn().
> >
>
> klist_init() and sigio_init() alloc nothing, but for consistency reason
> they shold.
>
> I like to do this in the combined error path for soneconn() and
> pru_attach().
>
> Index: sys/kern/uipc_socket.c
> ===
> RCS file: /cvs/src/sys/kern/uipc_socket.c,v
> retrieving revision 1.299
> diff -u -p -r1.299 uipc_socket.c
> --- sys/kern/uipc_socket.c27 Jan 2023 21:01:59 - 1.299
> +++ sys/kern/uipc_socket.c31 Jan 2023 18:44:57 -
> @@ -112,6 +112,16 @@ const struct filterops soexcept_filtops
> .f_process = filt_soprocess,
> };
>
> +void klist_soassertlk(void *);
> +int klist_solock(void *);
> +void klist_sounlock(void *, int);
> +
> +const struct klistops socket_klistops = {
> + .klo_assertlk = klist_soassertlk,
> + .klo_lock = klist_solock,
> + .klo_unlock = klist_sounlock,
> +};
> +
> #ifndef SOMINCONN
> #define SOMINCONN 80
> #endif /* SOMINCONN */
> @@ -148,6 +158,11 @@ soalloc(int wait)
> return (NULL);
> rw_init_flags(>so_lock, "solock", RWL_DUPOK);
> refcnt_init(>so_refcnt);
> + klist_init(>so_rcv.sb_klist, _klistops, so);
> + klist_init(>so_snd.sb_klist, _klistops, so);
> + sigio_init(>so_sigio);
> + TAILQ_INIT(>so_q0);
> + TAILQ_INIT(>so_q);
>
> return (so);
> }
> @@ -176,11 +191,6 @@ socreate(int dom, struct socket **aso, i
> if (prp->pr_type != type)
> return (EPROTOTYPE);
> so = soalloc(M_WAIT);
> - klist_init(>so_rcv.sb_klist, _klistops, so);
> - klist_init(>so_snd.sb_klist, _klistops, so);
> - sigio_init(>so_sigio);
> - TAILQ_INIT(>so_q0);
> - TAILQ_INIT(>so_q);
> so->so_type = type;
> if (suser(p) == 0)
> so->so_state = SS_PRIV;
> @@ -2333,12 +2343,6 @@ klist_sounlock(void *arg, int ls)
>
> sounlock(so);
> }
> -
> -const struct klistops socket_klistops = {
> - .klo_assertlk = klist_soassertlk,
> - .klo_lock = klist_solock,
> - .klo_unlock = klist_sounlock,
> -};
>
> #ifdef DDB
> void
> Index: sys/kern/uipc_socket2.c
> ===
> RCS file: /cvs/src/sys/kern/uipc_socket2.c,v
> retrieving revision 1.134
> diff -u -p -r1.134 uipc_socket2.c
> --- sys/kern/uipc_socket2.c 27 Jan 2023 18:46:34 - 1.134
> +++ sys/kern/uipc_socket2.c 31 Jan 2023 18:44:57 -
> @@ -41,7 +41,6 @@
> #include
> #include
> #include
> -#include
> #include
>
> /*
> @@ -213,12 +212,8 @@ sonewconn(struct socket *head, int conns
> /*
>* Inherit watermarks but those may get clamped in low mem situations.
>*/
> - if (soreserve(so, head->so_snd.sb_hiwat, head->so_rcv.sb_hiwat)) {
> - if (persocket)
> - sounlock(so);
> - pool_put(_pool, so);
> - return (NULL);
> - }
> + if (soreserve(so, head->so_snd.sb_hiwat, head->so_rcv.sb_hiwat))
> + goto error;
sonewconn() has a variable called 'error'. Maybe use some other label.
'fail' and 'bad' are quite frequent in the kernel.
OK visa@
> so->so_snd.sb_wat = head->so_snd.sb_wat;
> so->so_snd.sb_lowat = head->so_snd.sb_lowat;
> so->so_snd.sb_timeo_nsecs = head->so_snd.sb_timeo_nsecs;
> @@ -226,9 +221,6 @@ sonewconn(struct socket *head, int conns
> so->so_rcv.sb_lowat = head->so_rcv.sb_lowat;
> so->so_rcv.sb_timeo_nsecs = head->so_rcv.sb_timeo_nsecs;
>
> - klist_init(>so_rcv.sb_klist, _klistops, so);
> - klist_init(>so_snd.sb_klist, _klistops, so);
> - sigio_init(>so_sigio);
> sigio_copy(>so_sigio, >so_sigio);
>
> soqinsque(head, so, 0);
> @@ -259,13 +251,7 @@ sonewconn(struct socket *head, int conns
>
> if (error) {
> soqremque(so, 0);
> - if (persocket)
> -
Re: npppd(8): remove "pipex" option
Hi,
On Wed, 1 Feb 2023 21:32:29 +0300
Vitaliy Makkoveev wrote:
> On Wed, Feb 01, 2023 at 09:00:13PM +0900, YASUOKA Masahiko wrote:
>> ...
>>
>> But I think we should keep the part since it is needed when adding a
>> tunneling protocol which is not supported by pipex, or running npppd
>> on another OS.
>>
>> >> If having "pipex yes/no" configuration is misleading, we can improve
>> >> the man page or the configuration itself.
>> >
>> > pipex yes | no
>> > Specify whether npppd(8) uses pipex(4). The default is
>> > “yes”. The sysctl(8) variable net.pipex.enable should
>> > also be enabled to use pipex(4).
>> >
>> > There is no misleading. But with "pipex no" npppd(8) is usable with
>> > pppac(4), but with pppx(4) it is not. Also, I don't like that it
>> > successfully creates connection. Guess, it better to deny "pipex no"
>> > for pppx(4).
>>
>> I agree both.
>>
>
> So, deny "pipex no" for pppx(4) interfaces.
ok yasuoka
Thanks,
> Index: usr.sbin/npppd/npppd/npppd.conf.5
> ===
> RCS file: /cvs/src/usr.sbin/npppd/npppd/npppd.conf.5,v
> retrieving revision 1.30
> diff -u -p -r1.30 npppd.conf.5
> --- usr.sbin/npppd/npppd/npppd.conf.5 31 Mar 2022 17:27:30 - 1.30
> +++ usr.sbin/npppd/npppd/npppd.conf.5 1 Feb 2023 18:28:29 -
> @@ -362,6 +362,11 @@ variable
> .Va net.pipex.enable
> should also be enabled to use
> .Xr pipex 4 .
> +This value must be
> +.Dq yes
> +for
> +.Xr pppx 4
> +interfaces.
> .It Ic debug-dump-pktin Ar protocol ...
> If this option is specified,
> .Xr npppd 8
> Index: usr.sbin/npppd/npppd/parse.y
> ===
> RCS file: /cvs/src/usr.sbin/npppd/npppd/parse.y,v
> retrieving revision 1.25
> diff -u -p -r1.25 parse.y
> --- usr.sbin/npppd/npppd/parse.y 15 Oct 2021 15:01:28 - 1.25
> +++ usr.sbin/npppd/npppd/parse.y 1 Feb 2023 18:28:29 -
> @@ -924,6 +924,14 @@ bind : BIND TUNNEL FROM STRING AUTHENTI
> free($9);
> YYERROR;
> }
> + if (tunn->pipex == 0 && iface->is_pppx) {
> + yyerror("pipex should be enabled for"
> + " interface %s", $9);
> + free($4);
> + free($7);
> + free($9);
> + YYERROR;
> + }
> if ((n = malloc(sizeof(struct confbind))) == NULL) {
> yyerror("out of memory");
> free($4);
>
Re: npppd(8): remove "pipex" option
On Wed, Feb 01, 2023 at 09:00:13PM +0900, YASUOKA Masahiko wrote:
> Hi
>
> ...
>
> But I think we should keep the part since it is needed when adding a
> tunneling protocol which is not supported by pipex, or running npppd
> on another OS.
>
> >> If having "pipex yes/no" configuration is misleading, we can improve
> >> the man page or the configuration itself.
> >
> > pipex yes | no
> > Specify whether npppd(8) uses pipex(4). The default is
> > “yes”. The sysctl(8) variable net.pipex.enable should
> > also be enabled to use pipex(4).
> >
> > There is no misleading. But with "pipex no" npppd(8) is usable with
> > pppac(4), but with pppx(4) it is not. Also, I don't like that it
> > successfully creates connection. Guess, it better to deny "pipex no"
> > for pppx(4).
>
> I agree both.
>
So, deny "pipex no" for pppx(4) interfaces.
Index: usr.sbin/npppd/npppd/npppd.conf.5
===
RCS file: /cvs/src/usr.sbin/npppd/npppd/npppd.conf.5,v
retrieving revision 1.30
diff -u -p -r1.30 npppd.conf.5
--- usr.sbin/npppd/npppd/npppd.conf.5 31 Mar 2022 17:27:30 - 1.30
+++ usr.sbin/npppd/npppd/npppd.conf.5 1 Feb 2023 18:28:29 -
@@ -362,6 +362,11 @@ variable
.Va net.pipex.enable
should also be enabled to use
.Xr pipex 4 .
+This value must be
+.Dq yes
+for
+.Xr pppx 4
+interfaces.
.It Ic debug-dump-pktin Ar protocol ...
If this option is specified,
.Xr npppd 8
Index: usr.sbin/npppd/npppd/parse.y
===
RCS file: /cvs/src/usr.sbin/npppd/npppd/parse.y,v
retrieving revision 1.25
diff -u -p -r1.25 parse.y
--- usr.sbin/npppd/npppd/parse.y15 Oct 2021 15:01:28 - 1.25
+++ usr.sbin/npppd/npppd/parse.y1 Feb 2023 18:28:29 -
@@ -924,6 +924,14 @@ bind : BIND TUNNEL FROM STRING AUTHENTI
free($9);
YYERROR;
}
+ if (tunn->pipex == 0 && iface->is_pppx) {
+ yyerror("pipex should be enabled for"
+ " interface %s", $9);
+ free($4);
+ free($7);
+ free($9);
+ YYERROR;
+ }
if ((n = malloc(sizeof(struct confbind))) == NULL) {
yyerror("out of memory");
free($4);
Re: libcrypto for powerpc g5 xonly
> Date: Tue, 31 Jan 2023 20:19:19 -0500
> From: George Koehler
>
> OpenBSD/macppc can enforce xonly on the PowerPC G5. libcrypto linked
> with cc -Wl,--execute-only will SIGSEGV as the PowerPC asm of sha256
> tries to read a table from text. The fix is to move the table to
> rodata. To find the table, I would do
>
> bcl 20, 31, 1f
> 1:mflr%r7
> addis %r7, %r7, .Ltable-1b@ha
> addi%r7, %r7, .Ltable-1b@l
>
> This diff does so in perlasm syntax. The literal "@ha" and "@l" in
> this diff are for an ELF platform (like OpenBSD) and might break the
> build for AIX or Mac OS, but I suspect that nobody builds this asm
> for those platforms. (PowerPC Mac OS is long obsolete, ended at
> Mac OS X 10.5.8.) If someone wants to try the PowerPC asm on a
> not-ELF platform, please tell me.
>
> aes-ppc.pl would have the same problem, but we don't use aes-ppc.pl,
> so I provide no fix. ports/security/openssl/{1.0.2,1.1,3.0} has
> copies of aes-ppc.pl and sha512-ppc.pl with the same problem, but
> doesn't enable them on OpenBSD, so I don't plan to edit them.
>
> sha512-ppc.pl can emit code for sha256 or sha512, but we only use it
> for sha256. The code uses simple ops (add, subtract, bit logic,
> bit rotation), nothing more fancy. I don't know why it runs faster
> than the (not asm) sha256 in ports/security/openssl.
>
> ok for this diff in src/lib/libcrypto?
ok kettenis@
> Index: sha/asm/sha512-ppc.pl
> ===
> RCS file: /cvs/src/lib/libcrypto/sha/asm/sha512-ppc.pl,v
> retrieving revision 1.3
> diff -u -p -r1.3 sha512-ppc.pl
> --- sha/asm/sha512-ppc.pl 14 Nov 2015 14:53:13 - 1.3
> +++ sha/asm/sha512-ppc.pl 31 Jan 2023 22:03:47 -
> @@ -220,8 +220,11 @@ $func:
> $LD $G,`6*$SZ`($ctx)
> $LD $H,`7*$SZ`($ctx)
>
> - bl LPICmeup
> -LPICedup:
> + bcl 20,31,Lpc
> +Lpc:
> + mflr$Tbl
> + addis $Tbl,$Tbl,Ltable-Lpc\@ha
> + addi$Tbl,$Tbl,Ltable-Lpc\@l
> andi. r0,$inp,3
> bne Lunaligned
> Laligned:
> @@ -377,22 +380,8 @@ $code.=<<___;
> blr
> .long 0
> .byte 0,12,0x14,0,0,0,0,0
> -___
> -
> -# Ugly hack here, because PPC assembler syntax seem to vary too
> -# much from platforms to platform...
> -$code.=<<___;
> -.align 6
> -LPICmeup:
> - mflrr0
> - bcl 20,31,\$+4
> - mflr$Tbl; vv "distance" between . and 1st data entry
> - addi$Tbl,$Tbl,`64-8`
> - mtlrr0
> - blr
> - .long 0
> - .byte 0,12,0x14,0,0,0,0,0
> - .space `64-9*4`
> + .rodata
> +Ltable:
> ___
> $code.=<<___ if ($SZ==8);
> .long 0x428a2f98,0xd728ae22,0x71374491,0x23ef65cd
>
>
Re: npppd(8): remove "pipex" option
I'm sorry for sending 2 mails. First one was a draft. Please read second one.
Re: npppd(8): remove "pipex" option
Hi On Tue, 31 Jan 2023 11:30:43 +0300 Vitaliy Makkoveev wrote: > On Tue, Jan 31, 2023 at 01:40:19PM +0900, YASUOKA Masahiko wrote: >> Hi, >> >> On Sun, 29 Jan 2023 14:35:05 +0300 >> Vitaliy Makkoveev wrote: >> > While switchind pppx(4) and pppac(4) from selwakeup() to KNOTE(9), I >> > found npppd(8) doesn't create pppx interface with "pipex no" in >> > npppd.conf, but successfully connects the client. So packets don't flow. >> > However, the pppac(4) has no this problem, because corresponding pppac >> > interface always created when npppd(8) opened device node. >> > >> > In fact, npppd(8) will not work with pppx(4) interfaces without pipex(4) >> > support. Otherwise npppd(8) should create pppx(4) sessions with not >> > pipex(4) specific PIPEXASESSION ioctl(2) command. >> > >> > I propose to remove "pipex" option from npppd(8). We already have >> > "net.pipex.enable" sysctl MIB to control pipex behaviour. In the case >> > then "net.pipex.enable" is set to 0, pipex(4) sessions will be always >> > created, but the traffic will go outside pipex(4) layer. >> > >> > The "ifdef USE_NPPPD_PIPEX" left as is. If we decide to remove them, I >> > will do this with the next diffs. >> >> Will the next diff remove the networking part (MPPE, IP) as well? >> >> > Please note, we never have complains about the problem described above, >> > so I doubt someone uses npppd(8) with "pipex no" in the npppd.conf(5). >> >> I don't know why you configured "pipex no", I suppose it was for >> debug. I also actually use "pipex no" when debug or development. > > I used this option to test my/visa@ diff which replaced selwakeup() by > KNOTE(9) and found that pppx(4) case is broken if this option is set to > "no". So you used "pipex no" for test. That is the purpose of "pipex yes/no". > Since we have the ability of enable/disable pipex(4) globally, I > propose to remove this option. No, they have different purposes. Without "pipex yes/no" option, we can't test the networking part (IP, MPPE) of npppd without pipex. > in fact, for the pppx(4) case npppd(8) is absolutely useless without > pipex(4) support, so I don't see any reasons to build it without > pipex(4). I don't propose to remove the whole "ifdef USE_NPPPD_PIPEX" > blocks, only preprocessor directives. Sorry, if my suggestion was not > clear. Note that the networking part (IP, MPPE) of npppd works with/without pipex. So just removing #ifdef lines, npppd still has "the networking part without pipex". As the result of removing "pipex yes/no" option, we can't test that part. If you are to remove the networking part (IP, MPPE) of npppd completely, removing "pipex yes/no" option makes sense since we don't need to test it anymore. But I think we should keep the part since it is needed when adding a tunneling protocol which is not supported by pipex, or running npppd on another OS. >> If having "pipex yes/no" configuration is misleading, we can improve >> the man page or the configuration itself. > > pipex yes | no > Specify whether npppd(8) uses pipex(4). The default is > “yes”. The sysctl(8) variable net.pipex.enable should > also be enabled to use pipex(4). > > There is no misleading. But with "pipex no" npppd(8) is usable with > pppac(4), but with pppx(4) it is not. Also, I don't like that it > successfully creates connection. Guess, it better to deny "pipex no" > for pppx(4). I agree both.
Re: npppd(8): remove "pipex" option
On Tue, 31 Jan 2023 11:30:43 +0300 Vitaliy Makkoveev wrote: > On Tue, Jan 31, 2023 at 01:40:19PM +0900, YASUOKA Masahiko wrote: >> Hi, >> >> On Sun, 29 Jan 2023 14:35:05 +0300 >> Vitaliy Makkoveev wrote: >> > While switchind pppx(4) and pppac(4) from selwakeup() to KNOTE(9), I >> > found npppd(8) doesn't create pppx interface with "pipex no" in >> > npppd.conf, but successfully connects the client. So packets don't flow. >> > However, the pppac(4) has no this problem, because corresponding pppac >> > interface always created when npppd(8) opened device node. >> > >> > In fact, npppd(8) will not work with pppx(4) interfaces without pipex(4) >> > support. Otherwise npppd(8) should create pppx(4) sessions with not >> > pipex(4) specific PIPEXASESSION ioctl(2) command. >> > >> > I propose to remove "pipex" option from npppd(8). We already have >> > "net.pipex.enable" sysctl MIB to control pipex behaviour. In the case >> > then "net.pipex.enable" is set to 0, pipex(4) sessions will be always >> > created, but the traffic will go outside pipex(4) layer. >> > >> > The "ifdef USE_NPPPD_PIPEX" left as is. If we decide to remove them, I >> > will do this with the next diffs. >> >> Will the next diff remove the networking part (MPPE, IP) as well? >> >> > Please note, we never have complains about the problem described above, >> > so I doubt someone uses npppd(8) with "pipex no" in the npppd.conf(5). >> >> I don't know why you configured "pipex no", I suppose it was for >> debug. I also actually use "pipex no" when debug or development. > > I used this option to test my/visa@ diff which replaced selwakeup() by > KNOTE(9) and found that pppx(4) case is broken if this option is set to > "no". So you used "pipex no" for test. That exactly is the purpose of "pipex yes/no". > Since we have the ability of enable/disable pipex(4) globally, I > propose to remove this option. No, they have different purposes. Without "pipex yes/no" option, we can't test the networking part (IP, MPPE) of npppd for without pipex. > in fact, for the pppx(4) case npppd(8) is absolutely useless without > pipex(4) support, so I don't see any reasons to build it without > pipex(4). I don't propose to remove the whole "ifdef USE_NPPPD_PIPEX" > blocks, only preprocessor directives. Sorry, if my suggestion was not > clear. Note that the networking part (IP, MPPE) of npppd works with/without pipex. So just removing #ifdef lines, npppd still has "the networking part without pipex". As the result of removing "pipex yes/no" option, we can't test that part. If you are to remove the networking part (IP, MPPE) of npppd completely, removing "pipex yes/no" option makes sense since we don't need to test it anymore. But I'd like to keep the networking part since it is needed when adding a tunneling protocol which is not supported by pipex, or running npppd on another OS. >> If having "pipex yes/no" configuration is misleading, we can improve >> the man page or the configuration itself. > > pipex yes | no > Specify whether npppd(8) uses pipex(4). The default is > “yes”. The sysctl(8) variable net.pipex.enable should > also be enabled to use pipex(4). > > There is no misleading. But with "pipex no" npppd(8) is usable with > pppac(4), but with pppx(4) it is not. Also, I don't like that it > successfully creates connection. Guess, it better to deny "pipex no" > for pppx(4). >
