From patchwork Tue Sep 13 18:04:23 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Xin Long X-Patchwork-Id: 669578 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3sYXdc2P7Mz9sRZ for ; Wed, 14 Sep 2016 04:05:08 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b=AtfxXG3l; dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1759884AbcIMSFC (ORCPT ); Tue, 13 Sep 2016 14:05:02 -0400 Received: from mail-pa0-f67.google.com ([209.85.220.67]:36542 "EHLO mail-pa0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755296AbcIMSEw (ORCPT ); Tue, 13 Sep 2016 14:04:52 -0400 Received: by mail-pa0-f67.google.com with SMTP id p2so3258138pap.3; Tue, 13 Sep 2016 11:04:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:cc:subject:date:message-id:in-reply-to:references :in-reply-to:references; bh=mhKoD019yboWe08ZM89tsFKJbyS8/42lFuG78/KAy3A=; b=AtfxXG3lTAN2KgGLGqB+3TYgR1bHq2jYcbpefx9zjLRPWG/SB/sIGhW7dumgWBD7YQ z4ngbNjSjEBAWXEIdxWZWofLWpdF2A+P2rG6zXhlVjpJ/S07968i3SBROyTWwwgqgn7Y 1P+j5tu1wr0XGULs6RKjTu5k9tcNy3ov9lCfpbfFm8Xp1HnJkdv0o6vJolgDRbX7pCf2 50fTBdb7N/HJAasqhF1O9A2AGvh/86uvGRVoWQC8SYrzEurXbS+pK8o6yP+viLlJVX9E RwVZa2aswozRzCZNDlMyITpTG+1HEc6E/OZ8SO9u6ADGufcd0nrB2k1U1UQI+z95f0EV DHIw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:in-reply-to:references; bh=mhKoD019yboWe08ZM89tsFKJbyS8/42lFuG78/KAy3A=; b=doEg8sRvmUCWnHCzokNbPqk2tJ0YZKkZ/cgmMZcqzFyz+AC8VnJHA5pcOkJMhuswGq ZIHl9+N3OR+OTiNkX9auW8J8LODKZX9xSol4FOOfaiiAN2v0RZhPE970w9ADNZMDZPwK VWqRV03sRVAVrJWS8hGKmSi3B8H+hL9xrcWfKosK7zxSifIm1gELHX5GQenM4eTeq506 KjzRNJ/mjAoysPE6y76Y7dZNLiBA3RX51FpAMllQzuPTSReQkYc1dDAcZN5w3b9sqT+U AIItSiuftBe9GktEZ+mLmskiGUtvlt9p2uaitPUf3Wb26c+oyVahpZXzApPX+slL6pyG VodA== X-Gm-Message-State: AE9vXwPblaI53QjLQghABNehCnjlYKIE/qAwfD8uy+L3R8Eq607hTGyxNBSXRJGOeDDiVQ== X-Received: by 10.66.79.138 with SMTP id j10mr3572714pax.60.1473789891765; Tue, 13 Sep 2016 11:04:51 -0700 (PDT) Received: from localhost (138.128.208.20.16clouds.com. [138.128.208.20]) by smtp.gmail.com with ESMTPSA id ci9sm27136563pad.34.2016.09.13.11.04.50 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 13 Sep 2016 11:04:51 -0700 (PDT) From: Xin Long To: network dev , linux-sctp@vger.kernel.org Cc: davem@davemloft.net, Marcelo Ricardo Leitner , Vlad Yasevich , daniel@iogearbox.net Subject: [PATCHv2 net 6/6] sctp: not return ENOMEM err back in sctp_packet_transmit Date: Wed, 14 Sep 2016 02:04:23 +0800 Message-Id: <099da3a4fed2e30a29326e9ffb2517858d29e288.1473789537.git.lucien.xin@gmail.com> X-Mailer: git-send-email 2.1.0 In-Reply-To: <6599cec72f3ff913d3b99e60d5f2014df26915de.1473789537.git.lucien.xin@gmail.com> References: <4f21a8880a3f8e7daed8a53f10649a1d4dfbb12a.1473789537.git.lucien.xin@gmail.com> <10622d6256e2c49fa7ec555dec3bd35b47a50b2b.1473789537.git.lucien.xin@gmail.com> <6b43f456f94dfcaeabc8071806f0609fed59a595.1473789537.git.lucien.xin@gmail.com> <6599cec72f3ff913d3b99e60d5f2014df26915de.1473789537.git.lucien.xin@gmail.com> In-Reply-To: References: Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org As David and Marcelo's suggestion, ENOMEM err shouldn't return back to user in transmit path. Instead, sctp's retransmit would take care of the chunks that fail to send because of ENOMEM. This patch is only to do some release job when alloc_skb fails, not to return ENOMEM back any more. Besides, it also cleans up sctp_packet_transmit's err path, and fixes some issues in err path: - It didn't free the head skb in nomem: path. - No need to check nskb in no_route: path. - It should goto err: path if alloc_skb fails for head. - Not all the NOMEMs should free nskb. Signed-off-by: Xin Long --- net/sctp/output.c | 47 ++++++++++++++++++++++------------------------- 1 file changed, 22 insertions(+), 25 deletions(-) diff --git a/net/sctp/output.c b/net/sctp/output.c index f2597a9..0c605ec 100644 --- a/net/sctp/output.c +++ b/net/sctp/output.c @@ -442,14 +442,14 @@ int sctp_packet_transmit(struct sctp_packet *packet, gfp_t gfp) * time. Application may notice this error. */ pr_err_once("Trying to GSO but underlying device doesn't support it."); - goto nomem; + goto err; } } else { pkt_size = packet->size; } head = alloc_skb(pkt_size + MAX_HEADER, gfp); if (!head) - goto nomem; + goto err; if (gso) { NAPI_GRO_CB(head)->last = head; skb_shinfo(head)->gso_type = sk->sk_gso_type; @@ -470,8 +470,12 @@ int sctp_packet_transmit(struct sctp_packet *packet, gfp_t gfp) } } dst = dst_clone(tp->dst); - if (!dst) - goto no_route; + if (!dst) { + if (asoc) + IP_INC_STATS(sock_net(asoc->base.sk), + IPSTATS_MIB_OUTNOROUTES); + goto nodst; + } skb_dst_set(head, dst); /* Build the SCTP header. */ @@ -622,8 +626,10 @@ int sctp_packet_transmit(struct sctp_packet *packet, gfp_t gfp) if (!gso) break; - if (skb_gro_receive(&head, nskb)) + if (skb_gro_receive(&head, nskb)) { + kfree_skb(nskb); goto nomem; + } nskb = NULL; if (WARN_ON_ONCE(skb_shinfo(head)->gso_segs >= sk->sk_gso_max_segs)) @@ -717,18 +723,13 @@ int sctp_packet_transmit(struct sctp_packet *packet, gfp_t gfp) } head->ignore_df = packet->ipfragok; tp->af_specific->sctp_xmit(head, tp); + goto out; -out: - sctp_packet_reset(packet); - return err; -no_route: - kfree_skb(head); - if (nskb != head) - kfree_skb(nskb); - - if (asoc) - IP_INC_STATS(sock_net(asoc->base.sk), IPSTATS_MIB_OUTNOROUTES); +nomem: + if (packet->auth && list_empty(&packet->auth->list)) + sctp_chunk_free(packet->auth); +nodst: /* FIXME: Returning the 'err' will effect all the associations * associated with a socket, although only one of the paths of the * association is unreachable. @@ -737,22 +738,18 @@ no_route: * required. */ /* err = -EHOSTUNREACH; */ -err: - /* Control chunks are unreliable so just drop them. DATA chunks - * will get resent or dropped later. - */ + kfree_skb(head); +err: list_for_each_entry_safe(chunk, tmp, &packet->chunk_list, list) { list_del_init(&chunk->list); if (!sctp_chunk_is_data(chunk)) sctp_chunk_free(chunk); } - goto out; -nomem: - if (packet->auth && list_empty(&packet->auth->list)) - sctp_chunk_free(packet->auth); - err = -ENOMEM; - goto err; + +out: + sctp_packet_reset(packet); + return err; } /********************************************************************