[ovs-dev] [PATCH 7/9] tunneling: Add userspace tunnel support for Geneve.

[Pravin Shelar]

On Tue, Apr 7, 2015 at 4:59 PM, Jesse Gross <jesse at nicira.com> wrote:
> On Tue, Apr 7, 2015 at 4:22 PM, Pravin Shelar <pshelar at nicira.com> wrote:
>> On Mon, Mar 30, 2015 at 3:14 PM, Jesse Gross <jesse at nicira.com> wrote:
>>> diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
>>> index 0c9f5a4..ef96862 100644
>>> --- a/lib/netdev-vport.c
>>> +++ b/lib/netdev-vport.c
>>> @@ -61,6 +61,11 @@ static struct vlog_rate_limit err_rl = VLOG_RATE_LIMIT_INIT(60, 5);
>>>                        sizeof(struct udp_header) +         \
>>>                        sizeof(struct vxlanhdr))
>>>
>> ...

>>> +netdev_geneve_pop_header(struct netdev *netdev_ OVS_UNUSED,
>> ....
>>
>>> +
>>> +static int
>>> +netdev_geneve_push_header(const struct netdev *netdev OVS_UNUSED,
>>> +                          struct dp_packet **packets, int cnt,
>>> +                          const struct ovs_action_push_tnl *data)
>>> +{
>>> +    int i;
>>> +
>>> +    for (i = 0; i < cnt; i++) {
>>> +        push_udp_header(packets[i], data->header, data->header_len);
>>> +        packets[i]->md = PKT_METADATA_INITIALIZER(u32_to_odp(data->out_port));
>>> +    }
>>> +    return 0;
>>> +}
>>> +
>> This looks like vxlan_push, Is there reason for having two different function?
>
> Looking a little bigger, GRE also has pretty much the same function;
> the only difference being that it calls a GRE function inside the
> loop. Similarly all three pop functions are basically the same except
> with a different inner function. I think we can avoid all of this by
> making the netdev functions provide a loop for us, which is a little
> less generic but is a little cleaner in practice. Any reason to not do
> this?
>

At this point I do not see them changing. So lets refactor this code.