Re: SIOCADDMULTI doesn't work, proposed fix
On Mon, 1 Feb 1999 02:02:48 -0500 (EST), Bill Paul wp...@skynet.ctr.columbia.edu said: to compare 0 bytes worth of data and returns success). In my opinion, this is a bug: either the equal() macro should return false, or the zero length field should be detected by a sanity check and the function should return EINVAL. OK, I'll agree that this is a bug. -GAWollman -- Garrett A. Wollman | O Siem / We are all family / O Siem / We're all the same woll...@lcs.mit.edu | O Siem / The fires of freedom Opinions not those of| Dance in the burning flame MIT, LCS, CRS, or NSA| - Susan Aglukark and Chad Irschick To Unsubscribe: send mail to majord...@freebsd.org with unsubscribe freebsd-current in the body of the message
SIOCADDMULTI doesn't work, proposed fix
After experimenting some more, I've come to the conclusion that trying to
manually add a non-IP ethernet multicast address doesn't work properly.
The ether_resolvemulti() assumes that addresses will be specified as
either AF_LINK or AF_INET; if the family is AF_LINK, it assumes that
a struct sockaddr_dl will be used. However, the user is supposed to
pass the address using a struct ifreq, and struct ifreq uses struct
sockaddr, not struct sockaddr_dl.
The original code in 2.2.x expected a struct sockaddr with a family
of AF_UNSPEC. This no longer works in 3.0, which breaks compatibility.
Among other things, the Columbia Appletalk port doesn't work because
of this.
As an aside, the equal() macro in /sys/net/if.c does a bcmp() using
sa_len as the length of the data to check, but doesn't account for
the possibility of sa_len being 0 (this makes it always return true,
which can yield false positives).
The patches included with this post change /sys/net/if.c and
/sys/net/if_ethersubr.c so that adding a mutlicast address with
SIOCADDMULTI using a struct sockaddr and AF_UNSPEC works again. I would
like Those Who Know More Than I (tm) to review these changes and offer
criticisms and comments.
These patches are against 3.0-RELEASE but should apply to -current
and -stable as well.
-Bill
--
=
-Bill Paul(212) 854-6020 | System Manager, Master of Unix-Fu
Work: wp...@ctr.columbia.edu | Center for Telecommunications Research
Home: wp...@skynet.ctr.columbia.edu | Columbia University, New York City
=
It is not I who am crazy; it is I who am mad! - Ren Hoek, Space Madness
=
*** if.c.orig Sun Jan 31 17:13:01 1999
--- if.cSun Jan 31 17:10:36 1999
***
*** 186,192
register struct ifaddr *ifa;
#define equal(a1, a2) \
! (bcmp((caddr_t)(a1), (caddr_t)(a2), ((struct sockaddr *)(a1))-sa_len) == 0)
for (ifp = ifnet.tqh_first; ifp; ifp = ifp-if_link.tqe_next)
for (ifa = ifp-if_addrhead.tqh_first; ifa;
ifa = ifa-ifa_link.tqe_next) {
--- 186,193
register struct ifaddr *ifa;
#define equal(a1, a2) \
! (((struct sockaddr *)(a1))-sa_len \
!bcmp((caddr_t)(a1), (caddr_t)(a2), ((struct sockaddr *)(a1))-sa_len) == 0)
for (ifp = ifnet.tqh_first; ifp; ifp = ifp-if_link.tqe_next)
for (ifa = ifp-if_addrhead.tqh_first; ifa;
ifa = ifa-ifa_link.tqe_next) {
***
*** 636,642
return EOPNOTSUPP;
/* Don't let users screw up protocols' entries. */
! if (ifr-ifr_addr.sa_family != AF_LINK)
return EINVAL;
if (cmd == SIOCADDMULTI) {
--- 637,644
return EOPNOTSUPP;
/* Don't let users screw up protocols' entries. */
! if (ifr-ifr_addr.sa_family != AF_LINK
! ifr-ifr_addr.sa_family != AF_UNSPEC)
return EINVAL;
if (cmd == SIOCADDMULTI) {
*** if_ethersubr.c.orig Sun Jan 31 17:13:07 1999
--- if_ethersubr.c Sun Jan 31 17:00:54 1999
***
*** 778,783
--- 778,800
u_char *e_addr;
switch(sa-sa_family) {
+ case AF_UNSPEC:
+ e_addr = (u_char *)sa-sa_data;
+ if ((e_addr[0] 1) != 1)
+ return EADDRNOTAVAIL;
+ MALLOC(sdl, struct sockaddr_dl *, sizeof *sdl, M_IFMADDR,
+ M_WAITOK);
+ sdl-sdl_len = sizeof *sdl;
+ sdl-sdl_family = AF_LINK;
+ sdl-sdl_index = ifp-if_index;
+ sdl-sdl_type = IFT_ETHER;
+ sdl-sdl_nlen = 0;
+ sdl-sdl_alen = ETHER_ADDR_LEN;
+ sdl-sdl_slen = 0;
+ e_addr = LLADDR(sdl);
+ bcopy((char *)sa-sa_data, (char *)e_addr, ETHER_ADDR_LEN);
+ *llsa = (struct sockaddr *)sdl;
+ return 0;
case AF_LINK:
/*
* No mapping needed. Just check that it's a valid MC address.
To Unsubscribe: send mail to majord...@freebsd.org
with unsubscribe freebsd-current in the body of the message
SIOCADDMULTI doesn't work, proposed fix
On Sun, 31 Jan 1999 17:55:22 -0500 (EST), Bill Paul wp...@skynet.ctr.columbia.edu said: a struct sockaddr_dl will be used. However, the user is supposed to pass the address using a struct ifreq, and struct ifreq uses struct sockaddr, not struct sockaddr_dl. This is called ``poor man's inheritance''. I believe it is an error for any code to use AF_UNSPEC for any purpose other than masks (where it makes sense since the address family is normally not included in the mask). A `sockaddr_dl', while by default longer than a `sockaddr', in this case will fit withing the structure allotted. In the future, I fully expect that `sockaddr' will be of maximal length (we need this for IPv6). The patches included with this post change /sys/net/if.c and /sys/net/if_ethersubr.c so that adding a mutlicast address with SIOCADDMULTI using a struct sockaddr and AF_UNSPEC works again. I would like Those Who Know More Than I (tm) to review these changes and offer criticisms and comments. There are two things which should be done here. First, the kernel AppleTalk code should be fixed to join the necessary multicast groups when an interface is first configured for AppleTalk. (By preference the AARP implementation should be entirely in the kernel as well, but that's more of a challenge.) Second, the generic ether_resolvemulti function should be enhanced to know about AppleTalk multicast addresses. -GAWollman -- Garrett A. Wollman | O Siem / We are all family / O Siem / We're all the same woll...@lcs.mit.edu | O Siem / The fires of freedom Opinions not those of| Dance in the burning flame MIT, LCS, CRS, or NSA| - Susan Aglukark and Chad Irschick To Unsubscribe: send mail to majord...@freebsd.org with unsubscribe freebsd-current in the body of the message
Re: SIOCADDMULTI doesn't work, proposed fix
Of all the gin joints in all the towns in all the world, Garrett Wollman had to walk into mine and say: On Sun, 31 Jan 1999 17:55:22 -0500 (EST), Bill Paul wp...@skynet.ctr.columbia.edu said: a struct sockaddr_dl will be used. However, the user is supposed to pass the address using a struct ifreq, and struct ifreq uses struct sockaddr, not struct sockaddr_dl. This is called ``poor man's inheritance''. I believe it is an error for any code to use AF_UNSPEC for any purpose other than masks (where it makes sense since the address family is normally not included in the mask). A `sockaddr_dl', while by default longer than a `sockaddr', in this case will fit withing the structure allotted. In the future, I fully expect that `sockaddr' will be of maximal length (we need this for IPv6). There's still one small problem: the code as it stands now can return success and still not update the multicast filter. If you pass a structure with AF_LINK as the family but with the length set to 0, if_addmulti() falsely detects that the entry already matches an existing one and returns success (it the equal() macro equates to a bcmp(), which tries to compare 0 bytes worth of data and returns success). In my opinion, this is a bug: either the equal() macro should return false, or the zero length field should be detected by a sanity check and the function should return EINVAL. The patches included with this post change /sys/net/if.c and /sys/net/if_ethersubr.c so that adding a mutlicast address with SIOCADDMULTI using a struct sockaddr and AF_UNSPEC works again. I would like Those Who Know More Than I (tm) to review these changes and offer criticisms and comments. There are two things which should be done here. First, the kernel AppleTalk code should be fixed to join the necessary multicast groups when an interface is first configured for AppleTalk. (By preference the AARP implementation should be entirely in the kernel as well, but that's more of a challenge.) Second, the generic ether_resolvemulti function should be enhanced to know about AppleTalk multicast addresses. The Columbia Appletalk code is not the same as netatalk: it's implemented entirely in user space and uses BPF as well as manually joining multicast groups. The existing Columbia Appletalk port, which works on 2.2.x, uses SIOCADDMULTI with a family of AF_UNSPEC. I rifled through a bunch of man pages in 3.0-RELEASE trying to find the Right Way To Do This (tm) but came up empty. If the right way to do this is to cast the struct sockaddr to a struct sockaddr_dl and use AF_LINK, then this should be documented somewhere. (If it is documented and I missed it, feel free to slap me around and point me in the right direction.) -Bill -- = -Bill Paul(212) 854-6020 | System Manager, Master of Unix-Fu Work: wp...@ctr.columbia.edu | Center for Telecommunications Research Home: wp...@skynet.ctr.columbia.edu | Columbia University, New York City = It is not I who am crazy; it is I who am mad! - Ren Hoek, Space Madness = To Unsubscribe: send mail to majord...@freebsd.org with unsubscribe freebsd-current in the body of the message
