lwip pdu implementation: shift from contiguous to memp + pbuf

the old implementation stuffed the pdu struct in front of the
transport-level pbuf, and preserved it on the stack while the packages
were sent.

this had two advantages:

* saving ram
* preserving the other implementation's structure of pdu->hdr == pdu+1
  (ie. starting right after the pdu struct)

but two disadvantages:

* bad readability and pointer juggling in the send routine
* relying on the send function to block until the package is out (which
  works for simple implementations of lwip drivers but not dma etc)

if something really assumes the contiguous memory structure, it will
show (unfortunately by corrupting other pdu structs), and chances are
people use ->hdr anyway.
This commit is contained in:
chrysn committed 2015-01-16 12:03:25 +01:00
1 parent 00beb7f1f7
commit 6caf836023
5 files changed
+25 -41

No files matched your search

+5
View File
@@ -25,6 +25,10 @@
#define MEMP_NUM_COAPNODE 4
#endif
#ifndef MEMP_NUM_COAPPDU
#define MEMP_NUM_COAPPDU MEMP_NUM_COAPNODE
#endif
#ifndef MEMP_NUM_COAP_SUBSCRIPTION
#define MEMP_NUM_COAP_SUBSCRIPTION 4
#endif
@@ -41,6 +45,7 @@ LWIP_MEMPOOL(COAP_CONTEXT, MEMP_NUM_COAPCONTEXT, sizeof(coap_context_t), "COAP_C
LWIP_MEMPOOL(COAP_ENDPOINT, MEMP_NUM_COAPENDPOINT, sizeof(coap_endpoint_t), "COAP_ENDPOINT")
LWIP_MEMPOOL(COAP_PACKET, MEMP_NUM_COAPPACKET, sizeof(coap_packet_t), "COAP_PACKET")
LWIP_MEMPOOL(COAP_NODE, MEMP_NUM_COAPNODE, sizeof(coap_queue_t), "COAP_NODE")
LWIP_MEMPOOL(COAP_PDU, MEMP_NUM_COAPPDU, sizeof(coap_pdu_t), "COAP_PDU")
LWIP_MEMPOOL(COAP_subscription, MEMP_NUM_COAP_SUBSCRIPTION, sizeof(coap_subscription_t), "COAP_subscription")
LWIP_MEMPOOL(COAP_RESOURCE, MEMP_NUM_COAPRESOURCE, sizeof(coap_resource_t), "COAP_RESOURCE")
LWIP_MEMPOOL(COAP_RESOURCEATTR, MEMP_NUM_COAPRESOURCEATTR, sizeof(coap_attr_t), "COAP_RESOURCEATTR")
+1 -1
View File
@@ -28,7 +28,7 @@ void coap_memory_init(void);
* memory management on constrained devices.
*/
typedef enum {
COAP_STRING, COAP_ATTRIBUTE_NAME, COAP_ATTRIBUTE_VALUE, COAP_NODE, COAP_CONTEXT, COAP_ENDPOINT
COAP_STRING, COAP_ATTRIBUTE_NAME, COAP_ATTRIBUTE_VALUE, COAP_NODE, COAP_CONTEXT, COAP_ENDPOINT, COAP_PDU
} coap_memory_tag_t;
#ifndef WITH_LWIP
+2 -28
View File
@@ -533,7 +533,6 @@ coap_send_impl(coap_context_t *context,
const coap_address_t *dst,
coap_pdu_t *pdu) {
coap_tid_t id = COAP_INVALID_TID;
struct pbuf *p;
uint8_t err;
char *data_backup;
@@ -548,38 +547,13 @@ coap_send_impl(coap_context_t *context,
* should actually check that the pdu is not held by anyone but us. the
* respective pbuf is already exclusively owned by the pdu. */
p = pdu->pbuf;
LWIP_ASSERT("The PDU header is not where it is expected", pdu->hdr == p->payload + sizeof(coap_pdu_t));
err = pbuf_header(p, -sizeof(coap_pdu_t));
if (err)
{
debug("coap_send_impl: pbuf_header failed\n");
pbuf_free(p);
return id;
}
pbuf_realloc(pdu->pbuf, pdu->length);
coap_transaction_id(dst, pdu, &id);
pbuf_realloc(p, pdu->length);
udp_sendto(context->endpoint->pcb, p,
udp_sendto(context->endpoint->pcb, pdu->pbuf,
&dst->addr, dst->port);
pbuf_header(p, -(ptrdiff_t)((uint8_t*)pdu - (uint8_t*)p->payload) - sizeof(coap_pdu_t)); /* FIXME hack around udp_sendto not restoring; see http://lists.gnu.org/archive/html/lwip-users/2013-06/msg00008.html. for udp over ip over ethernet, this was -42; as we're doing ppp too, this has to be calculated generically */
err = pbuf_header(p, sizeof(coap_pdu_t));
LWIP_ASSERT("Cannot undo pbuf_header", err == 0);
/* restore destroyed pdu data */
LWIP_ASSERT("PDU not restored", p->payload == pdu);
pdu->max_size = p->tot_len - sizeof(coap_pdu_t); /* reduced after pbuf_realloc */
pdu->hdr = p->payload + sizeof(coap_pdu_t);
pdu->max_delta = 0; /* won't be used any more */
pdu->length = pdu->max_size;
pdu->data = data_backup;
pdu->pbuf = p;
return id;
}
#endif /* WITH_LWIP */
+16 -11
View File
@@ -43,9 +43,16 @@ void
coap_pdu_clear(coap_pdu_t *pdu, size_t size) {
assert(pdu);
#ifdef WITH_LWIP
/* the pdu itself is not wiped as opposed to the other implementations,
* because we have to rely on the pbuf to be set there. */
pdu->hdr = pdu->pbuf->payload;
memset(pdu->hdr, 0, size);
#else
memset(pdu, 0, sizeof(coap_pdu_t) + size);
pdu->max_size = size;
pdu->hdr = (coap_hdr_t *)((unsigned char *)pdu + sizeof(coap_pdu_t));
#endif
pdu->max_size = size;
pdu->hdr->version = COAP_DEFAULT_VERSION;
/* data is NULL unless explicitly set by coap_add_data() */
@@ -99,25 +106,22 @@ coap_pdu_init(unsigned char type, unsigned char code,
pdu = (coap_pdu_t *)memb_alloc(&pdu_storage);
#endif
#ifdef WITH_LWIP
pdu = (coap_pdu_t*)coap_malloc_type(COAP_PDU, sizeof(coap_pdu_t));
if (!pdu) return NULL;
p = pbuf_alloc(PBUF_TRANSPORT, size, PBUF_RAM);
if (p != NULL) {
u8_t header_error = pbuf_header(p, sizeof(coap_pdu_t));
/* we could catch that case and allocate larger memory in advance, but then
* again, we'd run into greater trouble with incoming packages anyway */
LWIP_ASSERT("CoAP PDU header does not fit in transport header", header_error == 0);
pdu = p->payload;
} else {
if (p == NULL) {
coap_free_type(COAP_PDU, pdu);
pdu = NULL;
}
#endif
if (pdu) {
#ifdef WITH_LWIP
pdu->pbuf = p;
#endif
coap_pdu_clear(pdu, size);
pdu->hdr->id = id;
pdu->hdr->type = type;
pdu->hdr->code = code;
#ifdef WITH_LWIP
pdu->pbuf = p;
#endif
}
return pdu;
}
@@ -147,6 +151,7 @@ coap_delete_pdu(coap_pdu_t *pdu) {
#ifdef WITH_LWIP
if (pdu != NULL) /* accepting double free as the other implementation accept that too */
pbuf_free(pdu->pbuf);
coap_free_type(COAP_PDU, pdu);
#endif
#ifdef WITH_CONTIKI
memb_free(&pdu_storage, pdu);
+1 -1
View File
@@ -213,7 +213,7 @@ typedef struct {
unsigned char *data; /**< payload */
#ifdef WITH_LWIP
struct pbuf *pbuf; /**< lwIP PBUF. The allocated coap_pdu_t will always reside inside the pbuf's payload, but the pointer has to be kept because no exact offset can be given. This field must not be accessed from outside, because the pbuf's reference count is checked to be 1 when the pbuf is assigned to the pdu, and the pbuf stays exclusive to this pdu. */
struct pbuf *pbuf; /**< lwIP PBUF. The package data will always reside inside the pbuf's payload, but this pointer has to be kept because no exact offset can be given. This field must not be accessed from outside, because the pbuf's reference count is checked to be 1 when the pbuf is assigned to the pdu, and the pbuf stays exclusive to this pdu. */
#endif
} coap_pdu_t;