Re: [U-Boot] [PATCH 13/28] net: Refactor bootp packet length computations
On Tuesday 24 January 2012 02:15:41 Joe Hershberger wrote: On Tue, Jan 24, 2012 at 1:05 AM, Simon Glass wrote: On Thu, Jan 19, 2012 at 4:53 PM, Joe Hershberger wrote: - debug(Transmitting DHCPREQUEST packet: len = %d\n, pktlen); #ifdef CONFIG_BOOTP_DHCP_REQUEST_DELAY udelay(CONFIG_BOOTP_DHCP_REQUEST_DELAY); #endif /* CONFIG_BOOTP_DHCP_REQUEST_DELAY */ + debug(Transmitting DHCPREQUEST packet: len = %d\n, pktlen); Did you move this on purpose? Yes... I figured a transmit debug message should be close to the transmit... not before a delay. generally we frown on these sort of side-commits ... -mike signature.asc Description: This is a digitally signed message part. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 13/28] net: Refactor bootp packet length computations
Hi Joe,
On Thu, Jan 19, 2012 at 4:53 PM, Joe Hershberger joe.hershber...@ni.com wrote:
Signed-off-by: Joe Hershberger joe.hershber...@ni.com
Cc: Joe Hershberger joe.hershber...@gmail.com
Cc: Wolfgang Denk w...@denx.de
---
net/bootp.c | 26 ++
1 files changed, 14 insertions(+), 12 deletions(-)
Refactor why or to what purpose? Perhaps a bit more detail in the
commit message?
diff --git a/net/bootp.c b/net/bootp.c
index 0c2af48..0d5f4cf 100644
--- a/net/bootp.c
+++ b/net/bootp.c
@@ -582,7 +582,8 @@ BootpRequest(void)
{
uchar *pkt, *iphdr;
struct Bootp_t *bp;
- int ext_len, pktlen, iplen;
+ int extlen, pktlen, iplen;
Is extlen better than ext_len?
+ int eth_hdr_size;
#ifdef CONFIG_BOOTP_RANDOM_DELAY
ulong i, rand_ms;
#endif
@@ -610,7 +611,8 @@ BootpRequest(void)
pkt = NetTxPacket;
memset((void *)pkt, 0, PKTSIZE);
- pkt += NetSetEther(pkt, NetBcastAddr, PROT_IP);
+ eth_hdr_size = NetSetEther(pkt, NetBcastAddr, PROT_IP);
+ pkt += eth_hdr_size;
/*
* Next line results in incorrect packet size being transmitted,
@@ -639,9 +641,9 @@ BootpRequest(void)
/* Request additional information from the BOOTP/DHCP server */
#if defined(CONFIG_CMD_DHCP)
- ext_len = DhcpExtended((u8 *)bp-bp_vend, DHCP_DISCOVER, 0, 0);
+ extlen = DhcpExtended((u8 *)bp-bp_vend, DHCP_DISCOVER, 0, 0);
#else
- ext_len = BootpExtended((u8 *)bp-bp_vend);
+ extlen = BootpExtended((u8 *)bp-bp_vend);
#endif
/*
@@ -660,9 +662,8 @@ BootpRequest(void)
* Calculate proper packet lengths taking into account the
* variable size of the options field
*/
- pktlen = ((int)(pkt-NetTxPacket)) + BOOTP_HDR_SIZE -
- sizeof(bp-bp_vend) + ext_len;
- iplen = BOOTP_HDR_SIZE - sizeof(bp-bp_vend) + ext_len;
+ iplen = BOOTP_HDR_SIZE - OPT_FIELD_SIZE + extlen;
+ pktlen = eth_hdr_size + IP_UDP_HDR_SIZE + iplen;
NetSetUDPHeader(iphdr, 0xL, PORT_BOOTPS, PORT_BOOTPC, iplen);
NetSetTimeout(SELECT_TIMEOUT, BootpTimeout);
@@ -798,13 +799,15 @@ static void DhcpSendRequestPkt(struct Bootp_t *bp_offer)
uchar *pkt, *iphdr;
struct Bootp_t *bp;
int pktlen, iplen, extlen;
+ int eth_hdr_size;
IPaddr_t OfferedIP;
debug(DhcpSendRequestPkt: Sending DHCPREQUEST\n);
pkt = NetTxPacket;
memset((void *)pkt, 0, PKTSIZE);
- pkt += NetSetEther(pkt, NetBcastAddr, PROT_IP);
+ eth_hdr_size = NetSetEther(pkt, NetBcastAddr, PROT_IP);
+ pkt += eth_hdr_size;
iphdr = pkt; /* We'll need this later to set proper pkt size */
pkt += IP_UDP_HDR_SIZE;
@@ -841,15 +844,14 @@ static void DhcpSendRequestPkt(struct Bootp_t *bp_offer)
extlen = DhcpExtended((u8 *)bp-bp_vend, DHCP_REQUEST,
NetDHCPServerIP, OfferedIP);
- pktlen = ((int)(pkt-NetTxPacket)) + BOOTP_HDR_SIZE -
- sizeof(bp-bp_vend) + extlen;
- iplen = BOOTP_HDR_SIZE - sizeof(bp-bp_vend) + extlen;
+ iplen = BOOTP_HDR_SIZE - OPT_FIELD_SIZE + extlen;
+ pktlen = eth_hdr_size + IP_UDP_HDR_SIZE + iplen;
NetSetUDPHeader(iphdr, 0xL, PORT_BOOTPS, PORT_BOOTPC, iplen);
- debug(Transmitting DHCPREQUEST packet: len = %d\n, pktlen);
#ifdef CONFIG_BOOTP_DHCP_REQUEST_DELAY
udelay(CONFIG_BOOTP_DHCP_REQUEST_DELAY);
#endif /* CONFIG_BOOTP_DHCP_REQUEST_DELAY */
+ debug(Transmitting DHCPREQUEST packet: len = %d\n, pktlen);
Did you move this on purpose?
NetSendPacket(NetTxPacket, pktlen);
}
--
1.6.0.2
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 13/28] net: Refactor bootp packet length computations
Hi Simon,
On Tue, Jan 24, 2012 at 1:05 AM, Simon Glass s...@chromium.org wrote:
Hi Joe,
On Thu, Jan 19, 2012 at 4:53 PM, Joe Hershberger joe.hershber...@ni.com
wrote:
Signed-off-by: Joe Hershberger joe.hershber...@ni.com
Cc: Joe Hershberger joe.hershber...@gmail.com
Cc: Wolfgang Denk w...@denx.de
---
net/bootp.c | 26 ++
1 files changed, 14 insertions(+), 12 deletions(-)
Refactor why or to what purpose? Perhaps a bit more detail in the
commit message?
Just removing the pointer subtraction similar to patch 12.
diff --git a/net/bootp.c b/net/bootp.c
index 0c2af48..0d5f4cf 100644
--- a/net/bootp.c
+++ b/net/bootp.c
@@ -582,7 +582,8 @@ BootpRequest(void)
{
uchar *pkt, *iphdr;
struct Bootp_t *bp;
- int ext_len, pktlen, iplen;
+ int extlen, pktlen, iplen;
Is extlen better than ext_len?
Just more consistent with other variables in the same calculations.
Not especially important, I guess.
+ int eth_hdr_size;
#ifdef CONFIG_BOOTP_RANDOM_DELAY
ulong i, rand_ms;
#endif
@@ -610,7 +611,8 @@ BootpRequest(void)
pkt = NetTxPacket;
memset((void *)pkt, 0, PKTSIZE);
- pkt += NetSetEther(pkt, NetBcastAddr, PROT_IP);
+ eth_hdr_size = NetSetEther(pkt, NetBcastAddr, PROT_IP);
+ pkt += eth_hdr_size;
/*
* Next line results in incorrect packet size being transmitted,
@@ -639,9 +641,9 @@ BootpRequest(void)
/* Request additional information from the BOOTP/DHCP server */
#if defined(CONFIG_CMD_DHCP)
- ext_len = DhcpExtended((u8 *)bp-bp_vend, DHCP_DISCOVER, 0, 0);
+ extlen = DhcpExtended((u8 *)bp-bp_vend, DHCP_DISCOVER, 0, 0);
#else
- ext_len = BootpExtended((u8 *)bp-bp_vend);
+ extlen = BootpExtended((u8 *)bp-bp_vend);
#endif
/*
@@ -660,9 +662,8 @@ BootpRequest(void)
* Calculate proper packet lengths taking into account the
* variable size of the options field
*/
- pktlen = ((int)(pkt-NetTxPacket)) + BOOTP_HDR_SIZE -
- sizeof(bp-bp_vend) + ext_len;
- iplen = BOOTP_HDR_SIZE - sizeof(bp-bp_vend) + ext_len;
+ iplen = BOOTP_HDR_SIZE - OPT_FIELD_SIZE + extlen;
+ pktlen = eth_hdr_size + IP_UDP_HDR_SIZE + iplen;
NetSetUDPHeader(iphdr, 0xL, PORT_BOOTPS, PORT_BOOTPC, iplen);
NetSetTimeout(SELECT_TIMEOUT, BootpTimeout);
@@ -798,13 +799,15 @@ static void DhcpSendRequestPkt(struct Bootp_t
*bp_offer)
uchar *pkt, *iphdr;
struct Bootp_t *bp;
int pktlen, iplen, extlen;
+ int eth_hdr_size;
IPaddr_t OfferedIP;
debug(DhcpSendRequestPkt: Sending DHCPREQUEST\n);
pkt = NetTxPacket;
memset((void *)pkt, 0, PKTSIZE);
- pkt += NetSetEther(pkt, NetBcastAddr, PROT_IP);
+ eth_hdr_size = NetSetEther(pkt, NetBcastAddr, PROT_IP);
+ pkt += eth_hdr_size;
iphdr = pkt; /* We'll need this later to set proper pkt size */
pkt += IP_UDP_HDR_SIZE;
@@ -841,15 +844,14 @@ static void DhcpSendRequestPkt(struct Bootp_t
*bp_offer)
extlen = DhcpExtended((u8 *)bp-bp_vend, DHCP_REQUEST,
NetDHCPServerIP, OfferedIP);
- pktlen = ((int)(pkt-NetTxPacket)) + BOOTP_HDR_SIZE -
- sizeof(bp-bp_vend) + extlen;
- iplen = BOOTP_HDR_SIZE - sizeof(bp-bp_vend) + extlen;
+ iplen = BOOTP_HDR_SIZE - OPT_FIELD_SIZE + extlen;
+ pktlen = eth_hdr_size + IP_UDP_HDR_SIZE + iplen;
NetSetUDPHeader(iphdr, 0xL, PORT_BOOTPS, PORT_BOOTPC, iplen);
- debug(Transmitting DHCPREQUEST packet: len = %d\n, pktlen);
#ifdef CONFIG_BOOTP_DHCP_REQUEST_DELAY
udelay(CONFIG_BOOTP_DHCP_REQUEST_DELAY);
#endif /* CONFIG_BOOTP_DHCP_REQUEST_DELAY */
+ debug(Transmitting DHCPREQUEST packet: len = %d\n, pktlen);
Did you move this on purpose?
Yes... I figured a transmit debug message should be close to the
transmit... not before a delay.
NetSendPacket(NetTxPacket, pktlen);
}
Best regards,
-Joe
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
