Re: [ovs-dev] [PATCH 09/41] ofp-util: Prepare Packet Out decoder for other Open Flow versions
On Wed, Aug 08, 2012 at 05:00:02PM -0700, Ben Pfaff wrote: > On Thu, Aug 09, 2012 at 08:56:10AM +0900, Simon Horman wrote: > > On Wed, Aug 08, 2012 at 04:42:30PM -0700, Ben Pfaff wrote: > > > On Wed, Aug 08, 2012 at 06:49:44AM +0900, Simon Horman wrote: > > > > Signed-off-by: Simon Horman > > > > { > > > > -const struct ofp_packet_out *opo; > > > > -enum ofperr error; > > > > enum ofpraw raw; > > > > struct ofpbuf b; > > > > +enum ofperr bad_in_port_err = OFPERR_OFPET_BAD_REQUEST; > > > > > > > > ofpbuf_use_const(&b, oh, ntohs(oh->length)); > > > > raw = ofpraw_pull_assert(&b); > > > > -assert(raw == OFPRAW_OFPT10_PACKET_OUT); > > > > > > > > -opo = ofpbuf_pull(&b, sizeof *opo); > > > > -po->buffer_id = ntohl(opo->buffer_id); > > > > -po->in_port = ntohs(opo->in_port); > > > > +if (raw == OFPRAW_OFPT10_PACKET_OUT) { > > > > +enum ofperr error; > > > > +const struct ofp_packet_out *opo = ofpbuf_pull(&b, sizeof > > > > *opo); > > > > + > > > > +po->buffer_id = ntohl(opo->buffer_id); > > > > +po->in_port = ntohs(opo->in_port); > > > > + > > > > +error = ofpacts_pull_openflow10(&b, ntohs(opo->actions_len), > > > > ofpacts); > > > > +if (error) { > > > > +return error; > > > > +} > > > > +bad_in_port_err = OFPERR_NXBRC_BAD_IN_PORT; > > > > +} else { > > > > +NOT_REACHED(); > > > > +} > > > > > > I think that we should consistently use OFPERR_NXBRC_BAD_IN_PORT. > > > OFPERR_OFPET_BAD_REQUEST isn't a good substitute because it isn't > > > encodable as an error at all (it is an error category, not an error > > > itself). > > > > > > If there's a better choice for a standard error code in some OpenFlow > > > version then I'm happy to use that. > > > > I'm comfortable with using OFPERR_NXBRC_BAD_IN_PORT if you are. > > Thanks. > > This is what I ended up with: Thanks, looks good. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 09/41] ofp-util: Prepare Packet Out decoder for other Open Flow versions
On Thu, Aug 09, 2012 at 08:56:10AM +0900, Simon Horman wrote: > On Wed, Aug 08, 2012 at 04:42:30PM -0700, Ben Pfaff wrote: > > On Wed, Aug 08, 2012 at 06:49:44AM +0900, Simon Horman wrote: > > > Signed-off-by: Simon Horman > > > { > > > -const struct ofp_packet_out *opo; > > > -enum ofperr error; > > > enum ofpraw raw; > > > struct ofpbuf b; > > > +enum ofperr bad_in_port_err = OFPERR_OFPET_BAD_REQUEST; > > > > > > ofpbuf_use_const(&b, oh, ntohs(oh->length)); > > > raw = ofpraw_pull_assert(&b); > > > -assert(raw == OFPRAW_OFPT10_PACKET_OUT); > > > > > > -opo = ofpbuf_pull(&b, sizeof *opo); > > > -po->buffer_id = ntohl(opo->buffer_id); > > > -po->in_port = ntohs(opo->in_port); > > > +if (raw == OFPRAW_OFPT10_PACKET_OUT) { > > > +enum ofperr error; > > > +const struct ofp_packet_out *opo = ofpbuf_pull(&b, sizeof *opo); > > > + > > > +po->buffer_id = ntohl(opo->buffer_id); > > > +po->in_port = ntohs(opo->in_port); > > > + > > > +error = ofpacts_pull_openflow10(&b, ntohs(opo->actions_len), > > > ofpacts); > > > +if (error) { > > > +return error; > > > +} > > > +bad_in_port_err = OFPERR_NXBRC_BAD_IN_PORT; > > > +} else { > > > +NOT_REACHED(); > > > +} > > > > I think that we should consistently use OFPERR_NXBRC_BAD_IN_PORT. > > OFPERR_OFPET_BAD_REQUEST isn't a good substitute because it isn't > > encodable as an error at all (it is an error category, not an error > > itself). > > > > If there's a better choice for a standard error code in some OpenFlow > > version then I'm happy to use that. > > I'm comfortable with using OFPERR_NXBRC_BAD_IN_PORT if you are. Thanks. This is what I ended up with: --8<--cut here-->8-- From: Simon Horman Date: Wed, 8 Aug 2012 06:49:44 +0900 Subject: [PATCH] ofp-util: Prepare Packet Out decoder for other Open Flow versions Signed-off-by: Simon Horman Signed-off-by: Ben Pfaff --- lib/ofp-util.c | 25 +++-- 1 files changed, 15 insertions(+), 10 deletions(-) diff --git a/lib/ofp-util.c b/lib/ofp-util.c index 4632482..64eb5ca 100644 --- a/lib/ofp-util.c +++ b/lib/ofp-util.c @@ -2149,18 +2149,27 @@ ofputil_decode_packet_out(struct ofputil_packet_out *po, const struct ofp_header *oh, struct ofpbuf *ofpacts) { -const struct ofp_packet_out *opo; -enum ofperr error; enum ofpraw raw; struct ofpbuf b; ofpbuf_use_const(&b, oh, ntohs(oh->length)); raw = ofpraw_pull_assert(&b); -assert(raw == OFPRAW_OFPT10_PACKET_OUT); -opo = ofpbuf_pull(&b, sizeof *opo); -po->buffer_id = ntohl(opo->buffer_id); -po->in_port = ntohs(opo->in_port); +if (raw == OFPRAW_OFPT10_PACKET_OUT) { +enum ofperr error; +const struct ofp_packet_out *opo = ofpbuf_pull(&b, sizeof *opo); + +po->buffer_id = ntohl(opo->buffer_id); +po->in_port = ntohs(opo->in_port); + +error = ofpacts_pull_openflow10(&b, ntohs(opo->actions_len), ofpacts); +if (error) { +return error; +} +} else { +NOT_REACHED(); +} + if (po->in_port >= OFPP_MAX && po->in_port != OFPP_LOCAL && po->in_port != OFPP_NONE && po->in_port != OFPP_CONTROLLER) { VLOG_WARN_RL(&bad_ofmsg_rl, "packet-out has bad input port %#"PRIx16, @@ -2168,10 +2177,6 @@ ofputil_decode_packet_out(struct ofputil_packet_out *po, return OFPERR_NXBRC_BAD_IN_PORT; } -error = ofpacts_pull_openflow10(&b, ntohs(opo->actions_len), ofpacts); -if (error) { -return error; -} po->ofpacts = ofpacts->data; po->ofpacts_len = ofpacts->size; -- 1.7.2.5 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 09/41] ofp-util: Prepare Packet Out decoder for other Open Flow versions
On Wed, Aug 08, 2012 at 04:42:30PM -0700, Ben Pfaff wrote: > On Wed, Aug 08, 2012 at 06:49:44AM +0900, Simon Horman wrote: > > Signed-off-by: Simon Horman > > { > > -const struct ofp_packet_out *opo; > > -enum ofperr error; > > enum ofpraw raw; > > struct ofpbuf b; > > +enum ofperr bad_in_port_err = OFPERR_OFPET_BAD_REQUEST; > > > > ofpbuf_use_const(&b, oh, ntohs(oh->length)); > > raw = ofpraw_pull_assert(&b); > > -assert(raw == OFPRAW_OFPT10_PACKET_OUT); > > > > -opo = ofpbuf_pull(&b, sizeof *opo); > > -po->buffer_id = ntohl(opo->buffer_id); > > -po->in_port = ntohs(opo->in_port); > > +if (raw == OFPRAW_OFPT10_PACKET_OUT) { > > +enum ofperr error; > > +const struct ofp_packet_out *opo = ofpbuf_pull(&b, sizeof *opo); > > + > > +po->buffer_id = ntohl(opo->buffer_id); > > +po->in_port = ntohs(opo->in_port); > > + > > +error = ofpacts_pull_openflow10(&b, ntohs(opo->actions_len), > > ofpacts); > > +if (error) { > > +return error; > > +} > > +bad_in_port_err = OFPERR_NXBRC_BAD_IN_PORT; > > +} else { > > +NOT_REACHED(); > > +} > > I think that we should consistently use OFPERR_NXBRC_BAD_IN_PORT. > OFPERR_OFPET_BAD_REQUEST isn't a good substitute because it isn't > encodable as an error at all (it is an error category, not an error > itself). > > If there's a better choice for a standard error code in some OpenFlow > version then I'm happy to use that. I'm comfortable with using OFPERR_NXBRC_BAD_IN_PORT if you are. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 09/41] ofp-util: Prepare Packet Out decoder for other Open Flow versions
On Wed, Aug 08, 2012 at 06:49:44AM +0900, Simon Horman wrote: > Signed-off-by: Simon Horman > { > -const struct ofp_packet_out *opo; > -enum ofperr error; > enum ofpraw raw; > struct ofpbuf b; > +enum ofperr bad_in_port_err = OFPERR_OFPET_BAD_REQUEST; > > ofpbuf_use_const(&b, oh, ntohs(oh->length)); > raw = ofpraw_pull_assert(&b); > -assert(raw == OFPRAW_OFPT10_PACKET_OUT); > > -opo = ofpbuf_pull(&b, sizeof *opo); > -po->buffer_id = ntohl(opo->buffer_id); > -po->in_port = ntohs(opo->in_port); > +if (raw == OFPRAW_OFPT10_PACKET_OUT) { > +enum ofperr error; > +const struct ofp_packet_out *opo = ofpbuf_pull(&b, sizeof *opo); > + > +po->buffer_id = ntohl(opo->buffer_id); > +po->in_port = ntohs(opo->in_port); > + > +error = ofpacts_pull_openflow10(&b, ntohs(opo->actions_len), > ofpacts); > +if (error) { > +return error; > +} > +bad_in_port_err = OFPERR_NXBRC_BAD_IN_PORT; > +} else { > +NOT_REACHED(); > +} I think that we should consistently use OFPERR_NXBRC_BAD_IN_PORT. OFPERR_OFPET_BAD_REQUEST isn't a good substitute because it isn't encodable as an error at all (it is an error category, not an error itself). If there's a better choice for a standard error code in some OpenFlow version then I'm happy to use that. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev