datapath/flow: Fix ovs_flow_stats_get/clear RCU dereference.
[sliver-openvswitch.git] / datapath / datapath.c
index f045fe4..0c77045 100644 (file)
 
 int ovs_net_id __read_mostly;
 
+/* Check if need to build a reply message.
+ * OVS userspace sets the NLM_F_ECHO flag if it needs the reply. */
+static bool ovs_must_notify(struct genl_info *info,
+                           const struct genl_multicast_group *grp)
+{
+       return info->nlhdr->nlmsg_flags & NLM_F_ECHO ||
+               netlink_has_listeners(genl_info_net(info)->genl_sock, grp->id);
+}
+
 static void ovs_notify(struct sk_buff *skb, struct genl_info *info,
                       struct genl_multicast_group *grp)
 {
@@ -175,6 +184,7 @@ static struct hlist_head *vport_hash_bucket(const struct datapath *dp,
        return &dp->ports[port_no & (DP_VPORT_HASH_BUCKETS - 1)];
 }
 
+/* Called with ovs_mutex or RCU read lock. */
 struct vport *ovs_lookup_vport(const struct datapath *dp, u16 port_no)
 {
        struct vport *vport;
@@ -400,7 +410,7 @@ static int queue_userspace_packet(struct datapath *dp, struct sk_buff *skb,
 #endif
                .snd_portid = upcall_info->portid,
        };
-       size_t len, plen;
+       size_t len;
        unsigned int hlen;
        int err, dp_ifindex;
 
@@ -472,9 +482,12 @@ static int queue_userspace_packet(struct datapath *dp, struct sk_buff *skb,
        skb_zerocopy(user_skb, skb, skb->len, hlen);
 
        /* Pad OVS_PACKET_ATTR_PACKET if linear copy was performed */
-       if (!(dp->user_features & OVS_DP_F_UNALIGNED) &&
-           (plen = (ALIGN(user_skb->len, NLA_ALIGNTO) - user_skb->len)) > 0)
-               memset(skb_put(user_skb, plen), 0, plen);
+       if (!(dp->user_features & OVS_DP_F_UNALIGNED)) {
+               size_t plen = NLA_ALIGN(user_skb->len) - user_skb->len;
+
+               if (plen > 0)
+                       memset(skb_put(user_skb, plen), 0, plen);
+       }
 
        ((struct nlmsghdr *) user_skb->data)->nlmsg_len = user_skb->len;
 
@@ -522,7 +535,7 @@ static int ovs_packet_cmd_execute(struct sk_buff *skb, struct genl_info *info)
                packet->protocol = htons(ETH_P_802_2);
 
        /* Build an sw_flow for sending this packet. */
-       flow = ovs_flow_alloc(false);
+       flow = ovs_flow_alloc();
        err = PTR_ERR(flow);
        if (IS_ERR(flow))
                goto err_kfree_skb;
@@ -650,7 +663,7 @@ static size_t ovs_flow_cmd_msg_size(const struct sw_flow_actions *acts)
                + nla_total_size(acts->actions_len); /* OVS_FLOW_ATTR_ACTIONS */
 }
 
-/* Called with ovs_mutex. */
+/* Called with ovs_mutex or RCU read lock. */
 static int ovs_flow_cmd_fill_info(struct sw_flow *flow, struct datapath *dp,
                                  struct sk_buff *skb, u32 portid,
                                  u32 seq, u32 flags, u8 cmd)
@@ -741,27 +754,39 @@ error:
        return err;
 }
 
+/* Must be called with ovs_mutex. */
 static struct sk_buff *ovs_flow_cmd_alloc_info(struct sw_flow *flow,
-                                              struct genl_info *info)
+                                              struct genl_info *info,
+                                              bool always)
 {
+       struct sk_buff *skb;
        size_t len;
 
+       if (!always && !ovs_must_notify(info, &ovs_dp_flow_multicast_group))
+               return NULL;
+
        len = ovs_flow_cmd_msg_size(ovsl_dereference(flow->sf_acts));
 
-       return genlmsg_new_unicast(len, info, GFP_KERNEL);
+       skb = genlmsg_new_unicast(len, info, GFP_KERNEL);
+
+       if (!skb)
+               return ERR_PTR(-ENOMEM);
+
+       return skb;
 }
 
+/* Must be called with ovs_mutex. */
 static struct sk_buff *ovs_flow_cmd_build_info(struct sw_flow *flow,
                                               struct datapath *dp,
                                               struct genl_info *info,
-                                              u8 cmd)
+                                              u8 cmd, bool always)
 {
        struct sk_buff *skb;
        int retval;
 
-       skb = ovs_flow_cmd_alloc_info(flow, info);
-       if (!skb)
-               return ERR_PTR(-ENOMEM);
+       skb = ovs_flow_cmd_alloc_info(flow, info, always);
+       if (!skb || IS_ERR(skb))
+               return skb;
 
        retval = ovs_flow_cmd_fill_info(flow, dp, skb, info->snd_portid,
                                        info->snd_seq, 0, cmd);
@@ -780,7 +805,6 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb, struct genl_info *info)
        struct datapath *dp;
        struct sw_flow_actions *acts = NULL;
        struct sw_flow_match match;
-       bool exact_5tuple;
        int error;
 
        /* Extract key. */
@@ -789,7 +813,7 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb, struct genl_info *info)
                goto error;
 
        ovs_match_init(&match, &key, &mask);
-       error = ovs_nla_get_match(&match, &exact_5tuple,
+       error = ovs_nla_get_match(&match,
                                  a[OVS_FLOW_ATTR_KEY], a[OVS_FLOW_ATTR_MASK]);
        if (error)
                goto error;
@@ -809,6 +833,7 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb, struct genl_info *info)
                        goto err_kfree;
                }
        } else if (info->genlhdr->cmd == OVS_FLOW_CMD_NEW) {
+               /* OVS_FLOW_CMD_NEW must have actions. */
                error = -EINVAL;
                goto error;
        }
@@ -828,7 +853,7 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb, struct genl_info *info)
                        goto err_unlock_ovs;
 
                /* Allocate flow. */
-               flow = ovs_flow_alloc(!exact_5tuple);
+               flow = ovs_flow_alloc();
                if (IS_ERR(flow)) {
                        error = PTR_ERR(flow);
                        goto err_unlock_ovs;
@@ -845,11 +870,10 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb, struct genl_info *info)
                        goto err_flow_free;
                }
 
-               reply = ovs_flow_cmd_build_info(flow, dp, info, OVS_FLOW_CMD_NEW);
+               reply = ovs_flow_cmd_build_info(flow, dp, info,
+                                               OVS_FLOW_CMD_NEW, false);
        } else {
                /* We found a matching flow. */
-               struct sw_flow_actions *old_acts;
-
                /* Bail out if we're not allowed to modify an existing flow.
                 * We accept NLM_F_CREATE in place of the intended NLM_F_EXCL
                 * because Generic Netlink treats the latter as a dump
@@ -862,18 +886,19 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb, struct genl_info *info)
                        goto err_unlock_ovs;
 
                /* The unmasked key has to be the same for flow updates. */
-               error = -EINVAL;
-               if (!ovs_flow_cmp_unmasked_key(flow, &match)) {
-                       OVS_NLERR("Flow modification message rejected, unmasked key does not match.\n");
+               if (!ovs_flow_cmp_unmasked_key(flow, &match))
                        goto err_unlock_ovs;
-               }
 
-               /* Update actions. */
-               old_acts = ovsl_dereference(flow->sf_acts);
-               rcu_assign_pointer(flow->sf_acts, acts);
-               ovs_nla_free_flow_actions(old_acts);
+               /* Update actions, if present. */
+               if (acts) {
+                       struct sw_flow_actions *old_acts;
 
-               reply = ovs_flow_cmd_build_info(flow, dp, info, OVS_FLOW_CMD_NEW);
+                       old_acts = ovsl_dereference(flow->sf_acts);
+                       rcu_assign_pointer(flow->sf_acts, acts);
+                       ovs_nla_free_flow_actions(old_acts);
+               }
+               reply = ovs_flow_cmd_build_info(flow, dp, info,
+                                               OVS_FLOW_CMD_NEW, false);
 
                /* Clear stats. */
                if (a[OVS_FLOW_ATTR_CLEAR])
@@ -881,11 +906,14 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb, struct genl_info *info)
        }
        ovs_unlock();
 
-       if (!IS_ERR(reply))
-               ovs_notify(reply, info, &ovs_dp_flow_multicast_group);
-       else
-               netlink_set_err(sock_net(skb->sk)->genl_sock, 0,
-                               ovs_dp_flow_multicast_group.id, PTR_ERR(reply));
+       if (reply) {
+               if (!IS_ERR(reply))
+                       ovs_notify(reply, info, &ovs_dp_flow_multicast_group);
+               else
+                       netlink_set_err(sock_net(skb->sk)->genl_sock, 0,
+                                       ovs_dp_flow_multicast_group.id,
+                                       PTR_ERR(reply));
+       }
        return 0;
 
 err_flow_free:
@@ -915,7 +943,7 @@ static int ovs_flow_cmd_get(struct sk_buff *skb, struct genl_info *info)
        }
 
        ovs_match_init(&match, &key, NULL);
-       err = ovs_nla_get_match(&match, NULL, a[OVS_FLOW_ATTR_KEY], NULL);
+       err = ovs_nla_get_match(&match, a[OVS_FLOW_ATTR_KEY], NULL);
        if (err)
                return err;
 
@@ -932,7 +960,7 @@ static int ovs_flow_cmd_get(struct sk_buff *skb, struct genl_info *info)
                goto unlock;
        }
 
-       reply = ovs_flow_cmd_build_info(flow, dp, info, OVS_FLOW_CMD_NEW);
+       reply = ovs_flow_cmd_build_info(flow, dp, info, OVS_FLOW_CMD_NEW, true);
        if (IS_ERR(reply)) {
                err = PTR_ERR(reply);
                goto unlock;
@@ -969,7 +997,7 @@ static int ovs_flow_cmd_del(struct sk_buff *skb, struct genl_info *info)
        }
 
        ovs_match_init(&match, &key, NULL);
-       err = ovs_nla_get_match(&match, NULL, a[OVS_FLOW_ATTR_KEY], NULL);
+       err = ovs_nla_get_match(&match, a[OVS_FLOW_ATTR_KEY], NULL);
        if (err)
                goto unlock;
 
@@ -979,22 +1007,26 @@ static int ovs_flow_cmd_del(struct sk_buff *skb, struct genl_info *info)
                goto unlock;
        }
 
-       reply = ovs_flow_cmd_alloc_info(flow, info);
-       if (!reply) {
-               err = -ENOMEM;
+       reply = ovs_flow_cmd_alloc_info(flow, info, false);
+       if (IS_ERR(reply)) {
+               err = PTR_ERR(reply);
                goto unlock;
        }
 
        ovs_flow_tbl_remove(&dp->table, flow);
 
-       err = ovs_flow_cmd_fill_info(flow, dp, reply, info->snd_portid,
-                                    info->snd_seq, 0, OVS_FLOW_CMD_DEL);
-       BUG_ON(err < 0);
+       if (reply) {
+               err = ovs_flow_cmd_fill_info(flow, dp, reply, info->snd_portid,
+                                            info->snd_seq, 0,
+                                            OVS_FLOW_CMD_DEL);
+               BUG_ON(err < 0);
+       }
 
        ovs_flow_free(flow, true);
        ovs_unlock();
 
-       ovs_notify(reply, info, &ovs_dp_flow_multicast_group);
+       if (reply)
+               ovs_notify(reply, info, &ovs_dp_flow_multicast_group);
        return 0;
 unlock:
        ovs_unlock();
@@ -1094,6 +1126,7 @@ static size_t ovs_dp_cmd_msg_size(void)
        return msgsize;
 }
 
+/* Called with ovs_mutex or RCU read lock. */
 static int ovs_dp_cmd_fill_info(struct datapath *dp, struct sk_buff *skb,
                                u32 portid, u32 seq, u32 flags, u8 cmd)
 {
@@ -1109,9 +1142,7 @@ static int ovs_dp_cmd_fill_info(struct datapath *dp, struct sk_buff *skb,
 
        ovs_header->dp_ifindex = get_dpifindex(dp);
 
-       rcu_read_lock();
        err = nla_put_string(skb, OVS_DP_ATTR_NAME, ovs_dp_name(dp));
-       rcu_read_unlock();
        if (err)
                goto nla_put_failure;
 
@@ -1136,25 +1167,12 @@ error:
        return -EMSGSIZE;
 }
 
-static struct sk_buff *ovs_dp_cmd_build_info(struct datapath *dp,
-                                            struct genl_info *info, u8 cmd)
+static struct sk_buff *ovs_dp_cmd_alloc_info(struct genl_info *info)
 {
-       struct sk_buff *skb;
-       int retval;
-
-       skb = genlmsg_new_unicast(ovs_dp_cmd_msg_size(), info, GFP_KERNEL);
-       if (!skb)
-               return ERR_PTR(-ENOMEM);
-
-       retval = ovs_dp_cmd_fill_info(dp, skb, info->snd_portid, info->snd_seq, 0, cmd);
-       if (retval < 0) {
-               kfree_skb(skb);
-               return ERR_PTR(retval);
-       }
-       return skb;
+       return genlmsg_new_unicast(ovs_dp_cmd_msg_size(), info, GFP_KERNEL);
 }
 
-/* Called with ovs_mutex. */
+/* Called with rcu_read_lock or ovs_mutex. */
 static struct datapath *lookup_datapath(struct net *net,
                                        struct ovs_header *ovs_header,
                                        struct nlattr *a[OVS_DP_ATTR_MAX + 1])
@@ -1166,10 +1184,8 @@ static struct datapath *lookup_datapath(struct net *net,
        else {
                struct vport *vport;
 
-               rcu_read_lock();
                vport = ovs_vport_locate(net, nla_data(a[OVS_DP_ATTR_NAME]));
                dp = vport && vport->port_no == OVSP_LOCAL ? vport->dp : NULL;
-               rcu_read_unlock();
        }
        return dp ? dp : ERR_PTR(-ENODEV);
 }
@@ -1179,7 +1195,7 @@ static void ovs_dp_reset_user_features(struct sk_buff *skb, struct genl_info *in
        struct datapath *dp;
 
        dp = lookup_datapath(sock_net(skb->sk), info->userhdr, info->attrs);
-       if (!dp)
+       if (IS_ERR(dp))
                return;
 
        WARN(dp->user_features, "Dropping previously announced user features\n");
@@ -1206,12 +1222,14 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, struct genl_info *info)
        if (!a[OVS_DP_ATTR_NAME] || !a[OVS_DP_ATTR_UPCALL_PID])
                goto err;
 
-       ovs_lock();
+       reply = ovs_dp_cmd_alloc_info(info);
+       if (!reply)
+               return -ENOMEM;
 
        err = -ENOMEM;
        dp = kzalloc(sizeof(*dp), GFP_KERNEL);
        if (dp == NULL)
-               goto err_unlock_ovs;
+               goto err_free_reply;
 
        ovs_dp_set_net(dp, hold_net(sock_net(skb->sk)));
 
@@ -1246,6 +1264,9 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, struct genl_info *info)
 
        ovs_dp_change(dp, a);
 
+       /* So far only local changes have been made, now need the lock. */
+       ovs_lock();
+
        vport = new_vport(&parms);
        if (IS_ERR(vport)) {
                err = PTR_ERR(vport);
@@ -1264,10 +1285,9 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, struct genl_info *info)
                goto err_destroy_ports_array;
        }
 
-       reply = ovs_dp_cmd_build_info(dp, info, OVS_DP_CMD_NEW);
-       err = PTR_ERR(reply);
-       if (IS_ERR(reply))
-               goto err_destroy_local_port;
+       err = ovs_dp_cmd_fill_info(dp, reply, info->snd_portid,
+                                  info->snd_seq, 0, OVS_DP_CMD_NEW);
+       BUG_ON(err < 0);
 
        ovs_net = net_generic(ovs_dp_get_net(dp), ovs_net_id);
        list_add_tail_rcu(&dp->list_node, &ovs_net->dps);
@@ -1277,9 +1297,8 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, struct genl_info *info)
        ovs_notify(reply, info, &ovs_dp_datapath_multicast_group);
        return 0;
 
-err_destroy_local_port:
-       ovs_dp_detach_port(ovs_vport_ovsl(dp, OVSP_LOCAL));
 err_destroy_ports_array:
+       ovs_unlock();
        kfree(dp->ports);
 err_destroy_percpu:
        free_percpu(dp->stats_percpu);
@@ -1288,8 +1307,8 @@ err_destroy_table:
 err_free_dp:
        release_net(ovs_dp_get_net(dp));
        kfree(dp);
-err_unlock_ovs:
-       ovs_unlock();
+err_free_reply:
+       kfree_skb(reply);
 err:
        return err;
 }
@@ -1327,25 +1346,29 @@ static int ovs_dp_cmd_del(struct sk_buff *skb, struct genl_info *info)
        struct datapath *dp;
        int err;
 
+       reply = ovs_dp_cmd_alloc_info(info);
+       if (!reply)
+               return -ENOMEM;
+
        ovs_lock();
        dp = lookup_datapath(sock_net(skb->sk), info->userhdr, info->attrs);
        err = PTR_ERR(dp);
        if (IS_ERR(dp))
-               goto unlock;
+               goto err_unlock_free;
 
-       reply = ovs_dp_cmd_build_info(dp, info, OVS_DP_CMD_DEL);
-       err = PTR_ERR(reply);
-       if (IS_ERR(reply))
-               goto unlock;
+       err = ovs_dp_cmd_fill_info(dp, reply, info->snd_portid,
+                                  info->snd_seq, 0, OVS_DP_CMD_DEL);
+       BUG_ON(err < 0);
 
        __dp_destroy(dp);
-       ovs_unlock();
 
+       ovs_unlock();
        ovs_notify(reply, info, &ovs_dp_datapath_multicast_group);
-
        return 0;
-unlock:
+
+err_unlock_free:
        ovs_unlock();
+       kfree_skb(reply);
        return err;
 }
 
@@ -1355,29 +1378,29 @@ static int ovs_dp_cmd_set(struct sk_buff *skb, struct genl_info *info)
        struct datapath *dp;
        int err;
 
+       reply = ovs_dp_cmd_alloc_info(info);
+       if (!reply)
+               return -ENOMEM;
+
        ovs_lock();
        dp = lookup_datapath(sock_net(skb->sk), info->userhdr, info->attrs);
        err = PTR_ERR(dp);
        if (IS_ERR(dp))
-               goto unlock;
+               goto err_unlock_free;
 
        ovs_dp_change(dp, info->attrs);
 
-       reply = ovs_dp_cmd_build_info(dp, info, OVS_DP_CMD_NEW);
-       if (IS_ERR(reply)) {
-               err = PTR_ERR(reply);
-               netlink_set_err(sock_net(skb->sk)->genl_sock, 0,
-                               ovs_dp_datapath_multicast_group.id, err);
-               err = 0;
-               goto unlock;
-       }
+       err = ovs_dp_cmd_fill_info(dp, reply, info->snd_portid,
+                                  info->snd_seq, 0, OVS_DP_CMD_NEW);
+       BUG_ON(err < 0);
 
        ovs_unlock();
        ovs_notify(reply, info, &ovs_dp_datapath_multicast_group);
-
        return 0;
-unlock:
+
+err_unlock_free:
        ovs_unlock();
+       kfree_skb(reply);
        return err;
 }
 
@@ -1387,24 +1410,26 @@ static int ovs_dp_cmd_get(struct sk_buff *skb, struct genl_info *info)
        struct datapath *dp;
        int err;
 
-       ovs_lock();
+       reply = ovs_dp_cmd_alloc_info(info);
+       if (!reply)
+               return -ENOMEM;
+
+       rcu_read_lock();
        dp = lookup_datapath(sock_net(skb->sk), info->userhdr, info->attrs);
        if (IS_ERR(dp)) {
                err = PTR_ERR(dp);
-               goto unlock;
-       }
-
-       reply = ovs_dp_cmd_build_info(dp, info, OVS_DP_CMD_NEW);
-       if (IS_ERR(reply)) {
-               err = PTR_ERR(reply);
-               goto unlock;
+               goto err_unlock_free;
        }
+       err = ovs_dp_cmd_fill_info(dp, reply, info->snd_portid,
+                                  info->snd_seq, 0, OVS_DP_CMD_NEW);
+       BUG_ON(err < 0);
+       rcu_read_unlock();
 
-       ovs_unlock();
        return genlmsg_reply(reply, info);
 
-unlock:
-       ovs_unlock();
+err_unlock_free:
+       rcu_read_unlock();
+       kfree_skb(reply);
        return err;
 }
 
@@ -1517,7 +1542,12 @@ error:
        return err;
 }
 
-/* Called with ovs_mutex or RCU read lock. */
+static struct sk_buff *ovs_vport_cmd_alloc_info(void)
+{
+       return nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+}
+
+/* Called with ovs_mutex, only via ovs_dp_notify_wq(). */
 struct sk_buff *ovs_vport_cmd_build_info(struct vport *vport, u32 portid,
                                         u32 seq, u8 cmd)
 {
@@ -1579,33 +1609,35 @@ static int ovs_vport_cmd_new(struct sk_buff *skb, struct genl_info *info)
        u32 port_no;
        int err;
 
-       err = -EINVAL;
        if (!a[OVS_VPORT_ATTR_NAME] || !a[OVS_VPORT_ATTR_TYPE] ||
            !a[OVS_VPORT_ATTR_UPCALL_PID])
-               goto exit;
+               return -EINVAL;
+
+       port_no = a[OVS_VPORT_ATTR_PORT_NO]
+               ? nla_get_u32(a[OVS_VPORT_ATTR_PORT_NO]) : 0;
+       if (port_no >= DP_MAX_PORTS)
+               return -EFBIG;
+
+       reply = ovs_vport_cmd_alloc_info();
+       if (!reply)
+               return -ENOMEM;
 
        ovs_lock();
        dp = get_dp(sock_net(skb->sk), ovs_header->dp_ifindex);
        err = -ENODEV;
        if (!dp)
-               goto exit_unlock;
-
-       if (a[OVS_VPORT_ATTR_PORT_NO]) {
-               port_no = nla_get_u32(a[OVS_VPORT_ATTR_PORT_NO]);
-
-               err = -EFBIG;
-               if (port_no >= DP_MAX_PORTS)
-                       goto exit_unlock;
+               goto exit_unlock_free;
 
+       if (port_no) {
                vport = ovs_vport_ovsl(dp, port_no);
                err = -EBUSY;
                if (vport)
-                       goto exit_unlock;
+                       goto exit_unlock_free;
        } else {
                for (port_no = 1; ; port_no++) {
                        if (port_no >= DP_MAX_PORTS) {
                                err = -EFBIG;
-                               goto exit_unlock;
+                               goto exit_unlock_free;
                        }
                        vport = ovs_vport_ovsl(dp, port_no);
                        if (!vport)
@@ -1623,25 +1655,23 @@ static int ovs_vport_cmd_new(struct sk_buff *skb, struct genl_info *info)
        vport = new_vport(&parms);
        err = PTR_ERR(vport);
        if (IS_ERR(vport))
-               goto exit_unlock;
+               goto exit_unlock_free;
 
        err = 0;
        if (a[OVS_VPORT_ATTR_STATS])
                ovs_vport_set_stats(vport, nla_data(a[OVS_VPORT_ATTR_STATS]));
 
-       reply = ovs_vport_cmd_build_info(vport, info->snd_portid, info->snd_seq,
-                                        OVS_VPORT_CMD_NEW);
-       if (IS_ERR(reply)) {
-               err = PTR_ERR(reply);
-               ovs_dp_detach_port(vport);
-               goto exit_unlock;
-       }
+       err = ovs_vport_cmd_fill_info(vport, reply, info->snd_portid,
+                                     info->snd_seq, 0, OVS_VPORT_CMD_NEW);
+       BUG_ON(err < 0);
+       ovs_unlock();
 
        ovs_notify(reply, info, &ovs_dp_vport_multicast_group);
+       return 0;
 
-exit_unlock:
+exit_unlock_free:
        ovs_unlock();
-exit:
+       kfree_skb(reply);
        return err;
 }
 
@@ -1652,28 +1682,26 @@ static int ovs_vport_cmd_set(struct sk_buff *skb, struct genl_info *info)
        struct vport *vport;
        int err;
 
+       reply = ovs_vport_cmd_alloc_info();
+       if (!reply)
+               return -ENOMEM;
+
        ovs_lock();
        vport = lookup_vport(sock_net(skb->sk), info->userhdr, a);
        err = PTR_ERR(vport);
        if (IS_ERR(vport))
-               goto exit_unlock;
+               goto exit_unlock_free;
 
        if (a[OVS_VPORT_ATTR_TYPE] &&
            nla_get_u32(a[OVS_VPORT_ATTR_TYPE]) != vport->ops->type) {
                err = -EINVAL;
-               goto exit_unlock;
-       }
-
-       reply = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
-       if (!reply) {
-               err = -ENOMEM;
-               goto exit_unlock;
+               goto exit_unlock_free;
        }
 
        if (a[OVS_VPORT_ATTR_OPTIONS]) {
                err = ovs_vport_set_options(vport, a[OVS_VPORT_ATTR_OPTIONS]);
                if (err)
-                       goto exit_free;
+                       goto exit_unlock_free;
        }
 
        if (a[OVS_VPORT_ATTR_STATS])
@@ -1685,15 +1713,14 @@ static int ovs_vport_cmd_set(struct sk_buff *skb, struct genl_info *info)
        err = ovs_vport_cmd_fill_info(vport, reply, info->snd_portid,
                                      info->snd_seq, 0, OVS_VPORT_CMD_NEW);
        BUG_ON(err < 0);
-
        ovs_unlock();
+
        ovs_notify(reply, info, &ovs_dp_vport_multicast_group);
        return 0;
 
-exit_free:
-       kfree_skb(reply);
-exit_unlock:
+exit_unlock_free:
        ovs_unlock();
+       kfree_skb(reply);
        return err;
 }
 
@@ -1704,30 +1731,33 @@ static int ovs_vport_cmd_del(struct sk_buff *skb, struct genl_info *info)
        struct vport *vport;
        int err;
 
+       reply = ovs_vport_cmd_alloc_info();
+       if (!reply)
+               return -ENOMEM;
+
        ovs_lock();
        vport = lookup_vport(sock_net(skb->sk), info->userhdr, a);
        err = PTR_ERR(vport);
        if (IS_ERR(vport))
-               goto exit_unlock;
+               goto exit_unlock_free;
 
        if (vport->port_no == OVSP_LOCAL) {
                err = -EINVAL;
-               goto exit_unlock;
+               goto exit_unlock_free;
        }
 
-       reply = ovs_vport_cmd_build_info(vport, info->snd_portid,
-                                        info->snd_seq, OVS_VPORT_CMD_DEL);
-       err = PTR_ERR(reply);
-       if (IS_ERR(reply))
-               goto exit_unlock;
-
-       err = 0;
+       err = ovs_vport_cmd_fill_info(vport, reply, info->snd_portid,
+                                     info->snd_seq, 0, OVS_VPORT_CMD_DEL);
+       BUG_ON(err < 0);
        ovs_dp_detach_port(vport);
+       ovs_unlock();
 
        ovs_notify(reply, info, &ovs_dp_vport_multicast_group);
+       return 0;
 
-exit_unlock:
+exit_unlock_free:
        ovs_unlock();
+       kfree_skb(reply);
        return err;
 }
 
@@ -1739,24 +1769,25 @@ static int ovs_vport_cmd_get(struct sk_buff *skb, struct genl_info *info)
        struct vport *vport;
        int err;
 
+       reply = ovs_vport_cmd_alloc_info();
+       if (!reply)
+               return -ENOMEM;
+
        rcu_read_lock();
        vport = lookup_vport(sock_net(skb->sk), ovs_header, a);
        err = PTR_ERR(vport);
        if (IS_ERR(vport))
-               goto exit_unlock;
-
-       reply = ovs_vport_cmd_build_info(vport, info->snd_portid,
-                                        info->snd_seq, OVS_VPORT_CMD_NEW);
-       err = PTR_ERR(reply);
-       if (IS_ERR(reply))
-               goto exit_unlock;
-
+               goto exit_unlock_free;
+       err = ovs_vport_cmd_fill_info(vport, reply, info->snd_portid,
+                                     info->snd_seq, 0, OVS_VPORT_CMD_NEW);
+       BUG_ON(err < 0);
        rcu_read_unlock();
 
        return genlmsg_reply(reply, info);
 
-exit_unlock:
+exit_unlock_free:
        rcu_read_unlock();
+       kfree_skb(reply);
        return err;
 }
 
@@ -1767,11 +1798,12 @@ static int ovs_vport_cmd_dump(struct sk_buff *skb, struct netlink_callback *cb)
        int bucket = cb->args[0], skip = cb->args[1];
        int i, j = 0;
 
+       rcu_read_lock();
        dp = get_dp(sock_net(skb->sk), ovs_header->dp_ifindex);
-       if (!dp)
+       if (!dp) {
+               rcu_read_unlock();
                return -ENODEV;
-
-       rcu_read_lock();
+       }
        for (i = bucket; i < DP_VPORT_HASH_BUCKETS; i++) {
                struct vport *vport;