From patchwork Fri Nov 3 14:36:41 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ashish Varma X-Patchwork-Id: 834061 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=openvswitch.org (client-ip=140.211.169.12; helo=mail.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="GtEJ5rS4"; dkim-atps=neutral Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3yTC3m3PFdz9s7p for ; Sat, 4 Nov 2017 06:40:36 +1100 (AEDT) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id D1622D39; Fri, 3 Nov 2017 19:40:34 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id 3BE61D38 for ; Fri, 3 Nov 2017 19:40:34 +0000 (UTC) X-Greylist: whitelisted by SQLgrey-1.7.6 Received: from mail-pf0-f193.google.com (mail-pf0-f193.google.com [209.85.192.193]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 2C598224 for ; Fri, 3 Nov 2017 19:40:33 +0000 (UTC) Received: by mail-pf0-f193.google.com with SMTP id d28so2915383pfe.2 for ; Fri, 03 Nov 2017 12:40:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:subject:date:message-id; bh=znA8CwRlpyzbpAWcyUIXW76ikrKvVlqikmb95gll2Tw=; b=GtEJ5rS4rvdtgwDCAHynRONYwE5H0E6remOtWc1xoQ23CUB1veno3xvP7CKrkYwxmC 5e5cbbHpMhj1HE4kgDAwylDTqFfG1beoSZiJx1++ReZ5WYvTZVfnVzk7+CllViyao+82 ztGHhQFfY9zF2G+0JmKzGiPSNVsaOGFVxwjs3+gpxqmSO7gESr5CKKITUDdVjzMDn7ed QF6H3vcBCmutaeGGFPLdxQnMKdPqP2oTFDQJn1zBj2H5ESkXlEOFea0oTunrJRibbj8w Iu2SolF3z3rLqxU/n0Vv0yOY2X4Fe02K5TIRkqifiLzakFx35hgSyqdqpuRmilHAVil9 V6lA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id; bh=znA8CwRlpyzbpAWcyUIXW76ikrKvVlqikmb95gll2Tw=; b=BbN5DqCY61FnIky2ZV1mwD5vVXBptHAHDZrRqpP4TTX9FiCvZqCTglJdIatwns46Cv emkMbBAg0N50X/FJOmGUVgDtmgqOR+j8WJrCn8/XTNbn4/4wqQ1t+EU6bj2qcAZzbjtr X/NYUnh2fUMiDZB38M51uprs8b9bd7E+ZVjjrVpMdUMmxjDaGHqBvF3P7Y2JogIVDsLb BYb363/InHEbPUGNjuwEjAzsfzno7EkWjkBoT94BWn0eUzBtMElm9TuazxkE9tZ4K9ab mxHKKFJoE7fVeIiVNK7EMpfi1ISkWiNA/L8fplMXbAZiHCn5ebFMVEuLrKQqhSKwKXx0 roDg== X-Gm-Message-State: AMCzsaVTSRWiMxtCrKEbf7sEvv067238x2ZB1XO8I/Eo7Iu9CX9NBXui 13Xb0B4FVdMJqXAOuVicP6QruA== X-Google-Smtp-Source: ABhQp+QxeKj7m0qwYJDebo6wM43RTiXFjWU6GWcdHFVP7ZAJ5m3Ug8XsPjMcl447fLDd6hgQ33If2A== X-Received: by 10.159.206.132 with SMTP id bg4mr7815686plb.129.1509738032299; Fri, 03 Nov 2017 12:40:32 -0700 (PDT) Received: from ashish-Ubuntu-WorkStation.eng.vmware.com ([208.91.1.34]) by smtp.gmail.com with ESMTPSA id a25sm3912574pgd.36.2017.11.03.12.40.30 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Fri, 03 Nov 2017 12:40:31 -0700 (PDT) From: Ashish Varma To: dev@openvswitch.org Date: Fri, 3 Nov 2017 07:36:41 -0700 Message-Id: <1509719801-18428-1-git-send-email-ashishvarma.ovs@gmail.com> X-Mailer: git-send-email 2.7.4 X-Spam-Status: No, score=1.5 required=5.0 tests=DATE_IN_PAST_03_06, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_SPAM autolearn=disabled version=3.3.1 X-Spam-Level: * X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Subject: [ovs-dev] [PATCH] netdev, dpif: fix the crash/assert on port delete X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org a crash is seen in "netdev_ports_remove" when an interface is deleted and added back in the system and when the interface is part of a bridge configuration. e.g. steps: create a tap0 interface using "ip tuntap add.." add the tap0 interface to br0 using "ovs-vsctl add-port.." delete the tap0 interface from system using "ip tuntap del.." add the tap0 interface back in system using "ip tuntap add.." (this changes the ifindex of the interface) delete tap0 from br0 using "ovs-vsctl del-port.." In the function "netdev_ports_insert", two hmap entries were created for mapping "portnum -> netdev" and "ifindex -> portnum". When the interface is deleted from the system, the "netdev_ports_remove" function is not getting called and the old ifindex entry is not getting cleaned up from the "ifindex_to_port" hmap. As part of the fix, added function "dpif_port_remove" which will call "netdev_ports_remove" in the path where the interface deletion from the system is detected. Also, in "netdev_ports_remove", added the code where the "ifindex_to_port_data" (ifindex -> portnum map node) is getting freed when the ifindex is not available any more. (as the interface is already deleted.) VMware-BZ: #1975788 Signed-off-by: Ashish Varma --- AUTHORS.rst | 1 + lib/dpif.c | 6 ++++++ lib/dpif.h | 1 + lib/netdev.c | 22 +++++++++++++++------- ofproto/ofproto-dpif.c | 6 ++++++ 5 files changed, 29 insertions(+), 7 deletions(-) diff --git a/AUTHORS.rst b/AUTHORS.rst index 3d61c04..7eecd54 100644 --- a/AUTHORS.rst +++ b/AUTHORS.rst @@ -62,6 +62,7 @@ Ariel Tubaltsev atubaltsev@vmware.com Arnoldo Lutz arnoldo.lutz.guevara@hpe.com Arun Sharma arun.sharma@calsoftinc.com Aryan TaheriMonfared aryan.taherimonfared@uis.no +Ashish Varma ashishvarma.ovs@gmail.com Ashwin Swaminathan ashwinds@arista.com Babu Shanmugam bschanmu@redhat.com Ben Pfaff blp@ovn.org diff --git a/lib/dpif.c b/lib/dpif.c index 3233bc8..6b2734b 100644 --- a/lib/dpif.c +++ b/lib/dpif.c @@ -623,6 +623,12 @@ dpif_port_del(struct dpif *dpif, odp_port_t port_no) return error; } +int +dpif_port_remove(struct dpif *dpif, odp_port_t port_no) +{ + return netdev_ports_remove(port_no, dpif->dpif_class); +} + /* Makes a deep copy of 'src' into 'dst'. */ void dpif_port_clone(struct dpif_port *dst, const struct dpif_port *src) diff --git a/lib/dpif.h b/lib/dpif.h index d9ded8b..c2d1c61 100644 --- a/lib/dpif.h +++ b/lib/dpif.h @@ -452,6 +452,7 @@ const char *dpif_port_open_type(const char *datapath_type, const char *port_type); int dpif_port_add(struct dpif *, struct netdev *, odp_port_t *port_nop); int dpif_port_del(struct dpif *, odp_port_t port_no); +int dpif_port_remove(struct dpif *, odp_port_t port_no); /* A port within a datapath. * diff --git a/lib/netdev.c b/lib/netdev.c index 704b38f..954da92 100644 --- a/lib/netdev.c +++ b/lib/netdev.c @@ -2179,6 +2179,7 @@ struct ifindex_to_port_data { struct hmap_node node; int ifindex; odp_port_t port; + const struct dpif_class *dpif_class; }; #define NETDEV_PORTS_HASH_INT(port, dpif) \ @@ -2228,6 +2229,7 @@ netdev_ports_insert(struct netdev *netdev, const struct dpif_class *dpif_class, ifidx = xzalloc(sizeof *ifidx); ifidx->ifindex = ifindex; ifidx->port = dpif_port->port_no; + ifidx->dpif_class = dpif_class; hmap_insert(&port_to_netdev, &data->node, hash); hmap_insert(&ifindex_to_port, &ifidx->node, ifidx->ifindex); @@ -2266,10 +2268,9 @@ netdev_ports_remove(odp_port_t port_no, const struct dpif_class *dpif_class) if (data) { int ifindex = netdev_get_ifindex(data->netdev); + struct ifindex_to_port_data *ifidx = NULL; if (ifindex > 0) { - struct ifindex_to_port_data *ifidx = NULL; - HMAP_FOR_EACH_WITH_HASH (ifidx, node, ifindex, &ifindex_to_port) { if (ifidx->port == port_no) { hmap_remove(&ifindex_to_port, &ifidx->node); @@ -2279,11 +2280,18 @@ netdev_ports_remove(odp_port_t port_no, const struct dpif_class *dpif_class) } ovs_assert(ifidx); } else { - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); - - VLOG_WARN_RL(&rl, "netdev ports map has dpif port %"PRIu32 - " but netdev has no ifindex: %s", port_no, - ovs_strerror(ifindex)); + /* case where the interface is already deleted form the datapath + * (e.g. tunctl -d or ip tuntap del), then the ifindex is not + * valid anymore. Traverse the HMAP for the port number. */ + HMAP_FOR_EACH (ifidx, node, &ifindex_to_port) { + if ((ifidx->port == port_no) && + (ifidx->dpif_class == dpif_class)) { + hmap_remove(&ifindex_to_port, &ifidx->node); + free(ifidx); + break; + } + } + ovs_assert(ifidx); } dpif_port_destroy(&data->dpif_port); diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index 4bc942d..779aef6 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -1923,6 +1923,12 @@ port_destruct(struct ofport *port_, bool del) if (!port->is_tunnel) { dpif_port_del(ofproto->backer->dpif, port->odp_port); } + } else if (del) { + /* The underlying device is already deleted (e.g. tunctl -d). + * Calling dpif_port_remove to do local cleanup for the netdev */ + if (!port->is_tunnel) { + dpif_port_remove(ofproto->backer->dpif, port->odp_port); + } } if (port->peer) {