Re: [Xen-devel] [PATCH] xen/netfront: handle compound page fragments on transmit

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2012-11-21 at 02:52 +0000, ANNIE LI wrote:
> 
> On 2012-11-20 19:40, Ian Campbell wrote:
> > An SKB paged fragment can consist of a compound page with order>  0.
> > However the netchannel protocol deals only in PAGE_SIZE frames.
> >
> > Handle this in xennet_make_frags by iterating over the frames which
> > make up the page.
> >
> > This is the netfront equivalent to 6a8ed462f16b for netback.
> >
> > Signed-off-by: Ian Campbell<ian.campbell@xxxxxxxxxx>
> > Cc: netdev@xxxxxxxxxxxxxxx
> > Cc: xen-devel@xxxxxxxxxxxxx
> > Cc: Eric Dumazet<edumazet@xxxxxxxxxx>
> > Cc: Konrad Rzeszutek Wilk<konrad@xxxxxxxxxx>
> > Cc: ANNIE LI<annie.li@xxxxxxxxxx>
> > Cc: Sander Eikelenboom<linux@xxxxxxxxxxxxxx>
> > Cc: Stefan Bader<stefan.bader@xxxxxxxxxxxxx>
> > ---
> >   drivers/net/xen-netfront.c |   58 
> > +++++++++++++++++++++++++++++++++----------
> >   1 files changed, 44 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
> > index caa0110..a12b99a 100644
> > --- a/drivers/net/xen-netfront.c
> > +++ b/drivers/net/xen-netfront.c
> > @@ -452,24 +452,54 @@ static void xennet_make_frags(struct sk_buff *skb, 
> > struct net_device *dev,
> >     /* Grant backend access to each skb fragment page. */
> >     for (i = 0; i<  frags; i++) {
> >             skb_frag_t *frag = skb_shinfo(skb)->frags + i;
> > +           struct page *page = skb_frag_page(frag);
> > +           unsigned long size = skb_frag_size(frag);
> > +           unsigned long offset = frag->page_offset;
> 
> There are following definitions at the beginning of xennet_make_frags,
> 
>          unsigned int offset = offset_in_page(data);
>          unsigned int len = skb_headlen(skb);

So they are, well spotted.

> Is it better to reuse those definitions, and not define new size and 
> offset again in this for loop? And unsigned int is enough here, right?

Yes to both.
> [...]
> Over 80 characters?
[...]
> Over 80 characters?

Both fixed, thanks for your review.

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel