Thanks.

On Tue, Oct 08, 2013 at 04:05:25PM -0700, Alex Wang wrote:
> Looks good to me, patches 1/13 - 9/13 are all good to me,
> 
> 
> On Tue, Oct 8, 2013 at 3:57 PM, Ben Pfaff <b...@nicira.com> wrote:
> 
> > Thanks, I folded in this incremental:
> >
> > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> > index 9a9f841..4d01ec9 100644
> > --- a/ofproto/ofproto-dpif.c
> > +++ b/ofproto/ofproto-dpif.c
> > @@ -3838,22 +3838,20 @@ facet_free(struct facet *facet)
> >
> >  /* Executes, within 'ofproto', the 'n_actions' actions in 'actions' on
> >   * 'packet', which arrived on 'in_port'. */
> > -static bool
> > +static int
> >  execute_odp_actions(struct ofproto_dpif *ofproto, const struct flow *flow,
> >                      const struct nlattr *odp_actions, size_t actions_len,
> >                      struct ofpbuf *packet)
> >  {
> >      struct odputil_keybuf keybuf;
> >      struct ofpbuf key;
> > -    int error;
> >
> >      ofpbuf_use_stack(&key, &keybuf, sizeof keybuf);
> >      odp_flow_key_from_flow(&key, flow,
> >                             ofp_port_to_odp_port(ofproto,
> > flow->in_port.ofp_port));
> >
> > -    error = dpif_execute(ofproto->backer->dpif, key.data, key.size,
> > -                         odp_actions, actions_len, packet);
> > -    return !error;
> > +    return dpif_execute(ofproto->backer->dpif, key.data, key.size,
> > +                        odp_actions, actions_len, packet);
> >  }
> >
> >  /* Remove 'facet' from its ofproto and free up the associated memory:
> >
> >
> > On Tue, Oct 08, 2013 at 03:22:31PM -0700, Alex Wang wrote:
> > > This patch breaks several unit tests, because, execute_odp_actions()
> > > returns "!error".  we should just return error.
> > >
> > > All other changes looks good to me,
> > >
> > >
> > > On Mon, Sep 23, 2013 at 10:49 AM, Ben Pfaff <b...@nicira.com> wrote:
> > >
> > > > Signed-off-by: Ben Pfaff <b...@nicira.com>
> > > > ---
> > > >  ofproto/ofproto-dpif.c |   27 ++++++++-------------------
> > > >  1 file changed, 8 insertions(+), 19 deletions(-)
> > > >
> > > > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> > > > index 80874b8..93db491 100644
> > > > --- a/ofproto/ofproto-dpif.c
> > > > +++ b/ofproto/ofproto-dpif.c
> > > > @@ -4950,9 +4950,8 @@ send_packet(const struct ofport_dpif *ofport,
> > struct
> > > > ofpbuf *packet)
> > > >  {
> > > >      struct ofproto_dpif *ofproto =
> > ofproto_dpif_cast(ofport->up.ofproto);
> > > >      uint64_t odp_actions_stub[1024 / 8];
> > > > -    struct ofpbuf key, odp_actions;
> > > > +    struct ofpbuf odp_actions;
> > > >      struct dpif_flow_stats stats;
> > > > -    struct odputil_keybuf keybuf;
> > > >      struct ofpact_output output;
> > > >      struct xlate_out xout;
> > > >      struct xlate_in xin;
> > > > @@ -4961,13 +4960,10 @@ send_packet(const struct ofport_dpif *ofport,
> > > > struct ofpbuf *packet)
> > > >      int error;
> > > >
> > > >      ofpbuf_use_stub(&odp_actions, odp_actions_stub, sizeof
> > > > odp_actions_stub);
> > > > -    ofpbuf_use_stack(&key, &keybuf, sizeof keybuf);
> > > >
> > > >      /* Use OFPP_NONE as the in_port to avoid special packet
> > processing. */
> > > >      in_port_.ofp_port = OFPP_NONE;
> > > >      flow_extract(packet, 0, 0, NULL, &in_port_, &flow);
> > > > -    odp_flow_key_from_flow(&key, &flow, ofp_port_to_odp_port(ofproto,
> > > > -
> > OFPP_LOCAL));
> > > >      dpif_flow_stats_extract(&flow, packet, time_msec(), &stats);
> > > >
> > > >      ofpact_init(&output.ofpact, OFPACT_OUTPUT, sizeof output);
> > > > @@ -4980,10 +4976,11 @@ send_packet(const struct ofport_dpif *ofport,
> > > > struct ofpbuf *packet)
> > > >      xin.resubmit_stats = &stats;
> > > >      xlate_actions(&xin, &xout);
> > > >
> > > > -    error = dpif_execute(ofproto->backer->dpif,
> > > > -                         key.data, key.size,
> > > > -                         xout.odp_actions.data, xout.odp_actions.size,
> > > > -                         packet);
> > > > +    /* The kernel, however, doesn't know about OFPP_NONE.  Use a real
> > > > port. */
> > > > +    flow.in_port.ofp_port = OFPP_LOCAL;
> > > > +    error = execute_odp_actions(ofproto, &flow,
> > > > +                                xout.odp_actions.data,
> > > > xout.odp_actions.size,
> > > > +                                packet);
> > > >      xlate_out_uninit(&xout);
> > > >
> > > >      if (error) {
> > > > @@ -5056,17 +5053,9 @@ packet_out(struct ofproto *ofproto_, struct
> > ofpbuf
> > > > *packet,
> > > >             const struct ofpact *ofpacts, size_t ofpacts_len)
> > > >  {
> > > >      struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
> > > > -    struct odputil_keybuf keybuf;
> > > >      struct dpif_flow_stats stats;
> > > >      struct xlate_out xout;
> > > >      struct xlate_in xin;
> > > > -    struct ofpbuf key;
> > > > -
> > > > -
> > > > -    ofpbuf_use_stack(&key, &keybuf, sizeof keybuf);
> > > > -    odp_flow_key_from_flow(&key, flow,
> > > > -                           ofp_port_to_odp_port(ofproto,
> > > > -                                      flow->in_port.ofp_port));
> > > >
> > > >      dpif_flow_stats_extract(flow, packet, time_msec(), &stats);
> > > >
> > > > @@ -5076,8 +5065,8 @@ packet_out(struct ofproto *ofproto_, struct
> > ofpbuf
> > > > *packet,
> > > >      xin.ofpacts = ofpacts;
> > > >
> > > >      xlate_actions(&xin, &xout);
> > > > -    dpif_execute(ofproto->backer->dpif, key.data, key.size,
> > > > -                 xout.odp_actions.data, xout.odp_actions.size,
> > packet);
> > > > +    execute_odp_actions(ofproto, flow,
> > > > +                        xout.odp_actions.data, xout.odp_actions.size,
> > > > packet);
> > > >      xlate_out_uninit(&xout);
> > > >
> > > >      return 0;
> > > > --
> > > > 1.7.10.4
> > > >
> > > > _______________________________________________
> > > > dev mailing list
> > > > dev@openvswitch.org
> > > > http://openvswitch.org/mailman/listinfo/dev
> > > >
> >
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to