RE: [Xen-devel][PATCH] VNIF: Using smart polling instead of event notification.

[Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>]

On Thu, 2009-10-01 at 15:02 +0100, Xu, Dongxiao wrote:
> We found that the event notification frequency is still high in some network 
> cases. NAPI polls only for a little
> time slot and does not efficient enough in our backend/frontend case. 
> Actually our patch repeated calling NAPI
> interface to do more polling, and netback will NOT notify netfront during 
> this period. Once netfront polling out
> all the data, and finds that there is no more data arrive/send during the 
> next 100ms, the timer will stop working 
> to end the polling. 

Isn't that an argument for improving NAPI rather than coding workarounds
into each driver? As physical NICs increase in speed it seems like NAPI
will need to increase its efficiency in a similar manner.

It would be interesting to run the patches past netdev@vger.

>     This filed 'smart_poll_active' is shared by netfront and netback,
> to indicate whether netfront is polling data. 
> So this filed is necessary for netback to notify netfront if this flag
> is not set. 

Right, but that doesn't justify sticking a netfront specific field in a
generic ring structure.

I can't help but wonder if there is some better way to communicate this
shared state. For instance netback sets the flag every time it notifies
the frontend, therefore isn't receiving an interrupt on the frontend
enough to infer that smart polling should be marked as active? Or can
this information be encoded into sring->{req,rsp}_event and the
smartpoll checks integrated into RING_PUSH_*_AND_CHECK_NOTIFY?

At the very least the field should be called {req,rsp}_smartpoll_active
to allow other consumers of the generic ring structure to use it if they
wish.

Ian.

> 
> Thanks!
> Dongxiao
> 
> ________________________________________
> From: Ian Campbell [Ian.Campbell@xxxxxxxxxx]
> Sent: Thursday, October 01, 2009 3:03 AM
> To: Xu, Dongxiao
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxx; Keir Fraser
> Subject: Re: [Xen-devel][PATCH] VNIF: Using smart polling instead of event    
>   notification.
> 
> You are adding a netfront specific field to the generic ring structure?
> That seems rather ugly.
> 
> Is this even necessary as a piece of shared state? Once netback and
> netfront have agreed, via xenstore, to use the feature netback seems to
> set the flag every time it would have previously notified netfront.
> 
> Ian.
> 
> On Thu, 2009-10-01 at 01:22 +0100, Xu, Dongxiao wrote:
> > Resend and put the patch in attachment.
> >
> > Patch the Xen version of ring.h
> >
> > Signed-off-by: Dongxiao Xu <dongxiao.xu@xxxxxxxxx>
> >
> > diff -r 8fc927798476 xen/include/public/io/ring.h
> > --- a/xen/include/public/io/ring.h      Tue Sep 01 11:36:51 2009 +0100
> > +++ b/xen/include/public/io/ring.h      Thu Oct 01 02:11:45 2009 +0800
> > @@ -97,7 +97,8 @@ struct __name##_sring {
> >  struct __name##_sring {                                                 \
> >      RING_IDX req_prod, req_event;                                       \
> >      RING_IDX rsp_prod, rsp_event;                                       \
> > -    uint8_t  pad[48];                                                   \
> > +    uint8_t  netfront_smartpoll_active;                                 \
> > +    uint8_t  pad[47];                                                   \
> >      union __name##_sring_entry ring[1]; /* variable-length */           \
> >  };                                                                      \
> >                                                                          \
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@xxxxxxxxxxxxxxxxxxx
> > http://lists.xensource.com/xen-devel


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel