Bala,
On 03.08.15 20:49, Bala Manoharan wrote:
Ivan,
On 3 August 2015 at 22:58, Ivan Khoronzhuk <ivan.khoronz...@linaro.org
<mailto:ivan.khoronz...@linaro.org>> wrote:
Bala,
On 03.08.15 20:16, Bala Manoharan wrote:
Ivan,
On 3 August 2015 at 22:33, Ivan Khoronzhuk
<ivan.khoronz...@linaro.org <mailto:ivan.khoronz...@linaro.org>
<mailto:ivan.khoronz...@linaro.org
<mailto:ivan.khoronz...@linaro.org>>> wrote:
Bala,
On 03.08.15 19:51, Bala Manoharan wrote:
On 3 August 2015 at 22:16, Ivan Khoronzhuk
<ivan.khoronz...@linaro.org
<mailto:ivan.khoronz...@linaro.org>
<mailto:ivan.khoronz...@linaro.org
<mailto:ivan.khoronz...@linaro.org>>
<mailto:ivan.khoronz...@linaro.org
<mailto:ivan.khoronz...@linaro.org>
<mailto:ivan.khoronz...@linaro.org
<mailto:ivan.khoronz...@linaro.org>>>> wrote:
Bala,
.......
what in case of TCP? Incorrect
seq num?
Looks like a coding error. TCP should
be added
using a
boolean flag.
Incorrect seq num is tested in the ASSERT
function after
receiving the
packet.
Seq num is used to check whether the
received
packet is
the same
which
was sent.
That's not enough. You use the same "fail"
function before
and after.
It's not correct when you read it before
sending
and after
sending.
It can be assigned 1 while packet creation.
Read before sending: ffff
Read after sending: ffff
No errors - it's the same.
But assigned 1, in fact that's error. And
will not
be caught.
I understand your point. seq number should be
tested for !=
TEST_SEQ_INVALID using a separate assert after
receiving the packet.
Smth similar. But I'm not sure if it's needed at all.
I've better added check like seq_num < 20 to cover all
seqnumbers
that's not possible. (Maybe 20 or other num, but it
shouldn't be
very big)
But seq_num is somthing you generate and add right? So
why do u
need to
check for multiple numbers?
we just need to know if the packet is the same which we
sent through
scheduler/poll queue.
Do you return different numbers for different error case or
something?
No. It just thought.
Maybe it's not correct place for such deep check of seqnum
assigning.
I just wanted to point that when you use cls_pkt_set_seq()
inside of
packet creation function it doesn't mean that this number
is correctly
assigned in the packet.
When you compare set and rcv values, you should use real
value, you've
set, not read. It's needed to exclude some strange errors.
Maybe it's not correct place for catching such king of errors.
But anyway, I'll better change it.
For instance:
What about to use following in each test:
cls_pkt_set_seq(pkt, ++seq_num);
rcv_pkt(pkt);
ASSERT(seq_num == cls_pkt_get_seq(pkt))
instead of:
seq_num = cls_pkt_get_seq(pkt);
rcv_pkt(pkt);
ASSERT(seq_num == cls_pkt_get_seq(pkt))
cls_pkt_get_seq(pkt) can always return the same value or etc.
You will never catch the error.
If you want to do it in packet creation function, then it's
better to check on range of real seqnums (that was my
proposition).
Actually the current code flow is something like,
pkt = create_pkt() // This call sets the seq num by calling
I think it's not correct place to set seqnum
I tend to disagree since we are creating a packet and we should be able
to add most of the parameters in this single call.
As you wish,
But you tend to pass test successfully in case of error in
cls_pkt_get_seq or cls_pkt_set_seq or odp_packet_copydata_out or
odp_packet_copydata_int ...
Pay attention that in case of success it doesn't mean that you receive
what you sent. That means, you if you have error with PMR and error with
above functions you can pass the test successfully.
You see my point, so no need to continue.
I have no objections do like you think to be better. It's not very
big problem.
--
Regards,
Ivan Khoronzhuk
_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp
