From 4beacc4ca0b2dd166dd50b1b8614f87fe0b67516 Mon Sep 17 00:00:00 2001 From: Sylvain Rochet Date: Sun, 24 Jan 2016 20:07:54 +0100 Subject: [PATCH] PPP, VJ, fixed TCP retransmission We used to modify in place the packet payload during compression but TCP stack requires that we don't change the packet payload, therefore we now copy the whole packet before compression. Signed-off-by: Sylvain Rochet --- src/include/netif/ppp/vj.h | 2 +- src/netif/ppp/ppp.c | 10 ++++++++-- src/netif/ppp/vj.c | 32 ++++++++++++++++++++++++-------- 3 files changed, 33 insertions(+), 11 deletions(-) diff --git a/src/include/netif/ppp/vj.h b/src/include/netif/ppp/vj.h index a5e51bf3..f2e1f8b4 100644 --- a/src/include/netif/ppp/vj.h +++ b/src/include/netif/ppp/vj.h @@ -151,7 +151,7 @@ struct vjcompress { #define VJF_TOSS 1U /* tossing rcvd frames because of input err */ extern void vj_compress_init (struct vjcompress *comp); -extern u8_t vj_compress_tcp (struct vjcompress *comp, struct pbuf *pb); +extern u8_t vj_compress_tcp (struct vjcompress *comp, struct pbuf **pb); extern void vj_uncompress_err (struct vjcompress *comp); extern int vj_uncompress_uncomp(struct pbuf *nb, struct vjcompress *comp); extern int vj_uncompress_tcp (struct pbuf **nb, struct vjcompress *comp); diff --git a/src/netif/ppp/ppp.c b/src/netif/ppp/ppp.c index d0f4079c..135ffded 100644 --- a/src/netif/ppp/ppp.c +++ b/src/netif/ppp/ppp.c @@ -487,15 +487,21 @@ static err_t ppp_netif_output(struct netif *netif, struct pbuf *pb, u16_t protoc * this is an IP packet. */ if (protocol == PPP_IP && pcb->vj_enabled) { - switch (vj_compress_tcp(&pcb->vj_comp, pb)) { + switch (vj_compress_tcp(&pcb->vj_comp, &pb)) { case TYPE_IP: /* No change... - protocol = PPP_IP_PROTOCOL; */ + protocol = PPP_IP; */ break; case TYPE_COMPRESSED_TCP: + /* vj_compress_tcp() returns a new allocated pbuf, indicate we should free + * our duplicated pbuf later */ + fpb = pb; protocol = PPP_VJC_COMP; break; case TYPE_UNCOMPRESSED_TCP: + /* vj_compress_tcp() returns a new allocated pbuf, indicate we should free + * our duplicated pbuf later */ + fpb = pb; protocol = PPP_VJC_UNCOMP; break; default: diff --git a/src/netif/ppp/vj.c b/src/netif/ppp/vj.c index abe8aa6f..19d8a2f8 100644 --- a/src/netif/ppp/vj.c +++ b/src/netif/ppp/vj.c @@ -132,9 +132,10 @@ vj_compress_init(struct vjcompress *comp) * compressed. */ u8_t -vj_compress_tcp(struct vjcompress *comp, struct pbuf *pb) +vj_compress_tcp(struct vjcompress *comp, struct pbuf **pb) { - struct ip_hdr *ip = (struct ip_hdr *)pb->payload; + struct pbuf *np = *pb; + struct ip_hdr *ip = (struct ip_hdr *)np->payload; struct cstate *cs = comp->last_cs->cs_next; u16_t ilen = IPH_HL(ip); u16_t hlen; @@ -158,7 +159,7 @@ vj_compress_tcp(struct vjcompress *comp, struct pbuf *pb) * `compressible' (i.e., ACK isn't set or some other control bit is * set). */ - if ((IPH_OFFSET(ip) & PP_HTONS(0x3fff)) || pb->tot_len < 40) { + if ((IPH_OFFSET(ip) & PP_HTONS(0x3fff)) || np->tot_len < 40) { return (TYPE_IP); } th = (struct tcp_hdr *)&((u32_t*)ip)[ilen]; @@ -169,11 +170,26 @@ vj_compress_tcp(struct vjcompress *comp, struct pbuf *pb) /* Check that the TCP/IP headers are contained in the first buffer. */ hlen = ilen + TCPH_HDRLEN(th); hlen <<= 2; - if (pb->len < hlen) { + if (np->len < hlen) { PPPDEBUG(LOG_INFO, ("vj_compress_tcp: header len %d spans buffers\n", hlen)); return (TYPE_IP); } + /* TCP stack requires that we don't change the packet payload, therefore we copy + * the whole packet before compression. */ + np = pbuf_alloc(PBUF_RAW, np->tot_len, PBUF_POOL); + if (!np) { + return (TYPE_IP); + } + + if (pbuf_copy(np, *pb) != ERR_OK) { + pbuf_free(np); + return (TYPE_IP); + } + + *pb = np; + ip = (struct ip_hdr *)np->payload; + /* * Packet is compressible -- we're going to send either a * COMPRESSED_TCP or UNCOMPRESSED_TCP packet. Either way we need @@ -371,20 +387,20 @@ vj_compress_tcp(struct vjcompress *comp, struct pbuf *pb) if (!comp->compressSlot || comp->last_xmit != cs->cs_id) { comp->last_xmit = cs->cs_id; hlen -= deltaS + 4; - if (pbuf_header(pb, -(s16_t)hlen)){ + if (pbuf_header(np, -(s16_t)hlen)){ /* Can we cope with this failing? Just assert for now */ LWIP_ASSERT("pbuf_header failed\n", 0); } - cp = (u8_t*)pb->payload; + cp = (u8_t*)np->payload; *cp++ = (u8_t)(changes | NEW_C); *cp++ = cs->cs_id; } else { hlen -= deltaS + 3; - if (pbuf_header(pb, -(s16_t)hlen)) { + if (pbuf_header(np, -(s16_t)hlen)) { /* Can we cope with this failing? Just assert for now */ LWIP_ASSERT("pbuf_header failed\n", 0); } - cp = (u8_t*)pb->payload; + cp = (u8_t*)np->payload; *cp++ = (u8_t)changes; } *cp++ = (u8_t)(deltaA >> 8);