Re: [PATCH net] openvswitch: compute needed headroom for internal vports

[Jesse Gross <jesse@kernel.org>]

On Tue, Jan 12, 2016 at 12:44 PM, pravin shelar <pshelar@ovn.org> wrote:
> On Tue, Jan 12, 2016 at 11:20 AM, Jesse Gross <jesse@kernel.org> wrote:
>> On Mon, Jan 11, 2016 at 4:34 PM, pravin shelar <pshelar@ovn.org> wrote:
>>> On Mon, Jan 11, 2016 at 12:43 AM, Paolo Abeni <pabeni@redhat.com> wrote:
>>>> On Fri, 2016-01-08 at 18:44 -0800, pravin shelar wrote:
>>>>> On Fri, Jan 8, 2016 at 4:42 PM, Jesse Gross <jesse@kernel.org> wrote:
>>>>> > On Fri, Jan 8, 2016 at 2:53 PM, pravin shelar <pshelar@ovn.org> wrote:
>>>>> >> On Fri, Jan 8, 2016 at 1:50 PM, Paolo Abeni <pabeni@redhat.com> wrote:
>>>>> >>> Currently the ovs internal vports always use a default needed_headroom.
>>>>> >>> This leads to a skb head copy while xmitting on ovs swith via vport
>>>>> >>> that add some kind of encapsulation (gre, geneve, etc.).
>>>>> >>>
>>>>> >>> This patch add book-keeping for the maximum needed_headroom used by
>>>>> >>> the non internal vports in any dp, updating it on vport creation and
>>>>> >>> deletion.
>>>>> >>>
>>>>> >>> Said value is than used as needed_headroom for internal vports,
>>>>> >>> avoiding the above copy.
>>>>> >>>
>>>>> >> Why is this done only for internal devices? In most common case of
>>>>> >> traffic the packet enters OVS from tap or other netdev type vport
>>>>> >> device.
>>>>> >
>>>>> > How would you influence the allocation for non-internal devices?
>>>>>
>>>>> Today there is no way of influencing this. But we could add new
>>>>> skb-headroom parameter to netdev for packets that are received on the
>>>>> device. This new parameter could be controlled from master devices
>>>>> like OVS, Bridge, etc. To set this value we need new ndo operation. So
>>>>> that it can work on devices like tap where it would just set this new
>>>>> value and in case of ovs-internal or veth device, it can also update
>>>>> needed_headroom.
>>>>
>>>> My idea was to continue working along this lines.
>>>>
>>>> However I thought to get there incrementally, i.e. handle internal
>>>> vports only first. Can this be ok for you?
>>>>
>>>
>>> If the final implementation is going to change alot, then I do not see
>>> much value in this change going in first.
>>
>> Even if the code will change in the future, it seems like an
>> incremental improvement that will help in some cases so I don't see
>> much reason to not do this part now.
>
> I am not sure which cases it help. Can you tell me use cases for
> internal port in production?

Any traffic coming from the hypervisor itself (as well as tunnels
although unless you are doing double encapsulation then this patch
doesn't matter in that case).

Since we're in the merge window now, maybe it makes sense to just go
for the full version in the next cycle in any case.