Skip to content

Commit

Permalink
qdisc: validate skb without holding lock
Browse files Browse the repository at this point in the history 
Validation of skb can be pretty expensive :

GSO segmentation and/or checksum computations.

We can do this without holding qdisc lock, so that other cpus
can queue additional packets.

Trick is that requeued packets were already validated, so we carry
a boolean so that sch_direct_xmit() can validate a fresh skb list,
or directly use an old one.

Tested on 40Gb NIC (8 TX queues) and 200 concurrent flows, 48 threads
host.

Turning TSO on or off had no effect on throughput, only few more cpu
cycles. Lock contention on qdisc lock disappeared.

Same if disabling TX checksum offload.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
Eric Dumazet authored and David S. Miller committed Oct 3, 2014
1 parent 6a05880 commit 55a93b3
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 38 deletions.
2 changes: 1 addition & 1 deletion include/linux/netdevice.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2821,7 +2821,7 @@ int dev_set_mac_address(struct net_device *, struct sockaddr *);
int dev_change_carrier(struct net_device *, bool new_carrier);
int dev_get_phys_port_id(struct net_device *dev,
struct netdev_phys_port_id *ppid);
struct sk_buff *validate_xmit_skb(struct sk_buff *skb, struct net_device *dev);
struct sk_buff *validate_xmit_skb_list(struct sk_buff *skb, struct net_device *dev);
struct sk_buff *dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
struct netdev_queue *txq, int *ret);
int __dev_forward_skb(struct net_device *dev, struct sk_buff *skb);
Expand Down
2 changes: 1 addition & 1 deletion include/net/pkt_sched.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ void qdisc_put_stab(struct qdisc_size_table *tab);
void qdisc_warn_nonwc(const char *txt, struct Qdisc *qdisc);
int sch_direct_xmit(struct sk_buff *skb, struct Qdisc *q,
struct net_device *dev, struct netdev_queue *txq,
spinlock_t *root_lock);
spinlock_t *root_lock, bool validate);

void __qdisc_run(struct Qdisc *q);

Expand Down
29 changes: 26 additions & 3 deletions net/core/dev.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2655,7 +2655,7 @@ struct sk_buff *validate_xmit_vlan(struct sk_buff *skb, netdev_features_t featur
return skb;
}

struct sk_buff *validate_xmit_skb(struct sk_buff *skb, struct net_device *dev)
static struct sk_buff *validate_xmit_skb(struct sk_buff *skb, struct net_device *dev)
{
netdev_features_t features;

Expand Down Expand Up @@ -2720,6 +2720,30 @@ struct sk_buff *validate_xmit_skb(struct sk_buff *skb, struct net_device *dev)
return NULL;
}

struct sk_buff *validate_xmit_skb_list(struct sk_buff *skb, struct net_device *dev)
{
struct sk_buff *next, *head = NULL, *tail;

while (skb) {
next = skb->next;
skb->next = NULL;
skb = validate_xmit_skb(skb, dev);
if (skb) {
struct sk_buff *end = skb;

while (end->next)
end = end->next;
if (!head)
head = skb;
else
tail->next = skb;
tail = end;
}
skb = next;
}
return head;
}

static void qdisc_pkt_len_init(struct sk_buff *skb)
{
const struct skb_shared_info *shinfo = skb_shinfo(skb);
Expand Down Expand Up @@ -2786,8 +2810,7 @@ static inline int __dev_xmit_skb(struct sk_buff *skb, struct Qdisc *q,

qdisc_bstats_update(q, skb);

skb = validate_xmit_skb(skb, dev);
if (skb && sch_direct_xmit(skb, q, dev, txq, root_lock)) {
if (sch_direct_xmit(skb, q, dev, txq, root_lock, true)) {
if (unlikely(contended)) {
spin_unlock(&q->busylock);
contended = false;
Expand Down
61 changes: 28 additions & 33 deletions net/sched/sch_generic.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,40 +56,34 @@ static inline int dev_requeue_skb(struct sk_buff *skb, struct Qdisc *q)
return 0;
}

static struct sk_buff *try_bulk_dequeue_skb(struct Qdisc *q,
struct sk_buff *head_skb,
int bytelimit)
static void try_bulk_dequeue_skb(struct Qdisc *q,
struct sk_buff *skb,
const struct netdev_queue *txq)
{
struct sk_buff *skb, *tail_skb = head_skb;
int bytelimit = qdisc_avail_bulklimit(txq) - skb->len;

while (bytelimit > 0) {
skb = q->dequeue(q);
if (!skb)
break;
struct sk_buff *nskb = q->dequeue(q);

bytelimit -= skb->len; /* covers GSO len */
skb = validate_xmit_skb(skb, qdisc_dev(q));
if (!skb)
if (!nskb)
break;

while (tail_skb->next) /* GSO list goto tail */
tail_skb = tail_skb->next;

tail_skb->next = skb;
tail_skb = skb;
bytelimit -= nskb->len; /* covers GSO len */
skb->next = nskb;
skb = nskb;
}

return head_skb;
skb->next = NULL;
}

/* Note that dequeue_skb can possibly return a SKB list (via skb->next).
* A requeued skb (via q->gso_skb) can also be a SKB list.
*/
static inline struct sk_buff *dequeue_skb(struct Qdisc *q)
static struct sk_buff *dequeue_skb(struct Qdisc *q, bool *validate)
{
struct sk_buff *skb = q->gso_skb;
const struct netdev_queue *txq = q->dev_queue;

*validate = true;
if (unlikely(skb)) {
/* check the reason of requeuing without tx lock first */
txq = skb_get_tx_queue(txq->dev, skb);
Expand All @@ -98,21 +92,16 @@ static inline struct sk_buff *dequeue_skb(struct Qdisc *q)
q->q.qlen--;
} else
skb = NULL;
/* skb in gso_skb were already validated */
*validate = false;
} else {
if (!(q->flags & TCQ_F_ONETXQUEUE) ||
!netif_xmit_frozen_or_stopped(txq)) {
int bytelimit = qdisc_avail_bulklimit(txq);

skb = q->dequeue(q);
if (skb) {
bytelimit -= skb->len;
skb = validate_xmit_skb(skb, qdisc_dev(q));
}
if (skb && qdisc_may_bulk(q))
skb = try_bulk_dequeue_skb(q, skb, bytelimit);
try_bulk_dequeue_skb(q, skb, txq);
}
}

return skb;
}

Expand Down Expand Up @@ -156,19 +145,24 @@ static inline int handle_dev_cpu_collision(struct sk_buff *skb,
*/
int sch_direct_xmit(struct sk_buff *skb, struct Qdisc *q,
struct net_device *dev, struct netdev_queue *txq,
spinlock_t *root_lock)
spinlock_t *root_lock, bool validate)
{
int ret = NETDEV_TX_BUSY;

/* And release qdisc */
spin_unlock(root_lock);

HARD_TX_LOCK(dev, txq, smp_processor_id());
if (!netif_xmit_frozen_or_stopped(txq))
skb = dev_hard_start_xmit(skb, dev, txq, &ret);
/* Note that we validate skb (GSO, checksum, ...) outside of locks */
if (validate)
skb = validate_xmit_skb_list(skb, dev);

HARD_TX_UNLOCK(dev, txq);
if (skb) {
HARD_TX_LOCK(dev, txq, smp_processor_id());
if (!netif_xmit_frozen_or_stopped(txq))
skb = dev_hard_start_xmit(skb, dev, txq, &ret);

HARD_TX_UNLOCK(dev, txq);
}
spin_lock(root_lock);

if (dev_xmit_complete(ret)) {
Expand Down Expand Up @@ -217,9 +211,10 @@ static inline int qdisc_restart(struct Qdisc *q)
struct net_device *dev;
spinlock_t *root_lock;
struct sk_buff *skb;
bool validate;

/* Dequeue packet */
skb = dequeue_skb(q);
skb = dequeue_skb(q, &validate);
if (unlikely(!skb))
return 0;

Expand All @@ -229,7 +224,7 @@ static inline int qdisc_restart(struct Qdisc *q)
dev = qdisc_dev(q);
txq = skb_get_tx_queue(dev, skb);

return sch_direct_xmit(skb, q, dev, txq, root_lock);
return sch_direct_xmit(skb, q, dev, txq, root_lock, validate);
}

void __qdisc_run(struct Qdisc *q)
Expand Down

0 comments on commit 55a93b3

Please sign in to comment.