[PATCH] if_lagg driver enhancements.

[Steven Hartland]

> IMHO, the patch introduces a layering violation, which is bad. This would
> lead to problems in future. From a quick look I don't see any right now,
> and patch is compatible with carp(4) just accidentially :)
>
> I would suggest the following approach:
>
> 1) Network protocols should register theirselves on the ifnet_link_event
>   EVENTHANDLER(9).
> 2) The inet4 should send gratutious ARP on this event.
> 3) The inet6 should send NA.
>
> As you see the patch would be entirely not about lagg(4) :)
>
> We've got some minor obstacles on the suggested way:
>
> - The if_link_state_change() function suppresses any processing if the link
>   hasn't changed, for example UP -> UP event.
>
> We can overcome this by adding a new pseudo state LINK_STATE_UPAGAIN (or
> LINK_STATE_UPCHANGED or LINK_STATE_UPANOTHER or any better name you can
> imagine). This pseudo state can't be stored in the ifp->if_link_state, but
> it can be used to keep the state LINK_STATE_UP, but force triggering link
> state hooks.
>
> I think this approach is more clean and error prone. It can lead only to
> extraneous gratutious ARP sent in some cases, which is not critical.

Hi Gleb I know this is an old one, but I've picked it up as I'm going to
need it fixed for a project that goes live before Christmas. TBH I'm
quite surprised that this is still an issue, as it makes failover mode
pretty useless, in its current form.

As I understand you're proposal, it was to have each protocol e.g.
ipv4, ipv6 register an ifnet_link_event handler. This when combined with
lagg_linkstate using LINK_STATE_UPAGAIN (or some other name for it)
to ensure that ifnet_link_event is fired so that even when lagg failover
mode transitions from backup link to master link live, hence laggX going
from UP(backup) -> UP(master) a broadcast is still triggered.

Would this be a correct summary of your proposal?

One thing that springs to mind is to avoid introducing a new if_link_state
value and instead have lagg invoke the event directly in this special case.
This is it would avoid the unnecessary do_link_state_change call and all
the processing associated with that which is unneeded and could have
unintended side effects.

Alternatively still have the additional link_state but have
if_link_state_change process it directly without enqueueing the task for
do_link_state_change, which keeps the layer separation more strictly.

What do you think?

	Regards
	Steve