Run the original ifnet_arrival_event before linking the interface.
Otherwise there is a race window when interface is already visible, but
not all of the protocols have completed their attach. Provide a new event
handler ifnet_attached_event, that is executed when the inteface is fully
visible. Use it in route(4) socket and netlink(4) to announce new
interface to the userland. Properly document the ifnet events in if_var.h.
Details
- Reviewers
melifaro zlei - Group Reviewers
network - Commits
- rG713b57c414b7: net: split ifnet_arrival_event into two events
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| sys/net/if.c | ||
|---|---|---|
| 938 | The original ifnet_arrival_event reads, "the interface is attached and global visible". But after this change, "the interface is attached but is not guaranteed to be global visible". There are consumers, sys/net/rtsock.c:EVENTHANDLER_DEFINE(ifnet_arrival_event, rts_handle_ifnet_arrival, NULL, 0); ... sys/netlink/route/iface.c: EVENTHANDLER_DEREGISTER(ifnet_arrival_event, ifattach_event); rely on the former behavior, so that netlink clients or tsock clients can see global visible ifnet. I'm not sure what's the best approach, but maybe a new EVENTHANDLER is wanted to attach IPv4 and IPv6 stacks to an interface ? | |
| sys/net/if.c | ||
|---|---|---|
| 938 |
That's a good observation, thanks.
...
These two do not actually work with the global glue, they don't need the interface to be fully linked in. However, they do announce the interface arrival to the userland. Hypothetically there could be a race with the application seeing the announcement and immediately doing an ioctl/netlink call, that reaches kernel faster than other eventhandlers do their work.
I'd rather create a second event handler that would be called after a complete attach of the interface. The rtsock and the netlink would be its only consumers for now. What do you think? | |
| sys/net/if.c | ||
|---|---|---|
| 938 | There're other consumers of ifnet_arrival_event. I'm a little worried that they may expect a fully constructed ifnet, i.e. consumers see that the if_inet and if_inet6 are also initialized. So, again, we should definitely make the concept ifnet_arrival_event clear. How will consumers expect from it. PS. the ifnet_departure_event is also misleading. I see there're ether_ifattach_event and infiniband_ifattach_event. Probably we can introduce a new generic ifnet_ifattach_event for all ifnets. It guarantees that the ifnet is **FULLY* constructed and global visible. So that ifnet_arrival_event comes before if_link_ifnet() and then ifnet_ifattach_event. I have WIP to postpone if_attach() as late as possible. Ideally ifnet_ifattach_event can take place of ether_ifattach_event and infiniband_ifattach_event. | |
There're other consumers of ifnet_arrival_event. I'm a little worried that they may expect a fully constructed ifnet, i.e. consumers see that the if_inet and if_inet6 are also initialized.
I checked them. They all allocate their own contexts, they don't use if_inet or if_inet6.
Probably we can introduce a new generic ifnet_ifattach_event for all ifnets. It guarantees that the ifnet is FULLY constructed and global visible. So that ifnet_arrival_event comes before if_link_ifnet() and then ifnet_ifattach_event.
This is exactly what I suggested above!
Let's agree on naming. I also don't like the arrival and departure words. But I don't want to make a renaming sweep embedded into this change. We can discuss this part of naming later. Let's agree on the name of the new event that would be executed when the ifnet is FULLY constructed and global visible. I'd suggest name ifname_attached_event. The past participle will emphasize the notion that the attach is completely done. The Netlink and routing socket will use this event.