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

[Piotr Jaroszyński]

>> imho, net/netdevice.c:net_step() should only be called by
>> core/process.c:step(). Piotr did mentioned that it is just
>> an idea, and suggest that we can move the actual actions
>> from net_step() to another function. And both netdev_close()
>> , and net_step() can call that function. This does make
>> the code flow less confusing, and easier to maintain.
>>
>> For the original design. We can assume that all the network
>> receive flow(ex: netdevice->ipv4->tcp->app) is happened in
>> ''netdevice process'', if we do the polling in netdev_close(),
>> it'll break that assumption. It might not hurt a lot anyway.
>> But this makes me think if there is a better way.
>>
>> On the other hand, the function name netdev_close() would
>> make me think that it tries to nullify/free the resources.
>> Instead of processing data. In some cases, the close()
>> event DO wait for resources to be freed and safe to close.
>> But I rare see a design that actually do the works in
>> close(), and tries to make it possible to free the
>> resources.
>
> I agree with pretty much all of that.  close() is the wrong place to put this
> kind of thing.

I agree it's a hacky place to do that, but ...

> I would suggest something like:
>
>  core/activity.c:
>
>    static unsigned int num_activities = 0;
>
>    void activity_start ( void ) {
>      num_activities++;
>    }
>
>    void activity_stop ( void ) {
>      num_activities--;
>    }
>
>    int activity_wait ( unsigned long max_timeout ) {
>      while ( num_activities && <not timed out> ) {
>        step();
>      }
>      return ( ( num_activities == 0 ) ? 0 : -EBUSY );
>    }
>
> 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.

-- 
Best Regards
Piotr Jaroszyński