Re: net80211: use BSS load information when choosing access point
Hello Stefan, sorry for the late reply, find my comments below. On Wed, Nov 3, 2021 at 4:47 PM Stefan Sperling wrote: > On Wed, Nov 03, 2021 at 04:03:08PM +0300, Sergey Ryazanov wrote: >> This topic was discussed a couple of weeks ago on the hostapd mailing >> list (http://lists.infradead.org/pipermail/hostap/2021-October/039961.html). >> And Jouni Malinen provide a few concerns about switching to the QBSS >> load IE usage. I would like to quote him: >> >>> I would also point out that at least the last time I did some testing >>> between vendor implementations, the reported values were completely >>> different between two APs on the same channel in more or less the same >>> location in the test setup.. In other words, I would not place much, if >>> any, trust in this value being something that could be compared between >>> two different APs. The only thing that seemed to be more or less >>> comparable was the values from the same AP device in a sense that the >>> channel load value increased when there was more traffic on the >>> channel.. >> >> So I do not think that the complete switching to the BSS load metric >> is a good idea. But utilizing the AP assistance in the BSS selection >> procedure sounds nice. > > My patch ignores RSSI on purpose in case BSS load is advertised precisely > because we need to figure out whether "channel load" is indicated reliably. > The standard doesn't really go into much detail about how to decide whether > a channel is "busy". It suggests that either virtual or physical carrier > sense could be used. I guess just letting any faint but valid wifi signal > block the channel would be a bad idea. So vendors may need to use some > threshold to decide whether a given signal is actually keeping the channel > busy in the vicinity of the AP. As with RSSI, measurements of such physical > quantities could differ between any two given APs and between moments in > time for a single AP. > > If channel load is not a reliable indicator I would hope vendors are at > least able to reliably count and report the number of associated stations? The number of associated stations is a comparable metric. But it provides no information about the channel utilization. Consider two APs: one with 10 associated but idle smartphones and one with two laptops that receive a 20 mbit/s video stream each. It is better to connect to the first AP with the idle channel, but the stations number metric directs us to quite loaded AP. -- Sergey
Re: net80211: use BSS load information when choosing access point
Hello Stefan, On Sun, Oct 31, 2021 at 9:25 PM Stefan Sperling wrote: > Some access points advertise BSS load information in beacons in > order to help clients make informed roaming decisions. > > BSS load information includes the number of associated stations, > the channel utilization (this takes other networks on the same > channel into account), and current admission capacity (interesting > for clients which use QoS, which we do not). > > We currently ignore BSS load information and only use RSSI to tell APs > apart. With this patch we store bss load information for APs during > scans if available, and take it into account when chosing an access point. > > Unfortunately, I do not have a suitable AP available to test with. > tcpdump can be used to check whether an access point supports this > feature. Run this command while associated to the access point: > tcpdump -n -i iwm0 -y IEEE802_11_RADIO -s 4096 -v > > If the AP reports BSS load information you should see something like > this among the information printed by tcpdump: > 64 stations, 100% utilization, admission capacity 0us/s > > It would help to get this patch tested in an environment where access > points advertise BSS load information, with a roaming test between > different access points. > I wrote down information about how roaming can be tested here: > https://marc.info/?l=openbsd-tech=163329420019842=2 This topic was discussed a couple of weeks ago on the hostapd mailing list (http://lists.infradead.org/pipermail/hostap/2021-October/039961.html). And Jouni Malinen provide a few concerns about switching to the QBSS load IE usage. I would like to quote him: > I would also point out that at least the last time I did some testing > between vendor implementations, the reported values were completely > different between two APs on the same channel in more or less the same > location in the test setup.. In other words, I would not place much, if > any, trust in this value being something that could be compared between > two different APs. The only thing that seemed to be more or less > comparable was the values from the same AP device in a sense that the > channel load value increased when there was more traffic on the > channel.. So I do not think that the complete switching to the BSS load metric is a good idea. But utilizing the AP assistance in the BSS selection procedure sounds nice. -- Sergey
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 sockad
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
Re: [PATCH] pipex(4): rework PPP input
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. Anyway if a session collision become a case, we could additionally demux ingress traffic by the destination UDP port. -- Sergey
[RFC] ppp: add pipex(4) support
Add minimal support for integration with pipex(4). Initialize interface
structure, pass IOCTLs to pipex(4) specific handler, etc.
Acceleration is working only in the DL path only at the moment.
Modification is not ready for applying. Work still in process.
---
sys/net/if_ppp.c| 16
sys/net/if_pppvar.h | 4
sys/net/ppp_tty.c | 17 +
3 files changed, 37 insertions(+)
diff --git sys/net/if_ppp.c sys/net/if_ppp.c
index 3f03d18b6ab..482eef37732 100644
--- sys/net/if_ppp.c
+++ sys/net/if_ppp.c
@@ -129,6 +129,10 @@
#include
#include
+#ifdef PIPEX
+#include
+#endif
+
#include "bpfilter.h"
#ifdef VJC
@@ -198,6 +202,9 @@ pppattach(void)
{
LIST_INIT(_softc_list);
if_clone_attach(_cloner);
+#ifdef PIPEX
+ pipex_init();
+#endif
}
int
@@ -228,6 +235,11 @@ ppp_clone_create(struct if_clone *ifc, int unit)
#if NBPFILTER > 0
bpfattach(>if_bpf, ifp, DLT_PPP, PPP_HDRLEN);
#endif
+
+#ifdef PIPEX
+ pipex_iface_init(>sc_pipex_iface, >sc_if);
+#endif
+
NET_LOCK();
LIST_INSERT_HEAD(_softc_list, sc, sc_list);
NET_UNLOCK();
@@ -247,6 +259,10 @@ ppp_clone_destroy(struct ifnet *ifp)
LIST_REMOVE(sc, sc_list);
NET_UNLOCK();
+#ifdef PIPEX
+ pipex_iface_fini(>sc_pipex_iface);
+#endif
+
if_detach(ifp);
free(sc, M_DEVBUF, 0);
diff --git sys/net/if_pppvar.h sys/net/if_pppvar.h
index 1c4dd9977b5..57f3b76c4f6 100644
--- sys/net/if_pppvar.h
+++ sys/net/if_pppvar.h
@@ -139,6 +139,10 @@ struct ppp_softc {
u_char sc_rawin[16]; /* chars as received */
int sc_rawin_count; /* # in sc_rawin */
LIST_ENTRY(ppp_softc) sc_list; /* all ppp interfaces */
+
+#ifdef PIPEX
+ struct pipex_iface_context sc_pipex_iface; /* pipex context */
+#endif
};
#ifdef _KERNEL
diff --git sys/net/ppp_tty.c sys/net/ppp_tty.c
index 124b9df59ac..93bcd62ae4b 100644
--- sys/net/ppp_tty.c
+++ sys/net/ppp_tty.c
@@ -122,6 +122,10 @@
#include
#endif
+#ifdef PIPEX
+#include
+#endif
+
#include
#include
#include
@@ -425,6 +429,19 @@ ppptioctl(struct tty *tp, u_long cmd, caddr_t data, int
flag, struct proc *p)
bcopy(sc->sc_asyncmap, data, sizeof(sc->sc_asyncmap));
break;
+#ifdef PIPEX
+case PIPEXSMODE:
+case PIPEXGMODE:
+case PIPEXASESSION:
+case PIPEXDSESSION:
+case PIPEXCSESSION:
+case PIPEXGSTAT:
+ if ((error = suser(p)) != 0)
+ break;
+ error = pipex_ioctl(>sc_pipex_iface, cmd, data);
+ break;
+#endif
+
default:
NET_LOCK();
error = pppioctl(sc, cmd, data, flag, p);
--
2.26.0
[RFC] pppd: add pipex(4) L2TP control support
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).
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 (!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 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
Re: [PATCH] pipex(4): rework PPP input
Hello, 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. > I don't see > packet capture problems on them. Can you catch and share how to > reproduce this problem with pppx(4) or pppac(4)? > > Also did you test your diff with pppx(4) and pppac(4)? I am entirely missed the fact that pppx(4) also have IFT_PPP type. Thank you for pointing me. I will recheck pppx(4) work and return as soon as I will have a better solution. -- Sergey
Re: [PATCH] pipex(4): rework PPP input
2 Hi Vitaliy,
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.
Then I started digging the code and came across the fact that pipex
has two packet capture points. One place is in pipex_ip_input(), where
pure IP packets (without a PPP header) are passed to bpf, and the
other is in pipex_ppp_input(), where only CCP frames are passed to bpf
and only if the associated network interface is of PPP type.
My first thought was to add another one hook to the pipex_ppp_input()
to intercept IP frames before the PPP header stripping. But then I
spotted that pipex_ppp_input() have a common pattern in the frames
processing, but it was poorly expressed since before the proposed
change the code is looks like this (in pseudo code):
if COMP
COMP sanity checks
ppp header stripping<- duplicate #1
call MPPE handler
exit
if CCP
CCP sanity checks
bpf call
ppp header stripping<- duplicate #2
call MPPE handler
exit
ppp header stripping<- duplicate #3
packet data alignment
if IP
IP sanity checks
call IP handler
exit
if IPv6
IPv6 sanity checks
call IPv6 handler
exit
if unknown PPP frame type
produce a big fat error message
As you can see for each frame type the code performs a specific sanity
check, strips the PPP header and then call a frame specific handler.
Also CCP frames on a ppp(4) interface are passed to bpf.
So I rearrange the code to make I a bit more clear:
* group sanity checks in the beginning of the function;
* place bpf call for ppp(4) interfaces after sanity checks, but
_before_ PPP header stripping;
* place the header stripping common for all types with optional data alignment;
* group frame specific handler calls in the end of the function;
* use switch/case to help a compiler and human readers :)
Now the function code is looks like this:
if COMP
COMP sanity checks
if CCP
CCP sanity checks
if IP
IP sanity checks and prepare for alignment
if IPv6
IPv6 sanity checks and prepare for alignment
if unknown PPP frame type
produce a big fat error message
call bpf for ppp(4) interfaces while frame still has the PPP header
strip the ppp header and perform optional alignment
if COMP
COMP sanity checks
if CCP
CCP sanity checks
if IP
IP sanity checks and prepare for alignment
if IPv6
IPv6 sanity checks and prepare for alignment
Diff is just unable to express such change in a human friendly way :(
It is better to look at the code before diff applying and after to see
the change.
The final hank just blocks stripped IP packets passing to bpf on
ppp(4) network interfaces to avoid duplicate (and mangled) packets in
a dump.
So, after the change we will get: function behaves as early, tcpdump
perfectly sniffs on a ppp interface with activated pipex, checks are
performed before expensive packet modifications, and function code is
readable and now it is clear what exactly function do for each type of
frames.
Oh, also pipex.c become shorter by the 13 lines.
> Also your diff does two different things: (1) split frames checks and
> input, and (2) modify frames passing to bpf(4). I guess you should split
> your diff by two different.
The bpf call modifications just prevent frame duplication in the dump.
So I think Its better to keep them here.
Buf if someone is against such wide modifications, then I am Ok to
send two patches: one for mangled packets dumping prevention and one
for PPP input rework.
> I did some inlined comments below.
>
> >
> > Also forbid IP/IPv6 frames (without PPP header) passing to BPF on
> > PPP interfaces to avoid mess.
> >
> > Initialy this change was made as a part of pipex(4) and ppp(4)
> > integration work. But, since this change make the core a bit more clear
> > I would like to publish it now.
> >
> > Ok?
> >
> > ---
> > sys/net/pipex.c | 95 -
> > 1 file changed, 54 insertions(+), 41 deletions(-)
> >
> > diff --git sys/net/pipex.c sys/net/pipex.c
> > index c433e4beaa6..e0066a61598 100644
> > --- sys/net/pipex.c
> > +++ sys/net/pipex.c
> > @@ -970,41 +970,68 @@ drop:
> > Static void
> > pipex_ppp_input(struct mbuf *m0, struct pipex_session *session, int
> > decrypted)
> > {
> &
[PATCH] pipex(4): rework PPP input
Split checks from frame accepting with header removing in the common
PPP input function. This should fix packet capture on a PPP interfaces.
Also forbid IP/IPv6 frames (without PPP header) passing to BPF on
PPP interfaces to avoid mess.
Initialy this change was made as a part of pipex(4) and ppp(4)
integration work. But, since this change make the core a bit more clear
I would like to publish it now.
Ok?
---
sys/net/pipex.c | 95 -
1 file changed, 54 insertions(+), 41 deletions(-)
diff --git sys/net/pipex.c sys/net/pipex.c
index c433e4beaa6..e0066a61598 100644
--- sys/net/pipex.c
+++ sys/net/pipex.c
@@ -970,41 +970,68 @@ drop:
Static void
pipex_ppp_input(struct mbuf *m0, struct pipex_session *session, int decrypted)
{
- int proto, hlen = 0;
+ int proto, hlen = 0, align = 0;
struct mbuf *n;
KASSERT(m0->m_pkthdr.len >= PIPEX_PPPMINLEN);
proto = pipex_ppp_proto(m0, session, 0, );
+ switch (proto) {
+ case PPP_IP:
+ if (session->ip_forward == 0)
+ goto drop;
+ if (!decrypted && pipex_session_is_mppe_required(session))
+ /*
+* if ip packet received when mppe
+* is required, discard it.
+*/
+ goto drop;
+ align = 1;
+ break;
+#ifdef INET6
+ case PPP_IPV6:
+ if (session->ip6_forward == 0)
+ goto drop;
+ if (!decrypted && pipex_session_is_mppe_required(session))
+ /*
+* if ip packet received when mppe
+* is required, discard it.
+*/
+ goto drop;
+ align = 1;
+ break;
+#endif
#ifdef PIPEX_MPPE
- if (proto == PPP_COMP) {
+ case PPP_COMP:
if (decrypted)
goto drop;
/* checked this on ppp_common_input() already. */
KASSERT(pipex_session_is_mppe_accepted(session));
-
- m_adj(m0, hlen);
- pipex_mppe_input(m0, session);
- return;
- }
- if (proto == PPP_CCP) {
+ break;
+ case PPP_CCP:
if (decrypted)
goto drop;
+ break;
+#endif
+ default:
+ if (decrypted)
+ goto drop;
+ /* protocol must be checked on pipex_common_input() already */
+ KASSERT(0);
+ goto drop;
+ }
#if NBPFILTER > 0
- {
+ {
struct ifnet *ifp = session->pipex_iface->ifnet_this;
+
if (ifp->if_bpf && ifp->if_type == IFT_PPP)
bpf_mtap(ifp->if_bpf, m0, BPF_DIRECTION_IN);
- }
-#endif
- m_adj(m0, hlen);
- pipex_ccp_input(m0, session);
- return;
}
#endif
+
m_adj(m0, hlen);
- if (!ALIGNED_POINTER(mtod(m0, caddr_t), uint32_t)) {
+ if (align && !ALIGNED_POINTER(mtod(m0, caddr_t), uint32_t)) {
n = m_dup_pkt(m0, 0, M_NOWAIT);
if (n == NULL)
goto drop;
@@ -1014,35 +1041,21 @@ pipex_ppp_input(struct mbuf *m0, struct pipex_session
*session, int decrypted)
switch (proto) {
case PPP_IP:
- if (session->ip_forward == 0)
- goto drop;
- if (!decrypted && pipex_session_is_mppe_required(session))
- /*
-* if ip packet received when mppe
-* is required, discard it.
-*/
- goto drop;
pipex_ip_input(m0, session);
- return;
+ break;
#ifdef INET6
case PPP_IPV6:
- if (session->ip6_forward == 0)
- goto drop;
- if (!decrypted && pipex_session_is_mppe_required(session))
- /*
-* if ip packet received when mppe
-* is required, discard it.
-*/
- goto drop;
pipex_ip6_input(m0, session);
- return;
+ break;
+#endif
+#ifdef PIPEX_MPPE
+ case PPP_COMP:
+ pipex_mppe_input(m0, session);
+ break;
+ case PPP_CCP:
+ pipex_ccp_input(m0, session);
+ break;
#endif
- default:
- if (decrypted)
- goto drop;
- /* protocol must be checked on pipex_common_input() already */
- KASSERT(0);
- goto drop;
}
return;
@@ -1105,7 +1118,7 @@ pipex_ip_input(struct mbuf *m0, struct pipex_session
*session)
len =
[PATCH] ppp(4): use common bpf filter hook
Use bpf filter hook from the common interface structure. This simplifies
the code by unifying it and prepare ppp(4) for pipex(4) support.
Ok?
---
sys/net/if_ppp.c| 16
sys/net/if_pppvar.h | 1 -
2 files changed, 8 insertions(+), 9 deletions(-)
diff --git sys/net/if_ppp.c sys/net/if_ppp.c
index 192ec7c91e0..4cba9a8778c 100644
--- sys/net/if_ppp.c
+++ sys/net/if_ppp.c
@@ -204,9 +204,11 @@ int
ppp_clone_create(struct if_clone *ifc, int unit)
{
struct ppp_softc *sc;
+ struct ifnet *ifp;
sc = malloc(sizeof(*sc), M_DEVBUF, M_WAITOK|M_ZERO);
sc->sc_unit = unit;
+ ifp = >sc_if;
snprintf(sc->sc_if.if_xname, sizeof sc->sc_if.if_xname, "%s%d",
ifc->ifc_name, unit);
sc->sc_if.if_softc = sc;
@@ -224,7 +226,7 @@ ppp_clone_create(struct if_clone *ifc, int unit)
if_attach(>sc_if);
if_alloc_sadl(>sc_if);
#if NBPFILTER > 0
- bpfattach(>sc_bpf, >sc_if, DLT_PPP, PPP_HDRLEN);
+ bpfattach(>if_bpf, ifp, DLT_PPP, PPP_HDRLEN);
#endif
NET_LOCK();
LIST_INSERT_HEAD(_softc_list, sc, sc_list);
@@ -754,11 +756,9 @@ pppoutput(struct ifnet *ifp, struct mbuf *m0, struct
sockaddr *dst,
}
#if NBPFILTER > 0
- /*
-* See if bpf wants to look at the packet.
-*/
- if (sc->sc_bpf)
- bpf_mtap(sc->sc_bpf, m0, BPF_DIRECTION_OUT);
+ /* See if bpf wants to look at the packet. */
+ if (ifp->if_bpf)
+ bpf_mtap(ifp->if_bpf, m0, BPF_DIRECTION_OUT);
#endif
/*
@@ -1369,8 +1369,8 @@ ppp_inproc(struct ppp_softc *sc, struct mbuf *m)
#if NBPFILTER > 0
/* See if bpf wants to look at the packet. */
- if (sc->sc_bpf)
- bpf_mtap(sc->sc_bpf, m, BPF_DIRECTION_IN);
+ if (ifp->if_bpf)
+ bpf_mtap(ifp->if_bpf, m, BPF_DIRECTION_IN);
#endif
rv = 0;
diff --git sys/net/if_pppvar.h sys/net/if_pppvar.h
index 87f7d1798bb..9dc774a0515 100644
--- sys/net/if_pppvar.h
+++ sys/net/if_pppvar.h
@@ -113,7 +113,6 @@ struct ppp_softc {
struct mbuf *sc_togo; /* output packet ready to go */
struct mbuf_list sc_npqueue; /* output packets not to be sent yet */
struct pppstat sc_stats; /* count of bytes/pkts sent/rcvd */
- caddr_t sc_bpf; /* hook for BPF */
enumNPmode sc_npmode[NUM_NP]; /* what to do with each NP */
struct compressor *sc_xcomp; /* transmit compressor */
void*sc_xc_state; /* transmit compressor state */
--
2.26.0
[PATCH] tcpdump: add ppp address/protocol compression support
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?
---
usr.sbin/tcpdump/print-ppp.c | 29 +
1 file changed, 17 insertions(+), 12 deletions(-)
diff --git usr.sbin/tcpdump/print-ppp.c usr.sbin/tcpdump/print-ppp.c
index 21f5d154847..60027178942 100644
--- usr.sbin/tcpdump/print-ppp.c
+++ usr.sbin/tcpdump/print-ppp.c
@@ -341,20 +341,26 @@ void
ppp_print(const u_char *p, u_int length)
{
uint16_t proto;
- int l;
+ int l, pfl;
l = snapend - p;
- if (l < sizeof(proto)) {
+ /* Check for compressed protocol field */
+ if (l >= 1 && (p[0] & 0x1) != 0)
+ pfl = sizeof(uint8_t);
+ else
+ pfl = sizeof(uint16_t);
+
+ if (l < pfl) {
printf("[|ppp]");
return;
}
- proto = EXTRACT_16BITS(p);
+ proto = pfl == sizeof(uint8_t) ? p[0] : EXTRACT_16BITS(p);
- p += sizeof(proto);
- l -= sizeof(proto);
- length -= sizeof(proto);
+ p += pfl;
+ l -= pfl;
+ length -= pfl;
if (eflag)
ppp_protoname(proto);
@@ -1385,12 +1391,11 @@ ppp_hdlc_print(const u_char *p, u_int length)
address = p[0];
control = p[1];
- p += sizeof(address) + sizeof(control);
- l -= sizeof(address) + sizeof(control);
- length -= sizeof(address) + sizeof(control);
-
switch (address) {
case 0xff: /* All-Stations */
+ p += sizeof(address) + sizeof(control);
+ l -= sizeof(address) + sizeof(control);
+ length -= sizeof(address) + sizeof(control);
if (eflag)
printf("%02x %02x %u ", address, control, length);
@@ -1402,8 +1407,8 @@ ppp_hdlc_print(const u_char *p, u_int length)
ppp_print(p, length);
break;
- default:
- printf("ppp address 0x%02x unknown", address);
+ default: /* Assume address compression */
+ ppp_print(p, length);
break;
}
return;
--
2.26.0
L2TP: rise download speed with ppp(4) + pipex(4)
Hello,
I would like to announce that I finally reached 80 mbps of download
via pure L2TP tunnel (without IPsec).
I digged through ppp(4) and pppd(8) code several times and I could not
find any bottlenecks. Looks like we need to change whole design of
ppp(4) to reach any usable download speed.
So I change direction and add pipex(4) support to ppp(4), only in Rx
path for now. And the same machine (Pentium D @3GHz) give me 80 mbps
of download rate instead of 20 mbps when run without pipex.
If somebody wants to reproduce the trick, you need a couple of patches
(one for ppp driver and second for xl2tpd) and utility to configure
pipex session (my version is here
https://github.com/rsa9000/pipexctl). Patches inlined below and
attached to mail due to gmail's rule to mangle whitespaces.
First of all you need to patch and rebuild the kernel. Then you need
to patch xl2tpd package and rebuild it too. Then activate pipex via
sysctl, configure and start xl2tpd as usual. Make sure that packets
are successfully traveling via established link.
Then the most trickly part: you need parse ouput of xl2tpd (to console
or syslog) and fetch Tunnel-Id & Session-Id values of running tunnel.
Then run (consult pipex man page for parameters description):
# pipexctl addsess l2tp
Command should looks like:
# pipexctl addsess l2tp AA BB A.B.C.D E.F.G.H 255.255.255.0
I.J.K.L:1701 M.N.O.P:1701 CC DD
Actually most parameters except your Session-Id could be zero.
My further plans includes following actions:
- add pipex support to Tx path of ppp(4)
- backport plugins support from upstream to pppd(8)
- make a plugin for pppd(8) to facilitate pipex usage (a-la pppol2tp for Linux)
- add pipex plugin configuration support to xl2tpd (both to our port
and to upstream)
I would like to ask, does someone have any objections against such changes?
I would like to complete this work until next release, but it all
depends on whether I have free time or not.
Kernel patch:
diff -ru sys.orig/net/if_ppp.c sys/net/if_ppp.c
--- sys.orig/net/if_ppp.c 2015-11-20 09:22:09.0 +0300
+++ sys/net/if_ppp.c 2016-06-21 02:41:59.0 +0300
@@ -129,6 +129,10 @@
#include
#include
+#ifdef PIPEX
+#include
+#endif
+
#include "bpfilter.h"
#ifdef VJC
@@ -199,6 +203,9 @@
{
LIST_INIT(_softc_list);
if_clone_attach(_cloner);
+#ifdef PIPEX
+ pipex_init();
+#endif
}
int
@@ -236,6 +243,10 @@
LIST_INSERT_HEAD(_softc_list, sc, sc_list);
splx(s);
+#ifdef PIPEX
+ pipex_iface_init(>pipex_iface, >sc_if);
+#endif
+
return (0);
}
@@ -248,6 +259,10 @@
if (sc->sc_devp != NULL)
return (EBUSY);
+#ifdef PIPEX
+ pipex_iface_fini(>pipex_iface);
+#endif
+
s = splnet();
LIST_REMOVE(sc, sc_list);
splx(s);
@@ -564,6 +579,19 @@
break;
#endif
+#ifdef PIPEX
+ case PIPEXSMODE:
+ case PIPEXGMODE:
+ case PIPEXASESSION:
+ case PIPEXDSESSION:
+ case PIPEXCSESSION:
+ case PIPEXGSTAT:
+ s = splnet();
+ error = pipex_ioctl(>pipex_iface, cmd, data);
+ splx(s);
+ return error;
+#endif
+
default:
return (-1);
}
diff -ru sys.orig/net/if_pppvar.h sys/net/if_pppvar.h
--- sys.orig/net/if_pppvar.h 2015-11-06 12:04:36.0 +0300
+++ sys/net/if_pppvar.h 2016-06-21 02:41:59.0 +0300
@@ -140,6 +140,10 @@
u_char sc_rawin[16]; /* chars as received */
int sc_rawin_count; /* # in sc_rawin */
LIST_ENTRY(ppp_softc) sc_list; /* all ppp interfaces */
+
+#ifdef PIPEX
+ struct pipex_iface_context pipex_iface; /* pipex context */
+#endif
};
#ifdef _KERNEL
diff -ru sys.orig/net/ppp_tty.c sys/net/ppp_tty.c
--- sys.orig/net/ppp_tty.c 2016-01-25 21:47:00.0 +0300
+++ sys/net/ppp_tty.c 2016-06-21 02:41:59.0 +0300
@@ -122,6 +122,10 @@
#include
#endif
+#ifdef PIPEX
+#include
+#endif
+
#include
#include
#include
x2tpd patch:
--- network.c.orig
+++ network.c
@@ -25,6 +25,10 @@
#ifndef LINUX
# include
#endif
+#ifdef OPENBSD
+#include
+#include
+#endif
#include "l2tp.h"
#include "ipsecmast.h"
#include "misc.h"/* for IPADDY macro */
@@ -83,6 +87,10 @@
#endif
+arg=1;
+if (setsockopt(server_socket, IPPROTO_IP, IP_PIPEX, ,
sizeof(arg)) != 0)
+l2tp_log(LOG_CRIT, "setsockopt pipex: %s\n", strerror(errno));
+
#ifdef USE_KERNEL
if (gconfig.forceuserspace)
{
--
Sergey
diff -ru sys.orig/net/if_ppp.c sys/net/if_ppp.c
--- sys.orig/net/if_ppp.c 2015-11-20 09:22:09.0 +0300
+++ sys/net/if_ppp.c 2016-06-21 02:41:59.0 +0300
@@ -129,6 +129,10 @@
#include
#include
+#ifdef PIPEX
+#include
+#endif
+
#include "bpfilter.h"
#ifdef VJC
@@ -199,6 +203,9 @@
{
LIST_INIT(_softc_list);
if_clone_attach(_cloner);
+#ifdef PIPEX
+ pipex_init();
+#endif
}
int
@@ -236,6 +243,10 @@
LIST_INSERT_HEAD(_softc_list, sc, sc_list);
splx(s);
+#ifdef PIPEX
+ pipex_iface_init(>pipex_iface, >sc_if);
+#endif
+
return (0);
}
@@ -248,6 +259,10 @@
if (sc->sc_devp != NULL)
return (EBUSY);
+#ifdef PIPEX
+ pipex_iface_fini(>pipex_iface);
+#endif
+
s = splnet();
Re: Pure L2TP client upload performance
2015-06-26 9:20 GMT+03:00 Claudio Jeker cje...@diehard.n-r-g.com:
On Fri, Jun 26, 2015 at 04:59:32AM +0300, Sergey Ryazanov wrote:
Hello,
during building l2tp tunnel with xl2tpd-1.3.1 I was faced with its too
low upload performance. When download, the speed is 20 mbit/s at
nearly 100% CPU utilization. CPU is Pentium D 930 3 GHz. When upload,
the speed is below 2 mbit/s at nearly zero CPU utilization.
First, I examined xl2tpd code and did not find any potential issues.
Then I compiled it with -pg option and do a quick test with help of
iperf(1): 4 TCP flows, direction is toward the L2TP server, 2 min
test. Then I run gprof and got pretty strange output:
[skip]
During upload tests, everything looks like if xl2tpd doesn't perform
any work and stucks somewhere in I/O operation.
May be there are some options, what could be tuned to speedup ppp(4)
I/O performance or did I missed something during my tests? I am in
doubts. Any clues?
Can you get a ktrace output to figure out what write is doing?
Could it be that it busy loops with EINTR or EAGAIN?
It sure smells like something is going on there.
I did make the trace, which shows that write(2) works good, there are no
errors:
# kdump -f ktrace.out-0-tx | grep 'RET write' | wc -l
23999
# kdump -f ktrace.out-0-tx | grep 'RET write.*errno' | wc -l
0
That was bad news. Let's talk about something good. I finally found a way
to speed up the upload. I got 91 mbit/s, as reported by speedtest.net,
over 100 mbit Ethernet link (at 100% CPU utilization, with the patched
non-SMP kernel).
Looks like the issue is caused by too small size of pty output buffer, and
too small watermarks, which control the pty buffer filling. When pty driver
requests the tty allocation, it passes 0 as baud rate. For any rates, which
is less or equal to 115200, tty driver allocates an output buffer of size
of 1024 byte. And most likely, hardcoded watermarks in the ppp discipline
code are selected according to this buffer size. May be these values were
reasonable for 56k modems, but not for 100 mbit uplink.
Patch for tests is inlined below. All numbers are arbitrary selected values.
I just took first reasonable values and got a positive result, without any
further experiments.
This patch is not suitable for merging, since it just quick and dirty fix.
To solve the issue in more generic way I see several approaches, each of
which has pros and cons:
(a) increase default value (as in this patch);
(b) provide some API (IOCTL) to control buffer size from pppd(8);
(c) make some hack that would reveal pty for high-speed links and increase
their buffer.
Any thoughts?
P.S. If I can get 91 mbit/s of upload rate, then why I get only 20 mbit/s
of download rate on the same machine?
Index: kern/tty_pty.c
===
RCS file: /cvs/src/sys/kern/tty_pty.c,v
retrieving revision 1.70
diff -u -p -r1.70 tty_pty.c
--- kern/tty_pty.c 10 Feb 2015 21:56:10 - 1.70
+++ kern/tty_pty.c 28 Jun 2015 14:18:16 -
@@ -58,6 +58,7 @@
#include sys/rwlock.h
#define BUFSIZ 100 /* Chunk size iomoved to/from user */
+#define PTY_DEF_BAUD 100
/*
* pts == /dev/tty[p-zP-T][0-9a-zA-Z]
@@ -192,7 +193,7 @@ check_pty(int minor)
if (!pt_softc[minor]) {
pti = malloc(sizeof(struct pt_softc), M_DEVBUF,
M_WAITOK|M_ZERO);
- pti-pt_tty = ttymalloc(0);
+ pti-pt_tty = ttymalloc(PTY_DEF_BAUD);
ptydevname(minor, pti);
pt_softc[minor] = pti;
}
@@ -235,7 +236,7 @@ ptsopen(dev_t dev, int flag, int devtype
pti = pt_softc[minor(dev)];
if (!pti-pt_tty) {
- tp = pti-pt_tty = ttymalloc(0);
+ tp = pti-pt_tty = ttymalloc(PTY_DEF_BAUD);
} else
tp = pti-pt_tty;
if ((tp-t_state TS_ISOPEN) == 0) {
@@ -413,7 +414,7 @@ ptcopen(dev_t dev, int flag, int devtype
pti = pt_softc[minor(dev)];
if (!pti-pt_tty) {
- tp = pti-pt_tty = ttymalloc(0);
+ tp = pti-pt_tty = ttymalloc(PTY_DEF_BAUD);
} else
tp = pti-pt_tty;
if (tp-t_oproc)
Index: net/ppp_tty.c
===
RCS file: /cvs/src/sys/net/ppp_tty.c,v
retrieving revision 1.33
diff -u -p -r1.33 ppp_tty.c
--- net/ppp_tty.c 3 Jun 2015 00:50:09 - 1.33
+++ net/ppp_tty.c 28 Jun 2015 14:18:16 -
@@ -163,8 +163,8 @@ struct pool ppp_pkts;
/* This is a NetBSD-1.0 or later kernel. */
#define CCOUNT(q) ((q)-c_cc)
-#define PPP_LOWAT 100 /* Process more output when LOWAT on queue
*/
-#definePPP_HIWAT 400 /* Don't start a new packet if HIWAT
on queue */
+#define PPP_LOWAT 1024/* Process
Re: Pure L2TP client upload performance
2015-06-26 9:20 GMT+03:00 Claudio Jeker cje...@diehard.n-r-g.com:
On Fri, Jun 26, 2015 at 04:59:32AM +0300, Sergey Ryazanov wrote:
Hello,
during building l2tp tunnel with xl2tpd-1.3.1 I was faced with its too
low upload performance. When download, the speed is 20 mbit/s at
nearly 100% CPU utilization. CPU is Pentium D 930 3 GHz. When upload,
the speed is below 2 mbit/s at nearly zero CPU utilization.
First, I examined xl2tpd code and did not find any potential issues.
Then I compiled it with -pg option and do a quick test with help of
iperf(1): 4 TCP flows, direction is toward the L2TP server, 2 min
test. Then I run gprof and got pretty strange output:
[skip]
During upload tests, everything looks like if xl2tpd doesn't perform
any work and stucks somewhere in I/O operation.
May be there are some options, what could be tuned to speedup ppp(4)
I/O performance or did I missed something during my tests? I am in
doubts. Any clues?
Can you get a ktrace output to figure out what write is doing?
Could it be that it busy loops with EINTR or EAGAIN?
It sure smells like something is going on there.
I did make the trace, which shows that write(2) works good, there are no
errors:
# kdump -f ktrace.out-0-tx | grep 'RET write' | wc -l
23999
# kdump -f ktrace.out-0-tx | grep 'RET write.*errno' | wc -l
0
That was bad news. Let's talk about something good. I finally found a way
to speed up the upload. I got 91 mbit/s, as reported by speedtest.net,
over 100 mbit Ethernet link (at 100% CPU utilization, with the patched
non-SMP kernel).
Looks like the issue is caused by too small size of pty output buffer, and
too small watermarks, which control the pty buffer filling. When pty driver
requests the tty allocation, it passes 0 as baud rate. For any rates, which
is less or equal to 115200, tty driver allocates an output buffer of size
of 1024 byte. And most likely, hardcoded watermarks in the ppp discipline
code are selected according to this buffer size. May be these values were
reasonable for 56k modems, but not for 100 mbit uplink.
Patch for tests is inlined below. All numbers are arbitrary selected values.
I just took first reasonable values and got a positive result, without any
further experiments.
This patch is not suitable for merging, since it just quick and dirty fix.
To solve the issue in more generic way I see several approaches, each of
which has pros and cons:
(a) increase default value (as in this patch);
(b) provide some API (IOCTL) to control buffer size from pppd(8);
(c) make some hack that would reveal pty for high-speed links and increase
their buffer.
Any thoughts?
P.S. If I can get 91 mbit/s of upload rate, then why I get only 20 mbit/s
of download rate on the same machine?
Index: kern/tty_pty.c
===
RCS file: /cvs/src/sys/kern/tty_pty.c,v
retrieving revision 1.70
diff -u -p -r1.70 tty_pty.c
--- kern/tty_pty.c 10 Feb 2015 21:56:10 - 1.70
+++ kern/tty_pty.c 28 Jun 2015 14:18:16 -
@@ -58,6 +58,7 @@
#include sys/rwlock.h
#define BUFSIZ 100 /* Chunk size iomoved to/from user */
+#define PTY_DEF_BAUD 100
/*
* pts == /dev/tty[p-zP-T][0-9a-zA-Z]
@@ -192,7 +193,7 @@ check_pty(int minor)
if (!pt_softc[minor]) {
pti = malloc(sizeof(struct pt_softc), M_DEVBUF,
M_WAITOK|M_ZERO);
- pti-pt_tty = ttymalloc(0);
+ pti-pt_tty = ttymalloc(PTY_DEF_BAUD);
ptydevname(minor, pti);
pt_softc[minor] = pti;
}
@@ -235,7 +236,7 @@ ptsopen(dev_t dev, int flag, int devtype
pti = pt_softc[minor(dev)];
if (!pti-pt_tty) {
- tp = pti-pt_tty = ttymalloc(0);
+ tp = pti-pt_tty = ttymalloc(PTY_DEF_BAUD);
} else
tp = pti-pt_tty;
if ((tp-t_state TS_ISOPEN) == 0) {
@@ -413,7 +414,7 @@ ptcopen(dev_t dev, int flag, int devtype
pti = pt_softc[minor(dev)];
if (!pti-pt_tty) {
- tp = pti-pt_tty = ttymalloc(0);
+ tp = pti-pt_tty = ttymalloc(PTY_DEF_BAUD);
} else
tp = pti-pt_tty;
if (tp-t_oproc)
Index: net/ppp_tty.c
===
RCS file: /cvs/src/sys/net/ppp_tty.c,v
retrieving revision 1.33
diff -u -p -r1.33 ppp_tty.c
--- net/ppp_tty.c 3 Jun 2015 00:50:09 - 1.33
+++ net/ppp_tty.c 28 Jun 2015 14:18:16 -
@@ -163,8 +163,8 @@ struct pool ppp_pkts;
/* This is a NetBSD-1.0 or later kernel. */
#define CCOUNT(q) ((q)-c_cc)
-#define PPP_LOWAT 100 /* Process more output when LOWAT on queue */
-#definePPP_HIWAT 400 /* Don't start a new packet if HIWAT on
queue */
+#define PPP_LOWAT 1024/* Process more output when LOWAT on queue */
+#define PPP_HIWAT 4096/* Don't start a new
Re: Pure L2TP client upload performance
2015-06-26 9:20 GMT+03:00 Claudio Jeker cje...@diehard.n-r-g.com: On Fri, Jun 26, 2015 at 04:59:32AM +0300, Sergey Ryazanov wrote: Hello, during building l2tp tunnel with xl2tpd-1.3.1 I was faced with its too low upload performance. When download, the speed is 20 mbit/s at nearly 100% CPU utilization. CPU is Pentium D 930 3 GHz. When upload, the speed is below 2 mbit/s at nearly zero CPU utilization. First, I examined xl2tpd code and did not find any potential issues. Then I compiled it with -pg option and do a quick test with help of iperf(1): 4 TCP flows, direction is toward the L2TP server, 2 min test. Then I run gprof and got pretty strange output: Full output: http://pastebin.com/dm4wz6VK granularity: each sample hit covers 4 byte(s) for 11.11% of 0.09 seconds % cumulative self self total time seconds secondscalls ms/call ms/call name 77.8 0.07 0.0724311 0.00 0.00 write [4] 22.2 0.09 0.0235897 0.00 0.00 _thread_sys_sendmsg [7] 0.0 0.09 0.00 109136 0.00 0.00 memset [46] 0.0 0.09 0.00 107997 0.00 0.00 ___errno [926] 0.0 0.09 0.0071740 0.00 0.00 read_packet [47] 0.0 0.09 0.0071726 0.00 0.00 _thread_sys_read [927] 0.0 0.09 0.0060154 0.00 0.00 _thread_sys_gettimeofday [928] 0.0 0.09 0.0060114 0.00 0.00 process_signal [32] 0.0 0.09 0.0060113 0.00 0.00 build_fdset [29] 0.0 0.09 0.0060113 0.00 0.00 process_schedule [26] 0.0 0.09 0.0060113 0.00 0.00 select [48] 0.0 0.09 0.0035897 0.00 0.00 udp_xmit [6] Another one run also gives something strange: Full output: http://pastebin.com/vXvHPuyj granularity: each sample hit covers 4 byte(s) for 14.29% of 0.07 seconds % cumulative self self total time seconds secondscalls ms/call ms/call name 71.4 0.05 0.0524348 0.00 0.00 write [4] 14.3 0.06 0.0160265 0.00 0.00 select [6] 14.3 0.07 0.01 10 1.00 1.00 calloc [7] 0.0 0.07 0.00 109324 0.00 0.00 memset [55] 0.0 0.07 0.00 108285 0.00 0.00 ___errno [925] 0.0 0.07 0.0071997 0.00 0.00 read_packet [56] 0.0 0.07 0.0071980 0.00 0.00 _thread_sys_read [926] 0.0 0.07 0.0060282 0.00 0.00 _thread_sys_gettimeofday [927] 0.0 0.07 0.0060266 0.00 0.00 process_signal [50] 0.0 0.07 0.0060265 0.00 0.00 build_fdset [49] 0.0 0.07 0.0060265 0.00 0.00 process_schedule [57] 0.0 0.07 0.0036020 0.00 0.00 _thread_sys_sendmsg [928] 0.0 0.07 0.0036020 0.00 0.00 udp_xmit [58] For comparison I changed direction and did a download test: Full output: http://pastebin.com/FFF6xGcy granularity: each sample hit covers 4 byte(s) for 0.01% of 114.30 seconds % cumulative self self total time seconds secondscalls ms/call ms/call name 93.7 107.06 107.06 370804 0.29 0.29 write [5] 1.8 109.08 2.02 370770 0.01 0.01 recvmsg [6] 1.4 110.67 1.59 370799 0.00 0.00 _thread_sys_gettimeofday [8] 1.1 111.90 1.23 370770 0.00 0.30 handle_packet [4] 1.0 113.01 1.11 370761 0.00 0.00 add_fcs [9] 0.7 113.81 0.81 370781 0.00 0.00 select [10] 0.2 114.03 0.211 210.00 114172.55 network_thread [3] 0.1 114.14 0.12 mcount [11] 0.0 114.19 0.05 370781 0.00 0.00 build_fdset [12] 0.0 114.23 0.04 370771 0.00 0.00 get_call [13] 0.0 114.27 0.03 1112701 0.00 0.00 memset [14] 0.0 114.28 0.02 370781 0.00 0.00 process_schedule [7] 0.0 114.30 0.01 29 0.34 0.34 _thread_sys_sendmsg [20] 0.0 114.30 0.018 0.62 0.74 inet_ntoa [24] 0.0 114.30 0.00 370785 0.00 0.00 swaps [110] 0.0 114.30 0.00 370782 0.00 0.00 process_signal [41] 0.0 114.30 0.00 370770 0.00 0.00 recycle_buf [111] During upload tests, everything looks like if xl2tpd doesn't perform any work and stucks somewhere in I/O operation. May be there are some options, what could be tuned to speedup ppp(4) I/O performance or did I missed something during my tests? I am in doubts. Any clues? Can you get a ktrace output to figure out what write is doing? Could it be that it busy loops with EINTR or EAGAIN? It sure smells like something is going
Pure L2TP client upload performance
Hello, during building l2tp tunnel with xl2tpd-1.3.1 I was faced with its too low upload performance. When download, the speed is 20 mbit/s at nearly 100% CPU utilization. CPU is Pentium D 930 3 GHz. When upload, the speed is below 2 mbit/s at nearly zero CPU utilization. First, I examined xl2tpd code and did not find any potential issues. Then I compiled it with -pg option and do a quick test with help of iperf(1): 4 TCP flows, direction is toward the L2TP server, 2 min test. Then I run gprof and got pretty strange output: Full output: http://pastebin.com/dm4wz6VK granularity: each sample hit covers 4 byte(s) for 11.11% of 0.09 seconds % cumulative self self total time seconds secondscalls ms/call ms/call name 77.8 0.07 0.0724311 0.00 0.00 write [4] 22.2 0.09 0.0235897 0.00 0.00 _thread_sys_sendmsg [7] 0.0 0.09 0.00 109136 0.00 0.00 memset [46] 0.0 0.09 0.00 107997 0.00 0.00 ___errno [926] 0.0 0.09 0.0071740 0.00 0.00 read_packet [47] 0.0 0.09 0.0071726 0.00 0.00 _thread_sys_read [927] 0.0 0.09 0.0060154 0.00 0.00 _thread_sys_gettimeofday [928] 0.0 0.09 0.0060114 0.00 0.00 process_signal [32] 0.0 0.09 0.0060113 0.00 0.00 build_fdset [29] 0.0 0.09 0.0060113 0.00 0.00 process_schedule [26] 0.0 0.09 0.0060113 0.00 0.00 select [48] 0.0 0.09 0.0035897 0.00 0.00 udp_xmit [6] Another one run also gives something strange: Full output: http://pastebin.com/vXvHPuyj granularity: each sample hit covers 4 byte(s) for 14.29% of 0.07 seconds % cumulative self self total time seconds secondscalls ms/call ms/call name 71.4 0.05 0.0524348 0.00 0.00 write [4] 14.3 0.06 0.0160265 0.00 0.00 select [6] 14.3 0.07 0.01 10 1.00 1.00 calloc [7] 0.0 0.07 0.00 109324 0.00 0.00 memset [55] 0.0 0.07 0.00 108285 0.00 0.00 ___errno [925] 0.0 0.07 0.0071997 0.00 0.00 read_packet [56] 0.0 0.07 0.0071980 0.00 0.00 _thread_sys_read [926] 0.0 0.07 0.0060282 0.00 0.00 _thread_sys_gettimeofday [927] 0.0 0.07 0.0060266 0.00 0.00 process_signal [50] 0.0 0.07 0.0060265 0.00 0.00 build_fdset [49] 0.0 0.07 0.0060265 0.00 0.00 process_schedule [57] 0.0 0.07 0.0036020 0.00 0.00 _thread_sys_sendmsg [928] 0.0 0.07 0.0036020 0.00 0.00 udp_xmit [58] For comparison I changed direction and did a download test: Full output: http://pastebin.com/FFF6xGcy granularity: each sample hit covers 4 byte(s) for 0.01% of 114.30 seconds % cumulative self self total time seconds secondscalls ms/call ms/call name 93.7 107.06 107.06 370804 0.29 0.29 write [5] 1.8 109.08 2.02 370770 0.01 0.01 recvmsg [6] 1.4 110.67 1.59 370799 0.00 0.00 _thread_sys_gettimeofday [8] 1.1 111.90 1.23 370770 0.00 0.30 handle_packet [4] 1.0 113.01 1.11 370761 0.00 0.00 add_fcs [9] 0.7 113.81 0.81 370781 0.00 0.00 select [10] 0.2 114.03 0.211 210.00 114172.55 network_thread [3] 0.1 114.14 0.12 mcount [11] 0.0 114.19 0.05 370781 0.00 0.00 build_fdset [12] 0.0 114.23 0.04 370771 0.00 0.00 get_call [13] 0.0 114.27 0.03 1112701 0.00 0.00 memset [14] 0.0 114.28 0.02 370781 0.00 0.00 process_schedule [7] 0.0 114.30 0.01 29 0.34 0.34 _thread_sys_sendmsg [20] 0.0 114.30 0.018 0.62 0.74 inet_ntoa [24] 0.0 114.30 0.00 370785 0.00 0.00 swaps [110] 0.0 114.30 0.00 370782 0.00 0.00 process_signal [41] 0.0 114.30 0.00 370770 0.00 0.00 recycle_buf [111] During upload tests, everything looks like if xl2tpd doesn't perform any work and stucks somewhere in I/O operation. May be there are some options, what could be tuned to speedup ppp(4) I/O performance or did I missed something during my tests? I am in doubts. Any clues? -- Sergey
patch: pppd: fix crash in LCP reject codepath
pppd(8) creates Reject message by moving with memcpy(3) rejected
option to left in buffer. If moving distance is less then option
length than it falls in coping between overlapping regions case. What
leads to crash.
The following patch fixes this issue by replacing memcpy(3) by
memmove(3).
Index: pppd.h
===
RCS file: /cvs/src/usr.sbin/pppd/pppd.h,v
retrieving revision 1.18
diff -u -p -r1.18 pppd.h
--- pppd.h 16 Jan 2015 06:40:19 - 1.18
+++ pppd.h 12 Jun 2015 12:11:14 -
@@ -402,6 +402,7 @@ extern struct option_info devnam_info;
#define UNTIMEOUT(r, f)untimeout((r), (f))
#define BCOPY(s, d, l) memcpy(d, s, l)
+#define BMOVE(s, d, l) memmove(d, s, l)
#define BZERO(s, n)memset(s, 0, n)
#define EXIT(u)quit()
Index: lcp.c
===
RCS file: /cvs/src/usr.sbin/pppd/lcp.c,v
retrieving revision 1.11
diff -u -p -r1.11 lcp.c
--- lcp.c 15 Jan 2015 23:19:48 - 1.11
+++ lcp.c 12 Jun 2015 12:11:14 -
@@ -1441,7 +1441,7 @@ endswitch:
if (orc == CONFREJ) { /* Reject this CI */
rc = CONFREJ;
if (cip != rejp)/* Need to move rejected CI? */
- BCOPY(cip, rejp, cilen); /* Move it */
+ BMOVE(cip, rejp, cilen); /* Move it (NB: overlapped regions) */
INCPTR(cilen, rejp);/* Update output pointer */
}
}
--
Sergey
Re: patch: pppd: fix crash in LCP reject codepath
Friday, June 12, 2015, 4:09:09 PM, you wrote: pppd(8) creates Reject message by moving with memcpy(3) rejected option to left in buffer. If moving distance is less then option length than it falls in coping between overlapping regions case. What leads to crash. The following patch fixes this issue by replacing memcpy(3) by memmove(3). Sorry, looks like my email client replace all tabs by whitespaces. Please find correct patch in the attachment. -- Sergey pppd-fix-crash-in-lcp-reject.patch Description: Binary data
