Commit 41001faf authored by Xin Long's avatar Xin Long Committed by David S. Miller

sctp: not return ENOMEM err back in sctp_packet_transmit

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: default avatarXin Long <lucien.xin@gmail.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 83dbc3d4
...@@ -442,14 +442,14 @@ int sctp_packet_transmit(struct sctp_packet *packet, gfp_t gfp) ...@@ -442,14 +442,14 @@ int sctp_packet_transmit(struct sctp_packet *packet, gfp_t gfp)
* time. Application may notice this error. * time. Application may notice this error.
*/ */
pr_err_once("Trying to GSO but underlying device doesn't support it."); pr_err_once("Trying to GSO but underlying device doesn't support it.");
goto nomem; goto err;
} }
} else { } else {
pkt_size = packet->size; pkt_size = packet->size;
} }
head = alloc_skb(pkt_size + MAX_HEADER, gfp); head = alloc_skb(pkt_size + MAX_HEADER, gfp);
if (!head) if (!head)
goto nomem; goto err;
if (gso) { if (gso) {
NAPI_GRO_CB(head)->last = head; NAPI_GRO_CB(head)->last = head;
skb_shinfo(head)->gso_type = sk->sk_gso_type; skb_shinfo(head)->gso_type = sk->sk_gso_type;
...@@ -470,8 +470,12 @@ int sctp_packet_transmit(struct sctp_packet *packet, gfp_t gfp) ...@@ -470,8 +470,12 @@ int sctp_packet_transmit(struct sctp_packet *packet, gfp_t gfp)
} }
} }
dst = dst_clone(tp->dst); dst = dst_clone(tp->dst);
if (!dst) if (!dst) {
goto no_route; if (asoc)
IP_INC_STATS(sock_net(asoc->base.sk),
IPSTATS_MIB_OUTNOROUTES);
goto nodst;
}
skb_dst_set(head, dst); skb_dst_set(head, dst);
/* Build the SCTP header. */ /* Build the SCTP header. */
...@@ -622,8 +626,10 @@ int sctp_packet_transmit(struct sctp_packet *packet, gfp_t gfp) ...@@ -622,8 +626,10 @@ int sctp_packet_transmit(struct sctp_packet *packet, gfp_t gfp)
if (!gso) if (!gso)
break; break;
if (skb_gro_receive(&head, nskb)) if (skb_gro_receive(&head, nskb)) {
kfree_skb(nskb);
goto nomem; goto nomem;
}
nskb = NULL; nskb = NULL;
if (WARN_ON_ONCE(skb_shinfo(head)->gso_segs >= if (WARN_ON_ONCE(skb_shinfo(head)->gso_segs >=
sk->sk_gso_max_segs)) sk->sk_gso_max_segs))
...@@ -717,18 +723,13 @@ int sctp_packet_transmit(struct sctp_packet *packet, gfp_t gfp) ...@@ -717,18 +723,13 @@ int sctp_packet_transmit(struct sctp_packet *packet, gfp_t gfp)
} }
head->ignore_df = packet->ipfragok; head->ignore_df = packet->ipfragok;
tp->af_specific->sctp_xmit(head, tp); tp->af_specific->sctp_xmit(head, tp);
goto out;
out: nomem:
sctp_packet_reset(packet); if (packet->auth && list_empty(&packet->auth->list))
return err; sctp_chunk_free(packet->auth);
no_route:
kfree_skb(head);
if (nskb != head)
kfree_skb(nskb);
if (asoc)
IP_INC_STATS(sock_net(asoc->base.sk), IPSTATS_MIB_OUTNOROUTES);
nodst:
/* FIXME: Returning the 'err' will effect all the associations /* FIXME: Returning the 'err' will effect all the associations
* associated with a socket, although only one of the paths of the * associated with a socket, although only one of the paths of the
* association is unreachable. * association is unreachable.
...@@ -737,22 +738,18 @@ int sctp_packet_transmit(struct sctp_packet *packet, gfp_t gfp) ...@@ -737,22 +738,18 @@ int sctp_packet_transmit(struct sctp_packet *packet, gfp_t gfp)
* required. * required.
*/ */
/* err = -EHOSTUNREACH; */ /* err = -EHOSTUNREACH; */
err: kfree_skb(head);
/* Control chunks are unreliable so just drop them. DATA chunks
* will get resent or dropped later.
*/
err:
list_for_each_entry_safe(chunk, tmp, &packet->chunk_list, list) { list_for_each_entry_safe(chunk, tmp, &packet->chunk_list, list) {
list_del_init(&chunk->list); list_del_init(&chunk->list);
if (!sctp_chunk_is_data(chunk)) if (!sctp_chunk_is_data(chunk))
sctp_chunk_free(chunk); sctp_chunk_free(chunk);
} }
goto out;
nomem: out:
if (packet->auth && list_empty(&packet->auth->list)) sctp_packet_reset(packet);
sctp_chunk_free(packet->auth); return err;
err = -ENOMEM;
goto err;
} }
/******************************************************************** /********************************************************************
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment