From 36ce148c6d2105dc2e485ec169bcc9998ffebf9d Mon Sep 17 00:00:00 2001 From: Pravin Shelar Date: Thu, 29 Sep 2011 16:33:06 -0700 Subject: [PATCH] datapath: Avoid skb-clone in upcall There is not need to clone skb while sending packet to user-space. Since data is only read from packet skb. Signed-off-by: Pravin Shelar Acked-by: Jesse Gross --- datapath/actions.c | 4 --- datapath/datapath.c | 70 +++++++++++++++++++-------------------------- 2 files changed, 30 insertions(+), 44 deletions(-) diff --git a/datapath/actions.c b/datapath/actions.c index cba12e66c..36437a4ba 100644 --- a/datapath/actions.c +++ b/datapath/actions.c @@ -248,10 +248,6 @@ static int output_userspace(struct datapath *dp, struct sk_buff *skb, u64 arg) { struct dp_upcall_info upcall; - skb = skb_clone(skb, GFP_ATOMIC); - if (!skb) - return -ENOMEM; - upcall.cmd = OVS_PACKET_CMD_ACTION; upcall.key = &OVS_CB(skb)->flow->key; upcall.userdata = arg; diff --git a/datapath/datapath.c b/datapath/datapath.c index 63a393221..15c1e33b2 100644 --- a/datapath/datapath.c +++ b/datapath/datapath.c @@ -311,6 +311,7 @@ void dp_process_received_packet(struct vport *p, struct sk_buff *skb) upcall.cmd = OVS_PACKET_CMD_MISS; upcall.key = &key; dp_upcall(dp, skb, &upcall); + kfree_skb(skb); stats_counter_off = offsetof(struct dp_stats_percpu, n_missed); goto out; } @@ -357,8 +358,10 @@ static struct genl_family dp_packet_genl_family = { .maxattr = OVS_PACKET_ATTR_MAX }; -int dp_upcall(struct datapath *dp, struct sk_buff *skb, const struct dp_upcall_info *upcall_info) +int dp_upcall(struct datapath *dp, struct sk_buff *skb, + const struct dp_upcall_info *upcall_info) { + struct sk_buff *segs = NULL; struct dp_stats_percpu *stats; u32 pid; int err; @@ -370,7 +373,6 @@ int dp_upcall(struct datapath *dp, struct sk_buff *skb, const struct dp_upcall_i if (pid == 0) { err = -ENOTCONN; - kfree_skb(skb); goto err; } @@ -379,18 +381,25 @@ int dp_upcall(struct datapath *dp, struct sk_buff *skb, const struct dp_upcall_i /* Break apart GSO packets into their component pieces. Otherwise * userspace may try to stuff a 64kB packet into a 1500-byte MTU. */ if (skb_is_gso(skb)) { - struct sk_buff *nskb = skb_gso_segment(skb, NETIF_F_SG | NETIF_F_HW_CSUM); + segs = skb_gso_segment(skb, NETIF_F_SG | NETIF_F_HW_CSUM); - if (IS_ERR(nskb)) { - kfree_skb(skb); - err = PTR_ERR(nskb); + if (IS_ERR(segs)) { + err = PTR_ERR(segs); goto err; } - consume_skb(skb); - skb = nskb; + skb = segs; } err = queue_userspace_packets(dp, pid, skb, upcall_info); + if (segs) { + struct sk_buff *next; + /* Free GSO-segments */ + do { + next = segs->next; + kfree_skb(segs); + } while ((segs = next) != NULL); + } + if (err) goto err; @@ -418,33 +427,24 @@ static int queue_userspace_packets(struct datapath *dp, u32 pid, const struct dp_upcall_info *upcall_info) { int dp_ifindex; - struct sk_buff *nskb; - int err; dp_ifindex = get_dpifindex(dp); - if (!dp_ifindex) { - err = -ENODEV; - nskb = skb->next; - goto err_kfree_skbs; - } + if (!dp_ifindex) + return -ENODEV; do { struct ovs_header *upcall; struct sk_buff *user_skb; /* to be queued to userspace */ struct nlattr *nla; unsigned int len; - - nskb = skb->next; - skb->next = NULL; + int err; err = vlan_deaccel_tag(skb); if (unlikely(err)) - goto err_kfree_skbs; + return err; - if (nla_attr_size(skb->len) > USHRT_MAX) { - err = -EFBIG; - goto err_kfree_skbs; - } + if (nla_attr_size(skb->len) > USHRT_MAX) + return -EFBIG; len = sizeof(struct ovs_header); len += nla_total_size(skb->len); @@ -453,12 +453,11 @@ static int queue_userspace_packets(struct datapath *dp, u32 pid, len += nla_total_size(8); user_skb = genlmsg_new(len, GFP_ATOMIC); - if (!user_skb) { - err = -ENOMEM; - goto err_kfree_skbs; - } + if (!user_skb) + return -ENOMEM; - upcall = genlmsg_put(user_skb, 0, 0, &dp_packet_genl_family, 0, upcall_info->cmd); + upcall = genlmsg_put(user_skb, 0, 0, &dp_packet_genl_family, + 0, upcall_info->cmd); upcall->dp_ifindex = dp_ifindex; nla = nla_nest_start(user_skb, OVS_PACKET_ATTR_KEY); @@ -477,20 +476,11 @@ static int queue_userspace_packets(struct datapath *dp, u32 pid, err = genlmsg_unicast(&init_net, user_skb, pid); if (err) - goto err_kfree_skbs; + return err; - consume_skb(skb); - skb = nskb; - } while (skb); - return 0; + } while ((skb = skb->next)); -err_kfree_skbs: - kfree_skb(skb); - while ((skb = nskb) != NULL) { - nskb = skb->next; - kfree_skb(skb); - } - return err; + return 0; } /* Called with genl_mutex. */ -- 2.43.0