On Mon, Mar 21, 2011 at 05:29:25PM -0700, Ethan Jackson wrote:
> > + * If 'rw_packet' is nonnull, then it must contain the same data as
> > + * upcall->packet. ?'rw_packet' is allowed to be the same ofpbuf as
> > + * upcall->packet. ?It is modified in-place into an OFPT_PACKET_IN message
> > + * according to 'pin', and then ofputil_encode_packet_in() returns 
> > 'rw_packet'.
> > + * If 'rw_packet' has enough headroom to insert a "struct ofp_packet_in", 
> > this
> > + * is more efficient than ofputil_encode_packet_in() because it does not 
> > copy
> > + * the packet payload. */
> 
> I think this comment could use a short explanation of what upcall is,
> or a reference to where one would find out.  I was a bit confused when
> first reading it.

This is a cut-and-paste error.  Actually each instance of 'upcall'
here should be 'pin', which is the name of the function parameter.

> > +struct ofpbuf *
> > +ofputil_encode_packet_in(const struct ofputil_packet_in *pin,
> > + ? ? ? ? ? ? ? ? ? ? ? ?struct ofpbuf *rw_packet)
> > +{
> > + ? ?int total_len = pin->packet->size;
> > + ? ?struct ofp_packet_in *opi;
> > +
> > + ? ?if (rw_packet) {
> > + ? ? ? ?if (pin->send_len < rw_packet->size) {
> > + ? ? ? ? ? ?rw_packet->size = pin->send_len;
> > + ? ? ? ?}
> > + ? ?} else {
> > + ? ? ? ?rw_packet = ofpbuf_clone_data_with_headroom(
> > + ? ? ? ? ? ?pin->packet->data, MIN(pin->send_len, pin->packet->size),
> > + ? ? ? ? ? ?offsetof(struct ofp_packet_in, data));
> > + ? ?}
> > +
> > +
> 
> Extra newline here.

Fixed, thank you.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to