[ovs-dev] [PATCH] dpif-linux: Zero 'stats' outputs of dpif_operate() ops on failure.
Ben Pfaff
blp at nicira.com
Mon Jun 25 23:45:36 UTC 2012
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 <jschmidt at nicira.com>
> > Signed-off-by: Ben Pfaff <blp at nicira.com>
> > ---
> > 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
> > dev at openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
>
More information about the dev
mailing list