Fix memory leak when OFPP_TABLE is used for a packet that matches no flow.
[sliver-openvswitch.git] / datapath / datapath.c
index e03372b..28da3bb 100644 (file)
 #include <linux/netfilter_bridge.h>
 #include <linux/inetdevice.h>
 #include <linux/list.h>
+#include <linux/rculist.h>
 
 #include "openflow-netlink.h"
 #include "datapath.h"
 #include "table.h"
 #include "chain.h"
+#include "dp_dev.h"
 #include "forward.h"
 #include "flow.h"
-#include "datapath_t.h"
 
 #include "compat.h"
 
@@ -60,24 +61,27 @@ struct net_bridge_port {
 static struct genl_family dp_genl_family;
 static struct genl_multicast_group mc_group;
 
-int dp_dev_setup(struct net_device *dev);  
-
 /* It's hard to imagine wanting more than one datapath, but... */
 #define DP_MAX 32
 
-/* datapaths.  Protected on the read side by rcu_read_lock, on the write side
- * by dp_mutex.
+/* Datapaths.  Protected on the read side by rcu_read_lock, on the write side
+ * by dp_mutex.  dp_mutex is almost completely redundant with genl_mutex
+ * maintained by the Generic Netlink code, but the timeout path needs mutual
+ * exclusion too.
  *
  * It is safe to access the datapath and net_bridge_port structures with just
- * the dp_mutex, but to access the chain you need to take the rcu_read_lock
- * also (because dp_mutex doesn't prevent flows from being destroyed).
+ * dp_mutex.
  */
 static struct datapath *dps[DP_MAX];
-static DEFINE_MUTEX(dp_mutex);
+DEFINE_MUTEX(dp_mutex);
+EXPORT_SYMBOL(dp_mutex);
 
 static int dp_maint_func(void *data);
 static int send_port_status(struct net_bridge_port *p, uint8_t status);
 static int dp_genl_openflow_done(struct netlink_callback *);
+static struct net_bridge_port *new_nbp(struct datapath *,
+                                      struct net_device *, int port_no);
+static int del_switch_port(struct net_bridge_port *);
 
 /* nla_shrink - reduce amount of space reserved by nla_reserve
  * @skb: socket buffer from which to recover room
@@ -199,13 +203,9 @@ alloc_openflow_skb(struct datapath *dp, size_t openflow_len, uint8_t type,
 static int
 send_openflow_skb(struct sk_buff *skb, const struct sender *sender) 
 {
-       int err = (sender
-                  ? genlmsg_unicast(skb, sender->pid)
-                  : genlmsg_multicast(skb, 0, mc_group.id, GFP_ATOMIC));
-       if (err && net_ratelimit())
-               printk(KERN_WARNING "send_openflow_skb: send failed: %d\n",
-                      err);
-       return err;
+       return (sender
+               ? genlmsg_unicast(skb, sender->pid)
+               : genlmsg_multicast(skb, 0, mc_group.id, GFP_ATOMIC));
 }
 
 /* Generates a unique datapath id.  It incorporates the datapath index
@@ -244,9 +244,7 @@ uint64_t gen_datapath_id(uint16_t dp_idx)
 }
 
 /* Creates a new datapath numbered 'dp_idx'.  Returns 0 for success or a
- * negative error code.
- *
- * Not called with any locks. */
+ * negative error code. */
 static int new_dp(int dp_idx)
 {
        struct datapath *dp;
@@ -258,9 +256,8 @@ static int new_dp(int dp_idx)
        if (!try_module_get(THIS_MODULE))
                return -ENODEV;
 
-       mutex_lock(&dp_mutex);
-       dp = rcu_dereference(dps[dp_idx]);
-       if (dp != NULL) {
+       /* Exit early if a datapath with that number already exists. */
+       if (dps[dp_idx]) {
                err = -EEXIST;
                goto err_unlock;
        }
@@ -270,44 +267,49 @@ static int new_dp(int dp_idx)
        if (dp == NULL)
                goto err_unlock;
 
+       /* Setup our "of" device */
+       err = dp_dev_setup(dp);
+       if (err)
+               goto err_free_dp;
+
        dp->dp_idx = dp_idx;
        dp->id = gen_datapath_id(dp_idx);
        dp->chain = chain_create(dp);
        if (dp->chain == NULL)
-               goto err_free_dp;
+               goto err_destroy_dp_dev;
        INIT_LIST_HEAD(&dp->port_list);
 
-#if 0
-       /* Setup our "of" device */
-       dp->dev.priv = dp;
-       rtnl_lock();
-       err = dp_dev_setup(&dp->dev);
-       rtnl_unlock();
-       if (err != 0) 
-               printk("datapath: problem setting up 'of' device\n");
-#endif
+       dp->local_port = new_nbp(dp, dp->netdev, OFPP_LOCAL);
+       if (IS_ERR(dp->local_port)) {
+               err = PTR_ERR(dp->local_port);
+               goto err_destroy_local_port;
+       }
 
        dp->flags = 0;
        dp->miss_send_len = OFP_DEFAULT_MISS_SEND_LEN;
 
        dp->dp_task = kthread_run(dp_maint_func, dp, "dp%d", dp_idx);
        if (IS_ERR(dp->dp_task))
-               goto err_free_dp;
+               goto err_destroy_chain;
 
-       rcu_assign_pointer(dps[dp_idx], dp);
-       mutex_unlock(&dp_mutex);
+       dps[dp_idx] = dp;
 
        return 0;
 
+err_destroy_local_port:
+       del_switch_port(dp->local_port);
+err_destroy_chain:
+       chain_destroy(dp->chain);
+err_destroy_dp_dev:
+       dp_dev_destroy(dp);
 err_free_dp:
        kfree(dp);
 err_unlock:
-       mutex_unlock(&dp_mutex);
        module_put(THIS_MODULE);
                return err;
 }
 
-/* Find and return a free port number under 'dp'.  Called under dp_mutex. */
+/* Find and return a free port number under 'dp'. */
 static int find_portno(struct datapath *dp)
 {
        int i;
@@ -318,59 +320,57 @@ static int find_portno(struct datapath *dp)
 }
 
 static struct net_bridge_port *new_nbp(struct datapath *dp,
-                                                                          struct net_device *dev)
+                                      struct net_device *dev, int port_no)
 {
        struct net_bridge_port *p;
-       int port_no;
 
-       port_no = find_portno(dp);
-       if (port_no < 0)
-               return ERR_PTR(port_no);
+       if (dev->br_port != NULL)
+               return ERR_PTR(-EBUSY);
 
        p = kzalloc(sizeof(*p), GFP_KERNEL);
        if (p == NULL)
                return ERR_PTR(-ENOMEM);
 
-       p->dp = dp;
+       rtnl_lock();
+       dev_set_promiscuity(dev, 1);
+       rtnl_unlock();
        dev_hold(dev);
+       p->dp = dp;
        p->dev = dev;
        p->port_no = port_no;
+       if (port_no != OFPP_LOCAL)
+               rcu_assign_pointer(dev->br_port, p);
+       if (port_no < OFPP_MAX)
+               rcu_assign_pointer(dp->ports[port_no], p); 
+       list_add_rcu(&p->node, &dp->port_list);
 
        return p;
 }
 
-/* Called with dp_mutex. */
 int add_switch_port(struct datapath *dp, struct net_device *dev)
 {
        struct net_bridge_port *p;
+       int port_no;
 
-       if (dev->flags & IFF_LOOPBACK || dev->type != ARPHRD_ETHER)
+       if (dev->flags & IFF_LOOPBACK || dev->type != ARPHRD_ETHER
+           || is_dp_dev(dev))
                return -EINVAL;
 
-       if (dev->br_port != NULL)
-               return -EBUSY;
+       port_no = find_portno(dp);
+       if (port_no < 0)
+               return port_no;
 
-       p = new_nbp(dp, dev);
+       p = new_nbp(dp, dev, port_no);
        if (IS_ERR(p))
                return PTR_ERR(p);
 
-       dev_hold(dev);
-       rcu_assign_pointer(dev->br_port, p);
-       rtnl_lock();
-       dev_set_promiscuity(dev, 1);
-       rtnl_unlock();
-
-       rcu_assign_pointer(dp->ports[p->port_no], p);
-       list_add_rcu(&p->node, &dp->port_list);
-
        /* Notify the ctlpath that this port has been added */
        send_port_status(p, OFPPR_ADD);
 
        return 0;
 }
 
-/* Delete 'p' from switch.
- * Called with dp_mutex. */
+/* Delete 'p' from switch. */
 static int del_switch_port(struct net_bridge_port *p)
 {
        /* First drop references to device. */
@@ -378,7 +378,8 @@ static int del_switch_port(struct net_bridge_port *p)
        dev_set_promiscuity(p->dev, -1);
        rtnl_unlock();
        list_del_rcu(&p->node);
-       rcu_assign_pointer(p->dp->ports[p->port_no], NULL);
+       if (p->port_no != OFPP_LOCAL)
+               rcu_assign_pointer(p->dp->ports[p->port_no], NULL);
        rcu_assign_pointer(p->dev->br_port, NULL);
 
        /* Then wait until no one is still using it, and destroy it. */
@@ -393,18 +394,10 @@ static int del_switch_port(struct net_bridge_port *p)
        return 0;
 }
 
-/* Called with dp_mutex. */
 static void del_dp(struct datapath *dp)
 {
        struct net_bridge_port *p, *n;
 
-#if 0
-       /* Unregister the "of" device of this dp */
-       rtnl_lock();
-       unregister_netdevice(&dp->dev);
-       rtnl_unlock();
-#endif
-
        kthread_stop(dp->dp_task);
 
        /* Drop references to DP. */
@@ -412,6 +405,15 @@ static void del_dp(struct datapath *dp)
                del_switch_port(p);
        rcu_assign_pointer(dps[dp->dp_idx], NULL);
 
+       /* Kill off local_port dev references from buffered packets that have
+        * associated dst entries. */
+       synchronize_rcu();
+       fwd_discard_all();
+
+       /* Destroy dp->netdev.  (Must follow deleting switch ports since
+        * dp->local_port has a reference to it.) */
+       dp_dev_destroy(dp);
+
        /* Wait until no longer in use, then destroy it. */
        synchronize_rcu();
        chain_destroy(dp->chain);
@@ -424,80 +426,47 @@ static int dp_maint_func(void *data)
        struct datapath *dp = (struct datapath *) data;
 
        while (!kthread_should_stop()) {
-#if 1
                chain_timeout(dp->chain);
-#else
-               int count = chain_timeout(dp->chain);
-               chain_print_stats(dp->chain);
-               if (count)
-                       printk("%d flows timed out\n", count);
-#endif
                msleep_interruptible(MAINT_SLEEP_MSECS);
        }
                
        return 0;
 }
 
+static void
+do_port_input(struct net_bridge_port *p, struct sk_buff *skb) 
+{
+       /* Push the Ethernet header back on. */
+       skb_push(skb, ETH_HLEN);
+       fwd_port_input(p->dp->chain, skb, p->port_no);
+}
+
 /*
  * Used as br_handle_frame_hook.  (Cannot run bridge at the same time, even on
- * different set of devices!)  Returns 0 if *pskb should be processed further,
- * 1 if *pskb is handled. */
+ * different set of devices!)
+ */
 #if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,22)
 /* Called with rcu_read_lock. */
 static struct sk_buff *dp_frame_hook(struct net_bridge_port *p,
                                         struct sk_buff *skb)
 {
-       struct ethhdr *eh = eth_hdr(skb);
-       struct sk_buff *skb_local = NULL;
-
-
-       if (compare_ether_addr(eh->h_dest, skb->dev->dev_addr) == 0) 
-               return skb;
-
-       if (is_broadcast_ether_addr(eh->h_dest)
-                               || is_multicast_ether_addr(eh->h_dest)
-                               || is_local_ether_addr(eh->h_dest)) 
-               skb_local = skb_clone(skb, GFP_ATOMIC);
-
-       /* Push the Ethernet header back on. */
-       if (skb->protocol == htons(ETH_P_8021Q))
-               skb_push(skb, VLAN_ETH_HLEN);
-       else
-               skb_push(skb, ETH_HLEN);
-
-       fwd_port_input(p->dp->chain, skb, p->port_no);
-
-       return skb_local;
+       do_port_input(p, skb);
+       return NULL;
 }
 #elif LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,0)
 static int dp_frame_hook(struct net_bridge_port *p, struct sk_buff **pskb)
 {
-       /* Push the Ethernet header back on. */
-       if ((*pskb)->protocol == htons(ETH_P_8021Q))
-               skb_push(*pskb, VLAN_ETH_HLEN);
-       else
-               skb_push(*pskb, ETH_HLEN);
-
-       fwd_port_input(p->dp->chain, *pskb, p->port_no);
+       do_port_input(p, *pskb);
        return 1;
 }
-#else 
+#else
 /* NB: This has only been tested on 2.4.35 */
-
-/* Called without any locks (?) */
 static void dp_frame_hook(struct sk_buff *skb)
 {
        struct net_bridge_port *p = skb->dev->br_port;
-
-       /* Push the Ethernet header back on. */
-       if (skb->protocol == htons(ETH_P_8021Q))
-               skb_push(skb, VLAN_ETH_HLEN);
-       else
-               skb_push(skb, ETH_HLEN);
-
        if (p) {
                rcu_read_lock();
-               fwd_port_input(p->dp->chain, skb, p->port_no);
+               do_port_input(p, skb);
                rcu_read_unlock();
        } else
                kfree_skb(skb);
@@ -507,17 +476,6 @@ static void dp_frame_hook(struct sk_buff *skb)
 /* Forwarding output path.
  * Based on net/bridge/br_forward.c. */
 
-/* Don't forward packets to originating port or with flooding disabled */
-static inline int should_deliver(const struct net_bridge_port *p,
-                       const struct sk_buff *skb)
-{
-       if ((skb->dev == p->dev) || (p->flags & BRIDGE_PORT_NO_FLOOD)) {
-               return 0;
-       } 
-
-       return 1;
-}
-
 static inline unsigned packet_length(const struct sk_buff *skb)
 {
        int length = skb->len - ETH_HLEN;
@@ -526,15 +484,18 @@ static inline unsigned packet_length(const struct sk_buff *skb)
        return length;
 }
 
+/* Send packets out all the ports except the originating one.  If the
+ * "flood" argument is set, only send along the minimum spanning tree.
+ */
 static int
-flood(struct datapath *dp, struct sk_buff *skb)
+output_all(struct datapath *dp, struct sk_buff *skb, int flood)
 {
+       u32 disable = flood ? BRIDGE_PORT_NO_FLOOD : 0;
        struct net_bridge_port *p;
-       int prev_port;
+       int prev_port = -1;
 
-       prev_port = -1;
        list_for_each_entry_rcu (p, &dp->port_list, node) {
-               if (!should_deliver(p, skb))
+               if (skb->dev == p->dev || p->flags & disable)
                        continue;
                if (prev_port != -1) {
                        struct sk_buff *clone = skb_clone(skb, GFP_ATOMIC);
@@ -559,8 +520,11 @@ flood(struct datapath *dp, struct sk_buff *skb)
 int dp_set_origin(struct datapath *dp, uint16_t in_port,
                           struct sk_buff *skb)
 {
-       if (in_port < OFPP_MAX && dp->ports[in_port]) {
-               skb->dev = dp->ports[in_port]->dev;
+       struct net_bridge_port *p = (in_port < OFPP_MAX ? dp->ports[in_port]
+                                    : in_port == OFPP_LOCAL ? dp->local_port
+                                    : NULL);
+       if (p) {
+               skb->dev = p->dev;
                return 0;
        }
        return -ENOENT;
@@ -570,46 +534,49 @@ int dp_set_origin(struct datapath *dp, uint16_t in_port,
  */
 int dp_output_port(struct datapath *dp, struct sk_buff *skb, int out_port)
 {
-       struct net_bridge_port *p;
-       int len = skb->len;
-
        BUG_ON(!skb);
        if (out_port == OFPP_FLOOD)
-               return flood(dp, skb);
+               return output_all(dp, skb, 1);
+       else if (out_port == OFPP_ALL)
+               return output_all(dp, skb, 0);
        else if (out_port == OFPP_CONTROLLER)
                return dp_output_control(dp, skb, fwd_save_skb(skb), 0,
                                                  OFPR_ACTION);
        else if (out_port == OFPP_TABLE) {
+               struct net_bridge_port *p = skb->dev->br_port;
                struct sw_flow_key key;
                struct sw_flow *flow;
 
-               flow_extract(skb, skb->dev->br_port->port_no, &key);
+               flow_extract(skb, p ? p->port_no : OFPP_LOCAL, &key);
                flow = chain_lookup(dp->chain, &key);
                if (likely(flow != NULL)) {
                        flow_used(flow, skb);
                        execute_actions(dp, skb, &key, flow->actions, flow->n_actions);
                        return 0;
                }
+               kfree_skb(skb);
                return -ESRCH;
-       } else if (out_port >= OFPP_MAX)
-               goto bad_port;
+       } else if (out_port == OFPP_LOCAL) {
+               struct net_device *dev = dp->netdev;
+               return dev ? dp_dev_recv(dev, skb) : -ESRCH;
+       } else if (out_port >= 0 && out_port < OFPP_MAX) {
+               struct net_bridge_port *p = dp->ports[out_port];
+               int len = skb->len;
+               if (p == NULL)
+                       goto bad_port;
+               skb->dev = p->dev; 
+               if (packet_length(skb) > skb->dev->mtu) {
+                       printk("dropped over-mtu packet: %d > %d\n",
+                              packet_length(skb), skb->dev->mtu);
+                       kfree_skb(skb);
+                       return -E2BIG;
+               }
 
-       p = dp->ports[out_port];
-       if (p == NULL)
-               goto bad_port;
+               dev_queue_xmit(skb);
 
-       skb->dev = p->dev;
-       if (packet_length(skb) > skb->dev->mtu) {
-               printk("dropped over-mtu packet: %d > %d\n",
-                                       packet_length(skb), skb->dev->mtu);
-               kfree_skb(skb);
-               return -E2BIG;
+               return len;
        }
 
-       dev_queue_xmit(skb);
-
-       return len;
-
 bad_port:
        kfree_skb(skb);
        if (net_ratelimit())
@@ -631,6 +598,7 @@ dp_output_control(struct datapath *dp, struct sk_buff *skb,
         * forward the whole packet? */
        struct sk_buff *f_skb;
        struct ofp_packet_in *opi;
+       struct net_bridge_port *p;
        size_t fwd_len, opi_len;
        int err;
 
@@ -646,7 +614,8 @@ dp_output_control(struct datapath *dp, struct sk_buff *skb,
        }
        opi->buffer_id      = htonl(buffer_id);
        opi->total_len      = htons(skb->len);
-       opi->in_port        = htons(skb->dev->br_port->port_no);
+       p = skb->dev->br_port;
+       opi->in_port        = htons(p ? p->port_no : OFPP_LOCAL);
        opi->reason         = reason;
        opi->pad            = 0;
        memcpy(opi->data, skb_mac_header(skb), fwd_len);
@@ -763,16 +732,14 @@ dp_send_config_reply(struct datapath *dp, const struct sender *sender)
 int
 dp_update_port_flags(struct datapath *dp, const struct ofp_phy_port *opp)
 {
-       struct net_bridge_port *p;
-
-       p = dp->ports[htons(opp->port_no)];
-
+       int port_no = ntohs(opp->port_no);
+       struct net_bridge_port *p = (port_no < OFPP_MAX ? dp->ports[port_no]
+                                    : port_no == OFPP_LOCAL ? dp->local_port
+                                    : NULL);
        /* Make sure the port id hasn't changed since this was sent */
-       if (!p || memcmp(opp->hw_addr, p->dev->dev_addr, ETH_ALEN) != 0) 
+       if (!p || memcmp(opp->hw_addr, p->dev->dev_addr, ETH_ALEN))
                return -1;
-       
        p->flags = htonl(opp->flags);
-
        return 0;
 }
 
@@ -817,6 +784,7 @@ dp_send_flow_expired(struct datapath *dp, struct sw_flow *flow)
 
        return send_openflow_skb(skb, NULL);
 }
+EXPORT_SYMBOL(dp_send_flow_expired);
 
 int
 dp_send_error_msg(struct datapath *dp, const struct sender *sender, 
@@ -838,6 +806,22 @@ dp_send_error_msg(struct datapath *dp, const struct sender *sender,
        return send_openflow_skb(skb, sender);
 }
 
+int
+dp_send_echo_reply(struct datapath *dp, const struct sender *sender,
+                  const struct ofp_header *rq)
+{
+       struct sk_buff *skb;
+       struct ofp_header *reply;
+
+       reply = alloc_openflow_skb(dp, ntohs(rq->length), OFPT_ECHO_REPLY,
+                                  sender, &skb);
+       if (!reply)
+               return -ENOMEM;
+
+       memcpy(reply + 1, rq + 1, ntohs(rq->length) - sizeof *rq);
+       return send_openflow_skb(skb, sender);
+}
+
 /* Generic Netlink interface.
  *
  * See netlink(7) for an introduction to netlink.  See
@@ -891,7 +875,6 @@ static int dp_genl_del(struct sk_buff *skb, struct genl_info *info)
        if (!info->attrs[DP_GENL_A_DP_IDX])
                return -EINVAL;
 
-       mutex_lock(&dp_mutex);
        dp = dp_get(nla_get_u32((info->attrs[DP_GENL_A_DP_IDX])));
        if (!dp)
                err = -ENOENT;
@@ -899,7 +882,6 @@ static int dp_genl_del(struct sk_buff *skb, struct genl_info *info)
                del_dp(dp);
                err = 0;
        }
-       mutex_unlock(&dp_mutex);
        return err;
 }
 
@@ -983,7 +965,6 @@ static int dp_genl_add_del_port(struct sk_buff *skb, struct genl_info *info)
                return -EINVAL;
 
        /* Get datapath. */
-       mutex_lock(&dp_mutex);
        dp = dp_get(nla_get_u32(info->attrs[DP_GENL_A_DP_IDX]));
        if (!dp) {
                err = -ENOENT;
@@ -1012,7 +993,6 @@ static int dp_genl_add_del_port(struct sk_buff *skb, struct genl_info *info)
 out_put:
        dev_put(port);
 out:
-       mutex_unlock(&dp_mutex);
        return err;
 }
 
@@ -1043,26 +1023,22 @@ static int dp_genl_openflow(struct sk_buff *skb, struct genl_info *info)
        if (!info->attrs[DP_GENL_A_DP_IDX] || !va)
                return -EINVAL;
 
-       rcu_read_lock();
        dp = dp_get(nla_get_u32(info->attrs[DP_GENL_A_DP_IDX]));
-       if (!dp) {
-               err = -ENOENT;
-               goto out;
-       }
+       if (!dp)
+               return -ENOENT;
 
-       if (nla_len(va) < sizeof(struct ofp_header)) {
-               err = -EINVAL;
-               goto out;
-       }
+       if (nla_len(va) < sizeof(struct ofp_header))
+               return -EINVAL;
        oh = nla_data(va);
 
        sender.xid = oh->xid;
        sender.pid = info->snd_pid;
        sender.seq = info->snd_seq;
-       err = fwd_control_input(dp->chain, &sender, nla_data(va), nla_len(va));
 
-out:
-       rcu_read_unlock();
+       mutex_lock(&dp_mutex);
+       err = fwd_control_input(dp->chain, &sender,
+                               nla_data(va), nla_len(va));
+       mutex_unlock(&dp_mutex);
        return err;
 }
 
@@ -1246,7 +1222,7 @@ static int table_stats_dump(struct datapath *dp, void *state,
                memset(ots->pad, 0, sizeof ots->pad);
                ots->max_entries = htonl(stats.max_flows);
                ots->active_count = htonl(stats.n_flows);
-               ots->matched_count = cpu_to_be64(0); /* FIXME */
+               ots->matched_count = cpu_to_be64(stats.n_matched);
        }
        return 0;
 }
@@ -1378,7 +1354,6 @@ dp_genl_openflow_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
         * struct genl_ops.  This kluge supports earlier versions also. */
        cb->done = dp_genl_openflow_done;
 
-       rcu_read_lock();
        if (!cb->args[0]) {
                struct nlattr *attrs[DP_GENL_A_MAX + 1];
                struct ofp_stats_request *rq;
@@ -1391,21 +1366,17 @@ dp_genl_openflow_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
                if (err < 0)
                        return err;
 
-               err = -EINVAL;
-
                if (!attrs[DP_GENL_A_DP_IDX])
-                       goto out;
+                       return -EINVAL;
                dp_idx = nla_get_u16(attrs[DP_GENL_A_DP_IDX]);
                dp = dp_get(dp_idx);
-               if (!dp) {
-                       err = -ENOENT;
-                       goto out;
-               }
+               if (!dp)
+                       return -ENOENT;
 
                va = attrs[DP_GENL_A_OPENFLOW];
                len = nla_len(va);
                if (!va || len < sizeof *rq)
-                       goto out;
+                       return -EINVAL;
 
                rq = nla_data(va);
                type = ntohs(rq->type);
@@ -1414,12 +1385,12 @@ dp_genl_openflow_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
                    || ntohs(rq->header.length) != len
                    || type >= ARRAY_SIZE(stats)
                    || !stats[type].dump)
-                       goto out;
+                       return -EINVAL;
 
                s = &stats[type];
                body_len = len - offsetof(struct ofp_stats_request, body);
                if (body_len < s->min_body || body_len > s->max_body)
-                       goto out;
+                       return -EINVAL;
 
                cb->args[0] = 1;
                cb->args[1] = dp_idx;
@@ -1429,7 +1400,7 @@ dp_genl_openflow_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
                        void *state;
                        err = s->init(dp, rq->body, body_len, &state);
                        if (err)
-                               goto out;
+                               return err;
                        cb->args[4] = (long) state;
                }
        } else if (cb->args[0] == 1) {
@@ -1437,13 +1408,10 @@ dp_genl_openflow_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
                s = &stats[cb->args[2]];
 
                dp = dp_get(dp_idx);
-               if (!dp) {
-                       err = -ENOENT;
-                       goto out;
-               }
+               if (!dp)
+                       return -ENOENT;
        } else {
-               err = 0;
-               goto out;
+               return 0;
        }
 
        sender.xid = cb->args[3];
@@ -1452,10 +1420,8 @@ dp_genl_openflow_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
 
        osr = put_openflow_headers(dp, skb, OFPT_STATS_REPLY, &sender,
                                   &max_openflow_len);
-       if (IS_ERR(osr)) {
-               err = PTR_ERR(osr);
-               goto out;
-       }
+       if (IS_ERR(osr))
+               return PTR_ERR(osr);
        osr->type = htons(s - stats);
        osr->flags = 0;
        resize_openflow_skb(skb, &osr->header, max_openflow_len);
@@ -1474,8 +1440,6 @@ dp_genl_openflow_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
                err = skb->len;
        }
 
-out:
-       rcu_read_unlock();
        return err;
 }
 
@@ -1498,20 +1462,6 @@ static struct genl_ops dp_genl_ops_openflow = {
        .dumpit = dp_genl_openflow_dumpit,
 };
 
-static struct nla_policy dp_genl_benchmark_policy[DP_GENL_A_MAX + 1] = {
-       [DP_GENL_A_DP_IDX] = { .type = NLA_U32 },
-       [DP_GENL_A_NPACKETS] = { .type = NLA_U32 },
-       [DP_GENL_A_PSIZE] = { .type = NLA_U32 },
-};
-
-static struct genl_ops dp_genl_ops_benchmark_nl = {
-       .cmd = DP_GENL_C_BENCHMARK_NL,
-       .flags = GENL_ADMIN_PERM, /* Requires CAP_NET_ADMIN privilege. */
-       .policy = dp_genl_benchmark_policy,
-       .doit = dp_genl_benchmark_nl,
-       .dumpit = NULL,
-};
-
 static struct genl_ops *dp_genl_all_ops[] = {
        /* Keep this operation first.  Generic Netlink dispatching
         * looks up operations with linear search, so we want it at the
@@ -1523,7 +1473,6 @@ static struct genl_ops *dp_genl_all_ops[] = {
        &dp_genl_ops_query_dp,
        &dp_genl_ops_add_port,
        &dp_genl_ops_del_port,
-       &dp_genl_ops_benchmark_nl,
 };
 
 static int dp_init_netlink(void)