michaelh/zephyr
1
0
Fork 0
Code Releases Activity

canbus: isotp: allow frames with padding

Browse source 
ISO 15765-2 specifies that the CAN frame length for single frames and
the last consecutive frame may or may not be optimized to the actual
length of the PDU payload. The ISO-TP implementation should only
consider the information from the N_PCI section.

The previous implementation did not allow padding. With this commit,
padding is allowed by default and can even be required to be more
compliant to AUTOSAR standards.

Signed-off-by: Martin Jäger <martin@libre.solar>
This commit is contained in:
Martin Jäger 2021-01-02 16:02:38 +01:00 • committed by Anas Nashif
commit f5496147b5
3 changed files with 114 additions and 24 deletions
Download patch file Download diff file Expand all files Collapse all files

16
subsys/canbus/isotp/Kconfig
View file

@ -47,6 +47,22 @@ config ISOTP_CR_TIMEOUT
Cr (receiver consecutive frame) timeout.
ISO 15765-2: 1000ms
config ISOTP_REQUIRE_RX_PADDING
bool "Require padding for received messages"
help
If enabled, SFs and the last CF must always have a DLC of 8 bytes
(for classic CAN) and unused bytes must be padded by the sending
device. This setting allows to be compliant to AUTOSAR Specification
of CAN Transport Layer.
By default, received CAN frames with or without padding are accepted.
config ISOTP_ENABLE_TX_PADDING
bool "Enable padding for transmitted messages"
help
Add padding bytes 0xCC (as recommended by Bosch) if the PDU payload
does not fit exactly into the CAN frame.
config ISOTP_WORKQUEUE_PRIO
int "Priority level of the RX and TX work queue"
default 2

79
subsys/canbus/isotp/isotp.c
View file

@ -136,6 +136,7 @@ static void receive_send_fc(struct isotp_recv_ctx *ctx, uint8_t fs)
.id = ctx->tx_addr.ext_id
};
uint8_t *data = frame.data;
uint8_t payload_len;
int ret;
__ASSERT_NO_MSG(!(fs & ISOTP_PCI_TYPE_MASK));
@ -147,7 +148,16 @@ static void receive_send_fc(struct isotp_recv_ctx *ctx, uint8_t fs)
*data++ = ISOTP_PCI_TYPE_FC | fs;
*data++ = ctx->opts.bs;
*data++ = ctx->opts.stmin;
frame.dlc = data - frame.data;
payload_len = data - frame.data;
#if defined(CONFIG_ISOTP_REQUIRE_RX_PADDING) || \
defined(CONFIG_ISOTP_ENABLE_TX_PADDING)
/* AUTOSAR requirement SWS_CanTp_00347 */
memset(&frame.data[payload_len], 0xCC, ISOTP_CAN_DL - payload_len);
frame.dlc = ISOTP_CAN_DL;
#else
frame.dlc = payload_len;
#endif
ret = can_send(ctx->can_dev, &frame, K_MSEC(ISOTP_A),
receive_can_tx_isr, ctx);
@ -264,10 +274,10 @@ static void receive_state_machine(struct isotp_recv_ctx *ctx)
switch (ctx->state) {
case ISOTP_RX_STATE_PROCESS_SF:
ctx->buf->len = receive_get_sf_length(ctx->buf);
ctx->length = receive_get_sf_length(ctx->buf);
ud_rem_len = net_buf_user_data(ctx->buf);
*ud_rem_len = 0;
LOG_DBG("SM process SF of length %d", ctx->buf->len);
LOG_DBG("SM process SF of length %d", ctx->length);
net_buf_put(&ctx->fifo, ctx->buf);
ctx->state = ISOTP_RX_STATE_RECYCLE;
receive_state_machine(ctx);
@ -380,6 +390,7 @@ static void receive_work_handler(struct k_work *item)
static void process_ff_sf(struct isotp_recv_ctx *ctx, struct zcan_frame *frame)
{
int index = 0;
uint8_t payload_len;
if (ctx->rx_addr.use_ext_addr) {
if (frame->data[index++] != ctx->rx_addr.ext_addr) {
@ -391,19 +402,30 @@ static void process_ff_sf(struct isotp_recv_ctx *ctx, struct zcan_frame *frame)
case ISOTP_PCI_TYPE_FF:
LOG_DBG("Got FF IRQ");
if (frame->dlc != ISOTP_CAN_DL) {
LOG_INF("FF DL does not match. Ignore");
LOG_INF("FF DLC invalid. Ignore");
return;
}
payload_len = ISOTP_CAN_DL;
ctx->state = ISOTP_RX_STATE_PROCESS_FF;
ctx->sn_expected = 1;
break;
case ISOTP_PCI_TYPE_SF:
LOG_DBG("Got SF IRQ");
if ((frame->data[index] & ISOTP_PCI_FF_DL_UPPER_MASK) +
index + 1 != frame->dlc) {
LOG_INF("SF DL does not match. Ignore");
#ifdef CONFIG_ISOTP_REQUIRE_RX_PADDING
/* AUTOSAR requirement SWS_CanTp_00345 */
if (frame->dlc != ISOTP_CAN_DL) {
LOG_INF("SF DLC invalid. Ignore");
return;
}
#endif
payload_len = index + 1 + (frame->data[index] &
ISOTP_PCI_SF_DL_MASK);
if (payload_len > frame->dlc) {
LOG_INF("SF DL does not fit. Ignore");
return;
}
@ -415,7 +437,7 @@ static void process_ff_sf(struct isotp_recv_ctx *ctx, struct zcan_frame *frame)
return;
}
net_buf_add_mem(ctx->buf, &frame->data[index], frame->dlc - index);
net_buf_add_mem(ctx->buf, &frame->data[index], payload_len - index);
}
static inline void receive_add_mem(struct isotp_recv_ctx *ctx, uint8_t *data,
@ -432,7 +454,7 @@ static inline void receive_add_mem(struct isotp_recv_ctx *ctx, uint8_t *data,
net_buf_add_mem(ctx->act_frag, data, tailroom);
ctx->act_frag = ctx->act_frag->frags;
if (!ctx->act_frag) {
LOG_ERR("No fragmet left to append data");
LOG_ERR("No fragment left to append data");
receive_report_error(ctx, ISOTP_N_BUFFER_OVERFLW);
return;
}
@ -444,6 +466,7 @@ static void process_cf(struct isotp_recv_ctx *ctx, struct zcan_frame *frame)
{
uint32_t *ud_rem_len = (uint32_t *)net_buf_user_data(ctx->buf);
int index = 0;
uint32_t data_len;
if (ctx->rx_addr.use_ext_addr) {
if (frame->data[index++] != ctx->rx_addr.ext_addr) {
@ -470,14 +493,20 @@ static void process_cf(struct isotp_recv_ctx *ctx, struct zcan_frame *frame)
return;
}
if (frame->dlc - index > ctx->length) {
LOG_ERR("The frame contains more bytes than expected");
#ifdef CONFIG_ISOTP_REQUIRE_RX_PADDING
/* AUTOSAR requirement SWS_CanTp_00346 */
if (frame->dlc != ISOTP_CAN_DL) {
LOG_ERR("CF DL invalid");
receive_report_error(ctx, ISOTP_N_ERROR);
return;
}
#endif
LOG_DBG("Got CF irq. Appending data");
receive_add_mem(ctx, &frame->data[index], frame->dlc - index);
ctx->length -= frame->dlc - index;
data_len = (ctx->length > frame->dlc - index) ? frame->dlc - index :
ctx->length;
receive_add_mem(ctx, &frame->data[index], data_len);
ctx->length -= data_len;
LOG_DBG("%d bytes remaining", ctx->length);
if (ctx->length == 0) {
@ -755,6 +784,15 @@ static void send_process_fc(struct isotp_send_ctx *ctx,
return;
}
#ifdef CONFIG_ISOTP_ENABLE_TX_PADDING
/* AUTOSAR requirement SWS_CanTp_00349 */
if (frame->dlc != ISOTP_CAN_DL) {
LOG_ERR("FC DL invalid. Ignore");
send_report_error(ctx, ISOTP_N_ERROR);
return;
}
#endif
switch (*data++ & ISOTP_PCI_FS_MASK) {
case ISOTP_PCI_FS_CTS:
ctx->state = ISOTP_TX_SEND_CF;
@ -853,7 +891,13 @@ static inline int send_sf(struct isotp_send_ctx *ctx)
__ASSERT_NO_MSG(len <= ISOTP_CAN_DL - index);
memcpy(&frame.data[index], data, len);
#ifdef CONFIG_ISOTP_ENABLE_TX_PADDING
/* AUTOSAR requirement SWS_CanTp_00348 */
memset(&frame.data[index + len], 0xCC, ISOTP_CAN_DL - len - index);
frame.dlc = ISOTP_CAN_DL;
#else
frame.dlc = len + index;
#endif
ctx->state = ISOTP_TX_SEND_SF;
ret = can_send(ctx->can_dev, &frame, K_MSEC(ISOTP_A),
@ -926,10 +970,17 @@ static inline int send_cf(struct isotp_send_ctx *ctx)
rem_len = get_ctx_data_length(ctx);
len = MIN(rem_len, ISOTP_CAN_DL - index);
rem_len -= len;
frame.dlc = len + index;
data = get_data_ctx(ctx);
memcpy(&frame.data[index], data, len);
#ifdef CONFIG_ISOTP_ENABLE_TX_PADDING
/* AUTOSAR requirement SWS_CanTp_00348 */
memset(&frame.data[index + len], 0xCC, ISOTP_CAN_DL - len - index);
frame.dlc = ISOTP_CAN_DL;
#else
frame.dlc = len + index;
#endif
ret = can_send(ctx->can_dev, &frame, K_MSEC(ISOTP_A),
send_can_tx_isr, ctx);
if (ret == CAN_TX_OK) {

43
tests/subsys/canbus/isotp/conformance/src/main.c
View file

@ -31,13 +31,19 @@
#define FC_PCI_BYTE_1(FS) ((FC_PCI_TYPE << PCI_TYPE_POS) | (FS))
#define FC_PCI_BYTE_2(BS) (BS)
#define FC_PCI_BYTE_3(ST_MIN) (ST_MIN)
#define DATA_SIZE_FC 3
#define CF_PCI_TYPE 2
#define CF_PCI_BYTE_1 (CF_PCI_TYPE << PCI_TYPE_POS)
#define STMIN_VAL_1 5
#define STMIN_VAL_2 50
#define STMIN_UPPER_TOLERANCE 5
#if defined(CONFIG_ISOTP_ENABLE_TX_PADDING) || \
defined(CONFIG_ISOTP_ENABLE_TX_PADDING)
#define DATA_SIZE_FC CAN_DL
#else
#define DATA_SIZE_FC 3
#endif
#define CEIL(A, B) (((A) + (B) - 1) / (B))
#define BS_TIMEOUT_UPPER_MS 1100
@ -167,6 +173,8 @@ static void get_sf(struct isotp_recv_ctx *recv_ctx, size_t data_size)
zassert_equal(ret, 0, "Data differ");
}
#ifdef CONFIG_ISOTP_REQUIRE_RX_PADDING
static void get_sf_ignore(struct isotp_recv_ctx *recv_ctx)
{
int ret;
@ -175,6 +183,8 @@ static void get_sf_ignore(struct isotp_recv_ctx *recv_ctx)
zassert_equal(ret, ISOTP_RECV_TIMEOUT, "recv returned %d", ret);
}
#endif
static void send_test_data(const uint8_t *data, size_t len)
{
int ret;
@ -246,11 +256,18 @@ static void check_frame_series(struct frame_desired *frames, size_t length,
zassert_equal(ret, 0, "Timeout waiting for msg nr %d. ret: %d",
i, ret);
#if !defined(CONFIG_ISOTP_REQUIRE_RX_PADDING) && \
!defined(CONFIG_ISOTP_ENABLE_TX_PADDING)
zassert_equal(frame.dlc, desired->length,
"DLC of frame nr %d differ. Desired: %d, Got: %d",
i, desired->length, frame.dlc);
#endif
#if !defined(CONFIG_ISOTP_ENABLE_TX_PADDING)
ret = check_data(frame.data, desired->data, desired->length);
zassert_equal(ret, 0, "Data differ");
#endif
desired++;
}
ret = k_msgq_get(msgq, &frame, K_MSEC(200));
@ -289,7 +306,9 @@ static void prepare_cf_frames(struct frame_desired *frames, size_t frames_cnt,
memcpy(&des_frames[i].data[1], data_ptr, DATA_SIZE_CF);
if (remaining_length < DATA_SIZE_CF) {
#ifndef CONFIG_ISOTP_ENABLE_TX_PADDING
frames[i].length = remaining_length + 1;
#endif
remaining_length = 0;
}
@ -338,11 +357,13 @@ static void test_receive_sf(void)
single_frame.data[0] = SF_PCI_BYTE_LEN_8;
send_frame_series(&single_frame, 1, rx_addr.std_id);
#ifdef CONFIG_ISOTP_REQUIRE_RX_PADDING
single_frame.data[0] = SF_PCI_BYTE_1;
single_frame.length = 7;
send_frame_series(&single_frame, 1, rx_addr.std_id);
get_sf_ignore(&recv_ctx);
#endif
isotp_unbind(&recv_ctx);
}
@ -392,11 +413,13 @@ static void test_receive_sf_ext(void)
single_frame.data[1] = SF_PCI_BYTE_1;
send_frame_series(&single_frame, 1, rx_addr.std_id);
#ifdef CONFIG_ISOTP_REQUIRE_RX_PADDING
single_frame.data[1] = SF_PCI_BYTE_2_EXT;
single_frame.length = 7;
send_frame_series(&single_frame, 1, rx_addr.std_id);
get_sf_ignore(&recv_ctx);
#endif
isotp_unbind(&recv_ctx);
}
@ -418,7 +441,7 @@ static void test_send_data(void)
fc_frame.data[0] = FC_PCI_BYTE_1(FC_PCI_CTS);
fc_frame.data[1] = FC_PCI_BYTE_2(0);
fc_frame.data[2] = FC_PCI_BYTE_3(0);
fc_frame.length = 3;
fc_frame.length = DATA_SIZE_FC;
prepare_cf_frames(des_frames, ARRAY_SIZE(des_frames), data_ptr,
remaining_length);
@ -457,7 +480,7 @@ static void test_send_data_blocks(void)
fc_frame.data[0] = FC_PCI_BYTE_1(FC_PCI_CTS);
fc_frame.data[1] = FC_PCI_BYTE_2(fc_opts.bs);
fc_frame.data[2] = FC_PCI_BYTE_3(0);
fc_frame.length = 3;
fc_frame.length = DATA_SIZE_FC;
prepare_cf_frames(des_frames, ARRAY_SIZE(des_frames), data_ptr,
remaining_length);
@ -520,7 +543,7 @@ static void test_receive_data(void)
fc_frame.data[0] = FC_PCI_BYTE_1(FC_PCI_CTS);
fc_frame.data[1] = FC_PCI_BYTE_2(fc_opts_single.bs);
fc_frame.data[2] = FC_PCI_BYTE_3(fc_opts_single.stmin);
fc_frame.length = 3;
fc_frame.length = DATA_SIZE_FC;
prepare_cf_frames(des_frames, ARRAY_SIZE(des_frames), data_ptr,
remaining_length);
@ -564,7 +587,7 @@ static void test_receive_data_blocks(void)
fc_frame.data[0] = FC_PCI_BYTE_1(FC_PCI_CTS);
fc_frame.data[1] = FC_PCI_BYTE_2(fc_opts.bs);
fc_frame.data[2] = FC_PCI_BYTE_3(fc_opts.stmin);
fc_frame.length = 3;
fc_frame.length = DATA_SIZE_FC;
prepare_cf_frames(des_frames, ARRAY_SIZE(des_frames), data_ptr,
remaining_length);
@ -615,7 +638,7 @@ static void test_send_timeouts(void)
fc_cts_frame.data[0] = FC_PCI_BYTE_1(FC_PCI_CTS);
fc_cts_frame.data[1] = FC_PCI_BYTE_2(fc_opts.bs);
fc_cts_frame.data[2] = FC_PCI_BYTE_3(0);
fc_cts_frame.length = 3;
fc_cts_frame.length = DATA_SIZE_FC;
/* Test timeout for first FC*/
start_time = k_uptime_get_32();
@ -716,7 +739,7 @@ static void test_stmin(void)
fc_frame.data[0] = FC_PCI_BYTE_1(FC_PCI_CTS);
fc_frame.data[1] = FC_PCI_BYTE_2(2);
fc_frame.data[2] = FC_PCI_BYTE_3(STMIN_VAL_1);
fc_frame.length = 3;
fc_frame.length = DATA_SIZE_FC;
filter_id = attach_msgq(rx_addr.std_id);
zassert_true((filter_id >= 0), "Negative filter number [%d]",
@ -771,7 +794,7 @@ void test_receiver_fc_errors(void)
fc_frame.data[0] = FC_PCI_BYTE_1(FC_PCI_CTS);
fc_frame.data[1] = FC_PCI_BYTE_2(fc_opts.bs);
fc_frame.data[2] = FC_PCI_BYTE_3(fc_opts.stmin);
fc_frame.length = 3;
fc_frame.length = DATA_SIZE_FC;
filter_id = attach_msgq(tx_addr.std_id);
zassert_true((filter_id >= 0), "Negative filter number [%d]",
@ -819,7 +842,7 @@ void test_sender_fc_errors(void)
fc_frame.data[0] = FC_PCI_BYTE_1(3);
fc_frame.data[1] = FC_PCI_BYTE_2(fc_opts.bs);
fc_frame.data[2] = FC_PCI_BYTE_3(fc_opts.stmin);
fc_frame.length = 3;
fc_frame.length = DATA_SIZE_FC;
k_sem_reset(&send_compl_sem);
ret = isotp_send(&send_ctx, can_dev, random_data, DATA_SEND_LENGTH,
@ -841,7 +864,7 @@ void test_sender_fc_errors(void)
ret = isotp_send(&send_ctx, can_dev, random_data, 5*1024,
&tx_addr, &rx_addr, NULL, NULL);
zassert_equal(ret, ISOTP_N_BUFFER_OVERFLW,
"Expected overflow bot got %d", ret);
"Expected overflow but got %d", ret);
isotp_unbind(&recv_ctx);
filter_id = attach_msgq(tx_addr.std_id);