[PATCH] bonding: move ipoib_header_ops to vmlinux

[PATCH] bonding: move ipoib_header_ops to vmlinux

[Wengang Wang]

When last slave of a bonding master is removed, the bonding then does not work.
At the time if packet_snd is called against with a master net_device, it calls
then header_ops->create which points to slave's header_ops. In case the slave
is ipoib and the module is unloaded, header_ops would point to invalid address.
Accessing it will cause problem.
This patch tries to fix this issue by moving ipoib_header_ops to vmlinux to keep
it valid even when ipoib module is unloaded.

Signed-off-by: Wengang Wang <wen.gang.wang-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
---
 drivers/infiniband/ulp/ipoib/ipoib.h      | 10 ---------
 drivers/infiniband/ulp/ipoib/ipoib_main.c | 28 +------------------------
 include/linux/ibdevice.h                  | 15 ++++++++++++++
 include/linux/if_infiniband.h             | 11 ++++++++++
 include/uapi/linux/if_infiniband.h        | 16 ++++++++++++---
 net/Makefile                              |  2 +-
 net/infiniband/Makefile                   |  5 +++++
 net/infiniband/infiniband.c               | 34 +++++++++++++++++++++++++++++++
 8 files changed, 80 insertions(+), 41 deletions(-)
 create mode 100644 include/linux/ibdevice.h
 create mode 100644 include/linux/if_infiniband.h
 create mode 100644 net/infiniband/Makefile
 create mode 100644 net/infiniband/infiniband.c

diff --git a/drivers/infiniband/ulp/ipoib/ipoib.h b/drivers/infiniband/ulp/ipoib/ipoib.h
index d7562be..7c25670 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib.h
+++ b/drivers/infiniband/ulp/ipoib/ipoib.h
@@ -121,16 +121,6 @@ enum {
 
 /* structs */
 
-struct ipoib_header {
-	__be16	proto;
-	u16	reserved;
-};
-
-struct ipoib_cb {
-	struct qdisc_skb_cb	qdisc_cb;
-	u8			hwaddr[INFINIBAND_ALEN];
-};
-
 static inline struct ipoib_cb *ipoib_skb_cb(const struct sk_buff *skb)
 {
 	BUILD_BUG_ON(sizeof(skb->cb) < sizeof(struct ipoib_cb));
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c
index 58b5aa3..9233085 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
@@ -34,6 +34,7 @@
 
 #include "ipoib.h"
 
+#include <linux/ibdevice.h>
 #include <linux/module.h>
 
 #include <linux/init.h>
@@ -807,29 +808,6 @@ static void ipoib_timeout(struct net_device *dev)
 	/* XXX reset QP, etc. */
 }
 
-static int ipoib_hard_header(struct sk_buff *skb,
-			     struct net_device *dev,
-			     unsigned short type,
-			     const void *daddr, const void *saddr, unsigned len)
-{
-	struct ipoib_header *header;
-	struct ipoib_cb *cb = ipoib_skb_cb(skb);
-
-	header = (struct ipoib_header *) skb_push(skb, sizeof *header);
-
-	header->proto = htons(type);
-	header->reserved = 0;
-
-	/*
-	 * we don't rely on dst_entry structure,  always stuff the
-	 * destination address into skb->cb so we can figure out where
-	 * to send the packet later.
-	 */
-	memcpy(cb->hwaddr, daddr, INFINIBAND_ALEN);
-
-	return sizeof *header;
-}
-
 static void ipoib_set_mcast_list(struct net_device *dev)
 {
 	struct ipoib_dev_priv *priv = netdev_priv(dev);
@@ -1328,10 +1306,6 @@ void ipoib_dev_cleanup(struct net_device *dev)
 	ipoib_neigh_hash_uninit(dev);
 }
 
-static const struct header_ops ipoib_header_ops = {
-	.create	= ipoib_hard_header,
-};
-
 static const struct net_device_ops ipoib_netdev_ops = {
 	.ndo_uninit		 = ipoib_uninit,
 	.ndo_open		 = ipoib_open,
diff --git a/include/linux/ibdevice.h b/include/linux/ibdevice.h
new file mode 100644
index 0000000..8418974
--- /dev/null
+++ b/include/linux/ibdevice.h
@@ -0,0 +1,15 @@
+/*
+ * ipoib	Implementation of ipoib_header_ops here.
+ *
+ * Authors:	Wengang Wang <wen.gang.wang-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
+ */
+#ifndef _LINUX_IBDEVICE_H
+#define _LINUX_IBDEVICE_H
+
+#include <linux/netdevice.h>
+
+#ifdef __KERNEL__
+extern const struct header_ops ipoib_header_ops;
+#endif /* __KERNEL__ */
+
+#endif	/* _LINUX_IBDEVICE_H */
diff --git a/include/linux/if_infiniband.h b/include/linux/if_infiniband.h
new file mode 100644
index 0000000..9f2d0cf
--- /dev/null
+++ b/include/linux/if_infiniband.h
@@ -0,0 +1,11 @@
+/*
+ * ipoib	Implementation of ipoib_header_ops here.
+ *
+ * Authors:	Wengang Wang <wen.gang.wang-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
+ */
+#ifndef _LINUX_IF_INFINIBAND_H
+#define _LINUX_IF_INFINIBAND_H
+
+#include <uapi/linux/if_infiniband.h>
+
+#endif	/* _LINUX_IF_INFINIBAND_H */
diff --git a/include/uapi/linux/if_infiniband.h b/include/uapi/linux/if_infiniband.h
index 7d958475..9190ee3 100644
--- a/include/uapi/linux/if_infiniband.h
+++ b/include/uapi/linux/if_infiniband.h
@@ -21,9 +21,19 @@
  * $Id$
  */
 
-#ifndef _LINUX_IF_INFINIBAND_H
-#define _LINUX_IF_INFINIBAND_H
+#ifndef _UAPI_LINUX_IF_INFINIBAND_H
+#define _UAPI_LINUX_IF_INFINIBAND_H
 
+#include <net/sch_generic.h>
 #define INFINIBAND_ALEN		20	/* Octets in IPoIB HW addr	*/
 
-#endif /* _LINUX_IF_INFINIBAND_H */
+struct ipoib_header {
+	__be16	proto;
+	u16	reserved;
+};
+
+struct ipoib_cb {
+	struct qdisc_skb_cb	qdisc_cb;
+	u8			hwaddr[INFINIBAND_ALEN];
+};
+#endif /* _UAPI_LINUX_IF_INFINIBAND_H */
diff --git a/net/Makefile b/net/Makefile
index 7ed1970..5d00a13 100644
--- a/net/Makefile
+++ b/net/Makefile
@@ -14,7 +14,7 @@ obj-$(CONFIG_NET)		+= $(tmp-y)
 
 # LLC has to be linked before the files in net/802/
 obj-$(CONFIG_LLC)		+= llc/
-obj-$(CONFIG_NET)		+= ethernet/ 802/ sched/ netlink/
+obj-$(CONFIG_NET)		+= ethernet/ 802/ sched/ netlink/ infiniband/
 obj-$(CONFIG_NETFILTER)		+= netfilter/
 obj-$(CONFIG_INET)		+= ipv4/
 obj-$(CONFIG_XFRM)		+= xfrm/
diff --git a/net/infiniband/Makefile b/net/infiniband/Makefile
new file mode 100644
index 0000000..c8a5be0
--- /dev/null
+++ b/net/infiniband/Makefile
@@ -0,0 +1,5 @@
+#
+# Makefile for the Linux ip over infiniband layer.
+#
+
+obj-y					+= infiniband.o
diff --git a/net/infiniband/infiniband.c b/net/infiniband/infiniband.c
new file mode 100644
index 0000000..a458ec5
--- /dev/null
+++ b/net/infiniband/infiniband.c
@@ -0,0 +1,34 @@
+/*
+ * ipoib	Implementation of ipoib_header_ops here.
+ *
+ * Authors:	Wengang Wang <wen.gang.wang-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
+ */
+
+#include <linux/if_infiniband.h>
+
+static int ipoib_hard_header(struct sk_buff *skb,
+			     struct net_device *dev,
+			     unsigned short type,
+			     const void *daddr, const void *saddr, unsigned len)
+{
+	struct ipoib_header *header;
+	struct ipoib_cb *cb = (struct ipoib_cb *)skb->cb;
+
+	header = (struct ipoib_header *)skb_push(skb, sizeof(*header));
+
+	header->proto = htons(type);
+	header->reserved = 0;
+
+	/* we don't rely on dst_entry structure,  always stuff the
+	 * destination address into skb->cb so we can figure out where
+	 * to send the packet later.
+	 */
+	memcpy(cb->hwaddr, daddr, INFINIBAND_ALEN);
+
+	return sizeof(*header);
+}
+
+const struct header_ops ipoib_header_ops = {
+	.create	= ipoib_hard_header,
+};
+EXPORT_SYMBOL(ipoib_header_ops);
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] bonding: move ipoib_header_ops to vmlinux

[David Miller]

From: Wengang Wang <wen.gang.wang-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
Date: Tue, 25 Nov 2014 13:36:08 +0800

> When last slave of a bonding master is removed, the bonding then does not work.
> At the time if packet_snd is called against with a master net_device, it calls
> then header_ops->create which points to slave's header_ops. In case the slave
> is ipoib and the module is unloaded, header_ops would point to invalid address.
> Accessing it will cause problem.
> This patch tries to fix this issue by moving ipoib_header_ops to vmlinux to keep
> it valid even when ipoib module is unloaded.
> 
> Signed-off-by: Wengang Wang <wen.gang.wang-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>

IPOIB should not work over bonding as it requires that the device
use ARPHRD_ETHER.

Someone mentioned this, and I did not see any response.

Please show how a legitimate real bonding configuration can be
created, reproduce a stray memory access, and therefore potentially
cause a crash.

Using various debugging features of the kernel should allow you to
trigger an assertion quite easily if this bug really exists.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] bonding: move ipoib_header_ops to vmlinux

[Or Gerlitz]

On 11/25/2014 8:07 AM, David Miller wrote:
> IPOIB should not work over bonding as it requires that the device
> use ARPHRD_ETHER.

Hi Dave,

IPoIB devices can be enslaved to both bonding and teaming in their HA mode,
the bond device type becomes ARPHRD_INFINIBAND when this happens.

Or.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] bonding: move ipoib_header_ops to vmlinux

[Jay Vosburgh]

Or Gerlitz <ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:

>On 11/25/2014 8:07 AM, David Miller wrote:
>> IPOIB should not work over bonding as it requires that the device
>> use ARPHRD_ETHER.
>
>Hi Dave,
>
>IPoIB devices can be enslaved to both bonding and teaming in their HA mode,
>the bond device type becomes ARPHRD_INFINIBAND when this happens.

	The point was that pktgen disallows ARPHRD_INFINIBAND, not that
bonding does.

	Pktgen specifically checks for type != ARPHRD_ETHER, so the
IPoIB bond should not be able to be used with pkgten.  My suspicion is
that pktgen is being configured on the bond first, then an IPoIB slave
is added to the bond; this would change its type in a way that pktgen
wouldn't notice.

	-J

---
	-Jay Vosburgh, jay.vosburgh-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] bonding: move ipoib_header_ops to vmlinux

[David Miller]

From: Jay Vosburgh <jay.vosburgh-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
Date: Tue, 25 Nov 2014 10:41:17 -0800

> Or Gerlitz <ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:
> 
>>On 11/25/2014 8:07 AM, David Miller wrote:
>>> IPOIB should not work over bonding as it requires that the device
>>> use ARPHRD_ETHER.
>>
>>Hi Dave,
>>
>>IPoIB devices can be enslaved to both bonding and teaming in their HA mode,
>>the bond device type becomes ARPHRD_INFINIBAND when this happens.
> 
> 	The point was that pktgen disallows ARPHRD_INFINIBAND, not that
> bonding does.
> 
> 	Pktgen specifically checks for type != ARPHRD_ETHER, so the
> IPoIB bond should not be able to be used with pkgten.  My suspicion is
> that pktgen is being configured on the bond first, then an IPoIB slave
> is added to the bond; this would change its type in a way that pktgen
> wouldn't notice.

+1
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] bonding: move ipoib_header_ops to vmlinux

[Wengang]

于 2014年11月26日 02:44, David Miller 写道:
> From: Jay Vosburgh <jay.vosburgh-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
> Date: Tue, 25 Nov 2014 10:41:17 -0800
>
>> Or Gerlitz <ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:
>>
>>> On 11/25/2014 8:07 AM, David Miller wrote:
>>>> IPOIB should not work over bonding as it requires that the device
>>>> use ARPHRD_ETHER.
>>> Hi Dave,
>>>
>>> IPoIB devices can be enslaved to both bonding and teaming in their HA mode,
>>> the bond device type becomes ARPHRD_INFINIBAND when this happens.
>> 	The point was that pktgen disallows ARPHRD_INFINIBAND, not that
>> bonding does.
>>
>> 	Pktgen specifically checks for type != ARPHRD_ETHER, so the
>> IPoIB bond should not be able to be used with pkgten.  My suspicion is
>> that pktgen is being configured on the bond first, then an IPoIB slave
>> is added to the bond; this would change its type in a way that pktgen
>> wouldn't notice.
> +1

I think it go this way:

1) bond_master is ready
2) bond_enslave enslave a IPOIB interface calling bond_setup_by_slave
3) then bond_setup_by_slave set change master type to ARPHRD_INFINIBAND.

code is like this:

1 /* enslave device <slave> to bond device <master> */
2 int bond_enslave(struct net_device *bond_dev, struct net_device 
*slave_dev)
3 {
4 <snip>...
5 /* set bonding device ether type by slave - bonding netdevices are
6 * created with ether_setup, so when the slave type is not ARPHRD_ETHER
7 * there is a need to override some of the type dependent attribs/funcs.
8 *
9 * bond ether type mutual exclusion - don't allow slaves of dissimilar
10 * ether type (eg ARPHRD_ETHER and ARPHRD_INFINIBAND) share the same bond
11 */
12 if (!bond_has_slaves(bond)) {
13 if (bond_dev->type != slave_dev->type) {
14 <snip>...
15 if (slave_dev->type != ARPHRD_ETHER)
16 bond_setup_by_slave(bond_dev, slave_dev);
17 else {
18 ether_setup(bond_dev);
19 bond_dev->priv_flags &= ~IFF_TX_SKB_SHARING;
20 }
21
22 call_netdevice_notifiers(NETDEV_POST_TYPE_CHANGE,
23 bond_dev);
24 }
25 <snip>...
26 }
27
28 static void bond_setup_by_slave(struct net_device *bond_dev,
29 struct net_device *slave_dev)
30 {
31 bond_dev->header_ops = slave_dev->header_ops;
32
33 bond_dev->type = slave_dev->type;
34 bond_dev->hard_header_len = slave_dev->hard_header_len;
35 bond_dev->addr_len = slave_dev->addr_len;
36
37 memcpy(bond_dev->broadcast, slave_dev->broadcast,
38 slave_dev->addr_len);
39 }
40

thanks
wengang
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] bonding: move ipoib_header_ops to vmlinux

[Wengang Wang]

Hi David and Jay,

Then about about the change in this patch?

thanks,
wengang

在 2014年11月26日 09:30, Wengang 写道:
> 于 2014年11月26日 02:44, David Miller 写道:
>> From: Jay Vosburgh <jay.vosburgh@canonical.com>
>> Date: Tue, 25 Nov 2014 10:41:17 -0800
>>
>>> Or Gerlitz <ogerlitz@mellanox.com> wrote:
>>>
>>>> On 11/25/2014 8:07 AM, David Miller wrote:
>>>>> IPOIB should not work over bonding as it requires that the device
>>>>> use ARPHRD_ETHER.
>>>> Hi Dave,
>>>>
>>>> IPoIB devices can be enslaved to both bonding and teaming in their 
>>>> HA mode,
>>>> the bond device type becomes ARPHRD_INFINIBAND when this happens.
>>>     The point was that pktgen disallows ARPHRD_INFINIBAND, not that
>>> bonding does.
>>>
>>>     Pktgen specifically checks for type != ARPHRD_ETHER, so the
>>> IPoIB bond should not be able to be used with pkgten.  My suspicion is
>>> that pktgen is being configured on the bond first, then an IPoIB slave
>>> is added to the bond; this would change its type in a way that pktgen
>>> wouldn't notice.
>> +1
>
> I think it go this way:
>
> 1) bond_master is ready
> 2) bond_enslave enslave a IPOIB interface calling bond_setup_by_slave
> 3) then bond_setup_by_slave set change master type to ARPHRD_INFINIBAND.
>
> code is like this:
>
> 1 /* enslave device <slave> to bond device <master> */
> 2 int bond_enslave(struct net_device *bond_dev, struct net_device 
> *slave_dev)
> 3 {
> 4 <snip>...
> 5 /* set bonding device ether type by slave - bonding netdevices are
> 6 * created with ether_setup, so when the slave type is not ARPHRD_ETHER
> 7 * there is a need to override some of the type dependent attribs/funcs.
> 8 *
> 9 * bond ether type mutual exclusion - don't allow slaves of dissimilar
> 10 * ether type (eg ARPHRD_ETHER and ARPHRD_INFINIBAND) share the same 
> bond
> 11 */
> 12 if (!bond_has_slaves(bond)) {
> 13 if (bond_dev->type != slave_dev->type) {
> 14 <snip>...
> 15 if (slave_dev->type != ARPHRD_ETHER)
> 16 bond_setup_by_slave(bond_dev, slave_dev);
> 17 else {
> 18 ether_setup(bond_dev);
> 19 bond_dev->priv_flags &= ~IFF_TX_SKB_SHARING;
> 20 }
> 21
> 22 call_netdevice_notifiers(NETDEV_POST_TYPE_CHANGE,
> 23 bond_dev);
> 24 }
> 25 <snip>...
> 26 }
> 27
> 28 static void bond_setup_by_slave(struct net_device *bond_dev,
> 29 struct net_device *slave_dev)
> 30 {
> 31 bond_dev->header_ops = slave_dev->header_ops;
> 32
> 33 bond_dev->type = slave_dev->type;
> 34 bond_dev->hard_header_len = slave_dev->hard_header_len;
> 35 bond_dev->addr_len = slave_dev->addr_len;
> 36
> 37 memcpy(bond_dev->broadcast, slave_dev->broadcast,
> 38 slave_dev->addr_len);
> 39 }
> 40
>
> thanks
> wengang
> -- 
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] bonding: move ipoib_header_ops to vmlinux

[Wengang]

Anyone please respond to this?

thanks,
wengang

于 2014年12月03日 09:50, Wengang Wang 写道:
> Hi David and Jay,
>
> Then about about the change in this patch?
>
> thanks,
> wengang
>
> 在 2014年11月26日 09:30, Wengang 写道:
>> 于 2014年11月26日 02:44, David Miller 写道:
>>> From: Jay Vosburgh <jay.vosburgh-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
>>> Date: Tue, 25 Nov 2014 10:41:17 -0800
>>>
>>>> Or Gerlitz <ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:
>>>>
>>>>> On 11/25/2014 8:07 AM, David Miller wrote:
>>>>>> IPOIB should not work over bonding as it requires that the device
>>>>>> use ARPHRD_ETHER.
>>>>> Hi Dave,
>>>>>
>>>>> IPoIB devices can be enslaved to both bonding and teaming in their 
>>>>> HA mode,
>>>>> the bond device type becomes ARPHRD_INFINIBAND when this happens.
>>>>     The point was that pktgen disallows ARPHRD_INFINIBAND, not that
>>>> bonding does.
>>>>
>>>>     Pktgen specifically checks for type != ARPHRD_ETHER, so the
>>>> IPoIB bond should not be able to be used with pkgten.  My suspicion is
>>>> that pktgen is being configured on the bond first, then an IPoIB slave
>>>> is added to the bond; this would change its type in a way that pktgen
>>>> wouldn't notice.
>>> +1
>>
>> I think it go this way:
>>
>> 1) bond_master is ready
>> 2) bond_enslave enslave a IPOIB interface calling bond_setup_by_slave
>> 3) then bond_setup_by_slave set change master type to ARPHRD_INFINIBAND.
>>
>> code is like this:
>>
>> 1 /* enslave device <slave> to bond device <master> */
>> 2 int bond_enslave(struct net_device *bond_dev, struct net_device 
>> *slave_dev)
>> 3 {
>> 4 <snip>...
>> 5 /* set bonding device ether type by slave - bonding netdevices are
>> 6 * created with ether_setup, so when the slave type is not ARPHRD_ETHER
>> 7 * there is a need to override some of the type dependent 
>> attribs/funcs.
>> 8 *
>> 9 * bond ether type mutual exclusion - don't allow slaves of dissimilar
>> 10 * ether type (eg ARPHRD_ETHER and ARPHRD_INFINIBAND) share the 
>> same bond
>> 11 */
>> 12 if (!bond_has_slaves(bond)) {
>> 13 if (bond_dev->type != slave_dev->type) {
>> 14 <snip>...
>> 15 if (slave_dev->type != ARPHRD_ETHER)
>> 16 bond_setup_by_slave(bond_dev, slave_dev);
>> 17 else {
>> 18 ether_setup(bond_dev);
>> 19 bond_dev->priv_flags &= ~IFF_TX_SKB_SHARING;
>> 20 }
>> 21
>> 22 call_netdevice_notifiers(NETDEV_POST_TYPE_CHANGE,
>> 23 bond_dev);
>> 24 }
>> 25 <snip>...
>> 26 }
>> 27
>> 28 static void bond_setup_by_slave(struct net_device *bond_dev,
>> 29 struct net_device *slave_dev)
>> 30 {
>> 31 bond_dev->header_ops = slave_dev->header_ops;
>> 32
>> 33 bond_dev->type = slave_dev->type;
>> 34 bond_dev->hard_header_len = slave_dev->hard_header_len;
>> 35 bond_dev->addr_len = slave_dev->addr_len;
>> 36
>> 37 memcpy(bond_dev->broadcast, slave_dev->broadcast,
>> 38 slave_dev->addr_len);
>> 39 }
>> 40
>>
>> thanks
>> wengang
>> -- 
>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> -- 
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] bonding: move ipoib_header_ops to vmlinux

[Wengang]

Hi, Anyone please review this patch? David? Jay? please.

thanks,
wengang

于 2014年12月15日 09:12, Wengang 写道:
> Anyone please respond to this?
>
> thanks,
> wengang
>
> 于 2014年12月03日 09:50, Wengang Wang 写道:
>> Hi David and Jay,
>>
>> Then about about the change in this patch?
>>
>> thanks,
>> wengang
>>
>> 在 2014年11月26日 09:30, Wengang 写道:
>>> 于 2014年11月26日 02:44, David Miller 写道:
>>>> From: Jay Vosburgh <jay.vosburgh@canonical.com>
>>>> Date: Tue, 25 Nov 2014 10:41:17 -0800
>>>>
>>>>> Or Gerlitz <ogerlitz@mellanox.com> wrote:
>>>>>
>>>>>> On 11/25/2014 8:07 AM, David Miller wrote:
>>>>>>> IPOIB should not work over bonding as it requires that the device
>>>>>>> use ARPHRD_ETHER.
>>>>>> Hi Dave,
>>>>>>
>>>>>> IPoIB devices can be enslaved to both bonding and teaming in 
>>>>>> their HA mode,
>>>>>> the bond device type becomes ARPHRD_INFINIBAND when this happens.
>>>>>     The point was that pktgen disallows ARPHRD_INFINIBAND, not that
>>>>> bonding does.
>>>>>
>>>>>     Pktgen specifically checks for type != ARPHRD_ETHER, so the
>>>>> IPoIB bond should not be able to be used with pkgten.  My 
>>>>> suspicion is
>>>>> that pktgen is being configured on the bond first, then an IPoIB 
>>>>> slave
>>>>> is added to the bond; this would change its type in a way that pktgen
>>>>> wouldn't notice.
>>>> +1
>>>
>>> I think it go this way:
>>>
>>> 1) bond_master is ready
>>> 2) bond_enslave enslave a IPOIB interface calling bond_setup_by_slave
>>> 3) then bond_setup_by_slave set change master type to 
>>> ARPHRD_INFINIBAND.
>>>
>>> code is like this:
>>>
>>> 1 /* enslave device <slave> to bond device <master> */
>>> 2 int bond_enslave(struct net_device *bond_dev, struct net_device 
>>> *slave_dev)
>>> 3 {
>>> 4 <snip>...
>>> 5 /* set bonding device ether type by slave - bonding netdevices are
>>> 6 * created with ether_setup, so when the slave type is not 
>>> ARPHRD_ETHER
>>> 7 * there is a need to override some of the type dependent 
>>> attribs/funcs.
>>> 8 *
>>> 9 * bond ether type mutual exclusion - don't allow slaves of dissimilar
>>> 10 * ether type (eg ARPHRD_ETHER and ARPHRD_INFINIBAND) share the 
>>> same bond
>>> 11 */
>>> 12 if (!bond_has_slaves(bond)) {
>>> 13 if (bond_dev->type != slave_dev->type) {
>>> 14 <snip>...
>>> 15 if (slave_dev->type != ARPHRD_ETHER)
>>> 16 bond_setup_by_slave(bond_dev, slave_dev);
>>> 17 else {
>>> 18 ether_setup(bond_dev);
>>> 19 bond_dev->priv_flags &= ~IFF_TX_SKB_SHARING;
>>> 20 }
>>> 21
>>> 22 call_netdevice_notifiers(NETDEV_POST_TYPE_CHANGE,
>>> 23 bond_dev);
>>> 24 }
>>> 25 <snip>...
>>> 26 }
>>> 27
>>> 28 static void bond_setup_by_slave(struct net_device *bond_dev,
>>> 29 struct net_device *slave_dev)
>>> 30 {
>>> 31 bond_dev->header_ops = slave_dev->header_ops;
>>> 32
>>> 33 bond_dev->type = slave_dev->type;
>>> 34 bond_dev->hard_header_len = slave_dev->hard_header_len;
>>> 35 bond_dev->addr_len = slave_dev->addr_len;
>>> 36
>>> 37 memcpy(bond_dev->broadcast, slave_dev->broadcast,
>>> 38 slave_dev->addr_len);
>>> 39 }
>>> 40
>>>
>>> thanks
>>> wengang
>>> -- 
>

Re: [PATCH] bonding: move ipoib_header_ops to vmlinux

[Wengang]

Hi David,

This is a real case not a potential crash. The call stack is like this:

crash> bt
PID: 47323 TASK: ffff881722954140 CPU: 13 COMMAND: "arping"
#0 [ffff881518437860] machine_kexec at ffffffff8103aac9
#1 [ffff8815184378d0] crash_kexec at ffffffff810b9943
#2 [ffff8815184379a0] oops_end at ffffffff8150e9b8
#3 [ffff8815184379d0] no_context at ffffffff8104855c
#4 [ffff881518437a10] __bad_area_nosemaphore at ffffffff81048685
#5 [ffff881518437a60] bad_area_nosemaphore at ffffffff810487e3
#6 [ffff881518437a70] do_page_fault at ffffffff81511558
#7 [ffff881518437b80] page_fault at ffffffff8150df55
[exception RIP: packet_snd+608]
RIP: ffffffff814ddbc0 RSP: ffff881518437c38 RFLAGS: 00010282
RAX: ffffffffa0316040 RBX: ffff881518437e58 RCX: 0000000000000000
RDX: 0000000000000048 RSI: 0000000000000038 RDI: ffff88172508a080
RBP: ffff881518437ca8 R8: ffff88176568f400 R9: 0000000000000038
R10: ffff88172508a080 R11: 0000000000000000 R12: ffff8817e94f2080
R13: ffff8817eba0f400 R14: ffff8817eaef6000 R15: 0000000000000038
ORIG_RAX: ffffffffffffffff CS: 0010 SS: 0018
#8 [ffff881518437cb0] packet_sendmsg at ffffffff814ddee3
#9 [ffff881518437cc0] sock_sendmsg at ffffffff8142ad3f
#10 [ffff881518437e40] sys_sendto at ffffffff8142af09
#11 [ffff881518437f80] system_call_fastpath at ffffffff81515c42
RIP: 00007f4a03095853 RSP: 00007fffad354bf8 RFLAGS: 00010202
RAX: 000000000000002c RBX: ffffffff81515c42 RCX: 00007f4a02fde7ce
RDX: 0000000000000038 RSI: 00007fffad354ab0 RDI: 0000000000000003
RBP: 0000000000000038 R8: 00007f4a03b87e00 R9: 0000000000000020
R10: 0000000000000000 R11: 0000000000000246 R12: 00007f4a03b87e00
R13: 00007f4a03b87e4c R14: 00007fffad354ae4 R15: 00007f4a03b87e98
ORIG_RAX: 000000000000002c CS: 0033 SS: 002b

Though the crash is not based on mainline code, mainline has the same issue.

I think Or Gerlitz answered the question "IPOIB should not work over 
bonding as it requires that the device use ARPHRD_ETHER.".

IPoIB devices can be enslaved to both bonding and teaming in their HA mode,
the bond device type becomes ARPHRD_INFINIBAND when this happens.

So, what information else do you need?

thanks,
wengang


于 2014年11月25日 14:07, David Miller 写道:
> From: Wengang Wang <wen.gang.wang-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> Date: Tue, 25 Nov 2014 13:36:08 +0800
>
>> When last slave of a bonding master is removed, the bonding then does not work.
>> At the time if packet_snd is called against with a master net_device, it calls
>> then header_ops->create which points to slave's header_ops. In case the slave
>> is ipoib and the module is unloaded, header_ops would point to invalid address.
>> Accessing it will cause problem.
>> This patch tries to fix this issue by moving ipoib_header_ops to vmlinux to keep
>> it valid even when ipoib module is unloaded.
>>
>> Signed-off-by: Wengang Wang <wen.gang.wang-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> IPOIB should not work over bonding as it requires that the device
> use ARPHRD_ETHER.
>
> Someone mentioned this, and I did not see any response.
>
> Please show how a legitimate real bonding configuration can be
> created, reproduce a stray memory access, and therefore potentially
> cause a crash.
>
> Using various debugging features of the kernel should allow you to
> trigger an assertion quite easily if this bug really exists.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] bonding: move ipoib_header_ops to vmlinux

[David Miller]

From: Wengang <wen.gang.wang-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
Date: Mon, 29 Dec 2014 15:11:32 +0800

> So, what information else do you need?

I need a patch formally (re-)submitted.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] bonding: move ipoib_header_ops to vmlinux

[Cong Wang]

On Mon, Nov 24, 2014 at 9:36 PM, Wengang Wang <wen.gang.wang@oracle.com> wrote:
> When last slave of a bonding master is removed, the bonding then does not work.
> At the time if packet_snd is called against with a master net_device, it calls
> then header_ops->create which points to slave's header_ops. In case the slave
> is ipoib and the module is unloaded, header_ops would point to invalid address.
> Accessing it will cause problem.
> This patch tries to fix this issue by moving ipoib_header_ops to vmlinux to keep
> it valid even when ipoib module is unloaded.
>

I still don't think this is the right way to fix the bug, because
there are other modules which have the same bug, for example,
net/bluetooth/6lowpan.c (unless it can't be enslaved into a bond
of course, I haven't verified). We don't want to move them all
to builtin kernel, do we?

On the other hand, as Jay explained, bonding is probably
the only one which has the problem, why not solve it in bonding?
I mean why do we have to adjust other modules just for bonding?

When its slave device gets removed, some netdev event is sent
to the master, so why not reset the header_ops to ether header ops
up on this event? Something like below (not even compile tested):


diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 184c434..aedccae 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1726,6 +1726,7 @@ static int __bond_release_one(struct net_device *bond_dev,
        if (!bond_has_slaves(bond)) {
                bond_set_carrier(bond);
                eth_hw_addr_random(bond_dev);
+               ether_setup(bond_dev);
        }

        unblock_netpoll_tx();

It should solve more than just your ipoib case.

Last but not least, since you said you saw the crash in packet_snd(),
it would also be interesting to know why packet_snd() still picks this
bond dev after all its slaves are gone, after all it is not a functional
device without any slave (its carrier is off).

Thanks.

Re: [PATCH] bonding: move ipoib_header_ops to vmlinux

[Wengang]

于 2014年12月30日 05:32, Cong Wang 写道:
> On Mon, Nov 24, 2014 at 9:36 PM, Wengang Wang <wen.gang.wang@oracle.com> wrote:
>> When last slave of a bonding master is removed, the bonding then does not work.
>> At the time if packet_snd is called against with a master net_device, it calls
>> then header_ops->create which points to slave's header_ops. In case the slave
>> is ipoib and the module is unloaded, header_ops would point to invalid address.
>> Accessing it will cause problem.
>> This patch tries to fix this issue by moving ipoib_header_ops to vmlinux to keep
>> it valid even when ipoib module is unloaded.
>>
> I still don't think this is the right way to fix the bug, because
> there are other modules which have the same bug, for example,
> net/bluetooth/6lowpan.c (unless it can't be enslaved into a bond
> of course, I haven't verified). We don't want to move them all
> to builtin kernel, do we?

I don't think bluetooth needs bonding(and you didn't verify at all).
Do you have strong reason(examples)?

> On the other hand, as Jay explained, bonding is probably
> the only one which has the problem, why not solve it in bonding?
> I mean why do we have to adjust other modules just for bonding?

There are more than one way we do things. For this case, considering
needs, complexity and stability I think moving ipoib_header_ops is the
right way to go.

> When its slave device gets removed, some netdev event is sent
> to the master, so why not reset the header_ops to ether header ops
> up on this event? Something like below (not even compile tested):
>
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 184c434..aedccae 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -1726,6 +1726,7 @@ static int __bond_release_one(struct net_device *bond_dev,
>          if (!bond_has_slaves(bond)) {
>                  bond_set_carrier(bond);
>                  eth_hw_addr_random(bond_dev);
> +               ether_setup(bond_dev);
>          }
>
>          unblock_netpoll_tx();
>
> It should solve more than just your ipoib case.
Do you think it's safe in a race condition? Before you change 
header_ops, you have no
way to prevent another thread from accessing the old one.

> Last but not least, since you said you saw the crash in packet_snd(),
> it would also be interesting to know why packet_snd() still picks this
> bond dev after all its slaves are gone, after all it is not a functional
> device without any slave (its carrier is off).

It's quite normal in a race situation.

thanks,
wengang
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] bonding: move ipoib_header_ops to vmlinux

[David Miller]

From: Wengang <wen.gang.wang-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
Date: Tue, 30 Dec 2014 11:01:42 +0800

> There are more than one way we do things. For this case, considering
> needs, complexity and stability I think moving ipoib_header_ops is the
> right way to go.

I completely disagree, it's a gross hack at best.

It's papering over the real problem.

When we have references to released objects in other areas of the
networking stack, we don't move those objects into the static kernel
image as a fix.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] bonding: move ipoib_header_ops to vmlinux

[Wengang]

于 2014年12月30日 12:25, David Miller 写道:
> From: Wengang <wen.gang.wang@oracle.com>
> Date: Tue, 30 Dec 2014 11:01:42 +0800
>
>> There are more than one way we do things. For this case, considering
>> needs, complexity and stability I think moving ipoib_header_ops is the
>> right way to go.
> I completely disagree, it's a gross hack at best.
>
> It's papering over the real problem.
>
> When we have references to released objects in other areas of the
> networking stack, we don't move those objects into the static kernel
> image as a fix.

OK. Let me see if I can make a patch to match what you want.

Thanks
wengang

[PATCH] bonding: move ipoib_header_ops to vmlinux

[Wengang Wang]

When last slave of a bonding master is removed, the bonding then does not work.
At the time if packet_snd is called against with a master net_device, it calls
then header_ops->create which points to slave's header_ops. In case the slave
is ipoib and the module is unloaded, header_ops would point to invalid address.
Accessing it will cause problem.
This patch tries to fix this issue by moving ipoib_header_ops to vmlinux to keep
it valid even when ipoib module is unloaded.

Signed-off-by: Wengang Wang <wen.gang.wang-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
---
 drivers/infiniband/ulp/ipoib/ipoib.h      | 10 ---------
 drivers/infiniband/ulp/ipoib/ipoib_main.c | 28 +------------------------
 include/linux/ibdevice.h                  | 15 ++++++++++++++
 include/linux/if_infiniband.h             | 11 ++++++++++
 include/uapi/linux/if_infiniband.h        | 16 ++++++++++++---
 net/Makefile                              |  2 +-
 net/infiniband/Makefile                   |  5 +++++
 net/infiniband/infiniband.c               | 34 +++++++++++++++++++++++++++++++
 8 files changed, 80 insertions(+), 41 deletions(-)
 create mode 100644 include/linux/ibdevice.h
 create mode 100644 include/linux/if_infiniband.h
 create mode 100644 net/infiniband/Makefile
 create mode 100644 net/infiniband/infiniband.c

diff --git a/drivers/infiniband/ulp/ipoib/ipoib.h b/drivers/infiniband/ulp/ipoib/ipoib.h
index d7562be..7c25670 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib.h
+++ b/drivers/infiniband/ulp/ipoib/ipoib.h
@@ -121,16 +121,6 @@ enum {
 
 /* structs */
 
-struct ipoib_header {
-	__be16	proto;
-	u16	reserved;
-};
-
-struct ipoib_cb {
-	struct qdisc_skb_cb	qdisc_cb;
-	u8			hwaddr[INFINIBAND_ALEN];
-};
-
 static inline struct ipoib_cb *ipoib_skb_cb(const struct sk_buff *skb)
 {
 	BUILD_BUG_ON(sizeof(skb->cb) < sizeof(struct ipoib_cb));
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c
index 58b5aa3..9233085 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
@@ -34,6 +34,7 @@
 
 #include "ipoib.h"
 
+#include <linux/ibdevice.h>
 #include <linux/module.h>
 
 #include <linux/init.h>
@@ -807,29 +808,6 @@ static void ipoib_timeout(struct net_device *dev)
 	/* XXX reset QP, etc. */
 }
 
-static int ipoib_hard_header(struct sk_buff *skb,
-			     struct net_device *dev,
-			     unsigned short type,
-			     const void *daddr, const void *saddr, unsigned len)
-{
-	struct ipoib_header *header;
-	struct ipoib_cb *cb = ipoib_skb_cb(skb);
-
-	header = (struct ipoib_header *) skb_push(skb, sizeof *header);
-
-	header->proto = htons(type);
-	header->reserved = 0;
-
-	/*
-	 * we don't rely on dst_entry structure,  always stuff the
-	 * destination address into skb->cb so we can figure out where
-	 * to send the packet later.
-	 */
-	memcpy(cb->hwaddr, daddr, INFINIBAND_ALEN);
-
-	return sizeof *header;
-}
-
 static void ipoib_set_mcast_list(struct net_device *dev)
 {
 	struct ipoib_dev_priv *priv = netdev_priv(dev);
@@ -1328,10 +1306,6 @@ void ipoib_dev_cleanup(struct net_device *dev)
 	ipoib_neigh_hash_uninit(dev);
 }
 
-static const struct header_ops ipoib_header_ops = {
-	.create	= ipoib_hard_header,
-};
-
 static const struct net_device_ops ipoib_netdev_ops = {
 	.ndo_uninit		 = ipoib_uninit,
 	.ndo_open		 = ipoib_open,
diff --git a/include/linux/ibdevice.h b/include/linux/ibdevice.h
new file mode 100644
index 0000000..8418974
--- /dev/null
+++ b/include/linux/ibdevice.h
@@ -0,0 +1,15 @@
+/*
+ * ipoib	Implementation of ipoib_header_ops here.
+ *
+ * Authors:	Wengang Wang <wen.gang.wang-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
+ */
+#ifndef _LINUX_IBDEVICE_H
+#define _LINUX_IBDEVICE_H
+
+#include <linux/netdevice.h>
+
+#ifdef __KERNEL__
+extern const struct header_ops ipoib_header_ops;
+#endif /* __KERNEL__ */
+
+#endif	/* _LINUX_IBDEVICE_H */
diff --git a/include/linux/if_infiniband.h b/include/linux/if_infiniband.h
new file mode 100644
index 0000000..9f2d0cf
--- /dev/null
+++ b/include/linux/if_infiniband.h
@@ -0,0 +1,11 @@
+/*
+ * ipoib	Implementation of ipoib_header_ops here.
+ *
+ * Authors:	Wengang Wang <wen.gang.wang-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
+ */
+#ifndef _LINUX_IF_INFINIBAND_H
+#define _LINUX_IF_INFINIBAND_H
+
+#include <uapi/linux/if_infiniband.h>
+
+#endif	/* _LINUX_IF_INFINIBAND_H */
diff --git a/include/uapi/linux/if_infiniband.h b/include/uapi/linux/if_infiniband.h
index 7d958475..9190ee3 100644
--- a/include/uapi/linux/if_infiniband.h
+++ b/include/uapi/linux/if_infiniband.h
@@ -21,9 +21,19 @@
  * $Id$
  */
 
-#ifndef _LINUX_IF_INFINIBAND_H
-#define _LINUX_IF_INFINIBAND_H
+#ifndef _UAPI_LINUX_IF_INFINIBAND_H
+#define _UAPI_LINUX_IF_INFINIBAND_H
 
+#include <net/sch_generic.h>
 #define INFINIBAND_ALEN		20	/* Octets in IPoIB HW addr	*/
 
-#endif /* _LINUX_IF_INFINIBAND_H */
+struct ipoib_header {
+	__be16	proto;
+	u16	reserved;
+};
+
+struct ipoib_cb {
+	struct qdisc_skb_cb	qdisc_cb;
+	u8			hwaddr[INFINIBAND_ALEN];
+};
+#endif /* _UAPI_LINUX_IF_INFINIBAND_H */
diff --git a/net/Makefile b/net/Makefile
index 7ed1970..5d00a13 100644
--- a/net/Makefile
+++ b/net/Makefile
@@ -14,7 +14,7 @@ obj-$(CONFIG_NET)		+= $(tmp-y)
 
 # LLC has to be linked before the files in net/802/
 obj-$(CONFIG_LLC)		+= llc/
-obj-$(CONFIG_NET)		+= ethernet/ 802/ sched/ netlink/
+obj-$(CONFIG_NET)		+= ethernet/ 802/ sched/ netlink/ infiniband/
 obj-$(CONFIG_NETFILTER)		+= netfilter/
 obj-$(CONFIG_INET)		+= ipv4/
 obj-$(CONFIG_XFRM)		+= xfrm/
diff --git a/net/infiniband/Makefile b/net/infiniband/Makefile
new file mode 100644
index 0000000..c8a5be0
--- /dev/null
+++ b/net/infiniband/Makefile
@@ -0,0 +1,5 @@
+#
+# Makefile for the Linux ip over infiniband layer.
+#
+
+obj-y					+= infiniband.o
diff --git a/net/infiniband/infiniband.c b/net/infiniband/infiniband.c
new file mode 100644
index 0000000..a458ec5
--- /dev/null
+++ b/net/infiniband/infiniband.c
@@ -0,0 +1,34 @@
+/*
+ * ipoib	Implementation of ipoib_header_ops here.
+ *
+ * Authors:	Wengang Wang <wen.gang.wang-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
+ */
+
+#include <linux/if_infiniband.h>
+
+static int ipoib_hard_header(struct sk_buff *skb,
+			     struct net_device *dev,
+			     unsigned short type,
+			     const void *daddr, const void *saddr, unsigned len)
+{
+	struct ipoib_header *header;
+	struct ipoib_cb *cb = (struct ipoib_cb *)skb->cb;
+
+	header = (struct ipoib_header *)skb_push(skb, sizeof(*header));
+
+	header->proto = htons(type);
+	header->reserved = 0;
+
+	/* we don't rely on dst_entry structure,  always stuff the
+	 * destination address into skb->cb so we can figure out where
+	 * to send the packet later.
+	 */
+	memcpy(cb->hwaddr, daddr, INFINIBAND_ALEN);
+
+	return sizeof(*header);
+}
+
+const struct header_ops ipoib_header_ops = {
+	.create	= ipoib_hard_header,
+};
+EXPORT_SYMBOL(ipoib_header_ops);
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] bonding: move ipoib_header_ops to vmlinux

[David Miller]

From: Wengang Wang <wen.gang.wang-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
Date: Tue, 30 Dec 2014 11:04:42 +0800

> When last slave of a bonding master is removed, the bonding then does not work.
> At the time if packet_snd is called against with a master net_device, it calls
> then header_ops->create which points to slave's header_ops. In case the slave
> is ipoib and the module is unloaded, header_ops would point to invalid address.
> Accessing it will cause problem.
> This patch tries to fix this issue by moving ipoib_header_ops to vmlinux to keep
> it valid even when ipoib module is unloaded.
> 
> Signed-off-by: Wengang Wang <wen.gang.wang-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>

Like others, I absolutely do not consider it sustainable to keep
moving header_ops implementations into the static kernel image.

We're just papering over the real problem, which is making sure all
dangling references to something really go away when we release/unload
an object.

Point it to a dummy set of ops and do a synchronize_net() or similar.

I'm not applying this patch, sorry.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html