[Differential] D10485: Replace dhcp option 150 by 66

[rgrimes]

rgrimes added a comment.


  RFC1048 is obsoleted, first by 1533, then by 2132.   Please lets follow current RFC's when seeking information and refering to them. Refering to obsolete RFC's is going to lead to obsolete code.  I see at least in the comment of adding vend_end that you refer to RFC2132, it is unclear why you refered to 1048 in the description of the change.  RFC2132 names code 66 "TFTP server name".  I can not find in the RFC that option 150 is defined.  I do find from google searches that this was/is a cisco specific value:  "DHCP option 150 provides the IP addresses of a list of TFTP servers. • DHCP option 66 gives the IP address or the hostname of a single TFTP server. Note Cisco IP Phones might also include DHCP option 3 in their requests, which sets the default route. A single request might include both options 150 and 66."  This clarifies that it is possible to send both.
  
  The code change as it stands is a correction to a use of a non standard DHCP option and should move forward on that, it may be worth making a release notes: yes entry in updating saying we no longer support the cisco specific DHCP option 150, or to implement that option with proper processing (I dont think the current code would actually use a list but I did not verify that.)
  
  The old code processed this item as a list of ADDRESSES, which is what cisco says it was.  The new code also does this, but option 66 is a NAME, which can probably also contain a dotted quad address, but assuming it is a dotted quad address is a bug.

INLINE COMMENTS

> bootp.c:444
> +			val = strsep(&cp, VEND_INFO_END);
> +			if ((ipaddr = inet_addr(val)) != INADDR_NONE)
> +				tftpip.s_addr = ipaddr;

Here inet_addr(val) is assuming that val is a dotted quad, the specification does not say that.  It says Name.  Name in the spec usually means this can be a hostname or a dotted quad.

> bootp.h:97
>  #define TAG_INTF_MTU		((unsigned char)  26)
> +#define TAG_TFTP_SERVER		((unsigned char)  66)
> +

This should be TAG_TFTP_SERVER_NAME which is what the current RFC calls code 66, making it clearer that this is a name and not necessarily a dotted quad address.

REVISION DETAIL
  https://reviews.freebsd.org/D10485

EMAIL PREFERENCES
  https://reviews.freebsd.org/settings/panel/emailpreferences/

To: kczekirda, bapt, oshogbo, tsoome, sbruno, #network, freebsd-net-list, imp
Cc: rgrimes, garga, ler, asomers