On Thu, Dec 22, 2016 at 08:03:07PM +0530, Bala Manoharan wrote:
> Regards,
> Bala
>
>
> On 22 December 2016 at 19:22, Josep Puigdemont
> <[email protected]> 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
> >> <[email protected]> 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 <[email protected]>
> >> >> ---
> >> >> 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 <[email protected]>
> >> >>
> >> >> 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.
Then you don't need to reverse the MACs for big endian. Since this is just
an array of bytes, like a string, the order of the bytes stays the same no
matter what the endianess of the machine is, so just declare it as:
#define PKTIO_SRC_MAC {1, 2, 3, 4, 5, 6}
/Josep
>
> -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 */
