Hi Joe,
On Thu, Jan 19, 2012 at 4:53 PM, Joe Hershberger joe.hershberger@ni.com wrote:
Rename parameter len to payload_len in NetSendUDPPacket: this name more explicitly claims that it does not include the header size Rename CDPHandler to CDPReceive: this is not called as a handler, so don't name it that way Rename OPT_SIZE to OPT_FIELD_SIZE: clearer constant name and also remove related BOOTP_SIZE which was unused and doesn't take into account VLAN packets Rename tmp to reply_ip_addr in arp.c Alphabetize includes in net.c Replace magic numbers in arp.c with constants Add a more explicit comment about 802.2
Yes but why lump all of these together? It would benefit from 3-4 separate commits IMO.
Regards, Simon
Signed-off-by: Joe Hershberger joe.hershberger@ni.com Cc: Joe Hershberger joe.hershberger@gmail.com Cc: Wolfgang Denk wd@denx.de
include/net.h | 11 +++++++- net/arp.c | 38 ++++++++++++++++---------------- net/bootp.c | 10 ++++---- net/bootp.h | 7 ++--- net/cdp.c | 2 +- net/cdp.h | 2 +- net/net.c | 67 +++++++++++++++++++++++++++++--------------------------- 7 files changed, 73 insertions(+), 64 deletions(-)
diff --git a/include/net.h b/include/net.h index 9de1181..add2080 100644 --- a/include/net.h +++ b/include/net.h @@ -169,7 +169,8 @@ struct Ethernet_t { };
#define ETHER_HDR_SIZE 14 /* Ethernet header size */ -#define E802_HDR_SIZE 22 /* 802 ethernet header size */
- /* 802.2 + SNAP + ethernet header size */
+#define E802_HDR_SIZE (ETHER_HDR_SIZE + 8)
/* * Ethernet header @@ -231,7 +232,9 @@ struct ARP_t { # define ARP_ETHER 1 /* Ethernet hardware address */ ushort ar_pro; /* Format of protocol address */ uchar ar_hln; /* Length of hardware address */ +# define ARP_HLEN 6 uchar ar_pln; /* Length of protocol address */ +# define ARP_PLEN 4 ushort ar_op; /* Operation */ # define ARPOP_REQUEST 1 /* Request to resolve address */ # define ARPOP_REPLY 2 /* Response to previous request */ @@ -245,6 +248,10 @@ struct ARP_t { * specific hardware/protocol combinations. */ uchar 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 */ @@ -431,7 +438,7 @@ extern void NetSendPacket(uchar *, int);
/* Transmit UDP packet, performing ARP request if needed */ extern int NetSendUDPPacket(uchar *ether, IPaddr_t dest, int dport,
- int sport, int len);
- int sport, int payload_len);
/* Processes a received packet */ extern void NetReceive(volatile uchar *, int); diff --git a/net/arp.c b/net/arp.c index 96ffb85..456decd 100644 --- a/net/arp.c +++ b/net/arp.c @@ -63,16 +63,16 @@ void ArpRequest(void)
arp->ar_hrd = htons(ARP_ETHER); arp->ar_pro = htons(PROT_IP);
- arp->ar_hln = 6;
- arp->ar_pln = 4;
- arp->ar_hln = ARP_HLEN;
- arp->ar_pln = ARP_PLEN;
arp->ar_op = htons(ARPOP_REQUEST);
/* source ET addr */
- memcpy(&arp->ar_data[0], NetOurEther, 6);
- memcpy(&arp->ar_sha, NetOurEther, ARP_HLEN);
/* source IP addr */
- NetWriteIP((uchar *) &arp->ar_data[6], NetOurIP);
- NetWriteIP(&arp->ar_spa, NetOurIP);
/* dest ET addr = 0 */
- memset(&arp->ar_data[10], '\0', 6);
- memset(&arp->ar_tha, 0, ARP_HLEN);
if ((NetArpWaitPacketIP & NetOurSubnetMask) != (NetOurIP & NetOurSubnetMask)) { if (NetOurGatewayIP == 0) { @@ -85,7 +85,7 @@ void ArpRequest(void) NetArpWaitReplyIP = NetArpWaitPacketIP; }
- NetWriteIP((uchar *) &arp->ar_data[16], NetArpWaitReplyIP);
- NetWriteIP(&arp->ar_tpa, NetArpWaitReplyIP);
(void) eth_send(NetTxPacket, (pkt - NetTxPacket) + ARP_HDR_SIZE); }
@@ -116,7 +116,7 @@ void ArpTimeoutCheck(void) void ArpReceive(struct Ethernet_t *et, struct IP_UDP_t *ip, int len) { struct ARP_t *arp;
- IPaddr_t tmp;
- IPaddr_t reply_ip_addr;
uchar *pkt;
/* @@ -139,15 +139,15 @@ void ArpReceive(struct Ethernet_t *et, struct IP_UDP_t *ip, int len) return; if (ntohs(arp->ar_pro) != PROT_IP) return;
- if (arp->ar_hln != 6)
- if (arp->ar_hln != ARP_HLEN)
return;
- if (arp->ar_pln != 4)
- if (arp->ar_pln != ARP_PLEN)
return;
if (NetOurIP == 0) return;
- if (NetReadIP(&arp->ar_data[16]) != NetOurIP)
- if (NetReadIP(&arp->ar_tpa) != NetOurIP)
return;
switch (ntohs(arp->ar_op)) { @@ -157,10 +157,10 @@ void ArpReceive(struct Ethernet_t *et, struct IP_UDP_t *ip, int len) pkt = (uchar *)et; pkt += NetSetEther(pkt, et->et_src, PROT_ARP); arp->ar_op = htons(ARPOP_REPLY);
- memcpy(&arp->ar_data[10], &arp->ar_data[0], 6);
- NetCopyIP(&arp->ar_data[16], &arp->ar_data[6]);
- memcpy(&arp->ar_data[0], NetOurEther, 6);
- NetCopyIP(&arp->ar_data[6], &NetOurIP);
- memcpy(&arp->ar_tha, &arp->ar_sha, ARP_HLEN);
- NetCopyIP(&arp->ar_tpa, &arp->ar_spa);
- memcpy(&arp->ar_sha, NetOurEther, ARP_HLEN);
- NetCopyIP(&arp->ar_spa, &NetOurIP);
(void) eth_send((uchar *)et, (pkt - (uchar *)et) + ARP_HDR_SIZE); return; @@ -173,28 +173,28 @@ void ArpReceive(struct Ethernet_t *et, struct IP_UDP_t *ip, int len) #ifdef CONFIG_KEEP_SERVERADDR if (NetServerIP == NetArpWaitPacketIP) { char buf[20];
- sprintf(buf, "%pM", arp->ar_data);
- sprintf(buf, "%pM", arp->ar_sha);
setenv("serveraddr", buf); } #endif
- tmp = NetReadIP(&arp->ar_data[6]);
- reply_ip_addr = NetReadIP(&arp->ar_spa);
/* matched waiting packet's address */
- if (tmp == NetArpWaitReplyIP) {
- if (reply_ip_addr == NetArpWaitReplyIP) {
debug("Got ARP REPLY, set eth addr (%pM)\n", arp->ar_data);
/* save address for later use */ memcpy(NetArpWaitPacketMAC,
- &arp->ar_data[0], 6);
- &arp->ar_sha, ARP_HLEN);
#ifdef CONFIG_NETCONSOLE NetGetHandler()(0, 0, 0, 0, 0); #endif /* modify header, and transmit it */ memcpy(((struct Ethernet_t *)NetArpWaitTxPacket)->
- et_dest, NetArpWaitPacketMAC, 6);
- et_dest, NetArpWaitPacketMAC, ARP_HLEN);
(void) eth_send(NetArpWaitTxPacket, NetArpWaitTxPacketSize);
diff --git a/net/bootp.c b/net/bootp.c index e95419f..2be8083 100644 --- a/net/bootp.c +++ b/net/bootp.c @@ -73,7 +73,7 @@ static int BootpCheckPkt(uchar *pkt, unsigned dest, unsigned src, unsigned len)
if (dest != PORT_BOOTPC || src != PORT_BOOTPS) retval = -1;
- else if (len < sizeof(struct Bootp_t) - OPT_SIZE)
- else if (len < sizeof(struct Bootp_t) - OPT_FIELD_SIZE)
retval = -2; else if (bp->bp_op != OP_BOOTREQUEST && bp->bp_op != OP_BOOTREPLY && @@ -369,8 +369,8 @@ static int DhcpExtended(u8 *e, int message_type, IPaddr_t ServerID,
*e++ = 57; /* Maximum DHCP Message Size */ *e++ = 2;
- *e++ = (576 - 312 + OPT_SIZE) >> 8;
- *e++ = (576 - 312 + OPT_SIZE) & 0xff;
- *e++ = (576 - 312 + OPT_FIELD_SIZE) >> 8;
- *e++ = (576 - 312 + OPT_FIELD_SIZE) & 0xff;
if (ServerID) { int tmp = ntohl(ServerID); @@ -520,8 +520,8 @@ static int BootpExtended(u8 *e)
*e++ = 57; /* Maximum DHCP Message Size */ *e++ = 2;
- *e++ = (576 - 312 + OPT_SIZE) >> 16;
- *e++ = (576 - 312 + OPT_SIZE) & 0xff;
- *e++ = (576 - 312 + OPT_FIELD_SIZE) >> 16;
- *e++ = (576 - 312 + OPT_FIELD_SIZE) & 0xff;
#endif
#if defined(CONFIG_BOOTP_SUBNETMASK) diff --git a/net/bootp.h b/net/bootp.h index 1cf9a02..ecbcc4d 100644 --- a/net/bootp.h +++ b/net/bootp.h @@ -20,13 +20,13 @@ */ #if defined(CONFIG_CMD_DHCP) /* Minimum DHCP Options size per RFC2131 - results in 576 byte pkt */ -#define OPT_SIZE 312 +#define OPT_FIELD_SIZE 312 #if defined(CONFIG_BOOTP_VENDOREX) extern u8 *dhcp_vendorex_prep(u8 *e); /*rtn new e after add own opts. */ extern u8 *dhcp_vendorex_proc(u8 *e); /*rtn next e if mine,else NULL */ #endif #else -#define OPT_SIZE 64 +#define OPT_FIELD_SIZE 64 #endif
struct Bootp_t { @@ -48,11 +48,10 @@ struct Bootp_t { 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_SIZE]; /* Vendor information */
- char bp_vend[OPT_FIELD_SIZE]; /* Vendor information */
};
#define BOOTP_HDR_SIZE sizeof(struct Bootp_t) -#define BOOTP_SIZE (ETHER_HDR_SIZE + IP_UDP_HDR_SIZE + BOOTP_HDR_SIZE)
/**********************************************************************/ /* diff --git a/net/cdp.c b/net/cdp.c index d617f18..38b79bd 100644 --- a/net/cdp.c +++ b/net/cdp.c @@ -245,7 +245,7 @@ CDPDummyHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src, }
void -CDPHandler(const uchar *pkt, unsigned len) +CDPReceive(const uchar *pkt, unsigned len) { const uchar *t; const ushort *ss; diff --git a/net/cdp.h b/net/cdp.h index fef744e..88191c4 100644 --- a/net/cdp.h +++ b/net/cdp.h @@ -12,7 +12,7 @@ #define __CDP_H__
void CDPStart(void); -void CDPHandler(const uchar *pkt, unsigned len); +void CDPReceive(const uchar *pkt, unsigned len);
#endif /* __CDP_H__ */
diff --git a/net/net.c b/net/net.c index 023802d..9bf74d5 100644 --- a/net/net.c +++ b/net/net.c @@ -75,32 +75,32 @@
#include <common.h> -#include <watchdog.h> #include <command.h> #include <net.h> -#include "arp.h" -#include "bootp.h" -#include "tftp.h" -#ifdef CONFIG_CMD_RARP -#include "rarp.h" -#endif -#include "nfs.h" -#ifdef CONFIG_STATUS_LED +#if defined(CONFIG_STATUS_LED) #include <status_led.h> #include <miiphy.h> #endif -#if defined(CONFIG_CMD_SNTP) -#include "sntp.h" -#endif +#include <watchdog.h> +#include "arp.h" +#include "bootp.h" #if defined(CONFIG_CMD_CDP) #include "cdp.h" #endif #if defined(CONFIG_CMD_DNS) #include "dns.h" #endif +#include "nfs.h" #if defined(CONFIG_CMD_PING) #include "ping.h" #endif +#if defined(CONFIG_CMD_RARP) +#include "rarp.h" +#endif +#if defined(CONFIG_CMD_SNTP) +#include "sntp.h" +#endif +#include "tftp.h"
DECLARE_GLOBAL_DATA_PTR;
@@ -598,7 +598,8 @@ NetSendPacket(uchar *pkt, int len) }
int -NetSendUDPPacket(uchar *ether, IPaddr_t dest, int dport, int sport, int len) +NetSendUDPPacket(uchar *ether, IPaddr_t dest, int dport, int sport,
- int payload_len)
{ uchar *pkt;
@@ -624,14 +625,14 @@ NetSendUDPPacket(uchar *ether, IPaddr_t dest, int dport, int sport, int len) pkt = NetArpWaitTxPacket; pkt += NetSetEther(pkt, NetArpWaitPacketMAC, PROT_IP);
- NetSetIP(pkt, dest, dport, sport, len);
- NetSetIP(pkt, dest, dport, sport, payload_len);
memcpy(pkt + IP_UDP_HDR_SIZE, (uchar *)NetTxPacket + (pkt - (uchar *)NetArpWaitTxPacket) +
- IP_UDP_HDR_SIZE, len);
- IP_UDP_HDR_SIZE, payload_len);
/* size of the waiting packet */ NetArpWaitTxPacketSize = (pkt - NetArpWaitTxPacket) +
- IP_UDP_HDR_SIZE + len;
- IP_UDP_HDR_SIZE + payload_len;
/* and do the ARP request */ NetArpWaitTry = 1; @@ -644,8 +645,9 @@ NetSendUDPPacket(uchar *ether, IPaddr_t dest, int dport, int sport, int len)
pkt = (uchar *)NetTxPacket; pkt += NetSetEther(pkt, ether, PROT_IP);
- NetSetIP(pkt, dest, dport, sport, len);
- eth_send(NetTxPacket, (pkt - NetTxPacket) + IP_UDP_HDR_SIZE + len);
- NetSetIP(pkt, dest, dport, sport, payload_len);
- eth_send(NetTxPacket, (pkt - NetTxPacket) + IP_UDP_HDR_SIZE +
- payload_len);
return 0; /* transmitted */ } @@ -859,9 +861,9 @@ NetReceive(volatile uchar *inpkt, int len) { struct Ethernet_t *et; struct IP_UDP_t *ip;
- IPaddr_t tmp;
- IPaddr_t dst_ip;
IPaddr_t src_ip;
- int x;
- int eth_proto;
#if defined(CONFIG_CMD_CDP) int iscdp; #endif @@ -896,20 +898,21 @@ NetReceive(volatile uchar *inpkt, int len) if (mynvlanid == (ushort)-1) mynvlanid = VLAN_NONE;
- x = ntohs(et->et_protlen);
- eth_proto = ntohs(et->et_protlen);
debug("packet received\n");
- if (x < 1514) {
- if (eth_proto < 1514) {
/*
- * Got a 802 packet. Check the other protocol field.
- * Got a 802.2 packet. Check the other protocol field.
- * XXX VLAN over 802.2+SNAP not implemented!
*/
- x = ntohs(et->et_prot);
- eth_proto = ntohs(et->et_prot);
ip = (struct IP_UDP_t *)(NetRxPacket + E802_HDR_SIZE); len -= E802_HDR_SIZE;
- } else if (x != PROT_VLAN) { /* normal packet */
- } else if (eth_proto != PROT_VLAN) { /* normal packet */
ip = (struct IP_UDP_t *)(NetRxPacket + ETHER_HDR_SIZE); len -= ETHER_HDR_SIZE;
@@ -932,17 +935,17 @@ NetReceive(volatile uchar *inpkt, int len)
cti = ntohs(vet->vet_tag); vlanid = cti & VLAN_IDMASK;
- x = ntohs(vet->vet_type);
- eth_proto = ntohs(vet->vet_type);
ip = (struct IP_UDP_t *)(NetRxPacket + VLAN_ETHER_HDR_SIZE); len -= VLAN_ETHER_HDR_SIZE; }
- debug("Receive from protocol 0x%x\n", x);
- debug("Receive from protocol 0x%x\n", eth_proto);
#if defined(CONFIG_CMD_CDP) if (iscdp) {
- CDPHandler((uchar *)ip, len);
- CDPReceive((uchar *)ip, len);
return; } #endif @@ -955,7 +958,7 @@ NetReceive(volatile uchar *inpkt, int len) return; }
- switch (x) {
- switch (eth_proto) {
case PROT_ARP: ArpReceive(et, ip, len); @@ -994,10 +997,10 @@ NetReceive(volatile uchar *inpkt, int len) return; } /* If it is not for us, ignore it */
- tmp = NetReadIP(&ip->ip_dst);
- if (NetOurIP && tmp != NetOurIP && tmp != 0xFFFFFFFF) {
- dst_ip = NetReadIP(&ip->ip_dst);
- if (NetOurIP && dst_ip != NetOurIP && dst_ip != 0xFFFFFFFF) {
#ifdef CONFIG_MCAST_TFTP
- if (Mcast_addr != tmp)
- if (Mcast_addr != dst_ip)
#endif return; } -- 1.6.0.2
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot