rp-pppoe: allow MTU to be increased up to 1500
authorSimon Arlott <simon.arlott.org.uk>
Tue, 4 May 2010 19:06:06 +0000 (20:06 +0100)
committerPaul Mackerras <paulus@samba.org>
Tue, 24 Aug 2010 00:55:32 +0000 (10:55 +1000)
The ethernet data limit on rp-pppoe has been increased to 1508 to
allow an MTU of 1500 to be used. To prevent problems the interface
MTU is checked and used to lower the configured MTU/MRU.

If MIN(MTU/MRU) is > 1492, PPP-Max-Payload is added to PADI and
PADR. If PPP-Max-Payload is received in PADO or PADS, it will be
used to lower the configured MTU/MRU as required.

The MTU/MRU settings are stored and reloaded whenever a connection
is made, to allow for the peer or interface MTU/MRU to increase if
used with persist option.

Conforming to RFC4638, if no PPP-Max-Payload is received, the
negotiated MRU will be limited to 1492.

pppd/plugins/rp-pppoe/common.c
pppd/plugins/rp-pppoe/discovery.c
pppd/plugins/rp-pppoe/plugin.c
pppd/plugins/rp-pppoe/pppoe-discovery.c
pppd/plugins/rp-pppoe/pppoe.h

index a49efe7e43546525a6673380430b25da66e9e068..3b8e0140a37bf6e9e6919c310483ab34a4885bde 100644 (file)
@@ -58,7 +58,7 @@ parsePacket(PPPoEPacket *packet, ParseFunc *func, void *extra)
     }
 
     /* Do some sanity checks on packet */
-    if (len > ETH_DATA_LEN - 6) { /* 6-byte overhead for PPPoE header */
+    if (len > ETH_JUMBO_LEN - PPPOE_OVERHEAD) { /* 6-byte overhead for PPPoE header */
        error("Invalid PPPoE packet length (%u)", len);
        return -1;
     }
@@ -246,6 +246,9 @@ void pppoe_printpkt(PPPoEPacket *packet,
        case TAG_RELAY_SESSION_ID:
            printer(arg, "relay-session-id");
            break;
+       case TAG_PPP_MAX_PAYLOAD:
+           printer(arg, "PPP-max-payload");
+           break;
        case TAG_SERVICE_NAME_ERROR:
            printer(arg, "service-name-error");
            text = 1;
index 6e25ae7361e9167b7f1ab01f58b4ba8e6d1d584c..a856490f96fb7ba887ae2e8dda16a91eae358769 100644 (file)
@@ -14,6 +14,8 @@ static char const RCSID[] =
 #define _GNU_SOURCE 1
 #include "pppoe.h"
 #include "pppd/pppd.h"
+#include "pppd/fsm.h"
+#include "pppd/lcp.h"
 
 #include <string.h>
 #include <stdlib.h>
@@ -110,6 +112,7 @@ parsePADOTags(UINT16_t type, UINT16_t len, unsigned char *data,
 {
     struct PacketCriteria *pc = (struct PacketCriteria *) extra;
     PPPoEConnection *conn = pc->conn;
+    UINT16_t mru;
     int i;
 
     switch(type) {
@@ -140,6 +143,19 @@ parsePADOTags(UINT16_t type, UINT16_t len, unsigned char *data,
        conn->relayId.length = htons(len);
        memcpy(conn->relayId.payload, data, len);
        break;
+    case TAG_PPP_MAX_PAYLOAD:
+       if (len == sizeof(mru)) {
+           memcpy(&mru, data, sizeof(mru));
+           mru = ntohs(mru);
+           if (mru >= ETH_PPPOE_MTU) {
+               if (lcp_allowoptions[0].mru > mru)
+                   lcp_allowoptions[0].mru = mru;
+               if (lcp_wantoptions[0].mru > mru)
+                   lcp_wantoptions[0].mru = mru;
+               conn->seenMaxPayload = 1;
+           }
+       }
+       break;
     case TAG_SERVICE_NAME_ERROR:
        error("PADO: Service-Name-Error: %.*s", (int) len, data);
        conn->error = 1;
@@ -172,10 +188,24 @@ parsePADSTags(UINT16_t type, UINT16_t len, unsigned char *data,
              void *extra)
 {
     PPPoEConnection *conn = (PPPoEConnection *) extra;
+    UINT16_t mru;
     switch(type) {
     case TAG_SERVICE_NAME:
        dbglog("PADS: Service-Name: '%.*s'", (int) len, data);
        break;
+    case TAG_PPP_MAX_PAYLOAD:
+       if (len == sizeof(mru)) {
+           memcpy(&mru, data, sizeof(mru));
+           mru = ntohs(mru);
+           if (mru >= ETH_PPPOE_MTU) {
+               if (lcp_allowoptions[0].mru > mru)
+                   lcp_allowoptions[0].mru = mru;
+               if (lcp_wantoptions[0].mru > mru)
+                   lcp_wantoptions[0].mru = mru;
+               conn->seenMaxPayload = 1;
+           }
+       }
+       break;
     case TAG_SERVICE_NAME_ERROR:
        error("PADS: Service-Name-Error: %.*s", (int) len, data);
        conn->error = 1;
@@ -259,6 +289,19 @@ sendPADI(PPPoEConnection *conn)
        plen += sizeof(pid) + TAG_HDR_SIZE;
     }
 
+    /* Add our maximum MTU/MRU */
+    if (MIN(lcp_allowoptions[0].mru, lcp_wantoptions[0].mru) > ETH_PPPOE_MTU) {
+       PPPoETag maxPayload;
+       UINT16_t mru = htons(MIN(lcp_allowoptions[0].mru, lcp_wantoptions[0].mru));
+       maxPayload.type = htons(TAG_PPP_MAX_PAYLOAD);
+       maxPayload.length = htons(sizeof(mru));
+       memcpy(maxPayload.payload, &mru, sizeof(mru));
+       CHECK_ROOM(cursor, packet.payload, sizeof(mru) + TAG_HDR_SIZE);
+       memcpy(cursor, &maxPayload, sizeof(mru) + TAG_HDR_SIZE);
+       cursor += sizeof(mru) + TAG_HDR_SIZE;
+       plen += sizeof(mru) + TAG_HDR_SIZE;
+    }
+
     packet.length = htons(plen);
 
     sendPacket(conn, conn->discoverySocket, &packet, (int) (plen + HDR_SIZE));
@@ -289,6 +332,7 @@ waitForPADO(PPPoEConnection *conn, int timeout)
     pc.serviceNameOK = (conn->serviceName) ? 0 : 1;
     pc.seenACName    = 0;
     pc.seenServiceName = 0;
+    conn->seenMaxPayload = 0;
     conn->error = 0;
 
     do {
@@ -413,6 +457,19 @@ sendPADR(PPPoEConnection *conn)
        plen += sizeof(pid) + TAG_HDR_SIZE;
     }
 
+    /* Add our maximum MTU/MRU */
+    if (MIN(lcp_allowoptions[0].mru, lcp_wantoptions[0].mru) > ETH_PPPOE_MTU) {
+       PPPoETag maxPayload;
+       UINT16_t mru = htons(MIN(lcp_allowoptions[0].mru, lcp_wantoptions[0].mru));
+       maxPayload.type = htons(TAG_PPP_MAX_PAYLOAD);
+       maxPayload.length = htons(sizeof(mru));
+       memcpy(maxPayload.payload, &mru, sizeof(mru));
+       CHECK_ROOM(cursor, packet.payload, sizeof(mru) + TAG_HDR_SIZE);
+       memcpy(cursor, &maxPayload, sizeof(mru) + TAG_HDR_SIZE);
+       cursor += sizeof(mru) + TAG_HDR_SIZE;
+       plen += sizeof(mru) + TAG_HDR_SIZE;
+    }
+
     /* Copy cookie and relay-ID if needed */
     if (conn->cookie.type) {
        CHECK_ROOM(cursor, packet.payload,
@@ -566,6 +623,14 @@ discovery(PPPoEConnection *conn)
        timeout *= 2;
     } while (conn->discoveryState == STATE_SENT_PADR);
 
+    if (!conn->seenMaxPayload) {
+       /* RFC 4638: MUST limit MTU/MRU to 1492 */
+       if (lcp_allowoptions[0].mru > ETH_PPPOE_MTU)
+           lcp_allowoptions[0].mru = ETH_PPPOE_MTU;
+       if (lcp_wantoptions[0].mru > ETH_PPPOE_MTU)
+           lcp_wantoptions[0].mru = ETH_PPPOE_MTU;
+    }
+
     /* We're done. */
     conn->discoveryState = STATE_SESSION;
     return;
index 9bd66433f3fec933905f6d073814d0292d3936a1..97011aaab938cc5066281846113ebf1bac32921a 100644 (file)
@@ -130,6 +130,31 @@ static int
 PPPOEConnectDevice(void)
 {
     struct sockaddr_pppox sp;
+    struct ifreq ifr;
+    int s;
+
+    /* Restore configuration */
+    lcp_allowoptions[0].mru = conn->mtu;
+    lcp_wantoptions[0].mru = conn->mru;
+
+    /* Update maximum MRU */
+    s = socket(AF_INET, SOCK_DGRAM, 0);
+    if (s < 0) {
+       error("Can't get MTU for %s: %m", conn->ifName);
+       goto errout;
+    }
+    strncpy(ifr.ifr_name, conn->ifName, sizeof(ifr.ifr_name));
+    if (ioctl(s, SIOCGIFMTU, &ifr) < 0) {
+       error("Can't get MTU for %s: %m", conn->ifName);
+       close(s);
+       goto errout;
+    }
+    close(s);
+
+    if (lcp_allowoptions[0].mru > ifr.ifr_mtu - TOTAL_OVERHEAD)
+       lcp_allowoptions[0].mru = ifr.ifr_mtu - TOTAL_OVERHEAD;
+    if (lcp_wantoptions[0].mru > ifr.ifr_mtu - TOTAL_OVERHEAD)
+       lcp_wantoptions[0].mru = ifr.ifr_mtu - TOTAL_OVERHEAD;
 
     conn->acName = acName;
     conn->serviceName = pppd_pppoe_service;
@@ -163,6 +188,7 @@ PPPOEConnectDevice(void)
        error("Failed to create PPPoE socket: %m");
        goto errout;
     }
+
     sp.sa_family = AF_PPPOX;
     sp.sa_protocol = PX_PROTO_OE;
     sp.sa_addr.pppoe.sid = conn->session;
@@ -381,6 +407,10 @@ void pppoe_check_options(void)
     if (lcp_wantoptions[0].mru > MAX_PPPOE_MTU)
        lcp_wantoptions[0].mru = MAX_PPPOE_MTU;
 
+    /* Save configuration */
+    conn->mtu = lcp_allowoptions[0].mru;
+    conn->mru = lcp_wantoptions[0].mru;
+
     ccp_allowoptions[0].deflate = 0;
     ccp_wantoptions[0].deflate = 0;
 
index 318f8583f98b246d99482330e7e33e3429c3f416..3d3bf4eecc816c59aad3aee24b94944150593147 100644 (file)
@@ -277,7 +277,7 @@ parsePacket(PPPoEPacket *packet, ParseFunc *func, void *extra)
     }
 
     /* Do some sanity checks on packet */
-    if (len > ETH_DATA_LEN - 6) { /* 6-byte overhead for PPPoE header */
+    if (len > ETH_JUMBO_LEN - PPPOE_OVERHEAD) { /* 6-byte overhead for PPPoE header */
        fprintf(stderr, "Invalid PPPoE packet length (%u)\n", len);
        return -1;
     }
index 3dba439082d3d2a38011cff34b6961fd5290c867..9ab2eee3914c3718daa055375f32095beaa8a8f5 100644 (file)
@@ -129,6 +129,7 @@ extern UINT16_t Eth_PPPOE_Session;
 #define TAG_AC_COOKIE          0x0104
 #define TAG_VENDOR_SPECIFIC    0x0105
 #define TAG_RELAY_SESSION_ID   0x0110
+#define TAG_PPP_MAX_PAYLOAD    0x0120
 #define TAG_SERVICE_NAME_ERROR 0x0201
 #define TAG_AC_SYSTEM_ERROR    0x0202
 #define TAG_GENERIC_ERROR      0x0203
@@ -167,6 +168,13 @@ extern UINT16_t Eth_PPPOE_Session;
 #define IPV4ALEN     4
 #define SMALLBUF   256
 
+/* There are other fixed-size buffers preventing
+   this from being increased to 16110. The buffer
+   sizes would need to be properly de-coupled from
+   the default MRU. For now, getting up to 1500 is
+   enough. */
+#define ETH_JUMBO_LEN 1508
+
 /* A PPPoE Packet, including Ethernet headers */
 typedef struct PPPoEPacketStruct {
     struct ethhdr ethHdr;      /* Ethernet header */
@@ -174,7 +182,7 @@ typedef struct PPPoEPacketStruct {
     unsigned int code:8;       /* PPPoE code */
     unsigned int session:16;   /* PPPoE session */
     unsigned int length:16;    /* Payload length */
-    unsigned char payload[ETH_DATA_LEN]; /* A bit of room to spare */
+    unsigned char payload[ETH_JUMBO_LEN]; /* A bit of room to spare */
 } PPPoEPacket;
 
 #define PPPOE_VER(vt)          ((vt) >> 4)
@@ -184,15 +192,18 @@ typedef struct PPPoEPacketStruct {
 /* Header size of a PPPoE packet */
 #define PPPOE_OVERHEAD 6  /* type, code, session, length */
 #define HDR_SIZE (sizeof(struct ethhdr) + PPPOE_OVERHEAD)
-#define MAX_PPPOE_PAYLOAD (ETH_DATA_LEN - PPPOE_OVERHEAD)
-#define MAX_PPPOE_MTU (MAX_PPPOE_PAYLOAD - 2)
+#define MAX_PPPOE_PAYLOAD (ETH_JUMBO_LEN - PPPOE_OVERHEAD)
+#define PPP_OVERHEAD 2  /* protocol */
+#define MAX_PPPOE_MTU (MAX_PPPOE_PAYLOAD - PPP_OVERHEAD)
+#define TOTAL_OVERHEAD (PPPOE_OVERHEAD + PPP_OVERHEAD)
+#define ETH_PPPOE_MTU (ETH_DATA_LEN - TOTAL_OVERHEAD)
 
 /* PPPoE Tag */
 
 typedef struct PPPoETagStruct {
     unsigned int type:16;      /* tag type */
     unsigned int length:16;    /* Length of payload */
-    unsigned char payload[ETH_DATA_LEN]; /* A LOT of room to spare */
+    unsigned char payload[ETH_JUMBO_LEN]; /* A LOT of room to spare */
 } PPPoETag;
 /* Header size of a PPPoE tag */
 #define TAG_HDR_SIZE 4
@@ -233,6 +244,9 @@ typedef struct PPPoEConnectionStruct {
     int error;                 /* Error packet received */
     int debug;                 /* Set to log packets sent and received */
     int discoveryTimeout;       /* Timeout for discovery packets */
+    int seenMaxPayload;
+    int mtu;                   /* Stored MTU */
+    int mru;                   /* Stored MRU */
 } PPPoEConnection;
 
 /* Structure used to determine acceptable PADO or PADS packet */