Re: IPFW2 layer2 filtering broken - PATCH

[Gary Allan <dragonfly@xxxxxxxxxxxxxxx>]

> Matthew Dillon wrote:
>     Well, our m_freem() allows m to be NULL so the NULL check is not
>     necessary.
>
>     From my read of the code, the 'eh = mtod(...)' is necessary, but 
>     most of the time the returned 'm' will be the same as the passed 'm'
>     so I am not surprised that you did not seeh = mtod(m, struct ether_header *);e any difference.
>
>     Your email wasn't quite clear on the point... what change did you make
>     which seemed to fix the problem for you?  It couldn't be the m_freem()
>     change and you seem to indicate that it wasn't the 'eh = mtod...'
>     change either.
>
> 					-Matt
> 					Matthew Dillon 
> 					<dillon@xxxxxxxxxxxxx>

Apologies,

The change was the removal of the "eh = mtod(m, struct ether_header *);" 
line. When present all incoming packets are dropped with sysctl 
net.link.ether.ipfw=1 and ipfw2 loaded.

I don't know if removing the line is the correct solution as I don't 
understand the code well enough but everything appears to be working 
after the change.

Amended diff:

diff -ruN if_ethersubr.c if_ethersubr.c.new
--- if_ethersubr.c      2005-01-24 18:24:07.000000000 +0000
+++ if_ethersubr.c.new  2005-01-24 18:48:56.000000000 +0000
@@ -680,7 +680,6 @@
                        m_freem(m);
                        return;
                }
-               eh = mtod(m, struct ether_header *);
        }

        ether_type = ntohs(eh->ether_type);



I narrowed the symptoms down to the second location where 
ether_ipfw_chk() is invoked and compared the source to FreeBSD RELENG_4. 
Also ether_ipfw_chk is called from another location within 
if_ethersubr.c. Here there is a second mtod() call not present in the 
FreeBSD code. (I don't know if thats a problem or not.)


* $DragonFly: src/sys/net/if_ethersubr.c,v 1.25 2005/01/06 09:14:13 hsu 
Exp $
 */

. ..

if (IPFW_LOADED && ether_ipfw != 0) {
                struct ether_header save_eh, *eh;

                eh = mtod(m, struct ether_header *);
                save_eh = *eh;
                m_adj(m, ETHER_HDR_LEN);
                if (!ether_ipfw_chk(&m, ifp, &rule, eh, FALSE)) {
                        if (m != NULL) {
                                m_freem(m);
                                return ENOBUFS; /* pkt dropped */
                        } else
                                return 0;       /* consumed e.g. in a 
pipe */
                }
                eh = mtod(m, struct ether_header *);
                /* packet was ok, restore the ethernet header */
                if ((void *)(eh + 1) == (void *)m->m_data) {
                        m->m_data -= ETHER_HDR_LEN ;
                        m->m_len += ETHER_HDR_LEN ;
                        m->m_pkthdr.len += ETHER_HDR_LEN ;
                } else {


Regards

G.Allan