Hi Sergey,
On Tue, Mar 31, 2015 at 12:17 PM, Sergey Temerkhanov < s.temerkhanov@gmail.com> wrote:
This commit fixes incorrect DHCP/BOOTP packet layout caused by 'ulong' type size difference on 64 and 32-bit architectures. It also converts protocol header structures to use explicitly sized fields and renames NetReadLong()/NetCopyLong() to NetReadU32/NetCopyU32() accordingly
Signed-off-by: Radha Mohan Chintakuntla rchintakuntla@cavium.com Signed-off-by: Sergey Temerkhanov s.temerkhanov@gmail.com
Cc: joe.hershberger@gmail.com
include/net.h | 122
+++++++++++++++++++++++++++++-----------------------------
net/bootp.c | 24 ++++++------ net/bootp.h | 24 ++++++------ 3 files changed, 85 insertions(+), 85 deletions(-)
diff --git a/include/net.h b/include/net.h index 237c932..588ab7d 100644 --- a/include/net.h +++ b/include/net.h @@ -183,9 +183,9 @@ u32 ether_crc(size_t len, unsigned char const *p); */
struct ethernet_hdr {
uchar et_dest[6]; /* Destination node */uchar et_src[6]; /* Source node */ushort et_protlen; /* Protocol or length */
u8 et_dest[6]; /* Destination node */u8 et_src[6]; /* Source node */u16 et_protlen; /* Protocol or length */};
/* Ethernet header size */ @@ -194,16 +194,16 @@ struct ethernet_hdr { #define ETH_FCS_LEN 4 /* Octets in the FCS */
struct e802_hdr {
uchar et_dest[6]; /* Destination node */uchar et_src[6]; /* Source node */ushort et_protlen; /* Protocol or length */uchar et_dsap; /* 802 DSAP */uchar et_ssap; /* 802 SSAP */uchar et_ctl; /* 802 control */uchar et_snap1; /* SNAP */uchar et_snap2;uchar et_snap3;ushort et_prot; /* 802 protocol */
u8 et_dest[6]; /* Destination node */u8 et_src[6]; /* Source node */u16 et_protlen; /* Protocol or length */u8 et_dsap; /* 802 DSAP */u8 et_ssap; /* 802 SSAP */u8 et_ctl; /* 802 control */u8 et_snap1; /* SNAP */u8 et_snap2;u8 et_snap3;u16 et_prot; /* 802 protocol */};
/* 802 + SNAP + ethernet header size */ @@ -213,11 +213,11 @@ struct e802_hdr {
Virtual LAN Ethernet header*/ struct vlan_ethernet_hdr {
uchar vet_dest[6]; /* Destination node */uchar vet_src[6]; /* Source node */ushort vet_vlan_type; /* PROT_VLAN */ushort vet_tag; /* TAG of VLAN */ushort vet_type; /* protocol type */
u8 vet_dest[6]; /* Destination node */u8 vet_src[6]; /* Source node */u16 vet_vlan_type; /* PROT_VLAN */u16 vet_tag; /* TAG of VLAN */u16 vet_type; /* protocol type */};
/* VLAN Ethernet header size */ @@ -235,14 +235,14 @@ struct vlan_ethernet_hdr {
Internet Protocol (IP) header.*/ struct ip_hdr {
uchar ip_hl_v; /* header length and version */uchar ip_tos; /* type of service */ushort ip_len; /* total length */ushort ip_id; /* identification */ushort ip_off; /* fragment offset field */uchar ip_ttl; /* time to live */uchar ip_p; /* protocol */ushort ip_sum; /* checksum */
u8 ip_hl_v; /* header length and version */u8 ip_tos; /* type of service */u16 ip_len; /* total length */u16 ip_id; /* identification */u16 ip_off; /* fragment offset field */u8 ip_ttl; /* time to live */u8 ip_p; /* protocol */u16 ip_sum; /* checksum */ IPaddr_t ip_src; /* Source IP address */ IPaddr_t ip_dst; /* Destination IP address */}; @@ -259,20 +259,20 @@ struct ip_hdr {
Internet Protocol (IP) + UDP header.*/ struct ip_udp_hdr {
uchar ip_hl_v; /* header length and version */uchar ip_tos; /* type of service */ushort ip_len; /* total length */ushort ip_id; /* identification */ushort ip_off; /* fragment offset field */uchar ip_ttl; /* time to live */uchar ip_p; /* protocol */ushort ip_sum; /* checksum */
u8 ip_hl_v; /* header length and version */u8 ip_tos; /* type of service */u16 ip_len; /* total length */u16 ip_id; /* identification */u16 ip_off; /* fragment offset field */u8 ip_ttl; /* time to live */u8 ip_p; /* protocol */u16 ip_sum; /* checksum */ IPaddr_t ip_src; /* Source IP address */ IPaddr_t ip_dst; /* Destination IP address */
ushort udp_src; /* UDP source port */ushort udp_dst; /* UDP destination port */ushort udp_len; /* Length of UDP packet */ushort udp_xsum; /* Checksum */
u16 udp_src; /* UDP source port */u16 udp_dst; /* UDP destination port */u16 udp_len; /* Length of UDP packet */u16 udp_xsum; /* Checksum */};
#define IP_UDP_HDR_SIZE (sizeof(struct ip_udp_hdr)) @@ -282,14 +282,14 @@ struct ip_udp_hdr {
Address Resolution Protocol (ARP) header.*/ struct arp_hdr {
ushort ar_hrd; /* Format of hardware address */
u16 ar_hrd; /* Format of hardware address */# define ARP_ETHER 1 /* Ethernet hardware address */
ushort ar_pro; /* Format of protocol address */uchar ar_hln; /* Length of hardware address */
u16 ar_pro; /* Format of protocol address */u8 ar_hln; /* Length of hardware address */# define ARP_HLEN 6
uchar ar_pln; /* Length of protocol address */
u8 ar_pln; /* Length of protocol address */# define ARP_PLEN 4
ushort ar_op; /* Operation */
u16 ar_op; /* Operation */# define ARPOP_REQUEST 1 /* Request to resolve address */ # define ARPOP_REPLY 2 /* Response to previous request */
@@ -301,16 +301,16 @@ struct arp_hdr { * the sizes above, and are defined as appropriate for * specific hardware/protocol combinations. */
uchar ar_data[0];
u8 ar_data[0];#define ar_sha ar_data[0] #define ar_spa ar_data[ARP_HLEN] #define ar_tha ar_data[ARP_HLEN + ARP_PLEN] #define ar_tpa ar_data[ARP_HLEN + ARP_PLEN + ARP_HLEN] #if 0
uchar ar_sha[]; /* Sender hardware address */uchar ar_spa[]; /* Sender protocol address */uchar ar_tha[]; /* Target hardware address */uchar ar_tpa[]; /* Target protocol address */
u8 ar_sha[]; /* Sender hardware address */u8 ar_spa[]; /* Sender protocol address */u8 ar_tha[]; /* Target hardware address */u8 ar_tpa[]; /* Target protocol address */#endif /* 0 */ };
@@ -332,20 +332,20 @@ struct arp_hdr { #define ICMP_NOT_REACH_PORT 3 /* Port unreachable */
struct icmp_hdr {
uchar type;uchar code;ushort checksum;
u8 type;u8 code;u16 checksum; union { struct {
ushort id;ushort sequence;
u16 id;u16 sequence; } echo;
ulong gateway;
u32 gateway; struct {
ushort unused;ushort mtu;
u16 unused;u16 mtu; } frag;
uchar data[0];
u8 data[0]; } un;};
@@ -608,9 +608,9 @@ static inline IPaddr_t NetReadIP(void *from) }
/* return ulong *in network byteorder* */ -static inline ulong NetReadLong(ulong *from) +static inline u32 NetReadU32(u32 *from) {
ulong l;
u32 l; memcpy((void *)&l, (void *)from, sizeof(l)); return l;@@ -629,9 +629,9 @@ static inline void NetCopyIP(void *to, void *from) }
/* copy ulong */ -static inline void NetCopyLong(ulong *to, ulong *from) +static inline void NetCopyU32(u32 *to, u32 *from)
This surely causes a checkpatch.pl warning. While you are touching this, you should name it "net_copy_u32()".
{
memcpy((void *)to, (void *)from, sizeof(ulong));
memcpy((void *)to, (void *)from, sizeof(u32));}
/** diff --git a/net/bootp.c b/net/bootp.c index 8106601..8ce0bdd 100644 --- a/net/bootp.c +++ b/net/bootp.c @@ -51,7 +51,7 @@ #define CONFIG_BOOTP_ID_CACHE_SIZE 4 #endif
-ulong bootp_ids[CONFIG_BOOTP_ID_CACHE_SIZE]; +u32 bootp_ids[CONFIG_BOOTP_ID_CACHE_SIZE]; unsigned int bootp_num_ids; int BootpTry; ulong bootp_start; @@ -59,7 +59,7 @@ ulong bootp_timeout;
#if defined(CONFIG_CMD_DHCP) static dhcp_state_t dhcp_state = INIT; -static unsigned long dhcp_leasetime; +static u32 dhcp_leasetime; static IPaddr_t NetDHCPServerIP; static void DhcpHandler(uchar *pkt, unsigned dest, IPaddr_t sip,
unsigned src,
unsigned len);@@ -125,7 +125,7 @@ static int BootpCheckPkt(uchar *pkt, unsigned dest,
unsigned src, unsigned len)
retval = -4; else if (bp->bp_hlen != HWL_ETHER) retval = -5;
else if (!bootp_match_id(NetReadLong((ulong *)&bp->bp_id)))
else if (!bootp_match_id(NetReadU32(&bp->bp_id))) retval = -6; debug("Filtering pkt = %d\n", retval);@@ -350,7 +350,7 @@ BootpHandler(uchar *pkt, unsigned dest, IPaddr_t sip,
unsigned src,
BootpCopyNetParams(bp); /* Store net parameters from
reply */
/* Retrieve extended information (we must parse the vendor area)
*/
if (NetReadLong((ulong *)&bp->bp_vend[0]) ==
htonl(BOOTP_VENDOR_MAGIC))
if (NetReadU32((u32 *)&bp->bp_vend[0]) ==
htonl(BOOTP_VENDOR_MAGIC))
BootpVendorProcess((uchar *)&bp->bp_vend[4], len); NetSetTimeout(0, (thand_f *)0);@@ -661,7 +661,7 @@ BootpRequest(void) #ifdef CONFIG_BOOTP_RANDOM_DELAY ulong rand_ms; #endif
ulong BootpID;
u32 BootpID;
Please rename this to "bootp_id".
bootstage_mark_name(BOOTSTAGE_ID_BOOTP_START, "bootp_start");#if defined(CONFIG_CMD_DHCP) @@ -732,7 +732,7 @@ BootpRequest(void) BootpID += get_timer(0); BootpID = htonl(BootpID); bootp_add_id(BootpID);
NetCopyLong(&bp->bp_id, &BootpID);
NetCopyU32(&bp->bp_id, &BootpID); /* * Calculate proper packet lengths taking into account the@@ -770,7 +770,7 @@ static void DhcpOptionsProcess(uchar *popt, struct
Bootp_t *bp)
#if defined(CONFIG_CMD_SNTP) && defined(CONFIG_BOOTP_TIMEOFFSET) case 2: /* Time offset */ to_ptr = &NetTimeOffset;
NetCopyLong((ulong *)to_ptr, (ulong *)(popt + 2));
NetCopyU32((u32 *)to_ptr, (u32 *)(popt + 2)); NetTimeOffset = ntohl(NetTimeOffset); break;#endif @@ -806,7 +806,7 @@ static void DhcpOptionsProcess(uchar *popt, struct
Bootp_t *bp)
break;#endif case 51:
NetCopyLong(&dhcp_leasetime, (ulong *) (popt +
2));
NetCopyU32(&dhcp_leasetime, (u32 *) (popt + 2)); break; case 53: /* Ignore Message Type Option */ break;@@ -860,7 +860,7 @@ static void DhcpOptionsProcess(uchar *popt, struct
Bootp_t *bp)
static int DhcpMessageType(unsigned char *popt) {
if (NetReadLong((ulong *)popt) != htonl(BOOTP_VENDOR_MAGIC))
if (NetReadU32((u32 *)popt) != htonl(BOOTP_VENDOR_MAGIC)) return -1; popt += 4;@@ -911,7 +911,7 @@ static void DhcpSendRequestPkt(struct Bootp_t
*bp_offer)
* ID is the id of the OFFER packet */
NetCopyLong(&bp->bp_id, &bp_offer->bp_id);
NetCopyU32(&bp->bp_id, &bp_offer->bp_id); /* * Copy options from OFFER packet if present@@ -970,7 +970,7 @@ DhcpHandler(uchar *pkt, unsigned dest, IPaddr_t sip,
unsigned src,
debug("TRANSITIONING TO REQUESTING STATE\n"); dhcp_state = REQUESTING;
if (NetReadLong((ulong *)&bp->bp_vend[0]) ==
if (NetReadU32((u32 *)&bp->bp_vend[0]) == htonl(BOOTP_VENDOR_MAGIC)) DhcpOptionsProcess((u8 *)&bp->bp_vend[4],
bp);
@@ -986,7 +986,7 @@ DhcpHandler(uchar *pkt, unsigned dest, IPaddr_t sip,
unsigned src,
debug("DHCP State: REQUESTING\n"); if (DhcpMessageType((u8 *)bp->bp_vend) == DHCP_ACK) {
if (NetReadLong((ulong *)&bp->bp_vend[0]) ==
if (NetReadU32((u32 *)&bp->bp_vend[0]) == htonl(BOOTP_VENDOR_MAGIC)) DhcpOptionsProcess((u8 *)&bp->bp_vend[4],
bp);
/* Store net params from reply */diff --git a/net/bootp.h b/net/bootp.h index 3b95a0a..a771655 100644 --- a/net/bootp.h +++ b/net/bootp.h @@ -30,25 +30,25 @@ extern u8 *dhcp_vendorex_proc(u8 *e); /*rtn next e if
mine,else NULL */
#endif
struct Bootp_t {
uchar bp_op; /* Operation */
u8 bp_op; /* Operation */# define OP_BOOTREQUEST 1 # define OP_BOOTREPLY 2
uchar bp_htype; /* Hardware type */
u8 bp_htype; /* Hardware type */# define HWT_ETHER 1
uchar bp_hlen; /* Hardware address length */
u8 bp_hlen; /* Hardware address length */# define HWL_ETHER 6
uchar bp_hops; /* Hop count (gateway thing) */ulong bp_id; /* Transaction ID */ushort bp_secs; /* Seconds since boot */ushort bp_spare1; /* Alignment */
u8 bp_hops; /* Hop count (gateway thing) */u32 bp_id; /* Transaction ID */u16 bp_secs; /* Seconds since boot */u16 bp_spare1; /* Alignment */ IPaddr_t bp_ciaddr; /* Client IP address */ IPaddr_t bp_yiaddr; /* Your (client) IP address */ IPaddr_t bp_siaddr; /* Server IP address */ IPaddr_t bp_giaddr; /* Gateway IP address */
uchar bp_chaddr[16]; /* Client hardware address */char bp_sname[64]; /* Server host name */char bp_file[128]; /* Boot file name */char bp_vend[OPT_FIELD_SIZE]; /* Vendor information */
char bp_chaddr[16]; /* Client hardware address */char bp_sname[64]; /* Server host name */char bp_file[128]; /* Boot file name */char bp_vend[OPT_FIELD_SIZE]; /* Vendor information */};
#define BOOTP_HDR_SIZE sizeof(struct Bootp_t) @@ -59,7 +59,7 @@ struct Bootp_t { */
/* bootp.c */ -extern ulong BootpID; /* ID of cur BOOTP request */ +extern u32 BootpID; /* ID of cur BOOTP request */ extern char BootFile[128]; /* Boot file name */ extern int BootpTry;
I like what you're doing here, but I'm doing a lot of refactoring in this area (including most of this). I'm just going to pull this patch into the series I'm working on right now and make the changes I suggested.
I'll be sending a refactoring series out in the next few weeks that will include this.
Thanks, -Joe