[ovs-dev] [RFC PATCH 3/4] lib/ofpbuf: Remove 'l7' pointer.

[Jarno Rajahalme]

On Mar 24, 2014, at 3:38 PM, Ben Pfaff <blp at nicira.com> wrote:

> On Mon, Mar 24, 2014 at 10:59:05AM -0700, Jarno Rajahalme wrote:
>> Now that we don't need to parse TCP flags from the packet after
>> extraction, we usually do not need the 'l7' pointer any more.  When
>> needed, ofpbuf_get_tcp|udp|sctp|icmp_payload() or ofpbuf_get_l4_size()
>> can be used instead.
>> 
>> Removal of 'l7' was requested by Pravin for the DPDK datapath work, as
>> it simplifies packet parsing a bit.
>> 
>> Signed-off-by: Jarno Rajahalme <jrajahalme at nicira.com>
> 
> This makes me a little sad, because I like the abstraction.  But it is
> true that we don't use l7 much, so I guess it's OK to remove it to
> increase performance slightly.
> 

We needed l7 mostly to get TCP flags of every packet, but now that the flags are in the flow, there were only some minor use cases left. Also, most of them were satisfied with less powerful ofpbuf_get_l4_size(), which does not need to parse the exact beginning of the l7 payload.

> Is bfd_process_packet() unsafe?  I think that it should check that l7
> is nonnull.

It seems to me that NULL l7 should cause ofpbuf_at to return NULL, since the offset argument is likely to be huge. So that should not be a problem. I added an explicit check for !l7 for master, though. Do you think that should be backported?

> 
> Acked-by: Ben Pfaff <blp at nicira.com>

Pushed ot master,

  Jarno