Hi Ivan,

Comments Inline...

On 15 July 2015 at 02:48, Ivan Khoronzhuk <ivan.khoronz...@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.khoronz...@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
>
>
_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp

Reply via email to