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 */