Re: struct ifnet and ifaddr handling [was: Re: Making global variables of if.c MPSAFE]

[Ryota Ozaki <ozaki-r%netbsd.org@localhost>]

On Wed, Nov 19, 2014 at 12:18 AM, Masao Uebayashi <uebayasi%gmail.com@localhost> wrote:
> On Mon, Nov 17, 2014 at 7:03 PM, Ryota Ozaki <ozaki-r%netbsd.org@localhost> wrote:
>> - http://www.netbsd.org/~ozaki-r/psz-ifnet.diff
>
> IFNET_RENTER();
> IFNET_FOREACH() {
>         if->if_watchdog();
> }
> IFNET_REXIT();
>
> This is not safe; if_watchdog() may resend packets, too slow operation
> to do in critical section.

Sure.

>
> I think you need to allocate a global array if_watchdog_t
> **if_slowtimo_watchdog, like ifindex2ifnet, which grows and can store
> pointers of if_watchdog of all interfaces, and do:
>
> IFNET_RENTER();
> i = 0;
> IFNET_FOREACH() {
>         if_slowtimo_watchdog[i++] = if->if_watchdog;
> }
> IFNET_REXIT();
> for (j = 0; j < i; j++) {
>         (*(if_slowtimo_watchdog[j]))();
> }

It seems incomplete. We have to store both a function
pointer and an ifp for each ifnet. Additionally we have to
prevent the ifps from being freed during executing if_watchdog
functions. So a complete version would be:

IFNET_FOREACH() {
        if_watchdog_funcs[i++] = ifp->if_watchdog;
        if_watchdog_ifps[i++] = ifp;
        refcount_hold(ifp->if_refcount);
}
IFNET_REXIT();

for (j = 0; j < i; j++) {
        (*(if_watchdog_funcs[j]))(if_watchdog_ifps[j]);
        ifput(ifps[j]);
}

Weird :-/

  ozaki-r