On Mon, Feb 06, 2012 at 01:59:05PM -0800, Ethan Jackson wrote:
> >
> > (Only meaningful if buffer_id == -1.) */
> > + /* Followed by:
> > + * - Exactly 'actions_len' bytes (possibly 0 bytes, and always a
> > multiple
> > + * of 8) containing actions.
> > + * - If 'buffer_id' != -1, packet data to fill out the remainder of
> > the
> > + * message length.
> > + */
> >
>
> I think you mean "If 'buffer_id' == -1".
Yes, thanks.
I decided to make things even clearer:
@@ -450,14 +450,14 @@ OFP_ASSERT(sizeof(union ofp_action) == 8);
/* Send packet (controller -> datapath). */
struct ofp_packet_out {
struct ofp_header header;
- ovs_be32 buffer_id; /* ID assigned by datapath (-1 if none). */
+ ovs_be32 buffer_id; /* ID assigned by datapath or UINT32_MAX. */
ovs_be16 in_port; /* Packet's input port (OFPP_NONE if none).
*/
ovs_be16 actions_len; /* Size of action array in bytes. */
/* Followed by:
* - Exactly 'actions_len' bytes (possibly 0 bytes, and always a multiple
* of 8) containing actions.
- * - If 'buffer_id' != -1, packet data to fill out the remainder of the
- * message length.
+ * - If 'buffer_id' == UINT32_MAX, packet data to fill out the remainder
+ * of the message length.
*/
};
OFP_ASSERT(sizeof(struct ofp_packet_out) == 16);
> > + if (po->in_port >= OFPP_MAX && po->in_port != OFPP_NONE) {
> > + VLOG_WARN_RL(&bad_ofmsg_rl, "packet-out has bad input port
> > %#"PRIx16,
> > + po->in_port);
> > + return OFPERR_NXBRC_BAD_IN_PORT;
> > + }
> >
>
> OFPP_LOCAL is also a valid input port.
Good catch, thank you. I added "&& po->in_port != OFPP_LOCAL", which
matches what was previously in handle_packet_out().
>
> > + if (po.in_port >= OFPP_MAX && po.in_port != OFPP_LOCAL
> > + && po.in_port != OFPP_NONE) {
> > return OFPERR_NXBRC_BAD_IN_PORT;
> > }
> >
>
> This check is now redundant since ofptuil_decode_packet_out() does the same
> validation.
Thank you, I removed this check from here.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
