Re: [Ebtables-devel] [PATCH] bridge-nf: Allow IPTables to filter PPPoE traffic

[Bart De S. <bds...@pa...>]

Op do, 01-03-2007 te 14:43 -0500, schreef Michael Milner:
> > Op ma, 05-02-2007 te 16:48 -0500, schreef Michael Milner:
> >> Hi,
> >>
> >> I've put together some code to allow IPTables to filter PPPoE traffic in
> >> the same way that bridge-nf allows iptables to filter VLAN traffic.
> >> [snip]
> >> The patch is against 2.6.15.  Comments appreciated.  It works fine for
> >> me
> >> but I wanted some input before submitting it "officially".
> >
> > Please update this to 2.6.20. More comments are below.
> >
> > Thanks,
> > Bart
> >
> 
> I've made the changes you requested.  This patch is now based on 2.6.20.1.
>  I added a defined constant into if_pppoe.h.

Hi,

Thanks for the update, it looks good. I don't want to be annoying, but
still a few remarks:
- arp encapsulated in pppoe should be implemented too (this is used with
pppoe, right?), sorry that I didn't mention that in my previous answer
- nf_bridge_pad() should be implemented with only one return. The kernel
maintainers like code like this:
ret = 0;

if (skb->nf_bridge && skb->protocol == htons(ETH_P_8021Q))
	ret =  VLAN_HLEN;
else if (skb->nf_bridge && skb->protocol == htons(ETH_P_PPP_SES))
	ret = PPPOE_SES_HLEN;
return ret;

Apart from that it looks great.

cheers,
Bart