Skip to content

Commit fcac6c4

Browse files
troelsjessenjeanbaptistelab
authored andcommitted
Handle packets to be routed forward and those for the itf owner differently
In the current csp_can_pbuf_new() function implementation, CSP buffer exhaustion will cause the calling thread to crash (csp_buffer_get_always() will assert). In the context of a CSH instance acting as a router, this means that as soon as this situation occurs, the router will stop functioning *silently*, requiring a manual restart of CSH (the router thread will enter an infinite loop due to csp_panic() not being over-written in CSH (separate issue to be addressed)). We have observed this behaviour where bursts of packets to be routed will cause the router to stop working due to buffer exhaustion. To mitigate this issue, csp_can_pbuf_new() was changed to handle buffer exhaustion more gracefully, dropping to be routed packets, while still aborting in case where the packet was for the interface owner, but not buffers where available. Dropping packets will allow code to free up CSP buffers once the burst is over, making the infrastructure much more resilient. The same logic is applied in ZMQ's csp_zmqhub_task() function
1 parent aceef30 commit fcac6c4

3 files changed

Lines changed: 39 additions & 12 deletions

File tree

src/interfaces/csp_if_can.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,10 @@ static int csp_can1_rx(csp_iface_t * iface, uint32_t id, const uint8_t * data, u
5858
if (packet == NULL) {
5959
if (CFP_TYPE(id) == CFP_BEGIN) {
6060
packet = csp_can_pbuf_new(ifdata, id, task_woken);
61+
if (packet == NULL) {
62+
iface->drop++;
63+
return CSP_ERR_NOBUFS;
64+
}
6165
} else {
6266
iface->frame++;
6367
return CSP_ERR_INVAL;
@@ -269,6 +273,10 @@ static int csp_can2_rx(csp_iface_t * iface, uint32_t id, const uint8_t * data, u
269273
if (packet == NULL) {
270274
if (id & (CFP2_BEGIN_MASK << CFP2_BEGIN_OFFSET)) {
271275
packet = csp_can_pbuf_new(ifdata, id, task_woken);
276+
if (packet == NULL) {
277+
iface->drop++;
278+
return CSP_ERR_NOBUFS;
279+
}
272280
} else {
273281
iface->frame++;
274282
return CSP_ERR_INVAL;

src/interfaces/csp_if_can_pbuf.c

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#include <csp/csp_error.h>
77
#include <csp/arch/csp_time.h>
88
#include <csp/interfaces/csp_if_can.h>
9+
#include <csp/csp_iflist.h>
910

1011
#include "../csp_buffer_private.h"
1112

@@ -51,15 +52,25 @@ csp_packet_t * csp_can_pbuf_new(csp_can_interface_data_t * ifdata, uint32_t id,
5152

5253
uint32_t now = (task_woken) ? csp_get_ms_isr() : csp_get_ms();
5354

54-
csp_packet_t * packet = (task_woken) ? csp_buffer_get_always_isr() : csp_buffer_get_always();
55+
csp_packet_t * packet;
56+
if (csp_iflist_get_by_addr(((id >> CFP2_DST_OFFSET) & CFP2_DST_MASK)) != NULL) {
57+
/* The packet is for us, make sure we don't silently ignore the situation if we can't process it */
58+
packet = (task_woken) ? csp_buffer_get_always_isr() : csp_buffer_get_always();
59+
} else {
60+
/* The packet is not for us, it is ok to drop it if we don't have enough buffers*/
61+
packet = (task_woken) ? csp_buffer_get_isr(0) : csp_buffer_get(0);
62+
}
63+
64+
if (packet != NULL) {
5565

56-
packet->last_used = now;
57-
packet->cfpid = id;
58-
packet->remain = 0;
66+
packet->last_used = now;
67+
packet->cfpid = id;
68+
packet->remain = 0;
5969

60-
/* Insert at beginning, because easy */
61-
packet->next = ifdata->pbufs;
62-
ifdata->pbufs = packet;
70+
/* Insert at beginning, because easy */
71+
packet->next = ifdata->pbufs;
72+
ifdata->pbufs = packet;
73+
}
6374

6475
return packet;
6576
}

src/interfaces/csp_if_zmqhub.c

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -129,18 +129,26 @@ static void * csp_zmqhub_task(void * param) {
129129
continue;
130130
}
131131

132+
// Copy the data from zmq to csp
133+
const uint8_t * rx_data = zmq_msg_data(&msg);
134+
rx_data = csp_zmqhub_fixup_cspv1_del_dest_addr(rx_data, &datalen);
135+
132136
// Create new csp packet
133-
packet = csp_buffer_get(0);
137+
if (csp_iflist_get_by_addr(*((uint16_t*)&rx_data[2]) & 0x3FFF) != NULL) {
138+
/* The packet is for us, make sure we don't silently ignore the situation if we can't process it */
139+
packet = csp_buffer_get_always();
140+
} else {
141+
/* The packet is not for us, it is ok to drop it if we don't have enough buffers*/
142+
packet = csp_buffer_get(0);
143+
}
144+
134145
if (packet == NULL) {
135146
csp_print("RX %s: Failed to get csp_buffer(%u)\n", drv->iface.name, datalen);
147+
drv->iface.drop++;
136148
zmq_msg_close(&msg);
137149
continue;
138150
}
139151

140-
// Copy the data from zmq to csp
141-
uint8_t * rx_data = zmq_msg_data(&msg);
142-
rx_data = csp_zmqhub_fixup_cspv1_del_dest_addr(rx_data, &datalen);
143-
144152
csp_id_setup_rx(packet);
145153

146154
memcpy(packet->frame_begin, rx_data, datalen);

0 commit comments

Comments
 (0)