DLPI again
Joost Mulders
mail at j-mulders.demon.nl
Sun Dec 9 10:00:37 UTC 2001
Hi All,
In April this year, I send in a patch for DLPI (subject "more dlpi"). Ted's reply to this was:
> Thanks for the patch! I'm too paranoid to put it in before 3.0
> ships, but it will go in for 3.1, which should take less time than 3.0
> did. :'}
> _MelloN_
Well, the paranoiism is hereby justified. There is a bug in the patch that causes NAK's not to appear on the
wire. The patch did not make it to the release, so there's no problem.
The culprit in the patch was this piece from send_packet in the patch:
+ { /* encode DLSAP in dstaddr */
+ struct hardware *hwaddr;
! if (hto && hto -> hlen == interface -> hw_address.hlen) {
! hwaddr = hto;
}
else {
! hwaddr = &interface -> dlpi_broadcast_addr;
}
+ if (interface -> dlpi_sap_length < 0) {
+ memcpy (dstaddr, &hwaddr -> hbuf [1],
+ hwaddr -> hlen - 1);
+ memcpy (&dstaddr [hwaddr -> hlen - 1],
+ interface -> dlpi_sap,
+ ABS(interface -> dlpi_sap_length));
+ } else if (interface ->dlpi_sap_length > 0) {
+ memcpy (dstaddr, interface -> dlpi_sap,
+ interface -> dlpi_sap_length);
+ memcpy ((char*)&dstaddr [interface -> dlpi_sap_length],
+ &hwaddr -> hbuf [1], hwaddr -> hlen - 1);
+ }
+ addrlen = hwaddr -> hlen - 1 +
+ ABS (interface -> dlpi_sap_length);
+ } /* encode DLSAP */
+
Wen the server sends a NAK, hto is NULL assuming that the lower layer will provide a broadcast address. Thus
dlpi_broadcast_addr is used for the hardware address. Right. When assembling the dlpi destination address into
dstaddr, a memcpy is done from hwaddr->hbuf[1], thereby skipping the first byte of the dlpi broadcast address
and appending a zero byte at the end, resulting in an invalid ether address of ff:ff:ff:ff:ff:00, which caused
the NAK not to appear on the wire.
In other's words: Sigh!
Below is the story that was with the patch + a new diff against dhcp-3.0. I would be delighted if this diff
could make it to a release sometime. There's no hurry but I am bit ashamed by the original (pre-patch) code.
Joost.
--
> Hi all,
>
> Here's is a minor modification to dlpi.c.
>
> wanted to get rid of this ugly part of the code that was introduced
> in the previous patch:
>
> 579 /* sap = htons (ETHERTYPE_IP) kludge */
> 580 memset (sap, 0, sizeof (sap));
> 581 # if (BYTE_ORDER == LITTLE_ENDIAN)
> 582 sap [0] = 0x00;
> 583 sap [1] = 0x08;
> 584 # else
> 585 sap [0] = 0x08;
> 586 sap [1] = 0x00;
> 587 # endif
>
> The problem was that:
> (a) the sap value must be in network byte order and
> (b) the size of the sap value is implementation specific, so it is not
> allowed for ordinary people like me to say sap = htons (ETHERTYPE_IP),
> allthough -as far as I know- all ethernet dlpi modules use a short for the
> sap length.
>
> The le man page say's:
> ..Applications should not hardcode to this particular
> implementation-specific DLSAP address ...
>
>
> Luckily, there was a bit of an 'elegant' solution to this problem: when an info
> request is done on the descriptor after it is attached and bound, the dlpi API
> itself provides the sap value and -length as part of the response.
>
> The patch below does this info request and copies the sap value and -length to
> the interface info structure. When sending packets, the sap is copied from info
> to the packet-to-be-sent. So, we use the sap value that is provided by the api
> itself and that, therefore, cannot be wrong :}
>
> Furthermore, I added an ioctl that flushes the read descriptor right after the
> packetfilter is pushed. When starting the dhcp server, the UDP packets that
> came in between opening of the descriptor and pushing the packet filter, were
> reported as 'bogus'.
>
> The diff is made against dlpi.c from 3.0b2pl24 (v1.28). I discussed the sap
> issue with Ben Cottrell when I sent in the previous patch.
>
> I tested:
> 5.5.1 sparc
> 5.6 sparc
> 5.8 sparc
> 5.8 intel
>
*** common/dlpi.c.orig Sat Nov 10 00:26:10 2001
--- common/dlpi.c Sat Nov 10 00:22:54 2001
***************
*** 138,144 ****
static int dlpiopen PROTO ((char *ifname));
static int dlpiunit PROTO ((char *ifname));
static int dlpiinforeq PROTO ((int fd));
- static int dlpiphysaddrreq PROTO ((int fd, unsigned long addrtype));
static int dlpiattachreq PROTO ((int fd, unsigned long ppa));
static int dlpibindreq PROTO ((int fd, unsigned long sap, unsigned long max_conind,
unsigned long service_mode, unsigned long conn_mgmt,
--- 138,143 ----
***************
*** 147,153 ****
static int dlpiunbindreq PROTO ((int fd));
static int dlpiokack PROTO ((int fd, char *bufp));
static int dlpiinfoack PROTO ((int fd, char *bufp));
- static int dlpiphysaddrack PROTO ((int fd, char *bufp));
static int dlpibindack PROTO ((int fd, char *bufp));
static int dlpiunitdatareq PROTO ((int fd, unsigned char *addr,
int addrlen, unsigned long minpri,
--- 146,151 ----
***************
*** 198,207 ****
int sock;
int unit;
long buf [DLPI_MAXDLBUF];
! union DL_primitives *dlp;
! dlp = (union DL_primitives *)buf;
/* Open a DLPI device */
if ((sock = dlpiopen (info -> name)) < 0) {
log_fatal ("Can't open DLPI device for %s: %m", info -> name);
--- 196,206 ----
int sock;
int unit;
long buf [DLPI_MAXDLBUF];
! dl_info_ack_t *dlp;
! dlp = (dl_info_ack_t *)buf;
+
/* Open a DLPI device */
if ((sock = dlpiopen (info -> name)) < 0) {
log_fatal ("Can't open DLPI device for %s: %m", info -> name);
***************
*** 215,221 ****
if (dlpiinforeq(sock) < 0 || dlpiinfoack(sock, (char *)buf) < 0) {
log_fatal ("Can't get DLPI MAC type for %s: %m", info -> name);
} else {
! switch (dlp -> info_ack.dl_mac_type) {
case DL_CSMACD: /* IEEE 802.3 */
case DL_ETHER:
info -> hw_address.hbuf [0] = HTYPE_ETHER;
--- 214,220 ----
if (dlpiinforeq(sock) < 0 || dlpiinfoack(sock, (char *)buf) < 0) {
log_fatal ("Can't get DLPI MAC type for %s: %m", info -> name);
} else {
! switch (dlp -> dl_mac_type) {
case DL_CSMACD: /* IEEE 802.3 */
case DL_ETHER:
info -> hw_address.hbuf [0] = HTYPE_ETHER;
***************
*** 229,251 ****
break;
default:
log_fatal ("%s: unsupported DLPI MAC type %ld",
! info -> name, dlp -> info_ack.dl_mac_type);
break;
}
- /*
- * copy the sap length and broadcast address of this interface
- * to interface_info. This fixes nothing but seemed nicer than to
- * assume -2 and ffffff.
- */
- info -> dlpi_sap_length = dlp -> info_ack.dl_sap_length;
- info -> dlpi_broadcast_addr.hlen =
- dlp -> info_ack.dl_brdcst_addr_length;
- memcpy (info -> dlpi_broadcast_addr.hbuf,
- (char *)dlp + dlp -> info_ack.dl_brdcst_addr_offset,
- dlp -> info_ack.dl_brdcst_addr_length);
}
! if (dlp -> info_ack.dl_provider_style == DL_STYLE2) {
/*
* Attach to the device. If this fails, the device
* does not exist.
--- 228,239 ----
break;
default:
log_fatal ("%s: unsupported DLPI MAC type %ld",
! info -> name, dlp -> dl_mac_type);
break;
}
}
! if (dlp -> dl_provider_style == DL_STYLE2) {
/*
* Attach to the device. If this fails, the device
* does not exist.
***************
*** 267,286 ****
}
/*
! * Submit a DL_PHYS_ADDR_REQ request, to find
! * the hardware address
*/
! if (dlpiphysaddrreq (sock, DL_CURR_PHYS_ADDR) < 0
! || dlpiphysaddrack (sock, (char *)buf) < 0) {
! log_fatal ("Can't get DLPI hardware address for %s: %m",
! info -> name);
}
! info -> hw_address.hlen = dlp -> physaddr_ack.dl_addr_length + 1;
memcpy (&info -> hw_address.hbuf [1],
! (char *)buf + dlp -> physaddr_ack.dl_addr_offset,
! dlp -> physaddr_ack.dl_addr_length);
#ifdef USE_DLPI_RAW
if (strioctl (sock, DLIOCRAW, INFTIM, 0, 0) < 0) {
log_fatal ("Can't set DLPI RAW mode for %s: %m",
--- 255,323 ----
}
/*
! * Submit a DL_INFO_REQ once more after attaching and binding.
! * Then copy
! * hardware address and -length
! * broadcast address and -length
! * sap and -length
! * to the info structure for usage when sending packets.
! *
! * Copying this values here from the ack_t structure prevents us
! * for the need to guess values required for sending packets with
! * dlpi, especially the architecture dependent SAP value.
*/
! if (dlpiinforeq(sock) < 0 || dlpiinfoack(sock, (char *)buf) < 0) {
! log_fatal ("DL_INFO_REQ failed for %s: %m", info -> name);
}
! /* hardware address length */
! info -> hw_address.hlen = dlp -> dl_addr_length -
! ABS (dlp -> dl_sap_length) + 1;
!
! /* sap length */
! info -> dlpi_sap_length = dlp -> dl_sap_length;
!
!
! /*
! * the hardware address and sap together are named DLSAP which is
! * stored at dl_addr_offset with a total length of dl_addr_length.
! * If the dl_sap_length value is < 0, DLSAP is stored as
! * [hardware address] [sap]
! * If dl_sap_length > 0, DLSAP is stored as
! * [sap] [hardware address].
! */
! if (dlp -> dl_sap_length < 0) {
! /* hardware address */
memcpy (&info -> hw_address.hbuf [1],
! (char *) dlp + dlp -> dl_addr_offset,
! info -> hw_address.hlen - 1);
+ /* sap */
+ memcpy (info -> dlpi_sap,
+ (char *) dlp + dlp -> dl_addr_offset + dlp -> dl_addr_length
+ + dlp -> dl_sap_length,
+ ABS (dlp -> dl_sap_length));
+ } else if (dlp -> dl_sap_length > 0) {
+ memcpy (&info -> hw_address.hbuf [1],
+ /* hardware address */
+ (char *) dlp + dlp -> dl_addr_offset + dlp -> dl_sap_length,
+ info -> hw_address.hlen - 1);
+
+ /* sap */
+ memcpy (info -> dlpi_sap,
+ (char *) dlp + dlp -> dl_addr_offset,
+ dlp -> dl_sap_length);
+ }
+
+ /* broadcast address */
+ memcpy (info -> dlpi_broadcast_addr.hbuf,
+ (char *)dlp + dlp -> dl_brdcst_addr_offset,
+ dlp -> dl_brdcst_addr_length);
+
+ /* broadcast address length */
+ info -> dlpi_broadcast_addr.hlen =
+ dlp -> dl_brdcst_addr_length;
+
#ifdef USE_DLPI_RAW
if (strioctl (sock, DLIOCRAW, INFTIM, 0, 0) < 0) {
log_fatal ("Can't set DLPI RAW mode for %s: %m",
***************
*** 456,462 ****
* protocol should be udp. this is a byte compare, test for
* endianess.
*/
! offset = ETHER_H_PREFIX + ((u_int8_t *)&(iphdr.ip_p) - (u_int8_t *)&iphdr);
pf.Pf_Filter [pf.Pf_FilterLen++] = ENF_PUSHWORD + (offset / 2);
pf.Pf_Filter [pf.Pf_FilterLen++] = ENF_PUSHLIT | ENF_AND;
pf.Pf_Filter [pf.Pf_FilterLen++] = htons (0x00FF);
--- 493,500 ----
* protocol should be udp. this is a byte compare, test for
* endianess.
*/
! offset = ETHER_H_PREFIX + ((u_int8_t *)&(iphdr.ip_p) -
! (u_int8_t *)&iphdr);
pf.Pf_Filter [pf.Pf_FilterLen++] = ENF_PUSHWORD + (offset / 2);
pf.Pf_Filter [pf.Pf_FilterLen++] = ENF_PUSHLIT | ENF_AND;
pf.Pf_Filter [pf.Pf_FilterLen++] = htons (0x00FF);
***************
*** 466,473 ****
/* Install the filter... */
if (strioctl (info -> rfdesc, PFIOCSETF, INFTIM,
sizeof (pf), (char *)&pf) < 0) {
! log_fatal ("Can't set PFMOD receive filter on %s: %m", info -> name);
}
#endif /* USE_DLPI_PFMOD */
if (!quiet_interface_discovery)
--- 504,513 ----
/* Install the filter... */
if (strioctl (info -> rfdesc, PFIOCSETF, INFTIM,
sizeof (pf), (char *)&pf) < 0) {
! log_fatal ("Can't set PFMOD receive filter on %s: %m",
! info -> name);
}
+ ioctl(info -> rfdesc, I_FLUSH, FLUSHR);
#endif /* USE_DLPI_PFMOD */
if (!quiet_interface_discovery)
***************
*** 560,608 ****
#ifdef USE_DLPI_RAW
result = write (interface -> wfdesc, dbuf + fudge, dbuflen - fudge);
#else
! /*
! * Setup the destination address (DLSAP) in dstaddr
! *
! * If sap_length < 0 we must deliver the DLSAP as phys+sap.
! * If sap_length > 0 we must deliver the DLSAP as sap+phys.
! *
! * sap = Service Access Point == ETHERTYPE_IP
! * sap + datalink address is called DLSAP in dlpi speak.
! */
! { /* ENCODE DLSAP */
! unsigned char phys [DLPI_MAXDLADDR];
! unsigned char sap [4];
! int sap_len = interface -> dlpi_sap_length;
! int phys_len = interface -> hw_address.hlen - 1;
!
! /* sap = htons (ETHERTYPE_IP) kludge */
! memset (sap, 0, sizeof (sap));
! # if (BYTE_ORDER == LITTLE_ENDIAN)
! sap [0] = 0x00;
! sap [1] = 0x08;
! # else
! sap [0] = 0x08;
! sap [1] = 0x00;
! # endif
!
! if (hto && hto -> hlen == interface -> hw_address.hlen)
! memcpy ( phys, (char *) &hto -> hbuf [1], phys_len);
! else
! memcpy ( phys, interface -> dlpi_broadcast_addr.hbuf,
! interface -> dlpi_broadcast_addr.hlen);
!
! if (sap_len < 0) {
! memcpy ( dstaddr, phys, phys_len);
! memcpy ( (char *) &dstaddr [phys_len], sap, ABS (sap_len));
}
else {
! memcpy ( dstaddr, (void *) sap, sap_len);
! memcpy ( (char *) &dstaddr [sap_len], phys, phys_len);
}
- addrlen = phys_len + ABS (sap_len);
- } /* ENCODE DLSAP */
result = dlpiunitdatareq (interface -> wfdesc, dstaddr, addrlen,
0, 0, dbuf, dbuflen);
#endif /* USE_DLPI_RAW */
--- 600,631 ----
#ifdef USE_DLPI_RAW
result = write (interface -> wfdesc, dbuf + fudge, dbuflen - fudge);
#else
+ { /* encode DLSAP in dstaddr */
+ u_int8_t *hwaddr;
+ u_int8_t hwlen;
! if (hto && hto -> hlen == interface -> hw_address.hlen) {
! hwaddr = &hto -> hbuf [1];
! hwlen = hto -> hlen - 1;
}
else {
! hwaddr = &interface -> dlpi_broadcast_addr.hbuf [0];
! hwlen = interface -> dlpi_broadcast_addr.hlen;
}
+ if (interface -> dlpi_sap_length < 0) {
+ memcpy (dstaddr, hwaddr, hwlen);
+ memcpy (&dstaddr [hwlen], interface -> dlpi_sap,
+ ABS(interface -> dlpi_sap_length));
+ } else if (interface ->dlpi_sap_length > 0) {
+ memcpy (dstaddr, interface -> dlpi_sap,
+ interface -> dlpi_sap_length);
+ memcpy ((char*) &dstaddr [interface -> dlpi_sap_length],
+ hwaddr, hwlen);
+ }
+ addrlen = hwlen + ABS (interface -> dlpi_sap_length);
+ } /* encode DLSAP */
+
result = dlpiunitdatareq (interface -> wfdesc, dstaddr, addrlen,
0, 0, dbuf, dbuflen);
#endif /* USE_DLPI_RAW */
***************
*** 642,654 ****
}
# if !defined (USE_DLPI_RAW)
- /*
- * Copy the sender's hw address into hfrom
- * If sap_len < 0 the DLSAP is as phys+sap.
- * If sap_len > 0 the DLSAP is as sap+phys.
- *
- * sap is discarded here.
- */
{ /* DECODE DLSAP */
int sap_len = interface -> dlpi_sap_length;
int phys_len = interface -> hw_address.hlen - 1;
--- 665,670 ----
***************
*** 817,845 ****
return putmsg (fd, &ctl, (struct strbuf *)NULL, flags);
}
- /*
- * dlpiphysaddrreq - request the current physical address.
- */
- static int dlpiphysaddrreq (fd, addrtype)
- int fd;
- unsigned long addrtype;
- {
- dl_phys_addr_req_t physaddr_req;
- struct strbuf ctl;
- int flags;
- physaddr_req.dl_primitive = DL_PHYS_ADDR_REQ;
- physaddr_req.dl_addr_type = addrtype;
-
- ctl.maxlen = 0;
- ctl.len = sizeof (physaddr_req);
- ctl.buf = (char *)&physaddr_req;
-
- flags = RS_HIPRI;
-
- return putmsg (fd, &ctl, (struct strbuf *)NULL, flags);
- }
-
/*
* dlpiattachreq - send a request to attach to a specific unit.
*/
--- 833,839 ----
***************
*** 1040,1079 ****
return 0;
}
- /*
- * dlpiphysaddrack - receive an ack to a dlpiphysaddrreq.
- */
- int dlpiphysaddrack (fd, bufp)
- char *bufp;
- int fd;
- {
- union DL_primitives *dlp;
- struct strbuf ctl;
- int flags;
- ctl.maxlen = DLPI_MAXDLBUF;
- ctl.len = 0;
- ctl.buf = bufp;
-
- if (strgetmsg (fd, &ctl, (struct strbuf *)NULL, &flags,
- "dlpiphysaddrack") < 0) {
- return -1;
- }
-
- dlp = (union DL_primitives *)ctl.buf;
-
- if (!expected (DL_PHYS_ADDR_ACK, dlp, flags) < 0) {
- return -1;
- }
-
- if (ctl.len < sizeof (dl_phys_addr_ack_t)) {
- /* Returned structure is too short */
- return -1;
- }
-
- return 0;
- }
-
int dlpiunitdatareq (fd, addr, addrlen, minpri, maxpri, dbuf, dbuflen)
int fd;
unsigned char *addr;
--- 1034,1040 ----
*** includes/dhcpd.h.orig Sat Nov 10 00:34:38 2001
--- includes/dhcpd.h Sat Nov 10 00:43:19 2001
***************
*** 778,785 ****
/* Only used by DHCP client code. */
struct client_state *client;
# if defined (USE_DLPI_SEND) || defined (USE_DLPI_RECEIVE)
- int dlpi_sap_length;
struct hardware dlpi_broadcast_addr;
# endif /* DLPI_SEND || DLPI_RECEIVE */
};
--- 778,786 ----
/* Only used by DHCP client code. */
struct client_state *client;
# if defined (USE_DLPI_SEND) || defined (USE_DLPI_RECEIVE)
struct hardware dlpi_broadcast_addr;
+ int8_t dlpi_sap_length; /* can be negative */
+ u_int8_t dlpi_sap [17]; /* as in struct hardware */
# endif /* DLPI_SEND || DLPI_RECEIVE */
};
More information about the dhcp-hackers
mailing list