Re: Comments Please
On Sat, Oct 12, 2002 at 05:18:09PM -0600, M. Warner Losh wrote: OK. I'm not a network wonk, so I thought I'd run this by people here. What do people think. sounds ok -- removing explicit constants is always good. On passing: * While you are at it, grep etherbroadcastaddr sys/net*/* reveals the use of an explicit constant (6) in net/if_arp.h and netinet/if_ether.c; there is more of the same in net/bridge.c (my fault), net/if_atmsubr.c, netinet/if_ether.c, netncp/ncp_subr.c * there is no real reason to have etherbroadcastaddr as a variable. net/bridge.c has a macro, IS_ETHER_BROADCAST, which is much faster to evaluate on i386, and could be moved e.g. in net/ethernet.h and be used to check for ethernet broadcast addresses in net/if_ethersubr.c net/if_iso88025subr.c netatalk/aarp.c net/if_fddisubr.c This only leaves some usages of etherbroadcastaddr is in netinet/if_ether.c to set the address for outgoing broadcast packets. cheers luigi Warner --- //depot/user/imp/freebsd-imp/sys/net/if_ethersubr.c 2002/10/06 21:18:24 +++ //depot/user/imp/newcard/net/if_ethersubr.c 2002/10/11 22:58:57 @@ -124,7 +124,8 @@ static int ether_resolvemulti(struct ifnet *, struct sockaddr **, struct sockaddr *); -u_char etherbroadcastaddr[6] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff }; +u_char etherbroadcastaddr[ETHER_ADDR_LEN] = +{ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff }; #define senderr(e) do { error = (e); goto bad;} while (0) #define IFP2AC(IFP) ((struct arpcom *)IFP) @@ -149,7 +150,7 @@ { short type; int error = 0, hdrcmplt = 0; - u_char esrc[6], edst[6]; + u_char esrc[ETHER_ADDR_LEN], edst[ETHER_ADDR_LEN]; register struct rtentry *rt; register struct ether_header *eh; int loop_copy = 0; @@ -859,8 +860,8 @@ register struct sockaddr_dl *sdl; ifp-if_type = IFT_ETHER; - ifp-if_addrlen = 6; - ifp-if_hdrlen = 14; + ifp-if_addrlen = ETHER_ADDR_LEN; + ifp-if_hdrlen = ETHER_HDR_LEN; if_attach(ifp); ifp-if_mtu = ETHERMTU; ifp-if_resolvemulti = ether_resolvemulti; To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-net in the body of the message To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-net in the body of the message
Re: Comments Please
In message: [EMAIL PROTECTED] Luigi Rizzo [EMAIL PROTECTED] writes: : On Sat, Oct 12, 2002 at 05:18:09PM -0600, M. Warner Losh wrote: : OK. I'm not a network wonk, so I thought I'd run this by people : here. What do people think. : : sounds ok -- removing explicit constants is always good. : On passing: : : * While you are at it, : grep etherbroadcastaddr sys/net*/* : reveals the use of an explicit constant (6) in net/if_arp.h and : netinet/if_ether.c; there is more of the same in net/bridge.c : (my fault), net/if_atmsubr.c, netinet/if_ether.c, netncp/ncp_subr.c atmsubr? Doesn't ATM have its own constants? : * there is no real reason to have etherbroadcastaddr as a : variable. net/bridge.c has a macro, IS_ETHER_BROADCAST, : which is much faster to evaluate on i386, and : could be moved e.g. in net/ethernet.h and be used : to check for ethernet broadcast addresses in : net/if_ethersubr.c : net/if_iso88025subr.c : netatalk/aarp.c : net/if_fddisubr.c : This only leaves some usages of etherbroadcastaddr is in : netinet/if_ether.c to set the address for outgoing broadcast : packets. I'll let others deal with that. Warner To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-net in the body of the message
Re: Comments Please
On Sat, Oct 12, 2002 at 08:07:47PM -0600, M. Warner Losh wrote: ... : reveals the use of an explicit constant (6) in net/if_arp.h and : netinet/if_ether.c; there is more of the same in net/bridge.c : (my fault), net/if_atmsubr.c, netinet/if_ether.c, netncp/ncp_subr.c atmsubr? Doesn't ATM have its own constants? eh, that's the problem with explicit constants, you can never tell whether 6 is english, german or italian... in any case the relevant piece of code is: net/if_atmsubr.c: if (bcmp(alc, ATMLLC_HDR, 6)) { I have no idea if it has any relation with ethernet header sizes. cheers luigi To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-net in the body of the message
Re: Comments Please
In message: [EMAIL PROTECTED] Luigi Rizzo [EMAIL PROTECTED] writes: : On Sat, Oct 12, 2002 at 08:07:47PM -0600, M. Warner Losh wrote: : ... : : reveals the use of an explicit constant (6) in net/if_arp.h and : : netinet/if_ether.c; there is more of the same in net/bridge.c : : (my fault), net/if_atmsubr.c, netinet/if_ether.c, netncp/ncp_subr.c : : atmsubr? Doesn't ATM have its own constants? : : eh, that's the problem with explicit constants, you can never tell : whether 6 is english, german or italian... in any case the : relevant piece of code is: : : net/if_atmsubr.c: if (bcmp(alc, ATMLLC_HDR, 6)) { : : I have no idea if it has any relation with ethernet header sizes. Looks like that one should be something else, since it is an atm llc header. Of course the atm code should be using if_llc.h to get this stuff, but that's another story... Warner To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-net in the body of the message