Skip to content

Commit

Permalink
Merge branch 'xen-netback'
Browse files Browse the repository at this point in the history 
David Vrabel says:

====================
xen-netback: guest Rx queue drain and stall fixes

This series fixes two critical xen-netback bugs.

1. Netback may consume all of host memory by queuing an unlimited
   number of skb on the internal guest Rx queue.  This behaviour is
   guest triggerable.

2. Carrier flapping under high traffic rates which reduces
   performance.

The first patch is a prerequite.  Removing support for frontends with
feature-rx-notify makes it easier to reason about the correctness of
netback since it no longer has to support this outdated and broken
mode.
====================

Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
David S. Miller committed Oct 25, 2014
2 parents 5345c1d + ecf08d2 commit 4cc40af
Show file tree
Hide file tree
Showing 4 changed files with 251 additions and 203 deletions.
39 changes: 22 additions & 17 deletions drivers/net/xen-netback/common.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -176,10 +176,11 @@ struct xenvif_queue { /* Per-queue data for xenvif */
char rx_irq_name[IRQ_NAME_SIZE]; /* DEVNAME-qN-rx */
struct xen_netif_rx_back_ring rx;
struct sk_buff_head rx_queue;
RING_IDX rx_last_skb_slots;
unsigned long status;

struct timer_list rx_stalled;
unsigned int rx_queue_max;
unsigned int rx_queue_len;
unsigned long last_rx_time;
bool stalled;

struct gnttab_copy grant_copy_op[MAX_GRANT_COPY_OPS];

Expand All @@ -199,18 +200,14 @@ struct xenvif_queue { /* Per-queue data for xenvif */
struct xenvif_stats stats;
};

/* Maximum number of Rx slots a to-guest packet may use, including the
* slot needed for GSO meta-data.
*/
#define XEN_NETBK_RX_SLOTS_MAX (MAX_SKB_FRAGS + 1)

enum state_bit_shift {
/* This bit marks that the vif is connected */
VIF_STATUS_CONNECTED,
/* This bit signals the RX thread that queuing was stopped (in
* start_xmit), and either the timer fired or an RX interrupt came
*/
QUEUE_STATUS_RX_PURGE_EVENT,
/* This bit tells the interrupt handler that this queue was the reason
* for the carrier off, so it should kick the thread. Only queues which
* brought it down can turn on the carrier.
*/
QUEUE_STATUS_RX_STALLED
};

struct xenvif {
Expand All @@ -228,9 +225,6 @@ struct xenvif {
u8 ip_csum:1;
u8 ipv6_csum:1;

/* Internal feature information. */
u8 can_queue:1; /* can queue packets for receiver? */

/* Is this interface disabled? True when backend discovers
* frontend is rogue.
*/
Expand All @@ -240,6 +234,9 @@ struct xenvif {
/* Queues */
struct xenvif_queue *queues;
unsigned int num_queues; /* active queues, resource allocated */
unsigned int stalled_queues;

spinlock_t lock;

#ifdef CONFIG_DEBUG_FS
struct dentry *xenvif_dbg_root;
Expand All @@ -249,6 +246,14 @@ struct xenvif {
struct net_device *dev;
};

struct xenvif_rx_cb {
unsigned long expires;
int meta_slots_used;
bool full_coalesce;
};

#define XENVIF_RX_CB(skb) ((struct xenvif_rx_cb *)(skb)->cb)

static inline struct xenbus_device *xenvif_to_xenbus_device(struct xenvif *vif)
{
return to_xenbus_device(vif->dev->dev.parent);
Expand All @@ -272,8 +277,6 @@ void xenvif_xenbus_fini(void);

int xenvif_schedulable(struct xenvif *vif);

int xenvif_must_stop_queue(struct xenvif_queue *queue);

int xenvif_queue_stopped(struct xenvif_queue *queue);
void xenvif_wake_queue(struct xenvif_queue *queue);

Expand All @@ -296,6 +299,8 @@ void xenvif_kick_thread(struct xenvif_queue *queue);

int xenvif_dealloc_kthread(void *data);

void xenvif_rx_queue_tail(struct xenvif_queue *queue, struct sk_buff *skb);

/* Determine whether the needed number of slots (req) are available,
* and set req_event if not.
*/
Expand Down
74 changes: 15 additions & 59 deletions drivers/net/xen-netback/interface.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@
#define XENVIF_QUEUE_LENGTH 32
#define XENVIF_NAPI_WEIGHT 64

/* Number of bytes allowed on the internal guest Rx queue. */
#define XENVIF_RX_QUEUE_BYTES (XEN_NETIF_RX_RING_SIZE/2 * PAGE_SIZE)

/* This function is used to set SKBTX_DEV_ZEROCOPY as well as
* increasing the inflight counter. We need to increase the inflight
* counter because core driver calls into xenvif_zerocopy_callback
Expand All @@ -60,20 +63,11 @@ void xenvif_skb_zerocopy_complete(struct xenvif_queue *queue)
atomic_dec(&queue->inflight_packets);
}

static inline void xenvif_stop_queue(struct xenvif_queue *queue)
{
struct net_device *dev = queue->vif->dev;

if (!queue->vif->can_queue)
return;

netif_tx_stop_queue(netdev_get_tx_queue(dev, queue->id));
}

int xenvif_schedulable(struct xenvif *vif)
{
return netif_running(vif->dev) &&
test_bit(VIF_STATUS_CONNECTED, &vif->status);
test_bit(VIF_STATUS_CONNECTED, &vif->status) &&
!vif->disabled;
}

static irqreturn_t xenvif_tx_interrupt(int irq, void *dev_id)
Expand Down Expand Up @@ -114,16 +108,7 @@ int xenvif_poll(struct napi_struct *napi, int budget)
static irqreturn_t xenvif_rx_interrupt(int irq, void *dev_id)
{
struct xenvif_queue *queue = dev_id;
struct netdev_queue *net_queue =
netdev_get_tx_queue(queue->vif->dev, queue->id);

/* QUEUE_STATUS_RX_PURGE_EVENT is only set if either QDisc was off OR
* the carrier went down and this queue was previously blocked
*/
if (unlikely(netif_tx_queue_stopped(net_queue) ||
(!netif_carrier_ok(queue->vif->dev) &&
test_bit(QUEUE_STATUS_RX_STALLED, &queue->status))))
set_bit(QUEUE_STATUS_RX_PURGE_EVENT, &queue->status);
xenvif_kick_thread(queue);

return IRQ_HANDLED;
Expand Down Expand Up @@ -151,24 +136,13 @@ void xenvif_wake_queue(struct xenvif_queue *queue)
netif_tx_wake_queue(netdev_get_tx_queue(dev, id));
}

/* Callback to wake the queue's thread and turn the carrier off on timeout */
static void xenvif_rx_stalled(unsigned long data)
{
struct xenvif_queue *queue = (struct xenvif_queue *)data;

if (xenvif_queue_stopped(queue)) {
set_bit(QUEUE_STATUS_RX_PURGE_EVENT, &queue->status);
xenvif_kick_thread(queue);
}
}

static int xenvif_start_xmit(struct sk_buff *skb, struct net_device *dev)
{
struct xenvif *vif = netdev_priv(dev);
struct xenvif_queue *queue = NULL;
unsigned int num_queues = vif->num_queues;
u16 index;
int min_slots_needed;
struct xenvif_rx_cb *cb;

BUG_ON(skb->dev != dev);

Expand All @@ -191,30 +165,10 @@ static int xenvif_start_xmit(struct sk_buff *skb, struct net_device *dev)
!xenvif_schedulable(vif))
goto drop;

/* At best we'll need one slot for the header and one for each
* frag.
*/
min_slots_needed = 1 + skb_shinfo(skb)->nr_frags;

/* If the skb is GSO then we'll also need an extra slot for the
* metadata.
*/
if (skb_is_gso(skb))
min_slots_needed++;
cb = XENVIF_RX_CB(skb);
cb->expires = jiffies + rx_drain_timeout_jiffies;

/* If the skb can't possibly fit in the remaining slots
* then turn off the queue to give the ring a chance to
* drain.
*/
if (!xenvif_rx_ring_slots_available(queue, min_slots_needed)) {
queue->rx_stalled.function = xenvif_rx_stalled;
queue->rx_stalled.data = (unsigned long)queue;
xenvif_stop_queue(queue);
mod_timer(&queue->rx_stalled,
jiffies + rx_drain_timeout_jiffies);
}

skb_queue_tail(&queue->rx_queue, skb);
xenvif_rx_queue_tail(queue, skb);
xenvif_kick_thread(queue);

return NETDEV_TX_OK;
Expand Down Expand Up @@ -465,6 +419,8 @@ struct xenvif *xenvif_alloc(struct device *parent, domid_t domid,
vif->queues = NULL;
vif->num_queues = 0;

spin_lock_init(&vif->lock);

dev->netdev_ops = &xenvif_netdev_ops;
dev->hw_features = NETIF_F_SG |
NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
Expand Down Expand Up @@ -508,6 +464,8 @@ int xenvif_init_queue(struct xenvif_queue *queue)
init_timer(&queue->credit_timeout);
queue->credit_window_start = get_jiffies_64();

queue->rx_queue_max = XENVIF_RX_QUEUE_BYTES;

skb_queue_head_init(&queue->rx_queue);
skb_queue_head_init(&queue->tx_queue);

Expand Down Expand Up @@ -539,8 +497,6 @@ int xenvif_init_queue(struct xenvif_queue *queue)
queue->grant_tx_handle[i] = NETBACK_INVALID_HANDLE;
}

init_timer(&queue->rx_stalled);

return 0;
}

Expand All @@ -551,7 +507,6 @@ void xenvif_carrier_on(struct xenvif *vif)
dev_set_mtu(vif->dev, ETH_DATA_LEN);
netdev_update_features(vif->dev);
set_bit(VIF_STATUS_CONNECTED, &vif->status);
netif_carrier_on(vif->dev);
if (netif_running(vif->dev))
xenvif_up(vif);
rtnl_unlock();
Expand Down Expand Up @@ -611,6 +566,8 @@ int xenvif_connect(struct xenvif_queue *queue, unsigned long tx_ring_ref,
disable_irq(queue->rx_irq);
}

queue->stalled = true;

task = kthread_create(xenvif_kthread_guest_rx,
(void *)queue, "%s-guest-rx", queue->name);
if (IS_ERR(task)) {
Expand Down Expand Up @@ -674,7 +631,6 @@ void xenvif_disconnect(struct xenvif *vif)
netif_napi_del(&queue->napi);

if (queue->task) {
del_timer_sync(&queue->rx_stalled);
kthread_stop(queue->task);
queue->task = NULL;
}
Expand Down
Loading

0 comments on commit 4cc40af

Please sign in to comment.