On 15 July 2015 at 14:49, Ivan Khoronzhuk <[email protected]> wrote:
> Bala, > > On 15.07.15 11:31, Bala Manoharan wrote: > >> Hi Ivan, >> >> Comments Inline... >> >> On 15 July 2015 at 02:48, Ivan Khoronzhuk <[email protected] >> <mailto:[email protected]>> wrote: >> >> It's simple improvement is intended to open eyes on possible >> hidden issues when a packet can be lost (or sent to def CoS) >> while matching one of the rules of first PMR match set, but >> intendent to second PMR match set. To correctly check, the >> new dst CoS should be used, but for simplicity I used only one. >> It's not formated patch and is only for demonstration. >> >> Signed-off-by: Ivan Khoronzhuk <[email protected] >> <mailto:[email protected]>> >> --- >> .../classification/odp_classification_tests.c | 42 >> ++++++++++++++++++++++ >> 1 file changed, 42 insertions(+) >> >> diff --git >> a/test/validation/classification/odp_classification_tests.c >> b/test/validation/classification/odp_classification_tests.c >> index 6e8d152..b5daf32 100644 >> --- a/test/validation/classification/odp_classification_tests.c >> +++ b/test/validation/classification/odp_classification_tests.c >> @@ -41,7 +41,9 @@ >> #define TEST_PMR_SET 1 >> #define CLS_PMR_SET 5 >> #define CLS_PMR_SET_SADDR "10.0.0.6/32 <http://10.0.0.6/32>" >> +#define CLS_PMR_SET_DADDR "10.0.0.7/32 <http://10.0.0.7/32>" >> >> #define CLS_PMR_SET_SPORT 5000 >> +#define CLS_PMR_SET_SPORT2 5001 >> >> /* Config values for CoS L2 Priority */ >> #define TEST_L2_QOS 1 >> @@ -723,6 +725,7 @@ void configure_pktio_pmr_match_set_cos(void) >> odp_queue_param_t qparam; >> char cosname[ODP_COS_NAME_LEN]; >> char queuename[ODP_QUEUE_NAME_LEN]; >> + static odp_pmr_set_t pmr_set2; >> uint32_t addr = 0; >> uint32_t mask; >> >> @@ -743,6 +746,22 @@ void configure_pktio_pmr_match_set_cos(void) >> retval = odp_pmr_match_set_create(num_terms, pmr_terms, >> &pmr_set); >> CU_ASSERT(retval > 0); >> >> + parse_ipv4_string(CLS_PMR_SET_DADDR, &addr, &mask); >> + pmr_terms[0].term = ODP_PMR_DIP_ADDR; >> + pmr_terms[0].val = &addr; >> + pmr_terms[0].mask = &mask; >> + pmr_terms[0].val_sz = sizeof(addr); >> + >> + val = CLS_PMR_SET_SPORT2; >> + maskport = 0xffff; >> + pmr_terms[1].term = ODP_PMR_UDP_SPORT; >> + pmr_terms[1].val = &val; >> + pmr_terms[1].mask = &maskport; >> + pmr_terms[1].val_sz = sizeof(val); >> + >> + retval = odp_pmr_match_set_create(num_terms, pmr_terms, >> &pmr_set2); >> + CU_ASSERT(retval > 0); >> + >> sprintf(cosname, "cos_pmr_set"); >> cos_list[CLS_PMR_SET] = odp_cos_create(cosname); >> CU_ASSERT_FATAL(cos_list[CLS_PMR_SET] != ODP_COS_INVALID) >> @@ -764,6 +783,9 @@ void configure_pktio_pmr_match_set_cos(void) >> retval = odp_pktio_pmr_match_set_cos(pmr_set, pktio_loop, >> cos_list[CLS_PMR_SET]); >> CU_ASSERT(retval == 0); >> + retval = odp_pktio_pmr_match_set_cos(pmr_set2, pktio_loop, >> + cos_list[CLS_PMR_SET]); >> >> >> >> Since you are using a different pmr_match_set it is better to create >> another CoS and then attach it with this rule pmr_set2 rather than >> attaching the same CoS (cos_list[CLS_PMR_SET]). >> >> Coz the validation suite constructs a packet with different parameters, >> sends them through loop back interface and then verifies the queue in >> which the packet is arriving after classification. it is better to have >> different CoS for different PMR rules. >> > > I know, that's why I mentioned about it in comment message. > > >> IMO, this can be added as a separate Test Case rather than modifying on >> the same Test Case. >> > > It's only a proposition to add. > You can add this as separate test. I just want to show that current test > is not enough to test this primitive. > The current validation suite is just used to test the acceptance of different classification APIs. Strengthening of validation test suite is a pending activity which I am working on. > Even more, I propose to add another one test that will do the same but > with 3 PMRs in each PMRset and all of them on different layers. > For instance, DMAC - IPsrc - UDP. > Yes. This can be done. > > It can allow to see situations when API doesn't work as expected. > Some platforms, like my, can have different PDSPs for different > layers and match packets only in one direction, that can lead for > interesting hidden things. Regards, Bala > > > >> Regards, >> Bala >> >> + CU_ASSERT(retval == 0); >> } >> >> void test_pktio_pmr_match_set_cos(void) >> @@ -781,6 +803,8 @@ void test_pktio_pmr_match_set_cos(void) >> ip = (odph_ipv4hdr_t *)odp_packet_l3_ptr(pkt, NULL); >> parse_ipv4_string(CLS_PMR_SET_SADDR, &addr, &mask); >> ip->src_addr = odp_cpu_to_be_32(addr); >> + parse_ipv4_string(CLS_PMR_SET_DADDR, &addr, &mask); >> + ip->dst_addr = odp_cpu_to_be_32(addr); >> ip->chksum = 0; >> ip->chksum = odp_cpu_to_be_16(odph_ipv4_csum_update(pkt)); >> >> @@ -791,6 +815,24 @@ void test_pktio_pmr_match_set_cos(void) >> CU_ASSERT(queue == queue_list[CLS_PMR_SET]); >> CU_ASSERT(seq == cls_pkt_get_seq(pkt)); >> odp_packet_free(pkt); >> + >> + pkt = create_packet(false); >> + seq = cls_pkt_get_seq(pkt); >> + ip = (odph_ipv4hdr_t *)odp_packet_l3_ptr(pkt, NULL); >> + parse_ipv4_string(CLS_PMR_SET_SADDR, &addr, &mask); >> + ip->src_addr = odp_cpu_to_be_32(addr); >> + parse_ipv4_string(CLS_PMR_SET_DADDR, &addr, &mask); >> + ip->dst_addr = odp_cpu_to_be_32(addr); >> + ip->chksum = 0; >> + ip->chksum = odp_cpu_to_be_16(odph_ipv4_csum_update(pkt)); >> + >> + udp = (odph_udphdr_t *)odp_packet_l4_ptr(pkt, NULL); >> + udp->src_port = odp_cpu_to_be_16(CLS_PMR_SET_SPORT2); >> + enqueue_loop_interface(pkt); >> + pkt = receive_packet(&queue, ODP_TIME_SEC); >> + CU_ASSERT(queue == queue_list[CLS_PMR_SET]); >> + CU_ASSERT(seq == cls_pkt_get_seq(pkt)); >> + odp_packet_free(pkt); >> } >> >> static void classification_test_pmr_terms_avail(void) >> -- >> 1.9.1 >> >> >>
_______________________________________________ lng-odp mailing list [email protected] https://lists.linaro.org/mailman/listinfo/lng-odp
