Page MenuHomeFreeBSD

Implement SIOCGIFALIAS
ClosedPublic

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
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

gnn added a subscriber: gnn.

LGTM

This revision is now accepted and ready to land.Oct 2 2020, 3:25 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?

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().

This revision now requires review to proceed.Oct 2 2020, 4:24 PM
sys/netinet/in.c
689 ↗(On Diff #77784)

It seems we can stop search on first match.

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.

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.

Break out of the loop early on matching an address.

This revision is now accepted and ready to land.Oct 8 2020, 9:38 PM
This revision was automatically updated to reflect the committed changes.