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
[off-list ref] wrote:

On 07.01.2016 01:18, Jesse Gross wrote:

quoted

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.