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

On Fri, Jan 8, 2016 at 1:18 PM, Hannes Frederic Sowa
[off-list ref] wrote:

On 08.01.2016 22:12, Jesse Gross wrote:

quoted

On Fri, Jan 8, 2016 at 12:47 PM, Hannes Frederic Sowa
[off-list ref] wrote:

quoted

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?

bnx2x e.g. does refcounting on the port. So when it will implement geneve
offloading, it will refcount the vxlan port multiple times with the current
schema. Currently this is safe, as bnx2x doesn't care about geneve and thus
no further callbacks will happen. But I fear drivers being broken as soon as
they implement geneve additionally with the current schema.

OK, I understand what you mean now. I guess having a combined callback
function still seems cleaner to me overall and the exceptions are
relatively few, so I would probably continue to lean towards this
direction. In any case, I think it's hard for the core network stack
to completely handle the complexity as drivers can have various
restrictions on the number of ports, etc. that they support for
offloading.