[openib-general] [PATCH 00/12] ofed_1_2 - Neighbour update support

[Michael S. Tsirkin]

> > Could we miss events because skb has a desctructor?
> 
> Yes.  I looked through the ethernet drivers and didn't see anyone using
> destructors.  I thought perhaps this is ok for backports.  There are
> ways to address this issue:
> 
> 1) Enhance the current code to save off the original destructor function
> if it exists and put in ours.  Then when our function is called, we do
> our processing, then call the original destructor function.  We would
> need to save the original function ptr somewhere. 
> 
> 2) schedule the function to happen at a later time and hope the ARP
> subsystem has already updated the neigh table.  I opted against this
> approach because it doesn't ensure that the neigh entry was updated
> before we act on it.
> 
> > Can we just call the descructor function directly (this is what addr.c
> > did previously, and this apparently worked fine).
> 
> The original addr.c snoop code worked fine for IB address resolution and
> for the initial ARP resolution for iWARP devices, but not for notifying
> iWARP devices when a neighbour changes.  For instance, if the neighbour
> mac address changes, then the iWARP device needs to be notified so it
> can update its L2 table maintained in the device. 
> 
> We need to defer calling the destructor function until the ARP subsystem
> has processed this ARP packet.  Through testing, I saw that our snoop
> function gets called _before_ the ARP subsystem processes the ARP
> packet.  So the neighbour entry hasn't been updated yet.  Hooking via
> destructor calls our function _after_ the ARP subsystem has updated the
> neighbour.  So we can then lookup the neigh entry and do the callouts.

Not sure how do you mean all this. You do kfree_skb immediately in the
arp processing function. Will this not call the destructor directly?

Anyway, it seems too risky to change the code a lot now.
what I am concerned is that this could have broken working code.

To reduce the risk of problems for existing code,
I'd like to see something like the following:

	if (someone asked for notification on neighbour changes)
		do the destructor trick

	if (someone asked for notification on address resolution)
		call the destructor directly

Could you code this up please?

-- 
MST