Date: Sun, 9 Dec 2012 03:10:01 GMT From: Brett Glass <freebsd-prs@brettglass.com> To: freebsd-ports-bugs@FreeBSD.org Subject: Re: ports/173533: mpd5 PPTP server race condition with some clients Message-ID: <201212090310.qB93A1se055744@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
The following reply was made to PR ports/173533; it has been noted by GNATS.
From: Brett Glass <freebsd-prs@brettglass.com>
To: bug-followup@FreeBSD.org
Cc:
Subject: Re: ports/173533: mpd5 PPTP server race condition with some
clients
Date: Sat, 08 Dec 2012 20:02:49 -0700
Additional tests have shown that a few routers actually do need a
delay longer than 50 ms to set up PPTP and PPPoE connections. Here
is a revised patch that changes the default delay to 100 ms. It's
still an eyeblink, and less than the latency of a slow Internet
connection. But it's just enough to let the routers configure
themselves properly. Please check this code to ensure that it does
not violate any implicit "contracts" of which I am unaware, and if
it does not, please patch in the FreeBSD port collection and on
SourceForge. Thank you!
*** link.h.orig Thu Dec 6 23:11:52 2012
--- link.h Sat Dec 8 01:14:03 2012
***************
*** 34,43 ****
--- 34,47 ----
/* Default latency and bandwidth */
#define LINK_DEFAULT_BANDWIDTH 64000 /* 64k */
#define LINK_DEFAULT_LATENCY 2000 /* 2ms */
+ /* Default delay before starting LCP configuration negotiation */
+ #define LINK_DEFAULT_LCP_DELAY 100 /* 100 ms */
+ #define LINK_MAX_LCP_DELAY 30000 /* 30 seconds */
+
enum {
LINK_ACTION_FORWARD,
LINK_ACTION_BUNDLE,
LINK_ACTION_DROP
};
***************
*** 78,87 ****
--- 82,92 ----
struct linkconf {
u_short mtu; /* Initial MTU value */
u_short mru; /* Initial MRU value */
uint16_t mrru; /* Initial MRRU value */
uint32_t accmap; /* Initial ACCMAP value */
+ uint16_t lcp_delay; /* Delay before initiating LCP */
short retry_timeout; /* FSM timeout for retries */
short max_redial; /* Max failed connect attempts */
short redial_delay; /* Failed connect retry time */
char *ident; /* LCP ident string */
struct optinfo options; /* Configured options */
***************
*** 148,157 ****
--- 153,163 ----
PhysType type; /* Device type descriptor */
void *info; /* Type specific info */
MsgHandler pmsgs; /* Message channel */
struct pppTimer openTimer; /* Open retry timer */
+ struct pppTimer lcpDelayTimer; /* LCP delay timer */
};
/*
* VARIABLES
***************
*** 197,202 ****
const char *arg, ...);
extern const char *LinkMatchAction(Link l, int stage, char *login);
#endif
-
--- 203,207 ----
*** link.c.orig Thu Dec 6 23:11:46 2012
--- link.c Sat Dec 8 01:17:38 2012
***************
*** 41,50 ****
--- 41,51 ----
SET_FSM_RETRY,
SET_MAX_RETRY,
SET_RETRY_DELAY,
SET_MAX_CHILDREN,
SET_KEEPALIVE,
+ SET_LCP_DELAY,
SET_IDENT,
SET_ACCEPT,
SET_DENY,
SET_ENABLE,
SET_DISABLE,
***************
*** 60,69 ****
--- 61,71 ----
static int LinkSetCommand(Context ctx, int ac, char *av[], void *arg);
static void LinkMsg(int type, void *cookie);
static void LinkNgDataEvent(int type, void *cookie);
static void LinkReopenTimeout(void *arg);
+ static void LinkLcpDelayTimeout(void *arg);
/*
* GLOBAL VARIABLES
*/
***************
*** 104,113 ****
--- 106,117 ----
LinkSetCommand, NULL, 2, (void *) SET_MAX_CHILDREN },
{ "keep-alive {secs} {max}", "LCP echo keep-alives",
LinkSetCommand, NULL, 2, (void *) SET_KEEPALIVE },
{ "ident {string}", "LCP ident string",
LinkSetCommand, NULL, 2, (void *) SET_IDENT },
+ { "lcp-delay {msec}", "Delay before LCP start",
+ LinkSetCommand, NULL, 2, (void *) SET_LCP_DELAY },
{ "accept {opt ...}", "Accept option",
LinkSetCommand, NULL, 2, (void *) SET_ACCEPT },
{ "deny {opt ...}", "Deny option",
LinkSetCommand, NULL, 2, (void *) SET_DENY },
{ "enable {opt ...}", "Enable option",
***************
*** 244,253 ****
--- 248,278 ----
Log(LG_LINK, ("[%s] Link: UP event", l->name));
l->originate = PhysGetOriginate(l);
Log(LG_PHYS2, ("[%s] Link: origination is %s",
l->name, LINK_ORIGINATION(l->originate)));
+
+ if (l->conf.lcp_delay == 0) {
+ LcpUp(l);
+ } else {
+ TimerStop(&l->lcpDelayTimer);
+ TimerInit(&l->lcpDelayTimer, "LcpOpen",
+ l->conf.lcp_delay, LinkLcpDelayTimeout, l);
+ TimerStart(&l->lcpDelayTimer);
+ Log(LG_LINK, ("[%s] Link: delaying %d ms before initiating LCP negotiation",
+ l->name, l->conf.lcp_delay));
+ }
+ }
+
+ static void
+ LinkLcpDelayTimeout(void *arg)
+ {
+ Link const l = (Link)arg;
+
+ if (gShutdownInProgress)
+ return;
+
LcpUp(l);
}
/*
* LinkDown()
***************
*** 427,436 ****
--- 452,462 ----
l->latency = LINK_DEFAULT_LATENCY;
l->upReason = NULL;
l->upReasonValid = 0;
l->downReason = NULL;
l->downReasonValid = 0;
+ l->conf.lcp_delay = LINK_DEFAULT_LCP_DELAY;
Disable(&l->conf.options, LINK_CONF_CHAPMD5);
Accept(&l->conf.options, LINK_CONF_CHAPMD5);
Disable(&l->conf.options, LINK_CONF_CHAPMSv1);
***************
*** 1260,1269 ****
--- 1286,1296 ----
if (l->lcp.fsm.conf.echo_int == 0)
Printf("disabled\r\n");
else
Printf("every %d secs, timeout %d\r\n",
l->lcp.fsm.conf.echo_int, l->lcp.fsm.conf.echo_max);
+ Printf("\tLCP delay : %d msec\r\n", l->conf.lcp_delay);
Printf("\tIdent string : \"%s\"\r\n", l->conf.ident ?
l->conf.ident : "");
if (l->tmpl)
Printf("\tMax children : %d\r\n", l->conf.max_children);
Printf("Link incoming actions:\r\n");
SLIST_FOREACH(a, &l->actions, next) {
***************
*** 1563,1572 ****
--- 1590,1611 ----
return(-1);
l->lcp.fsm.conf.echo_int = atoi(av[0]);
l->lcp.fsm.conf.echo_max = atoi(av[1]);
break;
+ case SET_LCP_DELAY:
+ if (ac != 1)
+ return(-1);
+ val = atoi(*av);
+ if (val < 0)
+ Error("lcp-delay cannot be negative");
+ else if (val > LINK_MAX_LCP_DELAY)
+ Error("max lcp-delay is %d", LINK_MAX_LCP_DELAY);
+ else
+ l->conf.lcp_delay = val;
+ break;
+
case SET_IDENT:
if (ac != 1)
return(-1);
if (l->conf.ident != NULL) {
Freee(l->conf.ident);
***************
*** 1616,1621 ****
assert(0);
}
return(0);
}
-
--- 1655,1659 ----
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201212090310.qB93A1se055744>