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->sin_addr))
> > + return 0;
> > + if (!(port = strtoul(c + 1, &p, 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, &ptr, 0);
> > +
> > + return *ptr == '\0' ? 1 : 0;
> > +}
> > +
> > +static int
> > +setpipexl2tp(argv)
> > + char **argv;
> > +{
> > + BZERO((char *)&pipex_conf, sizeof(pipex_conf));
> > +
> > + if (!inet_atoss(argv[0], &pipex_conf.pr_local_address)) {
> > + option_error("pipex-l2tp: invalid local address of tunnel: %s",
> > argv[0]);
> > + return 0;
> > + }
> > + if (!inet_atoss(argv[1], &pipex_conf.pr_peer_address)) {
> > + option_error("pipex-l2tp: invalid peer address of tunnel: %s", argv[1]);
> > + return 0;
> > + }
> > + if (!strtou16(argv[2], &pipex_conf.pr_proto.l2tp.tunnel_id)) {
> > + option_error("pipex-l2tp: invalid local tunnel id: %s", argv[2]);
> > + return 0;
> > + }
> > + if (!strtou16(argv[3], &pipex_conf.pr_proto.l2tp.peer_tunnel_id)) {
> > + option_error("pipex-l2tp: invalid peer tunnel id: %s", argv[3]);
> > + return 0;
> > + }
> > + if (!strtou16(argv[4], &pipex_conf.pr_session_id)) {
> > + option_error("pipex-l2tp: invalid local call id: %s", argv[4]);
> > + return 0;
> > + }
> > + if (!strtou16(argv[5], &pipex_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 Cm xonxoff
> > Use software flow control (i.e., XON/XOFF) to control the flow of data on
> > the serial port.
> > +.It Cm pipex-l2tp Ar ltunaddr ptunaddr ltunid ptunid lcallid pcallid
> > +OpenBSD specific. Activate and configure kernel L2TP acceleration. Set
> > +pipex(4) local tunnel address to
> > +.Ar ltunaddr ,
> > +peer tunnel address to
> > +.Ar ptunaddr ,
> > +local L2TP tunnel id to
> > +.Ar ltunid ,
> > +peer L2TP tunnel id to
> > +.Ar ptunid ,
> > +local L2TP call id (local session id in terms of pipex(4)) to
> > +.Ar lcallid
> > +and peer L2TP call id (peer session id in terms of pipex(4)) to
> > +.Ar pcallid .
> > +This option should not be specified in the options file. Instead it
> > +should be passed to
> > +.Nm pppd
> > +by a L2TP daemon, that is responsible for L2TP tunnel establishing.
> > .El
> > .Sh OPTIONS FILES
> > Options can be taken from files as well as the command line.
> > diff --git usr.sbin/pppd/pppd.h usr.sbin/pppd/pppd.h
> > index 9cd332939a3..27ddc45bd78 100644
> > --- usr.sbin/pppd/pppd.h
> > +++ usr.sbin/pppd/pppd.h
> > @@ -51,7 +51,10 @@
> >
> > #include <sys/types.h> /* for u_int32_t, if defined */
> > #include <sys/time.h> /* for struct timeval */
> > +#include <netinet/in.h>
> > #include <net/ppp_defs.h>
> > +#include <net/if.h>
> > +#include <net/pipex.h>
> > #include <stdio.h> /* for FILE */
> > #include <stdarg.h>
> >
> > @@ -128,6 +131,7 @@ extern int refuse_chap; /* Don't wanna auth.
> > ourselves with CHAP */
> > extern struct bpf_program pass_filter; /* Filter for pkts to pass */
> > extern struct bpf_program active_filter; /* Filter for link-active
> > pkts */
> > #endif
> > +extern struct pipex_session_req pipex_conf; /* pipex(4) session
> > configuration */
> >
> > #ifdef MSLANMAN
> > extern int ms_lanman; /* Nonzero if use LanMan password instead of NT
> > */
> > @@ -306,6 +310,9 @@ int cifproxyarp(int, u_int32_t);
> > u_int32_t GetMask(u_int32_t); /* Get appropriate netmask for address
> > */
> > int lock(char *); /* Create lock file for device */
> > void unlock(void); /* Delete previously-created lock file */
> > +int apipex(u_int32_t, u_int32_t, u_int32_t);
> > + /* Add pipex(4) to interface */
> > +void dpipex(void); /* Delete pipex(4) from interface */
> > int daemon(int, int); /* Detach us from terminal session */
> > void logwtmp(const char *, const char *, const char *);
> > /* Write entry to wtmp file */
> > diff --git usr.sbin/pppd/sys-bsd.c usr.sbin/pppd/sys-bsd.c
> > index e8deee6d2ff..085888cf06f 100644
> > --- usr.sbin/pppd/sys-bsd.c
> > +++ usr.sbin/pppd/sys-bsd.c
> > @@ -801,6 +801,15 @@ ppp_recv_config(unit, mru, asyncmap, pcomp, accomp)
> > syslog(LOG_ERR, "ioctl(PPPIOCSFLAGS): %m");
> > quit();
> > }
> > + /* Update pipex(4) config */
> > + if (pcomp)
> > + pipex_conf.pr_ppp_flags |= PIPEX_PPP_PFC_ACCEPTED;
> > + else
> > + pipex_conf.pr_ppp_flags &= ~PIPEX_PPP_PFC_ACCEPTED;
> > + if (accomp)
> > + pipex_conf.pr_ppp_flags |= PIPEX_PPP_ACFC_ACCEPTED;
> > + else
> > + pipex_conf.pr_ppp_flags &= ~PIPEX_PPP_ACFC_ACCEPTED;
> > }
> >
> > /*
> > @@ -1510,3 +1519,50 @@ unlock()
> > lock_file = NULL;
> > }
> > }
> > +
> > +/*
> > + * apipex - enable pipex(4) and add session
> > + */
> > +int
> > +apipex(o, h, m)
> > + u_int32_t o, h, m;
> > +{
> > + int mode = 1;
> > +
> > + if (ioctl(ttyfd, PIPEXSMODE, &mode) == -1) {
> > + syslog(LOG_WARNING, "Couldn't enable pipex: %m");
> > + return -1;
> > + }
> > +
> > + /* Complete session creation request */
> > + pipex_conf.pr_ip_srcaddr.s_addr = o;
> > + pipex_conf.pr_ip_address.s_addr = h;
> > + pipex_conf.pr_ip_netmask.s_addr = m;
> > +
> > + if (ioctl(ttyfd, PIPEXASESSION, &pipex_conf) == -1) {
> > + syslog(LOG_WARNING, "Couldn't add pipex session: %m");
> > + return -1;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +/*
> > + * dpipex - del session and disable pipex(4)
> > + */
> > +void
> > +dpipex()
> > +{
> > + struct pipex_session_close_req cr;
> > + int mode = 0;
> > +
> > + /* Copy required data from the session addition request */
> > + cr.psr_protocol = pipex_conf.pr_protocol;
> > + cr.psr_session_id = pipex_conf.pr_session_id;
> > +
> > + if (ioctl(ttyfd, PIPEXDSESSION, &cr) == -1)
> > + syslog(LOG_WARNING, "Couldn't del pipex session: %m");
> > +
> > + if (ioctl(ttyfd, PIPEXSMODE, &mode) == -1)
> > + syslog(LOG_WARNING, "Couldn't disable pipex: %m");
> > +}
> > --
> > 2.26.0
> >
Is pppd(8) still using K&R 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.
--
:wq Claudio
