Re: [RFC PATCH] EtherIP tunnel driver (RFC 3387)

[Patrick McHardy <kaber@trash.net>]

Joerg Roedel wrote:
> On Fri, Sep 01, 2006 at 06:24:56PM +0200, Patrick McHardy wrote:
>>
>>>+	fl.oif = 0;
>>>+	fl.nl_u.ip4_u.daddr = tunnel->parms.iph.daddr;
>>>+	fl.nl_u.ip4_u.saddr = tunnel->parms.iph.saddr;
>>>+	fl.nl_u.ip4_u.saddr = 0;
>>>+	fl.proto = IPPROTO_ETHERIP;
>>
>>This leaves fields like nfmark or iif uninitialized.
> 
> 
> I changed the code to initialize iif to 0, but I am not sure how to
> initialize the fwmark field. Neigther the ipip driver nor the sit driver
> initialize this field. Is it right to initialize it to 0?

They both initialize the entire structure at once, which sets members
not explicitly initialized to zero.

>>>+		goto accept;
>>>+	
>>>+	if ((skb->pkt_type == PACKET_OTHERHOST) && (dev->flags & IFF_PROMISC))
>>>+		goto accept;
>>
>>
>>Why would you want to receive packets for other hosts picked up in
>>promiscous mode?
> 
> 
> This is because the device should be usable in a bridge to build a
> distributed layer 2 network. Furthermore, this could be usefull when I
> extend the driver to support multicast destinations.

Yes, I totally misunderstood the intention.

>>>+static void etherip_err(struct sk_buff *skb, u32 info)
>>>+{
>>
>>
>>You could propagte errors from destination unreachable messages
>>similar to what the IPIP driver does.
> 
> 
> I am not sure that this makes sense here. The IPIP driver handles
> ICMP_FRAG_NEEDED messages. But I think those messages will never be
> received by the EtherIP driver because it never sets the DF flag in the
> outgoing packets (to ensure the Ethernet MTU for protocols besides IP
> and IPv6). I added a comment about this issue to the etherip_tunnel_xmit
> function.

Thanks for the explanation, I didn't notice that it doesn't set DF.

-- 
VGER BF report: U 0.505164