[ovs-dev] [PATCH 2/2] ofproto-dpif: Fix bond accounting.
Ben Pfaff
blp at nicira.com
Wed May 18 23:40:27 UTC 2011
Calls to bond_account() and bond_choose_output_slave() had different ideas
for the vlan of a flow that did not have a tagged VLAN. The call to
bond_choose_output_slave() passed OFP_VLAN_NONE in this case, the call to
bond_account() passed 0. This meant that packets not on a VLAN weren't
accounted properly, which typically caused bond/show to show "0 kB load"
on active hashes. Obviously that broke rebalancing too.
I've verified that this fixes accounting. I haven't directly verified that
it fixes rebalancing, so it's possible that there is another issue too.
Reported-by: Michael Mao <mmao at nicira.com>
---
ofproto/ofproto-dpif.c | 31 ++++++++++++++++++++++++++++---
1 files changed, 28 insertions(+), 3 deletions(-)
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 4dec05d..714cdf6 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -2141,6 +2141,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)
@@ -2150,6 +2156,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;
@@ -2177,14 +2184,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;
}
}
}
--
1.7.4.4
More information about the dev
mailing list