Patchwork [02/19] Cleaning up the IPv6 MTU checking in the IPVS xmit code, by using a common helper function __mtu_check_toobig_v6().

login
register
mail settings
Submitter Patrick McHardy
Date Aug. 28, 2012, 9:48 p.m.
Message ID <1346190539-9963-3-git-send-email-kaber@trash.net>
Download mbox | patch
Permalink /patch/180540/
State Accepted
Headers show

Comments

Patrick McHardy - Aug. 28, 2012, 9:48 p.m.
From: Jesper Dangaard Brouer <brouer@redhat.com>

The MTU check for tunnel mode can also use this helper as
ntohs(old_iph->payload_len) + sizeof(struct ipv6hdr) is qual to
skb->len.  And the 'mtu' variable have been adjusted before
calling helper.

Notice, this also fixes a bug, as the the MTU check in ip_vs_dr_xmit_v6()
were missing a check for skb_is_gso().

This bug e.g. caused issues for KVM IPVS setups, where different
Segmentation Offloading techniques are utilized, between guests,
via the virtio driver.  This resulted in very bad performance,
due to the ICMPv6 "too big" messages didn't affect the sender.

Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
Signed-off-by: Patrick McHardy <kaber@trash.net>
---
 net/netfilter/ipvs/ip_vs_xmit.c |   21 +++++++++++++++------
 1 files changed, 15 insertions(+), 6 deletions(-)
Jesper Dangaard Brouer - Aug. 29, 2012, 8:02 a.m.
Just a little nitpick.

The original title/subj was:
  "ipvs: IPv6 MTU checking cleanup and bugfix"

And the curr/used title/subj were part of the commit text.


On Tue, 28 Aug 2012, Patrick McHardy wrote:

> From: Jesper Dangaard Brouer <brouer@redhat.com>
>
Insert:
  Cleaning up the IPv6 MTU checking in the IPVS xmit code, by using a
  common helper function __mtu_check_toobig_v6().

> The MTU check for tunnel mode can also use this helper as
> ntohs(old_iph->payload_len) + sizeof(struct ipv6hdr) is qual to
> skb->len.  And the 'mtu' variable have been adjusted before
> calling helper.
>
> Notice, this also fixes a bug, as the the MTU check in ip_vs_dr_xmit_v6()
> were missing a check for skb_is_gso().
>
> This bug e.g. caused issues for KVM IPVS setups, where different
> Segmentation Offloading techniques are utilized, between guests,
> via the virtio driver.  This resulted in very bad performance,
> due to the ICMPv6 "too big" messages didn't affect the sender.
>
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> Signed-off-by: Patrick McHardy <kaber@trash.net>
> ---
> net/netfilter/ipvs/ip_vs_xmit.c |   21 +++++++++++++++------
> 1 files changed, 15 insertions(+), 6 deletions(-)

[cut]

Hilsen
   Jesper Brouer

--
-------------------------------------------------------------------
MSc. Master of Computer Science
Dept. of Computer Science, University of Copenhagen
Author of http://www.adsl-optimizer.dk
-------------------------------------------------------------------
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Patrick McHardy - Aug. 29, 2012, 12:24 p.m.
On Wed, 29 Aug 2012, Jesper Dangaard Brouer wrote:

>
> Just a little nitpick.
>
> The original title/subj was:
> "ipvs: IPv6 MTU checking cleanup and bugfix"
>
> And the curr/used title/subj were part of the commit text.

Sorry, must have happened while importing the patch, I noticed it
afterwards, but was already too late.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pablo Neira - Aug. 30, 2012, 1:01 a.m.
On Wed, Aug 29, 2012 at 02:24:39PM +0200, Patrick McHardy wrote:
> On Wed, 29 Aug 2012, Jesper Dangaard Brouer wrote:
> 
> >
> >Just a little nitpick.
> >
> >The original title/subj was:
> >"ipvs: IPv6 MTU checking cleanup and bugfix"
> >
> >And the curr/used title/subj were part of the commit text.
> 
> Sorry, must have happened while importing the patch, I noticed it
> afterwards, but was already too late.

No problem, I'll fix this.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/net/netfilter/ipvs/ip_vs_xmit.c b/net/netfilter/ipvs/ip_vs_xmit.c
index 543a554..67a3978 100644
--- a/net/netfilter/ipvs/ip_vs_xmit.c
+++ b/net/netfilter/ipvs/ip_vs_xmit.c
@@ -85,6 +85,15 @@  __ip_vs_dst_check(struct ip_vs_dest *dest, u32 rtos)
 	return dst;
 }
 
+static inline bool
+__mtu_check_toobig_v6(const struct sk_buff *skb, u32 mtu)
+{
+	if (skb->len > mtu && !skb_is_gso(skb)) {
+		return true; /* Packet size violate MTU size */
+	}
+	return false;
+}
+
 /* Get route to daddr, update *saddr, optionally bind route to saddr */
 static struct rtable *do_output_route4(struct net *net, __be32 daddr,
 				       u32 rtos, int rt_mode, __be32 *saddr)
@@ -491,7 +500,7 @@  ip_vs_bypass_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
 
 	/* MTU checking */
 	mtu = dst_mtu(&rt->dst);
-	if (skb->len > mtu && !skb_is_gso(skb)) {
+	if (__mtu_check_toobig_v6(skb, mtu)) {
 		if (!skb->dev) {
 			struct net *net = dev_net(skb_dst(skb)->dev);
 
@@ -712,7 +721,7 @@  ip_vs_nat_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
 
 	/* MTU checking */
 	mtu = dst_mtu(&rt->dst);
-	if (skb->len > mtu && !skb_is_gso(skb)) {
+	if (__mtu_check_toobig_v6(skb, mtu)) {
 		if (!skb->dev) {
 			struct net *net = dev_net(skb_dst(skb)->dev);
 
@@ -946,8 +955,8 @@  ip_vs_tunnel_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
 	if (skb_dst(skb))
 		skb_dst(skb)->ops->update_pmtu(skb_dst(skb), NULL, skb, mtu);
 
-	if (mtu < ntohs(old_iph->payload_len) + sizeof(struct ipv6hdr) &&
-	    !skb_is_gso(skb)) {
+	/* MTU checking: Notice that 'mtu' have been adjusted before hand */
+	if (__mtu_check_toobig_v6(skb, mtu)) {
 		if (!skb->dev) {
 			struct net *net = dev_net(skb_dst(skb)->dev);
 
@@ -1113,7 +1122,7 @@  ip_vs_dr_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
 
 	/* MTU checking */
 	mtu = dst_mtu(&rt->dst);
-	if (skb->len > mtu) {
+	if (__mtu_check_toobig_v6(skb, mtu)) {
 		if (!skb->dev) {
 			struct net *net = dev_net(skb_dst(skb)->dev);
 
@@ -1349,7 +1358,7 @@  ip_vs_icmp_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
 
 	/* MTU checking */
 	mtu = dst_mtu(&rt->dst);
-	if (skb->len > mtu && !skb_is_gso(skb)) {
+	if (__mtu_check_toobig_v6(skb, mtu)) {
 		if (!skb->dev) {
 			struct net *net = dev_net(skb_dst(skb)->dev);