netdev - Re: [PATCH net-next v3 8/8] geneve: break dependency to network drivers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAEh+42izmdn5hHBxQWfSQEduWE2OeY0puM-5DEU-iHTWYXdobQ@mail.gmail.com>
Date:	Fri, 8 Jan 2016 13:12:40 -0800
From:	Jesse Gross <jesse@...nel.org>
To:	Hannes Frederic Sowa <hannes@...essinduktion.org>
Cc:	Linux Kernel Network Developers <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next v3 8/8] geneve: break dependency to network drivers

On Fri, Jan 8, 2016 at 12:47 PM, Hannes Frederic Sowa
<hannes@...essinduktion.org> wrote:
> On 07.01.2016 01:18, Jesse Gross wrote:
>>
>> I think we should merge vxlan_get_rx_port() and geneve_get_rx_port()
>> into a single generic function. For drivers that support both, they
>> now have two calls to get notified for all offloaded ports. This
>> actually can cause problems related to duplicates, similar to what you
>> feared before.
>
>
> Checking out the ndo_add_vxlan_port callbacks, we cannot change how we
> refresh the ports unfortunately. For some drivers it might be easy to
> update, some look a little bit more complicated.
>
> I guess we should try to keep more complexity in the core and make sure we
> only call those functions if necessary for the particular type of offload
> than to replicate the logic in each driver.
>
> If you agree I prepare this series with the changes to have separate
> callbacks, address your other feedback and send it out.

I'm not sure that I totally understand, do you have an example of a
problematic driver?

I think what I was proposing is fairly simple, so maybe I wasn't
totally clear. Here's what I think we can do:

In your most recent series, vxlan_get_rx_port() and
geneve_get_rx_port() have the same implementation. I think we can
merge them into a single function (i.e. udp_tunnel_get_rx_port()). For
all of the drivers that only implement VXLAN offloads, this is just a
1:1 switch so there should be no functionality change. There's only
one driver than currently implements both (i40e) and it calls both
vxlan_get_rx_port() and geneve_get_rx_port() back to back. We can just
collapse this into a single call and since that will trigger both
offloads to refresh, the behavior should again be the same as we have.

I do know that there are so drivers that try to do clever things as
far as refcounting, etc. However, since the callbacks are the same as
we did before, it shouldn't introduce any issues.