* Matt Dainty <m...@bodgit-n-scarper.com> [2012-01-02 16:30:55]:
> * Stuart Henderson <s...@spacehopper.org> [2011-12-31 10:12:41]:
> >
> > If the remote side doesn't echo our tag back, we should restrict ourselves
> > to PPPOE_MAXMTU.
> >
> > Certainly until we are checking this I think we should avoid automatically
> > raising pppoe(4)'s MTU based on pppoedev's MTU and require it to be set
> > explicitly on the pppoe iface (as well as the parent iface).
>
> Attached is an updated patch for sys/net/if_pppoe.c that now tracks if
> the remote end echoes back the max payload tag that we will have sent.
>
> However I'm not sure how best to set pppoe(4)'s MTU back down to
> PPPOE_MAXMTU (given all the extra logic is keyed off this value). Can I
> just set sc->sc_sppp.pp_if.if_mtu directly or should I bounce it
> through pppoe_ioctl() with SIOCSIFMTU? I can then update the patch and
> replace the FIXME with whatever code is necessary.
So once I stopped pppoe(8) from blindly agreeing to any increased MTU I
requested, I've now tested the below patches using an em(4) with a 1508
MTU against a server that doesn't support the larger MTU, pppoe0 will
now be set back to 1492 and PPP/LCP negotiation continues with the
correct MRU option. My Huawei/BT modem still negotiates the increased
size and PPP/LCP negotiation continues with the larger 1500 MRU. All
verified with some tcpdump captures.
I noticed "our" if_spppsubr.c compared to NetBSD's (re)sets the MRU
variables in sppp_lcp_up() so I moved the changes from sppp_lcp_open()
to there to prevent duplication and also fixed a small amount of
whitespace formatting (s/^ \t/\t/).
Comments?
Matt
--- sys/net/if_pppoe.c.orig Tue Dec 13 21:52:04 2011
+++ sys/net/if_pppoe.c Thu Jan 5 15:03:03 2012
@@ -86,6 +86,7 @@
#define PPPOE_TAG_ACCOOKIE 0x0104 /* AC cookie */
#define PPPOE_TAG_VENDOR 0x0105 /* vendor specific */
#define PPPOE_TAG_RELAYSID 0x0110 /* relay session id */
+#define PPPOE_TAG_MAX_PAYLOAD 0x0120 /* RFC 4638 max payload
*/
#define PPPOE_TAG_SNAME_ERR 0x0201 /* service name error */
#define PPPOE_TAG_ACSYS_ERR 0x0202 /* AC system error */
#define PPPOE_TAG_GENERIC_ERR 0x0203 /* gerneric error */
@@ -398,6 +399,7 @@
size_t ac_cookie_len;
size_t relay_sid_len;
int noff, err, errortag;
+ u_int16_t *max_payload;
u_int16_t tag, len;
u_int16_t session, plen;
u_int8_t *ac_cookie;
@@ -424,6 +426,7 @@
ac_cookie_len = 0;
relay_sid = NULL;
relay_sid_len = 0;
+ max_payload = NULL;
#ifdef PPPOE_SERVER
hunique = NULL;
hunique_len = 0;
@@ -531,6 +534,18 @@
relay_sid_len = len;
}
break;
+ case PPPOE_TAG_MAX_PAYLOAD:
+ if (max_payload == NULL) {
+ n = m_pulldown(m, off, len,
+ &noff);
+ if (n == NULL || len != 2) {
+ err_msg = "TAG MAX_PAYLOAD ERROR";
+ m = NULL;
+ break;
+ }
+ max_payload = (u_int16_t *)(mtod(n, caddr_t) +
noff);
+ }
+ break;
case PPPOE_TAG_SNAME_ERR:
err_msg = "SERVICE NAME ERROR";
errortag = 1;
@@ -679,6 +694,13 @@
sc->sc_relay_sid_len = relay_sid_len;
memcpy(sc->sc_relay_sid, relay_sid, relay_sid_len);
}
+ if (sc->sc_sppp.pp_if.if_mtu > PPPOE_MAXMTU &&
+ (max_payload == NULL ||
+ ntohs(*max_payload) != sc->sc_sppp.pp_if.if_mtu)) {
+ printf("%s: No valid PPP-Max-Payload tag received in
PADO\n",
+ sc->sc_sppp.pp_if.if_xname);
+ sc->sc_sppp.pp_if.if_mtu = PPPOE_MAXMTU;
+ }
memcpy(&sc->sc_dest, eh->ether_shost, sizeof(sc->sc_dest));
sc->sc_padr_retried = 0;
@@ -911,7 +933,7 @@
return (ENXIO);
}
- if (sc->sc_sppp.pp_if.if_mtu >
+ if (sc->sc_sppp.pp_if.if_mtu !=
eth_if->if_mtu - PPPOE_OVERHEAD) {
sc->sc_sppp.pp_if.if_mtu = eth_if->if_mtu -
PPPOE_OVERHEAD;
@@ -1068,6 +1090,8 @@
l2 = strlen(sc->sc_concentrator_name);
len += 2 + 2 + l2;
}
+ if (sc->sc_sppp.pp_if.if_mtu > PPPOE_MAXMTU)
+ len += 2 + 2 + 2;
/* allocate a buffer */
m0 = pppoe_get_mbuf(len + PPPOE_HEADERLEN); /* header len + payload
len */
@@ -1094,9 +1118,15 @@
PPPOE_ADD_16(p, PPPOE_TAG_HUNIQUE);
PPPOE_ADD_16(p, sizeof(sc->sc_unique));
memcpy(p, &sc->sc_unique, sizeof(sc->sc_unique));
+ p += sizeof(sc->sc_unique);
+ if (sc->sc_sppp.pp_if.if_mtu > PPPOE_MAXMTU) {
+ PPPOE_ADD_16(p, PPPOE_TAG_MAX_PAYLOAD);
+ PPPOE_ADD_16(p, 2);
+ PPPOE_ADD_16(p, (u_int16_t)sc->sc_sppp.pp_if.if_mtu);
+ }
+
#ifdef PPPOE_DEBUG
- p += sizeof(sc->sc_unique);
if (p - mtod(m0, u_int8_t *) != len + PPPOE_HEADERLEN)
panic("pppoe_send_padi: garbled output len, should be %ld, is
%ld",
(long)(len + PPPOE_HEADERLEN), (long)(p - mtod(m0, u_int8_t
*)));
@@ -1298,6 +1328,8 @@
len += 2 + 2 + sc->sc_ac_cookie_len; /* AC cookie */
if (sc->sc_relay_sid_len > 0)
len += 2 + 2 + sc->sc_relay_sid_len; /* Relay SID */
+ if (sc->sc_sppp.pp_if.if_mtu > PPPOE_MAXMTU)
+ len += 2 + 2 + 2;
m0 = pppoe_get_mbuf(len + PPPOE_HEADERLEN);
if (m0 == NULL)
@@ -1329,9 +1361,15 @@
PPPOE_ADD_16(p, PPPOE_TAG_HUNIQUE);
PPPOE_ADD_16(p, sizeof(sc->sc_unique));
memcpy(p, &sc->sc_unique, sizeof(sc->sc_unique));
+ p += sizeof(sc->sc_unique);
+ if (sc->sc_sppp.pp_if.if_mtu > PPPOE_MAXMTU) {
+ PPPOE_ADD_16(p, PPPOE_TAG_MAX_PAYLOAD);
+ PPPOE_ADD_16(p, 2);
+ PPPOE_ADD_16(p, (u_int16_t)sc->sc_sppp.pp_if.if_mtu);
+ }
+
#ifdef PPPOE_DEBUG
- p += sizeof(sc->sc_unique);
if (p - mtod(m0, u_int8_t *) != len + PPPOE_HEADERLEN)
panic("pppoe_send_padr: garbled output len, should be %ld, is
%ld",
(long)(len + PPPOE_HEADERLEN), (long)(p - mtod(m0,
u_int8_t *)));
--- sys/net/if_spppsubr.c.orig Tue Dec 13 22:16:00 2011
+++ sys/net/if_spppsubr.c Mon Jan 9 23:42:04 2012
@@ -2059,11 +2059,16 @@
return;
}
- sp->pp_alivecnt = 0;
- sp->lcp.opts = (1 << LCP_OPT_MAGIC);
- sp->lcp.magic = 0;
- sp->lcp.protos = 0;
- sp->lcp.mru = sp->lcp.their_mru = sp->pp_if.if_mtu;
+ sp->pp_alivecnt = 0;
+ sp->lcp.opts = (1 << LCP_OPT_MAGIC);
+ sp->lcp.magic = 0;
+ sp->lcp.protos = 0;
+ if (sp->pp_if.if_mtu != PP_MTU) {
+ sp->lcp.mru = sp->pp_if.if_mtu;
+ sp->lcp.opts |= (1 << LCP_OPT_MRU);
+ } else
+ sp->lcp.mru = PP_MTU;
+ sp->lcp.their_mru = PP_MTU;
getmicrouptime(&tv);
sp->pp_last_receive = sp->pp_last_activity = tv.tv_sec;
