[ovs-dev] [async-msgs 05/13] ofp-util: Add struct ofputil_packet_out, helper functions, and use it all.

[Ben Pfaff]

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.