Re: Comments Please

2002-10-12 Thread Luigi Rizzo
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

2002-10-12 Thread M. Warner Losh
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

2002-10-12 Thread Luigi Rizzo
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

2002-10-12 Thread M. Warner Losh
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