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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 31 Jan 2012 11:03:44 +0000
From:	Wei Liu <wei.liu2@...rix.com>
To:	Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
CC:	<wei.liu2@...rix.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"xen-devel@...ts.xensource.com" <xen-devel@...ts.xensource.com>,
	"Ian Campbell" <Ian.Campbell@...rix.com>
Subject: Re: [Xen-devel] [RFC PATCH V3 13/16] netback: stub for multi
 receive protocol support.

On Mon, 2012-01-30 at 21:47 +0000, Konrad Rzeszutek Wilk wrote:
> On Mon, Jan 30, 2012 at 02:45:31PM +0000, Wei Liu wrote:
> > Refactor netback, make stub for mutli receive protocols. Also stub
> 
> multi.
> 

Good catch, thanks.

> > existing code as protocol 0.
> 
> Why not 1?
> 

We have some existing xenolinux code which has not been upstreamed calls
this protocol 0, just try to be compatible.

> 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...
> 

It is not in use at the moment, and will be in use in the future.

> > 
> > 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?
> 

This is not a feature switch. Does it make sense to add "feature-"
prefix?

> > +			   "%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...
> 
> 

Good point.


Wei.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html