ofproto-dpif: Fix bond accounting.

[openvswitch] / ofproto / ofproto-dpif.c

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index b7194930d0a0e17bbf98605706f7a1b363dec053..d3cb92b91a0ef89b7901223fb6662dd6e8f8cae5 100644 (file)
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -351,8 +351,7 @@ static void handle_upcall(struct ofproto_dpif *, struct dpif_upcall *);
 static int expire(struct ofproto_dpif *);
 
 /* Utilities. */
-static int send_packet(struct ofproto_dpif *,
-                       uint32_t odp_port, uint16_t vlan_tci,
+static int send_packet(struct ofproto_dpif *, uint32_t odp_port,
                        const struct ofpbuf *packet);
 
 /* Global variables. */
@@ -830,6 +829,8 @@ bundle_del_port(struct ofport_dpif *port)
 {
     struct ofbundle *bundle = port->bundle;
 
+    bundle->ofproto->need_revalidate = true;
+
     list_remove(&port->bundle_node);
     port->bundle = NULL;
 
@@ -861,6 +862,7 @@ bundle_add_port(struct ofbundle *bundle, uint32_t ofp_port,
     }
 
     if (port->bundle != bundle) {
+        bundle->ofproto->need_revalidate = true;
         if (port->bundle) {
             bundle_del_port(port);
         }
@@ -990,7 +992,7 @@ bundle_set(struct ofproto *ofproto_, void *aux,
 
         LIST_FOR_EACH_SAFE (port, next_port, bundle_node, &bundle->ports) {
             for (i = 0; i < s->n_slaves; i++) {
-                if (s->slaves[i] == odp_port_to_ofp_port(port->odp_port)) {
+                if (s->slaves[i] == port->up.ofp_port) {
                     goto found;
                 }
             }
@@ -1029,6 +1031,7 @@ bundle_set(struct ofproto *ofproto_, void *aux,
             }
         } else {
             bundle->bond = bond_create(s->bond);
+            ofproto->need_revalidate = true;
         }
 
         LIST_FOR_EACH (port, bundle_node, &bundle->ports) {
@@ -1387,7 +1390,7 @@ port_run(struct ofport_dpif *ofport)
                               ETH_TYPE_CFM, sizeof *ccm);
             cfm_compose_ccm(ofport->cfm, ccm);
             send_packet(ofproto_dpif_cast(ofport->up.ofproto),
-                        ofport->odp_port, 0, &packet);
+                        ofport->odp_port, &packet);
             ofpbuf_uninit(&packet);
         }
     }
@@ -1587,7 +1590,7 @@ handle_miss_upcall(struct ofproto_dpif *ofproto, struct dpif_upcall *upcall)
     /* Check with in-band control to see if this packet should be sent
      * to the local port regardless of the flow table. */
     if (connmgr_msg_in_hook(ofproto->up.connmgr, &flow, upcall->packet)) {
-        send_packet(ofproto, OFPP_LOCAL, 0, upcall->packet);
+        send_packet(ofproto, ODPP_LOCAL, upcall->packet);
     }
 
     facet = facet_lookup_valid(ofproto, &flow);
@@ -2126,6 +2129,12 @@ facet_install(struct ofproto_dpif *p, struct facet *facet, bool zero_stats)
     }
 }
 
+static int
+vlan_tci_to_openflow_vlan(ovs_be16 vlan_tci)
+{
+    return vlan_tci != htons(0) ? vlan_tci_to_vid(vlan_tci) : OFP_VLAN_NONE;
+}
+
 static void
 facet_account(struct ofproto_dpif *ofproto,
               struct facet *facet, uint64_t extra_bytes)
@@ -2135,6 +2144,7 @@ facet_account(struct ofproto_dpif *ofproto,
     const struct nlattr *a;
     tag_type dummy = 0;
     unsigned int left;
+    ovs_be16 vlan_tci;
     int vlan;
 
     total_bytes = facet->byte_count + extra_bytes;
@@ -2162,14 +2172,32 @@ facet_account(struct ofproto_dpif *ofproto,
     if (!ofproto->has_bonded_bundles) {
         return;
     }
+
+    /* This loop feeds byte counters to bond_account() for rebalancing to use
+     * as a basis.  We also need to track the actual VLAN on which the packet
+     * is going to be sent to ensure that it matches the one passed to
+     * bond_choose_output_slave().  (Otherwise, we will account to the wrong
+     * hash bucket.) */
+    vlan_tci = facet->flow.vlan_tci;
     NL_ATTR_FOR_EACH_UNSAFE (a, left, facet->actions, facet->actions_len) {
-        if (nl_attr_type(a) == ODP_ACTION_ATTR_OUTPUT) {
-            struct ofport_dpif *port;
+        struct ofport_dpif *port;
 
+        switch (nl_attr_type(a)) {
+        case ODP_ACTION_ATTR_OUTPUT:
             port = get_odp_port(ofproto, nl_attr_get_u32(a));
             if (port && port->bundle && port->bundle->bond) {
-                bond_account(port->bundle->bond, &facet->flow, vlan, n_bytes);
+                bond_account(port->bundle->bond, &facet->flow,
+                             vlan_tci_to_openflow_vlan(vlan_tci), n_bytes);
             }
+            break;
+
+        case ODP_ACTION_ATTR_STRIP_VLAN:
+            vlan_tci = htons(0);
+            break;
+
+        case ODP_ACTION_ATTR_SET_DL_TCI:
+            vlan_tci = nl_attr_get_be16(a);
+            break;
         }
     }
 }
@@ -2613,23 +2641,16 @@ rule_modify_actions(struct rule *rule_,
     return error;
 }
 \f
-/* Sends 'packet' out of port 'odp_port' within 'ofproto'.  If 'vlan_tci' is
- * zero the packet will not have any 802.1Q hader; if it is nonzero, then the
- * packet will be sent with the VLAN TCI specified by 'vlan_tci & ~VLAN_CFI'.
- *
+/* Sends 'packet' out of port 'odp_port' within 'p'.
  * Returns 0 if successful, otherwise a positive errno value. */
 static int
-send_packet(struct ofproto_dpif *ofproto, uint32_t odp_port, uint16_t vlan_tci,
+send_packet(struct ofproto_dpif *ofproto, uint32_t odp_port,
             const struct ofpbuf *packet)
 {
     struct ofpbuf odp_actions;
     int error;
 
     ofpbuf_init(&odp_actions, 32);
-    if (vlan_tci != 0) {
-        nl_msg_put_u32(&odp_actions, ODP_ACTION_ATTR_SET_DL_TCI,
-                       ntohs(vlan_tci & ~VLAN_CFI));
-    }
     nl_msg_put_u32(&odp_actions, ODP_ACTION_ATTR_OUTPUT, odp_port);
     error = dpif_execute(ofproto->dpif, odp_actions.data, odp_actions.size,
                          packet);
@@ -3526,7 +3547,7 @@ is_admissible(struct ofproto_dpif *ofproto, const struct flow *flow,
 
     /* Find the port and bundle for the received packet. */
     in_port = get_ofp_port(ofproto, flow->in_port);
-    *in_bundlep = in_bundle = in_port->bundle;
+    *in_bundlep = in_bundle = in_port ? in_port->bundle : NULL;
     if (!in_port || !in_bundle) {
         /* No interface?  Something fishy... */
         if (have_packet) {