Re: [Xen-devel] Is: SKB_MAX_LEN bites again. Was: Re: bug disabling guest interface

[annie li <annie.li@xxxxxxxxxx>]

On 2013-3-12 23:25, Wei Liu wrote:
> On Tue, 2013-03-12 at 15:07 +0000, ANNIE LI wrote:
> > On 2013-3-12 20:18, Wei Liu wrote:
> > > On Tue, 2013-03-12 at 11:40 +0000, Ian Campbell wrote:
> > > > On Sun, 2013-03-10 at 19:18 +0000, Wei Liu wrote:
> > > > > On Sat, Mar 09, 2013 at 02:57:16AM +0000, Ian Campbell wrote:
> > > > > > > > - change MAX_SKB_FRAGS to 19 to accommodate all guests
> > > > > > Changing MAX_SKB_FRAGS is *not* an option upstream. This might be a
> > > > > > useful local hack but we need to drop the idea as a long term fix.
> > > > > >
> > > > > > > Ugh. The negotiations between host and guest is probably the best
> > > > > > > choice. The issues you are going to hit are that you might need
> > > > > > > to redo the skbs to match what the frontend's max is.
> > > > > > IMHO the right fix is for netback to coalesce as it copies from the
> > > > > > frontend if it needs to do so, it is copying anyway so it should be
> > > > > > cheap enough. I thought we had discussed this and someone was working on
> > > > > > implementing it. If not Annie then perhaps it was Matt or Siva (both now
> > > > > > CC'd)
> > > > > As a short term fix, can we use skb_linearize() if skb->nr_frags>=
> > > > > MAX_SKB_FRAGS?
> > > > No, because that would require changing the frontend, while this fix
> > > > needs to be in the backend if older guests are to continue working.
> > > >
> > > > You can't use skb_linearize in netback as is because you would first
> > > > need to be able to build the skb with nr_frags>= MAX_SKB_FRAGS in order
> > > > to pass it to that function.
> > > Yes, the idea is to define NETBK_MAX_SKB_FRAGS to some bigger number
> > > (say 20) to accommodate the possible maximum number of frags in
> > > frontend. The thing that truly matters it the skb->len, which should be
> > > <64K, nr_frags is not important.
> > I doubt this would work since you can not build out skb with nr_frags >=
> > MAX_SKB_FRAGS. See following code in skbuff.h,
> >
> > #if (65536/PAGE_SIZE + 1) < 16
> > #define MAX_SKB_FRAGS 16UL
> > #else
> > #define MAX_SKB_FRAGS (65536/PAGE_SIZE + 1)
> > #endif
> >
> > and every skb contains MAX_SKB_FRAGS frags,
> >
> > struct skb_shared_info {
> > ....
> > skb_frag_t      frags[MAX_SKB_FRAGS];
> > ...
> > }
> So I haven't gone this far to trigger this. :-(
>
> I just double checked with printk, nr_frags has not exceeded backend
> MAX_SBK_FRAGS even if I have frontend MAX_SKB_FRAGS = 25.
You mean you changed MAX_SKB_FRAGS to 25 in skbuff.h? Did your frontend 
send out skbs with 25 frags?
> > Coalescing frags before building skb could avoid this issue.
> "before building skb" is a bit vague to me. Is it netbk_count_request?
> Is it xen_netbk_fill_frags? If it is the latter, we are already too late
> to fix this. If it is the former, we don't even start copying.
I meant coalescing when doing grant copy after netbk_count_request and 
before xen_netbk_fill_frags. But we should take care of data offset of 
per request and merge them into pages.
Thanks
Annie
> Wei

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