[U-Boot-Users] multicast tftp: RFC2090

[U-Boot-Users] multicast tftp: RFC2090

[David Updegraff]

Hi.

looking for interest to support mutlicast tftp in u-boot.  I have a
version working according to RFC2090  working with 'atftp' with only a
dozen or so lines patch in net/tftp.c and a few in net/net.c.  But the
messy part is that the ethernet driver needs to have hooks to set up
mcast ether-addrs...

Has there already been a discussion about this that I missed that
someone could refer me to?

thnx.

-dbu.

[U-Boot-Users] multicast tftp: RFC2090

[Wolfgang Denk]

In message <f0l75r$jem$1@sea.gmane.org> you wrote:
> 
> looking for interest to support mutlicast tftp in u-boot.  I have a
> version working according to RFC2090  working with 'atftp' with only a
> dozen or so lines patch in net/tftp.c and a few in net/net.c.  But the
> messy part is that the ethernet driver needs to have hooks to set up
> mcast ether-addrs...
> 
> Has there already been a discussion about this that I missed that
> someone could refer me to?

This has indeed been discussed before; the thread was started here:

	Date: Tue, 29 Aug 2006 09:55:34 -0400
	From: mitsy <mitsy12@gmail.com>
	To: u-boot-users at lists.sourceforge.net
	Subject: [U-Boot-Users] multicast tftp client? 

Short summary: we are waiting for a clever implementation (i. e.
yours :-)

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH, HRB 165235 Munich, CEO: Wolfgang Denk
Office:  Kirchenstr. 5,       D-82194 Groebenzell,            Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Programmer's Lament: (Shakespeare, Macbeth, Act I, Scene vii)
        "That we but teach bloody instructions,
        which, being taught, return to plague the inventor..."

[U-Boot-Users] multicast tftp: RFC2090

[Ben Warren]

Hi Dave,

On Tue, 2007-04-24 at 10:19 -0500, David Updegraff wrote:
> Hi.
> 
> looking for interest to support mutlicast tftp in u-boot.  I have a
> version working according to RFC2090  working with 'atftp' with only a
> dozen or so lines patch in net/tftp.c and a few in net/net.c.  But the
> messy part is that the ethernet driver needs to have hooks to set up
> mcast ether-addrs...
> 
> Has there already been a discussion about this that I missed that
> someone could refer me to?
> 
This looks interesting.  Have you done much testing to see how it
reduces network traffic and server loading?  I imagine you'd probably
need quite a few boards booting simultaneously to see any benefit.

Anyway, I wouldn't worry too much about having to modify Ethernet
drivers to add mcast addresses.  I expect that interest in this feature
will be fairly low, and adding an mcast address to a MAC is typically
not very complicated.  It's pretty common in Linux drivers so people can
always steal from there. 

I'm a little more worried about purging the entries when you're done,
since I imagine you would typically want to enter Linux or whatever with
an empty multicast hash table.  Maybe this isn't an issue, though.

regards,
Ben 

[U-Boot-Users] multicast tftp: RFC2090

[David Updegraff]

Ben

lemme test a bit and I'll post a patch in a day or so; see what the more
experienced folks on the list think.

-dbu.

[U-Boot-Users] multicast tftp: RFC2090

[David Updegraff]

Here is a first pass; not yet request for merge.

Only have been able to test with 8343 TSEC and an old SC520-based
rtl8139 so far, and with only two concurrent downloads; against atftpd
server. Re-cycling the existing ext2 bitmap functions to do housekeeping.

In about a month I should have access to enough hardware to test 20+
concurrent tftpboots.

Suggestions & comments appreciated.

thnx.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mcast.patch
Type: text/x-patch
Size: 17314 bytes
Desc: not available
Url : http://lists.denx.de/pipermail/u-boot/attachments/20070501/5d49c116/attachment.bin 

[U-Boot-Users] multicast tftp: RFC2090

[Andy Fleming]

On 5/1/07, David Updegraff <dave@cray.com> wrote:
> Here is a first pass; not yet request for merge.
>
> Only have been able to test with 8343 TSEC and an old SC520-based
> rtl8139 so far, and with only two concurrent downloads; against atftpd
> server. Re-cycling the existing ext2 bitmap functions to do housekeeping.
>
> In about a month I should have access to enough hardware to test 20+
> concurrent tftpboots.
>
> Suggestions & comments appreciated.

I have to NACK this portion:

@@ -378,6 +378,28 @@ uint mii_parse_sr(uint mii_reg, struct tsec_private * priv)
 		priv->link = 1;
 	}

+	/* take a stab at 10/100  and Duplex info too; from linux mii.c */
+	mii_reg = (read_phy_reg(priv, PHY_ANAR) &  read_phy_reg(priv, PHY_ANLPAR));
+	if ( mii_reg & PHY_ANLPAR_TXFD ) {
+		priv->duplexity = 1;
+		priv->speed = 100;
+	}
+	else if ( mii_reg & PHY_ANLPAR_TX ) {
+		priv->duplexity = 0;
+		priv->speed = 100;
+	}
+	else if ( mii_reg & PHY_ANLPAR_TX ) {
+		priv->duplexity = 0;
+		priv->speed = 100;
+	}
+	else if ( mii_reg & PHY_ANLPAR_10FD ) {
+		priv->duplexity = 1;
+		priv->speed = 10;
+	} else {
+		priv->duplexity = 0;
+		priv->speed = 10;
+	}
+
 	return 0;
 }

@@ -1056,6 +1078,26 @@ struct phy_info phy_info_VSC8244 = {
 			   {miim_end,}
 			   },
 };
+/* a generic flavor.  */
+struct phy_info phy_info_generic =  {
+	0,
+	"Unknown/Generic PHY",
+	32,
+	(struct phy_cmd[]) { /* config */
+		{PHY_BMCR, PHY_BMCR_RESET, NULL},
+		{PHY_BMCR, PHY_BMCR_AUTON|PHY_BMCR_RST_NEG, NULL},
+		{miim_end,}
+	},
+	(struct phy_cmd[]) { /* startup */
+		{PHY_BMSR, miim_read, NULL},
+		{PHY_BMSR, miim_read, &mii_parse_sr},
+		{miim_end,}
+	},
+	(struct phy_cmd[]) { /* shutdown */
+		{miim_end,}
+	}
+};
+

 struct phy_info phy_info_dm9161 = {
 	0x0181b88,
@@ -1203,6 +1245,7 @@ struct phy_info *phy_info[] = {
 	&phy_info_lxt971,
 	&phy_info_VSC8244,
 	&phy_info_dp83865,
+	&phy_info_generic,
 	NULL
 };


Sadly, I've already applied it to my tree, but I've just determined
that it breaks gigabit links, even on boards with known PHYs.

I'm going to fix that, but these two hunks need to be in a separate
patch, anyway.  I will post the updated version.

Andy

[U-Boot-Users] multicast tftp: RFC2090

[David Updegraff]

Andy

whoops!  I see your point now!  the parse_sr shouldn't be taking the
stab at determining lnk.etc. if the curphy != generic  I guess.


> On 5/1/07, David Updegraff <dave@cray.com> wrote:
>> Here is a first pass; not yet request for merge.
>>
>> Only have been able to test with 8343 TSEC and an old SC520-based
>> rtl8139 so far, and with only two concurrent downloads; against atftpd
>> server. Re-cycling the existing ext2 bitmap functions to do housekeeping.
>>
>> In about a month I should have access to enough hardware to test 20+
>> concurrent tftpboots.
>>
>> Suggestions & comments appreciated.
> 
> I have to NACK this portion:
> 
> @@ -378,6 +378,28 @@ uint mii_parse_sr(uint mii_reg, struct tsec_private
> * priv)
>         priv->link = 1;
>     }
> 
> +    /* take a stab at 10/100  and Duplex info too; from linux mii.c */
> +    mii_reg = (read_phy_reg(priv, PHY_ANAR) &  read_phy_reg(priv,
> PHY_ANLPAR));
> +    if ( mii_reg & PHY_ANLPAR_TXFD ) {
> +        priv->duplexity = 1;
> +        priv->speed = 100;
> +    }
> +    else if ( mii_reg & PHY_ANLPAR_TX ) {
> +        priv->duplexity = 0;
> +        priv->speed = 100;
> +    }
> +    else if ( mii_reg & PHY_ANLPAR_TX ) {
> +        priv->duplexity = 0;
> +        priv->speed = 100;
> +    }
> +    else if ( mii_reg & PHY_ANLPAR_10FD ) {
> +        priv->duplexity = 1;
> +        priv->speed = 10;
> +    } else {
> +        priv->duplexity = 0;
> +        priv->speed = 10;
> +    }
> +
>     return 0;
> }
> 
> @@ -1056,6 +1078,26 @@ struct phy_info phy_info_VSC8244 = {
>                {miim_end,}
>                },
> };
> +/* a generic flavor.  */
> +struct phy_info phy_info_generic =  {
> +    0,
> +    "Unknown/Generic PHY",
> +    32,
> +    (struct phy_cmd[]) { /* config */
> +        {PHY_BMCR, PHY_BMCR_RESET, NULL},
> +        {PHY_BMCR, PHY_BMCR_AUTON|PHY_BMCR_RST_NEG, NULL},
> +        {miim_end,}
> +    },
> +    (struct phy_cmd[]) { /* startup */
> +        {PHY_BMSR, miim_read, NULL},
> +        {PHY_BMSR, miim_read, &mii_parse_sr},
> +        {miim_end,}
> +    },
> +    (struct phy_cmd[]) { /* shutdown */
> +        {miim_end,}
> +    }
> +};
> +
> 
> struct phy_info phy_info_dm9161 = {
>     0x0181b88,
> @@ -1203,6 +1245,7 @@ struct phy_info *phy_info[] = {
>     &phy_info_lxt971,
>     &phy_info_VSC8244,
>     &phy_info_dp83865,
> +    &phy_info_generic,
>     NULL
> };
> 
> 
> Sadly, I've already applied it to my tree, but I've just determined
> that it breaks gigabit links, even on boards with known PHYs.
> 
> I'm going to fix that, but these two hunks need to be in a separate
> patch, anyway.  I will post the updated version.
> 
> Andy

[U-Boot-Users] multicast tftp: RFC2090

[Andy Fleming]

On 5/1/07, David Updegraff <dave@cray.com> wrote:
> Andy
>
> whoops!  I see your point now!  the parse_sr shouldn't be taking the
> stab at determining lnk.etc. if the curphy != generic  I guess.


Yeah.  I've got a bead on a solution now, though.  I will have a patch
in an hour or so.

Andy

[U-Boot-Users] multicast tftp: RFC2090

[David Updegraff]

Ok; here my mulicast TFTP patch.  Have had the opportunity to test with
both the RTL8139 and TSEC ethernet drivers, with up to a dozen clients
concurrent.

In a way I'm tempted to simply remove the #if CONFIG_RFC7090 clutter as
it is benign if you happen to be talking to a non-multicast tftp server;
and would make things rather more readable.  But too timid...

-dbu.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: rfc7090.patch
Type: text/x-patch
Size: 15469 bytes
Desc: not available
Url : http://lists.denx.de/pipermail/u-boot/attachments/20070518/b41f91ce/attachment.bin 

[U-Boot-Users] multicast tftp: RFC2090

[Jerry Van Baren]

David Updegraff wrote:
> Ok; here my mulicast TFTP patch.  Have had the opportunity to test with
> both the RTL8139 and TSEC ethernet drivers, with up to a dozen clients
> concurrent.
> 
> In a way I'm tempted to simply remove the #if CONFIG_RFC7090 clutter as
> it is benign if you happen to be talking to a non-multicast tftp server;
> and would make things rather more readable.  But too timid...
> 
> -dbu.

Hi Dave,

Interesting, not as much change needed as I would have guessed.

Now I'm dying of curiosity... what is your impression of the usefulness 
of RFC7090?  It always struck me as a lab curiosity: in fairly 
artificial cases where a bunch of CPU boards are powered up 
simultaneously...
* a room full of machines with a master breaker
* a rack of CPUs
it would be a big win, but that is a fairly unusual setup in the areas I 
hang out in.

On the other hand, we have a customer that currently has up to 4 units 
in a rack, and in the future possibly more units in a rack, that could 
possibly benefit from RFC2090.

Best regards,
gvb

[U-Boot-Users] multicast tftp: RFC2090

[David Updegraff]

Jerry

IMHO.. its a no-brainer: it costs almost nothing, adds _very_ litte
code, incurs no penatly for FTP servers that dont support it or that
have no other concurrent downloaders... but has a _H_U_G_E_ impact where
its usefull.

I.e. to my view, RCF7090 should be _the_ tftp protocol.  All upside, no
downside that I see.  Aside -- of course! -- from the bugs we have not
yet found in that patch. -;)


> David Updegraff wrote:
>> Ok; here my mulicast TFTP patch.  Have had the opportunity to test with
>> both the RTL8139 and TSEC ethernet drivers, with up to a dozen clients
>> concurrent.
>>
>> In a way I'm tempted to simply remove the #if CONFIG_RFC7090 clutter as
>> it is benign if you happen to be talking to a non-multicast tftp server;
>> and would make things rather more readable.  But too timid...
>>
>> -dbu.
> 
> Hi Dave,
> 
> Interesting, not as much change needed as I would have guessed.
> 
> Now I'm dying of curiosity... what is your impression of the usefulness
> of RFC7090?  It always struck me as a lab curiosity: in fairly
> artificial cases where a bunch of CPU boards are powered up
> simultaneously...
> * a room full of machines with a master breaker
> * a rack of CPUs
> it would be a big win, but that is a fairly unusual setup in the areas I
> hang out in.
> 
> On the other hand, we have a customer that currently has up to 4 units
> in a rack, and in the future possibly more units in a rack, that could
> possibly benefit from RFC2090.
> 
> Best regards,
> gvb

[U-Boot-Users] multicast tftp: RFC2090

[Ben Warren]

HiDave,

On Fri, 2007-05-18 at 12:05 -0500, David Updegraff wrote: 
> Ok; here my mulicast TFTP patch.  Have had the opportunity to test with
> both the RTL8139 and TSEC ethernet drivers, with up to a dozen clients
> concurrent.
> 
> In a way I'm tempted to simply remove the #if CONFIG_RFC7090 clutter as
> it is benign if you happen to be talking to a non-multicast tftp server;
> and would make things rather more readable.  But too timid...
> 
> -dbu.

I apologize for taking so long to get to this.  I like it and will work
hard to get it included ASAP.  However, there are a few nits:

> diff --git a/README b/README
> index bb5b46e..29a308d 100644
> --- a/README
> +++ b/README
> @@ -1112,6 +1112,16 @@ The following options need to be configured:
>                 Defines a default value for theIP address of a TFTP
>                 server to contact when using the "tftboot" command.
>  
> +- Multicast TFTP Mode:
> +               CONFIG_RFC7090
> +
> +               Defines whether you want to support multicast TFTP as
> per
> +               rfc-7090; for example to work with atftp.  Lets lots of
> targets
> +               tftp down the same boot image concurrently.  Note: the
> ethernet
> +               driver in use must provide a function: mcast() to
> join/leave a
> +               multicast group.
> +
> +               CONFIG_BOOTP_RANDOM_DELAY
>  - BOOTP Recovery Mode:
>                 CONFIG_BOOTP_RANDOM_DELAY
>  
I'd rather see it called CONFIG_MCAST_TFTP (or something like that) and
reference the RFC in the README.  You're right that the CONFIG is
probably not needed anyway, but if it's in plain English it doesn't
really hurt and will save a few bytes for those not using it.

<snip>
>  #endif /* CFG_CMD_NET && CONFIG_NET_MULTI && CONFIG_RTL8139 */
> diff --git a/drivers/tsec.c b/drivers/tsec.c
> index b418773..66d01dd 100644
> --- a/drivers/tsec.c
> +++ b/drivers/tsec.c
> @@ -129,6 +129,9 @@ static int tsec_miiphy_write(char *devname, unsigned
> char addr,
>                              unsigned char reg, unsigned short value);
>  static int tsec_miiphy_read(char *devname, unsigned char addr,
>                             unsigned char reg, unsigned short *value);
> +#ifdef CONFIG_RFC2090  /* Multicast TFTP ? */
> +static int tsec_bcast_addr (struct eth_device *dev, u8 bcast_mac, u8
> set);
> +#endif
>  
>  /* Initialize device structure. Returns success if PHY
>   * initialization succeeded (i.e. if it recognizes the PHY)
> @@ -167,6 +170,9 @@ int tsec_initialize(bd_t * bis, int index, char
> *devname)
>         dev->halt = tsec_halt;
>         dev->send = tsec_send;
>         dev->recv = tsec_recv;
> +#ifdef CONFIG_RFC2090  /* Multicast TFTP ? */
> +       dev->mcast = tsec_bcast_addr;
> +#endif
>  
>         /* Tell u-boot to get the addr from the env */
>         for (i = 0; i < 6; i++)
> @@ -1479,4 +1485,46 @@ static int tsec_miiphy_write(char *devname,
> unsigned char addr,
>  #endif /* defined(CONFIG_MII) || (CONFIG_COMMANDS & CFG_CMD_MII)
>                 && !defined(BITBANGMII) */
>  
> +#ifdef CONFIG_RFC2090  /* Multicast TFTP ? */
> +
> +/* CREDITS: linux gianfar driver, slightly adjusted... thanx. */
> +
> +/* Set the appropriate hash bit for the given addr */
> +
> +/* The algorithm works like so:
> + * 1) Take the Destination Address (ie the multicast address), and
> + * do a CRC on it (little endian), and reverse the bits of the
> + * result.
> + * 2) Use the 8 most significant bits as a hash into a 256-entry
> + * table.  The table is controlled through 8 32-bit registers:
> + * gaddr0-7.  gaddr0's MSB is entry 0, and gaddr7's LSB is
> + * gaddr7.  This means that the 3 most significant bits in the
> + * hash index which gaddr register to use, and the 5 other bits
> + * indicate which bit (assuming an IBM numbering scheme, which
> + * for PowerPC (tm) is usually the case) in the tregister holds
> + * the entry. */
> +static int
> +tsec_bcast_addr (struct eth_device *dev, u8 bcast_mac, u8 set)
> +{
Why don't you call this function 'tsec_mcast_addr'?  Enabling the
reception of broadcast frames on this device is done by flipping a bit
in the RCTRL register.

<snip>

> @@ -102,7 +102,9 @@ struct eth_device {
>         int  (*send) (struct eth_device*, volatile void* pachet, int

I know this isn't yours, but while in here, can you fix this typo
please?  It looks kinda French, but I think it should then be 'paquet'.

<snip>
> +#ifdef CONFIG_RFC2090
> +int eth_mcast_join( IPaddr_t mcast_addr, u8 join);
> +u32 ether_crc (size_t len, unsigned char const *p);
> +#endif
> +
>  
>  /**********************************************************************/
>  /*
> diff --git a/net/eth.c b/net/eth.c
> index 0fc2211..22243eb 100644
> --- a/net/eth.c
> +++ b/net/eth.c
> @@ -369,6 +369,51 @@ void eth_set_enetaddr(int num, char *addr) {
>  
>         memcpy(dev->enetaddr, enetaddr, 6);
>  }
> +#ifdef CONFIG_RFC2090
> +/* Multicast.
> + * mcast_addr: multicast ipaddr from which multicast Mac is made
> + * join: 1=join, 0=leave.  
> + */
> +int eth_mcast_join( IPaddr_t mcast_ip, u8 join)
> +{
> + u8 mcast_mac[6];
> +       if (!eth_current || !eth_current->mcast) 
> +               return -1;
> +       mcast_mac[5] = htonl(mcast_ip) & 0xff;
> +       mcast_mac[4] = (htonl(mcast_ip)>>8) & 0xff;
> +       mcast_mac[3] = (htonl(mcast_ip)>>16) & 0x7f;
> +       mcast_mac[2] = 0x5e;
> +       mcast_mac[1] = 0x0;
> +       mcast_mac[0] = 0x1;
> +       return eth_current->mcast(eth_current, mcast_mac, join);
> +}
> +
> +/* the 'way' for ethernet-CRC-32. Spliced in from Linux lib/crc32.c 
> + * and this is the ethernet-crc method needed for TSEC -- and perhaps
> some 

Line's a bit long...

> + * other adapter -- hash tables
> + */
> +#define CRCPOLY_LE 0xedb88320
> +u32 ether_crc (size_t len, unsigned char const *p)
> +{
> +       int i;
> +       u32 crc;
> +       crc = ~0;
> +       while (len--) {
> +               crc ^= *p++;
> +               for (i = 0; i < 8; i++)
> +                       crc = (crc >> 1) ^ ((crc & 1) ? CRCPOLY_LE : 0);
> +       }
> +       /* an reverse the bits, cuz of way they arrive -- last-first */
> +       crc = (crc >> 16) | (crc << 16);
> +       crc = (crc >> 8 & 0x00ff00ff) | (crc << 8 & 0xff00ff00);
> +       crc = (crc >> 4 & 0x0f0f0f0f) | (crc << 4 & 0xf0f0f0f0);
> +       crc = (crc >> 2 & 0x33333333) | (crc << 2 & 0xcccccccc);
> +       crc = (crc >> 1 & 0x55555555) | (crc << 1 & 0xaaaaaaaa);
> +       return crc;
> +}
> +
If this is indeed different from the CRC-32 algorithms that we already
have (yeah, I got curious and wrote a little test program and got
different results, then laziness kicked in and I didn't go any further),
then it would be nice to put it in lib_generic or something like that.
For now, though, this is fine.
<snip>
> diff --git a/net/tftp.c b/net/tftp.c
> index f3a5471..21ccadb 100644
> --- a/net/tftp.c
> +++ b/net/tftp.c
> @@ -61,6 +61,31 @@ static char *tftp_filename;
>  extern flash_info_t flash_info[];
>  #endif
>  
> +#ifdef CONFIG_RFC2090  /* Multicast TFTP */
> +#include <malloc.h>

I guess you had a good reason for playing the malloc/free game instead
of a static allocation?
> +/* 8k bit map .. 8k blocks is 16M.. max?? */
> +#define MTFTP_BITMAPSIZE       0x2000
> +static unsigned *Bitmap;
> +static int PrevBitmapHole ;
> +static uchar ProhibitMcast=0;
> +static uchar MasterClient=0;
> +static uchar Multicast=0;
> +static void parse_multicast_oack(char *pkt,int len);
> +extern IPaddr_t Mcast_addr;
> +static int Mcast_port;
> +static ulong TftpEndingBlock; /* .. I can get 'last' block before
> done..*/
> +
> +static void
> +mcast_cleanup(void)
> +{
> +       if (Mcast_addr) eth_mcast_join(Mcast_addr, 0);
> +       if (Bitmap) free(Bitmap);
> +       Bitmap=NULL;
> +       Mcast_addr = Multicast = Mcast_port = 0;
> +       TftpEndingBlock = -1;
> +}
> +#endif /* RFC2090 */
> +
>  static __inline__ void
>  store_block (unsigned block, uchar * src, unsigned len)
>  {
> @@ -90,6 +115,10 @@ store_block (unsigned block, uchar * src, unsigned
> len)
>         {
>                 (void)memcpy((void *)(load_addr + offset), src, len);
>         }
> +#ifdef CONFIG_RFC2090
> +       if (Multicast)
> +               ext2_set_bit(block, Bitmap);
> +#endif
>  
>         if (NetBootFileXferSize < newsize)
>                 NetBootFileXferSize = newsize;
> @@ -108,6 +137,11 @@ TftpSend (void)
>         int                     len = 0;
>         volatile ushort *s;
>  
> +#ifdef CONFIG_RFC2090  
> +       /* Multicast TFTP.. non-MasterClients do not ACK data. */
> +       if (Multicast && (TftpState == STATE_DATA) && (MasterClient ==
> 0)) 
Line's a bit long here too.
> +               return;
> +#endif
>         /*
>          *      We will always be sending some sort of packet, so
>          *      cobble together the packet headers now.
> @@ -132,11 +166,24 @@ TftpSend (void)
>                 printf("send option \"timeout %s\"\n", (char *)pkt);
>  #endif
>                 pkt += strlen((char *)pkt) + 1;
> +#ifdef CONFIG_RFC2090  
> +               if (!ProhibitMcast && eth_get_dev()->mcast) {
> +                       pkt += sprintf((char *)pkt,"multicast");
> +                       *pkt++ = '\0';
> +                       *pkt++ = '\0';
> +               }
> +#endif /* RFC2090 */
>                 len = pkt - xp;
>                 break;
>  
> -       case STATE_DATA:
>         case STATE_OACK:
> +#ifdef CONFIG_RFC2090
> +               /* ..the next one that I want.. */
> +               if (Multicast)
> +                       TftpBlock =
> ext2_find_next_zero_bit(Bitmap,(MTFTP_BITMAPSIZE*8),0);
> +               /*..falling..*/
> +#endif
> +       case STATE_DATA:
>                 xp = pkt;
>                 s = (ushort *)pkt;
>                 *s++ = htons(TFTP_ACK);
> @@ -179,6 +226,9 @@ TftpHandler (uchar * pkt, unsigned dest, unsigned
> src, unsigned len)
>         ushort *s;
>  
>         if (dest != TftpOurPort) {
> +#ifdef CONFIG_RFC2090
> +               if (!Mcast_port || (dest != Mcast_port)) 
> +#endif
>                 return;
>         }
>         if (TftpState != STATE_RRQ && src != TftpServerPort) {
> @@ -208,6 +258,12 @@ TftpHandler (uchar * pkt, unsigned dest, unsigned
> src, unsigned len)
>  #endif
>                 TftpState = STATE_OACK;
>                 TftpServerPort = src;
> +#ifdef CONFIG_RFC2090
> +               parse_multicast_oack((char *)pkt,len-1);
> +               if ((Multicast) && (!MasterClient)) 
> +                       TftpState = STATE_DATA; /* non-master just
> listens */
Oh no, another long line.
> +               else
> +#endif
>                 TftpSend (); /* Send ACK */
>                 break;
>         case TFTP_DATA:
> @@ -248,6 +304,11 @@ TftpHandler (uchar * pkt, unsigned dest, unsigned
> src, unsigned len)
>                         TftpBlockWrap = 0;
>                         TftpBlockWrapOffset = 0;
>  
> +#ifdef CONFIG_RFC2090
> +                       if (Multicast) { /* ..happens commonly if mcast
> */
> +                               TftpLastBlock = TftpBlock - 1;
> +                       } else
> +#endif
>                         if (TftpBlock != 1) {   /* Assertion */
>                                 printf ("\nTFTP error: "
>                                         "First block is not block 1 (%
> ld)\n"
> @@ -274,8 +335,33 @@ TftpHandler (uchar * pkt, unsigned dest, unsigned
> src, unsigned len)
>                  *      Acknoledge the block just received, which will
> prompt
>                  *      the server for the next one.
>                  */
> +#ifdef CONFIG_RFC2090
> +               /* if I am the MasterClient, actively calculate what my
> next needed
> +                * block is; else I'm passive; not ACKING 
> +                */
> +               if (Multicast) {
> +                       if (len < TFTP_BLOCK_SIZE)  {
> +                               TftpEndingBlock = TftpBlock;
> +                       } else if (MasterClient) {
> +                               TftpBlock = PrevBitmapHole = 
> +                                               ext2_find_next_zero_bit(
> +                                                       Bitmap,(MTFTP_BITMAPSIZE*8),PrevBitmapHole);
Far too long.  These 8-character tabs can be a pain...  I'll stop
commenting about line length here.  Please make sure all lines are <
(U-boot max line length), which I believe is 78.

> +                               TftpLastBlock = TftpBlock;
> +                       }
> +               }
> +#endif
>                 TftpSend ();
>  
> +#ifdef CONFIG_RFC2090
> +               if (Multicast) {
> +                       if (MasterClient && (TftpBlock >=
> TftpEndingBlock)) {
> +                               puts ("\nMulticast tftp done\n");
> +                               mcast_cleanup();
> +                               NetState = NETLOOP_SUCCESS;
> +                       } 
> +               }
> +               else
> +#endif
>                 if (len < TFTP_BLOCK_SIZE) {
>                         /*
>                          *      We received the whole thing.  Try to
> @@ -290,6 +376,9 @@ TftpHandler (uchar * pkt, unsigned dest, unsigned
> src, unsigned len)
>                 printf ("\nTFTP error: '%s' (%d)\n",
>                                         pkt + 2, ntohs(*(ushort *)pkt));
>                 puts ("Starting again\n\n");
> +#ifdef CONFIG_RFC2090
> +               mcast_cleanup();
> +#endif
>                 NetStartAgain ();
>                 break;
>         }
> @@ -301,6 +390,9 @@ TftpTimeout (void)
>  {
>         if (++TftpTimeoutCount > TIMEOUT_COUNT) {
>                 puts ("\nRetry count exceeded; starting again\n");
> +#ifdef CONFIG_RFC2090
> +               mcast_cleanup();
> +#endif
>                 NetStartAgain ();
>         } else {+               memset (Bitmap,0,MTFTP_BITMAPSIZE);
> +               PrevBitmapHole = 0;
> +               Multicast = 1;
> +       }
> +       addr = string_to_ip(mc_adr);
> +       if (Mcast_addr != addr) {
> +               if (Mcast_addr)
> +                       eth_mcast_join(Mcast_addr, 0);
> +               if (eth_mcast_join(Mcast_addr=addr, 1)) {
> +                       printf ("Fail to set bcast, revert to vanilla
> TFTP\n");
> +                       ProhibitMcast = 1;
> +                       mcast_cleanup();
> +                       NetStartAgain ();
> +                       return;
> +               }
> +       }
> +       MasterClient = (unsigned char)simple_strtoul((char *)mc,&e,10);
> +       Mcast_port = (unsigned short)simple_strtoul(port,&e,10);
> +       printf ("Multicast: %s:%d [%d]\n", mc_adr, Mcast_port,
> MasterClient);
> +       return;
> +}
> +
> +
> +#endif /* Multicast TFTP */
> +
>  #endif /* CFG_CMD_NET */
> 
> 
>                 puts ("T ");+               memset (Bitmap,0,MTFTP_BITMAPSIZE);
> +               PrevBitmapHole = 0;
> +               Multicast = 1;
> +       }
> +       addr = string_to_ip(mc_adr);
> +       if (Mcast_addr != addr) {
> +               if (Mcast_addr)
> +                       eth_mcast_join(Mcast_addr, 0);
> +               if (eth_mcast_join(Mcast_addr=addr, 1)) {
> +                       printf ("Fail to set bcast, revert to vanilla
> TFTP\n");
> +                       ProhibitMcast = 1;
> +                       mcast_cleanup();
> +                       NetStartAgain ();
> +                       return;
> +               }
> +       }
> +       MasterClient = (unsigned char)simple_strtoul((char *)mc,&e,10);
> +       Mcast_port = (unsigned short)simple_strtoul(port,&e,10);
> +       printf ("Multicast: %s:%d [%d]\n", mc_adr, Mcast_port,
> MasterClient);
> +       return;
> +}
> +
> +
> +#endif /* Multicast TFTP */
> +
>  #endif /* CFG_CMD_NET */
> 
> 
> @@ -370,6 +462,7 @@ TftpStart (void)
>         TftpState = STATE_RRQ;
>         /* Use a pseudo-random port unless a specific port is set */
>         TftpOurPort = 1024 + (get_timer(0) % 3072);
> +
>  #ifdef CONFIG_TFTP_PORT
>         if ((ep = getenv("tftpdstp")) != NULL) {
>                 TftpServerPort = simple_strtol(ep, NULL, 10);
> @@ -382,8 +475,97 @@ TftpStart (void)
>  
>         /* zero out server ether in case the server ip has changed */
>         memset(NetServerEther, 0, 6);
> +#ifdef CONFIG_RFC2090  /* Multicast TFTP */
> +    mcast_cleanup();
> +#endif
>  
>         TftpSend ();
>  }
>  
> +#ifdef CONFIG_RFC2090  /* Multicast TFTP */
> +/* Credits: atftp project.
> + */
> +
> +/* pick up BcastAddr, Port, and whether I am [now] the master-client. *
> + * Frame:
> + *    +-------+-----------+---+-------~~-------+---+
> + *    |  opc  | multicast | 0 | addr, port, mc | 0 |
> + *    +-------+-----------+---+-------~~-------+---+
> + * The multicast addr/port becomes what I listen to, and if 'mc' is '1'
> then
> + * I am the new master-client so must send ACKs to DataBlocks.  If I am
> not
> + * master-client, I'm a passive client, gathering what DataBlocks I may
> and
> + * making note of which ones I got in my bitmask.
> + * In theory, I never go from master->passive..
> + * .. this comes in with pkt already pointing just past opc
> + */
> +static void parse_multicast_oack(char *pkt, int len)
> +{
> + int i;
> + IPaddr_t addr;
> + char *mc_adr, *port, *mc, *e;
> +
> +       mc_adr=port=mc=NULL;
> +       /* march along looking for 'multicast\0', which has to start at
> least
> +        * 14 bytes back from the end.
> +        */
> +       for (i=0;i<len-14;i++)
> +               if (strcmp (pkt+i,"multicast") == 0)
> +                       break;
> +       if (i >= (len-14)) {
> +               printf ("non-Multicast OACK, ign.\n");
> +               return;
> +       }
> +       i+=10; /* strlen multicast */
> +       mc_adr = pkt+i;
> +       for (;i<len;i++) {
> +               if (*(pkt+i) == ',') {
> +                       *(pkt+i) = '\0';
> +                       if (port) {
> +                               mc=pkt+i+1;
> +                               break;
> +                       } else {
> +                               port = pkt+i+1;
> +                       }
> +               }
> +       }
> +       if (!port || !mc_adr || !mc ) return;
> +       if (Multicast && MasterClient) {
> +               printf ("I got a OACK as master Client, WRONG!\n");
> +               return;
> +       }
> +       /* ..I now accept packets destined for this MCAST addr, port */
> +       if (!Multicast) {
> +               if (Bitmap) { 
> +                       printf ("BUGBUGBUG Dave! why is this set!\n");
> +                       free(Bitmap);
> +               }
Cute, but what if this happens and the user isn't named Dave?

> +               if (!(Bitmap = malloc (MTFTP_BITMAPSIZE))) {
> +                       printf ("OH NO! No Bitmap!\n");
> +                       return;
> +               }
Same comment, but minus the Dave part.

All in all, nice work.  Nicely commented too.

regards,
Ben

[U-Boot-Users] multicast tftp: RFC2090

[Wolfgang Denk]

In message <1181316292.8300.112.camel@saruman.qstreams.net> you wrote:
...
> > +       /* ..I now accept packets destined for this MCAST addr, port */
> > +       if (!Multicast) {
> > +               if (Bitmap) { 
> > +                       printf ("BUGBUGBUG Dave! why is this set!\n");
> > +                       free(Bitmap);

Maybe a "Bitmap = NULL;" is missing here?

Abd BTW: why is this variable called "Bitmap" ?

Please change this into "bitmap". Thanks.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
A meeting is an event at which the minutes are kept and the hours are
lost.

[U-Boot-Users] multicast tftp: RFC2090

[David Updegraff]

Wolfgang

ok.  Soon.

thnx.

> In message <1181316292.8300.112.camel@saruman.qstreams.net> you wrote:
> ...
>>> +       /* ..I now accept packets destined for this MCAST addr, port */
>>> +       if (!Multicast) {
>>> +               if (Bitmap) { 
>>> +                       printf ("BUGBUGBUG Dave! why is this set!\n");
>>> +                       free(Bitmap);
> 
> Maybe a "Bitmap = NULL;" is missing here?
> 
> Abd BTW: why is this variable called "Bitmap" ?
> 
> Please change this into "bitmap". Thanks.
> 
> Best regards,
> 
> Wolfgang Denk
> 

[U-Boot-Users] multicast tftp: RFC2090

[Ben Warren]

On Fri, 2007-06-08 at 17:39 +0200, Wolfgang Denk wrote: 
> In message <1181316292.8300.112.camel@saruman.qstreams.net> you wrote:
> ...
> > > +       /* ..I now accept packets destined for this MCAST addr, port */
> > > +       if (!Multicast) {
> > > +               if (Bitmap) { 
> > > +                       printf ("BUGBUGBUG Dave! why is this set!\n");
> > > +                       free(Bitmap);
> 
> Maybe a "Bitmap = NULL;" is missing here?
> 
> Abd BTW: why is this variable called "Bitmap" ?
> 
> Please change this into "bitmap". Thanks.
> 
> Best regards,
> 
> Wolfgang Denk
> 
Please forgive my ignorance, but is 'free()' implemented on all
architectures in the U-boot run-time code?  Some embedded things I've
worked on in the past didn't bother doing more than a stub since dynamic
memory allocation was generally frowned upon.  Of course, we were a
little more resource-constrained and it was years ago...

regards,
Ben

[U-Boot-Users] multicast tftp: RFC2090

[Wolfgang Denk]

In message <1181327758.8300.122.camel@saruman.qstreams.net> you wrote:
>
> Please forgive my ignorance, but is 'free()' implemented on all
> architectures in the U-boot run-time code?  Some embedded things I've

Yes, this is architecture independent code, and implements "standard"
malloc(), cmalloc(), free().

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The use of Microsoft crippleware systems is a sin that  carries  with
it its own punishment.
         -- Tom Christiansen in <6bo3fr$pj8$5@csnews.cs.colorado.edu>

[U-Boot-Users] multicast tftp: RFC2090

[Ben Warren]

On Fri, 2007-06-08 at 20:48 +0200, Wolfgang Denk wrote:
> In message <1181327758.8300.122.camel@saruman.qstreams.net> you wrote:
> >
> > Please forgive my ignorance, but is 'free()' implemented on all
> > architectures in the U-boot run-time code?  Some embedded things I've
> 
> Yes, this is architecture independent code, and implements "standard"
> malloc(), cmalloc(), free().

Good to know.  Thanks.

Ben

[U-Boot-Users] multicast tftp: RFC2090

[David Updegraff]

B.,W.

I lost access to my mini-cluster test platform for now, but will
re-submit when I get a chance to compile in and test your suggestions --
hopefully by next week.

-dbu.

[U-Boot-Users] multicast tftp: RFC2090

[David Updegraff]

Take two.

-dbu.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mcast.patch
Type: text/x-patch
Size: 17479 bytes
Desc: not available
Url : http://lists.denx.de/pipermail/u-boot/attachments/20070611/d7b3c17a/attachment.bin 

[U-Boot-Users] multicast tftp: RFC2090

[Wolfgang Denk]

In message <f4jqek$i3e$1@sea.gmane.org> you wrote:
>
> Take two.

Patch description and Signed-off-by line missing.

Coding style issues (trailing white space, indentation not by multiple
of 8 and not by TAB, etc.)

Please clean up.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
It seems intuitively obvious to me, which  means  that  it  might  be
wrong.                                                 -- Chris Torek