[ECOS] RedBoot: __eth_install_handler?

[ECOS] RedBoot: __eth_install_handler?

[Grant Edwards]

In RedBoot's net.h file, there's a declaration for
__eth_install_handler(), which would seem from the declaration to
do exactly what I want to do: set a call-back to handle packets
with Ethernet protocol type <whatever>.

That functionality hasn't been implemented, so I'm about to
implement it.  

Were I doing this just for myself, I would probably just use
CYG_HAL_TABLE_xxxx to set up handlers at compile/link time and
not bother with run-time handler set up.  It's a bit simpler,
and you don't to guess ahead of time how many different handler
need to be allowed.  [I really like the HAL_TABLE mechanism:
it's quite clever, and it's something that I've needed on a
number of occasions.]

However, if people think run-time handler set-up is useful,
I'll go ahead and implement the existing API instead of using
the HAL_TABLE method.

Opinions?

-- 
Grant Edwards
grante@visi.com

RE: [ECOS] RedBoot: __eth_install_handler?

[Gary Thomas]

On 22-Jan-2001 Grant Edwards wrote:
> 
> In RedBoot's net.h file, there's a declaration for
> __eth_install_handler(), which would seem from the declaration to
> do exactly what I want to do: set a call-back to handle packets
> with Ethernet protocol type <whatever>.
> 
> That functionality hasn't been implemented, so I'm about to
> implement it.  
> 
> Were I doing this just for myself, I would probably just use
> CYG_HAL_TABLE_xxxx to set up handlers at compile/link time and
> not bother with run-time handler set up.  It's a bit simpler,
> and you don't to guess ahead of time how many different handler
> need to be allowed.  [I really like the HAL_TABLE mechanism:
> it's quite clever, and it's something that I've needed on a
> number of occasions.]
> 
> However, if people think run-time handler set-up is useful,
> I'll go ahead and implement the existing API instead of using
> the HAL_TABLE method.

Setting up the handler at runtime is much better IMHO.  This allows
for differing handlers depending on the situation, etc.  In fact,
I just added similar functionality for ICMP packets.

Re: [ECOS] RedBoot: __eth_install_handler?

[Grant Edwards]

> > In RedBoot's net.h file, there's a declaration for
> > __eth_install_handler(), which would seem from the declaration to
> > do exactly what I want to do: set a call-back to handle packets
> > with Ethernet protocol type <whatever>.
> > 
> > That functionality hasn't been implemented, so I'm about to
> > implement it.  
> > 
> > Were I doing this just for myself, I would probably just use
> > CYG_HAL_TABLE_xxxx to set up handlers at compile/link time and
> > not bother with run-time handler set up.  It's a bit simpler,
> > and you don't to guess ahead of time how many different handler
> > need to be allowed.  [I really like the HAL_TABLE mechanism:
> > it's quite clever, and it's something that I've needed on a
> > number of occasions.]
> > 
> > However, if people think run-time handler set-up is useful,
> > I'll go ahead and implement the existing API instead of using
> > the HAL_TABLE method.
> 
> Setting up the handler at runtime is much better IMHO.

Done.

-- 
Grant Edwards
grante@visi.com

Re: [ECOS] RedBoot: __eth_install_handler?

[Grant Edwards]

On Mon, Jan 22, 2001 at 02:57:41PM -0600, Grant Edwards wrote:
> 
> > > In RedBoot's net.h file, there's a declaration for
> > > __eth_install_handler(), which would seem from the declaration to
> > > do exactly what I want to do: set a call-back to handle packets
> > > with Ethernet protocol type <whatever>.
> > > 
> > > That functionality hasn't been implemented, so I'm about to
> > > implement it.  

It doesn't look like the packet handler routines verify that
the length of a received Ethernet frame is long enough to
contain the amount of data expected.  For example, the IP
handler doesn't check to make sure that the number of bytes in
the Ethernet frame was was large enough to hold the amount of
data claimed in the IP header.

The only way for that to happen would be for another host to
intentionally send a short Ethernet frame (AFAIK). Even if that
happens it should be caught by the checksum verification,
right?

-- 
Grant Edwards
grante@visi.com

Re: [ECOS] RedBoot: __eth_install_handler?

[Grant Edwards]

Attached is a patch that impliments __eth_install_handler()
functionality.  It's currently hard-wired to allow up to 4
handlers. The normal IP/ARP/RARP handlers are still handled
seperately.

-- 
Grant Edwards
grante@visi.com

Re: [ECOS] RedBoot: __eth_install_handler?

[Gary Thomas]

On 22-Jan-2001 Grant Edwards wrote:
> On Mon, Jan 22, 2001 at 02:57:41PM -0600, Grant Edwards wrote:
>> 
>> > > In RedBoot's net.h file, there's a declaration for
>> > > __eth_install_handler(), which would seem from the declaration to
>> > > do exactly what I want to do: set a call-back to handle packets
>> > > with Ethernet protocol type <whatever>.
>> > > 
>> > > That functionality hasn't been implemented, so I'm about to
>> > > implement it.  
> 
> It doesn't look like the packet handler routines verify that
> the length of a received Ethernet frame is long enough to
> contain the amount of data expected.  For example, the IP
> handler doesn't check to make sure that the number of bytes in
> the Ethernet frame was was large enough to hold the amount of
> data claimed in the IP header.
> 
> The only way for that to happen would be for another host to
> intentionally send a short Ethernet frame (AFAIK). Even if that
> happens it should be caught by the checksum verification,
> right?
>

That would certainly be true - the checksum routines will try
and access the advertised length of data.  I see no way that the
computed value would match if the actual data was too short.

Re: [ECOS] RedBoot: __eth_install_handler?

[Grant Edwards]

> Attached is a patch that impliments __eth_install_handler()
> functionality.  It's currently hard-wired to allow up to 4
> handlers. The normal IP/ARP/RARP handlers are still handled
> seperately.

I should have added a warning:

The ethernet header data pointed to by pkt->eth_hdr is non-
persistent data.  It's an automatic variable in the function
calling the registered handler.  You should neither attempt to
free it nor expect it to be there after your handler returns to
the caller.

The pkt->pkt_bytes value passed to the registered handler
includes the 14 bytes in the Ethernet headers.  In hindsight, I
think __enet_poll should subtract 14 before calling the handler
so that pkt->pkt_bytes is the number of data bytes pointed to
by pkt->buf.

-- 
Grant Edwards
grante@visi.com

Re: [ECOS] RedBoot: __eth_install_handler?

[Andrew Lunn]

> > The only way for that to happen would be for another host to
> > intentionally send a short Ethernet frame (AFAIK). Even if that
> > happens it should be caught by the checksum verification,
> > right?
> >
> 
> That would certainly be true - the checksum routines will try
> and access the advertised length of data.  I see no way that the
> computed value would match if the actual data was too short.

Depends if your hardware is broken, leaves checksum verification to
the software, or passes runt packets up. Its a bad assumption to
make. It was made once before and it broken when the i82559 broke this
assumption for promisc mode.

        Andrew

Re: [ECOS] RedBoot: __eth_install_handler?

[Gary Thomas]

[-- Attachment #1: Type: text/plain, Size: 363 bytes --]

I've applied this, with a few [cosmetic] modifications.

Patch attached, thanks.

On 22-Jan-2001 Grant Edwards wrote:
> 
> Attached is a patch that impliments __eth_install_handler()
> functionality.  It's currently hard-wired to allow up to 4
> handlers. The normal IP/ARP/RARP handlers are still handled
> seperately.
> 
> -- 
> Grant Edwards
> grante@visi.com

[-- Attachment #2: diffs --]
[-- Type: text/x-diff, Size: 6480 bytes --]

Index: redboot/current/ChangeLog
===================================================================
RCS file: /home/cvs/ecc/ecc/redboot/current/ChangeLog,v
retrieving revision 1.81
diff -u -5 -p -r1.81 ChangeLog
--- redboot/current/ChangeLog	2001/01/30 15:27:17	1.81
+++ redboot/current/ChangeLog	2001/01/31 12:23:04
@@ -1,5 +1,12 @@
+2001-01-31  Grant Edwards <grante@visi.com>
+
+	* include/net/net.h: 
+	* src/net/enet.c (__eth_install_listener, __eth_remove_listener): 
+	New functions - allow user registerable callout to handle 
+	non-standard ethernet packets.
+
 2001-01-30  Gary Thomas  <gthomas@redhat.com>
 
 	* src/main.c (do_reset): HAL macro name changed.
 
 	* src/net/ping.c (do_ping): Remove warning.
@@ -360,11 +367,11 @@
 	* src/main.c (cyg_start): Change to use struct init_tab_entry per above
 
 2000-09-07 Andrew Lunn  <andrew.lunn@ascom.ch>
 
 	* src/load.c, include/redboot.h: renamed the getc functions to
- 	redboot_getc and made them globel so custom boot loaders can use
+ 	redboot_getc and made them global so custom boot loaders can use
  	them.
 
 2000-09-06  Gary Thomas  <gthomas@redhat.com>
 
 	* doc/users_guide: Add info about region locking/unlocking.
Index: redboot/current/include/net/net.h
===================================================================
RCS file: /home/cvs/ecc/ecc/redboot/current/include/net/net.h,v
retrieving revision 1.6
diff -u -5 -p -r1.6 net.h
--- redboot/current/include/net/net.h	2001/01/22 18:52:20	1.6
+++ redboot/current/include/net/net.h	2001/01/31 11:34:44
@@ -278,11 +278,11 @@ typedef struct _pktbuf {
     word	*buf;
 } pktbuf_t;
 
 
 /* protocol handler */
-typedef void (*pkt_handler_t)(pktbuf_t *pkt);
+typedef void (*pkt_handler_t)(pktbuf_t *pkt, eth_header_t *eth_hdr);
 
 /* ICMP fielder */
 typedef void (*icmp_handler_t)(pktbuf_t *pkt, ip_route_t *src_route);
 
 typedef struct _udp_socket {
@@ -375,12 +375,14 @@ extern void __pktbuf_dump(void);
 
 /*
  * Install handlers for ethernet packets.
  * Returns old handler.
  */
-extern pkt_handler_t __eth_install_handler(int eth_type,
-					   pkt_handler_t handler);
+extern pkt_handler_t __eth_install_listener(int eth_type,
+                                            pkt_handler_t handler);
+extern void __eth_remove_listener(int eth_type);
+
 /*
  * Set tcp port number for debugging. If not called, a default
  * will be selected.
  */
 extern void _bsp_net_set_debug_port(int portnum);
Index: redboot/current/src/net/enet.c
===================================================================
RCS file: /home/cvs/ecc/ecc/redboot/current/src/net/enet.c,v
retrieving revision 1.5
diff -u -5 -p -r1.5 enet.c
--- redboot/current/src/net/enet.c	2000/10/30 13:50:27	1.5
+++ redboot/current/src/net/enet.c	2001/01/31 11:34:44
@@ -21,11 +21,11 @@
 // The Original Code is eCos - Embedded Configurable Operating System,      
 // released September 30, 1998.                                             
 //                                                                          
 // The Initial Developer of the Original Code is Red Hat.                   
 // Portions created by Red Hat are                                          
-// Copyright (C) 1998, 1999, 2000 Red Hat, Inc.                             
+// Copyright (C) 1998, 1999, 2000, 2001 Red Hat, Inc.                             
 // All Rights Reserved.                                                     
 // -------------------------------------------                              
 //                                                                          
 //####COPYRIGHTEND####
 //==========================================================================
@@ -57,19 +57,69 @@ static int num_rarp = 0;
 #endif
 static int num_received = 0;
 static int num_transmitted = 0;
 #endif
 
+//
+// Support for user handlers of additional ethernet packets (nonIP)
+//
+
+#define NUM_EXTRA_HANDLERS 4
+static struct {
+    int type;
+    pkt_handler_t handler;
+} eth_handlers[NUM_EXTRA_HANDLERS];
+
+pkt_handler_t 
+__eth_install_listener(int eth_type, pkt_handler_t handler)
+{
+    int i, empty;
+    pkt_handler_t old;
+
+    empty = -1;
+    for (i = 0;  i < NUM_EXTRA_HANDLERS;  i++) {
+        if (eth_handlers[i].type == eth_type) {
+            // Replace existing handler
+            old = eth_handlers[i].handler;
+            eth_handlers[i].handler = handler;
+            return old;
+        }
+        if (eth_handlers[i].type == 0) {
+            empty = i;
+        }
+    }
+    if (empty >= 0) {
+        // Found a free slot
+        eth_handlers[empty].type = eth_type;
+        eth_handlers[empty].handler = handler;
+    }
+    printf("** Warning: can't install listener for ethernet type 0x%02x\n", eth_type);
+    return (pkt_handler_t)0;
+}
+
+void 
+__eth_remove_listener(int eth_type)
+{
+    int i;
+    
+    for (i = 0;  i < NUM_EXTRA_HANDLERS;  i++) {
+        if (eth_handlers[i].type == eth_type) {
+            eth_handlers[i].type = 0;
+        }
+    }    
+}
+
 /*
  * Non-blocking poll of ethernet link. Process packets until no more
  * are available.
  */
 void
 __enet_poll(void)
 {
     pktbuf_t *pkt;
     eth_header_t eth_hdr;
+    int i, type;
 
     while (true) {
         /*
          * Try to get a free pktbuf and return if none
          * are available.
@@ -84,11 +134,11 @@ __enet_poll(void)
         if ((pkt->pkt_bytes = eth_drv_read((char *)&eth_hdr, (char *)pkt->buf,
                                            ETH_MAX_PKTLEN)) > 0) {
 #if ENET_STATS
             ++num_received;
 #endif
-            switch (ntohs(eth_hdr.type)) {
+            switch (type = ntohs(eth_hdr.type)) {
 
             case ETH_TYPE_IP:
 #if ENET_STATS
                 ++num_ip;
 #endif
@@ -113,10 +163,15 @@ __enet_poll(void)
                 __rarp_handler(pkt);
                 break;
 #endif
 
             default:
+                for (i = 0;  i < NUM_EXTRA_HANDLERS;  i++) {
+                    if (eth_handlers[i].type == type) {
+                        (eth_handlers[i].handler)(pkt, &eth_hdr);
+                    }
+                }
                 __pktbuf_free(pkt);
                 break;
             }
         } else {
             __pktbuf_free(pkt);
@@ -143,11 +198,10 @@ __enet_send(pktbuf_t *pkt, enet_addr_t *
     eth_drv_write((char *)&eth_hdr, (char *)pkt->buf, pkt->pkt_bytes);
 #if ENET_STATS
     ++num_transmitted;
 #endif
 }
-
 
 #ifdef __LITTLE_ENDIAN__
 
 unsigned long  
 ntohl(unsigned long x)