* [PATCH net] bonding: avoid defaulting hard_header_len to ETH_HLEN on slave removal
@ 2017-04-27 17:29 Paolo Abeni
[not found] ` <733d454d3c36e99b55de5374c7664364975b171d.1493313626.git.pabeni-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
0 siblings, 1 reply; 4+ messages in thread
From: Paolo Abeni @ 2017-04-27 17:29 UTC (permalink / raw)
To: netdev-u79uwXL29TY76Z2rM5mHXA
Cc: Jay Vosburgh, David S. Miller, Honggang LI,
linux-rdma-u79uwXL29TY76Z2rM5mHXA
On slave list updates, the bonding driver computes its hard_header_len
as the maximum of all enslaved devices's hard_header_len.
If the slave list is empty, e.g. on last enslaved device removal,
ETH_HLEN is used.
Since the bonding header_ops are set only when the first enslaved
device is attached, the above can lead to header_ops->create()
being called with the wrong skb headroom in place.
If bond0 is configured on top of ipoib devices, with the
following commands:
ifup bond0
for slave in $BOND_SLAVES_LIST; do
ip link set dev $slave nomaster
done
ping -c 1 <ip on bond0 subnet>
we will obtain a skb_under_panic() with a similar call trace:
skb_push+0x3d/0x40
push_pseudo_header+0x17/0x30 [ib_ipoib]
ipoib_hard_header+0x4e/0x80 [ib_ipoib]
arp_create+0x12f/0x220
arp_send_dst.part.19+0x28/0x50
arp_solicit+0x115/0x290
neigh_probe+0x4d/0x70
__neigh_event_send+0xa7/0x230
neigh_resolve_output+0x12e/0x1c0
ip_finish_output2+0x14b/0x390
ip_finish_output+0x136/0x1e0
ip_output+0x76/0xe0
ip_local_out+0x35/0x40
ip_send_skb+0x19/0x40
ip_push_pending_frames+0x33/0x40
raw_sendmsg+0x7d3/0xb50
inet_sendmsg+0x31/0xb0
sock_sendmsg+0x38/0x50
SYSC_sendto+0x102/0x190
SyS_sendto+0xe/0x10
do_syscall_64+0x67/0x180
entry_SYSCALL64_slow_path+0x25/0x25
This change addresses the issue avoiding updating the bonding device
hard_header_len when the slaves list become empty, forbidding to
shrink it below the value used by header_ops->create().
The bug is there since commit 54ef31371407 ("[PATCH] bonding: Handle large
hard_header_len") but the panic can be triggered only since
commit fc791b633515 ("IB/ipoib: move back IB LL address into the hard
header").
Reported-by: Norbert P <noe-PRwTpj6vllL463JZfw7VRw@public.gmane.org>
Fixes: 54ef31371407 ("[PATCH] bonding: Handle large hard_header_len")
Fixes: fc791b633515 ("IB/ipoib: move back IB LL address into the hard header")
Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Paolo Abeni <pabeni-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
drivers/net/bonding/bond_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 8a4ba8b..34481c9 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1104,11 +1104,11 @@ static void bond_compute_features(struct bonding *bond)
gso_max_size = min(gso_max_size, slave->dev->gso_max_size);
gso_max_segs = min(gso_max_segs, slave->dev->gso_max_segs);
}
+ bond_dev->hard_header_len = max_hard_header_len;
done:
bond_dev->vlan_features = vlan_features;
bond_dev->hw_enc_features = enc_features | NETIF_F_GSO_ENCAP_ALL;
- bond_dev->hard_header_len = max_hard_header_len;
bond_dev->gso_max_segs = gso_max_segs;
netif_set_gso_max_size(bond_dev, gso_max_size);
--
2.9.3
--
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH net] bonding: avoid defaulting hard_header_len to ETH_HLEN on slave removal
[not found] ` <733d454d3c36e99b55de5374c7664364975b171d.1493313626.git.pabeni-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
@ 2017-04-27 22:54 ` Marcelo Ricardo Leitner
2017-04-27 23:08 ` Jay Vosburgh
2017-04-28 20:04 ` David Miller
2 siblings, 0 replies; 4+ messages in thread
From: Marcelo Ricardo Leitner @ 2017-04-27 22:54 UTC (permalink / raw)
To: Paolo Abeni
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, Jay Vosburgh, David S. Miller,
Honggang LI, linux-rdma-u79uwXL29TY76Z2rM5mHXA
On Thu, Apr 27, 2017 at 07:29:34PM +0200, Paolo Abeni wrote:
> On slave list updates, the bonding driver computes its hard_header_len
> as the maximum of all enslaved devices's hard_header_len.
> If the slave list is empty, e.g. on last enslaved device removal,
> ETH_HLEN is used.
>
> Since the bonding header_ops are set only when the first enslaved
> device is attached, the above can lead to header_ops->create()
> being called with the wrong skb headroom in place.
>
> If bond0 is configured on top of ipoib devices, with the
> following commands:
>
> ifup bond0
> for slave in $BOND_SLAVES_LIST; do
> ip link set dev $slave nomaster
> done
> ping -c 1 <ip on bond0 subnet>
>
> we will obtain a skb_under_panic() with a similar call trace:
> skb_push+0x3d/0x40
> push_pseudo_header+0x17/0x30 [ib_ipoib]
> ipoib_hard_header+0x4e/0x80 [ib_ipoib]
> arp_create+0x12f/0x220
> arp_send_dst.part.19+0x28/0x50
> arp_solicit+0x115/0x290
> neigh_probe+0x4d/0x70
> __neigh_event_send+0xa7/0x230
> neigh_resolve_output+0x12e/0x1c0
> ip_finish_output2+0x14b/0x390
> ip_finish_output+0x136/0x1e0
> ip_output+0x76/0xe0
> ip_local_out+0x35/0x40
> ip_send_skb+0x19/0x40
> ip_push_pending_frames+0x33/0x40
> raw_sendmsg+0x7d3/0xb50
> inet_sendmsg+0x31/0xb0
> sock_sendmsg+0x38/0x50
> SYSC_sendto+0x102/0x190
> SyS_sendto+0xe/0x10
> do_syscall_64+0x67/0x180
> entry_SYSCALL64_slow_path+0x25/0x25
>
> This change addresses the issue avoiding updating the bonding device
> hard_header_len when the slaves list become empty, forbidding to
> shrink it below the value used by header_ops->create().
>
> The bug is there since commit 54ef31371407 ("[PATCH] bonding: Handle large
> hard_header_len") but the panic can be triggered only since
> commit fc791b633515 ("IB/ipoib: move back IB LL address into the hard
> header").
>
> Reported-by: Norbert P <noe-PRwTpj6vllL463JZfw7VRw@public.gmane.org>
> Fixes: 54ef31371407 ("[PATCH] bonding: Handle large hard_header_len")
> Fixes: fc791b633515 ("IB/ipoib: move back IB LL address into the hard header")
> Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Paolo Abeni <pabeni-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
Thanks Paolo.
> drivers/net/bonding/bond_main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 8a4ba8b..34481c9 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -1104,11 +1104,11 @@ static void bond_compute_features(struct bonding *bond)
> gso_max_size = min(gso_max_size, slave->dev->gso_max_size);
> gso_max_segs = min(gso_max_segs, slave->dev->gso_max_segs);
> }
> + bond_dev->hard_header_len = max_hard_header_len;
>
> done:
> bond_dev->vlan_features = vlan_features;
> bond_dev->hw_enc_features = enc_features | NETIF_F_GSO_ENCAP_ALL;
> - bond_dev->hard_header_len = max_hard_header_len;
> bond_dev->gso_max_segs = gso_max_segs;
> netif_set_gso_max_size(bond_dev, gso_max_size);
>
> --
> 2.9.3
>
--
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net] bonding: avoid defaulting hard_header_len to ETH_HLEN on slave removal
[not found] ` <733d454d3c36e99b55de5374c7664364975b171d.1493313626.git.pabeni-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2017-04-27 22:54 ` Marcelo Ricardo Leitner
@ 2017-04-27 23:08 ` Jay Vosburgh
2017-04-28 20:04 ` David Miller
2 siblings, 0 replies; 4+ messages in thread
From: Jay Vosburgh @ 2017-04-27 23:08 UTC (permalink / raw)
To: Paolo Abeni
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, David S. Miller, Honggang LI,
linux-rdma-u79uwXL29TY76Z2rM5mHXA
Paolo Abeni <pabeni-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>On slave list updates, the bonding driver computes its hard_header_len
>as the maximum of all enslaved devices's hard_header_len.
>If the slave list is empty, e.g. on last enslaved device removal,
>ETH_HLEN is used.
>
>Since the bonding header_ops are set only when the first enslaved
>device is attached, the above can lead to header_ops->create()
>being called with the wrong skb headroom in place.
>
>If bond0 is configured on top of ipoib devices, with the
>following commands:
>
>ifup bond0
>for slave in $BOND_SLAVES_LIST; do
> ip link set dev $slave nomaster
>done
>ping -c 1 <ip on bond0 subnet>
>
>we will obtain a skb_under_panic() with a similar call trace:
> skb_push+0x3d/0x40
> push_pseudo_header+0x17/0x30 [ib_ipoib]
> ipoib_hard_header+0x4e/0x80 [ib_ipoib]
> arp_create+0x12f/0x220
> arp_send_dst.part.19+0x28/0x50
> arp_solicit+0x115/0x290
> neigh_probe+0x4d/0x70
> __neigh_event_send+0xa7/0x230
> neigh_resolve_output+0x12e/0x1c0
> ip_finish_output2+0x14b/0x390
> ip_finish_output+0x136/0x1e0
> ip_output+0x76/0xe0
> ip_local_out+0x35/0x40
> ip_send_skb+0x19/0x40
> ip_push_pending_frames+0x33/0x40
> raw_sendmsg+0x7d3/0xb50
> inet_sendmsg+0x31/0xb0
> sock_sendmsg+0x38/0x50
> SYSC_sendto+0x102/0x190
> SyS_sendto+0xe/0x10
> do_syscall_64+0x67/0x180
> entry_SYSCALL64_slow_path+0x25/0x25
>
>This change addresses the issue avoiding updating the bonding device
>hard_header_len when the slaves list become empty, forbidding to
>shrink it below the value used by header_ops->create().
>
>The bug is there since commit 54ef31371407 ("[PATCH] bonding: Handle large
>hard_header_len") but the panic can be triggered only since
>commit fc791b633515 ("IB/ipoib: move back IB LL address into the hard
>header").
>
>Reported-by: Norbert P <noe-PRwTpj6vllL463JZfw7VRw@public.gmane.org>
>Fixes: 54ef31371407 ("[PATCH] bonding: Handle large hard_header_len")
>Fixes: fc791b633515 ("IB/ipoib: move back IB LL address into the hard header")
>Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>Signed-off-by: Paolo Abeni <pabeni-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Jay Vosburgh <jay.vosburgh-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
> drivers/net/bonding/bond_main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index 8a4ba8b..34481c9 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -1104,11 +1104,11 @@ static void bond_compute_features(struct bonding *bond)
> gso_max_size = min(gso_max_size, slave->dev->gso_max_size);
> gso_max_segs = min(gso_max_segs, slave->dev->gso_max_segs);
> }
>+ bond_dev->hard_header_len = max_hard_header_len;
>
> done:
> bond_dev->vlan_features = vlan_features;
> bond_dev->hw_enc_features = enc_features | NETIF_F_GSO_ENCAP_ALL;
>- bond_dev->hard_header_len = max_hard_header_len;
> bond_dev->gso_max_segs = gso_max_segs;
> netif_set_gso_max_size(bond_dev, gso_max_size);
>
>--
>2.9.3
>
--
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net] bonding: avoid defaulting hard_header_len to ETH_HLEN on slave removal
[not found] ` <733d454d3c36e99b55de5374c7664364975b171d.1493313626.git.pabeni-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2017-04-27 22:54 ` Marcelo Ricardo Leitner
2017-04-27 23:08 ` Jay Vosburgh
@ 2017-04-28 20:04 ` David Miller
2 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2017-04-28 20:04 UTC (permalink / raw)
To: pabeni-H+wXaHxf7aLQT0dZR+AlfA
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, j.vosburgh-Re5JQEeQqe8AvxtiuMwx3w,
honli-H+wXaHxf7aLQT0dZR+AlfA, linux-rdma-u79uwXL29TY76Z2rM5mHXA
From: Paolo Abeni <pabeni-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Date: Thu, 27 Apr 2017 19:29:34 +0200
> On slave list updates, the bonding driver computes its hard_header_len
> as the maximum of all enslaved devices's hard_header_len.
> If the slave list is empty, e.g. on last enslaved device removal,
> ETH_HLEN is used.
>
> Since the bonding header_ops are set only when the first enslaved
> device is attached, the above can lead to header_ops->create()
> being called with the wrong skb headroom in place.
>
> If bond0 is configured on top of ipoib devices, with the
> following commands:
>
> ifup bond0
> for slave in $BOND_SLAVES_LIST; do
> ip link set dev $slave nomaster
> done
> ping -c 1 <ip on bond0 subnet>
>
> we will obtain a skb_under_panic() with a similar call trace:
...
> This change addresses the issue avoiding updating the bonding device
> hard_header_len when the slaves list become empty, forbidding to
> shrink it below the value used by header_ops->create().
>
> The bug is there since commit 54ef31371407 ("[PATCH] bonding: Handle large
> hard_header_len") but the panic can be triggered only since
> commit fc791b633515 ("IB/ipoib: move back IB LL address into the hard
> header").
>
> Reported-by: Norbert P <noe-PRwTpj6vllL463JZfw7VRw@public.gmane.org>
> Fixes: 54ef31371407 ("[PATCH] bonding: Handle large hard_header_len")
> Fixes: fc791b633515 ("IB/ipoib: move back IB LL address into the hard header")
> Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Paolo Abeni <pabeni-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Applied, thanks.
--
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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-04-28 20:04 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-27 17:29 [PATCH net] bonding: avoid defaulting hard_header_len to ETH_HLEN on slave removal Paolo Abeni
[not found] ` <733d454d3c36e99b55de5374c7664364975b171d.1493313626.git.pabeni-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2017-04-27 22:54 ` Marcelo Ricardo Leitner
2017-04-27 23:08 ` Jay Vosburgh
2017-04-28 20:04 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.