Re: dhcp integration into the platform


Marcus Shawcroft <marcus.shawcroft@...>
 

Hi,

The network interface patches proposed as a result of this thread have
generated a fair amount of discussion both in patch reviews and in
IRC. Now would seem like a good time to summarize where we are and
pull together some of the various discussion points that have been
raised.

Current status:

A bunch of preparatory patches to dhcpv4 have been merged. Notable changes:
- Public interface now provides net_dhcpv4_start(iface) and
net_dhcpv4_stop(iface).
- Various initialization issues that would prevent concurrent dhcpv4
operation on multiple ifaces are resolved.
- dhcpv4 will now remove leased resources from the network stack on
lease lapse/release.

There is one more small but significant dhcpv4 patch outstanding that
catches L2 up/down events and kicks the appropriate dhcpv4 machinery
per interface. This patch is currently blocked pending necessary
support in net_if (see below). Once this patch is in place an
application will be able to start (and stop dhcpv4) per interface as
now. Once started dhcpv4 will catch L2 up/down events and acquire,
renew and release leases as required, Eventually the responsibility
to call call net_dhcpv4_start/stop() may be moved from an application
to a 'connection manager'... but that is for the future.

The 'net_if' patches are in their third iteration and have generated
by far the most discussion.

The objective of the net_if patches is to arrange for L2 up/down
network management events to be raised when a functional L2 iface
becomes available for use, or conversely becomes unavailable. These
events can then be caught by dhcpv4 in order for dhcpv4 to manage
IP/L3 configuration.

In the current net_if implementation there are two significant
functions: net_if_up and net_if_down(). These functions call the
underlying L2 enable() callback, set and clear the net_if
NET_IF_ENABLED flag and raise NET_EVENT_IF_UP/DOWN network management
events.

After re-reading various comments and discussion on the existing patch
set I've come to the conclusion that there are two different world
views of the conceptual purpose of net_if_up() and net_if_down().

View 1:
net_if_up/down provide an interface for a higher/management layer to
communicate downwards and mark an iface as enabled or disabled
(irrespective of the state of the underlying L2)

This world view is supported by:
- these functions call down to the enable() callback in the underlying
L2 (ie they direcly call L2 telling it whether to enable or disable).
- in the absence of a connection manager the network stack hardwires a
call to net_if_up() for every iface at system boot (net_if_post_init).

View 2:
net_if_up/down provide an interface for an underlying L2 to
communicate upwards that an iface as up/working.

This world view is supported by:
- the bluetooth stack calls net_if_up/down on ipsp connect/disconnect
- the net_if_up/down terminology suggests this behaviour (as opposed
to being explicitly called enable/disable)

Conceptually there are four APIs here: enable/disable and up/down.
The former two provide a management interface that allows a higher
layer to requested that an iface is enabled or disabled, likely called
by a connection manager or equivalent. The latter two allow the stack
below the iface to report upwards whether or not an enabled iface
actually has a link up or not.

The l2 enable callback conceptually belongs with the enable/disable
interface. The network management event up/down signalling
conceptually belongs with the up/down interface.

In the current tree I think we have a slightly odd merge of the two
concepts where some code treats net_if_up/down() as if they implement
enable/disable semantics, while other code treats
net_if_up()/net_if_down() as if they implement up/down semantics.
Notably we have the network stack initialization code hardwiring
net_if_up() on all interfaces and we have L2 enable hung on
net_if_up/down() both of these behaviours associated with an
enable/disable semantic yet we also have the BT stack using
net_if_up/down() as a notification mechanism that L2 is up/down. (It
appears to me that for an iface associated with BT, the iface will be
up'd at system boot and then re-up'd on ipsp connect )

Various points that have come up in discussion (please correct me if I
misrepresent or miss some point of view):

1) Should we have enable/disable. The general view seems to be that
we don't have a solid use case for enable/disable therefore we should
not have them.

2) BT advertise should be disabled if a network interface is
disabled(). IMO this is actually the use case that suggests we should
keep enable/disable.

3) Should we have 1 or 2 net_if flags. The general view seems to be
that we should have only 1, I think in practice this is driven by
whether we keep or remove an enable/disable API.

4) Physical interfaces should not power up power down as a result of
L2 enable/disable, that should all be handled via a separate power
management API.


There are (at least) two ways forward:

1) We drop the enable/disable semantic. This implies:
- We remove the L2 enable() callback completely.
- We remove hardwired net_if_up() calls when the network stack boots.
- Every network device needs to call net_if_up() once it has a link
established (and net_if_down when it drops).
- BT stays as it is (advertise hardwired on)

2) We keep the enable/disable semantic. This implies:
- We split net_if_up/down into net_if_enable/disable() and
net_if_up/down() such that net_if_enable calls l2->enable() while
net_if_up/down deals with NET_IF_UP and raising net_event_if_up/down
- The hardwired net_if_up() calls at network stack boot are switched
to call net_if_enable()
- The BT L2 enable callback is used to turn advertising on/off
- Every network device needs to call net_if_up() once it has a link
established (and net_if_down when it drops). (BT already does this)

In option 1 we remove the mechanism we have to communicate to the BT
stack that advertising should be on/off.

IMHO route 2 is a better way forward.

Thoughts?

/Marcus

Join devel@lists.zephyrproject.org to automatically receive all group messages.