LVS
lvs-devel
Google
 
Web LinuxVirtualServer.org

Re: [PATCH] IPVS: Add handling of incoming ICMPV6_PKT_TOOBIG messages

To: Simon Horman <horms@xxxxxxxxxxxx>
Subject: Re: [PATCH] IPVS: Add handling of incoming ICMPV6_PKT_TOOBIG messages
Cc: lvs-devel@xxxxxxxxxxxxxxx, netdev@xxxxxxxxxxxxxxx, robert.gallagher@xxxxxxxxx
From: Julius Volz <julius.volz@xxxxxxxxx>
Date: Thu, 2 Jul 2009 16:43:39 +0200
Hi Simon,

On Sun, Jun 28, 2009 at 5:43 PM, Simon Horman<horms@xxxxxxxxxxxx> wrote:
> On Wed, Jun 24, 2009 at 03:22:32PM +0200, Julius Volz wrote:
>> Add handling of incoming ICMPv6 Packet Too Big messages. This message
>> is received when a realserver sends a packet >PMTU to the client. The
>> hop on this path with insufficient MTU will generate an ICMPv6 Packet
>> Too Big message back to the VIP. The LVS server receives this message,
>> but the call to the function handling this has been missing. Thus, IPVS
>> fails to forward the message to the real server, which then does not
>> adjust the path MTU. This patch adds the missing call to
>> ip_vs_in_icmp_v6() in ip_vs_in() to handle this situation.
>>
>> Thanks to Rob Gallagher from HEAnet for reporting this issue and for
>> testing this patch in production (with direct routing mode).
>>
>> Signed-off-by: Julius Volz <julius.volz@xxxxxxxxx>
>> Tested-by: Rob Gallagher <robert.gallagher@xxxxxxxxx>
>
> Hi Julius, Hi Rob,
>
> this seems reasonable to me, although it seems that the following
> code is common. I wonder if its repetition could be removed.
>
>                        if (related)
>                                return verdict;
>                        ip_vs_fill_iphdr(af, skb_network_header(skb), &iph);

I agree, though I see no "nice" way to remove this duplication
considering the ifs and #ifdefs around this. You could move the
related and verdict variables to the top of the function and then
recheck afterwards whether one of these ICMP-handling branches was
entered and put the common code in there. But this seems more
cumbersome to me than repeating the code. Maybe you see a nicer way?
Btw., exactly this structure already exists in ip_vs_out(), which is
why I adopted it like this for ip_vs_in().

Cheers,
Julius
--
To unsubscribe from this list: send the line "unsubscribe lvs-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

<Prev in Thread] Current Thread [Next in Thread>