[ipxe-devel] [iPXE-devel] [PATCHv2 4/6] [core] Keep scheduling while shell banner waiting

[Guo-Fu Tseng]

On Thu, 15 Jul 2010 00:30:31 +0100, Michael Brown wrote
> On Wednesday 14 Jul 2010 20:14:48 Piotr Jaroszyński wrote:
> > > <snip>
> > >
> > > You can then call activity_start() and activity_stop() at appropriate
> > > places (e.g. on creating/freeing a TCP connection or a TFTP connection;
> > > UDP-based protocols such as TFTP could also potentially benefit from
> > > waiting for a graceful shutdown).  You can call activity_wait() from
> > > appropriate places such as immediately prior to imgexec() in
> > > usr/autoboot.c.
> > 
> > One of those places would have to be in ifclose then, no? I like the
> > idea because it's more general, but on the other hand we might end up
> > with activity_wait in some places where we don't always need it (like
> > ifclosing a different interface than the one used by the activity).
> > Maybe it's not such a big deal though.
> 
> ifclose() is conceptually separate from netdev_close(), in that 
> ifclose() represents a "user level" call that is allowed to call 
> things like printf() and step().  It would in that sense be acceptable 
> for ifclose() to call activity_wait() before calling netdev_close().
> 
> That said, you could argue that ifclose is equivalent to something 
> like "ifconfig eth0 down" which would, I think, cause connections to 
> terminate gracelessly in exactly the same way as currently happens 
> with iPXE.  Maybe usr/autoboot.c is the more natural place to fit 
> calls to activity_wait()?
> 
> Michael

Thank you for this very useful idea!
I was thinking of putting something in the autoboot also, but did not
think of ''activity'' facility.

I think if we put the activity_wait() in two places might solve this
issue for most cases.

First, right before close_all_netdevs() in usr/autoboot.c:autoboot().
This makes it have the chance to gracefully close TCP when having
multiple NICs, and trying on the next one.

Second, right after checking for if it is started in core/init.c:shutdown().
When successfully find a bootable image and trying to boot, most of the
image->type->exec() that would leave gPXE's scope(ex: elfboot) would
call shutdown to clean-up resources.
And if we failed to boot on all cases, the core/main.c:main() would also
call shutdown to free the resources.

imho, if we place activity_wait() in these two places, we have the same
''scheduling'' behavior as before. And it seems clean enough to me. :)

PS: With previous shell_banner patch, we also don't have to worry about
some user did enabled the banner, and remote TCP server re-sends several
packets while gPXE waiting for user input and do nothing. :)

Guo-Fu Tseng