aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorEric Dumazet <edumazet@google.com>2012-06-12 02:03:51 -0400
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>2012-07-16 11:47:37 -0400
commit76886430203577bfa3b437630058aa0346cb3168 (patch)
treeec69ca8e54dd76c352a0be929c3d497c21f7e90d
parent4acd9a65e16de99c4afe89e4ac8a16b501c92450 (diff)
bonding: Fix corrupted queue_mapping
[ Upstream commit 5ee31c6898ea5537fcea160999d60dc63bc0c305 ] In the transmit path of the bonding driver, skb->cb is used to stash the skb->queue_mapping so that the bonding device can set its own queue mapping. This value becomes corrupted since the skb->cb is also used in __dev_xmit_skb. When transmitting through bonding driver, bond_select_queue is called from dev_queue_xmit. In bond_select_queue the original skb->queue_mapping is copied into skb->cb (via bond_queue_mapping) and skb->queue_mapping is overwritten with the bond driver queue. Subsequently in dev_queue_xmit, __dev_xmit_skb is called which writes the packet length into skb->cb, thereby overwriting the stashed queue mappping. In bond_dev_queue_xmit (called from hard_start_xmit), the queue mapping for the skb is set to the stashed value which is now the skb length and hence is an invalid queue for the slave device. If we want to save skb->queue_mapping into skb->cb[], best place is to add a field in struct qdisc_skb_cb, to make sure it wont conflict with other layers (eg : Qdiscc, Infiniband...) This patchs also makes sure (struct qdisc_skb_cb)->data is aligned on 8 bytes : netem qdisc for example assumes it can store an u64 in it, without misalignment penalty. Note : we only have 20 bytes left in (struct qdisc_skb_cb)->data[]. The largest user is CHOKe and it fills it. Based on a previous patch from Tom Herbert. Signed-off-by: Eric Dumazet <edumazet@google.com> Reported-by: Tom Herbert <therbert@google.com> Cc: John Fastabend <john.r.fastabend@intel.com> Cc: Roland Dreier <roland@kernel.org> Acked-by: Neil Horman <nhorman@tuxdriver.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-rw-r--r--drivers/net/bonding/bond_main.c9
-rw-r--r--include/net/sch_generic.h7
2 files changed, 10 insertions, 6 deletions
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index e6da842cb28..504e201f3fd 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -77,6 +77,7 @@
77#include <net/route.h> 77#include <net/route.h>
78#include <net/net_namespace.h> 78#include <net/net_namespace.h>
79#include <net/netns/generic.h> 79#include <net/netns/generic.h>
80#include <net/pkt_sched.h>
80#include "bonding.h" 81#include "bonding.h"
81#include "bond_3ad.h" 82#include "bond_3ad.h"
82#include "bond_alb.h" 83#include "bond_alb.h"
@@ -388,8 +389,6 @@ struct vlan_entry *bond_next_vlan(struct bonding *bond, struct vlan_entry *curr)
388 return next; 389 return next;
389} 390}
390 391
391#define bond_queue_mapping(skb) (*(u16 *)((skb)->cb))
392
393/** 392/**
394 * bond_dev_queue_xmit - Prepare skb for xmit. 393 * bond_dev_queue_xmit - Prepare skb for xmit.
395 * 394 *
@@ -403,7 +402,9 @@ int bond_dev_queue_xmit(struct bonding *bond, struct sk_buff *skb,
403 skb->dev = slave_dev; 402 skb->dev = slave_dev;
404 skb->priority = 1; 403 skb->priority = 1;
405 404
406 skb->queue_mapping = bond_queue_mapping(skb); 405 BUILD_BUG_ON(sizeof(skb->queue_mapping) !=
406 sizeof(qdisc_skb_cb(skb)->bond_queue_mapping));
407 skb->queue_mapping = qdisc_skb_cb(skb)->bond_queue_mapping;
407 408
408 if (unlikely(netpoll_tx_running(slave_dev))) 409 if (unlikely(netpoll_tx_running(slave_dev)))
409 bond_netpoll_send_skb(bond_get_slave_by_dev(bond, slave_dev), skb); 410 bond_netpoll_send_skb(bond_get_slave_by_dev(bond, slave_dev), skb);
@@ -4240,7 +4241,7 @@ static u16 bond_select_queue(struct net_device *dev, struct sk_buff *skb)
4240 /* 4241 /*
4241 * Save the original txq to restore before passing to the driver 4242 * Save the original txq to restore before passing to the driver
4242 */ 4243 */
4243 bond_queue_mapping(skb) = skb->queue_mapping; 4244 qdisc_skb_cb(skb)->bond_queue_mapping = skb->queue_mapping;
4244 4245
4245 if (unlikely(txq >= dev->real_num_tx_queues)) { 4246 if (unlikely(txq >= dev->real_num_tx_queues)) {
4246 do { 4247 do {
diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index f1fbe2d5e05..af2e047fc06 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -219,13 +219,16 @@ struct tcf_proto {
219 219
220struct qdisc_skb_cb { 220struct qdisc_skb_cb {
221 unsigned int pkt_len; 221 unsigned int pkt_len;
222 unsigned char data[24]; 222 u16 bond_queue_mapping;
223 u16 _pad;
224 unsigned char data[20];
223}; 225};
224 226
225static inline void qdisc_cb_private_validate(const struct sk_buff *skb, int sz) 227static inline void qdisc_cb_private_validate(const struct sk_buff *skb, int sz)
226{ 228{
227 struct qdisc_skb_cb *qcb; 229 struct qdisc_skb_cb *qcb;
228 BUILD_BUG_ON(sizeof(skb->cb) < sizeof(unsigned int) + sz); 230
231 BUILD_BUG_ON(sizeof(skb->cb) < offsetof(struct qdisc_skb_cb, data) + sz);
229 BUILD_BUG_ON(sizeof(qcb->data) < sz); 232 BUILD_BUG_ON(sizeof(qcb->data) < sz);
230} 233}
231 234