Re: svn commit: r205024 - head/sys/net

2010-03-13 Thread Paul B Mahol
On 3/11/10, Qing Li  wrote:
> Author: qingli
> Date: Thu Mar 11 17:56:46 2010
> New Revision: 205024
> URL: http://svn.freebsd.org/changeset/base/205024
>
> Log:
>   The if_tap interface is of IFT_ETHERNET type, but it
>   does not set or update the if_link_state variable.
>   As such RT_LINK_IS_UP() fails for the if_tap interface.
>
>   Also, the RT_LINK_IS_UP() needs to bypass all loopback
>   interfaces because loopback interfaces are considered
>   up logically as long as the system is running.
>
>   This patch fixes the above issues by setting and updating
>   the if_link_state variable when the tap interface is
>   opened or closed respectively. Similary approach is
>   already done in the if_tun device.
>
>   MFC after:  3 days
>
> Modified:
>   head/sys/net/if_tap.c
>   head/sys/net/route.h

Look at /sys/dev/if_ndis.c it is still broken.
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r205024 - head/sys/net

2010-03-12 Thread M. Warner Losh
julian> probably should add a flag that means "we have media state"
julian> and if it is not set, assume it is always on.

qing> That's a good idea. I will take your approach.

I do too.  While I have many of the older cards that don't support
media detect (because the chips don't report that info), it will take
some time to test them all out...

Warner
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r205024 - head/sys/net

2010-03-12 Thread Robert N. M. Watson

On Mar 12, 2010, at 6:56 PM, Qing Li wrote:

> Right now I like to implement Robert's suggestion of using if_capabilities
> field. I'd like to create a new flag, IFCAP_LINKSTATE_NOTIFY flag.
> The routing decision will check against the if_link_state if this bit
> is set.
> 
> Only a handful of drivers have this capability. So updating those drivers
> is a small task.
> 
> Do we agree on this approach?

I'm good with this approach -- do you mind going with IFCAP_LINKSTATE or 
something similarly a bit shorter? Most of the other capability names are 
pretty short, and it will prevent the display in ifconfig from growing too much 
as the capability will be common.

Thanks!

Robert___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r205024 - head/sys/net

2010-03-12 Thread Qing Li
>
> We've got LINK_STATE_UNKNOWN, we can just initialize if_link_state to
> this value in ether_ifattach(). And Qing should treat this value as
> LINK_STATE_UP in routing decision until better times.
>

Thanks everyone for your input.

I generally try to avoid overloading a variable to have more than 1 meaning.
For example, the if_tun interface is in LINK_STATE_UNKNOWN until
it's opened, then the link can be one of the other two states.
Also, some of the pseudo drivers such as if_tun do not call ether_ifattach().

Right now I like to implement Robert's suggestion of using if_capabilities
field. I'd like to create a new flag, IFCAP_LINKSTATE_NOTIFY flag.
The routing decision will check against the if_link_state if this bit
is set.

Only a handful of drivers have this capability. So updating those drivers
is a small task.

Do we agree on this approach?

-- Qing
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r205024 - head/sys/net

2010-03-12 Thread M. Warner Losh
In message: 
Juli Mallett  writes:
: On Thu, Mar 11, 2010 at 15:30, Qing Li  wrote:
: >>
: >> A couple of questions:
: >>
: >> (1) It used to be the case that quite a few interface drivers and types
: >> didn't have a notion of "link up" -- especially older ethernet devices.  Do
: >> those all have the same problem?  It was probably a design oversight that
: >> devices don't declare an explicit capability for "can report link state".
: >>
: >
: >  What you raised is definitely a possibility and these fixes take the
: >  similar approach. I am going to try and go through each of these
: >  drivers in /sys/dev/ and converting them, very soon.
: 
: Go through drivers in the embedded port directories, too.  The Octeon
: port's Ethernet driver was broken by this, and it looks like the
: Atheros if_arge is probably broken, too.  I would even suggest going
: back to the old behavior briefly while the port maintainers are given
: an opportunity to update their drivers.  Actually, it looks like only
: MIPS has Ethernet drivers outside of dev/ at a quick glance, but I'd
: be surprised if there weren't other broken examples.

arm has them as well.  sys/arm/at91/if_ate.c.  Powerpc does as well.

Warner
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r205024 - head/sys/net

2010-03-12 Thread Gleb Smirnoff
On Thu, Mar 11, 2010 at 11:15:21PM -0800, Julian Elischer wrote:
J> Juli Mallett wrote:
J> > On Thu, Mar 11, 2010 at 15:39, Qing Li  wrote:
J> >> I guess it's a good time to clean things up. The if_link_state code has 
been
J> >> around for quite some time, either it be fully utilized or not be there 
at all.
J> >> The inconsistency is the root cause.
J> > 
J> > Sure.  There is an increasing amount of stuff that network drivers are
J> > expected to do, but they work without doing them.  It's easy to think
J> > you have a functioning network driver and that you can get by without
J> > adding support for media changes and link status reporting, etc.
J> > 
J> >> I will try going through these tonight and hopefully the fix all take a
J> >> common approach.
J> 
J> probably should add a flag that means "we have media state"
J> and if it is not set, assume it is always on.

We've got LINK_STATE_UNKNOWN, we can just initialize if_link_state to
this value in ether_ifattach(). And Qing should treat this value as
LINK_STATE_UP in routing decision until better times.

-- 
Totus tuus, Glebius.
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r205024 - head/sys/net

2010-03-12 Thread Bruce Simpson

Any objections to making AF_NETLINK Foundation proposal public?

Might help FreeBSD get out of the niche a bit more; energy quantum wells 
are never great.

___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r205024 - head/sys/net

2010-03-12 Thread Robert N. M. Watson

On Mar 12, 2010, at 8:44 AM, Julian Elischer wrote:

> I'm confused about Julian's proposal because it seems to me that we 
> already know when a driver hasn't set or is unable to determine the link 
> state: it will (should) be set to LINK_STATE_UNKNOWN by default.
> 
> the question is whether there is any other meaning for this state.
> For example "I have not started up yet"

Right now LINK_STATE_UNKNOWN conflates three different conditions:

(1) I haven't and won't ever set the link state
(2) I haven't yet, but may in the future set the link state
(3) I wanted to check the link state, and the hardware is $*$**£*@@ so I left 
or set this value instead

My preferred solution is to advertise driver capabilities via the driver 
capabilities flag, if_capabilities, and define a new flag IFCAP_LINKSTATE (or 
something similar) that allows a driver to declare that it is able to determine 
link state. If the flag is set, then components like ECMP, but also dhclient, 
can reasonably expect that the driver will do its best to provide link state 
information, addressing the difference between (1) and (2) when a value of 
LINK_STATE_UNKNOWN is found.

If we are able to explicitly handle (3) in any of our drivers, then a new link 
state definition should be defined, LINK_STATE_ERROR, which should be set by 
the driver if it declared ICAP_LINKSTATE but was unable, due to hardware 
failure, to check it.

Robert___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r205024 - head/sys/net

2010-03-12 Thread Julian Elischer

Julian Elischer wrote:

Robert N. M. Watson wrote:




Today, we support three link state values:

170 /*
171  * Values for if_link_state.
172  */
173 #define LINK_STATE_UNKNOWN  0   /* link invalid/unknown */
174 #define LINK_STATE_DOWN 1   /* link is down */
175 #define LINK_STATE_UP   2   /* link is up */

I'm confused about Julian's proposal because it seems to me that we 
already know when a driver hasn't set or is unable to determine the 
link state: it will (should) be set to LINK_STATE_UNKNOWN by default.


the question is whether there is any other meaning for this state.
For example "I have not started up yet"



So the only question we don't know the answer to, at run-time, is 
whether a driver may *ever* set the link state (i.e., it thinks it 
knows how to), and hence whether or not tools like dhclient should 
try to wait for that to happen. That is the problem that an 
interface capability would solve.


For the purposes of ECMP, you just need to decide on your policy: 
map UNKNOWN to either UP or DOWN for your purposes.


yes
this is a good approach assuming that there is no other meaning.




Robert


___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r205024 - head/sys/net

2010-03-12 Thread Julian Elischer

Robert N. M. Watson wrote:

On Mar 12, 2010, at 8:30 AM, Qing Li wrote:


I believe what Julian means is the following:

1. in the driver, I do this

ifp->if_flags |= IFF_KNOWS_LINK_STATE;

2. In route.h, I do this

   if (ifp->flags & IFF_KNOWS_LINK_STATE) && (ifp->if_link_state ==
LINK_STATE_UP)
  return 1;


Please do *NOT* do this:

(1) Do not overload if_flags with more run-time set flags without well-defined 
atomicity properties
(2) Why isn't LINK_STATE_UNKNOWN already sufficient here?

The only change I think would be useful is adding a new IFCAP flag that allows 
a driver to statically declare that it will someday set the link state. But I 
don't think that helps with ECMP, that's just for the benefit of programs like 
dhclient that care about future events rather than current state.

Robert


I think that any driver that can set link stats will set it to 1 or 2 
and that we can treat a state of 0 as "I don't know what the heck you 
are talking about".


My comment on a flag for this purpose was in the abstract. I didn't
necessarily mean that a particular bit in a particular register be 
used for this purpose.. The aim is to be able to discern a driver that

will not give a useful result and should therefore be assumed to be
online at all times. It looks to me that this can be achieved
several ways, including looking for the UNKNOWN link state, or adding 
a bit in the word to indicate it is valid, to adding a bit somewhere else.






-- Qing


On Fri, Mar 12, 2010 at 12:26 AM, Robert N. M. Watson
 wrote:

On Mar 12, 2010, at 8:11 AM, Qing Li wrote:


I like Julian's suggestion because it is simple and very low risk.
And there isn't a need to check for interface type any more.
Here is why:

1. The interfaces that are popular and modern are already supporting
   link_state. So for these drivers, and there are just a few, I will go set
   its if_flags to include "can change link_state".

2. For the existing dated drivers, because that flag bit is never set,
   no check is done.

3. In the mean time, we try to convert the drivers progressively.

4. If one wants to do ECMP and not having packets go into a black
   hole when the physical link is down, that person can ping the ML
   and ask for driver compatibility list. If we haven't converted that
   particular driver by then, we will update the driver if it's capable
   at that time.


Today, we support three link state values:

170 /*
171  * Values for if_link_state.
172  */
173 #define LINK_STATE_UNKNOWN  0   /* link invalid/unknown */
174 #define LINK_STATE_DOWN 1   /* link is down */
175 #define LINK_STATE_UP   2   /* link is up */

I'm confused about Julian's proposal because it seems to me that we already 
know when a driver hasn't set or is unable to determine the link state: it will 
(should) be set to LINK_STATE_UNKNOWN by default.

So the only question we don't know the answer to, at run-time, is whether a 
driver may *ever* set the link state (i.e., it thinks it knows how to), and 
hence whether or not tools like dhclient should try to wait for that to happen. 
That is the problem that an interface capability would solve.

For the purposes of ECMP, you just need to decide on your policy: map UNKNOWN 
to either UP or DOWN for your purposes.

Robert


___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r205024 - head/sys/net

2010-03-12 Thread Robert N. M. Watson

On Mar 12, 2010, at 8:30 AM, Qing Li wrote:

> I believe what Julian means is the following:
> 
> 1. in the driver, I do this
> 
> ifp->if_flags |= IFF_KNOWS_LINK_STATE;
> 
> 2. In route.h, I do this
> 
>if (ifp->flags & IFF_KNOWS_LINK_STATE) && (ifp->if_link_state ==
> LINK_STATE_UP)
>   return 1;

Please do *NOT* do this:

(1) Do not overload if_flags with more run-time set flags without well-defined 
atomicity properties
(2) Why isn't LINK_STATE_UNKNOWN already sufficient here?

The only change I think would be useful is adding a new IFCAP flag that allows 
a driver to statically declare that it will someday set the link state. But I 
don't think that helps with ECMP, that's just for the benefit of programs like 
dhclient that care about future events rather than current state.

Robert


> 
> -- Qing
> 
> 
> On Fri, Mar 12, 2010 at 12:26 AM, Robert N. M. Watson
>  wrote:
>> 
>> On Mar 12, 2010, at 8:11 AM, Qing Li wrote:
>> 
>>> I like Julian's suggestion because it is simple and very low risk.
>>> And there isn't a need to check for interface type any more.
>>> Here is why:
>>> 
>>> 1. The interfaces that are popular and modern are already supporting
>>>link_state. So for these drivers, and there are just a few, I will go set
>>>its if_flags to include "can change link_state".
>>> 
>>> 2. For the existing dated drivers, because that flag bit is never set,
>>>no check is done.
>>> 
>>> 3. In the mean time, we try to convert the drivers progressively.
>>> 
>>> 4. If one wants to do ECMP and not having packets go into a black
>>>hole when the physical link is down, that person can ping the ML
>>>and ask for driver compatibility list. If we haven't converted that
>>>particular driver by then, we will update the driver if it's capable
>>>at that time.
>> 
>> 
>> Today, we support three link state values:
>> 
>> 170 /*
>> 171  * Values for if_link_state.
>> 172  */
>> 173 #define LINK_STATE_UNKNOWN  0   /* link invalid/unknown */
>> 174 #define LINK_STATE_DOWN 1   /* link is down */
>> 175 #define LINK_STATE_UP   2   /* link is up */
>> 
>> I'm confused about Julian's proposal because it seems to me that we already 
>> know when a driver hasn't set or is unable to determine the link state: it 
>> will (should) be set to LINK_STATE_UNKNOWN by default.
>> 
>> So the only question we don't know the answer to, at run-time, is whether a 
>> driver may *ever* set the link state (i.e., it thinks it knows how to), and 
>> hence whether or not tools like dhclient should try to wait for that to 
>> happen. That is the problem that an interface capability would solve.
>> 
>> For the purposes of ECMP, you just need to decide on your policy: map 
>> UNKNOWN to either UP or DOWN for your purposes.
>> 
>> Robert

___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r205024 - head/sys/net

2010-03-12 Thread Qing Li
Nope, I meant Julian, and what he proposed, and if I understood
correctly, is the simplest
approach and easily done.

-- Qing


On Fri, Mar 12, 2010 at 12:29 AM, Robert N. M. Watson
 wrote:
>
> On Mar 12, 2010, at 8:11 AM, Qing Li wrote:
>
>> I like Julian's suggestion because it is simple and very low risk.
>> And there isn't a need to check for interface type any more.
>> Here is why:
>
> Re-reading this e-mail: perhaps you mean Juli, not Julian?
>
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r205024 - head/sys/net

2010-03-12 Thread Qing Li
I believe what Julian means is the following:

1. in the driver, I do this

 ifp->if_flags |= IFF_KNOWS_LINK_STATE;

2. In route.h, I do this

if (ifp->flags & IFF_KNOWS_LINK_STATE) && (ifp->if_link_state ==
LINK_STATE_UP)
   return 1;

-- Qing


On Fri, Mar 12, 2010 at 12:26 AM, Robert N. M. Watson
 wrote:
>
> On Mar 12, 2010, at 8:11 AM, Qing Li wrote:
>
>> I like Julian's suggestion because it is simple and very low risk.
>> And there isn't a need to check for interface type any more.
>> Here is why:
>>
>> 1. The interfaces that are popular and modern are already supporting
>>    link_state. So for these drivers, and there are just a few, I will go set
>>    its if_flags to include "can change link_state".
>>
>> 2. For the existing dated drivers, because that flag bit is never set,
>>    no check is done.
>>
>> 3. In the mean time, we try to convert the drivers progressively.
>>
>> 4. If one wants to do ECMP and not having packets go into a black
>>    hole when the physical link is down, that person can ping the ML
>>    and ask for driver compatibility list. If we haven't converted that
>>    particular driver by then, we will update the driver if it's capable
>>    at that time.
>
>
> Today, we support three link state values:
>
> 170 /*
> 171  * Values for if_link_state.
> 172  */
> 173 #define LINK_STATE_UNKNOWN      0       /* link invalid/unknown */
> 174 #define LINK_STATE_DOWN         1       /* link is down */
> 175 #define LINK_STATE_UP           2       /* link is up */
>
> I'm confused about Julian's proposal because it seems to me that we already 
> know when a driver hasn't set or is unable to determine the link state: it 
> will (should) be set to LINK_STATE_UNKNOWN by default.
>
> So the only question we don't know the answer to, at run-time, is whether a 
> driver may *ever* set the link state (i.e., it thinks it knows how to), and 
> hence whether or not tools like dhclient should try to wait for that to 
> happen. That is the problem that an interface capability would solve.
>
> For the purposes of ECMP, you just need to decide on your policy: map UNKNOWN 
> to either UP or DOWN for your purposes.
>
> Robert
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r205024 - head/sys/net

2010-03-12 Thread Robert N. M. Watson

On Mar 12, 2010, at 8:11 AM, Qing Li wrote:

> I like Julian's suggestion because it is simple and very low risk.
> And there isn't a need to check for interface type any more.
> Here is why:

Re-reading this e-mail: perhaps you mean Juli, not Julian?

Robert

> 
> 1. The interfaces that are popular and modern are already supporting
>link_state. So for these drivers, and there are just a few, I will go set
>its if_flags to include "can change link_state".
> 
> 2. For the existing dated drivers, because that flag bit is never set,
>no check is done.
> 
> 3. In the mean time, we try to convert the drivers progressively.
> 
> 4. If one wants to do ECMP and not having packets go into a black
>hole when the physical link is down, that person can ping the ML
>and ask for driver compatibility list. If we haven't converted that
>particular driver by then, we will update the driver if it's capable
>at that time.
> 
> -- Qing
> 
> 
> On Fri, Mar 12, 2010 at 12:00 AM, Robert N. M. Watson
>  wrote:
>> 
>> On Mar 12, 2010, at 7:52 AM, Qing Li wrote:
>> 
 Is there any way we can pick up via an assertion that an interface driver 
 has failed to implement this functionality? This has never been a historic 
 requirement, so I suspect there are a lot of drivers floating around that 
 fail to meet the requirement. Also, is this for IFT_ETHER only, or also 
 other link types?
>>> 
>>> Not sure if I get the assertion suggestion. How would an assertion help 
>>> here ?
>> 
>> I think my proposal is similar to what Juli is suggesting:
>> 
>> - Define a new interface capability for link state detection.
>> - If a packet is sent or received on the interface, the capability is set, 
>> but the link state hasn't been set, panic.
>> - If a packet is sent received on the interface, the capability isn't set, 
>> and the link state has been set, panic.
>> 
>> That way the system blows up nicely and immediately, rather than dhclient 
>> simply never working, etc. Also, that way, testing for link state support is 
>> done at a point when we know the interface is live (a packet is sent or 
>> received).
>> 
>> Finally, it means that code interested in link state isn't testing for one 
>> of (n) IFT_ types it thinks should have link state, but instead testing 
>> specifically whether the driver declares link state support. Of course, then 
>> you have to decide how to behave if a configured interface ECMP is running 
>> on doesn't support link state: the answer there is probably to assume it is 
>> always up, which would make this work for all those drivers that current 
>> fail to implement it. And if the hardware can't support link state, which 
>> some historic (and perhaps future) link types can't, things still work.
>> 
>> Robert

___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r205024 - head/sys/net

2010-03-12 Thread Juli Mallett
On Fri, Mar 12, 2010 at 00:11, Qing Li  wrote:
> I like Julian's suggestion because it is simple and very low risk.
> And there isn't a need to check for interface type any more.
> Here is why:

For actual link state, you can already see whether a driver is in
UNKNOWN state, like:

%%%
/*
 * Values for if_link_state.
 */
#define LINK_STATE_UNKNOWN  0   /* link invalid/unknown */
#define LINK_STATE_DOWN 1   /* link is down */
#define LINK_STATE_UP   2   /* link is up */
%%%

Therefore you should be able to easily add assertions and debugging
printfs for interfaces that don't event support *setting* link state
-- which is the real problem more than changing link state, since a
driver probably won't report link as down if it has no intention of
changing it.

It doesn't help that there are two things wrt link state floating
around -- one in the form of LINK_STATE (the one that matters here)
and the other in ifmedia stuff.  I think that's confusing this
discussion gratuitously.

Juli.
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r205024 - head/sys/net

2010-03-12 Thread Robert N. M. Watson

On Mar 12, 2010, at 8:11 AM, Qing Li wrote:

> I like Julian's suggestion because it is simple and very low risk.
> And there isn't a need to check for interface type any more.
> Here is why:
> 
> 1. The interfaces that are popular and modern are already supporting
>link_state. So for these drivers, and there are just a few, I will go set
>its if_flags to include "can change link_state".
> 
> 2. For the existing dated drivers, because that flag bit is never set,
>no check is done.
> 
> 3. In the mean time, we try to convert the drivers progressively.
> 
> 4. If one wants to do ECMP and not having packets go into a black
>hole when the physical link is down, that person can ping the ML
>and ask for driver compatibility list. If we haven't converted that
>particular driver by then, we will update the driver if it's capable
>at that time.


Today, we support three link state values:

170 /*
171  * Values for if_link_state.
172  */
173 #define LINK_STATE_UNKNOWN  0   /* link invalid/unknown */
174 #define LINK_STATE_DOWN 1   /* link is down */
175 #define LINK_STATE_UP   2   /* link is up */

I'm confused about Julian's proposal because it seems to me that we already 
know when a driver hasn't set or is unable to determine the link state: it will 
(should) be set to LINK_STATE_UNKNOWN by default.

So the only question we don't know the answer to, at run-time, is whether a 
driver may *ever* set the link state (i.e., it thinks it knows how to), and 
hence whether or not tools like dhclient should try to wait for that to happen. 
That is the problem that an interface capability would solve.

For the purposes of ECMP, you just need to decide on your policy: map UNKNOWN 
to either UP or DOWN for your purposes.

Robert___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r205024 - head/sys/net

2010-03-12 Thread Qing Li
I like Julian's suggestion because it is simple and very low risk.
And there isn't a need to check for interface type any more.
Here is why:

1. The interfaces that are popular and modern are already supporting
link_state. So for these drivers, and there are just a few, I will go set
its if_flags to include "can change link_state".

2. For the existing dated drivers, because that flag bit is never set,
no check is done.

3. In the mean time, we try to convert the drivers progressively.

4. If one wants to do ECMP and not having packets go into a black
hole when the physical link is down, that person can ping the ML
and ask for driver compatibility list. If we haven't converted that
particular driver by then, we will update the driver if it's capable
at that time.

-- Qing


On Fri, Mar 12, 2010 at 12:00 AM, Robert N. M. Watson
 wrote:
>
> On Mar 12, 2010, at 7:52 AM, Qing Li wrote:
>
>>> Is there any way we can pick up via an assertion that an interface driver 
>>> has failed to implement this functionality? This has never been a historic 
>>> requirement, so I suspect there are a lot of drivers floating around that 
>>> fail to meet the requirement. Also, is this for IFT_ETHER only, or also 
>>> other link types?
>>
>> Not sure if I get the assertion suggestion. How would an assertion help here 
>> ?
>
> I think my proposal is similar to what Juli is suggesting:
>
> - Define a new interface capability for link state detection.
> - If a packet is sent or received on the interface, the capability is set, 
> but the link state hasn't been set, panic.
> - If a packet is sent received on the interface, the capability isn't set, 
> and the link state has been set, panic.
>
> That way the system blows up nicely and immediately, rather than dhclient 
> simply never working, etc. Also, that way, testing for link state support is 
> done at a point when we know the interface is live (a packet is sent or 
> received).
>
> Finally, it means that code interested in link state isn't testing for one of 
> (n) IFT_ types it thinks should have link state, but instead testing 
> specifically whether the driver declares link state support. Of course, then 
> you have to decide how to behave if a configured interface ECMP is running on 
> doesn't support link state: the answer there is probably to assume it is 
> always up, which would make this work for all those drivers that current fail 
> to implement it. And if the hardware can't support link state, which some 
> historic (and perhaps future) link types can't, things still work.
>
> Robert
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r205024 - head/sys/net

2010-03-12 Thread Robert N. M. Watson

On Mar 12, 2010, at 7:52 AM, Qing Li wrote:

>> Is there any way we can pick up via an assertion that an interface driver 
>> has failed to implement this functionality? This has never been a historic 
>> requirement, so I suspect there are a lot of drivers floating around that 
>> fail to meet the requirement. Also, is this for IFT_ETHER only, or also 
>> other link types?
> 
> Not sure if I get the assertion suggestion. How would an assertion help here ?

I think my proposal is similar to what Juli is suggesting:

- Define a new interface capability for link state detection.
- If a packet is sent or received on the interface, the capability is set, but 
the link state hasn't been set, panic.
- If a packet is sent received on the interface, the capability isn't set, and 
the link state has been set, panic.

That way the system blows up nicely and immediately, rather than dhclient 
simply never working, etc. Also, that way, testing for link state support is 
done at a point when we know the interface is live (a packet is sent or 
received).

Finally, it means that code interested in link state isn't testing for one of 
(n) IFT_ types it thinks should have link state, but instead testing 
specifically whether the driver declares link state support. Of course, then 
you have to decide how to behave if a configured interface ECMP is running on 
doesn't support link state: the answer there is probably to assume it is always 
up, which would make this work for all those drivers that current fail to 
implement it. And if the hardware can't support link state, which some historic 
(and perhaps future) link types can't, things still work.

Robert___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r205024 - head/sys/net

2010-03-11 Thread Qing Li
>
> Is there any way we can pick up via an assertion that an interface driver has 
> failed to implement this functionality? This has never been a historic 
> requirement, so I suspect there are a lot of drivers floating around that 
> fail to meet the requirement. Also, is this for IFT_ETHER only, or also other 
> link types?
>

Not sure if I get the assertion suggestion. How would an assertion help here ?

-- Qing
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r205024 - head/sys/net

2010-03-11 Thread Robert N. M. Watson

On Mar 12, 2010, at 12:18 AM, Qing Li wrote:

>  You definitely have a very good point here.  I was a bit surprised
>  during debugging that the link state is not consistently initialized
>  and by far not enforced across all of the drivers. Admittedly I checked
>  the most commonly deployed devices and they are in good state.
> 
>  I certainly appreciate your patience on this one and will try to get
>  it resolved quickly.

One of the reasons drivers don't do this consistently his that historically, 
hardware has not consistently supported link state detection. This now does 
seem to be a standard feature, but I think it would be useful to continue to 
support some notion of a driver not supporting it, hence my thoughts on a link 
state capability: only test link state if the driver can implement it. 
Otherwise, you end up with a link state undefined state, which likely comes to 
much the same thing, and is presumably what in practice you get today on 
drivers that don't set it.

Robert___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r205024 - head/sys/net

2010-03-11 Thread Robert N. M. Watson

On Mar 11, 2010, at 11:30 PM, Qing Li wrote:

>  What you raised is definitely a possibility and these fixes take the
>  similar approach. I am going to try and go through each of these
>  drivers in /sys/dev/ and converting them, very soon.

Is there any way we can pick up via an assertion that an interface driver has 
failed to implement this functionality? This has never been a historic 
requirement, so I suspect there are a lot of drivers floating around that fail 
to meet the requirement. Also, is this for IFT_ETHER only, or also other link 
types?

Robert___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r205024 - head/sys/net

2010-03-11 Thread Qing Li
That's a good idea. I will take your approach.

-- Qing


On Thu, Mar 11, 2010 at 11:15 PM, Julian Elischer  wrote:
> Juli Mallett wrote:
>>
>> On Thu, Mar 11, 2010 at 15:39, Qing Li  wrote:
>>>
>>> I guess it's a good time to clean things up. The if_link_state code has
>>> been
>>> around for quite some time, either it be fully utilized or not be there
>>> at all.
>>> The inconsistency is the root cause.
>>
>> Sure.  There is an increasing amount of stuff that network drivers are
>> expected to do, but they work without doing them.  It's easy to think
>> you have a functioning network driver and that you can get by without
>> adding support for media changes and link status reporting, etc.
>>
>>> I will try going through these tonight and hopefully the fix all take a
>>> common approach.
>
> probably should add a flag that means "we have media state"
> and if it is not set, assume it is always on.
>
>>
>> If you can think of a way to add some invariants (warn the first time
>> a driver receives a packet without having ever set the link state,
>> make sure the media status callback sets the valid flag in the
>> request, etc) that would probably be very helpful for people who are
>> writing network drivers.  If I hadn't been following the threads about
>> your changes, I would have had to spend much longer fixing the Octeon
>> port's Ethernet driver, wondering why suddenly it broke and
>> instrumenting the routing code.  A printf would go a long way.
>>
>> Juli.
>
>
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r205024 - head/sys/net

2010-03-11 Thread Julian Elischer

Juli Mallett wrote:

On Thu, Mar 11, 2010 at 15:39, Qing Li  wrote:

I guess it's a good time to clean things up. The if_link_state code has been
around for quite some time, either it be fully utilized or not be there at all.
The inconsistency is the root cause.


Sure.  There is an increasing amount of stuff that network drivers are
expected to do, but they work without doing them.  It's easy to think
you have a functioning network driver and that you can get by without
adding support for media changes and link status reporting, etc.


I will try going through these tonight and hopefully the fix all take a
common approach.


probably should add a flag that means "we have media state"
and if it is not set, assume it is always on.



If you can think of a way to add some invariants (warn the first time
a driver receives a packet without having ever set the link state,
make sure the media status callback sets the valid flag in the
request, etc) that would probably be very helpful for people who are
writing network drivers.  If I hadn't been following the threads about
your changes, I would have had to spend much longer fixing the Octeon
port's Ethernet driver, wondering why suddenly it broke and
instrumenting the routing code.  A printf would go a long way.

Juli.


___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r205024 - head/sys/net

2010-03-11 Thread John Hay
On Thu, Mar 11, 2010 at 03:35:13PM -0800, Juli Mallett wrote:
> On Thu, Mar 11, 2010 at 15:30, Qing Li  wrote:
> >>
> >> A couple of questions:
> >>
> >> (1) It used to be the case that quite a few interface drivers and types
> >> didn't have a notion of "link up" -- especially older ethernet devices. ?Do
> >> those all have the same problem? ?It was probably a design oversight that
> >>?devices don't declare an explicit capability for "can report link state".
> >>
> >
> > ?What you raised is definitely a possibility and these fixes take the
> > ?similar approach. I am going to try and go through each of these
> > ?drivers in /sys/dev/ and converting them, very soon.
> 
> Go through drivers in the embedded port directories, too.  The Octeon
> port's Ethernet driver was broken by this, and it looks like the
> Atheros if_arge is probably broken, too.  I would even suggest going
> back to the old behavior briefly while the port maintainers are given
> an opportunity to update their drivers.  Actually, it looks like only
> MIPS has Ethernet drivers outside of dev/ at a quick glance, but I'd
> be surprised if there weren't other broken examples.

There is also if_npe in the arm/xscale/ixp425 directory and probably
others in the rest of the arm directories.

John
-- 
John Hay -- j...@meraka.csir.co.za / j...@freebsd.org
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r205024 - head/sys/net

2010-03-11 Thread Qing Li
>
> If you can think of a way to add some invariants (warn the first time
> a driver receives a packet without having ever set the link state,
> make sure the media status callback sets the valid flag in the
> request, etc) that would probably be very helpful for people who are
> writing network drivers.  If I hadn't been following the threads about
> your changes, I would have had to spend much longer fixing the Octeon
> port's Ethernet driver, wondering why suddenly it broke and
> instrumenting the routing code.  A printf would go a long way.
>

  You definitely have a very good point here.  I was a bit surprised
  during debugging that the link state is not consistently initialized
  and by far not enforced across all of the drivers. Admittedly I checked
  the most commonly deployed devices and they are in good state.

  I certainly appreciate your patience on this one and will try to get
  it resolved quickly.

  -- Qing
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r205024 - head/sys/net

2010-03-11 Thread Juli Mallett
On Thu, Mar 11, 2010 at 15:39, Qing Li  wrote:
> I guess it's a good time to clean things up. The if_link_state code has been
> around for quite some time, either it be fully utilized or not be there at 
> all.
> The inconsistency is the root cause.

Sure.  There is an increasing amount of stuff that network drivers are
expected to do, but they work without doing them.  It's easy to think
you have a functioning network driver and that you can get by without
adding support for media changes and link status reporting, etc.

> I will try going through these tonight and hopefully the fix all take a
> common approach.

If you can think of a way to add some invariants (warn the first time
a driver receives a packet without having ever set the link state,
make sure the media status callback sets the valid flag in the
request, etc) that would probably be very helpful for people who are
writing network drivers.  If I hadn't been following the threads about
your changes, I would have had to spend much longer fixing the Octeon
port's Ethernet driver, wondering why suddenly it broke and
instrumenting the routing code.  A printf would go a long way.

Juli.
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r205024 - head/sys/net

2010-03-11 Thread Qing Li
I guess it's a good time to clean things up. The if_link_state code has been
around for quite some time, either it be fully utilized or not be there at all.
The inconsistency is the root cause.

I will try going through these tonight and hopefully the fix all take a
common approach.

-- Qing


On Thu, Mar 11, 2010 at 3:35 PM, Juli Mallett  wrote:
> On Thu, Mar 11, 2010 at 15:30, Qing Li  wrote:
>>>
>>> A couple of questions:
>>>
>>> (1) It used to be the case that quite a few interface drivers and types
>>> didn't have a notion of "link up" -- especially older ethernet devices.  Do
>>> those all have the same problem?  It was probably a design oversight that
>>> devices don't declare an explicit capability for "can report link state".
>>>
>>
>>  What you raised is definitely a possibility and these fixes take the
>>  similar approach. I am going to try and go through each of these
>>  drivers in /sys/dev/ and converting them, very soon.
>
> Go through drivers in the embedded port directories, too.  The Octeon
> port's Ethernet driver was broken by this, and it looks like the
> Atheros if_arge is probably broken, too.  I would even suggest going
> back to the old behavior briefly while the port maintainers are given
> an opportunity to update their drivers.  Actually, it looks like only
> MIPS has Ethernet drivers outside of dev/ at a quick glance, but I'd
> be surprised if there weren't other broken examples.
>
> Juli.
>
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r205024 - head/sys/net

2010-03-11 Thread Juli Mallett
On Thu, Mar 11, 2010 at 15:30, Qing Li  wrote:
>>
>> A couple of questions:
>>
>> (1) It used to be the case that quite a few interface drivers and types
>> didn't have a notion of "link up" -- especially older ethernet devices.  Do
>> those all have the same problem?  It was probably a design oversight that
>> devices don't declare an explicit capability for "can report link state".
>>
>
>  What you raised is definitely a possibility and these fixes take the
>  similar approach. I am going to try and go through each of these
>  drivers in /sys/dev/ and converting them, very soon.

Go through drivers in the embedded port directories, too.  The Octeon
port's Ethernet driver was broken by this, and it looks like the
Atheros if_arge is probably broken, too.  I would even suggest going
back to the old behavior briefly while the port maintainers are given
an opportunity to update their drivers.  Actually, it looks like only
MIPS has Ethernet drivers outside of dev/ at a quick glance, but I'd
be surprised if there weren't other broken examples.

Juli.
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r205024 - head/sys/net

2010-03-11 Thread Qing Li
>
> A couple of questions:
>
> (1) It used to be the case that quite a few interface drivers and types
> didn't have a notion of "link up" -- especially older ethernet devices.  Do
> those all have the same problem?  It was probably a design oversight that
> devices don't declare an explicit capability for "can report link state".
>

  What you raised is definitely a possibility and these fixes take the
  similar approach. I am going to try and go through each of these
  drivers in /sys/dev/ and converting them, very soon.

>
> (2) While loopback interfaces don't really have a link state, they can be
> administratively down -- should/do you check that as well as link state?
> And more generally, even if link is up, administratively down should be
> obeyed?
>

  For loopback interfaces, althgouth administrative these can be taken down,
  I personally cannot think one practical usage case where ECMP across
  loopback interfaces would be interesting or usefaul. So I can think of
  very little reason to be concerned in the loopback case.

>
> Finally, it would be neat if there were a way to have information beyond
> link state influence the choice to balance to a particular route/interface.
>  For example, imagine if I have a router with ECMP, and on the other side on
> a single ethernet segment, I have two DSL modems.  The ethernet link will
> remain up, but I may (via out-of-band mechanisms, such as SNMP or an active
> probe) be able to tell that one of the DSL lines is down.  Is there a way to
> push that information into the kernel currently without deleting the routes,
> and instead say "yeah, but for ECMP purposes this is 'down'"?
>

  The above really falls into policy based routing. And policy based
  routing infrastrucutre is something I have already been working on
but cannot yet
  push back into -current. In fact Julian and I had a conversation about
  this topic during the FIBs implementation time in late 2008.

  This infrastructure enhancement is definitely coming but I cannot yet prvoide
  a timeline for merge back.

  It's mostly a time issue.

  Let me know if I have answered these questions to your satisfaction.

  -- Qing
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r205024 - head/sys/net

2010-03-11 Thread Robert Watson


On Thu, 11 Mar 2010, Qing Li wrote:


 The if_tap interface is of IFT_ETHERNET type, but it
 does not set or update the if_link_state variable.
 As such RT_LINK_IS_UP() fails for the if_tap interface.

 Also, the RT_LINK_IS_UP() needs to bypass all loopback
 interfaces because loopback interfaces are considered
 up logically as long as the system is running.

 This patch fixes the above issues by setting and updating
 the if_link_state variable when the tap interface is
 opened or closed respectively. Similary approach is
 already done in the if_tun device.


A couple of questions:

(1) It used to be the case that quite a few interface drivers and types didn't
have a notion of "link up" -- especially older ethernet devices.  Do those
all have the same problem?  It was probably a design oversight that
devices don't declare an explicit capability for "can report link state".

(2) While loopback interfaces don't really have a link state, they can be
administratively down -- should/do you check that as well as link state?
And more generally, even if link is up, administratively down should be
obeyed?

Finally, it would be neat if there were a way to have information beyond link 
state influence the choice to balance to a particular route/interface.  For 
example, imagine if I have a router with ECMP, and on the other side on a 
single ethernet segment, I have two DSL modems.  The ethernet link will remain 
up, but I may (via out-of-band mechanisms, such as SNMP or an active probe) be 
able to tell that one of the DSL lines is down.  Is there a way to push that 
information into the kernel currently without deleting the routes, and instead 
say "yeah, but for ECMP purposes this is 'down'"?


Robert
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"