From: Pravin B Shelar Date: Mon, 22 Dec 2014 12:53:02 +0000 (-0800) Subject: datapath: Simplify vport_send() error handling. X-Git-Tag: v2.4.0~754 X-Git-Url: http://git.cascardo.eti.br/?p=cascardo%2Fovs.git;a=commitdiff_plain;h=93258bd717fe8d08711bca6bfc12a7825287f213 datapath: Simplify vport_send() error handling. Today vport-send has complex error handling because it involves freeing skb and updating stats depending on return value from vport send implementation. This can be simplified by delegating responsibility of freeing skb to the vport implementation for all cases. So that vport-send needs just update stats. Signed-off-by: Pravin B Shelar Acked-by: Jesse Gross --- diff --git a/datapath/linux/compat/vxlan.c b/datapath/linux/compat/vxlan.c index b9fd8bad3..0389b28cb 100644 --- a/datapath/linux/compat/vxlan.c +++ b/datapath/linux/compat/vxlan.c @@ -187,8 +187,10 @@ int vxlan_xmit_skb(struct vxlan_sock *vs, /* Need space for new headers (invalidates iph ptr) */ err = skb_cow_head(skb, min_headroom); - if (unlikely(err)) + if (unlikely(err)) { + kfree_skb(skb); return err; + } if (vlan_tx_tag_present(skb)) { if (unlikely(!__vlan_put_tag(skb, @@ -219,7 +221,7 @@ int vxlan_xmit_skb(struct vxlan_sock *vs, skb = handle_offloads(skb); if (IS_ERR(skb)) - return 0; + return PTR_ERR(skb); return iptunnel_xmit(vs->sock->sk, rt, skb, src, dst, IPPROTO_UDP, tos, ttl, df, false); diff --git a/datapath/vport-geneve.c b/datapath/vport-geneve.c index a65dcb6b4..d54101e65 100644 --- a/datapath/vport-geneve.c +++ b/datapath/vport-geneve.c @@ -368,8 +368,10 @@ static int geneve_send(struct vport *vport, struct sk_buff *skb) int sent_len; int err; - if (unlikely(!OVS_CB(skb)->egress_tun_info)) - return -EINVAL; + if (unlikely(!OVS_CB(skb)->egress_tun_info)) { + err = -EINVAL; + goto error; + } tun_key = &OVS_CB(skb)->egress_tun_info->tunnel; @@ -406,6 +408,7 @@ static int geneve_send(struct vport *vport, struct sk_buff *skb) skb->vlan_proto, vlan_tx_tag_get(skb)))) { err = -ENOMEM; + skb = NULL; goto err_free_rt; } vlan_set_tci(skb, 0); @@ -422,7 +425,8 @@ static int geneve_send(struct vport *vport, struct sk_buff *skb) /* Offloading */ skb = handle_offloads(skb); if (IS_ERR(skb)) { - err = 0; + err = PTR_ERR(skb); + skb = NULL; goto err_free_rt; } @@ -439,6 +443,7 @@ static int geneve_send(struct vport *vport, struct sk_buff *skb) err_free_rt: ip_rt_put(rt); error: + kfree_skb(skb); return err; } diff --git a/datapath/vport-gre.c b/datapath/vport-gre.c index 41c025db4..daf7fc348 100644 --- a/datapath/vport-gre.c +++ b/datapath/vport-gre.c @@ -72,7 +72,7 @@ static struct sk_buff *__build_header(struct sk_buff *skb, tun_key = &OVS_CB(skb)->egress_tun_info->tunnel; skb = gre_handle_offloads(skb, !!(tun_key->tun_flags & TUNNEL_CSUM)); if (IS_ERR(skb)) - return NULL; + return skb; tpi.flags = filter_tnl_flags(tun_key->tun_flags) | gre64_flag; @@ -179,6 +179,7 @@ static int __send(struct vport *vport, struct sk_buff *skb, skb->vlan_proto, vlan_tx_tag_get(skb)))) { err = -ENOMEM; + skb = NULL; goto err_free_rt; } vlan_set_tci(skb, 0); @@ -186,8 +187,9 @@ static int __send(struct vport *vport, struct sk_buff *skb, /* Push Tunnel header. */ skb = __build_header(skb, tunnel_hlen, seq, gre64_flag); - if (unlikely(!skb)) { - err = 0; + if (IS_ERR(skb)) { + err = PTR_ERR(skb); + skb = NULL; goto err_free_rt; } @@ -201,6 +203,7 @@ static int __send(struct vport *vport, struct sk_buff *skb, err_free_rt: ip_rt_put(rt); error: + kfree_skb(skb); return err; } @@ -286,8 +289,10 @@ static int gre_send(struct vport *vport, struct sk_buff *skb) { int hlen; - if (unlikely(!OVS_CB(skb)->egress_tun_info)) + if (unlikely(!OVS_CB(skb)->egress_tun_info)) { + kfree_skb(skb); return -EINVAL; + } hlen = ip_gre_calc_hlen(OVS_CB(skb)->egress_tun_info->tunnel.tun_flags); @@ -370,8 +375,10 @@ static int gre64_send(struct vport *vport, struct sk_buff *skb) GRE_HEADER_SECTION; /* GRE SEQ */ __be32 seq; - if (unlikely(!OVS_CB(skb)->egress_tun_info)) + if (unlikely(!OVS_CB(skb)->egress_tun_info)) { + kfree_skb(skb); return -EINVAL; + } if (OVS_CB(skb)->egress_tun_info->tunnel.tun_flags & TUNNEL_CSUM) hlen += GRE_HEADER_SECTION; diff --git a/datapath/vport-lisp.c b/datapath/vport-lisp.c index 04f11ac43..db4d06fee 100644 --- a/datapath/vport-lisp.c +++ b/datapath/vport-lisp.c @@ -449,15 +449,17 @@ static int lisp_send(struct vport *vport, struct sk_buff *skb) int sent_len; int err; - if (unlikely(!OVS_CB(skb)->egress_tun_info)) - return -EINVAL; + if (unlikely(!OVS_CB(skb)->egress_tun_info)) { + err = -EINVAL; + goto error; + } tun_key = &OVS_CB(skb)->egress_tun_info->tunnel; if (skb->protocol != htons(ETH_P_IP) && skb->protocol != htons(ETH_P_IPV6)) { - kfree_skb(skb); - return 0; + err = 0; + goto error; } /* Route lookup */ @@ -500,7 +502,8 @@ static int lisp_send(struct vport *vport, struct sk_buff *skb) /* Offloading */ skb = handle_offloads(skb); if (IS_ERR(skb)) { - err = 0; + err = PTR_ERR(skb); + skb = NULL; goto err_free_rt; } @@ -517,6 +520,7 @@ static int lisp_send(struct vport *vport, struct sk_buff *skb) err_free_rt: ip_rt_put(rt); error: + kfree_skb(skb); return err; } diff --git a/datapath/vport-vxlan.c b/datapath/vport-vxlan.c index 8689853b8..81347f2f8 100644 --- a/datapath/vport-vxlan.c +++ b/datapath/vport-vxlan.c @@ -182,7 +182,9 @@ static int vxlan_tnl_send(struct vport *vport, struct sk_buff *skb) htonl(be64_to_cpu(tun_key->tun_id) << 8)); if (err < 0) ip_rt_put(rt); + return err; error: + kfree_skb(skb); return err; } diff --git a/datapath/vport.c b/datapath/vport.c index 274e47f28..e3f495efe 100644 --- a/datapath/vport.c +++ b/datapath/vport.c @@ -492,9 +492,9 @@ int ovs_vport_send(struct vport *vport, struct sk_buff *skb) u64_stats_update_end(&stats->syncp); } else if (sent < 0) { ovs_vport_record_error(vport, VPORT_E_TX_ERROR); - kfree_skb(skb); - } else + } else { ovs_vport_record_error(vport, VPORT_E_TX_DROPPED); + } return sent; }