tapv2: CLI and binary API fixes

1. When interface create encouners an error (see test below),
the same id cannot be used again.
This is due to hash_set is called too early in the function. After the
hash entry is set, there are different errors may cause the interface
create to be aborted. But we didn't remove the hash entry when error is
encountered. The fix is to move the hash_set call near the end which has
no more "goto error"

DBGvpp# create tap id 1 rx-ring-size 1021 tx-ring-size 1021
create tap id 1 rx-ring-size 1021 tx-ring-size 1021
create tap: ring size must be power of 2
DBGvpp# create tap id 1 rx-ring-size 1024 tx-ring-size 1024
create tap id 1 rx-ring-size 1024 tx-ring-size 1024
create tap: interface already exists
DBGvpp#

2. multiple issues exist with api_format.c with the below command

binary-api tap_create_v2 id 4 hw-addr 90:e2:ba:76:cf:2f rx-ring-size 1024 tx-ring-size 1024

- hw_addr is not taken due to the test for random mac is inverted
- id is an integer, not a string
- integer values were not converted to network format

Change-Id: I5a669d702a80ad158517df46f0ab089e4d0d692e
Signed-off-by: Steven <sluong@cisco.com>

diff --git a/src/vat/api_format.c b/src/vat/api_format.c
index 9962561..dc7111d 100644 (file)
--- a/src/vat/api_format.c
+++ b/src/vat/api_format.c
@@ -7782,7 +7782,7 @@ api_tap_create_v2 (vat_main_t * vam)
   u8 host_ip6_gw_set = 0;
   u32 host_ip6_prefix_len = 0;
   int ret;
-  int rx_ring_sz = 0, tx_ring_sz = 0;
+  u32 rx_ring_sz = 0, tx_ring_sz = 0;
 
   memset (mac_address, 0, sizeof (mac_address));
 
@@ -7793,7 +7793,7 @@ api_tap_create_v2 (vat_main_t * vam)
        {
          random_mac = 0;
        }
-      else if (unformat (i, "id %s", &id))
+      else if (unformat (i, "id %u", &id))
        ;
       else if (unformat (i, "host-if-name %s", &host_if_name))
        ;
@@ -7875,15 +7875,15 @@ api_tap_create_v2 (vat_main_t * vam)
 
   mp->use_random_mac = random_mac;
 
-  mp->id = id;
+  mp->id = ntohl (id);
   mp->host_namespace_set = host_ns != 0;
   mp->host_bridge_set = host_bridge != 0;
   mp->host_ip4_addr_set = host_ip4_prefix_len != 0;
   mp->host_ip6_addr_set = host_ip6_prefix_len != 0;
-  mp->rx_ring_sz = rx_ring_sz;
-  mp->tx_ring_sz = tx_ring_sz;
+  mp->rx_ring_sz = ntohs (rx_ring_sz);
+  mp->tx_ring_sz = ntohs (tx_ring_sz);
 
-  if (random_mac)
+  if (random_mac == 0)
     clib_memcpy (mp->mac_address, mac_address, 6);
   if (host_mac_addr_set)
     clib_memcpy (mp->host_mac_addr, host_mac_addr, 6);
@@ -22776,7 +22776,7 @@ _(tap_delete,                                                           \
   "<vpp-if-name> | sw_if_index <id>")                                   \
 _(sw_interface_tap_dump, "")                                            \
 _(tap_create_v2,                                                        \
-  "name <name> [hw-addr <mac-addr>] [host-ns <name>] [rx-ring-size <num> [tx-ring-size <num>]") \
+  "id <num> [hw-addr <mac-addr>] [host-ns <name>] [rx-ring-size <num> [tx-ring-size <num>]") \
 _(tap_delete_v2,                                                        \
   "<vpp-if-name> | sw_if_index <id>")                                   \
 _(sw_interface_tap_v2_dump, "")                                         \

diff --git a/src/vnet/devices/tap/tap.c b/src/vnet/devices/tap/tap.c
index eb5a7a9..8005b34 100644 (file)
--- a/src/vnet/devices/tap/tap.c
+++ b/src/vnet/devices/tap/tap.c
@@ -130,8 +130,6 @@ tap_create_if (vlib_main_t * vm, tap_create_if_args_t * args)
   vif->tap_fd = -1;
   vif->id = args->id;
 
-  hash_set (tm->dev_instance_by_interface_id, vif->id, vif->dev_instance);
-
   if ((vif->fd = open ("/dev/vhost-net", O_RDWR | O_NONBLOCK)) < 0)
     {
       args->rv = VNET_API_ERROR_SYSCALL_ERROR_1;
@@ -383,6 +381,8 @@ tap_create_if (vlib_main_t * vm, tap_create_if_args_t * args)
       goto error;
     }
 
+  hash_set (tm->dev_instance_by_interface_id, vif->id, vif->dev_instance);
+
   sw = vnet_get_hw_sw_interface (vnm, vif->hw_if_index);
   vif->sw_if_index = sw->sw_if_index;
   args->sw_if_index = vif->sw_if_index;

diff --git a/src/vnet/devices/tap/tapv2_api.c b/src/vnet/devices/tap/tapv2_api.c
index 34472c0..c6da0c9 100644 (file)
--- a/src/vnet/devices/tap/tapv2_api.c
+++ b/src/vnet/devices/tap/tapv2_api.c
@@ -59,7 +59,7 @@ vl_api_tap_create_v2_t_handler (vl_api_tap_create_v2_t * mp)
 
   memset (ap, 0, sizeof (*ap));
 
-  ap->id = mp->id;
+  ap->id = ntohl (mp->id);
   if (!mp->use_random_mac)
     {
       clib_memcpy (ap->mac_addr, mp->mac_address, 6);

diff --git a/src/vpp/api/custom_dump.c b/src/vpp/api/custom_dump.c
index 57c8bb3..28f16d2 100644 (file)
--- a/src/vpp/api/custom_dump.c
+++ b/src/vpp/api/custom_dump.c
@@ -558,7 +558,7 @@ static void *vl_api_tap_create_v2_t_print
   memset (null_mac, 0, sizeof (null_mac));
 
   s = format (0, "SCRIPT: tap_create_v2 ");
-  s = format (s, "id %s ", mp->id);
+  s = format (s, "id %u ", mp->id);
   if (memcmp (mp->mac_address, null_mac, 6))
     s = format (s, "mac-address %U ",
                format_ethernet_address, mp->mac_address);