Re: batman-adv: design suggestions

[Sven Eckelmann]

Vasiliy Kulikov wrote:
> On Tue, Aug 10, 2010 at 00:34 +0400, Vasiliy Kulikov wrote:
> > 2) It seems to me that NF_HOOK() at hard-interface.c:458 is misused:
> >     ...
> > 	
> > 	ret = NF_HOOK(PF_BRIDGE, NF_BR_LOCAL_IN, skb, dev, NULL,
> > 	
> > 		      batman_skb_recv_finish);
> > 	
> > 	if (ret != 1)
> > 	
> > 		goto err_out;
> > 	
> > 	/* packet should hold at least type and version */
> > 	if (unlikely(skb_headlen(skb) < 2))
> > 	
> > 		goto err_free;
> > 	
> > 	/* expect a valid ethernet header here. */
> > 	if (unlikely(skb->mac_len != sizeof(struct ethhdr)
> > 	
> > 				|| !skb_mac_header(skb)))
> > 		
> > 		goto err_free;
> > 		
> >     ...
> >     
> >     static int batman_skb_recv_finish(struct sk_buff *skb)
> >     {
> >     
> >         return NF_ACCEPT;
> >     
> >     }
> >   
> >   As I understand, if there is any hook that returns NF_STOLEN, then skb
> >   is leaked.
> 
[...]

> b) Why do you use bridge tables at all? This layer does not know
> anything about batman layer, only ethernet that is only a tunnel for
> batman. So, it is able to hook traffic from concrete prev-hop routers,
> but not from original sources of packets. I think it is not enough for
> network filter.
>    Also if you want to process [*] cases you have to append fake
> ethernet headers before network header as NF_HOOK() would use ethernet
> header.

Because a different person (no one from the actual development team) wanted to 
have it for testing purposes. Maybe we just drop it again.

thanks,
	Sven

Attachment: signature.asc
Description: This is a digitally signed message part.