Re: [lng-odp] [PATCHv2, 1/1] validation: pktio: fix invalid mac addr

2016-12-22 Thread Josep Puigdemont 
On Thu, Dec 22, 2016 at 08:03:07PM +0530, Bala Manoharan wrote:
> Regards,
> Bala
> 
> 
> On 22 December 2016 at 19:22, Josep Puigdemont
>  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
> >>  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 
> >> >> ---
> >> >> 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 
> >> >>
> >> >> 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   100
> >> >>  #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, _mac_be, ODPH_ETHADDR_LEN);
> >> >> + memcpy(eth->dst.addr, _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 */

Re: [lng-odp] [PATCHv2, 1/1] validation: pktio: fix invalid mac addr

2016-12-22 Thread Bala Manoharan 
Regards,
Bala


On 22 December 2016 at 19:22, Josep Puigdemont
 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
>>  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 
>> >> ---
>> >> 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 
>> >>
>> >> 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   100
>> >>  #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, _mac_be, ODPH_ETHADDR_LEN);
>> >> + memcpy(eth->dst.addr, _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 */

Re: [lng-odp] [PATCHv2, 1/1] validation: pktio: fix invalid mac addr

2016-12-22 Thread Bill Fischofer 
On Thu, Dec 22, 2016 at 7:52 AM, Josep Puigdemont
 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
>>  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 
>> >> ---
>> >> 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 
>> >>
>> >> 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   100
>> >>  #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:
>
> #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;

+1 for Josep's suggestion.

> ...
>
> 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, _mac_be, ODPH_ETHADDR_LEN);
>> >> + memcpy(eth->dst.addr, _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 */

Re: [lng-odp] [PATCHv2, 1/1] validation: pktio: fix invalid mac addr

2016-12-22 Thread Josep Puigdemont 
On Thu, Dec 22, 2016 at 12:22:50PM +0530, Bala Manoharan wrote:
> Regards,
> Bala
> 
> 
> On 8 December 2016 at 21:03, Josep Puigdemont
>  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 
> >> ---
> >> 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 
> >>
> >> 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   100
> >>  #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:

#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, _mac_be, ODPH_ETHADDR_LEN);
> >> + memcpy(eth->dst.addr, _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 */

Re: [lng-odp] [PATCHv2, 1/1] validation: pktio: fix invalid mac addr

2016-12-21 Thread Bala Manoharan 
Regards,
Bala


On 8 December 2016 at 21:03, Josep Puigdemont
 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 
>> ---
>> 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 
>>
>> 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   100
>>  #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.

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, _mac_be, ODPH_ETHADDR_LEN);
>> + memcpy(eth->dst.addr, _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 */

Re: [lng-odp] [PATCHv2, 1/1] validation: pktio: fix invalid mac addr

2016-12-08 Thread Josep Puigdemont 
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 
> ---
> 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 
> 
> 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   100
>  #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.

>  
>   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, _mac_be, ODPH_ETHADDR_LEN);
> + memcpy(eth->dst.addr, _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 */

Re: [lng-odp] [PATCHv2 1/1] validation: pktio: fix invalid mac addr

2016-11-17 Thread Bala Manoharan 
Ping. Review needed.

Regards,
Bala


On 10 November 2016 at 19:58, Balasubramanian Manoharan
 wrote:
> Fixes https://bugs.linaro.org/show_bug.cgi?id=2496
>
> Signed-off-by: Balasubramanian Manoharan 
> ---
> v2: Incorporate review comments
>  test/common_plat/validation/api/pktio/pktio.c | 24 +---
>  1 file changed, 21 insertions(+), 3 deletions(-)
>
> 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   100
>  #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];
> 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
>
> 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, _mac_be, ODPH_ETHADDR_LEN);
> +   memcpy(eth->dst.addr, _mac_be, ODPH_ETHADDR_LEN);
> eth->type = odp_cpu_to_be_16(ODPH_ETHTYPE_IPV4);
>
> /* IP */
> --
> 1.9.1
>

[lng-odp] [PATCHv2 1/1] validation: pktio: fix invalid mac addr

2016-11-10 Thread Balasubramanian Manoharan 
Fixes https://bugs.linaro.org/show_bug.cgi?id=2496

Signed-off-by: Balasubramanian Manoharan 
---
v2: Incorporate review comments
 test/common_plat/validation/api/pktio/pktio.c | 24 +---
 1 file changed, 21 insertions(+), 3 deletions(-)

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   100
 #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];
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
 
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, _mac_be, ODPH_ETHADDR_LEN);
+   memcpy(eth->dst.addr, _mac_be, ODPH_ETHADDR_LEN);
eth->type = odp_cpu_to_be_16(ODPH_ETHTYPE_IPV4);
 
/* IP */
-- 
1.9.1