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

2010-03-13 Thread Paul B Mahol
On 3/11/10, Qing Li qin...@freebsd.org 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

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

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

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

2010-03-12 Thread Juli Mallett
On Fri, Mar 12, 2010 at 00:11, Qing Li qin...@freebsd.org 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:

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 rwat...@freebsd.org wrote: On Mar 12, 2010, at 8:11 AM, Qing Li wrote: I like Julian's suggestion because it is

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;

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)

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

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

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 qin...@freebsd.org 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

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

2010-03-12 Thread M. Warner Losh
In message: eaa228be1003111535s12febe62x91124b0a015fd...@mail.gmail.com Juli Mallett jmall...@freebsd.org writes: : On Thu, Mar 11, 2010 at 15:30, Qing Li qin...@freebsd.org wrote: : : A couple of questions: : : (1) It used to be the case that quite a few interface drivers and

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

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

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

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

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

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

2010-03-11 Thread Juli Mallett
On Thu, Mar 11, 2010 at 15:30, Qing Li qin...@freebsd.org 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

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

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

2010-03-11 Thread Juli Mallett
On Thu, Mar 11, 2010 at 15:39, Qing Li qin...@freebsd.org 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

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

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 qin...@freebsd.org 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

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 qin...@freebsd.org 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

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 jul...@elischer.org wrote: Juli Mallett wrote: On Thu, Mar 11, 2010 at 15:39, Qing Li qin...@freebsd.org wrote: I guess it's a good time to clean things up. The if_link_state code has

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

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

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