ifnet_byindex() actually requires network epoch

[glebius]

Summary

Sweep over potentially unsafe calls to ifnet_byindex() and wrap them
in epoch. Most of the code touched remains unsafe, as the returned
pointer is being used after epoch exit. Mark that with a comment.

Validate the index argument inside the function, reducing argument
validation requirement from the callers and making V_if_index
private to if.c.

[melifaro]

sys/net/if.c

353

Worth considering using uint (or even uint32_t) as an index identifier.
It'll allow to avoid unnecessary (idx < 0 checks).

sys/netinet/igmp.c

512 ↗

(On Diff #99451)

Maybe worth moving it to out_locked: , so the future readers won't have to check whether ifp is indeed not used below?

sys/netinet6/mld6.c

359 ↗

(On Diff #99451)

Worth removing?

sys/netinet6/scope6.c

377 ↗

(On Diff #99451)

What do you think of having an explicit

bool ifnet_checkindex(unit idx)
{
  struct epoch_tracket et;

  NET_EPOCH_ENTER(et);
  bool result = ifnet_byindex(idx) != NULL;
  NET_EPOCH_EXIT(et);
}

?

[kp]

sys/net/if.c

353

ifp->if_index is an unsigned int, so it makes sense to also use that here.

(As an aside, struct in6_defrouter and struct in6_prefix have it as a u_short, and probably need to be fixed. That might be painful, as they're shared with user space.)

[glebius]

sys/netinet/igmp.c

512 ↗

(On Diff #99451)

Not sure. SYSCTL_OUT() potentially may sleep. It won't sleep in this function, since we wire the buffer. But that may change. Anyway this function has its own problems, like unlocked access to the igi_head. I'd better not include improvements to particular protocols into this sweeping ifnet_byindex() KPI change.

sys/netinet6/scope6.c

377 ↗

(On Diff #99451)

Don't know much about IPv6. Would it be correct to set sin6_scope_id to non-existent index, freed a moment ago? Cause this can happen now. If this needs to be fixed, then once it is fixed, such function ifnet_checkindex() would be not used.