From mboxrd@z Thu Jan  1 00:00:00 1970
From: Joe Hershberger <joe.hershberger@gmail.com>
Date: Tue, 24 Jan 2012 01:15:41 -0600
Subject: [U-Boot] [PATCH 13/28] net: Refactor bootp packet length
	computations
In-Reply-To: <CAPnjgZ2NVKzWBUHvC8FKS3_f9pXNV8fYzuHyhrJF_nsBpqxr8Q@mail.gmail.com>
References: <1327020811-1538-1-git-send-email-joe.hershberger@ni.com>
	<1327020811-1538-14-git-send-email-joe.hershberger@ni.com>
	<CAPnjgZ2NVKzWBUHvC8FKS3_f9pXNV8fYzuHyhrJF_nsBpqxr8Q@mail.gmail.com>
Message-ID: <CANr=Z=YLhORQGACj3RQwGQCqnaHTsX_KujBXFcinH48B+QojFw@mail.gmail.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Simon,

On Tue, Jan 24, 2012 at 1:05 AM, Simon Glass <sjg@chromium.org> wrote:
> Hi Joe,
>
> On Thu, Jan 19, 2012 at 4:53 PM, Joe Hershberger <joe.hershberger@ni.com> wrote:
>> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
>> Cc: Joe Hershberger <joe.hershberger@gmail.com>
>> Cc: Wolfgang Denk <wd@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, 0xFFFFFFFFL, 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, 0xFFFFFFFFL, 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