From 2e1c8967faf4e9f7b45471df02e4e5b07fbb520a Mon Sep 17 00:00:00 2001 From: Ole Troan Date: Wed, 10 Apr 2019 09:44:23 +0200 Subject: API: Fix shared memory only action handlers. Some API action handlers called vl_msg_ai_send_shmem() directly. That breaks Unix domain socket API transport. A couple (bond / vhost) also tried to send a sw_interface_event directly, but did not send the message to all that had registred interest. That scheme never worked correctly. Refactored and improved the interface event code. Change-Id: Idb90edfd8703c6ae593b36b4eeb4d3ed7da5c808 Signed-off-by: Ole Troan --- src/vnet/bonding/bond_api.c | 80 +++----------------- src/vnet/devices/tap/tapv2_api.c | 24 +----- src/vnet/devices/virtio/vhost_user_api.c | 22 ------ src/vnet/devices/virtio/virtio_api.c | 25 ------- src/vnet/interface_api.c | 121 ++++++++++++++++++------------- src/vnet/ip/ip_api.c | 9 +-- 6 files changed, 86 insertions(+), 195 deletions(-) (limited to 'src/vnet') diff --git a/src/vnet/bonding/bond_api.c b/src/vnet/bonding/bond_api.c index 50bae5d528d..07c2cbc5ae6 100644 --- a/src/vnet/bonding/bond_api.c +++ b/src/vnet/bonding/bond_api.c @@ -51,49 +51,17 @@ _(BOND_DETACH_SLAVE, bond_detach_slave) \ _(SW_INTERFACE_BOND_DUMP, sw_interface_bond_dump)\ _(SW_INTERFACE_SLAVE_DUMP, sw_interface_slave_dump) -static void -bond_send_sw_interface_event_deleted (vpe_api_main_t * am, - unix_shared_memory_queue_t * q, - u32 sw_if_index) -{ - vl_api_sw_interface_event_t *mp; - - mp = vl_msg_api_alloc (sizeof (*mp)); - clib_memset (mp, 0, sizeof (*mp)); - mp->_vl_msg_id = ntohs (VL_API_SW_INTERFACE_EVENT); - mp->sw_if_index = ntohl (sw_if_index); - - mp->admin_up_down = 0; - mp->link_up_down = 0; - mp->deleted = 1; - vl_msg_api_send_shmem (q, (u8 *) & mp); -} - static void vl_api_bond_delete_t_handler (vl_api_bond_delete_t * mp) { vlib_main_t *vm = vlib_get_main (); int rv; - vpe_api_main_t *vam = &vpe_api_main; vl_api_bond_delete_reply_t *rmp; - unix_shared_memory_queue_t *q; u32 sw_if_index = ntohl (mp->sw_if_index); rv = bond_delete_if (vm, sw_if_index); - q = vl_api_client_index_to_input_queue (mp->client_index); - if (!q) - return; - - rmp = vl_msg_api_alloc (sizeof (*rmp)); - rmp->_vl_msg_id = ntohs (VL_API_BOND_DELETE_REPLY); - rmp->context = mp->context; - rmp->retval = ntohl (rv); - - vl_msg_api_send_shmem (q, (u8 *) & rmp); - - if (!rv) - bond_send_sw_interface_event_deleted (vam, q, sw_if_index); + REPLY_MACRO (VL_API_BOND_DELETE_REPLY); } static void @@ -101,7 +69,6 @@ vl_api_bond_create_t_handler (vl_api_bond_create_t * mp) { vlib_main_t *vm = vlib_get_main (); vl_api_bond_create_reply_t *rmp; - unix_shared_memory_queue_t *q; bond_create_if_args_t _a, *ap = &_a; clib_memset (ap, 0, sizeof (*ap)); @@ -118,19 +85,14 @@ vl_api_bond_create_t_handler (vl_api_bond_create_t * mp) ap->lb = mp->lb; bond_create_if (vm, ap); - q = vl_api_client_index_to_input_queue (mp->client_index); - if (!q) - return; + int rv = ap->rv; - if (ap->rv != 0) - return; - rmp = vl_msg_api_alloc (sizeof (*rmp)); - rmp->_vl_msg_id = ntohs (VL_API_BOND_CREATE_REPLY); - rmp->context = mp->context; - rmp->retval = ntohl (ap->rv); - rmp->sw_if_index = ntohl (ap->sw_if_index); - - vl_msg_api_send_shmem (q, (u8 *) & rmp); + /* *INDENT-OFF* */ + REPLY_MACRO2(VL_API_BOND_CREATE_REPLY, + ({ + rmp->sw_if_index = ntohl (ap->sw_if_index); + })); + /* *INDENT-ON* */ } static void @@ -138,8 +100,8 @@ vl_api_bond_enslave_t_handler (vl_api_bond_enslave_t * mp) { vlib_main_t *vm = vlib_get_main (); vl_api_bond_enslave_reply_t *rmp; - unix_shared_memory_queue_t *q; bond_enslave_args_t _a, *ap = &_a; + int rv = 0; clib_memset (ap, 0, sizeof (*ap)); @@ -150,16 +112,7 @@ vl_api_bond_enslave_t_handler (vl_api_bond_enslave_t * mp) bond_enslave (vm, ap); - q = vl_api_client_index_to_input_queue (mp->client_index); - if (!q) - return; - - rmp = vl_msg_api_alloc (sizeof (*rmp)); - rmp->_vl_msg_id = ntohs (VL_API_BOND_ENSLAVE_REPLY); - rmp->context = mp->context; - rmp->retval = ntohl (ap->rv); - - vl_msg_api_send_shmem (q, (u8 *) & rmp); + REPLY_MACRO (VL_API_BOND_ENSLAVE_REPLY); } static void @@ -167,24 +120,15 @@ vl_api_bond_detach_slave_t_handler (vl_api_bond_detach_slave_t * mp) { vlib_main_t *vm = vlib_get_main (); vl_api_bond_detach_slave_reply_t *rmp; - unix_shared_memory_queue_t *q; bond_detach_slave_args_t _a, *ap = &_a; + int rv = 0; clib_memset (ap, 0, sizeof (*ap)); ap->slave = ntohl (mp->sw_if_index); bond_detach_slave (vm, ap); - q = vl_api_client_index_to_input_queue (mp->client_index); - if (!q) - return; - - rmp = vl_msg_api_alloc (sizeof (*rmp)); - rmp->_vl_msg_id = ntohs (VL_API_BOND_DETACH_SLAVE_REPLY); - rmp->context = mp->context; - rmp->retval = htonl (ap->rv); - - vl_msg_api_send_shmem (q, (u8 *) & rmp); + REPLY_MACRO (VL_API_BOND_DETACH_SLAVE_REPLY); } static void diff --git a/src/vnet/devices/tap/tapv2_api.c b/src/vnet/devices/tap/tapv2_api.c index 1260afdd24f..40ff22eaeaa 100644 --- a/src/vnet/devices/tap/tapv2_api.c +++ b/src/vnet/devices/tap/tapv2_api.c @@ -136,31 +136,12 @@ vl_api_tap_create_v2_t_handler (vl_api_tap_create_v2_t * mp) vl_api_send_msg (reg, (u8 *) rmp); } -static void -tap_send_sw_interface_event_deleted (vpe_api_main_t * am, - vl_api_registration_t * reg, - u32 sw_if_index) -{ - vl_api_sw_interface_event_t *mp; - - mp = vl_msg_api_alloc (sizeof (*mp)); - clib_memset (mp, 0, sizeof (*mp)); - mp->_vl_msg_id = ntohs (VL_API_SW_INTERFACE_EVENT); - mp->sw_if_index = ntohl (sw_if_index); - - mp->admin_up_down = 0; - mp->link_up_down = 0; - mp->deleted = 1; - vl_api_send_msg (reg, (u8 *) mp); -} - static void vl_api_tap_delete_v2_t_handler (vl_api_tap_delete_v2_t * mp) { vnet_main_t *vnm = vnet_get_main (); vlib_main_t *vm = vlib_get_main (); int rv; - vpe_api_main_t *vam = &vpe_api_main; vl_api_tap_delete_v2_reply_t *rmp; vl_api_registration_t *reg; u32 sw_if_index = ntohl (mp->sw_if_index); @@ -179,10 +160,7 @@ vl_api_tap_delete_v2_t_handler (vl_api_tap_delete_v2_t * mp) vl_api_send_msg (reg, (u8 *) rmp); if (!rv) - { - vnet_clear_sw_interface_tag (vnm, sw_if_index); - tap_send_sw_interface_event_deleted (vam, reg, sw_if_index); - } + vnet_clear_sw_interface_tag (vnm, sw_if_index); } static void diff --git a/src/vnet/devices/virtio/vhost_user_api.c b/src/vnet/devices/virtio/vhost_user_api.c index 8142cf352ed..4c765f30961 100644 --- a/src/vnet/devices/virtio/vhost_user_api.c +++ b/src/vnet/devices/virtio/vhost_user_api.c @@ -48,26 +48,6 @@ _(MODIFY_VHOST_USER_IF, modify_vhost_user_if) \ _(DELETE_VHOST_USER_IF, delete_vhost_user_if) \ _(SW_INTERFACE_VHOST_USER_DUMP, sw_interface_vhost_user_dump) -/* - * WARNING: replicated pending api refactor completion - */ -static void -send_sw_interface_event_deleted (vpe_api_main_t * am, - vl_api_registration_t * reg, u32 sw_if_index) -{ - vl_api_sw_interface_event_t *mp; - - mp = vl_msg_api_alloc (sizeof (*mp)); - clib_memset (mp, 0, sizeof (*mp)); - mp->_vl_msg_id = ntohs (VL_API_SW_INTERFACE_EVENT); - mp->sw_if_index = ntohl (sw_if_index); - - mp->admin_up_down = 0; - mp->link_up_down = 0; - mp->deleted = 1; - vl_api_send_msg (reg, (u8 *) mp); -} - static void vl_api_create_vhost_user_if_t_handler (vl_api_create_vhost_user_if_t * mp) { @@ -135,7 +115,6 @@ vl_api_delete_vhost_user_if_t_handler (vl_api_delete_vhost_user_if_t * mp) { int rv = 0; vl_api_delete_vhost_user_if_reply_t *rmp; - vpe_api_main_t *vam = &vpe_api_main; u32 sw_if_index = ntohl (mp->sw_if_index); vl_api_registration_t *reg; @@ -152,7 +131,6 @@ vl_api_delete_vhost_user_if_t_handler (vl_api_delete_vhost_user_if_t * mp) return; vnet_clear_sw_interface_tag (vnm, sw_if_index); - send_sw_interface_event_deleted (vam, reg, sw_if_index); } } diff --git a/src/vnet/devices/virtio/virtio_api.c b/src/vnet/devices/virtio/virtio_api.c index 238c6adfb43..ff123ae081c 100644 --- a/src/vnet/devices/virtio/virtio_api.c +++ b/src/vnet/devices/virtio/virtio_api.c @@ -85,24 +85,6 @@ vl_api_virtio_pci_create_t_handler (vl_api_virtio_pci_create_t * mp) vl_api_send_msg (reg, (u8 *) rmp); } -static void -virtio_pci_send_sw_interface_event_deleted (vpe_api_main_t * am, - vl_api_registration_t * reg, - u32 sw_if_index) -{ - vl_api_sw_interface_event_t *mp; - - mp = vl_msg_api_alloc (sizeof (*mp)); - clib_memset (mp, 0, sizeof (*mp)); - mp->_vl_msg_id = htons (VL_API_SW_INTERFACE_EVENT); - mp->sw_if_index = htonl (sw_if_index); - - mp->admin_up_down = 0; - mp->link_up_down = 0; - mp->deleted = 1; - vl_api_send_msg (reg, (u8 *) mp); -} - static void vl_api_virtio_pci_delete_t_handler (vl_api_virtio_pci_delete_t * mp) { @@ -112,10 +94,8 @@ vl_api_virtio_pci_delete_t_handler (vl_api_virtio_pci_delete_t * mp) int rv = 0; vnet_hw_interface_t *hw; virtio_if_t *vif; - vpe_api_main_t *vam = &vpe_api_main; vl_api_virtio_pci_delete_reply_t *rmp; vl_api_registration_t *reg; - u32 sw_if_index = ntohl (mp->sw_if_index); hw = vnet_get_sup_hw_interface (vnm, htonl (mp->sw_if_index)); if (hw == NULL || virtio_device_class.index != hw->dev_class_index) @@ -139,11 +119,6 @@ reply: rmp->retval = htonl (rv); vl_api_send_msg (reg, (u8 *) rmp); - - if (!rv) - { - virtio_pci_send_sw_interface_event_deleted (vam, reg, sw_if_index); - } } static void diff --git a/src/vnet/interface_api.c b/src/vnet/interface_api.c index bd5588e2625..f49af9473e5 100644 --- a/src/vnet/interface_api.c +++ b/src/vnet/interface_api.c @@ -694,38 +694,36 @@ vl_api_sw_interface_clear_stats_t_handler (vl_api_sw_interface_clear_stats_t * REPLY_MACRO (VL_API_SW_INTERFACE_CLEAR_STATS_REPLY); } -#define API_LINK_STATE_EVENT 1 -#define API_ADMIN_UP_DOWN_EVENT 2 - -static int -event_data_cmp (void *a1, void *a2) +/* + * Events used for sw_interface_events + */ +enum api_events { - uword *e1 = a1; - uword *e2 = a2; - - return (word) e1[0] - (word) e2[0]; -} + API_LINK_STATE_UP_EVENT = 1 << 1, + API_LINK_STATE_DOWN_EVENT = 1 << 2, + API_ADMIN_UP_EVENT = 1 << 3, + API_ADMIN_DOWN_EVENT = 1 << 4, + API_SW_INTERFACE_ADD_EVENT = 1 << 5, + API_SW_INTERFACE_DEL_EVENT = 1 << 6, +}; static void send_sw_interface_event (vpe_api_main_t * am, vpe_client_registration_t * reg, vl_api_registration_t * vl_reg, - vnet_sw_interface_t * swif) + u32 sw_if_index, enum api_events events) { vl_api_sw_interface_event_t *mp; - vnet_main_t *vnm = am->vnet_main; - vnet_hw_interface_t *hi = vnet_get_sup_hw_interface (vnm, - swif->sw_if_index); mp = vl_msg_api_alloc (sizeof (*mp)); clib_memset (mp, 0, sizeof (*mp)); mp->_vl_msg_id = ntohs (VL_API_SW_INTERFACE_EVENT); - mp->sw_if_index = ntohl (swif->sw_if_index); + mp->sw_if_index = ntohl (sw_if_index); mp->client_index = reg->client_index; mp->pid = reg->client_pid; - - mp->admin_up_down = (swif->flags & VNET_SW_INTERFACE_FLAG_ADMIN_UP) ? 1 : 0; - mp->link_up_down = (hi->flags & VNET_HW_INTERFACE_FLAG_LINK_UP) ? 1 : 0; + mp->admin_up_down = events & API_ADMIN_UP_EVENT ? 1 : 0; + mp->link_up_down = events & API_LINK_STATE_UP_EVENT ? 1 : 0; + mp->deleted = events & API_SW_INTERFACE_DEL_EVENT ? 1 : 0; vl_api_send_msg (vl_reg, (u8 *) mp); } @@ -734,13 +732,13 @@ link_state_process (vlib_main_t * vm, vlib_node_runtime_t * rt, vlib_frame_t * f) { vpe_api_main_t *vam = &vpe_api_main; - vnet_main_t *vnm = vam->vnet_main; - vnet_sw_interface_t *swif; - uword *event_data = 0; + uword *event_by_sw_if_index = 0; vpe_client_registration_t *reg; int i; - u32 prev_sw_if_index; vl_api_registration_t *vl_reg; + uword event_type; + uword *event_data = 0; + u32 sw_if_index; vam->link_state_process_up = 1; @@ -748,42 +746,33 @@ link_state_process (vlib_main_t * vm, { vlib_process_wait_for_event (vm); - /* Unified list of changed link or admin state sw_if_indices */ - vlib_process_get_events_with_type - (vm, &event_data, API_LINK_STATE_EVENT); - vlib_process_get_events_with_type - (vm, &event_data, API_ADMIN_UP_DOWN_EVENT); - - /* Sort, so we can eliminate duplicates */ - vec_sort_with_function (event_data, event_data_cmp); - - prev_sw_if_index = ~0; + /* Batch up events */ + while ((event_type = vlib_process_get_events (vm, &event_data)) != ~0) + { + for (i = 0; i < vec_len (event_data); i++) + { + sw_if_index = event_data[i]; + vec_validate_init_empty (event_by_sw_if_index, sw_if_index, 0); + event_by_sw_if_index[sw_if_index] |= event_type; + } + vec_reset_length (event_data); + } - for (i = 0; i < vec_len (event_data); i++) + for (i = 0; i < vec_len (event_by_sw_if_index); i++) { - /* Only one message per swif */ - if (prev_sw_if_index == event_data[i]) + if (event_by_sw_if_index[i] == 0) continue; - prev_sw_if_index = event_data[i]; /* *INDENT-OFF* */ pool_foreach(reg, vam->interface_events_registrations, ({ vl_reg = vl_api_client_index_to_registration (reg->client_index); if (vl_reg) - { - /* sw_interface may be deleted already */ - if (!pool_is_free_index (vnm->interface_main.sw_interfaces, - event_data[i])) - { - swif = vnet_get_sw_interface (vnm, event_data[i]); - send_sw_interface_event (vam, reg, vl_reg, swif); - } - } + send_sw_interface_event (vam, reg, vl_reg, i, event_by_sw_if_index[i]); })); /* *INDENT-ON* */ } - vec_reset_length (event_data); + vec_reset_length (event_by_sw_if_index); } return 0; @@ -793,6 +782,9 @@ static clib_error_t *link_up_down_function (vnet_main_t * vm, u32 hw_if_index, u32 flags); static clib_error_t *admin_up_down_function (vnet_main_t * vm, u32 hw_if_index, u32 flags); +static clib_error_t *sw_interface_add_del_function (vnet_main_t * vm, + u32 sw_if_index, + u32 flags); /* *INDENT-OFF* */ VLIB_REGISTER_NODE (link_state_process_node,static) = { @@ -804,6 +796,7 @@ VLIB_REGISTER_NODE (link_state_process_node,static) = { VNET_SW_INTERFACE_ADMIN_UP_DOWN_FUNCTION (admin_up_down_function); VNET_HW_INTERFACE_LINK_UP_DOWN_FUNCTION (link_up_down_function); +VNET_SW_INTERFACE_ADD_DEL_FUNCTION (sw_interface_add_del_function); static clib_error_t * link_up_down_function (vnet_main_t * vm, u32 hw_if_index, u32 flags) @@ -812,9 +805,13 @@ link_up_down_function (vnet_main_t * vm, u32 hw_if_index, u32 flags) vnet_hw_interface_t *hi = vnet_get_hw_interface (vm, hw_if_index); if (vam->link_state_process_up) - vlib_process_signal_event (vam->vlib_main, - link_state_process_node.index, - API_LINK_STATE_EVENT, hi->sw_if_index); + { + enum api_events event = + flags ? API_LINK_STATE_UP_EVENT : API_LINK_STATE_DOWN_EVENT; + vlib_process_signal_event (vam->vlib_main, + link_state_process_node.index, event, + hi->sw_if_index); + } return 0; } @@ -829,9 +826,29 @@ admin_up_down_function (vnet_main_t * vm, u32 sw_if_index, u32 flags) * routine. */ if (vam->link_state_process_up) - vlib_process_signal_event (vam->vlib_main, - link_state_process_node.index, - API_ADMIN_UP_DOWN_EVENT, sw_if_index); + { + enum api_events event = + flags ? API_ADMIN_UP_EVENT : API_ADMIN_DOWN_EVENT; + vlib_process_signal_event (vam->vlib_main, + link_state_process_node.index, event, + sw_if_index); + } + return 0; +} + +static clib_error_t * +sw_interface_add_del_function (vnet_main_t * vm, u32 sw_if_index, u32 flags) +{ + vpe_api_main_t *vam = &vpe_api_main; + + if (vam->link_state_process_up) + { + enum api_events event = + flags ? API_SW_INTERFACE_ADD_EVENT : API_SW_INTERFACE_DEL_EVENT; + vlib_process_signal_event (vam->vlib_main, + link_state_process_node.index, event, + sw_if_index); + } return 0; } diff --git a/src/vnet/ip/ip_api.c b/src/vnet/ip/ip_api.c index 9ecdfc8e244..ce3456d77d9 100644 --- a/src/vnet/ip/ip_api.c +++ b/src/vnet/ip/ip_api.c @@ -3343,11 +3343,10 @@ vl_api_ip_reassembly_set_t_handler (vl_api_ip_reassembly_set_t * mp) void vl_api_ip_reassembly_get_t_handler (vl_api_ip_reassembly_get_t * mp) { - unix_shared_memory_queue_t *q; + vl_api_registration_t *rp; - q = vl_api_client_index_to_input_queue (mp->client_index); - - if (q == 0) + rp = vl_api_client_index_to_registration (mp->client_index); + if (rp == 0) return; vl_api_ip_reassembly_get_reply_t *rmp = vl_msg_api_alloc (sizeof (*rmp)); @@ -3371,7 +3370,7 @@ vl_api_ip_reassembly_get_t_handler (vl_api_ip_reassembly_get_t * mp) rmp->max_reassemblies = clib_host_to_net_u32 (rmp->max_reassemblies); rmp->expire_walk_interval_ms = clib_host_to_net_u32 (rmp->expire_walk_interval_ms); - vl_msg_api_send_shmem (q, (u8 *) & rmp); + vl_api_send_msg (rp, (u8 *) rmp); } void -- cgit 1.2.3-korg