[ovs-dev] [PATCH v2 10/15] netdev-vport: Introduce ip_build_header()

[pravin shelar]

On Fri, May 6, 2016 at 3:12 PM, Jesse Gross <jesse at kernel.org> wrote:
> On Thu, Apr 21, 2016 at 6:54 PM, Pravin B Shelar <pshelar at ovn.org> wrote:
>> diff --git a/lib/netdev-native-tnl.h b/lib/netdev-native-tnl.h
>> index dbe6bd0..a0dfa8c 100644
>> --- a/lib/netdev-native-tnl.h
>> +++ b/lib/netdev-native-tnl.h
>> @@ -82,6 +82,31 @@ ipv6_hdr(void *eth)
>>      return (void *)((char *)eth + sizeof (struct eth_header));
>>  }
>>
>> +static inline void *
>> +ip_build_header(struct ovs_action_push_tnl *data,
>> +                uint8_t next_proto,
>> +                unsigned int *hlen,
>> +                bool *is_ipv6)
>
> I would also prefix this function to indicate where it is coming from.
> I'm also not entirely sure that this should be an inline (especially
> after my next comment).
>
> There's a somewhat odd split between this function and
> tnl_port_build_header(). It seems like it would better to consolidate
> all of the IP header building here in a single place. I think it might
> also be better to split out the IP and Ethernet layers in the
> build/push/pop functions - even though they are always done together
> it is easier to understand from a layering perspective.

It it good idea to consolidate header build functionality in one place.
But I do not see how can we split out IP and Ether layer functionality
from push header function. So I have kept that part same.