From: Barry Spinney <spin...@mellanox.com> This patch fixes a bug in parse_ipv6 caused because the IPv6 payload length does not include the IPv6 header and a bug in parse_udp where l4_offset was subtracted from l3_offset instead of the other way around. Also fixed some bugs related to incorrect vlan_hdr_t. The corrected vlan header then required some fixes to odp_classification.c and the classifier validation test code. Also added ODPH_IPV6ADDR_LEN. Finally made a number of cosmetic changes to satisfy checkpatch.
Signed-off-by: Barry Spinney <spin...@mellanox.com> Signed-off-by: Bill Fischofer <bill.fischo...@linaro.org> --- helper/include/odp/helper/eth.h | 12 ++- helper/include/odp/helper/ip.h | 3 + platform/linux-generic/odp_classification.c | 24 +++--- platform/linux-generic/odp_packet.c | 92 +++++++++++----------- .../classification/odp_classification_common.c | 20 ++--- .../classification/odp_classification_tests.c | 7 +- 6 files changed, 85 insertions(+), 73 deletions(-) diff --git a/helper/include/odp/helper/eth.h b/helper/include/odp/helper/eth.h index 4597009..9f47ddf 100644 --- a/helper/include/odp/helper/eth.h +++ b/helper/include/odp/helper/eth.h @@ -64,7 +64,7 @@ ODP_STATIC_ASSERT(sizeof(odph_ethaddr_t) == ODPH_ETHADDR_LEN, typedef struct ODP_PACKED { odph_ethaddr_t dst; /**< Destination address */ odph_ethaddr_t src; /**< Source address */ - odp_u16be_t type; /**< Type */ + odp_u16be_t type; /**< EtherType */ } odph_ethhdr_t; /** @internal Compile time assert */ @@ -72,13 +72,17 @@ ODP_STATIC_ASSERT(sizeof(odph_ethhdr_t) == ODPH_ETHHDR_LEN, "ODPH_ETHHDR_T__SIZE_ERROR"); /** - * VLAN header + * IEEE 802.1Q VLAN header * - * @todo Check usage of tpid vs ethertype. Check outer VLAN TPID. + * This field is present when the EtherType (the odph_ethhdr_t type field) of + * the preceding ethernet header is ODPH_ETHTYPE_VLAN. The inner EtherType + * (the odph_vlanhdr_t type field) then indicates what comes next. Note that + * the so called TPID field isn't here because it overlaps with the + * odph_ethhdr_t type field. */ typedef struct ODP_PACKED { - odp_u16be_t tpid; /**< Tag protocol ID (located after ethhdr.src) */ odp_u16be_t tci; /**< Priority / CFI / VLAN ID */ + odp_u16be_t type; /**< Inner EtherType */ } odph_vlanhdr_t; /** @internal Compile time assert */ diff --git a/helper/include/odp/helper/ip.h b/helper/include/odp/helper/ip.h index f1f8cab..4cfc00f 100644 --- a/helper/include/odp/helper/ip.h +++ b/helper/include/odp/helper/ip.h @@ -152,6 +152,9 @@ static inline odp_u16sum_t odph_ipv4_csum_update(odp_packet_t pkt) /** IPv6 header length */ #define ODPH_IPV6HDR_LEN 40 +/** IPv6 address length in bytes */ +#define ODPH_IPV6ADDR_LEN 16 + /** The following constants can be used to access the three subfields * of the 4 byte ver_tc_flow field - namely the four bit Version subfield, * the eight bit Traffic Class subfield (TC) and the twenty bit Flow Label diff --git a/platform/linux-generic/odp_classification.c b/platform/linux-generic/odp_classification.c index 2062d75..25e1c2c 100644 --- a/platform/linux-generic/odp_classification.c +++ b/platform/linux-generic/odp_classification.c @@ -31,12 +31,12 @@ static pmr_tbl_t *pmr_tbl; cos_t *get_cos_entry_internal(odp_cos_t cos_id) { - return &(cos_tbl->cos_entry[_odp_typeval(cos_id)]); + return &cos_tbl->cos_entry[_odp_typeval(cos_id)]; } pmr_t *get_pmr_entry_internal(odp_pmr_t pmr_id) { - return &(pmr_tbl->pmr[_odp_typeval(pmr_id)]); + return &pmr_tbl->pmr[_odp_typeval(pmr_id)]; } int odp_classification_init_global(void) @@ -46,8 +46,8 @@ int odp_classification_init_global(void) int i; cos_shm = odp_shm_reserve("shm_odp_cos_tbl", - sizeof(cos_tbl_t), - sizeof(cos_t), 0); + sizeof(cos_tbl_t), + sizeof(cos_t), 0); if (cos_shm == ODP_SHM_INVALID) { ODP_ERR("shm allocation failed for shm_odp_cos_tbl"); @@ -67,8 +67,8 @@ int odp_classification_init_global(void) } pmr_shm = odp_shm_reserve("shm_odp_pmr_tbl", - sizeof(pmr_tbl_t), - sizeof(pmr_t), 0); + sizeof(pmr_tbl_t), + sizeof(pmr_t), 0); if (pmr_shm == ODP_SHM_INVALID) { ODP_ERR("shm allocation failed for shm_odp_pmr_tbl"); @@ -220,7 +220,6 @@ odp_pmr_t alloc_pmr(pmr_t **pmr) return ODP_PMR_INVAL; } - cos_t *get_cos_entry(odp_cos_t cos_id) { if (_odp_typeval(cos_id) >= ODP_COS_MAX_ENTRY || @@ -228,7 +227,7 @@ cos_t *get_cos_entry(odp_cos_t cos_id) return NULL; if (cos_tbl->cos_entry[_odp_typeval(cos_id)].s.valid == 0) return NULL; - return &(cos_tbl->cos_entry[_odp_typeval(cos_id)]); + return &cos_tbl->cos_entry[_odp_typeval(cos_id)]; } pmr_t *get_pmr_entry(odp_pmr_t pmr_id) @@ -238,12 +237,13 @@ pmr_t *get_pmr_entry(odp_pmr_t pmr_id) return NULL; if (pmr_tbl->pmr[_odp_typeval(pmr_id)].s.valid == 0) return NULL; - return &(pmr_tbl->pmr[_odp_typeval(pmr_id)]); + return &pmr_tbl->pmr[_odp_typeval(pmr_id)]; } int odp_cos_destroy(odp_cos_t cos_id) { cos_t *cos = get_cos_entry(cos_id); + if (NULL == cos) { ODP_ERR("Invalid odp_cos_t handle"); return -1; @@ -256,6 +256,7 @@ int odp_cos_destroy(odp_cos_t cos_id) int odp_cos_queue_set(odp_cos_t cos_id, odp_queue_t queue_id) { cos_t *cos = get_cos_entry(cos_id); + if (cos == NULL) { ODP_ERR("Invalid odp_cos_t handle"); return -1; @@ -355,6 +356,7 @@ int odp_pktio_error_cos_set(odp_pktio_t pktio_in, odp_cos_t error_cos) int odp_pktio_skip_set(odp_pktio_t pktio_in, uint32_t offset) { pktio_entry_t *entry = get_pktio_entry(pktio_in); + if (entry == NULL) { ODP_ERR("Invalid odp_cos_t handle"); return -1; @@ -367,6 +369,7 @@ int odp_pktio_skip_set(odp_pktio_t pktio_in, uint32_t offset) int odp_pktio_headroom_set(odp_pktio_t pktio_in, uint32_t headroom) { pktio_entry_t *entry = get_pktio_entry(pktio_in); + if (entry == NULL) { ODP_ERR("Invalid odp_pktio_t handle"); return -1; @@ -384,6 +387,7 @@ int odp_cos_with_l2_priority(odp_pktio_t pktio_in, uint32_t i; cos_t *cos; pktio_entry_t *entry = get_pktio_entry(pktio_in); + if (entry == NULL) { ODP_ERR("Invalid odp_pktio_t handle"); return -1; @@ -848,7 +852,7 @@ cos_t *match_qos_l2_cos(pmr_l2_cos_t *l2_cos, const uint8_t *pkt_addr, if (packet_hdr_has_l2(hdr) && hdr->input_flags.vlan && packet_hdr_has_eth(hdr)) { eth = (const odph_ethhdr_t *)(pkt_addr + hdr->l2_offset); - vlan = (const odph_vlanhdr_t *)(ð->type); + vlan = (const odph_vlanhdr_t *)(eth + 1); qos = odp_be_to_cpu_16(vlan->tci); qos = ((qos >> 13) & 0x07); cos = l2_cos->cos[qos]; diff --git a/platform/linux-generic/odp_packet.c b/platform/linux-generic/odp_packet.c index f1b9049..b002492 100644 --- a/platform/linux-generic/odp_packet.c +++ b/platform/linux-generic/odp_packet.c @@ -208,6 +208,7 @@ odp_event_t odp_packet_to_event(odp_packet_t pkt) void *odp_packet_head(odp_packet_t pkt) { odp_packet_hdr_t *pkt_hdr = odp_packet_hdr(pkt); + return buffer_map(&pkt_hdr->buf_hdr, 0, NULL, 0); } @@ -221,6 +222,7 @@ uint32_t odp_packet_buf_len(odp_packet_t pkt) void *odp_packet_data(odp_packet_t pkt) { odp_packet_hdr_t *pkt_hdr = odp_packet_hdr(pkt); + return packet_map(pkt_hdr, 0, NULL); } @@ -252,6 +254,7 @@ uint32_t odp_packet_tailroom(odp_packet_t pkt) void *odp_packet_tail(odp_packet_t pkt) { odp_packet_hdr_t *pkt_hdr = odp_packet_hdr(pkt); + return packet_map(pkt_hdr, pkt_hdr->frame_len, NULL); } @@ -375,6 +378,7 @@ void *odp_packet_offset(odp_packet_t pkt, uint32_t offset, uint32_t *len, if (addr != NULL && seg != NULL) { odp_buffer_bits_t seghandle; + seghandle.handle = (odp_buffer_t)pkt; seghandle.seg = (pkt_hdr->headroom + offset) / pkt_hdr->buf_hdr.segsize; @@ -436,6 +440,7 @@ uint32_t odp_packet_user_area_size(odp_packet_t pkt) void *odp_packet_l2_ptr(odp_packet_t pkt, uint32_t *len) { odp_packet_hdr_t *pkt_hdr = odp_packet_hdr(pkt); + if (!packet_hdr_has_l2(pkt_hdr)) return NULL; return packet_map(pkt_hdr, pkt_hdr->l2_offset, len); @@ -444,6 +449,7 @@ void *odp_packet_l2_ptr(odp_packet_t pkt, uint32_t *len) uint32_t odp_packet_l2_offset(odp_packet_t pkt) { odp_packet_hdr_t *pkt_hdr = odp_packet_hdr(pkt); + if (!packet_hdr_has_l2(pkt_hdr)) return ODP_PACKET_OFFSET_INVALID; return pkt_hdr->l2_offset; @@ -464,6 +470,7 @@ int odp_packet_l2_offset_set(odp_packet_t pkt, uint32_t offset) void *odp_packet_l3_ptr(odp_packet_t pkt, uint32_t *len) { odp_packet_hdr_t *pkt_hdr = odp_packet_hdr(pkt); + if (packet_parse_not_complete(pkt_hdr)) packet_parse_full(pkt_hdr); return packet_map(pkt_hdr, pkt_hdr->l3_offset, len); @@ -472,6 +479,7 @@ void *odp_packet_l3_ptr(odp_packet_t pkt, uint32_t *len) uint32_t odp_packet_l3_offset(odp_packet_t pkt) { odp_packet_hdr_t *pkt_hdr = odp_packet_hdr(pkt); + if (packet_parse_not_complete(pkt_hdr)) packet_parse_full(pkt_hdr); return pkt_hdr->l3_offset; @@ -493,6 +501,7 @@ int odp_packet_l3_offset_set(odp_packet_t pkt, uint32_t offset) void *odp_packet_l4_ptr(odp_packet_t pkt, uint32_t *len) { odp_packet_hdr_t *pkt_hdr = odp_packet_hdr(pkt); + if (packet_parse_not_complete(pkt_hdr)) packet_parse_full(pkt_hdr); return packet_map(pkt_hdr, pkt_hdr->l4_offset, len); @@ -501,6 +510,7 @@ void *odp_packet_l4_ptr(odp_packet_t pkt, uint32_t *len) uint32_t odp_packet_l4_offset(odp_packet_t pkt) { odp_packet_hdr_t *pkt_hdr = odp_packet_hdr(pkt); + if (packet_parse_not_complete(pkt_hdr)) packet_parse_full(pkt_hdr); return pkt_hdr->l4_offset; @@ -926,26 +936,27 @@ void odp_packet_print(odp_packet_t pkt) int max_len = 512; char str[max_len]; int len = 0; - int n = max_len-1; + int n = max_len - 1; odp_packet_hdr_t *hdr = odp_packet_hdr(pkt); - len += snprintf(&str[len], n-len, "Packet "); - len += odp_buffer_snprint(&str[len], n-len, (odp_buffer_t) pkt); - len += snprintf(&str[len], n-len, + len += snprintf(&str[len], n - len, "Packet "); + len += odp_buffer_snprint(&str[len], n - len, (odp_buffer_t)pkt); + len += snprintf(&str[len], n - len, " input_flags 0x%" PRIx32 "\n", hdr->input_flags.all); - len += snprintf(&str[len], n-len, + len += snprintf(&str[len], n - len, " error_flags 0x%" PRIx32 "\n", hdr->error_flags.all); - len += snprintf(&str[len], n-len, - " output_flags 0x%" PRIx32 "\n", hdr->output_flags.all); - len += snprintf(&str[len], n-len, + len += snprintf(&str[len], n - len, + " output_flags 0x%" PRIx32 "\n", + hdr->output_flags.all); + len += snprintf(&str[len], n - len, " l2_offset %" PRIu32 "\n", hdr->l2_offset); - len += snprintf(&str[len], n-len, + len += snprintf(&str[len], n - len, " l3_offset %" PRIu32 "\n", hdr->l3_offset); - len += snprintf(&str[len], n-len, + len += snprintf(&str[len], n - len, " l4_offset %" PRIu32 "\n", hdr->l4_offset); - len += snprintf(&str[len], n-len, + len += snprintf(&str[len], n - len, " frame_len %" PRIu32 "\n", hdr->frame_len); - len += snprintf(&str[len], n-len, + len += snprintf(&str[len], n - len, " input %" PRIu64 "\n", odp_pktio_to_u64(hdr->input)); str[len] = '\0'; @@ -1042,7 +1053,8 @@ static inline uint8_t parse_ipv6(odp_packet_hdr_t *pkt_hdr, const odph_ipv6hdr_ext_t *ipv6ext; uint32_t dstaddr0 = odp_be_to_cpu_32(ipv6->dst_addr[0]); - pkt_hdr->l3_len = odp_be_to_cpu_16(ipv6->payload_len); + pkt_hdr->l3_len = odp_be_to_cpu_16(ipv6->payload_len) + + ODPH_IPV6HDR_LEN; /* Basic sanity checks on IPv6 header */ if ((odp_be_to_cpu_32(ipv6->ver_tc_flow) >> 28) != 6 || @@ -1102,7 +1114,7 @@ static inline void parse_tcp(odp_packet_hdr_t *pkt_hdr, { const odph_tcphdr_t *tcp = (const odph_tcphdr_t *)*parseptr; - if (tcp->hl < sizeof(odph_tcphdr_t)/sizeof(uint32_t)) + if (tcp->hl < sizeof(odph_tcphdr_t) / sizeof(uint32_t)) pkt_hdr->error_flags.tcp_err = 1; else if ((uint32_t)tcp->hl * 4 > sizeof(odph_tcphdr_t)) pkt_hdr->input_flags.tcpopt = 1; @@ -1125,7 +1137,7 @@ static inline void parse_udp(odp_packet_hdr_t *pkt_hdr, if (udplen < sizeof(odph_udphdr_t) || udplen > (pkt_hdr->l3_len + - pkt_hdr->l3_offset - pkt_hdr->l4_offset)) { + pkt_hdr->l4_offset - pkt_hdr->l3_offset)) { pkt_hdr->error_flags.udp_err = 1; } @@ -1190,51 +1202,43 @@ int _odp_parse_common(odp_packet_hdr_t *pkt_hdr, const uint8_t *ptr) } /* Get Ethertype */ - parseptr = (const uint8_t *)ð->type; - ethtype = odp_be_to_cpu_16(*((const uint16_t *) - (const void *)parseptr)); + ethtype = odp_be_to_cpu_16(eth->type); + parseptr = (const uint8_t *)(eth + 1); + + /* Check for SNAP vs. DIX */ + if (ethtype < ODPH_ETH_LEN_MAX) { + pkt_hdr->input_flags.snap = 1; + if (ethtype > pkt_hdr->frame_len - offset) { + pkt_hdr->error_flags.snap_len = 1; + goto parse_exit; + } + ethtype = odp_be_to_cpu_16(*((const uint16_t *) + (uintptr_t)(parseptr + 6))); + offset += 8; + parseptr += 8; + } /* Parse the VLAN header(s), if present */ if (ethtype == ODPH_ETHTYPE_VLAN_OUTER) { pkt_hdr->input_flags.vlan_qinq = 1; pkt_hdr->input_flags.vlan = 1; - vlan = (const odph_vlanhdr_t *)(const void *)parseptr; - pkt_hdr->vlan_s_tag = ((ethtype << 16) | - odp_be_to_cpu_16(vlan->tci)); + vlan = (const odph_vlanhdr_t *)parseptr; + ethtype = odp_be_to_cpu_16(vlan->type); + pkt_hdr->vlan_s_tag = odp_be_to_cpu_16(vlan->tci); offset += sizeof(odph_vlanhdr_t); parseptr += sizeof(odph_vlanhdr_t); - ethtype = odp_be_to_cpu_16(*((const uint16_t *) - (const void *)parseptr)); } if (ethtype == ODPH_ETHTYPE_VLAN) { pkt_hdr->input_flags.vlan = 1; - vlan = (const odph_vlanhdr_t *)(const void *)parseptr; - pkt_hdr->vlan_c_tag = ((ethtype << 16) | - odp_be_to_cpu_16(vlan->tci)); + vlan = (const odph_vlanhdr_t *)parseptr; + ethtype = odp_be_to_cpu_16(vlan->type); + pkt_hdr->vlan_c_tag = odp_be_to_cpu_16(vlan->tci); offset += sizeof(odph_vlanhdr_t); parseptr += sizeof(odph_vlanhdr_t); - ethtype = odp_be_to_cpu_16(*((const uint16_t *) - (const void *)parseptr)); - } - - /* Check for SNAP vs. DIX */ - if (ethtype < ODPH_ETH_LEN_MAX) { - pkt_hdr->input_flags.snap = 1; - if (ethtype > pkt_hdr->frame_len - offset) { - pkt_hdr->error_flags.snap_len = 1; - goto parse_exit; - } - offset += 8; - parseptr += 8; - ethtype = odp_be_to_cpu_16(*((const uint16_t *) - (const void *)parseptr)); } - /* Consume Ethertype for Layer 3 parse */ - parseptr += 2; - /* Set l3_offset+flag only for known ethtypes */ pkt_hdr->input_flags.l3 = 1; pkt_hdr->l3_offset = offset; diff --git a/test/validation/classification/odp_classification_common.c b/test/validation/classification/odp_classification_common.c index c1afd00..7a42ac7 100644 --- a/test/validation/classification/odp_classification_common.c +++ b/test/validation/classification/odp_classification_common.c @@ -92,7 +92,7 @@ int cls_pkt_set_seq(odp_packet_t pkt) ip = (odph_ipv4hdr_t *)odp_packet_l3_ptr(pkt, NULL); offset = odp_packet_l4_offset(pkt); - CU_ASSERT_FATAL(offset != 0); + CU_ASSERT_FATAL(offset != ODP_PACKET_OFFSET_INVALID); if (ip->proto == ODPH_IPPROTO_UDP) status = odp_packet_copy_from_mem(pkt, offset + ODPH_UDPHDR_LEN, @@ -116,7 +116,7 @@ uint32_t cls_pkt_get_seq(odp_packet_t pkt) ip = (odph_ipv4hdr_t *)odp_packet_l3_ptr(pkt, NULL); offset = odp_packet_l4_offset(pkt); - if (!offset && !ip) + if (offset == ODP_PACKET_OFFSET_INVALID || ip == NULL) return TEST_SEQ_INVALID; if (ip->proto == ODPH_IPPROTO_UDP) @@ -269,19 +269,15 @@ odp_packet_t create_packet_len(odp_pool_t pool, bool vlan, offset += sizeof(odph_ethhdr_t); if (vlan) { /* Default vlan header */ - uint8_t *parseptr; - odph_vlanhdr_t *vlan; + odph_vlanhdr_t *vlan_hdr; - vlan = (odph_vlanhdr_t *)(ðhdr->type); - parseptr = (uint8_t *)vlan; - vlan->tci = odp_cpu_to_be_16(0); - vlan->tpid = odp_cpu_to_be_16(ODPH_ETHTYPE_VLAN); + ethhdr->type = odp_cpu_to_be_16(ODPH_ETHTYPE_VLAN); + vlan_hdr = (odph_vlanhdr_t *)(ethhdr + 1); + vlan_hdr->tci = odp_cpu_to_be_16(0); + vlan_hdr->type = odp_cpu_to_be_16(ODPH_ETHTYPE_IPV4); offset += sizeof(odph_vlanhdr_t); - parseptr += sizeof(odph_vlanhdr_t); - odp_u16be_t *type = (odp_u16be_t *)(void *)parseptr; - *type = odp_cpu_to_be_16(ODPH_ETHTYPE_IPV4); } else { - ethhdr->type = odp_cpu_to_be_16(ODPH_ETHTYPE_IPV4); + ethhdr->type = odp_cpu_to_be_16(ODPH_ETHTYPE_IPV4); } odp_packet_l3_offset_set(pkt, offset); diff --git a/test/validation/classification/odp_classification_tests.c b/test/validation/classification/odp_classification_tests.c index 1236056..ed45518 100644 --- a/test/validation/classification/odp_classification_tests.c +++ b/test/validation/classification/odp_classification_tests.c @@ -165,7 +165,6 @@ void configure_cls_pmr_chain(void) cos_list[CLS_PMR_CHAIN_SRC] = odp_cls_cos_create(cosname, &cls_param); CU_ASSERT_FATAL(cos_list[CLS_PMR_CHAIN_SRC] != ODP_COS_INVALID); - odp_queue_param_init(&qparam); qparam.type = ODP_QUEUE_TYPE_SCHED; qparam.sched.prio = ODP_SCHED_PRIO_NORMAL; @@ -389,6 +388,7 @@ void classification_test_pktio_set_skip(void) { int retval; size_t offset = 5; + retval = odp_pktio_skip_set(pktio_loop, offset); CU_ASSERT(retval == 0); @@ -406,6 +406,7 @@ void classification_test_pktio_set_headroom(void) { size_t headroom; int retval; + headroom = 5; retval = odp_pktio_headroom_set(pktio_loop, headroom); CU_ASSERT(retval == 0); @@ -475,15 +476,15 @@ void test_cos_with_l2_priority(void) odp_queue_t queue; odp_pool_t pool; uint32_t seqno = 0; - uint8_t i; + for (i = 0; i < CLS_L2_QOS_MAX; i++) { pkt = create_packet(pool_default, true, &seq, true); CU_ASSERT_FATAL(pkt != ODP_PACKET_INVALID); seqno = cls_pkt_get_seq(pkt); CU_ASSERT(seqno != TEST_SEQ_INVALID); ethhdr = (odph_ethhdr_t *)odp_packet_l2_ptr(pkt, NULL); - vlan = (odph_vlanhdr_t *)(ðhdr->type); + vlan = (odph_vlanhdr_t *)(ethhdr + 1); vlan->tci = odp_cpu_to_be_16(i << 13); enqueue_pktio_interface(pkt, pktio_loop); pkt = receive_packet(&queue, ODP_TIME_SEC_IN_NS); -- 2.5.0 _______________________________________________ lng-odp mailing list lng-odp@lists.linaro.org https://lists.linaro.org/mailman/listinfo/lng-odp