Re: [Xen-devel] [RFC PATCH V3 13/16] netback: stub for multi receive protocol support.

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Mon, Jan 30, 2012 at 02:45:31PM +0000, Wei Liu wrote:
> Refactor netback, make stub for mutli receive protocols. Also stub

multi.

> existing code as protocol 0.

Why not 1?

Why do we need a new rework without anything using it besides
the existing framework? OR if you are, you should say which
patch is doing it...

> 
> Now the file layout becomes:
> 
>  - interface.c: xenvif interfaces
>  - xenbus.c: xenbus related functions
>  - netback.c: common functions for various protocols
> 
> For different protocols:
> 
>  - xenvif_rx_protocolX.h: header file for the protocol, including
>                           protocol structures and functions
>  - xenvif_rx_protocolX.c: implementations
> 
> To add a new protocol:
> 
>  - include protocol header in common.h
>  - modify XENVIF_MAX_RX_PROTOCOL in common.h
>  - add protocol structure in xenvif.rx union
>  - stub in xenbus.c
>  - modify Makefile
> 
> A protocol should define five functions:
> 
>  - setup: setup frontend / backend ring connections
>  - teardown: teardown frontend / backend ring connections
>  - start_xmit: host start xmit (i.e. guest need to do rx)
>  - event: rx completion event
>  - action: prepare host side data for guest rx
> 
.. snip..

> -
> -	return resp;
> -}
> -
>  static inline int rx_work_todo(struct xenvif *vif)
>  {
>  	return !skb_queue_empty(&vif->rx_queue);
> @@ -1507,8 +999,8 @@ int xenvif_kthread(void *data)
>  		if (kthread_should_stop())
>  			break;
>  
> -		if (rx_work_todo(vif))
> -			xenvif_rx_action(vif);
> +		if (rx_work_todo(vif) && vif->action)
> +			vif->action(vif);
>  	}
>  
>  	return 0;
> diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
> index 79499fc..4067286 100644
> --- a/drivers/net/xen-netback/xenbus.c
> +++ b/drivers/net/xen-netback/xenbus.c
> @@ -415,6 +415,7 @@ static int connect_rings(struct backend_info *be)
>  	unsigned long rx_ring_ref[NETBK_MAX_RING_PAGES];
>  	unsigned int  tx_ring_order;
>  	unsigned int  rx_ring_order;
> +	unsigned int  rx_protocol;
>  
>  	err = xenbus_gather(XBT_NIL, dev->otherend,
>  			    "event-channel", "%u", &evtchn, NULL);
> @@ -510,6 +511,11 @@ static int connect_rings(struct backend_info *be)
>  		}
>  	}
>  
> +	err = xenbus_scanf(XBT_NIL, dev->otherend, "rx-protocol",

feature-rx-protocol?

> +			   "%u", &rx_protocol);
> +	if (err < 0)
> +		rx_protocol = XENVIF_MIN_RX_PROTOCOL;
> +

You should check to see if the protocol is higher than what we can support.
The guest could be playing funny games and putting in 39432...


>  	err = xenbus_scanf(XBT_NIL, dev->otherend, "request-rx-copy", "%u",
>  			   &rx_copy);
>  	if (err == -ENOENT) {
> @@ -559,7 +565,7 @@ static int connect_rings(struct backend_info *be)
>  	err = xenvif_connect(vif,
>  			     tx_ring_ref, (1U << tx_ring_order),
>  			     rx_ring_ref, (1U << rx_ring_order),
> -			     evtchn);
> +			     evtchn, rx_protocol);
>  	if (err) {
>  		int i;
>  		xenbus_dev_fatal(dev, err,