> -----Original Message-----
> From: lng-odp [mailto:[email protected]] On Behalf Of EXT
> Stuart
> Haslam
> Sent: Friday, January 22, 2016 8:48 PM
> To: [email protected]
> Subject: [lng-odp] [PATCHv2 1/5] validation: pktio: make MAC address and
> promiscuous mode optional
>
> Update the unit tests to allow for interfaces that don't have a MAC
> address and therefore also don't support promiscuous mode.
>
> The pktio promiscuous mode and MAC address APIs are intended to cope
> with the common case that an interface has a single default MAC
> address. The default MAC is the one to be used as the source address
> when originating packets from that interface, it is also what's used
> to filter packets when promiscuous mode is disabled. However in some
> cases this doesn't apply, a switch port or one end of an IPC pipe may
> not have a MAC address for example.
>
> The test will also now restore the state to how it was at the beginning
> of the test, which it didn't necessarily do previously.
>
> Signed-off-by: Stuart Haslam <[email protected]>
> ---
> test/validation/pktio/pktio.c | 80 +++++++++++++++++++++++++++++++--------
> ----
> 1 file changed, 58 insertions(+), 22 deletions(-)
>
> diff --git a/test/validation/pktio/pktio.c b/test/validation/pktio/pktio.c
> index 5923969..55e5471 100644
> --- a/test/validation/pktio/pktio.c
> +++ b/test/validation/pktio/pktio.c
> @@ -79,6 +79,11 @@ pkt_segmented_e pool_segmentation =
> PKT_POOL_UNSEGMENTED;
>
> odp_pool_t pool[MAX_NUM_IFACES] = {ODP_POOL_INVALID,
> ODP_POOL_INVALID};
>
> +/* the default source and destination MAC addresses to be used if the
> + * test interface doesn't return a MAC address via odp_pktio_mac_addr() */
> +static const char pktio_test_mac_src[] = {0x02, 0x03, 0x04, 0x05, 0x06,
> 0x07};
> +static const char pktio_test_mac_dst[] = {0x02, 0x03, 0x04, 0x05, 0x06,
> 0x08};
> +
> static void set_pool_len(odp_pool_param_t *params)
> {
> switch (pool_segmentation) {
> @@ -103,10 +108,12 @@ static void pktio_pkt_set_macs(odp_packet_t pkt,
> int ret;
>
> ret = odp_pktio_mac_addr(src, ð->src, sizeof(eth->src));
> - CU_ASSERT(ret == ODPH_ETHADDR_LEN);
> + if (ret <= 0)
> + memcpy(ð->src, pktio_test_mac_src, sizeof(eth->src));
>
> ret = odp_pktio_mac_addr(dst, ð->dst, sizeof(eth->dst));
> - CU_ASSERT(ret == ODPH_ETHADDR_LEN);
> + if (ret <= 0)
> + memcpy(ð->dst, pktio_test_mac_dst, sizeof(eth->dst));
> }
>
> static uint32_t pktio_pkt_set_seq(odp_packet_t pkt)
> @@ -610,32 +617,67 @@ void pktio_test_mtu(void)
> void pktio_test_promisc(void)
> {
> int ret;
> + int enabled;
>
> odp_pktio_t pktio = create_pktio(0, ODP_PKTIN_MODE_SCHED,
> ODP_PKTOUT_MODE_SEND);
> CU_ASSERT_FATAL(pktio != ODP_PKTIO_INVALID);
>
> - ret = odp_pktio_promisc_mode_set(pktio, 1);
> - CU_ASSERT(0 == ret);
> + /* read the current promisuous state */
> + enabled = odp_pktio_promisc_mode(pktio);
> + CU_ASSERT(enabled == 1 || enabled == 0 || enabled < 0);
>
> - /* Verify that promisc mode set */
> - ret = odp_pktio_promisc_mode(pktio);
> - CU_ASSERT(1 == ret);
> + /* attempt to set to the same state */
> + ret = odp_pktio_promisc_mode_set(pktio, enabled);
> + if (ret < 0) {
> + /* failed, that's ok, some interfaces don't support it, but
> + * they must set odp_errno.. */
> + CU_ASSERT(0 != odp_errno());
> + } else {
> + /* attempt to change to the other state */
> + ret = odp_pktio_promisc_mode_set(pktio, !enabled);
The packet_io API doesn't specify that errno must be set, so the value cannot
be checked here.
I would suggest adding a new 'not supported' return value to
odp_pktio_promisc_mode_set()
and odp_pktio_promisc_mode() functions.
For example:
/**
* Enable/Disable promiscuous mode on a packet IO interface.
*
* @param[in] pktio Packet IO handle.
* @param[in] enable 1 to enable, 0 to disable.
*
* @retval 0 on success
* @retval -1 on not supported
* @retval <-1 on failure
*/
>
> - ret = odp_pktio_promisc_mode_set(pktio, 0);
> - CU_ASSERT(0 == ret);
> + /* changed the state successfully, read back and verify */
> + ret = odp_pktio_promisc_mode(pktio);
> + CU_ASSERT(!enabled == ret);
>
> - /* Verify that promisc mode is not set */
> - ret = odp_pktio_promisc_mode(pktio);
> - CU_ASSERT(0 == ret);
> + /* change it back to original state */
> + ret = odp_pktio_promisc_mode_set(pktio, enabled);
> + CU_ASSERT(0 == ret);
> +
> + ret = odp_pktio_promisc_mode(pktio);
> + CU_ASSERT(enabled == ret);
> + }
>
> ret = odp_pktio_close(pktio);
> CU_ASSERT(ret == 0);
> }
>
> +static void pktio_print_mac(unsigned char *mac_addr, int mac_len)
> +{
> + int len = 0;
> + int i;
> + char mac_str[(ODP_PKTIO_MACADDR_MAXSIZE * 3) + 1];
> +
> + if (mac_len <= 0) {
> + printf("none ... ");
> + return;
> + }
> +
> + for (i = 0; i < mac_len; ++i) {
> + len += snprintf(mac_str + len, sizeof(mac_str) - len,
> + "%02hhx:", mac_addr[i]);
> + }
> +
> + if (len) {
> + mac_str[len - 1] = '\0';
> + printf("%s ... ", mac_str);
> + }
> +}
> +
> void pktio_test_mac(void)
> {
> - unsigned char mac_addr[ODPH_ETHADDR_LEN];
> + unsigned char mac_addr[ODP_PKTIO_MACADDR_MAXSIZE];
> int mac_len;
> int ret;
> odp_pktio_t pktio;
> @@ -644,18 +686,12 @@ void pktio_test_mac(void)
> ODP_PKTOUT_MODE_SEND);
> CU_ASSERT_FATAL(pktio != ODP_PKTIO_INVALID);
>
> - printf("testing mac for %s\n", iface_name[0]);
> + printf(" MAC for %s is ", iface_name[0]);
>
> mac_len = odp_pktio_mac_addr(pktio, mac_addr, sizeof(mac_addr));
The current packet_io API basically assumes that the underlying layer is
Ethernet --> odp_pktio_mac_addr(),
odp_pktio_promisc_mode_set(), and odp_pktio_promisc_mode_get() are always valid
calls. Would it be useful
to define a new API call for fetching the underlying type (e.g. Ethernet, VALE,
PCAP)? This way the application
could avoid calling unsupported API functions.
> - 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]);
> + CU_ASSERT(0 == mac_len || ODP_PKTIO_MACADDR_MAXSIZE >=
> mac_len);
>
> - /* Fail case: wrong addr_size. Expected <0. */
> - mac_len = odp_pktio_mac_addr(pktio, mac_addr, 2);
> - CU_ASSERT(mac_len < 0);
> + pktio_print_mac(mac_addr, mac_len);
>
> ret = odp_pktio_close(pktio);
> CU_ASSERT(0 == ret);
> --
> 2.1.1
>
> _______________________________________________
> lng-odp mailing list
> [email protected]
> https://lists.linaro.org/mailman/listinfo/lng-odp
_______________________________________________
lng-odp mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/lng-odp