Regards,
Bala

On 22 December 2016 at 19:22, Josep Puigdemont
<josep.puigdem...@linaro.org> wrote:
> On Thu, Dec 22, 2016 at 12:22:50PM +0530, Bala Manoharan wrote:
>> Regards,
>> Bala
>>
>>
>> On 8 December 2016 at 21:03, Josep Puigdemont
>> <josep.puigdem...@linaro.org> wrote:
>> > On Thu, Nov 10, 2016 at 07:58:39PM +0530, Bala Manoharan wrote:
>> >> Fixes https://bugs.linaro.org/show_bug.cgi?id=2496
>> >>
>> >> Signed-off-by: Balasubramanian Manoharan <bala.manoha...@linaro.org>
>> >> ---
>> >> v2: Incorporate review comments
>> >>  test/common_plat/validation/api/pktio/pktio.c | 24 
>> >> +++++++++++++++++++++---
>> >>  1 file changed, 21 insertions(+), 3 deletions(-)
>> >>
>> >> --
>> >> 1.9.1
>> >> Signed-off-by: Balasubramanian Manoharan <bala.manoha...@linaro.org>
>> >>
>> >> diff --git a/test/common_plat/validation/api/pktio/pktio.c 
>> >> b/test/common_plat/validation/api/pktio/pktio.c
>> >> index a6a18c3..7115def 100644
>> >> --- a/test/common_plat/validation/api/pktio/pktio.c
>> >> +++ b/test/common_plat/validation/api/pktio/pktio.c
>> >> @@ -31,6 +31,8 @@
>> >>  #define PKTIN_TS_MAX_RES       10000000000
>> >>  #define PKTIN_TS_CMP_RES       1
>> >>
>> >> +#define PKTIO_SRC_MAC                {1, 2, 3, 4, 5, 6}
>> >> +#define PKTIO_DST_MAC                {1, 2, 3, 4, 5, 6}
>> >>  #undef DEBUG_STATS
>> >>
>> >>  /** interface names used for testing */
>> >> @@ -241,16 +243,32 @@ static uint32_t pktio_init_packet(odp_packet_t pkt)
>> >>       odph_udphdr_t *udp;
>> >>       char *buf;
>> >>       uint16_t seq;
>> >> -     uint8_t mac[ODP_PKTIO_MACADDR_MAXSIZE] = {0};
>> >> +     uint8_t src_mac[ODP_PKTIO_MACADDR_MAXSIZE] = PKTIO_SRC_MAC;
>> >> +     uint8_t dst_mac[ODP_PKTIO_MACADDR_MAXSIZE] = PKTIO_DST_MAC;
>> >> +     uint8_t src_mac_be[ODP_PKTIO_MACADDR_MAXSIZE];
>> >> +     uint8_t dst_mac_be[ODP_PKTIO_MACADDR_MAXSIZE];
>> >
>> > we don't need big endian versions of the MAC address, it's a string of
>> > bytes, so it has no endianess.
>> >
>> >>       int pkt_len = odp_packet_len(pkt);
>> >> +     int i;
>> >> +
>> >> +     #if ODP_BYTE_ORDER == ODP_LITTLE_ENDIAN
>> >> +     for (i = 0; i < ODP_PKTIO_MACADDR_MAXSIZE; i++) {
>> >> +             src_mac_be[i] = src_mac[i];
>> >> +             dst_mac_be[i] = dst_mac[i];
>> >> +     }
>> >> +     #else
>> >> +     for (i = 0; i < ODP_PKTIO_MACADDR_MAXSIZE; i++) {
>> >> +             src_mac_be[i] = src_mac[ODP_PKTIO_MACADDR_MAXSIZE - i];
>> >> +             dst_mac_be[i] = dst_mac[ODP_PKTIO_MACADDR_MAXSIZE - i];
>> >> +     }
>> >> +     #endif
>> >
>> > this is not needed.
>> > For other than ODP_LITTLE_ENDIAN this just reverses the MAC address, but I
>> > guess it wouldn't matter for the test.
>>
>> This will have an issue since we have a mac addr based test case to be
>> added for PMR and it will fail if the address is not reversed.
>
> I don't understand why you would need the MAC to be reversed on big
> endian but not on little endian architectures... but if we really need
> a reversed MAC address here, I would suggest having it reversed in the
> macro, not at run-time:

I want the mac address to be same for both architectures that is the
reason I am reversing for big endian alone.

-Bala
>
> #if ODP_BYTE_ORDER == ODP_LITTLE_ENDIAN
> #define PKTIO_SRC_MAC {1, 2, 3, 4, 5, 6}
> #else
> #define PKTIO_SRC_MAC {6, 5, 4, 3, 2, 1}
> #endif
>
> ...
> uint8_t src_mac[ODP_PKTIO_MACADDR_MAXSIZE] = PKTIO_SRC_MAC;
> uint8_t dst_mac[ODP_PKTIO_MACADDR_MAXSIZE] = PKTIO_SRC_MAC;
> ...
>
> Also if the MACs are the same, we could use only one variable...
>
> Regard,
> /Josep
>
>>
>> Regards,
>> Bala
>> >
>> >>
>> >>       buf = odp_packet_data(pkt);
>> >>
>> >>       /* Ethernet */
>> >>       odp_packet_l2_offset_set(pkt, 0);
>> >>       eth = (odph_ethhdr_t *)buf;
>> >> -     memcpy(eth->src.addr, mac, ODPH_ETHADDR_LEN);
>> >> -     memcpy(eth->dst.addr, mac, ODPH_ETHADDR_LEN);
>> >> +     memcpy(eth->src.addr, &src_mac_be, ODPH_ETHADDR_LEN);
>> >> +     memcpy(eth->dst.addr, &dst_mac_be, ODPH_ETHADDR_LEN);
>> >
>> > use src_mac and dst_mac instead.
>> >
>> > Sorry for the late reply, I missed this earlier.
>> >
>> > /Josep
>> >>       eth->type = odp_cpu_to_be_16(ODPH_ETHTYPE_IPV4);
>> >>
>> >>       /* IP */

Reply via email to