[PATCH] net: Move destructor from neigh->ops to neigh_params

[PATCH] net: Move destructor from neigh->ops to neigh_params

[Roland Dreier]

This is a resend of a patch written by Michael S. Tsirkin
<mst@mellanox.co.il>.  I'd like to get an ACK or NAK of it from Dave
and other networking people, so that we can either merge it upstream
or try a different approach.  There definitely is a problem with
neighbour destructors that IP-over-IB is running into.

It would be good to know what the design was behind putting the
destructor method in neigh->ops in the first place.

Dave, if you want to merge this directly, that's fine.  Or I'm fine
with merging this through the IB tree if you'd prefer (if you want me
to do that, let me know if you think it's 2.6.16 material).

Thanks,
  Roland



struct neigh_ops currently has a destructor field, which no in-kernel
drivers outside of infiniband use.  The infiniband/ulp/ipoib in-tree
driver stashes some info in the neighbour structure (the results of
the second-stage lookup from ARP results to real link-level path), and
it uses neigh->ops->destructor to get a callback so it can clean up
this extra info when a neighbour is freed.  We've run into problems
with this: since the destructor is in an ops field that is shared
between neighbours that may belong to different net devices, there's
no way to set/clear it safely.

The following patch moves this field to neigh_parms where it can be
safely set, together with its twin neigh_setup.  Two additional
patches in the patch series update ipoib to use this new interface.

Signed-off-by: Michael S. Tsirkin <mst@mellanox.co.il>
Signed-off-by: Roland Dreier <rolandd@cisco.com>

---

diff --git a/include/net/neighbour.h b/include/net/neighbour.h
index 6fa9ae1..b0666d6 100644
--- a/include/net/neighbour.h
+++ b/include/net/neighbour.h
@@ -68,6 +68,7 @@ struct neigh_parms
 	struct net_device *dev;
 	struct neigh_parms *next;
 	int	(*neigh_setup)(struct neighbour *);
+	void	(*neigh_destructor)(struct neighbour *);
 	struct neigh_table *tbl;
 
 	void	*sysctl_table;
@@ -145,7 +146,6 @@ struct neighbour
 struct neigh_ops
 {
 	int			family;
-	void			(*destructor)(struct neighbour *);
 	void			(*solicit)(struct neighbour *, struct sk_buff*);
 	void			(*error_report)(struct neighbour *, struct sk_buff*);
 	int			(*output)(struct sk_buff*);
diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index e68700f..3489e23 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -586,8 +586,8 @@ void neigh_destroy(struct neighbour *nei
 			kfree(hh);
 	}
 
-	if (neigh->ops && neigh->ops->destructor)
-		(neigh->ops->destructor)(neigh);
+	if (neigh->parms->neigh_destructor)
+		(neigh->parms->neigh_destructor)(neigh);
 
 	skb_queue_purge(&neigh->arp_queue);
 
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c
index fd3f5c8..9588124 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
@@ -247,7 +247,6 @@ static void path_free(struct net_device 
 		if (neigh->ah)
 			ipoib_put_ah(neigh->ah);
 		*to_ipoib_neigh(neigh->neighbour) = NULL;
-		neigh->neighbour->ops->destructor = NULL;
 		kfree(neigh);
 	}
 
@@ -530,7 +529,6 @@ static void neigh_add_path(struct sk_buf
 err:
 	*to_ipoib_neigh(skb->dst->neighbour) = NULL;
 	list_del(&neigh->list);
-	neigh->neighbour->ops->destructor = NULL;
 	kfree(neigh);
 
 	++priv->stats.tx_dropped;
@@ -769,21 +767,9 @@ static void ipoib_neigh_destructor(struc
 		ipoib_put_ah(ah);
 }
 
-static int ipoib_neigh_setup(struct neighbour *neigh)
-{
-	/*
-	 * Is this kosher?  I can't find anybody in the kernel that
-	 * sets neigh->destructor, so we should be able to set it here
-	 * without trouble.
-	 */
-	neigh->ops->destructor = ipoib_neigh_destructor;
-
-	return 0;
-}
-
 static int ipoib_neigh_setup_dev(struct net_device *dev, struct neigh_parms *parms)
 {
-	parms->neigh_setup = ipoib_neigh_setup;
+	parms->neigh_destructor = ipoib_neigh_destructor;
 
 	return 0;
 }

Re: [PATCH] net: Move destructor from neigh->ops to neigh_params

[David S. Miller]

From: Roland Dreier <rdreier@cisco.com>
Date: Mon, 23 Jan 2006 13:27:32 -0800

> I'd like to get an ACK or NAK of it from Dave

Dave is in New Zealand at linux.conf.au, don't expect him to
be too active for at least a week...

[PATCH RESEND] net: Move destructor from neigh->ops to neigh_params

[Roland Dreier]

Sorry to distract everyone from the VJ channel discussion, but on the
other hand it looks like Dave is back... I'm resending this because
I'd really like to get this problem fixed but I want to make sure
we're doing it the right way.  So either an ACK or a NAK with guidance
would be great...


This is a resend of a patch written by Michael S. Tsirkin
<mst@mellanox.co.il>.  I'd like to get an ACK or NAK of it from Dave
and other networking people, so that we can either merge it upstream
or try a different approach.  There definitely is a problem with
neighbour destructors that IP-over-IB is running into.

It would be good to know what the design was behind putting the
destructor method in neigh->ops in the first place.

Dave, if you want to merge this directly, that's fine.  Or I'm fine
with merging this through the IB tree if you'd prefer (if you want me
to do that, let me know if you think it's 2.6.16 material).

Thanks,
  Roland



struct neigh_ops currently has a destructor field, which no in-kernel
drivers outside of infiniband use.  The infiniband/ulp/ipoib in-tree
driver stashes some info in the neighbour structure (the results of
the second-stage lookup from ARP results to real link-level path), and
it uses neigh->ops->destructor to get a callback so it can clean up
this extra info when a neighbour is freed.  We've run into problems
with this: since the destructor is in an ops field that is shared
between neighbours that may belong to different net devices, there's
no way to set/clear it safely.

The following patch moves this field to neigh_parms where it can be
safely set, together with its twin neigh_setup.  Two additional
patches in the patch series update ipoib to use this new interface.

Signed-off-by: Michael S. Tsirkin <mst@mellanox.co.il>
Signed-off-by: Roland Dreier <rolandd@cisco.com>

---

diff --git a/include/net/neighbour.h b/include/net/neighbour.h
index 6fa9ae1..b0666d6 100644
--- a/include/net/neighbour.h
+++ b/include/net/neighbour.h
@@ -68,6 +68,7 @@ struct neigh_parms
 	struct net_device *dev;
 	struct neigh_parms *next;
 	int	(*neigh_setup)(struct neighbour *);
+	void	(*neigh_destructor)(struct neighbour *);
 	struct neigh_table *tbl;
 
 	void	*sysctl_table;
@@ -145,7 +146,6 @@ struct neighbour
 struct neigh_ops
 {
 	int			family;
-	void			(*destructor)(struct neighbour *);
 	void			(*solicit)(struct neighbour *, struct sk_buff*);
 	void			(*error_report)(struct neighbour *, struct sk_buff*);
 	int			(*output)(struct sk_buff*);
diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index e68700f..3489e23 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -586,8 +586,8 @@ void neigh_destroy(struct neighbour *nei
 			kfree(hh);
 	}
 
-	if (neigh->ops && neigh->ops->destructor)
-		(neigh->ops->destructor)(neigh);
+	if (neigh->parms->neigh_destructor)
+		(neigh->parms->neigh_destructor)(neigh);
 
 	skb_queue_purge(&neigh->arp_queue);
 
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c
index fd3f5c8..9588124 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
@@ -247,7 +247,6 @@ static void path_free(struct net_device 
 		if (neigh->ah)
 			ipoib_put_ah(neigh->ah);
 		*to_ipoib_neigh(neigh->neighbour) = NULL;
-		neigh->neighbour->ops->destructor = NULL;
 		kfree(neigh);
 	}
 
@@ -530,7 +529,6 @@ static void neigh_add_path(struct sk_buf
 err:
 	*to_ipoib_neigh(skb->dst->neighbour) = NULL;
 	list_del(&neigh->list);
-	neigh->neighbour->ops->destructor = NULL;
 	kfree(neigh);
 
 	++priv->stats.tx_dropped;
@@ -769,21 +767,9 @@ static void ipoib_neigh_destructor(struc
 		ipoib_put_ah(ah);
 }
 
-static int ipoib_neigh_setup(struct neighbour *neigh)
-{
-	/*
-	 * Is this kosher?  I can't find anybody in the kernel that
-	 * sets neigh->destructor, so we should be able to set it here
-	 * without trouble.
-	 */
-	neigh->ops->destructor = ipoib_neigh_destructor;
-
-	return 0;
-}
-
 static int ipoib_neigh_setup_dev(struct net_device *dev, struct neigh_parms *parms)
 {
-	parms->neigh_setup = ipoib_neigh_setup;
+	parms->neigh_destructor = ipoib_neigh_destructor;
 
 	return 0;
 }

Re: [PATCH RESEND] net: Move destructor from neigh->ops to neigh_params

[David S. Miller]

From: Roland Dreier <rdreier@cisco.com>
Date: Wed, 01 Feb 2006 17:28:10 -0800

> Sorry to distract everyone from the VJ channel discussion, but on the
> other hand it looks like Dave is back... I'm resending this because
> I'd really like to get this problem fixed but I want to make sure
> we're doing it the right way.  So either an ACK or a NAK with guidance
> would be great...

It's sitting in my backlog, and will be a net-2.6.17 issue not
a net-2.6.16 one as we're in bug fix mode there.

Sorry if you need this in 2.6.16, but that's not really practical.

Re: [PATCH RESEND] net: Move destructor from neigh->ops to neigh_params

[Roland Dreier]

    David> It's sitting in my backlog, and will be a net-2.6.17 issue
    David> not a net-2.6.16 one as we're in bug fix mode there.

    David> Sorry if you need this in 2.6.16, but that's not really
    David> practical.

No, that's fine... I was just resending in case you were using RED to
manage your backlog.  This is a real issue but I don't think it's
hitting a lot of people.

 - R.

Re: [PATCH RESEND] net: Move destructor from neigh->ops to neigh_params

[YOSHIFUJI Hideaki / 吉藤英明]

In article <adabqxq1rdh.fsf@cisco.com> (at Wed, 01 Feb 2006 17:28:10 -0800), Roland Dreier <rdreier@cisco.com> says:

> Sorry to distract everyone from the VJ channel discussion, but on the
> other hand it looks like Dave is back... I'm resending this because
> I'd really like to get this problem fixed but I want to make sure
> we're doing it the right way.  So either an ACK or a NAK with guidance
> would be great...

Sorry for silence.
Since we have "setup," it'd be natural to have destruct;
I seems sane to me.

--yoshfuji

Re: Re: [PATCH RESEND] net: Move destructor from neigh->ops to neigh_params

[Roland Dreier]

Hi Dave, have you had a chance to look at this?  I can resend again if
you've lost the original mail.  Also, let me know if you want me to
merge this through my tree when 2.6.17 opens up.

The only feedback I've seen is that Yoshfuji-san has said that this
looks sane.

Thanks,
  Roland

Re: Re: [PATCH RESEND] net: Move destructor from neigh->ops to neigh_params

[David S. Miller]

From: Roland Dreier <rdreier@cisco.com>
Date: Mon, 20 Feb 2006 19:42:41 -0800

> Hi Dave, have you had a chance to look at this?

Not yet, it's very low on the priority list at the moment,
but I do still have it in my inbox so don't worry.

Re: Re: [PATCH RESEND] net: Move destructor from neigh->ops to neigh_params

[Roland Dreier]

    David> Not yet, it's very low on the priority list at the moment,
    David> but I do still have it in my inbox so don't worry.

Do you think you'll get a chance to look at it for 2.6.17?  If not we
can work around things in the IPoIB driver in a slightly uglier way
for 2.6.17.

Thanks,
  Roland

Re: Re: [PATCH RESEND] net: Move destructor from neigh->ops to neigh_params

[David S. Miller]

From: Roland Dreier <rdreier@cisco.com>
Date: Mon, 20 Feb 2006 20:48:30 -0800

> Do you think you'll get a chance to look at it for 2.6.17?

Yes I will.

Re: [PATCH RESEND] net: Move destructor from neigh->ops to neigh_params

[David S. Miller]

From: Roland Dreier <rdreier@cisco.com>
Date: Wed, 01 Feb 2006 17:28:10 -0800

> Dave, if you want to merge this directly, that's fine.  Or I'm fine
> with merging this through the IB tree if you'd prefer (if you want me
> to do that, let me know if you think it's 2.6.16 material).

Applied to net-2.6.17, thanks for being so incredibly patient :)

Re: [PATCH] net: Move destructor from neigh->ops to neigh_params

[Andrew Morton]

Roland Dreier <rdreier@cisco.com> wrote:
>
> struct neigh_ops currently has a destructor field, which no in-kernel
>  drivers outside of infiniband use.

net/atm/clip.c begs to disagree.

I'm also wondering why a combination of the net and infiniband trees does this:

drivers/infiniband/ulp/ipoib/ipoib_multicast.c: In function `ipoib_mcast_free':
drivers/infiniband/ulp/ipoib/ipoib_multicast.c:118: error: structure has no member named `destructor'

Re: Re: [PATCH] net: Move destructor from neigh->ops to neigh_params

[Shirley Ma]

[-- Attachment #1.1: Type: text/plain, Size: 518 bytes --]

>I'm also wondering why a combination of the net and infiniband trees does 
this:

>drivers/infiniband/ulp/ipoib/ipoib_multicast.c: In function 
`ipoib_mcast_free':
>drivers/infiniband/ulp/ipoib/ipoib_multicast.c:118: error: structure has 
no member named `destructor'

I guess this patch needs to be combined with another ipoib-neigh patch, 
which hasn't been checked into infiniband/ipoib yet.

Thanks
Shirley Ma
IBM Linux Technology Center
15300 SW Koll Parkway
Beaverton, OR 97006-6063
Phone(Fax): (503) 578-7638


[-- Attachment #1.2: Type: text/html, Size: 689 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] net: Move destructor from neigh->ops to neigh_params

[Roland Dreier]

    Roland> struct neigh_ops currently has a destructor field, which
    Roland> no in-kernel drivers outside of infiniband use.

    Andrew> net/atm/clip.c begs to disagree.

err... my fault for trusting the patch changelog and not
double-checking.  I think the fix is as simple as, although, given my
lack of ATM gear, this is untested:

diff --git a/net/atm/clip.c b/net/atm/clip.c
index 73370de..9d72817 100644
--- a/net/atm/clip.c
+++ b/net/atm/clip.c
@@ -289,7 +289,6 @@ static void clip_neigh_error(struct neig
 
 static struct neigh_ops clip_neigh_ops = {
 	.family =		AF_INET,
-	.destructor =		clip_neigh_destroy,
 	.solicit =		clip_neigh_solicit,
 	.error_report =		clip_neigh_error,
 	.output =		dev_queue_xmit,
@@ -346,6 +345,7 @@ static struct neigh_table clip_tbl = {
 
 	/* parameters are copied from ARP ... */
 	.parms = {
+		.destructor		= clip_neigh_destroy,
 		.tbl 			= &clip_tbl,
 		.base_reachable_time 	= 30 * HZ,
 		.retrans_time 		= 1 * HZ,

Re: [PATCH] net: Move destructor from neigh->ops to neigh_params

[David S. Miller]

From: Roland Dreier <rdreier@cisco.com>
Date: Mon, 06 Mar 2006 21:44:50 -0800

>     Roland> struct neigh_ops currently has a destructor field, which
>     Roland> no in-kernel drivers outside of infiniband use.
> 
>     Andrew> net/atm/clip.c begs to disagree.
> 
> err... my fault for trusting the patch changelog and not
> double-checking.  I think the fix is as simple as, although, given my
> lack of ATM gear, this is untested:

Looks good to me, applied.

Re: [PATCH] net: Move destructor from neigh->ops to neigh_params

[Roland Dreier]

    Andrew> I'm also wondering why a combination of the net and
    Andrew> infiniband trees does this:

    Andrew> drivers/infiniband/ulp/ipoib/ipoib_multicast.c: In
    Andrew> function `ipoib_mcast_free':
    Andrew> drivers/infiniband/ulp/ipoib/ipoib_multicast.c:118: error:
    Andrew> structure has no member named `destructor'

A cock-up on my part.  I think I fixed this when testing the patch,
and promptly forgot about doing it.  Then when I reposted the patch to
netdev, I decided to be very careful about things, so I went back to
pristine sources from Michael's message, which of course is missing
the following chunk:

diff --git a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
index fde442a..93c462e 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
@@ -115,7 +115,6 @@ static void ipoib_mcast_free(struct ipoi
 		if (neigh->ah)
 			ipoib_put_ah(neigh->ah);
 		*to_ipoib_neigh(neigh->neighbour) = NULL;
-		neigh->neighbour->ops->destructor = NULL;
 		kfree(neigh);
 	}
 

[PATCH] IPoIB: Fix build now that destructor is in neigh_params

[Roland Dreier]

Dave, here's an incremental patch that fixes the IPoIB build (which is
broken in net-2.6.17 because of my screw-up, which left out the chunk
below).  I'll also send a full patch that can replace the "Move
destructor from neigh->ops to neigh_params" patch if you'd rather
replace it in your tree.

Thanks, and sorry about the screw-up.

---

Get rid of the last place in IPoIB where we clear
neigh->neighbour->ops->destructor.  This is broken now that the
destructor member has moved to neigh_params.

Signed-off-by: Roland Dreier <rolandd@cisco.com>

diff --git a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
index a2408d7..19fd173 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
@@ -115,7 +115,6 @@ static void ipoib_mcast_free(struct ipoi
 		if (neigh->ah)
 			ipoib_put_ah(neigh->ah);
 		*to_ipoib_neigh(neigh->neighbour) = NULL;
-		neigh->neighbour->ops->destructor = NULL;
 		kfree(neigh);
 	}
 

Re: [PATCH] IPoIB: Fix build now that destructor is in neigh_params

[David S. Miller]

From: Roland Dreier <rdreier@cisco.com>
Date: Tue, 07 Mar 2006 11:21:08 -0800

> Dave, here's an incremental patch that fixes the IPoIB build (which is
> broken in net-2.6.17 because of my screw-up, which left out the chunk
> below).  I'll also send a full patch that can replace the "Move
> destructor from neigh->ops to neigh_params" patch if you'd rather
> replace it in your tree.
> 
> Thanks, and sorry about the screw-up.

Why not just put this into your -ipoib patch set in -mm?
The change was for IPOIB's sake anyways...

Re: [PATCH] IPoIB: Fix build now that destructor is in neigh_params

[Roland Dreier]

    David> Why not just put this into your -ipoib patch set in -mm?
    David> The change was for IPOIB's sake anyways...

OK, good idea.  I'll put this in my for-2.6.17 and for-mm queues.

 - R.

[PATCH] [NET]: Move destructor from neigh->ops to neigh_params

[Roland Dreier]

Here's the fixed version of the original patch.

---

From: Michael S. Tsirkin <mst@mellanox.co.il>

struct neigh_ops currently has a destructor field, which no in-kernel
drivers outside of infiniband use.  The infiniband/ulp/ipoib in-tree
driver stashes some info in the neighbour structure (the results of
the second-stage lookup from ARP results to real link-level path), and
it uses neigh->ops->destructor to get a callback so it can clean up
this extra info when a neighbour is freed.  We've run into problems
with this: since the destructor is in an ops field that is shared
between neighbours that may belong to different net devices, there's
no way to set/clear it safely.

The following patch moves this field to neigh_parms where it can be
safely set, together with its twin neigh_setup.  Two additional
patches in the patch series update ipoib to use this new interface.

Signed-off-by: Michael S. Tsirkin <mst@mellanox.co.il>
Signed-off-by: Roland Dreier <rolandd@cisco.com>

diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c
index c3b5f79..9d9cecd 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
@@ -247,7 +247,6 @@ static void path_free(struct net_device 
 		if (neigh->ah)
 			ipoib_put_ah(neigh->ah);
 		*to_ipoib_neigh(neigh->neighbour) = NULL;
-		neigh->neighbour->ops->destructor = NULL;
 		kfree(neigh);
 	}
 
@@ -530,7 +529,6 @@ static void neigh_add_path(struct sk_buf
 err:
 	*to_ipoib_neigh(skb->dst->neighbour) = NULL;
 	list_del(&neigh->list);
-	neigh->neighbour->ops->destructor = NULL;
 	kfree(neigh);
 
 	++priv->stats.tx_dropped;
@@ -769,21 +767,9 @@ static void ipoib_neigh_destructor(struc
 		ipoib_put_ah(ah);
 }
 
-static int ipoib_neigh_setup(struct neighbour *neigh)
-{
-	/*
-	 * Is this kosher?  I can't find anybody in the kernel that
-	 * sets neigh->destructor, so we should be able to set it here
-	 * without trouble.
-	 */
-	neigh->ops->destructor = ipoib_neigh_destructor;
-
-	return 0;
-}
-
 static int ipoib_neigh_setup_dev(struct net_device *dev, struct neigh_parms *parms)
 {
-	parms->neigh_setup = ipoib_neigh_setup;
+	parms->neigh_destructor = ipoib_neigh_destructor;
 
 	return 0;
 }
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
index a2408d7..19fd173 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
@@ -115,7 +115,6 @@ static void ipoib_mcast_free(struct ipoi
 		if (neigh->ah)
 			ipoib_put_ah(neigh->ah);
 		*to_ipoib_neigh(neigh->neighbour) = NULL;
-		neigh->neighbour->ops->destructor = NULL;
 		kfree(neigh);
 	}
 
diff --git a/include/net/neighbour.h b/include/net/neighbour.h
index 6fa9ae1..b0666d6 100644
--- a/include/net/neighbour.h
+++ b/include/net/neighbour.h
@@ -68,6 +68,7 @@ struct neigh_parms
 	struct net_device *dev;
 	struct neigh_parms *next;
 	int	(*neigh_setup)(struct neighbour *);
+	void	(*neigh_destructor)(struct neighbour *);
 	struct neigh_table *tbl;
 
 	void	*sysctl_table;
@@ -145,7 +146,6 @@ struct neighbour
 struct neigh_ops
 {
 	int			family;
-	void			(*destructor)(struct neighbour *);
 	void			(*solicit)(struct neighbour *, struct sk_buff*);
 	void			(*error_report)(struct neighbour *, struct sk_buff*);
 	int			(*output)(struct sk_buff*);
diff --git a/net/atm/clip.c b/net/atm/clip.c
index 73370de..9d72817 100644
--- a/net/atm/clip.c
+++ b/net/atm/clip.c
@@ -289,7 +289,6 @@ static void clip_neigh_error(struct neig
 
 static struct neigh_ops clip_neigh_ops = {
 	.family =		AF_INET,
-	.destructor =		clip_neigh_destroy,
 	.solicit =		clip_neigh_solicit,
 	.error_report =		clip_neigh_error,
 	.output =		dev_queue_xmit,
@@ -346,6 +345,7 @@ static struct neigh_table clip_tbl = {
 
 	/* parameters are copied from ARP ... */
 	.parms = {
+		.destructor		= clip_neigh_destroy,
 		.tbl 			= &clip_tbl,
 		.base_reachable_time 	= 30 * HZ,
 		.retrans_time 		= 1 * HZ,
diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index e68700f..3489e23 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -586,8 +586,8 @@ void neigh_destroy(struct neighbour *nei
 			kfree(hh);
 	}
 
-	if (neigh->ops && neigh->ops->destructor)
-		(neigh->ops->destructor)(neigh);
+	if (neigh->parms->neigh_destructor)
+		(neigh->parms->neigh_destructor)(neigh);
 
 	skb_queue_purge(&neigh->arp_queue);