bridge: Fix high cpu utilization.
[cascardo/ovs.git] / ofproto / ofproto-dpif.c
index aeff00d..1d964dd 100644 (file)
@@ -273,6 +273,10 @@ struct dpif_backer {
      * False if the datapath supports only 8-byte (or shorter) userdata. */
     bool variable_length_userdata;
 
+    /* True if the datapath supports masked data in OVS_ACTION_ATTR_SET
+     * actions. */
+    bool masked_set_action;
+
     /* Maximum number of MPLS label stack entries that the datapath supports
      * in a match */
     size_t max_mpls_depth;
@@ -528,6 +532,8 @@ type_run(const char *type)
         udpif_set_threads(backer->udpif, n_handlers, n_revalidators);
     }
 
+    dpif_poll_threads_set(backer->dpif, n_dpdk_rxqs, pmd_cpu_mask);
+
     if (backer->need_revalidate) {
         struct ofproto_dpif *ofproto;
         struct simap_node *node;
@@ -617,7 +623,8 @@ type_run(const char *type)
                               connmgr_has_in_band(ofproto->up.connmgr),
                               ofproto->backer->enable_recirc,
                               ofproto->backer->variable_length_userdata,
-                              ofproto->backer->max_mpls_depth);
+                              ofproto->backer->max_mpls_depth,
+                              ofproto->backer->masked_set_action);
 
             HMAP_FOR_EACH (bundle, hmap_node, &ofproto->bundles) {
                 xlate_bundle_set(ofproto, bundle, bundle->name,
@@ -631,16 +638,14 @@ type_run(const char *type)
                 int stp_port = ofport->stp_port
                     ? stp_port_no(ofport->stp_port)
                     : -1;
-                int rstp_port = ofport->rstp_port
-                    ? rstp_port_number(ofport->rstp_port)
-                    : -1;
                 xlate_ofport_set(ofproto, ofport->bundle, ofport,
                                  ofport->up.ofp_port, ofport->odp_port,
                                  ofport->up.netdev, ofport->cfm,
                                  ofport->bfd, ofport->peer, stp_port,
-                                 rstp_port, ofport->qdscp, ofport->n_qdscp,
-                                 ofport->up.pp.config, ofport->up.pp.state,
-                                 ofport->is_tunnel, ofport->may_enable);
+                                 ofport->rstp_port, ofport->qdscp,
+                                 ofport->n_qdscp, ofport->up.pp.config,
+                                 ofport->up.pp.state, ofport->is_tunnel,
+                                 ofport->may_enable);
             }
             xlate_txn_commit();
         }
@@ -845,6 +850,7 @@ struct odp_garbage {
 static bool check_variable_length_userdata(struct dpif_backer *backer);
 static size_t check_max_mpls_depth(struct dpif_backer *backer);
 static bool check_recirc(struct dpif_backer *backer);
+static bool check_masked_set_action(struct dpif_backer *backer);
 
 static int
 open_dpif_backer(const char *type, struct dpif_backer **backerp)
@@ -939,8 +945,8 @@ open_dpif_backer(const char *type, struct dpif_backer **backerp)
     shash_add(&all_dpif_backers, type, backer);
 
     backer->enable_recirc = check_recirc(backer);
-    backer->variable_length_userdata = check_variable_length_userdata(backer);
     backer->max_mpls_depth = check_max_mpls_depth(backer);
+    backer->masked_set_action = check_masked_set_action(backer);
     backer->rid_pool = recirc_id_pool_create();
 
     error = dpif_recv_set(backer->dpif, backer->recv_set_enable);
@@ -955,6 +961,11 @@ open_dpif_backer(const char *type, struct dpif_backer **backerp)
         udpif_set_threads(backer->udpif, n_handlers, n_revalidators);
     }
 
+    /* This check fails if performed before udpif threads have been set,
+     * as the kernel module checks that the 'pid' in userspace action
+     * is non-zero. */
+    backer->variable_length_userdata = check_variable_length_userdata(backer);
+
     return error;
 }
 
@@ -1110,7 +1121,8 @@ check_max_mpls_depth(struct dpif_backer *backer)
         odp_flow_key_from_flow(&key, &flow, NULL, 0, false);
 
         error = dpif_flow_put(backer->dpif, DPIF_FP_CREATE,
-                              ofpbuf_data(&key), ofpbuf_size(&key), NULL, 0, NULL, 0, NULL);
+                              ofpbuf_data(&key), ofpbuf_size(&key), NULL, 0,
+                              NULL, 0, NULL);
         if (error && error != EEXIST) {
             if (error != EINVAL) {
                 VLOG_WARN("%s: MPLS stack length feature probe failed (%s)",
@@ -1119,7 +1131,8 @@ check_max_mpls_depth(struct dpif_backer *backer)
             break;
         }
 
-        error = dpif_flow_del(backer->dpif, ofpbuf_data(&key), ofpbuf_size(&key), NULL);
+        error = dpif_flow_del(backer->dpif, ofpbuf_data(&key),
+                              ofpbuf_size(&key), NULL);
         if (error) {
             VLOG_WARN("%s: failed to delete MPLS feature probe flow",
                       dpif_name(backer->dpif));
@@ -1131,6 +1144,55 @@ check_max_mpls_depth(struct dpif_backer *backer)
     return n;
 }
 
+/* Tests whether 'backer''s datapath supports masked data in
+ * OVS_ACTION_ATTR_SET actions.  We need to disable some features on older
+ * datapaths that don't support this feature. */
+static bool
+check_masked_set_action(struct dpif_backer *backer)
+{
+    struct eth_header *eth;
+    struct ofpbuf actions;
+    struct dpif_execute execute;
+    struct ofpbuf packet;
+    int error;
+    struct ovs_key_ethernet key, mask;
+
+    /* Compose a set action that will cause an EINVAL error on older
+     * datapaths that don't support masked set actions.
+     * Avoid using a full mask, as it could be translated to a non-masked
+     * set action instead. */
+    ofpbuf_init(&actions, 64);
+    memset(&key, 0x53, sizeof key);
+    memset(&mask, 0x7f, sizeof mask);
+    commit_masked_set_action(&actions, OVS_KEY_ATTR_ETHERNET, &key, &mask,
+                             sizeof key);
+
+    /* Compose a dummy ethernet packet. */
+    ofpbuf_init(&packet, ETH_HEADER_LEN);
+    eth = ofpbuf_put_zeros(&packet, ETH_HEADER_LEN);
+    eth->eth_type = htons(0x1234);
+
+    /* Execute the actions.  On older datapaths this fails with EINVAL, on
+     * newer datapaths it succeeds. */
+    execute.actions = ofpbuf_data(&actions);
+    execute.actions_len = ofpbuf_size(&actions);
+    execute.packet = &packet;
+    execute.md = PKT_METADATA_INITIALIZER(0);
+    execute.needs_help = false;
+
+    error = dpif_execute(backer->dpif, &execute);
+
+    ofpbuf_uninit(&packet);
+    ofpbuf_uninit(&actions);
+
+    if (error) {
+        /* Masked set action is not supported. */
+        VLOG_INFO("%s: datapath does not support masked set action feature.",
+                  dpif_name(backer->dpif));
+    }
+    return !error;
+}
+
 static int
 construct(struct ofproto *ofproto_)
 {
@@ -1568,7 +1630,7 @@ port_construct(struct ofport *port_)
     port->bundle = NULL;
     port->cfm = NULL;
     port->bfd = NULL;
-    port->may_enable = true;
+    port->may_enable = false;
     port->stp_port = NULL;
     port->stp_state = STP_DISABLED;
     port->rstp_port = NULL;
@@ -1683,9 +1745,7 @@ port_destruct(struct ofport *port_)
     if (port->stp_port) {
         stp_port_disable(port->stp_port);
     }
-    if (port->rstp_port) {
-        rstp_delete_port(port->rstp_port);
-    }
+    set_rstp_port(port_, NULL);
     if (ofproto->sflow) {
         dpif_sflow_del_port(ofproto->sflow, port->odp_port);
     }
@@ -1908,27 +1968,21 @@ get_bfd_status(struct ofport *ofport_, struct smap *smap)
 \f
 /* Spanning Tree. */
 
+/* Called while rstp_mutex is held. */
 static void
-rstp_send_bpdu_cb(struct ofpbuf *pkt, int port_num, void *ofproto_)
+rstp_send_bpdu_cb(struct ofpbuf *pkt, void *ofport_, void *ofproto_)
 {
     struct ofproto_dpif *ofproto = ofproto_;
-    struct rstp_port *rp = rstp_get_port(ofproto->rstp, port_num);
-    struct ofport_dpif *ofport;
-
-    ofport = rstp_port_get_aux(rp);
-    if (!ofport) {
-        VLOG_WARN_RL(&rl, "%s: cannot send BPDU on unknown port %d",
-                ofproto->up.name, port_num);
+    struct ofport_dpif *ofport = ofport_;
+    struct eth_header *eth = ofpbuf_l2(pkt);
+
+    netdev_get_etheraddr(ofport->up.netdev, eth->eth_src);
+    if (eth_addr_is_zero(eth->eth_src)) {
+        VLOG_WARN_RL(&rl, "%s port %d: cannot send RSTP BPDU on a port which "
+                     "does not have a configured source MAC address.",
+                     ofproto->up.name, ofp_to_u16(ofport->up.ofp_port));
     } else {
-        struct eth_header *eth = ofpbuf_l2(pkt);
-
-        netdev_get_etheraddr(ofport->up.netdev, eth->eth_src);
-        if (eth_addr_is_zero(eth->eth_src)) {
-            VLOG_WARN_RL(&rl, "%s: cannot send BPDU on port %d "
-                    "with unknown MAC", ofproto->up.name, port_num);
-        } else {
-            ofproto_dpif_send_packet(ofport, pkt);
-        }
+        ofproto_dpif_send_packet(ofport, pkt);
     }
     ofpbuf_delete(pkt);
 }
@@ -1972,7 +2026,7 @@ set_rstp(struct ofproto *ofproto_, const struct ofproto_rstp_settings *s)
     if (s) {
         if (!ofproto->rstp) {
             ofproto->rstp = rstp_create(ofproto_->name, s->address,
-              rstp_send_bpdu_cb, ofproto);
+                                        rstp_send_bpdu_cb, ofproto);
             ofproto->rstp_last_tick = time_msec();
         }
         rstp_set_bridge_address(ofproto->rstp, s->address);
@@ -2065,16 +2119,16 @@ rstp_run(struct ofproto_dpif *ofproto)
         long long int now = time_msec();
         long long int elapsed = now - ofproto->rstp_last_tick;
         struct rstp_port *rp;
+        struct ofport_dpif *ofport;
+
         /* Every second, decrease the values of the timers. */
         if (elapsed >= 1000) {
             rstp_tick_timers(ofproto->rstp);
             ofproto->rstp_last_tick = now;
         }
-        while (rstp_get_changed_port(ofproto->rstp, &rp)) {
-            struct ofport_dpif *ofport = rstp_port_get_aux(rp);
-            if (ofport) {
-                update_rstp_port_state(ofport);
-            }
+        rp = NULL;
+        while ((ofport = rstp_get_next_changed_port_aux(ofproto->rstp, &rp))) {
+            update_rstp_port_state(ofport);
         }
         /* FIXME: This check should be done on-event (i.e., when setting
          * p->fdb_flush) and not periodically.
@@ -2212,7 +2266,7 @@ set_stp_port(struct ofport *ofport_,
         }
         return 0;
     } else if (sp && stp_port_no(sp) != s->port_num
-            && ofport == stp_port_get_aux(sp)) {
+               && ofport == stp_port_get_aux(sp)) {
         /* The port-id changed, so disable the old one if it's not
          * already in use by another port. */
         stp_port_disable(sp);
@@ -2317,65 +2371,31 @@ stp_wait(struct ofproto_dpif *ofproto)
  * there are no duplicates. */
 static void
 set_rstp_port(struct ofport *ofport_,
-        const struct ofproto_port_rstp_settings *s)
+              const struct ofproto_port_rstp_settings *s)
 {
     struct ofport_dpif *ofport = ofport_dpif_cast(ofport_);
     struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofport->up.ofproto);
     struct rstp_port *rp = ofport->rstp_port;
-    int stp_port;
 
     if (!s || !s->enable) {
         if (rp) {
+            rstp_port_unref(rp);
             ofport->rstp_port = NULL;
-            rstp_delete_port(rp);
             update_rstp_port_state(ofport);
         }
-        return;
-    } else if (rp && rstp_port_number(rp) != s->port_num
-                  && ofport == rstp_port_get_aux(rp)) {
-        /* The port-id changed, so disable the old one if it's not
-         * already in use by another port. */
-        if (s->port_num != 0) {
-            xlate_txn_start();
-            stp_port = ofport->stp_port ? stp_port_no(ofport->stp_port) : -1;
-            xlate_ofport_set(ofproto, ofport->bundle, ofport,
-                    ofport->up.ofp_port, ofport->odp_port,
-                    ofport->up.netdev, ofport->cfm,
-                    ofport->bfd, ofport->peer, stp_port,
-                    s->port_num,
-                    ofport->qdscp, ofport->n_qdscp,
-                    ofport->up.pp.config, ofport->up.pp.state,
-                    ofport->is_tunnel, ofport->may_enable);
-            xlate_txn_commit();
-        }
-
-        rstp_port_set_aux(rp, ofport);
-        rstp_port_set_priority(rp, s->priority);
-        rstp_port_set_port_number(rp, s->port_num);
-        rstp_port_set_path_cost(rp, s->path_cost);
-        rstp_port_set_admin_edge(rp, s->admin_edge_port);
-        rstp_port_set_auto_edge(rp, s->auto_edge);
-        rstp_port_set_mcheck(rp, s->mcheck);
-
-        update_rstp_port_state(ofport);
-
         return;
     }
-    rp = ofport->rstp_port = rstp_get_port(ofproto->rstp, s->port_num);
-    /* Enable RSTP on port */
+
+    /* Check if need to add a new port. */
     if (!rp) {
         rp = ofport->rstp_port = rstp_add_port(ofproto->rstp);
     }
-    /* Setters */
-    rstp_port_set_aux(rp, ofport);
-    rstp_port_set_priority(rp, s->priority);
-    rstp_port_set_port_number(rp, s->port_num);
-    rstp_port_set_path_cost(rp, s->path_cost);
-    rstp_port_set_admin_edge(rp, s->admin_edge_port);
-    rstp_port_set_auto_edge(rp, s->auto_edge);
-    rstp_port_set_mcheck(rp, s->mcheck);
 
+    rstp_port_set(rp, s->port_num, s->priority, s->path_cost,
+                  s->admin_edge_port, s->auto_edge, s->mcheck, ofport);
     update_rstp_port_state(ofport);
+    /* Synchronize operational status. */
+    rstp_port_set_mac_operational(rp, ofport->may_enable);
 }
 
 static void
@@ -2392,11 +2412,8 @@ get_rstp_port_status(struct ofport *ofport_,
     }
 
     s->enabled = true;
-    s->port_id = rstp_port_get_id(rp);
-    s->state = rstp_port_get_state(rp);
-    s->role = rstp_port_get_role(rp);
-    rstp_port_get_counts(rp, &s->tx_count, &s->rx_count,
-                         &s->error_count, &s->uptime);
+    rstp_port_get_status(rp, &s->port_id, &s->state, &s->role, &s->tx_count,
+                         &s->rx_count, &s->error_count, &s->uptime);
 }
 
 \f
@@ -3132,16 +3149,15 @@ port_run(struct ofport_dpif *ofport)
 
     if (ofport->may_enable != enable) {
         struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofport->up.ofproto);
-        ofproto->backer->need_revalidate = REV_PORT_TOGGLED;
-    }
 
-    ofport->may_enable = enable;
+        ofproto->backer->need_revalidate = REV_PORT_TOGGLED;
 
-    if (ofport->rstp_port) {
-        if (rstp_port_get_mac_operational(ofport->rstp_port) != enable) {
+        if (ofport->rstp_port) {
             rstp_port_set_mac_operational(ofport->rstp_port, enable);
         }
     }
+
+    ofport->may_enable = enable;
 }
 
 static int