ifnet: Add some sanity checks
AcceptedPublic
Actions

Authored by zlei on Mon, Mar 16, 8:23 AM.

Details

Reviewers

glebius
kp

Group Reviewers

network

Summary

To be more robust since the checking is performed where the interface
is referenced.

While here, remove a redundant check from if_vmove_loan().

MFC after: 2 weeks

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

zlei created this revision.Mon, Mar 16, 8:23 AM

Herald added subscribers: melifaro, ae, imp. · View Herald TranscriptMon, Mar 16, 8:23 AM

zlei requested review of this revision.Mon, Mar 16, 8:23 AM

I'm not sure how these assertions will be beneficial if we have to add them to every function of the ifp [un]linking.
IMHO, KASSERT/MPASS are existing to verify programmatic assumptions.
And yes, these changes are definitely verifies programmatic assumptions.
However, I don't think that using them to verify every programmatic assumptions per function is appropriate.

sys/net/if.c
2132	while you're here, could you please fix its style too?

kp accepted this revision as: kp.Mon, Mar 16, 10:59 AM

This revision is now accepted and ready to land.Mon, Mar 16, 10:59 AM

zlei marked an inline comment as done.Mon, Mar 16, 11:15 AM

zlei added inline comments.

sys/net/if.c
2132	while you're here, could you please fix its style too? The flag `IFF_DYING` is a bit useless in practices. The writes to `if_flags` is not lock protected, hence a thread is not guareenteed to see it. Also it is a bit confusing to see a `IFF_DYING` interface on a "alive" list. I believe the only valid consumer of `IFF_DYING` is the driver, to prevent potential race condition on ioctls. Anyway this flag do not belong to if_flags. There're plans to remove `IFF_DYING`. See also https://reviews.freebsd.org/D49412. I do not want to touch the style right now.