I’ll also change all the “getter” functions to be spelled without the “get”, as
done by Pravin for his patch earlier today on ofpbufs.
Unless there are other comments, I’ll not send a new patch for review, as this
is not affecting functionality in any way.
Jarno
On Apr 2, 2014, at 11:33 AM, Jarno Rajahalme <jrajaha...@nicira.com> wrote:
>
> On Apr 2, 2014, at 11:23 AM, Lori Jakab <loja...@cisco.com> wrote:
>
>> On 4/2/14, 8:59 PM, Jarno Rajahalme wrote:
>>
>> [...]
>>
>>> diff --git a/lib/ofpbuf.h b/lib/ofpbuf.h
>>> index 8d1cb11..fa8a3f0 100644
>>> --- a/lib/ofpbuf.h
>>> +++ b/lib/ofpbuf.h
>>> @@ -36,26 +36,46 @@ enum OVS_PACKED_ENUM ofpbuf_source {
>>> };
>>> /* Buffer for holding arbitrary data. An ofpbuf is automatically
>>> reallocated
>>> - * as necessary if it grows too large for the available memory. */
>>> + * as necessary if it grows too large for the available memory.
>>> + *
>>> + * 'frame' and offset conventions:
>>> + *
>>> + * Network frames (aka "packets"): 'frame' MUST be set to the start of the
>>> + * packet, layer offsets MAY be set as appropriate for the packet.
>>> + * Additionally, we assume in many places that the 'frame' and 'data'
>>> are
>>> + * the same for packets.
>>> + *
>>> + * OpenFlow messages: 'frame' points to the start of the OpenFlow
>>> + * header, while 'l3_ofs' is the length of the OpenFlow header.
>>> + * When parsing, the 'data' will move past these, as data is being
>>> + * pulled from the OpenFlow message.
>>> + *
>>> + * Actions: When encoding OVS action lists, the 'frame' is used
>>> + * as a pointer to the beginning of the current action (see
>>> ofpact_put()).
>>> + *
>>> + * rconn: Reuses 'frame' as a private pointer while queuing.
>>> + */
>>> struct ofpbuf {
>>> void *base; /* First byte of allocated space. */
>>> uint32_t allocated; /* Number of bytes allocated. */
>>> uint32_t size; /* Number of bytes in use. */
>>> void *data; /* First byte actually in use. */
>>> - void *l2; /* Link-level header. */
>>> - uint16_t l2_5_ofs; /* MPLS label stack offset from l2, or
>>> + void *frame; /* Packet frame start, or NULL. */
>>> + uint16_t l2_5_ofs; /* MPLS label stack offset from 'packet',
>>> or
>>> * UINT16_MAX */
>>> - uint16_t l3_ofs; /* Network-level header offset from l2, or
>>> - * UINT16_MAX. */
>>> - uint16_t l4_ofs; /* Transport-level header offset from l2,
>>> or
>>> - UINT16_MAX. */
>>> + uint16_t l3_ofs; /* Network-level header offset from
>>> 'packet',
>>> + or UINT16_MAX. */
>>> + uint16_t l4_ofs; /* Transport-level header offset from
>>> 'packet',
>>> + or UINT16_MAX. */
>>
>> Shouldn't you say 'frame' in the comments instead of 'packet' (using quotes
>> usually refers to struct members) ?
>
> Yes I should. I’ll fix this before pushing to master.
>
> Thanks,
>
> Jarno
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
