Re: svn commit: r205024 - head/sys/net
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
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
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
> > 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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
> > 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
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
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
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
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
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
> > 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
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
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
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
> > 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
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"