hw/net/spapr_llan.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
From 09905773a00e417d3a37c12350d9e55466fdce8a Mon Sep 17 00:00:00 2001
From: Elena Afanasova <eafanasova@gmail.com>
Date: Fri, 9 Oct 2020 06:41:36 -0700
Subject: [PATCH] hw/net: move allocation to the heap due to very large stack
frame
Signed-off-by: Elena Afanasova <eafanasova@gmail.com>
---
hw/net/spapr_llan.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/hw/net/spapr_llan.c b/hw/net/spapr_llan.c
index 2093f1bad0..581320a0e7 100644
--- a/hw/net/spapr_llan.c
+++ b/hw/net/spapr_llan.c
@@ -688,7 +688,8 @@ static target_ulong h_send_logical_lan(PowerPCCPU *cpu,
SpaprVioDevice *sdev = spapr_vio_find_by_reg(spapr->vio_bus, reg);
SpaprVioVlan *dev = VIO_SPAPR_VLAN_DEVICE(sdev);
unsigned total_len;
- uint8_t *lbuf, *p;
+ uint8_t *p;
+ g_autofree uint8_t *lbuf = NULL;
int i, nbufs;
int ret;
@@ -729,7 +730,7 @@ static target_ulong h_send_logical_lan(PowerPCCPU *cpu,
return H_RESOURCE;
}
- lbuf = alloca(total_len);
+ lbuf = g_malloc(total_len);
p = lbuf;
for (i = 0; i < nbufs; i++) {
ret = spapr_vio_dma_read(sdev, VLAN_BD_ADDR(bufs[i]),
--
2.25.1
On 10/9/20 9:02 AM, Elena Afanasova wrote: > From 09905773a00e417d3a37c12350d9e55466fdce8a Mon Sep 17 00:00:00 2001 > From: Elena Afanasova <eafanasova@gmail.com> > Date: Fri, 9 Oct 2020 06:41:36 -0700 > Subject: [PATCH] hw/net: move allocation to the heap due to very large stack > frame > > Signed-off-by: Elena Afanasova <eafanasova@gmail.com> > --- > hw/net/spapr_llan.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
On 10/9/20 4:02 PM, Elena Afanasova wrote:
> From 09905773a00e417d3a37c12350d9e55466fdce8a Mon Sep 17 00:00:00 2001
> From: Elena Afanasova <eafanasova@gmail.com>
> Date: Fri, 9 Oct 2020 06:41:36 -0700
> Subject: [PATCH] hw/net: move allocation to the heap due to very large stack
> frame
>
> Signed-off-by: Elena Afanasova <eafanasova@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> hw/net/spapr_llan.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/hw/net/spapr_llan.c b/hw/net/spapr_llan.c
> index 2093f1bad0..581320a0e7 100644
> --- a/hw/net/spapr_llan.c
> +++ b/hw/net/spapr_llan.c
> @@ -688,7 +688,8 @@ static target_ulong h_send_logical_lan(PowerPCCPU *cpu,
> SpaprVioDevice *sdev = spapr_vio_find_by_reg(spapr->vio_bus, reg);
> SpaprVioVlan *dev = VIO_SPAPR_VLAN_DEVICE(sdev);
> unsigned total_len;
> - uint8_t *lbuf, *p;
> + uint8_t *p;
> + g_autofree uint8_t *lbuf = NULL;
> int i, nbufs;
> int ret;
>
> @@ -729,7 +730,7 @@ static target_ulong h_send_logical_lan(PowerPCCPU *cpu,
> return H_RESOURCE;
> }
>
> - lbuf = alloca(total_len);
> + lbuf = g_malloc(total_len);
> p = lbuf;
> for (i = 0; i < nbufs; i++) {
> ret = spapr_vio_dma_read(sdev, VLAN_BD_ADDR(bufs[i]),
>
On Fri, 09 Oct 2020 07:02:56 -0700
Elena Afanasova <eafanasova@gmail.com> wrote:
> From 09905773a00e417d3a37c12350d9e55466fdce8a Mon Sep 17 00:00:00 2001
> From: Elena Afanasova <eafanasova@gmail.com>
> Date: Fri, 9 Oct 2020 06:41:36 -0700
> Subject: [PATCH] hw/net: move allocation to the heap due to very large stack
> frame
>
Something seems to have gone wrong with the message body here, which
results in an awkward changelog... but maybe someone can fix that when
applying the patch.
> Signed-off-by: Elena Afanasova <eafanasova@gmail.com>
> ---
Reviewed-by: Greg Kurz <groug@kaod.org>
> hw/net/spapr_llan.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/hw/net/spapr_llan.c b/hw/net/spapr_llan.c
> index 2093f1bad0..581320a0e7 100644
> --- a/hw/net/spapr_llan.c
> +++ b/hw/net/spapr_llan.c
> @@ -688,7 +688,8 @@ static target_ulong h_send_logical_lan(PowerPCCPU *cpu,
> SpaprVioDevice *sdev = spapr_vio_find_by_reg(spapr->vio_bus, reg);
> SpaprVioVlan *dev = VIO_SPAPR_VLAN_DEVICE(sdev);
> unsigned total_len;
> - uint8_t *lbuf, *p;
> + uint8_t *p;
> + g_autofree uint8_t *lbuf = NULL;
> int i, nbufs;
> int ret;
>
> @@ -729,7 +730,7 @@ static target_ulong h_send_logical_lan(PowerPCCPU *cpu,
> return H_RESOURCE;
> }
>
> - lbuf = alloca(total_len);
> + lbuf = g_malloc(total_len);
> p = lbuf;
> for (i = 0; i < nbufs; i++) {
> ret = spapr_vio_dma_read(sdev, VLAN_BD_ADDR(bufs[i]),
Elena Afanasova <eafanasova@gmail.com> 于2020年10月9日周五 下午10:03写道：
>
> From 09905773a00e417d3a37c12350d9e55466fdce8a Mon Sep 17 00:00:00 2001
> From: Elena Afanasova <eafanasova@gmail.com>
> Date: Fri, 9 Oct 2020 06:41:36 -0700
> Subject: [PATCH] hw/net: move allocation to the heap due to very large stack
> frame
>
> Signed-off-by: Elena Afanasova <eafanasova@gmail.com>
Reviewed-by: Li Qiang <liq3ea@gmail.com>
> ---
> hw/net/spapr_llan.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/hw/net/spapr_llan.c b/hw/net/spapr_llan.c
> index 2093f1bad0..581320a0e7 100644
> --- a/hw/net/spapr_llan.c
> +++ b/hw/net/spapr_llan.c
> @@ -688,7 +688,8 @@ static target_ulong h_send_logical_lan(PowerPCCPU *cpu,
> SpaprVioDevice *sdev = spapr_vio_find_by_reg(spapr->vio_bus, reg);
> SpaprVioVlan *dev = VIO_SPAPR_VLAN_DEVICE(sdev);
> unsigned total_len;
> - uint8_t *lbuf, *p;
> + uint8_t *p;
> + g_autofree uint8_t *lbuf = NULL;
> int i, nbufs;
> int ret;
>
> @@ -729,7 +730,7 @@ static target_ulong h_send_logical_lan(PowerPCCPU *cpu,
> return H_RESOURCE;
> }
>
> - lbuf = alloca(total_len);
> + lbuf = g_malloc(total_len);
> p = lbuf;
> for (i = 0; i < nbufs; i++) {
> ret = spapr_vio_dma_read(sdev, VLAN_BD_ADDR(bufs[i]),
> --
> 2.25.1
>
>
>
On Fri, Oct 09, 2020 at 07:02:56AM -0700, Elena Afanasova wrote:
> >From 09905773a00e417d3a37c12350d9e55466fdce8a Mon Sep 17 00:00:00 2001
> From: Elena Afanasova <eafanasova@gmail.com>
> Date: Fri, 9 Oct 2020 06:41:36 -0700
> Subject: [PATCH] hw/net: move allocation to the heap due to very large stack
> frame
Patch looks fine, but some more details of the motivation would be
nice. I wouldn't have thought that the size of a network packet
counted as a "very large" stack frame by userspace standards.
> Signed-off-by: Elena Afanasova <eafanasova@gmail.com>
> ---
> hw/net/spapr_llan.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/hw/net/spapr_llan.c b/hw/net/spapr_llan.c
> index 2093f1bad0..581320a0e7 100644
> --- a/hw/net/spapr_llan.c
> +++ b/hw/net/spapr_llan.c
> @@ -688,7 +688,8 @@ static target_ulong h_send_logical_lan(PowerPCCPU *cpu,
> SpaprVioDevice *sdev = spapr_vio_find_by_reg(spapr->vio_bus, reg);
> SpaprVioVlan *dev = VIO_SPAPR_VLAN_DEVICE(sdev);
> unsigned total_len;
> - uint8_t *lbuf, *p;
> + uint8_t *p;
> + g_autofree uint8_t *lbuf = NULL;
> int i, nbufs;
> int ret;
>
> @@ -729,7 +730,7 @@ static target_ulong h_send_logical_lan(PowerPCCPU *cpu,
> return H_RESOURCE;
> }
>
> - lbuf = alloca(total_len);
> + lbuf = g_malloc(total_len);
> p = lbuf;
> for (i = 0; i < nbufs; i++) {
> ret = spapr_vio_dma_read(sdev, VLAN_BD_ADDR(bufs[i]),
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
On Sat, 2020-10-10 at 17:07 +1100, David Gibson wrote:
> On Fri, Oct 09, 2020 at 07:02:56AM -0700, Elena Afanasova wrote:
> > > From 09905773a00e417d3a37c12350d9e55466fdce8a Mon Sep 17 00:00:00
> > > 2001
> > From: Elena Afanasova <eafanasova@gmail.com>
> > Date: Fri, 9 Oct 2020 06:41:36 -0700
> > Subject: [PATCH] hw/net: move allocation to the heap due to very
> > large stack
> > frame
>
> Patch looks fine, but some more details of the motivation would be
> nice. I wouldn't have thought that the size of a network packet
> counted as a "very large" stack frame by userspace standards.
>
gcc with wstack-usage warns that stack frame size may be ~65Kbytes
> > Signed-off-by: Elena Afanasova <eafanasova@gmail.com>
> > ---
> > hw/net/spapr_llan.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/net/spapr_llan.c b/hw/net/spapr_llan.c
> > index 2093f1bad0..581320a0e7 100644
> > --- a/hw/net/spapr_llan.c
> > +++ b/hw/net/spapr_llan.c
> > @@ -688,7 +688,8 @@ static target_ulong
> > h_send_logical_lan(PowerPCCPU *cpu,
> > SpaprVioDevice *sdev = spapr_vio_find_by_reg(spapr->vio_bus,
> > reg);
> > SpaprVioVlan *dev = VIO_SPAPR_VLAN_DEVICE(sdev);
> > unsigned total_len;
> > - uint8_t *lbuf, *p;
> > + uint8_t *p;
> > + g_autofree uint8_t *lbuf = NULL;
> > int i, nbufs;
> > int ret;
> >
> > @@ -729,7 +730,7 @@ static target_ulong
> > h_send_logical_lan(PowerPCCPU *cpu,
> > return H_RESOURCE;
> > }
> >
> > - lbuf = alloca(total_len);
> > + lbuf = g_malloc(total_len);
> > p = lbuf;
> > for (i = 0; i < nbufs; i++) {
> > ret = spapr_vio_dma_read(sdev, VLAN_BD_ADDR(bufs[i]),
On Sat, Oct 10, 2020 at 08:53:00AM -0700, Elena Afanasova wrote:
> On Sat, 2020-10-10 at 17:07 +1100, David Gibson wrote:
> > On Fri, Oct 09, 2020 at 07:02:56AM -0700, Elena Afanasova wrote:
> > > > From 09905773a00e417d3a37c12350d9e55466fdce8a Mon Sep 17 00:00:00
> > > > 2001
> > > From: Elena Afanasova <eafanasova@gmail.com>
> > > Date: Fri, 9 Oct 2020 06:41:36 -0700
> > > Subject: [PATCH] hw/net: move allocation to the heap due to very
> > > large stack
> > > frame
> >
> > Patch looks fine, but some more details of the motivation would be
> > nice. I wouldn't have thought that the size of a network packet
> > counted as a "very large" stack frame by userspace standards.
> >
>
> gcc with wstack-usage warns that stack frame size may be ~65Kbytes
AFAICT, -Wstack-usage takes a parameter giving what size it will
complain about. What was that value, and what was the rationale for
choosing it?
>
> > > Signed-off-by: Elena Afanasova <eafanasova@gmail.com>
> > > ---
> > > hw/net/spapr_llan.c | 5 +++--
> > > 1 file changed, 3 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/hw/net/spapr_llan.c b/hw/net/spapr_llan.c
> > > index 2093f1bad0..581320a0e7 100644
> > > --- a/hw/net/spapr_llan.c
> > > +++ b/hw/net/spapr_llan.c
> > > @@ -688,7 +688,8 @@ static target_ulong
> > > h_send_logical_lan(PowerPCCPU *cpu,
> > > SpaprVioDevice *sdev = spapr_vio_find_by_reg(spapr->vio_bus,
> > > reg);
> > > SpaprVioVlan *dev = VIO_SPAPR_VLAN_DEVICE(sdev);
> > > unsigned total_len;
> > > - uint8_t *lbuf, *p;
> > > + uint8_t *p;
> > > + g_autofree uint8_t *lbuf = NULL;
> > > int i, nbufs;
> > > int ret;
> > >
> > > @@ -729,7 +730,7 @@ static target_ulong
> > > h_send_logical_lan(PowerPCCPU *cpu,
> > > return H_RESOURCE;
> > > }
> > >
> > > - lbuf = alloca(total_len);
> > > + lbuf = g_malloc(total_len);
> > > p = lbuf;
> > > for (i = 0; i < nbufs; i++) {
> > > ret = spapr_vio_dma_read(sdev, VLAN_BD_ADDR(bufs[i]),
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
On 12/10/2020 07.30, David Gibson wrote: > On Sat, Oct 10, 2020 at 08:53:00AM -0700, Elena Afanasova wrote: >> On Sat, 2020-10-10 at 17:07 +1100, David Gibson wrote: >>> On Fri, Oct 09, 2020 at 07:02:56AM -0700, Elena Afanasova wrote: >>>>> From 09905773a00e417d3a37c12350d9e55466fdce8a Mon Sep 17 00:00:00 >>>>> 2001 >>>> From: Elena Afanasova <eafanasova@gmail.com> >>>> Date: Fri, 9 Oct 2020 06:41:36 -0700 >>>> Subject: [PATCH] hw/net: move allocation to the heap due to very >>>> large stack >>>> frame >>> >>> Patch looks fine, but some more details of the motivation would be >>> nice. I wouldn't have thought that the size of a network packet >>> counted as a "very large" stack frame by userspace standards. >>> >> >> gcc with wstack-usage warns that stack frame size may be ~65Kbytes > > AFAICT, -Wstack-usage takes a parameter giving what size it will > complain about. What was that value, and what was the rationale for > choosing it? I think this is one of the tasks from: https://wiki.qemu.org/Contribute/BiteSizedTasks#Compiler-driven_cleanups It has been added by Paolo in 2016: https://wiki.qemu.org/index.php?title=Contribute/BiteSizedTasks&diff=5368&oldid=5367 ... so maybe Paolo can comment on the size that has been chosen here...? Thomas
On 12/10/20 12:44, Thomas Huth wrote: > I think this is one of the tasks from: > > https://wiki.qemu.org/Contribute/BiteSizedTasks#Compiler-driven_cleanups > > It has been added by Paolo in 2016: > > https://wiki.qemu.org/index.php?title=Contribute/BiteSizedTasks&diff=5368&oldid=5367 > > ... so maybe Paolo can comment on the size that has been chosen here...? I used 16K, mostly because it is a nice round number. 8k is too small due to PATH_MAX-sized variables. 16k seemed to be plenty and triggered in few-enough places that the cleanup is viable. Paolo
On Mon, Oct 12, 2020 at 03:45:02PM +0200, Paolo Bonzini wrote: > On 12/10/20 12:44, Thomas Huth wrote: > > I think this is one of the tasks from: > > > > https://wiki.qemu.org/Contribute/BiteSizedTasks#Compiler-driven_cleanups > > > > It has been added by Paolo in 2016: > > > > https://wiki.qemu.org/index.php?title=Contribute/BiteSizedTasks&diff=5368&oldid=5367 > > > > ... so maybe Paolo can comment on the size that has been chosen here...? > > I used 16K, mostly because it is a nice round number. 8k is too small > due to PATH_MAX-sized variables. 16k seemed to be plenty and triggered > in few-enough places that the cleanup is viable. Ok. Why are large stack frames bad in qemu? -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
On Tue, 2020-10-13 at 16:32 +1100, David Gibson wrote: > On Mon, Oct 12, 2020 at 03:45:02PM +0200, Paolo Bonzini wrote: > > On 12/10/20 12:44, Thomas Huth wrote: > > > I think this is one of the tasks from: > > > > > > > > > https://wiki.qemu.org/Contribute/BiteSizedTasks#Compiler-driven_cleanups > > > > > > It has been added by Paolo in 2016: > > > > > > > > > https://wiki.qemu.org/index.php?title=Contribute/BiteSizedTasks&diff=5368&oldid=5367 > > > > > > ... so maybe Paolo can comment on the size that has been chosen > > > here...? > > > > I used 16K, mostly because it is a nice round number. 8k is too > > small > > due to PATH_MAX-sized variables. 16k seemed to be plenty and > > triggered > > in few-enough places that the cleanup is viable. > > Ok. Why are large stack frames bad in qemu? > I think that the main issue here is alloca() because it can lead to UB.
On Wed, Oct 14, 2020 at 07:15:47AM -0700, Elena Afanasova wrote: > On Tue, 2020-10-13 at 16:32 +1100, David Gibson wrote: > > On Mon, Oct 12, 2020 at 03:45:02PM +0200, Paolo Bonzini wrote: > > > On 12/10/20 12:44, Thomas Huth wrote: > > > > I think this is one of the tasks from: > > > > > > > > > > > > https://wiki.qemu.org/Contribute/BiteSizedTasks#Compiler-driven_cleanups > > > > > > > > It has been added by Paolo in 2016: > > > > > > > > > > > > https://wiki.qemu.org/index.php?title=Contribute/BiteSizedTasks&diff=5368&oldid=5367 > > > > > > > > ... so maybe Paolo can comment on the size that has been chosen > > > > here...? > > > > > > I used 16K, mostly because it is a nice round number. 8k is too > > > small > > > due to PATH_MAX-sized variables. 16k seemed to be plenty and > > > triggered > > > in few-enough places that the cleanup is viable. > > > > Ok. Why are large stack frames bad in qemu? > > > > I think that the main issue here is alloca() because it can lead to UB. That's a fair point. I've applied the patch to ppc-for-5.2, with a tweak to the commit message. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
David Gibson <david@gibson.dropbear.id.au> 于2020年10月10日周六 下午2:34写道：
>
> On Fri, Oct 09, 2020 at 07:02:56AM -0700, Elena Afanasova wrote:
> > >From 09905773a00e417d3a37c12350d9e55466fdce8a Mon Sep 17 00:00:00 2001
> > From: Elena Afanasova <eafanasova@gmail.com>
> > Date: Fri, 9 Oct 2020 06:41:36 -0700
> > Subject: [PATCH] hw/net: move allocation to the heap due to very large stack
> > frame
>
> Patch looks fine, but some more details of the motivation would be
> nice. I wouldn't have thought that the size of a network packet
> counted as a "very large" stack frame by userspace standards.
>
It is also a best practice to avoid large stack allocation according.
-->https://wiki.sei.cmu.edu/confluence/display/c/MEM05-C.+Avoid+large+stack+allocations
Though I don't see any issue here.
Thanks,
Li Qiang
> > Signed-off-by: Elena Afanasova <eafanasova@gmail.com>
> > ---
> > hw/net/spapr_llan.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/net/spapr_llan.c b/hw/net/spapr_llan.c
> > index 2093f1bad0..581320a0e7 100644
> > --- a/hw/net/spapr_llan.c
> > +++ b/hw/net/spapr_llan.c
> > @@ -688,7 +688,8 @@ static target_ulong h_send_logical_lan(PowerPCCPU *cpu,
> > SpaprVioDevice *sdev = spapr_vio_find_by_reg(spapr->vio_bus, reg);
> > SpaprVioVlan *dev = VIO_SPAPR_VLAN_DEVICE(sdev);
> > unsigned total_len;
> > - uint8_t *lbuf, *p;
> > + uint8_t *p;
> > + g_autofree uint8_t *lbuf = NULL;
> > int i, nbufs;
> > int ret;
> >
> > @@ -729,7 +730,7 @@ static target_ulong h_send_logical_lan(PowerPCCPU *cpu,
> > return H_RESOURCE;
> > }
> >
> > - lbuf = alloca(total_len);
> > + lbuf = g_malloc(total_len);
> > p = lbuf;
> > for (i = 0; i < nbufs; i++) {
> > ret = spapr_vio_dma_read(sdev, VLAN_BD_ADDR(bufs[i]),
>
> --
> David Gibson | I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
> | _way_ _around_!
> http://www.ozlabs.org/~dgibson
On Sun, Oct 11, 2020 at 10:23:49AM +0800, Li Qiang wrote:
> David Gibson <david@gibson.dropbear.id.au> 于2020年10月10日周六 下午2:34写道：
> >
> > On Fri, Oct 09, 2020 at 07:02:56AM -0700, Elena Afanasova wrote:
> > > >From 09905773a00e417d3a37c12350d9e55466fdce8a Mon Sep 17 00:00:00 2001
> > > From: Elena Afanasova <eafanasova@gmail.com>
> > > Date: Fri, 9 Oct 2020 06:41:36 -0700
> > > Subject: [PATCH] hw/net: move allocation to the heap due to very large stack
> > > frame
> >
> > Patch looks fine, but some more details of the motivation would be
> > nice. I wouldn't have thought that the size of a network packet
> > counted as a "very large" stack frame by userspace standards.
> >
>
> It is also a best practice to avoid large stack allocation according.
> -->https://wiki.sei.cmu.edu/confluence/display/c/MEM05-C.+Avoid+large+stack+allocations
Hm, yeah, it's not really clear what "large" means in that context.
It mostly seems to be concerned with allocations controlled by an
external attacker, in which case we could be talking up to INT_MAX.
Even with jumbo frames the most we're talking here is ~64kiB.
>
> Though I don't see any issue here.
>
> Thanks,
> Li Qiang
>
> > > Signed-off-by: Elena Afanasova <eafanasova@gmail.com>
> > > ---
> > > hw/net/spapr_llan.c | 5 +++--
> > > 1 file changed, 3 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/hw/net/spapr_llan.c b/hw/net/spapr_llan.c
> > > index 2093f1bad0..581320a0e7 100644
> > > --- a/hw/net/spapr_llan.c
> > > +++ b/hw/net/spapr_llan.c
> > > @@ -688,7 +688,8 @@ static target_ulong h_send_logical_lan(PowerPCCPU *cpu,
> > > SpaprVioDevice *sdev = spapr_vio_find_by_reg(spapr->vio_bus, reg);
> > > SpaprVioVlan *dev = VIO_SPAPR_VLAN_DEVICE(sdev);
> > > unsigned total_len;
> > > - uint8_t *lbuf, *p;
> > > + uint8_t *p;
> > > + g_autofree uint8_t *lbuf = NULL;
> > > int i, nbufs;
> > > int ret;
> > >
> > > @@ -729,7 +730,7 @@ static target_ulong h_send_logical_lan(PowerPCCPU *cpu,
> > > return H_RESOURCE;
> > > }
> > >
> > > - lbuf = alloca(total_len);
> > > + lbuf = g_malloc(total_len);
> > > p = lbuf;
> > > for (i = 0; i < nbufs; i++) {
> > > ret = spapr_vio_dma_read(sdev, VLAN_BD_ADDR(bufs[i]),
> >
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
On 10/10/20 8:07 AM, David Gibson wrote:
> On Fri, Oct 09, 2020 at 07:02:56AM -0700, Elena Afanasova wrote:
>> >From 09905773a00e417d3a37c12350d9e55466fdce8a Mon Sep 17 00:00:00 2001
>> From: Elena Afanasova <eafanasova@gmail.com>
>> Date: Fri, 9 Oct 2020 06:41:36 -0700
>> Subject: [PATCH] hw/net: move allocation to the heap due to very large stack
>> frame
>
> Patch looks fine, but some more details of the motivation would be
> nice. I wouldn't have thought that the size of a network packet
> counted as a "very large" stack frame by userspace standards.
Maybe academia doing research on "super jumbo frames"?
"Super jumbo frames ... increase the path MTU of high-performance
national research and education networks from 1500 bytes to 9000
bytes or so, a subsequent increase, possibly to 64,000 bytes"
(https://en.wikipedia.org/wiki/Jumbo_frame#Super_jumbo_frames)
>
>> Signed-off-by: Elena Afanasova <eafanasova@gmail.com>
>> ---
>> hw/net/spapr_llan.c | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/net/spapr_llan.c b/hw/net/spapr_llan.c
>> index 2093f1bad0..581320a0e7 100644
>> --- a/hw/net/spapr_llan.c
>> +++ b/hw/net/spapr_llan.c
>> @@ -688,7 +688,8 @@ static target_ulong h_send_logical_lan(PowerPCCPU *cpu,
>> SpaprVioDevice *sdev = spapr_vio_find_by_reg(spapr->vio_bus, reg);
>> SpaprVioVlan *dev = VIO_SPAPR_VLAN_DEVICE(sdev);
>> unsigned total_len;
>> - uint8_t *lbuf, *p;
>> + uint8_t *p;
>> + g_autofree uint8_t *lbuf = NULL;
>> int i, nbufs;
>> int ret;
>>
>> @@ -729,7 +730,7 @@ static target_ulong h_send_logical_lan(PowerPCCPU *cpu,
>> return H_RESOURCE;
>> }
>>
>> - lbuf = alloca(total_len);
>> + lbuf = g_malloc(total_len);
>> p = lbuf;
>> for (i = 0; i < nbufs; i++) {
>> ret = spapr_vio_dma_read(sdev, VLAN_BD_ADDR(bufs[i]),
>
On 10/12/20 1:09 PM, Philippe Mathieu-Daudé wrote: > On 10/10/20 8:07 AM, David Gibson wrote: >> On Fri, Oct 09, 2020 at 07:02:56AM -0700, Elena Afanasova wrote: >>> >From 09905773a00e417d3a37c12350d9e55466fdce8a Mon Sep 17 00:00:00 2001 >>> From: Elena Afanasova <eafanasova@gmail.com> >>> Date: Fri, 9 Oct 2020 06:41:36 -0700 >>> Subject: [PATCH] hw/net: move allocation to the heap due to very >>> large stack >>> frame >> >> Patch looks fine, but some more details of the motivation would be >> nice. I wouldn't have thought that the size of a network packet >> counted as a "very large" stack frame by userspace standards. > > Maybe academia doing research on "super jumbo frames"? > > "Super jumbo frames ... increase the path MTU of high-performance > national research and education networks from 1500 bytes to 9000 > bytes or so, a subsequent increase, possibly to 64,000 bytes" > > (https://en.wikipedia.org/wiki/Jumbo_frame#Super_jumbo_frames) The one I was actually looking for is the IPv6 jumbogram: "An optional feature of IPv6, the jumbo payload option, allows the exchange of packets with payloads of up to one byte less than 4 GiB, by making use of a 32-bit length field." (https://en.wikipedia.org/wiki/Jumbogram)
© 2016 - 2025 Red Hat, Inc.