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
