Code review request
I've been shepherding this patch in my p4 tree for a long time. It removes the obsolete support for other systems in if_spppsubr.c. Is there a reason I shouldn't commit this? Warner Index: if_spppsubr.c === --- if_spppsubr.c (revision 182085) +++ if_spppsubr.c (working copy) @@ -23,38 +23,22 @@ #include -#if defined(__FreeBSD__) && __FreeBSD__ >= 3 #include "opt_inet.h" #include "opt_inet6.h" #include "opt_ipx.h" -#endif -#ifdef NetBSD1_3 -# if NetBSD1_3 > 6 -# include "opt_inet.h" -# include "opt_inet6.h" -# include "opt_iso.h" -# endif -#endif - #include #include #include #include #include #include -#if defined(__FreeBSD__) && __FreeBSD__ >= 3 #include -#endif #include #include #include -#if defined (__OpenBSD__) -#include -#else #include -#endif #include #include @@ -65,10 +49,6 @@ #include #include -#if defined (__NetBSD__) || defined (__OpenBSD__) -#include /* XXX for softnet */ -#endif - #include #include @@ -82,11 +62,7 @@ #include #endif -#if defined (__FreeBSD__) || defined (__OpenBSD__) -# include -#else -# include -#endif +#include #ifdef IPX #include @@ -95,12 +71,7 @@ #include -#if defined(__FreeBSD__) && __FreeBSD__ >= 3 -# define IOCTL_CMD_T u_long -#else -# define IOCTL_CMD_T int -#endif - +#define IOCTL_CMD_Tu_long #define MAXALIVECNT 3 /* max. alive packets */ /* @@ -261,13 +232,8 @@ void(*scr)(struct sppp *sp); }; -#if defined(__FreeBSD__) && __FreeBSD__ >= 3 && __FreeBSD_version < 501113 -#defineSPP_FMT "%s%d: " -#defineSPP_ARGS(ifp) (ifp)->if_name, (ifp)->if_unit -#else #defineSPP_FMT "%s: " #defineSPP_ARGS(ifp) (ifp)->if_xname -#endif #define SPPP_LOCK(sp) \ do { \ @@ -1422,11 +1388,7 @@ ++sp->pp_loopcnt; /* Generate new local sequence number */ -#if defined(__FreeBSD__) && __FreeBSD__ >= 3 sp->pp_seq[IDX_LCP] = random(); -#else - sp->pp_seq[IDX_LCP] ^= time.tv_sec ^ time.tv_usec; -#endif break; } sp->pp_loopcnt = 0; @@ -2671,11 +2633,7 @@ if (magic == ~sp->lcp.magic) { if (debug) log(-1, "magic glitch "); -#if defined(__FreeBSD__) && __FreeBSD__ >= 3 sp->lcp.magic = random(); -#else - sp->lcp.magic = time.tv_sec + time.tv_usec; -#endif } else { sp->lcp.magic = magic; if (debug) @@ -2856,11 +2814,7 @@ if (sp->lcp.opts & (1 << LCP_OPT_MAGIC)) { if (! sp->lcp.magic) -#if defined(__FreeBSD__) && __FreeBSD__ >= 3 sp->lcp.magic = random(); -#else - sp->lcp.magic = time.tv_sec + time.tv_usec; -#endif opt[i++] = LCP_OPT_MAGIC; opt[i++] = 6; opt[i++] = sp->lcp.magic >> 24; @@ -4383,15 +4337,7 @@ /* Compute random challenge. */ ch = (u_long *)sp->myauth.challenge; -#if defined(__FreeBSD__) && __FreeBSD__ >= 3 read_random(&seed, sizeof seed); -#else - { - struct timeval tv; - microtime(&tv); - seed = tv.tv_sec ^ tv.tv_usec; - } -#endif ch[0] = seed ^ random(); ch[1] = seed ^ random(); ch[2] = seed ^ random(); @@ -4900,17 +4846,7 @@ * aliases don't make any sense on a p2p link anyway. */ si = 0; -#if defined(__FreeBSD__) && __FreeBSD__ >= 3 TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) -#elif defined(__NetBSD__) || defined (__OpenBSD__) - for (ifa = TAILQ_FIRST(&ifp->if_addrlist); -ifa; -ifa = TAILQ_NEXT(ifa, ifa_list)) -#else - for (ifa = ifp->if_addrlist; -ifa; -ifa = ifa->ifa_next) -#endif if (ifa->ifa_addr->sa_family == AF_INET) { si = (struct sockaddr_in *)ifa->ifa_addr; sm = (struct sockaddr_in *)ifa->ifa_netmask; @@ -4949,17 +4885,7 @@ * aliases don't make any sense on a p2p link anyway. */ si = 0; -#if defined(__FreeBSD__) && __FreeBSD__ >= 3 TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) -#elif defined(__NetBSD__) || defined (__OpenBSD__) - for (ifa = TAILQ_FIRST(&ifp->if_addrlist); -ifa; -ifa = TAILQ_NEXT(ifa, ifa_list)) -#else - for (ifa = ifp->if_addrlist; -ifa; -ifa = ifa->ifa_next) -#endif { if (ifa->ifa_addr->sa_family == AF_INET) { @@ -4972,17 +4898,6 @@
Re: strange TCP issue on RELENG_7
At 02:18 AM 8/24/2008, Julian Elischer wrote: ok so it might be related to the MRT code... I assume he only has one Routing table.. Yes, just one routing table Theoretically it shoudl work the same as before if N==1 but I can imagine a case where it didn't. especially if arp and interffaces are involved.. Mike, can you give me a repro example? I am not sure how, but it happens on this machine within seconds of all its applications starting up. I am speculating it has to do with the fact the machine has multiple public interfaces as well as IPs aliased to lo0 so that packets will come in em1 destined for em1 and lo0, but will always go out em0 ? The same for the kernel from HEAD shows the same behaviour. The problem started with the tcp offload code http://lists.freebsd.org/pipermail/freebsd-stable/2008-August/044468.html has more details ---Mike ___ freebsd-net@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-net To unsubscribe, send any mail to "[EMAIL PROTECTED]"
[CFT/R] IPv4 source address selection
Hi, I have a patch, that was inspired by work from Y!, to do porper IPv4 source address selection for unbound sockets (with multi-IP jails). You can temporary find it here: http://people.freebsd.org/~bz/20080823-01-in_pcbladdr.diff People running my latest jail patches have been ``testing'' this without really knowing the last weeks. In case you wonder why, in the jail case, I loop over the ifa first before simply falling back to the primary jail IP (which is the only jail IP as in HEAD) -- this is because with the upcoming jail patches I have to check if any of possibly lots of IPs match any IP on an interface and only if none matches I have to fall back to the 'primary' jail IP. So the code has been prepared for upcoming changes already. Feel free to test it and report problems or unexpected behavior. Unless someone is going to cry it'll hit HEAD in a few days. /bz PS: in case you review this properly (not only glance at it or test it) let me know so I can punish you in the Reviewed by: line;-) -- Bjoern A. Zeeb Stop bit received. Insert coin for new game. ___ freebsd-net@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-net To unsubscribe, send any mail to "[EMAIL PROTECTED]"
Re: Code review request
M. Warner Losh wrote: I've been shepherding this patch in my p4 tree for a long time. It removes the obsolete support for other systems in if_spppsubr.c. Is there a reason I shouldn't commit this? Looks fine to me. ___ freebsd-net@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-net To unsubscribe, send any mail to "[EMAIL PROTECTED]"
Re: Code review request
M. Warner Losh wrote: I've been shepherding this patch in my p4 tree for a long time. It removes the obsolete support for other systems in if_spppsubr.c. Is there a reason I shouldn't commit this? It was there to ease the keeping code in sync with other systems. Please ask Joerg Wunsch before removal. rik Warner ___ freebsd-net@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-net To unsubscribe, send any mail to "[EMAIL PROTECTED]" ___ freebsd-net@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-net To unsubscribe, send any mail to "[EMAIL PROTECTED]"
Re: [CFT/R] IPv4 source address selection
Bjoern A. Zeeb wrote: Hi, I have a patch, that was inspired by work from Y!, to do porper IPv4 source address selection for unbound sockets (with multi-IP jails). Hi, This kinda overlaps with some other ideas I'd like to see go in. It looks good and if it's already been tested, it should probably go in anyway as it disentangles the logic and puts it in a separate function. I'm thinking we may wish to use criteria other than interface or jailed socket to select source address. I should point out though that we picked some stuff up from KAME to do source address selection but it's not in the IPv4 stack. cheers BMS ___ freebsd-net@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-net To unsubscribe, send any mail to "[EMAIL PROTECTED]"
Re: Code review request
In message: <[EMAIL PROTECTED]> Roman Kurakin <[EMAIL PROTECTED]> writes: : M. Warner Losh wrote: : > I've been shepherding this patch in my p4 tree for a long time. It : > removes the obsolete support for other systems in if_spppsubr.c. Is : > there a reason I shouldn't commit this? : > : It was there to ease the keeping code in sync with other systems. : Please ask Joerg Wunsch before removal. Yea, I knew that history. But there's been a lot of hacking on this file, and the ifdef's are for other systems that were contemporaneous with FreeBSD 3.0, but nothing newer. Plus the FreeBSDisms that are newer haven't been ifdef'd. Warner ___ freebsd-net@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-net To unsubscribe, send any mail to "[EMAIL PROTECTED]"
Code review
I did this a few years ago when trying to track down a problem with some realtek network chips that I was having problems with at Timing Solutions. I'd like to get this into the tree, since it was helpful then. Comments? Warner diff -ur src/sys/pci/if_rl.c newcard/src/sys/pci/if_rl.c --- src/sys/pci/if_rl.c 2008-08-23 22:21:15.0 -0600 +++ newcard/src/sys/pci/if_rl.c 2008-08-23 22:26:09.0 -0600 @@ -1253,18 +1253,120 @@ } static void +rl_twister_update(struct rl_softc *sc) +{ + uint16_t linktest; + static const uint32_t param[4][4] = { + {0xcb39de43, 0xcb39ce43, 0xfb38de03, 0xcb38de43}, + {0xcb39de43, 0xcb39ce43, 0xcb39ce83, 0xcb39ce83}, + {0xcb39de43, 0xcb39ce43, 0xcb39ce83, 0xcb39ce83}, + {0xbb39de43, 0xbb39ce43, 0xbb39ce83, 0xbb39ce83} + }; + + /* +* Tune the so-called twister registers of the RTL8139. These +* are used to compensate for impendence mismatches. The +* method for tuning these registes is undocumented and the +* following proceedure is collected from public sources. +*/ + switch (sc->rl_twister) + { + case CHK_LINK: + /* +* If we have a sufficent link, then we can proceed in +* the state machine to the next stage. If not, then +* disable further tuning after writing sane defaults. +*/ + if (CSR_READ_2(sc, RL_CSCFG) & RL_CSCFG_LINK_OK) { + CSR_WRITE_2(sc, RL_CSCFG, RL_CSCFG_LINK_DOWN_OFF_CMD); + sc->rl_twister = FIND_ROW; + } else { + CSR_WRITE_2(sc, RL_CSCFG, RL_CSCFG_LINK_DOWN_CMD); + CSR_WRITE_4(sc, RL_NWAYTST, RL_NWAYTST_CBL_TEST); + CSR_WRITE_4(sc, RL_PARA78, RL_PARA78_DEF); + CSR_WRITE_4(sc, RL_PARA7C, RL_PARA7C_DEF); + sc->rl_twister = DONE; + } + break; + case FIND_ROW: + /* +* Read how long it took to see the echo to find the tuning +* row to use. +*/ + linktest = CSR_READ_2(sc, RL_CSCFG) & RL_CSCFG_STATUS; + if (linktest == RL_CSCFG_ROW3) + sc->rl_twist_row = 3; + else if (linktest == RL_CSCFG_ROW2) + sc->rl_twist_row = 2; + else if (linktest == RL_CSCFG_ROW1) + sc->rl_twist_row = 1; + else + sc->rl_twist_row = 0; + sc->rl_twist_col = 0; + sc->rl_twister = SET_PARAM; + break; + case SET_PARAM: + if (sc->rl_twist_col == 0) + CSR_WRITE_4(sc, RL_NWAYTST, RL_NWAYTST_RESET); + CSR_WRITE_4(sc, RL_PARA7C, + param[sc->rl_twist_row][sc->rl_twist_col]); + if (++sc->rl_twist_col == 4) { + if (sc->rl_twist_row == 3) + sc->rl_twister = RECHK_LONG; + else + sc->rl_twister = DONE; + } + break; + case RECHK_LONG: + /* +* For long cables, we have to double check to make sure we +* don't mistune. +*/ + linktest = CSR_READ_2(sc, RL_CSCFG) & RL_CSCFG_STATUS; + if (linktest == RL_CSCFG_ROW3) + sc->rl_twister = DONE; + else { + CSR_WRITE_4(sc, RL_PARA7C, RL_PARA7C_RETUNE); + sc->rl_twister = RETUNE; + } + break; + case RETUNE: + /* Retune for a shorter cable (try column 2) */ + CSR_WRITE_4(sc, RL_NWAYTST, RL_NWAYTST_CBL_TEST); + CSR_WRITE_4(sc, RL_PARA78, RL_PARA78_DEF); + CSR_WRITE_4(sc, RL_PARA7C, RL_PARA7C_DEF); + CSR_WRITE_4(sc, RL_NWAYTST, RL_NWAYTST_RESET); + sc->rl_twist_row--; + sc->rl_twist_col = 0; + sc->rl_twister = SET_PARAM; + break; + + case DONE: + break; + } + +} + +static void rl_tick(void *xsc) { struct rl_softc *sc = xsc; struct mii_data *mii; + int ticks; RL_LOCK_ASSERT(sc); mii = device_get_softc(sc->rl_miibus); mii_tick(mii); + if (sc->rl_twister != DONE) + rl_twister_update(sc); + if (sc->rl_twister != DONE) + ticks = hz / 10; + else + ticks = hz; rl_watchdog(sc); - callout_reset(&sc->rl_stat_callout, hz, rl_tick, sc); + callout_reset(&sc->rl_stat_callout, ticks, rl_tick, sc); } #ifdef DEVICE_POLLING @@ -1490,6 +1592,13 @@ rl_stop(sc);