[U-Boot] [PATCH 13/28] net: Refactor bootp packet length computations

[Joe Hershberger]

Hi Simon,

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