| Message ID | 1478788119-3855-1-git-send-email-bala.manoharan@linaro.org |
|---|---|
| State | New |
| Headers | show |
Ping. Review needed. Regards, Bala On 10 November 2016 at 19:58, Balasubramanian Manoharan <bala.manoharan@linaro.org> wrote: > Fixes https://bugs.linaro.org/show_bug.cgi?id=2496 > > Signed-off-by: Balasubramanian Manoharan <bala.manoharan@linaro.org> > --- > 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 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]; > 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, &src_mac_be, ODPH_ETHADDR_LEN); > + memcpy(eth->dst.addr, &dst_mac_be, ODPH_ETHADDR_LEN); > eth->type = odp_cpu_to_be_16(ODPH_ETHTYPE_IPV4); > > /* IP */ > -- > 1.9.1 >
going to apply this just after current dev release. Maxim. On 11/14/16 12:37, Bala Manoharan wrote: > Ping. Review needed. > > Regards, > Bala > > > On 10 November 2016 at 19:58, Balasubramanian Manoharan > <bala.manoharan@linaro.org> wrote: >> Fixes https://bugs.linaro.org/show_bug.cgi?id=2496 >> >> Signed-off-by: Balasubramanian Manoharan <bala.manoharan@linaro.org> >> --- >> 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 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]; >> 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, &src_mac_be, ODPH_ETHADDR_LEN); >> + memcpy(eth->dst.addr, &dst_mac_be, ODPH_ETHADDR_LEN); >> eth->type = odp_cpu_to_be_16(ODPH_ETHTYPE_IPV4); >> >> /* IP */ >> -- >> 1.9.1 >>
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.manoharan@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.manoharan@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. > > 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 */
Regards, Bala On 8 December 2016 at 21:03, Josep Puigdemont <josep.puigdemont@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.manoharan@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.manoharan@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. 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 */
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.puigdemont@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.manoharan@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.manoharan@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: #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 */
On Thu, Dec 22, 2016 at 7:52 AM, Josep Puigdemont <josep.puigdemont@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.puigdemont@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.manoharan@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.manoharan@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: > > #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, &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 */
Regards, Bala On 22 December 2016 at 19:22, Josep Puigdemont <josep.puigdemont@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.puigdemont@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.manoharan@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.manoharan@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 */
On Thu, Dec 22, 2016 at 08:03:07PM +0530, Bala Manoharan wrote: > Regards, > Bala > > > On 22 December 2016 at 19:22, Josep Puigdemont > <josep.puigdemont@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.puigdemont@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.manoharan@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.manoharan@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. 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 */
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]; 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, &src_mac_be, ODPH_ETHADDR_LEN); + memcpy(eth->dst.addr, &dst_mac_be, ODPH_ETHADDR_LEN); eth->type = odp_cpu_to_be_16(ODPH_ETHTYPE_IPV4); /* IP */
Fixes https://bugs.linaro.org/show_bug.cgi?id=2496 Signed-off-by: Balasubramanian Manoharan <bala.manoharan@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