* Krzysztof Halasa <[EMAIL PROTECTED]> 2005-11-07 18:04
> Thomas Graf <[EMAIL PROTECTED]> writes:
> > void netif_carrier_on(struct net_device *dev)
> > {
> > + if ((dev->flags & IFF_WAIT) &&
> > + test_bit(__LINK_STATE_NOCARRIER, &dev->state))
> > + netif_dormant_on(dev);
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> Does not prevent spurious momentary IFF_RUNNING: netif_dormant_off()
> may come after it before "the session" is "established" as an effect
> of pre-carrier-flap activity (due to asynchronous notification) - so,
> in comparison to my patch, I don't see the gain. Not sure what locking
> (short of a single spinlock for all those flags) could solve that but
> I'm not a spinlock expert.
Well, firstly we cannot provide a perfect notification schema.
Take for example the carrier, when we reach netif_carrier_off(),
the carrier might in fact be up already. We then fire linkwatch
and while we wait for the workqueue to handle it, the carrier
state may theoretically change again. Events to userspace can
get lost due to various reasons, for example memory pressure.
What I actually want to say is that we probably cannot guarentee
an up-to-date informal of all parties, what we can do is avoid
is inconsistency. So considering the above case, netif_carrier_on()
has to be syncronized by the caller, this means the carrier state
may not change while we're inside the function. Secondly we know
that when the function is called, there must have been no carrier
for some time _if_ the carrier is currently set to off. This
means that the above code is only issued for legal carrier
resumes. The fact that we go to dormant state before changing the
carrier state makes dev_oper_state() behave correctly because in
case we actually call dev_oper_state() between netif_dormant_on()
and setting the carrier bit the interface will still be seen as
down and then switches to dormant in a single atomic operation.
> Protocol driver must still call netif_dormant_on() itself or IFF_RUNNING
> may be stuck for long.
The IFF_WAIT flag can only be set by root, it is in his reponsibly
to handle this correctly. It is intended for userspace although
it can of course be used by drivers or stackable interfaces as well.
> > if (test_and_clear_bit(__LINK_STATE_NOCARRIER, &dev->state))
> > linkwatch_fire_event(dev);
> > - if (netif_running(dev))
> > +
> > + if (netif_running(dev) && !netif_dormant(dev));
> > __netdev_watchdog_up(dev);
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> Could be called even if the above netif_dormant_on() set the status.
> Hope the protocol resets it when it gets 'carrier down' event.
Theoretically yes, it has the same "issues" as all other link states.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
