Xen devel - Mar 2013 - bug disabling guest interface

Hi all

We have recently encountered a bug in xen causing guest interface to be
disabled in certain conditions:
- dom0 with this patch https://patchwork.kernel.org/patch/2164311/
- guest kernel built in way that makes guest MAX_SKB_FRAGS larger than
MAX_SKB_FRAGS on dom0
For example dom0 kernel from here
http://au1.mirror.crc.id.au/repo/el6/x86_64/ 
Linux blade-b.maemo.org 3.7.9-1.el6xen.x86_64 #1 SMP Mon Feb 18 14:46:35
EST 2013 x86_64 x86_64 x86_64 GNU/Linux
And domU 
Linux stage.maemo.org 2.6.32-33-server #72-Ubuntu SMP Fri Jul 29 21:21:55
UTC 2011 x86_64 x86_64 x86_64 GNU/Linux

In this example dom0 MAX_SKB_FRAGS is at 17 while MAX_SKB_FRAGS at domU is
18

I did find a post making a comment about it
http://lists.xen.org/archives/html/xen-devel/2012-11/msg01072.html
>This highlights a couple of issues, the main one is that implicitly
>including MAX_SKB_FRAGS in the PV net protocol is just madness.

More details from host:
xenbr1: port 8(vif51.0) entered forwarding state
vif vif-51-0 vif51.0: Too many frags
vif vif-51-0 vif51.0: fatal error; disabling device
xenbr1: port 8(vif51.0) entered disabled state

[root@blade-b ~]# xm info
host                   : blade-b.maemo.org
release                : 3.7.9-1.el6xen.x86_64
version                : #1 SMP Mon Feb 18 14:46:35 EST 2013
machine                : x86_64
nr_cpus                : 12
nr_nodes               : 1
cores_per_socket       : 6
threads_per_core       : 2
cpu_mhz                : 2000
hw_caps                :
bfebfbff:2c100800:00000000:00003f40:17bee3ff:00000000:00000001:00000000
virt_caps              : hvm hvm_directio
total_memory           : 32735
free_memory            : 1076
free_cpus              : 0
xen_major              : 4
xen_minor              : 2
xen_extra              : .1
xen_caps               : xen-3.0-x86_64 xen-3.0-x86_32p hvm-3.0-x86_32
hvm-3.0-x86_32p hvm-3.0-x86_64 
xen_scheduler          : credit
xen_pagesize           : 4096
platform_params        : virt_start=0xffff800000000000
xen_changeset          : unavailable
xen_commandline        : dom0_mem=1024M loglvl=all guest_loglvl=all
cpuidle=0 cpufreq=none
cc_compiler            : gcc (GCC) 4.4.6 20120305 (Red Hat 4.4.6-4)
cc_compile_by          : mockbuild
cc_compile_domain      : crc.id.au
cc_compile_date        : Sat Feb 16 19:16:38 EST 2013
xend_config_format     : 4

On Tue, 2013-03-05 at 22:51 +0000, Jacek Milewicz wrote:
> Hi all
> 
> We have recently encountered a bug in xen causing guest interface to be
> disabled in certain conditions:
> - dom0 with this patch https://patchwork.kernel.org/patch/2164311/
> - guest kernel built in way that makes guest MAX_SKB_FRAGS larger than
> MAX_SKB_FRAGS on dom0
I''m interested in knowing more information. Have you always been using
MAX_SKB_FRAGS > default_value (16 or 17) for you DomUs?

If it is this case, you''re likely to be hitting this error path all the
time. It is just that XSA-39 chooses to shutdown vif if there is
something wrong. So my further questions is, do you notice any
performance problem in the case that netback silently drops your
packets?


Wei.

> I''m interested in knowing more information. Have you always been
using
> MAX_SKB_FRAGS > default_value (16 or 17) for you DomUs?
18 used to be default value on older kernels (anything older than 
https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/commit/include/linux/skbuff.h?id=9d4dde5215779f4099730194ad30624fdba3d8b2
-
2011-12-23 21:51:18)
> If it is this case, you''re likely to be hitting this error path
all the
> time. It is just
> that XSA-39 chooses to shutdown vif if there is something wrong. So my
> further questions is, do you notice any performance problem in the case 
> that
> netback silently drops your packets?
Basically what happened in our case is VMs were migrated to new host with 
new kernel, before that dom0 used to run kernel with MAX_SKB_FRAGS==18 so it 
was same for both dom0 and domU.
New host has kernel with XSA-39 fixes and smaller MAX_SKB_FRAGS (17), and 
that''s when we encountered that bug, we''ve never tried kernel
with smaller
MAX_SKB_FRAGS but without XSA_39 fix.

On Thu, Mar 7, 2013 at 2:34 AM, Jacek Milewicz
<jacekowski@jacekowski.org> wrote:
>> I''m interested in knowing more information. Have you always
been using
>> MAX_SKB_FRAGS > default_value (16 or 17) for you DomUs?
>
> 18 used to be default value on older kernels (anything older than
>
https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/commit/include/linux/skbuff.h?id=9d4dde5215779f4099730194ad30624fdba3d8b2
-
> 2011-12-23 21:51:18)
>
>> If it is this case, you''re likely to be hitting this error
path all the
>> time. It is just
>> that XSA-39 chooses to shutdown vif if there is something wrong. So my
>> further questions is, do you notice any performance problem in the case
>> that
>> netback silently drops your packets?
>
> Basically what happened in our case is VMs were migrated to new host with
> new kernel, before that dom0 used to run kernel with MAX_SKB_FRAGS==18 so
it
> was same for both dom0 and domU.
> New host has kernel with XSA-39 fixes and smaller MAX_SKB_FRAGS (17), and
> that''s when we encountered that bug, we''ve never tried
kernel with smaller
> MAX_SKB_FRAGS but without XSA_39 fix.
Sorry for my intrusion of this thread.  I would like to highlight that
this also affect guests/domUs running Windows Server 2003 in HVM and
the logs are similar with the OP:

kernel: vif vif-2-0 vifvm104.0: Too many frags
kernel: vif vif-2-0 vifvm104.0: fatal error; disabling device
kernel: eth0: port 4(vifvm104.0) entered disabled state

Thanks.

Kindest regards,
Giam Teck Choon

Hi all,

I''ve been following this with some interest - as I''ve been
getting
numerous reports of this happening recently.

As Jacek Milewicz posted recently:
http://lists.xen.org/archives/html/xen-devel/2013-03/msg00404.html

It seems to affect both linux and windows DomUs.

Is there any further information on a possible fix for this? Thoughts? 
Ideas? As it seems to be hitting multiple sites, I''d like to get it 
fixed asap...

-- 
Steven Haigh

Email: netwiz@crc.id.au
Web: https://www.crc.id.au
Phone: (03) 9001 6090 - 0412 935 897
Fax: (03) 8338 0299



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

Hi All
> 
> Is there any further information on a possible fix for this? Thoughts?
> Ideas? As it seems to be hitting multiple sites, I''d like to get
it
fixed asap...

There are 3 ways I can see this fixed:
- update guests so all have same MAX_SKB_FRAGS (that includes windows
drivers (windows drivers use 19 for MAX_SKB_FRAGS))
-add some sort of negotiation between host and guest
- change MAX_SKB_FRAGS to 19 to accommodate all guests

Unfortunately first one requires changes to the guest and most don''t
have
that luxury. So the only way I see it could be fixed without breaking
compatibility even more  is

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 821c7f4..82de0f5 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -143,8 +143,8 @@ struct sk_buff;
  * Since GRO uses frags we allocate at least 16 regardless of page
  * size.
  */
-#if (65536/PAGE_SIZE + 1) < 16
-#define MAX_SKB_FRAGS 16UL
+#if (65536/PAGE_SIZE + 1) < 19
+#define MAX_SKB_FRAGS 19UL
 #else
 #define MAX_SKB_FRAGS (65536/PAGE_SIZE + 1)
 #endif

On Fri, Mar 08, 2013 at 08:36:37PM +0100, Jacek Milewicz
wrote:
> Hi All
> 
> > 
> > Is there any further information on a possible fix for this? Thoughts?
> > Ideas? As it seems to be hitting multiple sites, I''d like to
get it
> fixed asap...
> 
> There are 3 ways I can see this fixed:
> - update guests so all have same MAX_SKB_FRAGS (that includes windows
> drivers (windows drivers use 19 for MAX_SKB_FRAGS))
> -add some sort of negotiation between host and guest
> - change MAX_SKB_FRAGS to 19 to accommodate all guests
> 
> Unfortunately first one requires changes to the guest and most
don''t have
> that luxury. So the only way I see it could be fixed without breaking
> compatibility even more  is
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.

Annie, Wei, Ian - were there some RFC patches floating around
for this?
> 
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 821c7f4..82de0f5 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -143,8 +143,8 @@ struct sk_buff;
>   * Since GRO uses frags we allocate at least 16 regardless of page
>   * size.
>   */
> -#if (65536/PAGE_SIZE + 1) < 16
> -#define MAX_SKB_FRAGS 16UL
> +#if (65536/PAGE_SIZE + 1) < 19
> +#define MAX_SKB_FRAGS 19UL
>  #else
>  #define MAX_SKB_FRAGS (65536/PAGE_SIZE + 1)
>  #endif
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
>

> > > Is there any further information on a possible fix for this?
Thoughts?
> > > Ideas? As it seems to be hitting multiple sites, I''d
like to get it
> > fixed asap...
> >
> > There are 3 ways I can see this fixed:
> > - update guests so all have same MAX_SKB_FRAGS (that includes windows
> > drivers (windows drivers use 19 for MAX_SKB_FRAGS)) -add some sort of
> > negotiation between host and guest
> > - change MAX_SKB_FRAGS to 19 to accommodate all guests
> >
> > Unfortunately first one requires changes to the guest and most
don''t
> > have that luxury. So the only way I see it could be fixed without
> > breaking compatibility even more  is
> 
> 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.
> 
> Annie, Wei, Ian - were there some RFC patches floating around for this?
> 
As much as I agree that negotiation is the best option in long term. It
relies on people upgrading their kernels - and this is not the case (only
people running kernels from before 2011-12-23 are affected by this bug
(I''m not sure about other platforms)).
And in a lot of cases (VPS providers) there is no easy way to upgrade
guest kernels.

On 9/03/2013 9:09 AM, Jacek Milewicz wrote:
>>>> Is there any further information on a possible fix for this?
> Thoughts?
>>>> Ideas? As it seems to be hitting multiple sites, I''d
like to get it
>>> fixed asap...
>>>
>>> There are 3 ways I can see this fixed:
>>> - update guests so all have same MAX_SKB_FRAGS (that includes
windows
>>> drivers (windows drivers use 19 for MAX_SKB_FRAGS)) -add some sort
of
>>> negotiation between host and guest
>>> - change MAX_SKB_FRAGS to 19 to accommodate all guests
>>>
>>> Unfortunately first one requires changes to the guest and most
don''t
>>> have that luxury. So the only way I see it could be fixed without
>>> breaking compatibility even more  is
>>
>> 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.
>>
>> Annie, Wei, Ian - were there some RFC patches floating around for this?
>>
>
> As much as I agree that negotiation is the best option in long term. It
> relies on people upgrading their kernels - and this is not the case (only
> people running kernels from before 2011-12-23 are affected by this bug
> (I''m not sure about other platforms)).
> And in a lot of cases (VPS providers) there is no easy way to upgrade
> guest kernels.
For what its worth, I''ve increased the value of MAX_SKB_FRAGS to 19 on 
my Dom0 kernel and pushed out new packages... I''ll see if I get any 
further reports...

-- 
Steven Haigh

Email: netwiz@crc.id.au
Web: https://www.crc.id.au
Phone: (03) 9001 6090 - 0412 935 897
Fax: (03) 8338 0299



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

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

If necessary netback could even allocate a larger order head in order to
accommodate very large packets, but I don''t expect that to be required
to fix the immediate issue we are seeing (but gives flexibility)

This should get us past the immediate issue of the upstream change from
18->16  frags thing. Longer term the negotiation will allow us to avoid
future incompatible changes in guest and host network stacks, as well as
allowing frontends on other OSes (in particular Windows) to havea better
chance of to DTRT.
 
> Annie, Wei, Ian - were there some RFC patches floating around
> for this?
> 
> > 
> > diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> > index 821c7f4..82de0f5 100644
> > --- a/include/linux/skbuff.h
> > +++ b/include/linux/skbuff.h
> > @@ -143,8 +143,8 @@ struct sk_buff;
> >   * Since GRO uses frags we allocate at least 16 regardless of page
> >   * size.
> >   */
> > -#if (65536/PAGE_SIZE + 1) < 16
> > -#define MAX_SKB_FRAGS 16UL
> > +#if (65536/PAGE_SIZE + 1) < 19
> > +#define MAX_SKB_FRAGS 19UL
> >  #else
> >  #define MAX_SKB_FRAGS (65536/PAGE_SIZE + 1)
> >  #endif
> > 
> > 
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xen.org
> > http://lists.xen.org/xen-devel
> >

On 9/03/2013 1:57 PM, 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.
I agree. Its a hack that seems to work until we have something else to 
offer though. I''ll be honest, the internals of the kernel interactions 
like this is a bit beyond my knowledge - but I get quite a few emails of 
stuff going strange. This one has increased dramatically in the past week.
>> 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)
I did see some talk about it on the xen-devel lists quite some time ago 
- however it seemed to die with no outcome that I could find. Maybe its 
a good time for a nudge on the matter :)
> If necessary netback could even allocate a larger order head in order to
> accommodate very large packets, but I don''t expect that to be
required
> to fix the immediate issue we are seeing (but gives flexibility)
>
> This should get us past the immediate issue of the upstream change from
> 18->16  frags thing. Longer term the negotiation will allow us to avoid
> future incompatible changes in guest and host network stacks, as well as
> allowing frontends on other OSes (in particular Windows) to havea better
> chance of to DTRT.
Whatever the fix, it has to be on the Dom0 kernel - as stated by others 
in the past, it isn''t a feasible fix to include changes on the client. 
There would be too many different guest OSes that would make 
implementation a nightmare - or in fact impossible.
>> Annie, Wei, Ian - were there some RFC patches floating around
>> for this?
I didn''t stumble across any in the list archives that I hunted for. Not
to say they don''t exist, but I didn''t find them if they do
exist.

-- 
Steven Haigh

Email: netwiz@crc.id.au
Web: https://www.crc.id.au
Phone: (03) 9001 6090 - 0412 935 897
Fax: (03) 8338 0299



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

Saturday, March 9, 2013, 3:57:16 AM, you 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)
> If necessary netback could even allocate a larger order head in order to
> accommodate very large packets, but I don''t expect that to be
required
> to fix the immediate issue we are seeing (but gives flexibility)
> This should get us past the immediate issue of the upstream change from
18->>16  frags thing. Longer term the negotiation will allow us to
avoid
> future incompatible changes in guest and host network stacks, as well as
> allowing frontends on other OSes (in particular Windows) to havea better
> chance of to DTRT.
>  
>> Annie, Wei, Ian - were there some RFC patches floating around
>> for this?
I think you refer to this thread ?

http://lists.xen.org/archives/html/xen-devel/2013-01/msg00198.html

And i see the annie''s patch and my response haven''t made the
list because annie dropped xen-devel from the response.
I will forward them ..

--
Sander




>> 
>> > 
>> > diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
>> > index 821c7f4..82de0f5 100644
>> > --- a/include/linux/skbuff.h
>> > +++ b/include/linux/skbuff.h
>> > @@ -143,8 +143,8 @@ struct sk_buff;
>> >   * Since GRO uses frags we allocate at least 16 regardless of
page
>> >   * size.
>> >   */
>> > -#if (65536/PAGE_SIZE + 1) < 16
>> > -#define MAX_SKB_FRAGS 16UL
>> > +#if (65536/PAGE_SIZE + 1) < 19
>> > +#define MAX_SKB_FRAGS 19UL
>> >  #else
>> >  #define MAX_SKB_FRAGS (65536/PAGE_SIZE + 1)
>> >  #endif
>> > 
>> > 
>> > _______________________________________________
>> > Xen-devel mailing list
>> > Xen-devel@lists.xen.org
>> > http://lists.xen.org/xen-devel
>> >

On 2013-3-9 4:36, Konrad Rzeszutek Wilk wrote:
> On Fri, Mar 08, 2013 at 08:36:37PM +0100, Jacek Milewicz wrote:
>> Hi All
>>
>>> Is there any further information on a possible fix for this?
Thoughts?
>>> Ideas? As it seems to be hitting multiple sites, I''d like
to get it
>> fixed asap...
>>
>> There are 3 ways I can see this fixed:
>> - update guests so all have same MAX_SKB_FRAGS (that includes windows
>> drivers (windows drivers use 19 for MAX_SKB_FRAGS))
>> -add some sort of negotiation between host and guest
>> - change MAX_SKB_FRAGS to 19 to accommodate all guests
>>
>> Unfortunately first one requires changes to the guest and most
don''t have
>> that luxury. So the only way I see it could be fixed without breaking
>> compatibility even more  is
> 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.
>
> Annie, Wei, Ian - were there some RFC patches floating around
> for this?
Actually I did plan to do it, but failed to start it because of windows 
WHQL things.

Thanks
Annie
>
>> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
>> index 821c7f4..82de0f5 100644
>> --- a/include/linux/skbuff.h
>> +++ b/include/linux/skbuff.h
>> @@ -143,8 +143,8 @@ struct sk_buff;
>>    * Since GRO uses frags we allocate at least 16 regardless of page
>>    * size.
>>    */
>> -#if (65536/PAGE_SIZE + 1)<  16
>> -#define MAX_SKB_FRAGS 16UL
>> +#if (65536/PAGE_SIZE + 1)<  19
>> +#define MAX_SKB_FRAGS 19UL
>>   #else
>>   #define MAX_SKB_FRAGS (65536/PAGE_SIZE + 1)
>>   #endif
>>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org
>> http://lists.xen.org/xen-devel
>>

On 2013-3-9 10:57, 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)
Sorry, I have been working on some windows whql thing these days, so did 
not start it till now.
I did do some patch for netfront before(Sander mentioned later), which 
did some segment for large skbs in netfront. This is different from what 
you mentioned. I assume we need to do some work in both netback and 
netfront.

Thanks
Annie
>
> If necessary netback could even allocate a larger order head in order to
> accommodate very large packets, but I don''t expect that to be
required
> to fix the immediate issue we are seeing (but gives flexibility)
>
> This should get us past the immediate issue of the upstream change from
> 18->16  frags thing. Longer term the negotiation will allow us to avoid
> future incompatible changes in guest and host network stacks, as well as
> allowing frontends on other OSes (in particular Windows) to havea better
> chance of to DTRT.
>
>> Annie, Wei, Ian - were there some RFC patches floating around
>> for this?
>>
>>> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
>>> index 821c7f4..82de0f5 100644
>>> --- a/include/linux/skbuff.h
>>> +++ b/include/linux/skbuff.h
>>> @@ -143,8 +143,8 @@ struct sk_buff;
>>>    * Since GRO uses frags we allocate at least 16 regardless of
page
>>>    * size.
>>>    */
>>> -#if (65536/PAGE_SIZE + 1)<  16
>>> -#define MAX_SKB_FRAGS 16UL
>>> +#if (65536/PAGE_SIZE + 1)<  19
>>> +#define MAX_SKB_FRAGS 19UL
>>>   #else
>>>   #define MAX_SKB_FRAGS (65536/PAGE_SIZE + 1)
>>>   #endif
>>>
>>>
>>> _______________________________________________
>>> Xen-devel mailing list
>>> Xen-devel@lists.xen.org
>>> http://lists.xen.org/xen-devel
>>>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

On 2013-3-9 20:53, Sander Eikelenboom wrote:
> Saturday, March 9, 2013, 3:57:16 AM, you 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)
>> If necessary netback could even allocate a larger order head in order
to
>> accommodate very large packets, but I don''t expect that to be
required
>> to fix the immediate issue we are seeing (but gives flexibility)
>> This should get us past the immediate issue of the upstream change from
> 18->>16  frags thing. Longer term the negotiation will allow us to
avoid
>> future incompatible changes in guest and host network stacks, as well
as
>> allowing frontends on other OSes (in particular Windows) to havea
better
>> chance of to DTRT.
>>
>>> Annie, Wei, Ian - were there some RFC patches floating around
>>> for this?
> I think you refer to this thread ?
>
> http://lists.xen.org/archives/html/xen-devel/2013-01/msg00198.html
>
> And i see the annie''s patch and my response haven''t made
the list because annie dropped xen-devel from the response.
> I will forward them ..
Hi Sander,

This is a patch in which I tried to do some fix in netfront, the thread 
you mentioned is facing the same issue with this one.

Thanks
Annie
>
> --
> Sander
>
>
>
>
>
>>>> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
>>>> index 821c7f4..82de0f5 100644
>>>> --- a/include/linux/skbuff.h
>>>> +++ b/include/linux/skbuff.h
>>>> @@ -143,8 +143,8 @@ struct sk_buff;
>>>>    * Since GRO uses frags we allocate at least 16 regardless of
page
>>>>    * size.
>>>>    */
>>>> -#if (65536/PAGE_SIZE + 1)<  16
>>>> -#define MAX_SKB_FRAGS 16UL
>>>> +#if (65536/PAGE_SIZE + 1)<  19
>>>> +#define MAX_SKB_FRAGS 19UL
>>>>   #else
>>>>   #define MAX_SKB_FRAGS (65536/PAGE_SIZE + 1)
>>>>   #endif
>>>>
>>>>
>>>> _______________________________________________
>>>> Xen-devel mailing list
>>>> Xen-devel@lists.xen.org
>>>> http://lists.xen.org/xen-devel
>>>>
>
>
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

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?


Wei.

On Sun, 2013-03-10 at 04:49 +0000, ANNIE LI wrote:
> I did do some patch for netfront before(Sander mentioned later), which 
> did some segment for large skbs in netfront. This is different from what 
> you mentioned. I assume we need to do some work in both netback and 
> netfront.
The fix I''m talking about in
<1362797836.8941.189.camel@hastur.hellion.org.uk> should be netback
only.

Ian.

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.

I''d worry about failing the higher order allocations needed to
linearize
in this way too -- better to coalesce as much as possible into single
page frags.

Ian.

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 wrote some code during weekend and it seemed to work, but I have not
verify it extensively.


Wei.

On Fri, Mar 08, 2013 at 11:09:56PM +0100, Jacek Milewicz
wrote:
> > > > Is there any further information on a possible fix for this?
> Thoughts?
> > > > Ideas? As it seems to be hitting multiple sites,
I''d like to get it
> > > fixed asap...
> > >
> > > There are 3 ways I can see this fixed:
> > > - update guests so all have same MAX_SKB_FRAGS (that includes
windows
> > > drivers (windows drivers use 19 for MAX_SKB_FRAGS)) -add some
sort of
> > > negotiation between host and guest
> > > - change MAX_SKB_FRAGS to 19 to accommodate all guests
> > >
> > > Unfortunately first one requires changes to the guest and most
don''t
> > > have that luxury. So the only way I see it could be fixed without
> > > breaking compatibility even more  is
> > 
> > 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.
> > 
> > Annie, Wei, Ian - were there some RFC patches floating around for
this?
> > 
> 
> As much as I agree that negotiation is the best option in long term. It
> relies on people upgrading their kernels - and this is not the case (only
> people running kernels from before 2011-12-23 are affected by this bug
> (I''m not sure about other platforms)).
> And in a lot of cases (VPS providers) there is no easy way to upgrade
> guest kernels. 
Sure. The other option in the xen-netback might be a configurable option
to let net-back know that it is running with older guests which expect
18 size MAX_SKB_FRAGS. That way we have the negotiation part, the code to
deal with backend MAX_SKB_FRAGS != frontend MAX_SKB_FRAGS and the value that
the system admin provides.

What I am saying is that if we get the negotiation protcol figured out
properly we can provide a per-guest value that the system admin
can over-write to say: "I know that this guest uses _this_ size".

On Tue, Mar 12, 2013 at 10:48:07AM -0400, Konrad Rzeszutek Wilk
wrote:
> On Fri, Mar 08, 2013 at 11:09:56PM +0100, Jacek Milewicz wrote:
> > > > > Is there any further information on a possible fix for
this?
> > Thoughts?
> > > > > Ideas? As it seems to be hitting multiple sites,
I''d like to get it
> > > > fixed asap...
> > > >
> > > > There are 3 ways I can see this fixed:
> > > > - update guests so all have same MAX_SKB_FRAGS (that
includes windows
> > > > drivers (windows drivers use 19 for MAX_SKB_FRAGS)) -add
some sort of
> > > > negotiation between host and guest
> > > > - change MAX_SKB_FRAGS to 19 to accommodate all guests
> > > >
> > > > Unfortunately first one requires changes to the guest and
most don''t
> > > > have that luxury. So the only way I see it could be fixed
without
> > > > breaking compatibility even more  is
> > > 
> > > 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.
> > > 
> > > Annie, Wei, Ian - were there some RFC patches floating around for
this?
> > > 
> > 
> > As much as I agree that negotiation is the best option in long term.
It
> > relies on people upgrading their kernels - and this is not the case
(only
> > people running kernels from before 2011-12-23 are affected by this bug
> > (I''m not sure about other platforms)).
> > And in a lot of cases (VPS providers) there is no easy way to upgrade
> > guest kernels. 
> 
> Sure. But the other option in the xen-netback might be a configurable
option
> to let net-back know that it is running with older guests which expect
> 18 size MAX_SKB_FRAGS. That way we have the negotiation part, the code to
> deal with backend MAX_SKB_FRAGS != frontend MAX_SKB_FRAGS and the value
that
> the system admin provides.
> 
Re-sending as I think it never made it out.

On Tue, 2013-03-12 at 14:49 +0000, Konrad Rzeszutek Wilk
wrote:
> On Fri, Mar 08, 2013 at 11:09:56PM +0100, Jacek Milewicz wrote:
> > > > > Is there any further information on a possible fix for
this?
> > Thoughts?
> > > > > Ideas? As it seems to be hitting multiple sites,
I''d like to get it
> > > > fixed asap...
> > > >
> > > > There are 3 ways I can see this fixed:
> > > > - update guests so all have same MAX_SKB_FRAGS (that
includes windows
> > > > drivers (windows drivers use 19 for MAX_SKB_FRAGS)) -add
some sort of
> > > > negotiation between host and guest
> > > > - change MAX_SKB_FRAGS to 19 to accommodate all guests
> > > >
> > > > Unfortunately first one requires changes to the guest and
most don''t
> > > > have that luxury. So the only way I see it could be fixed
without
> > > > breaking compatibility even more  is
> > > 
> > > 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.
> > > 
> > > Annie, Wei, Ian - were there some RFC patches floating around for
this?
> > > 
> > 
> > As much as I agree that negotiation is the best option in long term.
It
> > relies on people upgrading their kernels - and this is not the case
(only
> > people running kernels from before 2011-12-23 are affected by this bug
> > (I''m not sure about other platforms)).
> > And in a lot of cases (VPS providers) there is no easy way to upgrade
> > guest kernels. 
> 
> Sure. The other option in the xen-netback might be a configurable option
> to let net-back know that it is running with older guests which expect
> 18 size MAX_SKB_FRAGS. That way we have the negotiation part, the code to
> deal with backend MAX_SKB_FRAGS != frontend MAX_SKB_FRAGS and the value
that
> the system admin provides.
I think this is over complicating things. netback can and should be
modified to be able to cope with guests using frags == 18 (or larger)
even if it only itself supports frags == 16 by coalescing things in the
obvious way. This works for all guest supplied frames up to 64K in size,
regardless of the number of slots they use to represent that 64K.

The negotiation stuff is pretty orthogonal to this and is more about the
MTU of GSO frames. In terms of priority this is a distant second to the
above.

Ian.

On 03/13/2013 01:56 AM, Ian Campbell wrote:
> On Tue, 2013-03-12 at 14:49 +0000, Konrad Rzeszutek Wilk wrote:
>> On Fri, Mar 08, 2013 at 11:09:56PM +0100, Jacek Milewicz wrote:
>>>>>> Is there any further information on a possible fix for
this?
>>> Thoughts?
>>>>>> Ideas? As it seems to be hitting multiple sites,
I''d like to get it
>>>>> fixed asap...
>>>>>
>>>>> There are 3 ways I can see this fixed:
>>>>> - update guests so all have same MAX_SKB_FRAGS (that
includes windows
>>>>> drivers (windows drivers use 19 for MAX_SKB_FRAGS)) -add
some sort of
>>>>> negotiation between host and guest
>>>>> - change MAX_SKB_FRAGS to 19 to accommodate all guests
>>>>>
>>>>> Unfortunately first one requires changes to the guest and
most don''t
>>>>> have that luxury. So the only way I see it could be fixed
without
>>>>> breaking compatibility even more  is
>>>>
>>>> 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.
>>>>
>>>> Annie, Wei, Ian - were there some RFC patches floating around
for this?
>>>>
>>>
>>> As much as I agree that negotiation is the best option in long
term. It
>>> relies on people upgrading their kernels - and this is not the case
(only
>>> people running kernels from before 2011-12-23 are affected by this
bug
>>> (I''m not sure about other platforms)).
>>> And in a lot of cases (VPS providers) there is no easy way to
upgrade
>>> guest kernels.
>>
>> Sure. The other option in the xen-netback might be a configurable
option
>> to let net-back know that it is running with older guests which expect
>> 18 size MAX_SKB_FRAGS. That way we have the negotiation part, the code
to
>> deal with backend MAX_SKB_FRAGS != frontend MAX_SKB_FRAGS and the value
that
>> the system admin provides.
>
> I think this is over complicating things. netback can and should be
> modified to be able to cope with guests using frags == 18 (or larger)
> even if it only itself supports frags == 16 by coalescing things in the
> obvious way. This works for all guest supplied frames up to 64K in size,
> regardless of the number of slots they use to represent that 64K.
On this, while we just hacked this in the kernel source to increase 
MAX_SKB_FRAGS to 19, I believe Jacek Milewicz tested 18 to work fine on 
linux guests, but it seems Windows guests would still fail unless it was 
increased to 19.

-- 
Steven Haigh

Email: netwiz@crc.id.au
Web: https://www.crc.id.au
Phone: (03) 9001 6090 - 0412 935 897
Fax: (03) 8338 0299

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];
...
}

Coalescing frags before building skb could avoid this issue.

Thanks
Annie
>
> I wrote some code during weekend and it seemed to work, but I have not
> verify it extensively.
>
>
> Wei.
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

On Tue, 2013-03-12 at 15:05 +0000, Steven Haigh wrote:
> On 03/13/2013 01:56 AM, Ian Campbell wrote:
> > On Tue, 2013-03-12 at 14:49 +0000, Konrad Rzeszutek Wilk wrote:
> >> On Fri, Mar 08, 2013 at 11:09:56PM +0100, Jacek Milewicz wrote:
> >>>>>> Is there any further information on a possible fix
for this?
> >>> Thoughts?
> >>>>>> Ideas? As it seems to be hitting multiple sites,
I''d like to get it
> >>>>> fixed asap...
> >>>>>
> >>>>> There are 3 ways I can see this fixed:
> >>>>> - update guests so all have same MAX_SKB_FRAGS (that
includes windows
> >>>>> drivers (windows drivers use 19 for MAX_SKB_FRAGS))
-add some sort of
> >>>>> negotiation between host and guest
> >>>>> - change MAX_SKB_FRAGS to 19 to accommodate all guests
> >>>>>
> >>>>> Unfortunately first one requires changes to the guest
and most don''t
> >>>>> have that luxury. So the only way I see it could be
fixed without
> >>>>> breaking compatibility even more  is
> >>>>
> >>>> 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.
> >>>>
> >>>> Annie, Wei, Ian - were there some RFC patches floating
around for this?
> >>>>
> >>>
> >>> As much as I agree that negotiation is the best option in long
term. It
> >>> relies on people upgrading their kernels - and this is not the
case (only
> >>> people running kernels from before 2011-12-23 are affected by
this bug
> >>> (I''m not sure about other platforms)).
> >>> And in a lot of cases (VPS providers) there is no easy way to
upgrade
> >>> guest kernels.
> >>
> >> Sure. The other option in the xen-netback might be a configurable
option
> >> to let net-back know that it is running with older guests which
expect
> >> 18 size MAX_SKB_FRAGS. That way we have the negotiation part, the
code to
> >> deal with backend MAX_SKB_FRAGS != frontend MAX_SKB_FRAGS and the
value that
> >> the system admin provides.
> >
> > I think this is over complicating things. netback can and should be
> > modified to be able to cope with guests using frags == 18 (or larger)
> > even if it only itself supports frags == 16 by coalescing things in
the
> > obvious way. This works for all guest supplied frames up to 64K in
size,
> > regardless of the number of slots they use to represent that 64K.
> 
> On this, while we just hacked this in the kernel source to increase 
> MAX_SKB_FRAGS to 19, I believe Jacek Milewicz tested 18 to work fine on 
> linux guests, but it seems Windows guests would still fail unless it was 
> increased to 19.
No netback has TTBOMK ever support 19, so I don''t know how those guests
ever worked, but regardless doing coalescing in the backend would fix
this at least for frames which are <64K in size.

Ian.

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


Wei

On Tue, 2013-03-12 at 15:25 +0000, 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.
> 
> > 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.
> 
> 
Actually the copy is done with Hypervisor, I don''t know how it is
possible to do coalesce while copying.

FWIW I came up with an idea. Netback maintains a ring of skb_frag_t
groups. Each group has NETBK_SKB_MAX_FRAGS frags. So overall the size of
this ring is MAX_PENGING_REQS * NETBK_SKB_MAX_FRAGS *
sizeof(skb_frag_t). If NETBK_SKB_MAX_FRAGS == 20, skb_frag_t size == 16,
this ring is 80K large.

If we detect frontend''s frags > backend''s SKB_MAX_FRAGS, we
switch to
use the new ring, and set skb->nr_frags to some specific number (say -1)
and has frag[0].page.p set to ring index. Then we copy data as usual.
Later in before xen_netbk_fill_frags if we detect this skb is should be
constructed via previous skb frag ring, we accommodate those frags to
that skb.

if (skb should be constructed via frag ring)
    construct_skb()
else /* normal path
    xen_netbk_fill_frags

This idea is essentially adding a slow path though, because
construct_skb() potentially needs to copy / move data around.


Wei.

On Tue, Mar 12, 2013 at 08:13:39PM +0000, Wei Liu wrote:
> 
> Actually the copy is done with Hypervisor, I don''t know how it is
> possible to do coalesce while copying.
> 
> FWIW I came up with an idea. Netback maintains a ring of skb_frag_t
> groups. Each group has NETBK_SKB_MAX_FRAGS frags. So overall the size of
> this ring is MAX_PENGING_REQS * NETBK_SKB_MAX_FRAGS *
> sizeof(skb_frag_t). If NETBK_SKB_MAX_FRAGS == 20, skb_frag_t size == 16,
> this ring is 80K large.
Just realized two things: 1. the ring size estimation is wrong; 2. we
don''t need a extra ring for this. :-)

We can alter how frags are handled inside netback to solve this issue.
We can store start of pending_idx and end of pending_idx in
skb->frags[0] and skb->frags[1] if frontend''s MAX_SKB_FRAGS is
larger
than backend''s.


Wei.
> 
> If we detect frontend''s frags > backend''s
SKB_MAX_FRAGS, we switch to
> use the new ring, and set skb->nr_frags to some specific number (say -1)
> and has frag[0].page.p set to ring index. Then we copy data as usual.
> Later in before xen_netbk_fill_frags if we detect this skb is should be
> constructed via previous skb frag ring, we accommodate those frags to
> that skb.
> 
> if (skb should be constructed via frag ring)
>     construct_skb()
> else /* normal path
>     xen_netbk_fill_frags
> 
> This idea is essentially adding a slow path though, because
> construct_skb() potentially needs to copy / move data around.
> 
> 
> Wei.
>

> >
> > On this, while we just hacked this in the kernel source to increase
> > MAX_SKB_FRAGS to 19, I believe Jacek Milewicz tested 18 to work fine
on
> > linux guests, but it seems Windows guests would still fail unless it
was
> > increased to 19.
> 
> No netback has TTBOMK ever support 19, so I don''t know how those
guests
> ever worked, but regardless doing coalescing in the backend would fix
> this at least for frames which are <64K in size.
> 
Well Windows could easily generate a packet with more than 19 buffers, and I
have engineered situations where it could be much larger than that, but you
aren''t going to see such packets during regular usage.

Is there any consensus yet on what form max_buffers negotiation would take? Is
it as simple as the frontend and backend each writing the maximum value they can
support (without further work) to their respective xenstores?

What backends exist these days? Are BSD and Solaris backends still in existence?
What are their respective limitations?

James

This seems to create network performance issues. It seems that asterisk
under unstable xen and 3.8.2 kernel gives lots of unreachable errors


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

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
>

On 2013-3-13 4:13, Wei Liu wrote:
> Actually the copy is done with Hypervisor, I don''t know how it is
> possible to do coalesce while copying.
>
My understanding is we do coalescing in netback during grant copy. For 
example,

                  gop->source.u.ref = txreq.gref;
                  gop->source.domid = vif->domid;
                  gop->source.offset = txreq.offset;

                  gop->dest.u.gmfn = virt_to_mfn(page_address(page));  
--  here should be the page where we coalesce data into, it is likely 
the page is the one which previous request used.
                  gop->dest.domid = DOMID_SELF;
                  gop->dest.offset = txreq.offset;  -- dest.offset 
should be changed to something which represents the offset where we 
start coalescing data of this request

                  gop->len = txreq.size;  -- len should be the total len 
of data which we coalesce into a page
                  gop->flags = GNTCOPY_source_gref;

With this coalescing, it is possible that we could decrease the total 
frags number of skb in netback.

Thanks
Annie

On Tue, 2013-03-12 at 20:13 +0000, Wei Liu wrote:
> On Tue, 2013-03-12 at 15:25 +0000, 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.
> > 
> > > 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.
> > 
> > 
> 
> Actually the copy is done with Hypervisor, I don''t know how it is
> possible to do coalesce while copying.
This can be done trivially by making the appropriate gnttab_copy
hypercalls to copy from the guest memory into dom0 buffers, coalescing
as you go. IOW you split each ring slot request into multiple (batched)
copies in order to fill buffers etc.
> FWIW I came up with an idea. Netback maintains a ring of skb_frag_t
> groups. Each group has NETBK_SKB_MAX_FRAGS frags. So overall the size of
> this ring is MAX_PENGING_REQS * NETBK_SKB_MAX_FRAGS *
> sizeof(skb_frag_t). If NETBK_SKB_MAX_FRAGS == 20, skb_frag_t size == 16,
> this ring is 80K large.
> 
> If we detect frontend''s frags > backend''s
SKB_MAX_FRAGS, we switch to
> use the new ring, and set skb->nr_frags to some specific number (say -1)
> and has frag[0].page.p set to ring index. Then we copy data as usual.
> Later in before xen_netbk_fill_frags if we detect this skb is should be
> constructed via previous skb frag ring, we accommodate those frags to
> that skb.
> 
> if (skb should be constructed via frag ring)
>     construct_skb()
> else /* normal path
>     xen_netbk_fill_frags
> 
> This idea is essentially adding a slow path though, because
> construct_skb() potentially needs to copy / move data around.
This is insanely complex for the problem at hand IMHO.

Ian.

On Wed, 2013-03-13 at 09:43 +0000, Ian Campbell wrote:
> > 
> > This idea is essentially adding a slow path though, because
> > construct_skb() potentially needs to copy / move data around.
> 
> This is insanely complex for the problem at hand IMHO.
> 
Hah, I guess my brain has the ability to overcomplicate things. :-)


Wei.
> Ian.
>

On Wed, 2013-03-13 at 06:22 +0000, annie li wrote:
> 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 
Yes. In frontend''s kernel.
> send out skbs with 25 frags?
> 
No. The maximum number I see is 17.

I didn''t spend much time on this though. It''s just a little
experiment
during my weekend.


Wei.