Thanks, I'll push this in a minute.
On Mon, Jun 25, 2012 at 12:35:35AM -0400, Justin Pettit wrote: > Looks good to me. Thanks. > > --Justin > > > On Jun 20, 2012, at 1:55 PM, Ben Pfaff wrote: > > > When or DPIF_OP_FLOW_DEL operations failed, they left > > their 'stats' outputs uninitialized. For DPIF_OP_FLOW_DEL, this meant that > > the caller would read indeterminate data: > > > > Conditional jump or move depends on uninitialised value(s) > > at 0x805C1EB: subfacet_reset_dp_stats (ofproto-dpif.c:4410) > > by 0x80637D2: expire_batch (ofproto-dpif.c:3471) > > by 0x8066114: run (ofproto-dpif.c:3513) > > by 0x8059DF4: ofproto_run (ofproto.c:1035) > > by 0x8052E17: bridge_run (bridge.c:2005) > > by 0x8053F74: main (ovs-vswitchd.c:108) > > > > It's unusual for a delete operation to fail. The most common reason is an > > administrator running "ovs-dpctl del-flows". > > > > The only user of DPIF_OP_FLOW_PUT did not request stats, so this doesn't > > fix an actual bug for that case. > > > > Bug #11797. > > Reported-by: James Schmidt <[email protected]> > > Signed-off-by: Ben Pfaff <[email protected]> > > --- > > lib/dpif-linux.c | 34 ++++++++++++++++++++++++---------- > > 1 files changed, 24 insertions(+), 10 deletions(-) > > > > diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c > > index 62f6917..fcf6899 100644 > > --- a/lib/dpif-linux.c > > +++ b/lib/dpif-linux.c > > @@ -968,24 +968,38 @@ dpif_linux_operate__(struct dpif *dpif_, struct > > dpif_op **ops, size_t n_ops) > > switch (op->type) { > > case DPIF_OP_FLOW_PUT: > > put = &op->u.flow_put; > > - if (!op->error && put->stats) { > > - struct dpif_linux_flow reply; > > - > > - op->error = dpif_linux_flow_from_ofpbuf(&reply, > > txn->reply); > > + if (put->stats) { > > if (!op->error) { > > - dpif_linux_flow_get_stats(&reply, put->stats); > > + struct dpif_linux_flow reply; > > + > > + op->error = dpif_linux_flow_from_ofpbuf(&reply, > > + txn->reply); > > + if (!op->error) { > > + dpif_linux_flow_get_stats(&reply, put->stats); > > + } > > + } > > + > > + if (op->error) { > > + memset(put->stats, 0, sizeof *put->stats); > > } > > } > > break; > > > > case DPIF_OP_FLOW_DEL: > > del = &op->u.flow_del; > > - if (!op->error && del->stats) { > > - struct dpif_linux_flow reply; > > - > > - op->error = dpif_linux_flow_from_ofpbuf(&reply, > > txn->reply); > > + if (del->stats) { > > if (!op->error) { > > - dpif_linux_flow_get_stats(&reply, del->stats); > > + struct dpif_linux_flow reply; > > + > > + op->error = dpif_linux_flow_from_ofpbuf(&reply, > > + txn->reply); > > + if (!op->error) { > > + dpif_linux_flow_get_stats(&reply, del->stats); > > + } > > + } > > + > > + if (op->error) { > > + memset(del->stats, 0, sizeof *del->stats); > > } > > } > > break; > > -- > > 1.7.2.5 > > > > _______________________________________________ > > dev mailing list > > [email protected] > > http://openvswitch.org/mailman/listinfo/dev > _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
