| Message ID | 1436908729-18958-1-git-send-email-ivan.khoronzhuk@linaro.org |
|---|---|
| State | New |
| Headers | show |
Hi Ivan, Comments Inline... On 15 July 2015 at 02:48, Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> 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 <ivan.khoronzhuk@linaro.org> > --- > .../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" > +#define CLS_PMR_SET_DADDR "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. IMO, this can be added as a separate Test Case rather than modifying on the same Test Case. 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 > >
Bala, On 15.07.15 11:31, Bala Manoharan wrote: > Hi Ivan, > > Comments Inline... > > On 15 July 2015 at 02:48, Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org > <mailto:ivan.khoronzhuk@linaro.org>> 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 <ivan.khoronzhuk@linaro.org > <mailto:ivan.khoronzhuk@linaro.org>> > --- > .../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. 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. 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 > > + 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 > >
On 15 July 2015 at 14:49, Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote: > Bala, > > On 15.07.15 11:31, Bala Manoharan wrote: > >> Hi Ivan, >> >> Comments Inline... >> >> On 15 July 2015 at 02:48, Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org >> <mailto:ivan.khoronzhuk@linaro.org>> 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 <ivan.khoronzhuk@linaro.org >> <mailto:ivan.khoronzhuk@linaro.org>> >> --- >> .../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 >> >> >>
Bala, ..... On 15.07.15 12:25, Bala Manoharan wrote: > ..... > > > 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 be good also to add tests that allow to check PMRset containing PMRs on the same level (l2 - l2, l3 - l3, (l4-l4 can be, but not in my case, at least for now)).
> On 15-Jul-2015, at 7:58 pm, Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote: > > Bala, > > ..... > >> On 15.07.15 12:25, Bala Manoharan wrote: > ..... >> >> >> 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 be good also to add tests that allow to check PMRset containing > PMRs on the same level (l2 - l2, l3 - l3, (l4-l4 can be, but not in my > case, at least for now)). I agree. I will add them as part enhancing the classification validation suite. Regards, Bala
great, thanks! I'm just adding restriction when odp_pmr_match_set_create() returns an error if PMRs are on different layers. As I see the function has to return PMR number that can be created, but AFIU it returns number less then requested only in case when platform doesn't have enough resources for that, and in case if it cannot implement l2-l2-l3 it has to return error, but not 2. On 15.07.15 17:35, Bala wrote: > > >> On 15-Jul-2015, at 7:58 pm, Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote: >> >> Bala, >> >> ..... >> >>> On 15.07.15 12:25, Bala Manoharan wrote: >> ..... >>> >>> >>> 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 be good also to add tests that allow to check PMRset containing >> PMRs on the same level (l2 - l2, l3 - l3, (l4-l4 can be, but not in my >> case, at least for now)). > > I agree. I will add them as part enhancing the classification validation suite. > > Regards, > Bala >
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" +#define CLS_PMR_SET_DADDR "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]); + 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)
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 <ivan.khoronzhuk@linaro.org> --- .../classification/odp_classification_tests.c | 42 ++++++++++++++++++++++ 1 file changed, 42 insertions(+)