> -----Original Message-----
> From: [email protected] [mailto:lng-odp-
> [email protected]] On Behalf Of ext Ola Liljedahl
> Sent: Tuesday, February 03, 2015 6:48 PM
> To: [email protected]
> Subject: [lng-odp] [PATCHv5 02/18] api: odp_pktio.h: odp_pktio_mac_addr()
> return chars written or error
>
> Added define ODP_PKTIO_MACADDRSIZE which specifies the recommended
> output buffer size for odp_pktio_mac_addr().
> odp_pktio_mac_addr() takes output buffer size as input and returns
> number
> of chars written (on success), <0 on failure.
> Updated the implementation.
> Updated all usages in example and test programs.
>
> Signed-off-by: Ola Liljedahl <[email protected]>
> ---
> (This document/code contribution attached is provided under the terms of
> agreement LES-LTM-21309)
>
> include/odp/api/packet_io.h | 20 +++++++++++++--
> -----
> .../linux-generic/include/odp/plat/packet_io_types.h | 2 ++
> platform/linux-generic/odp_packet_io.c | 11 ++++++-----
> test/validation/odp_pktio.c | 8 ++++----
> 4 files changed, 25 insertions(+), 16 deletions(-)
>
> diff --git a/include/odp/api/packet_io.h b/include/odp/api/packet_io.h
> index da7bc3f..0c8af0c 100644
> --- a/include/odp/api/packet_io.h
> +++ b/include/odp/api/packet_io.h
> @@ -18,6 +18,7 @@
> extern "C" {
> #endif
>
> +#include <sys/types.h>
>
> /** @defgroup odp_packet_io ODP PACKET IO
> * Operations on a packet.
> @@ -39,6 +40,11 @@ extern "C" {
> * odp_pktio_t value to indicate any port
> */
>
> +/*
> + * @def ODP_PKTIO_MACADDRSIZE
> + * Minimum size of output buffer for odp_pktio_mac_addr()
Should this be defined as ODP_PKTIO_MACADDRSIZE_MAX or similar. If the
interface support two sizes this is the larger, although the configured and
currently used size would be the smaller.
I want to avoid applications mixing this into protocol definitions like this
struct ether_hdr {
uint8_t dst_mac[ODP_PKTIO_MACADDRSIZE];
uint8_t src_mac[ODP_PKTIO_MACADDRSIZE];
...
};
> + */
> +
> /**
> * Open an ODP packet IO instance
> *
> @@ -167,16 +173,16 @@ int odp_pktio_promisc_mode_set(odp_pktio_t id,
> odp_bool_t enable);
> int odp_pktio_promisc_mode(odp_pktio_t id);
>
> /**
> - * Get the default MAC address of a packet IO interface.
> + * Get the native MAC address of a packet IO interface.
Why this is now defined as "native". Does it mean the address burned into the
device. What if SW has overwritten that? Is it still native? Default is
referring to the default address, whatever that maybe.
> *
> - * @param id ODP packet IO handle.
> - * @param[out] mac_addr Storage for MAC address of the packet IO
> interface.
> - * @param addr_size Storage size for the address
> + * @param hdl ODP packet IO handle
This parameter renaming is not needed. All other packet_io.h API calls are
using "id" here. Parameter naming should be constant throughout the API. Do not
rename it in this patch.
I agree that "id" is not the best name for that parameter, but that's how it is
in this API right now. We need another patch for renaming that over all the API
calls.
Also "hdl" is not a good parameter name for an object. The name should link to
the object type, like "pktio" (for odp_pktio_t type) in this case. Parameter
list shows up in Doxygen function documentation without types.
-Petri
> + * @param[out] mac_addr Output buffer (use ODP_PKTIO_MACADDRSIZE)
> + * @param size Size of output buffer
> *
> - * @retval Number of bytes written on success, 0 on failure.
> + * @return Number of bytes written to buffer
> + * @retval <0 on failure
> */
> -size_t odp_pktio_mac_addr(odp_pktio_t id, void *mac_addr,
> - size_t addr_size);
> +ssize_t odp_pktio_mac_addr(odp_pktio_t hdl, void *mac_addr, ssize_t
> size);
>
> /**
> * Setup per-port default class-of-service.
> diff --git a/platform/linux-generic/include/odp/plat/packet_io_types.h
> b/platform/linux-generic/include/odp/plat/packet_io_types.h
> index a6bbacf..61dca13 100644
> --- a/platform/linux-generic/include/odp/plat/packet_io_types.h
> +++ b/platform/linux-generic/include/odp/plat/packet_io_types.h
> @@ -23,6 +23,8 @@ extern "C" {
> * @{
> */
>
> +#define ODP_PKTIO_MACADDRSIZE 16
> +
> typedef uint32_t odp_pktio_t;
>
> #define ODP_PKTIO_INVALID 0
> diff --git a/platform/linux-generic/odp_packet_io.c b/platform/linux-
> generic/odp_packet_io.c
> index bdb690e..f156dd3 100644
> --- a/platform/linux-generic/odp_packet_io.c
> +++ b/platform/linux-generic/odp_packet_io.c
> @@ -798,18 +798,19 @@ int odp_pktio_promisc_mode(odp_pktio_t id)
> }
>
>
> -size_t odp_pktio_mac_addr(odp_pktio_t id, void *mac_addr,
> - size_t addr_size)
> +ssize_t odp_pktio_mac_addr(odp_pktio_t id, void *mac_addr, ssize_t
> addr_size)
> {
> pktio_entry_t *entry;
>
> - if (addr_size < ETH_ALEN)
> - return 0;
> + if (addr_size < ETH_ALEN) {
> + /* Output buffer too small */
> + return -1;
> + }
>
> entry = get_pktio_entry(id);
> if (entry == NULL) {
> ODP_DBG("pktio entry %d does not exist\n", id);
> - return 0;
> + return -1;
> }
>
> lock_entry(entry);
> diff --git a/test/validation/odp_pktio.c b/test/validation/odp_pktio.c
> index 84121f5..e1ca793 100644
> --- a/test/validation/odp_pktio.c
> +++ b/test/validation/odp_pktio.c
> @@ -450,22 +450,22 @@ static void test_odp_pktio_promisc(void)
> static void test_odp_pktio_mac(void)
> {
> unsigned char mac_addr[ODPH_ETHADDR_LEN];
> - size_t mac_len;
> + ssize_t mac_len;
> int ret;
> odp_pktio_t pktio = create_pktio(iface_name[0]);
>
> printf("testing mac for %s\n", iface_name[0]);
>
> - mac_len = odp_pktio_mac_addr(pktio, mac_addr, ODPH_ETHADDR_LEN);
> + mac_len = odp_pktio_mac_addr(pktio, mac_addr, sizeof(mac_addr));
> CU_ASSERT(ODPH_ETHADDR_LEN == mac_len);
>
> printf(" %X:%X:%X:%X:%X:%X ",
> mac_addr[0], mac_addr[1], mac_addr[2],
> mac_addr[3], mac_addr[4], mac_addr[5]);
>
> - /* Fail case: wrong addr_size. Expected 0. */
> + /* Fail case: wrong addr_size. Expected <0. */
> mac_len = odp_pktio_mac_addr(pktio, mac_addr, 2);
> - CU_ASSERT(0 == mac_len);
> + CU_ASSERT(mac_len < 0);
>
> ret = odp_pktio_close(pktio);
> CU_ASSERT(0 == ret);
> --
> 1.9.1
>
>
> _______________________________________________
> lng-odp mailing list
> [email protected]
> http://lists.linaro.org/mailman/listinfo/lng-odp
_______________________________________________
lng-odp mailing list
[email protected]
http://lists.linaro.org/mailman/listinfo/lng-odp
