virtio: fix the interrupt 67/32167/8
authorMohsin Kazmi <sykazmi@cisco.com>
Wed, 28 Apr 2021 16:55:45 +0000 (18:55 +0200)
committerBeno�t Ganne <bganne@cisco.com>
Tue, 4 May 2021 16:48:11 +0000 (16:48 +0000)
Type: fix

virtio/tap interfaces set the empty buffers in the input node
for receiving data. Backend uses those buffers, fills them with
data and notifies the virtio/tap driver. But virtio/tap driver
gets into stall state if interface is created and configured
through exec script on VPP startup.conf and put the interface in
interrupt mode while VPP is only configured with main thread.

This patch fixes the problem by prefilling buffers during the
interface creation.

Change-Id: Ibc4d0e70e127ccc4b7cf8b2b18406ae4b02c73b4
Signed-off-by: Mohsin Kazmi <sykazmi@cisco.com>
src/vnet/devices/virtio/node.c
src/vnet/devices/virtio/virtio.c
src/vnet/devices/virtio/virtio_inline.h [new file with mode: 0644]

index df8c0fa..f91dc87 100644 (file)
 #include <vnet/ip/ip6_packet.h>
 #include <vnet/udp/udp_packet.h>
 #include <vnet/devices/virtio/virtio.h>
-
-#define foreach_virtio_input_error \
-  _(BUFFER_ALLOC, "buffer alloc error") \
-  _(UNKNOWN, "unknown")
-
-typedef enum
-{
-#define _(f,s) VIRTIO_INPUT_ERROR_##f,
-  foreach_virtio_input_error
-#undef _
-    VIRTIO_INPUT_N_ERROR,
-} virtio_input_error_t;
+#include <vnet/devices/virtio/virtio_inline.h>
 
 static char *virtio_input_error_strings[] = {
-#define _(n,s) s,
+#define _(n, s) s,
   foreach_virtio_input_error
 #undef _
 };
@@ -79,155 +68,6 @@ format_virtio_input_trace (u8 * s, va_list * args)
   return s;
 }
 
-static_always_inline void
-virtio_refill_vring_split (vlib_main_t * vm, virtio_if_t * vif,
-                          virtio_if_type_t type, virtio_vring_t * vring,
-                          const int hdr_sz, u32 node_index)
-{
-  u16 used, next, avail, n_slots, n_refill;
-  u16 sz = vring->size;
-  u16 mask = sz - 1;
-
-more:
-  used = vring->desc_in_use;
-
-  if (sz - used < sz / 8)
-    return;
-
-  /* deliver free buffers in chunks of 64 */
-  n_refill = clib_min (sz - used, 64);
-
-  next = vring->desc_next;
-  avail = vring->avail->idx;
-  n_slots =
-    vlib_buffer_alloc_to_ring_from_pool (vm, vring->buffers, next,
-                                        vring->size, n_refill,
-                                        vring->buffer_pool_index);
-
-  if (PREDICT_FALSE (n_slots != n_refill))
-    {
-      vlib_error_count (vm, node_index,
-                       VIRTIO_INPUT_ERROR_BUFFER_ALLOC, n_refill - n_slots);
-      if (n_slots == 0)
-       return;
-    }
-
-  while (n_slots)
-    {
-      vring_desc_t *d = &vring->desc[next];;
-      vlib_buffer_t *b = vlib_get_buffer (vm, vring->buffers[next]);
-      /*
-       * current_data may not be initialized with 0 and may contain
-       * previous offset. Here we want to make sure, it should be 0
-       * initialized.
-       */
-      b->current_data = -hdr_sz;
-      clib_memset (vlib_buffer_get_current (b), 0, hdr_sz);
-      d->addr =
-       ((type == VIRTIO_IF_TYPE_PCI) ? vlib_buffer_get_current_pa (vm,
-                                                                   b) :
-        pointer_to_uword (vlib_buffer_get_current (b)));
-      d->len = vlib_buffer_get_default_data_size (vm) + hdr_sz;
-      d->flags = VRING_DESC_F_WRITE;
-      vring->avail->ring[avail & mask] = next;
-      avail++;
-      next = (next + 1) & mask;
-      n_slots--;
-      used++;
-    }
-  clib_atomic_store_seq_cst (&vring->avail->idx, avail);
-  vring->desc_next = next;
-  vring->desc_in_use = used;
-  if ((clib_atomic_load_seq_cst (&vring->used->flags) &
-       VRING_USED_F_NO_NOTIFY) == 0)
-    {
-      virtio_kick (vm, vring, vif);
-    }
-  goto more;
-}
-
-static_always_inline void
-virtio_refill_vring_packed (vlib_main_t * vm, virtio_if_t * vif,
-                           virtio_if_type_t type, virtio_vring_t * vring,
-                           const int hdr_sz, u32 node_index)
-{
-  u16 used, next, n_slots, n_refill, flags = 0, first_desc_flags;
-  u16 sz = vring->size;
-
-more:
-  used = vring->desc_in_use;
-
-  if (sz == used)
-    return;
-
-  /* deliver free buffers in chunks of 64 */
-  n_refill = clib_min (sz - used, 64);
-
-  next = vring->desc_next;
-  first_desc_flags = vring->packed_desc[next].flags;
-  n_slots =
-    vlib_buffer_alloc_to_ring_from_pool (vm, vring->buffers, next,
-                                        sz, n_refill,
-                                        vring->buffer_pool_index);
-
-  if (PREDICT_FALSE (n_slots != n_refill))
-    {
-      vlib_error_count (vm, node_index,
-                       VIRTIO_INPUT_ERROR_BUFFER_ALLOC, n_refill - n_slots);
-      if (n_slots == 0)
-       return;
-    }
-
-  while (n_slots)
-    {
-      vring_packed_desc_t *d = &vring->packed_desc[next];
-      vlib_buffer_t *b = vlib_get_buffer (vm, vring->buffers[next]);
-      /*
-       * current_data may not be initialized with 0 and may contain
-       * previous offset. Here we want to make sure, it should be 0
-       * initialized.
-       */
-      b->current_data = -hdr_sz;
-      clib_memset (vlib_buffer_get_current (b), 0, hdr_sz);
-      d->addr =
-       ((type == VIRTIO_IF_TYPE_PCI) ? vlib_buffer_get_current_pa (vm,
-                                                                   b) :
-        pointer_to_uword (vlib_buffer_get_current (b)));
-      d->len = vlib_buffer_get_default_data_size (vm) + hdr_sz;
-
-      if (vring->avail_wrap_counter)
-       flags = (VRING_DESC_F_AVAIL | VRING_DESC_F_WRITE);
-      else
-       flags = (VRING_DESC_F_USED | VRING_DESC_F_WRITE);
-
-      d->id = next;
-      if (vring->desc_next == next)
-       first_desc_flags = flags;
-      else
-       d->flags = flags;
-
-      next++;
-      if (next >= sz)
-       {
-         next = 0;
-         vring->avail_wrap_counter ^= 1;
-       }
-      n_slots--;
-      used++;
-    }
-  CLIB_MEMORY_STORE_BARRIER ();
-  vring->packed_desc[vring->desc_next].flags = first_desc_flags;
-  vring->desc_next = next;
-  vring->desc_in_use = used;
-  CLIB_MEMORY_BARRIER ();
-  if (vring->device_event->flags != VRING_EVENT_F_DISABLE)
-    {
-      virtio_kick (vm, vring, vif);
-    }
-
-  goto more;
-}
-
 static_always_inline void
 virtio_needs_csum (vlib_buffer_t * b0, virtio_net_hdr_v1_t * hdr,
                   u8 * l4_proto, u8 * l4_hdr_sz, virtio_if_type_t type)
index 4f812b1..b8054d1 100644 (file)
@@ -30,6 +30,7 @@
 #include <vnet/ip/ip4_packet.h>
 #include <vnet/ip/ip6_packet.h>
 #include <vnet/devices/virtio/virtio.h>
+#include <vnet/devices/virtio/virtio_inline.h>
 #include <vnet/devices/virtio/pci.h>
 #include <vnet/interface/rx_queue_funcs.h>
 
@@ -220,6 +221,19 @@ virtio_set_packet_buffering (virtio_if_t * vif, u16 buffering_size)
   return error;
 }
 
+static void
+virtio_vring_fill (vlib_main_t *vm, virtio_if_t *vif, virtio_vring_t *vring)
+{
+  if (vif->is_packed)
+    virtio_refill_vring_packed (vm, vif, vif->type, vring,
+                               vif->virtio_net_hdr_sz,
+                               virtio_input_node.index);
+  else
+    virtio_refill_vring_split (vm, vif, vif->type, vring,
+                              vif->virtio_net_hdr_sz,
+                              virtio_input_node.index);
+}
+
 void
 virtio_vring_set_rx_queues (vlib_main_t *vm, virtio_if_t *vif)
 {
@@ -262,6 +276,10 @@ virtio_vring_set_rx_queues (vlib_main_t *vm, virtio_if_t *vif)
                                              file_index);
          i++;
        }
+      vnet_hw_if_set_rx_queue_mode (vnm, vring->queue_index,
+                                   VNET_HW_IF_RX_MODE_POLLING);
+      vring->mode = VNET_HW_IF_RX_MODE_POLLING;
+      virtio_vring_fill (vm, vif, vring);
     }
   vnet_hw_if_update_runtime_data (vnm, vif->hw_if_index);
 }
diff --git a/src/vnet/devices/virtio/virtio_inline.h b/src/vnet/devices/virtio/virtio_inline.h
new file mode 100644 (file)
index 0000000..209817d
--- /dev/null
@@ -0,0 +1,182 @@
+/*
+ * Copyright (c) 2015 Cisco and/or its affiliates.
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#ifndef __VIRTIO_INLINE_H__
+#define __VIRTIO_INLINE_H__
+
+#define foreach_virtio_input_error                                            \
+  _ (BUFFER_ALLOC, "buffer alloc error")                                      \
+  _ (UNKNOWN, "unknown")
+
+typedef enum
+{
+#define _(f, s) VIRTIO_INPUT_ERROR_##f,
+  foreach_virtio_input_error
+#undef _
+    VIRTIO_INPUT_N_ERROR,
+} virtio_input_error_t;
+
+static_always_inline void
+virtio_refill_vring_split (vlib_main_t *vm, virtio_if_t *vif,
+                          virtio_if_type_t type, virtio_vring_t *vring,
+                          const int hdr_sz, u32 node_index)
+{
+  u16 used, next, avail, n_slots, n_refill;
+  u16 sz = vring->size;
+  u16 mask = sz - 1;
+
+more:
+  used = vring->desc_in_use;
+
+  if (sz - used < sz / 8)
+    return;
+
+  /* deliver free buffers in chunks of 64 */
+  n_refill = clib_min (sz - used, 64);
+
+  next = vring->desc_next;
+  avail = vring->avail->idx;
+  n_slots = vlib_buffer_alloc_to_ring_from_pool (
+    vm, vring->buffers, next, vring->size, n_refill, vring->buffer_pool_index);
+
+  if (PREDICT_FALSE (n_slots != n_refill))
+    {
+      vlib_error_count (vm, node_index, VIRTIO_INPUT_ERROR_BUFFER_ALLOC,
+                       n_refill - n_slots);
+      if (n_slots == 0)
+       return;
+    }
+
+  while (n_slots)
+    {
+      vring_desc_t *d = &vring->desc[next];
+      ;
+      vlib_buffer_t *b = vlib_get_buffer (vm, vring->buffers[next]);
+      /*
+       * current_data may not be initialized with 0 and may contain
+       * previous offset. Here we want to make sure, it should be 0
+       * initialized.
+       */
+      b->current_data = -hdr_sz;
+      clib_memset (vlib_buffer_get_current (b), 0, hdr_sz);
+      d->addr = ((type == VIRTIO_IF_TYPE_PCI) ?
+                  vlib_buffer_get_current_pa (vm, b) :
+                  pointer_to_uword (vlib_buffer_get_current (b)));
+      d->len = vlib_buffer_get_default_data_size (vm) + hdr_sz;
+      d->flags = VRING_DESC_F_WRITE;
+      vring->avail->ring[avail & mask] = next;
+      avail++;
+      next = (next + 1) & mask;
+      n_slots--;
+      used++;
+    }
+  clib_atomic_store_seq_cst (&vring->avail->idx, avail);
+  vring->desc_next = next;
+  vring->desc_in_use = used;
+  if ((clib_atomic_load_seq_cst (&vring->used->flags) &
+       VRING_USED_F_NO_NOTIFY) == 0)
+    {
+      virtio_kick (vm, vring, vif);
+    }
+  goto more;
+}
+
+static_always_inline void
+virtio_refill_vring_packed (vlib_main_t *vm, virtio_if_t *vif,
+                           virtio_if_type_t type, virtio_vring_t *vring,
+                           const int hdr_sz, u32 node_index)
+{
+  u16 used, next, n_slots, n_refill, flags = 0, first_desc_flags;
+  u16 sz = vring->size;
+
+more:
+  used = vring->desc_in_use;
+
+  if (sz == used)
+    return;
+
+  /* deliver free buffers in chunks of 64 */
+  n_refill = clib_min (sz - used, 64);
+
+  next = vring->desc_next;
+  first_desc_flags = vring->packed_desc[next].flags;
+  n_slots = vlib_buffer_alloc_to_ring_from_pool (
+    vm, vring->buffers, next, sz, n_refill, vring->buffer_pool_index);
+
+  if (PREDICT_FALSE (n_slots != n_refill))
+    {
+      vlib_error_count (vm, node_index, VIRTIO_INPUT_ERROR_BUFFER_ALLOC,
+                       n_refill - n_slots);
+      if (n_slots == 0)
+       return;
+    }
+
+  while (n_slots)
+    {
+      vring_packed_desc_t *d = &vring->packed_desc[next];
+      vlib_buffer_t *b = vlib_get_buffer (vm, vring->buffers[next]);
+      /*
+       * current_data may not be initialized with 0 and may contain
+       * previous offset. Here we want to make sure, it should be 0
+       * initialized.
+       */
+      b->current_data = -hdr_sz;
+      clib_memset (vlib_buffer_get_current (b), 0, hdr_sz);
+      d->addr = ((type == VIRTIO_IF_TYPE_PCI) ?
+                  vlib_buffer_get_current_pa (vm, b) :
+                  pointer_to_uword (vlib_buffer_get_current (b)));
+      d->len = vlib_buffer_get_default_data_size (vm) + hdr_sz;
+
+      if (vring->avail_wrap_counter)
+       flags = (VRING_DESC_F_AVAIL | VRING_DESC_F_WRITE);
+      else
+       flags = (VRING_DESC_F_USED | VRING_DESC_F_WRITE);
+
+      d->id = next;
+      if (vring->desc_next == next)
+       first_desc_flags = flags;
+      else
+       d->flags = flags;
+
+      next++;
+      if (next >= sz)
+       {
+         next = 0;
+         vring->avail_wrap_counter ^= 1;
+       }
+      n_slots--;
+      used++;
+    }
+  CLIB_MEMORY_STORE_BARRIER ();
+  vring->packed_desc[vring->desc_next].flags = first_desc_flags;
+  vring->desc_next = next;
+  vring->desc_in_use = used;
+  CLIB_MEMORY_BARRIER ();
+  if (vring->device_event->flags != VRING_EVENT_F_DISABLE)
+    {
+      virtio_kick (vm, vring, vif);
+    }
+
+  goto more;
+}
+
+#endif
+
+/*
+ * fd.io coding-style-patch-verification: ON
+ *
+ * Local Variables:
+ * eval: (c-set-style "gnu")
+ * End:
+ */