Implement SIOCGIFALIAS
ClosedPublic
Actions

Authored by roy_marples.name on Oct 2 2020, 2:50 AM.

Details

Reviewers

gnn
melifaro
ae

Group Reviewers

manpages

Commits

rS366695: Implement SIOCGIFALIAS.

Summary

When we receive a route(4) message of RTM_DELADDR we need to check if it really has been deleted.
See https://github.com/freebsd/freebsd/blob/master/sys/netinet/raw_ip.c#L780 - if the link is taken down, we receive RTM_DELADDR but the address still exists on the interface!
Effectively, route(4) lies :/

Even if this is fixed (and it should be), we still need to check if the address exists incase it's been removed and quickly readded.
getifaddrs and sysctl are too heavy weight to check if an address exists and there is SIOCGIFAFLAG_IN6 to work out if an IPv6 address exists or not.
This patch brings parity for IPv4.

Test Plan

Recompile port net/dhcpcd to ensure that the SIOCGIFALIAS warning no longer exists.
Install it and run it in console A.
In console B bring the interface down and then up.
No errors for SIOCGIFALIAS should be reported.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

roy_marples.name created this revision.Oct 2 2020, 2:50 AM

Herald added a reviewer: manpages. · View Herald TranscriptOct 2 2020, 2:50 AM

Herald added subscribers: Contributor Reviews (src), melifaro, imp. · View Herald Transcript

roy_marples.name requested review of this revision.Oct 2 2020, 2:50 AM

LGTM

This revision is now accepted and ready to land.Oct 2 2020, 3:25 AM

melifaro accepted this revision.Oct 2 2020, 8:08 AM

melifaro added inline comments.

share/man/man4/netintro.4
353 ↗	(On Diff #77764)	As far as I see, `in_gifaddr_ioctl()` finds a single `ia` and fills the details of this `ia` in the provided structure. Could you please clarify what "additional addresses" mean here?

roy_marples.name added inline comments.Oct 2 2020, 8:15 AM

share/man/man4/netintro.4
353 ↗	(On Diff #77764)	You can have more than one IPv4 address assigned to an interface. These would be called additional addresses. If there could only ever be one address then we would not need this ioctl as `SIOCGIFADDR` would suffice.

I think IF_ADDR_WLOCK() is not required for this ioctl. It is enough to use NET_EPOCH_ENTER().

Use NET_EPOCH_{ENTER,EXIT}

This revision now requires review to proceed.Oct 2 2020, 4:24 PM

ae added inline comments.Oct 5 2020, 10:36 AM

sys/netinet/in.c
689 ↗	(On Diff #77784)	It seems we can stop search on first match.

roy_marples.name marked an inline comment as done.Oct 8 2020, 3:09 PM

roy_marples.name added inline comments.

sys/netinet/in.c
689 ↗	(On Diff #77784)	@ae Should we break early on just the address check or do we need that as well as the prison check? I don't know enough about prisons on FreeBSD to make a qualified judgement here and based this loop on the address deletion above.

ae added inline comments.Oct 8 2020, 4:27 PM

sys/netinet/in.c
689 ↗	(On Diff #77784)	Ah, I see the place where you took this code. if_addrhead list keeps addresses and it can not have duplicates from what I know. prison_check_ip4() just checks that address belongs to specified jail. So, if you find the match, all other iteration will be useless.