| Message ID | 1416893768-21369-1-git-send-email-wen.gang.wang@oracle.com (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
From: Wengang Wang <wen.gang.wang@oracle.com> 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@oracle.com> 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -J --- -Jay Vosburgh, jay.vosburgh@canonical.com -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
? 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 linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 >> -- >> 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 > > -- > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 >>> -- > -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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@oracle.com> > 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@oracle.com> > 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Wengang <wen.gang.wang@oracle.com> 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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@oracle.com> + */ +#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@oracle.com> + */ +#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@oracle.com> + */ + +#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);
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@oracle.com> --- 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