[ovs-dev] [PATCH 2/2] datapath: Simplify vport_send() error handling.
Pravin B Shelar
pshelar at nicira.com
Sat Dec 20 00:25:02 UTC 2014
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 <pshelar at nicira.com>
---
datapath/linux/compat/vxlan.c | 4 +++-
datapath/vport-geneve.c | 24 ++++++++++++++++++------
datapath/vport-gre.c | 16 +++++++++++-----
datapath/vport-lisp.c | 26 ++++++++++++++++++--------
datapath/vport-vxlan.c | 2 ++
datapath/vport.c | 4 ++--
6 files changed, 54 insertions(+), 22 deletions(-)
diff --git a/datapath/linux/compat/vxlan.c b/datapath/linux/compat/vxlan.c
index 76ae552..0389b28 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,
diff --git a/datapath/vport-geneve.c b/datapath/vport-geneve.c
index 6fcf1cc..6601e9d 100644
--- a/datapath/vport-geneve.c
+++ b/datapath/vport-geneve.c
@@ -329,17 +329,21 @@ static struct sk_buff *handle_offloads(struct sk_buff *skb)
return ovs_iptunnel_handle_offloads(skb, false, geneve_fix_segment);
}
#else
+
static struct sk_buff *handle_offloads(struct sk_buff *skb)
{
+ int err = 0;
+
if (skb_is_gso(skb)) {
- int err;
- if (skb_is_encapsulated(skb))
- return ERR_PTR(-ENOSYS);
+ if (skb_is_encapsulated(skb)) {
+ err = -ENOSYS;
+ goto error;
+ }
err = skb_unclone(skb, GFP_ATOMIC);
if (unlikely(err))
- return ERR_PTR(err);
+ goto error;
skb_shinfo(skb)->gso_type |= SKB_GSO_UDP_TUNNEL;
} else if (skb->ip_summed != CHECKSUM_PARTIAL)
@@ -347,6 +351,9 @@ static struct sk_buff *handle_offloads(struct sk_buff *skb)
skb->encapsulation = 1;
return skb;
+error:
+ kfree_skb(skb);
+ return ERR_PTR(err);
}
#endif
@@ -361,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;
@@ -398,6 +407,7 @@ static int geneve_send(struct vport *vport, struct sk_buff *skb)
if (unlikely(!__vlan_put_tag(skb,
skb->vlan_proto,
vlan_tx_tag_get(skb)))) {
+ skb = NULL;
err = -ENOMEM;
goto err_free_rt;
}
@@ -416,6 +426,7 @@ static int geneve_send(struct vport *vport, struct sk_buff *skb)
skb = handle_offloads(skb);
if (IS_ERR(skb)) {
err = PTR_ERR(skb);
+ skb = NULL;
goto err_free_rt;
}
@@ -432,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 41c025d..787618b 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,8 @@ 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 (unlikely(IS_ERR(skb))) {
+ err = PTR_ERR(skb);
goto err_free_rt;
}
@@ -201,6 +202,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 +288,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 +374,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 ce30f69..db4d06f 100644
--- a/datapath/vport-lisp.c
+++ b/datapath/vport-lisp.c
@@ -413,15 +413,18 @@ static struct sk_buff *handle_offloads(struct sk_buff *skb)
#else
static struct sk_buff *handle_offloads(struct sk_buff *skb)
{
+ int err = 0;
+
if (skb_is_gso(skb)) {
- int err;
- if (skb_is_encapsulated(skb))
- return ERR_PTR(-ENOSYS);
+ if (skb_is_encapsulated(skb)) {
+ err = -ENOSYS;
+ goto error;
+ }
err = skb_unclone(skb, GFP_ATOMIC);
if (unlikely(err))
- return ERR_PTR(err);
+ goto error;
skb_shinfo(skb)->gso_type |= SKB_GSO_UDP_TUNNEL;
} else if (skb->ip_summed != CHECKSUM_PARTIAL)
@@ -429,6 +432,9 @@ static struct sk_buff *handle_offloads(struct sk_buff *skb)
skb->encapsulation = 1;
return skb;
+error:
+ kfree_skb(skb);
+ return ERR_PTR(err);
}
#endif
@@ -443,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 */
@@ -495,6 +503,7 @@ static int lisp_send(struct vport *vport, struct sk_buff *skb)
skb = handle_offloads(skb);
if (IS_ERR(skb)) {
err = PTR_ERR(skb);
+ skb = NULL;
goto err_free_rt;
}
@@ -511,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 8689853..81347f2 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 274e47f..e3f495e 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;
}
--
1.7.1
More information about the dev
mailing list