Re: [PATCH RFC 3/4] xen-netfront: Factor queue-specific data into queue struct.

[Wei Liu <wei.liu2@citrix.com>]

On Wed, Jan 15, 2014 at 04:23:23PM +0000, Andrew J. Bennieston wrote:
[...]
> +
> +struct netfront_queue {
> +	unsigned int number; /* Queue number, 0-based */
> +	char name[IFNAMSIZ+4]; /* DEVNAME-qN */
> +	struct netfront_info *info;
>  
>  	struct napi_struct napi;
>  
> @@ -93,10 +96,8 @@ struct netfront_info {
>  	unsigned int tx_evtchn, rx_evtchn;
>  	unsigned int tx_irq, rx_irq;
>  	/* Only used when split event channels support is enabled */
> -	char tx_irq_name[IFNAMSIZ+4]; /* DEVNAME-tx */
> -	char rx_irq_name[IFNAMSIZ+4]; /* DEVNAME-rx */
> -
> -	struct xenbus_device *xbdev;
> +	char tx_irq_name[IFNAMSIZ+7]; /* DEVNAME-qN-tx */
> +	char rx_irq_name[IFNAMSIZ+7]; /* DEVNAME-qN-rx */

Basically you're anticipating the maximum number of queues is below 10
here. I think leaving one more byte here won't hurt, just in case you
will have more than 10 queues. The same goes to netback as well.

In your next patch you have max_queue as 16 by default, which has
already broken your assumption here already. :-)

>  
>  	spinlock_t   tx_lock;
>  	struct xen_netif_tx_front_ring tx;
> @@ -139,6 +140,17 @@ struct netfront_info {
>  	unsigned long rx_pfn_array[NET_RX_RING_SIZE];
>  	struct multicall_entry rx_mcl[NET_RX_RING_SIZE+1];
>  	struct mmu_update rx_mmu[NET_RX_RING_SIZE];
> +};
> +
[...]
>  static int xennet_start_xmit(struct sk_buff *skb, struct net_device *dev)
>  {
>  	unsigned short id;
> @@ -555,6 +575,15 @@ static int xennet_start_xmit(struct sk_buff *skb, struct net_device *dev)
>  	unsigned int offset = offset_in_page(data);
>  	unsigned int len = skb_headlen(skb);
>  	unsigned long flags;
> +	struct netfront_queue *queue = NULL;
> +	u16 queue_index;
> +
> +	/* Drop the packet if no queues are set up */
> +	if (np->num_queues < 1 || np->queues == NULL)

Same as the comment in netback, you won't need both.

And the rest of the patch is basically replacing np with queue and
putting things in loops. So I stopped here...

Wei.