Message ID | 20120827144059.5754.55303.stgit@dragon |
---|---|
State | Not Applicable |
Headers | show |
Hello, On Mon, 27 Aug 2012, Jesper Dangaard Brouer wrote: > This patch is necessary, to make IPVS work, after Patrick McHardys > IPv6 NAT defragmentation changes. > > Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com> > --- > > I would appriciate, if someone (e.g. Julian) double check the tunnel mode code. Sure > --Jesper > > net/netfilter/ipvs/ip_vs_xmit.c | 16 +++++++++++++--- > 1 files changed, 13 insertions(+), 3 deletions(-) > > @@ -956,8 +963,11 @@ ip_vs_tunnel_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp, > skb_dst(skb)->ops->update_pmtu(skb_dst(skb), NULL, skb, mtu); > > /* MTU checking: Special for tunnel mode */ > - if (mtu < ntohs(old_iph->payload_len) + sizeof(struct ipv6hdr) && > - !skb_is_gso(skb)) { > + if ((!IP6CB(skb)->frag_max_size && > + (mtu < ntohs(old_iph->payload_len) + sizeof(struct ipv6hdr) && > + !skb_is_gso(skb))) > + || IP6CB(skb)->frag_max_size + sizeof(struct ipv6hdr) > mtu) { > + mtu is already reduced with the new outer header size, may be we can just call __mtu_check_toobig_v6 with mtu? > if (!skb->dev) { > struct net *net = dev_net(skb_dst(skb)->dev); All other changes in patch 1 and 2 look ok and I'll ack them next time. Regards -- Julian Anastasov <ja@ssi.bg> -- 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
On Mon, 27 Aug 2012, Julian Anastasov wrote: > > Hello, > > On Mon, 27 Aug 2012, Jesper Dangaard Brouer wrote: > >> This patch is necessary, to make IPVS work, after Patrick McHardys >> IPv6 NAT defragmentation changes. >> >> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com> >> --- >> >> I would appriciate, if someone (e.g. Julian) double check the tunnel mode code. > > Sure > >> --Jesper >> >> net/netfilter/ipvs/ip_vs_xmit.c | 16 +++++++++++++--- >> 1 files changed, 13 insertions(+), 3 deletions(-) >> >> @@ -956,8 +963,11 @@ ip_vs_tunnel_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp, >> skb_dst(skb)->ops->update_pmtu(skb_dst(skb), NULL, skb, mtu); >> >> /* MTU checking: Special for tunnel mode */ >> - if (mtu < ntohs(old_iph->payload_len) + sizeof(struct ipv6hdr) && >> - !skb_is_gso(skb)) { >> + if ((!IP6CB(skb)->frag_max_size && >> + (mtu < ntohs(old_iph->payload_len) + sizeof(struct ipv6hdr) && >> + !skb_is_gso(skb))) >> + || IP6CB(skb)->frag_max_size + sizeof(struct ipv6hdr) > mtu) { >> + > > mtu is already reduced with the new outer header size, > may be we can just call __mtu_check_toobig_v6 with mtu? > >> if (!skb->dev) { >> struct net *net = dev_net(skb_dst(skb)->dev); > > All other changes in patch 1 and 2 look ok and > I'll ack them next time. I'd like to take the final patches through my nat-ipv6 tree in order to avoid temporary regressions. Please let me know when they're ready for applying. -- 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
On Tue, Aug 28, 2012 at 10:22:05AM +0200, Patrick McHardy wrote: > On Mon, 27 Aug 2012, Julian Anastasov wrote: > > > > > Hello, > > > >On Mon, 27 Aug 2012, Jesper Dangaard Brouer wrote: > > > >>This patch is necessary, to make IPVS work, after Patrick McHardys > >>IPv6 NAT defragmentation changes. > >> > >>Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com> > >>--- > >> > >>I would appriciate, if someone (e.g. Julian) double check the tunnel mode code. > > > > Sure > > > >>--Jesper > >> > >> net/netfilter/ipvs/ip_vs_xmit.c | 16 +++++++++++++--- > >> 1 files changed, 13 insertions(+), 3 deletions(-) > >> > >>@@ -956,8 +963,11 @@ ip_vs_tunnel_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp, > >> skb_dst(skb)->ops->update_pmtu(skb_dst(skb), NULL, skb, mtu); > >> > >> /* MTU checking: Special for tunnel mode */ > >>- if (mtu < ntohs(old_iph->payload_len) + sizeof(struct ipv6hdr) && > >>- !skb_is_gso(skb)) { > >>+ if ((!IP6CB(skb)->frag_max_size && > >>+ (mtu < ntohs(old_iph->payload_len) + sizeof(struct ipv6hdr) && > >>+ !skb_is_gso(skb))) > >>+ || IP6CB(skb)->frag_max_size + sizeof(struct ipv6hdr) > mtu) { > >>+ > > > > mtu is already reduced with the new outer header size, > >may be we can just call __mtu_check_toobig_v6 with mtu? > > > >> if (!skb->dev) { > >> struct net *net = dev_net(skb_dst(skb)->dev); > > > > All other changes in patch 1 and 2 look ok and > >I'll ack them next time. > > I'd like to take the final patches through my nat-ipv6 tree in order > to avoid temporary regressions. Please let me know when they're ready > for applying. I'm fine with these changes going through your tree (rather than my IPVS tree) once they are ready. -- 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
On Mon, 2012-08-27 at 18:20 +0300, Julian Anastasov wrote: > Hello, > > On Mon, 27 Aug 2012, Jesper Dangaard Brouer wrote: > > > This patch is necessary, to make IPVS work, after Patrick McHardys > > IPv6 NAT defragmentation changes. > > > > Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com> > > --- > > > > I would appriciate, if someone (e.g. Julian) double check the tunnel mode code. > > Sure > > > --Jesper > > > > net/netfilter/ipvs/ip_vs_xmit.c | 16 +++++++++++++--- > > 1 files changed, 13 insertions(+), 3 deletions(-) > > > > @@ -956,8 +963,11 @@ ip_vs_tunnel_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp, > > skb_dst(skb)->ops->update_pmtu(skb_dst(skb), NULL, skb, mtu); > > > > /* MTU checking: Special for tunnel mode */ > > - if (mtu < ntohs(old_iph->payload_len) + sizeof(struct ipv6hdr) && I guess: ntohs(old_iph->payload_len) + sizeof(struct ipv6hdr) Is the same as: skb->len > > - !skb_is_gso(skb)) { > > + if ((!IP6CB(skb)->frag_max_size && > > + (mtu < ntohs(old_iph->payload_len) + sizeof(struct ipv6hdr) && > > + !skb_is_gso(skb))) > > + || IP6CB(skb)->frag_max_size + sizeof(struct ipv6hdr) > mtu) { > > mtu is already reduced with the new outer header size, > may be we can just call __mtu_check_toobig_v6 with mtu? To Julian, is the extra sizeof(struct ipv6hdr) addition to frag_max_size, wrong? (as the mtu is already reduced) If above statements hold, I think we can simply use __mtu_check_toobig_v6() also for the tunnel case :-) --Jesper -- 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
Hello, On Tue, 28 Aug 2012, Jesper Dangaard Brouer wrote: > On Mon, 2012-08-27 at 18:20 +0300, Julian Anastasov wrote: > > > > > @@ -956,8 +963,11 @@ ip_vs_tunnel_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp, > > > skb_dst(skb)->ops->update_pmtu(skb_dst(skb), NULL, skb, mtu); > > > > > > /* MTU checking: Special for tunnel mode */ > > > - if (mtu < ntohs(old_iph->payload_len) + sizeof(struct ipv6hdr) && > > I guess: > ntohs(old_iph->payload_len) + sizeof(struct ipv6hdr) > Is the same as: > skb->len I think so. You can think of this in different way: all transmitters are called from same place, there is no difference in the packets we see. When we can use __mtu_check_toobig_v6 for other methods relying on skb->len being correct, we can do the same for tunnels, only that tunnels have lower MTU, that is the only difference. > > > - !skb_is_gso(skb)) { > > > + if ((!IP6CB(skb)->frag_max_size && > > > + (mtu < ntohs(old_iph->payload_len) + sizeof(struct ipv6hdr) && > > > + !skb_is_gso(skb))) > > > + || IP6CB(skb)->frag_max_size + sizeof(struct ipv6hdr) > mtu) { > > > > > mtu is already reduced with the new outer header size, > > may be we can just call __mtu_check_toobig_v6 with mtu? > > To Julian, is the extra sizeof(struct ipv6hdr) addition to > frag_max_size, wrong? (as the mtu is already reduced) Yes, sizeof(struct ipv6hdr) is needed only together with payload_len because payload_len does not include the first header. > If above statements hold, I think we can simply use > __mtu_check_toobig_v6() also for the tunnel case :-) Yep > --Jesper Regards -- Julian Anastasov <ja@ssi.bg> -- 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
The following patchset V2 makes IPVS compatible with Patrick McHardys IPv6 NAT patchset: http://thread.gmane.org/gmane.linux.network/239615 Specifically the part that improves defragmentation. Patch01: Cleanup (and fixes a bug) in IPVS MTU IPv6 handling In V2: we also handle tunnel mode, to use the common helper Patch02: Extend MTU check to account for IPv6 NAT defrag changes In V2: the tunnel mode is no longer a special case. This patchset is based upon: Homes ipvs-next tree: git://git.kernel.org/pub/scm/linux/kernel/git/horms/ipvs-next.git BUT with Patrick McHardy's IPv6 NAT patchset applied first, on top of commit: 3654e61137db891f5312e6dd813b961484b5fdf3 title: "ipvs: add pmtu_disc option to disable IP DF for TUN packets" --- Jesper Dangaard Brouer (2): ipvs: Extend MTU check to account for IPv6 NAT defrag changes ipvs: IPv6 MTU checking cleanup and bugfix net/netfilter/ipvs/ip_vs_xmit.c | 28 ++++++++++++++++++++++------ 1 files changed, 22 insertions(+), 6 deletions(-) -- Best regards, Jesper Dangaard Brouer MSc.CS, Sr. Network Kernel Developer at Red Hat Author of http://www.iptv-analyzer.org LinkedIn: http://www.linkedin.com/in/brouer -- 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
diff --git a/net/netfilter/ipvs/ip_vs_xmit.c b/net/netfilter/ipvs/ip_vs_xmit.c index e56f0df..20763de 100644 --- a/net/netfilter/ipvs/ip_vs_xmit.c +++ b/net/netfilter/ipvs/ip_vs_xmit.c @@ -88,7 +88,14 @@ __ip_vs_dst_check(struct ip_vs_dest *dest, u32 rtos) static inline bool __mtu_check_toobig_v6(const struct sk_buff *skb, u32 mtu) { - if (skb->len > mtu && !skb_is_gso(skb)) { + if (IP6CB(skb)->frag_max_size) { + /* frag_max_size tell us that, this packet have been + * defragmented by netfilter IPv6 conntrack module. + */ + if (IP6CB(skb)->frag_max_size > mtu) + return true; /* largest fragment violate MTU */ + } + else if (skb->len > mtu && !skb_is_gso(skb)) { return true; /* Packet size violate MTU size */ } return false; @@ -956,8 +963,11 @@ ip_vs_tunnel_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp, skb_dst(skb)->ops->update_pmtu(skb_dst(skb), NULL, skb, mtu); /* MTU checking: Special for tunnel mode */ - if (mtu < ntohs(old_iph->payload_len) + sizeof(struct ipv6hdr) && - !skb_is_gso(skb)) { + if ((!IP6CB(skb)->frag_max_size && + (mtu < ntohs(old_iph->payload_len) + sizeof(struct ipv6hdr) && + !skb_is_gso(skb))) + || IP6CB(skb)->frag_max_size + sizeof(struct ipv6hdr) > mtu) { + if (!skb->dev) { struct net *net = dev_net(skb_dst(skb)->dev);
This patch is necessary, to make IPVS work, after Patrick McHardys IPv6 NAT defragmentation changes. Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com> --- I would appriciate, if someone (e.g. Julian) double check the tunnel mode code. --Jesper net/netfilter/ipvs/ip_vs_xmit.c | 16 +++++++++++++--- 1 files changed, 13 insertions(+), 3 deletions(-) -- 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