On Mon, Nov 5, 2012 at 1:13 PM, Ansis Atteka <aatt...@nicira.com> wrote:
> On Mon, Nov 5, 2012 at 8:58 PM, Jesse Gross <je...@nicira.com> wrote:
> > On Sun, Nov 4, 2012 at 7:44 AM, Ansis Atteka <aatt...@nicira.com> wrote:
> >> On Tue, Oct 30, 2012 at 1:11 AM, Jesse Gross <je...@nicira.com> wrote:
> >>> On Sun, Oct 28, 2012 at 2:21 PM, Ansis Atteka <aatt...@nicira.com>
> wrote:
> >>>> diff --git a/datapath/actions.c b/datapath/actions.c
> >>>> index ec9b595..7e60b0f 100644
> >>>> --- a/datapath/actions.c
> >>>> +++ b/datapath/actions.c
> >>>> +static void set_ipv6_addr(struct sk_buff *skb, struct ipv6hdr *nh,
> >>>> + __be32 addr[4], const __be32 new_addr[4])
> >>>> +{
> >>>> + int transport_len = skb->len - skb_transport_offset(skb);
> >>>> +
> >>>> + if (nh->nexthdr == IPPROTO_TCP) {
> >>>> + if (likely(transport_len >= sizeof(struct tcphdr)))
> >>>> +
> inet_proto_csum_replace16(&tcp_hdr(skb)->check, skb,
> >>>> + addr, new_addr, 1);
> >>>> + } else if (nh->nexthdr == IPPROTO_UDP) {
> >>>> + if (likely(transport_len >= sizeof(struct udphdr))) {
> >>>> + struct udphdr *uh = udp_hdr(skb);
> >>>> +
> >>>> + if (uh->check ||
> >>>> + get_ip_summed(skb) == OVS_CSUM_PARTIAL) {
> >>>> + inet_proto_csum_replace16(&uh->check,
> skb,
> >>>> + addr,
> new_addr, 1);
> >>>> + if (!uh->check)
> >>>> + uh->check = CSUM_MANGLED_0;
> >>>> + }
> >>>> + }
> >>>> + }
> >>>
> >>> IPv6 has an extra little twist when it comes to checksums compared to
> >>> IPv4: routing headers. In an IPv6 packet, the destination IP address
> >>> used for the purposes of checksum calculation in the pseudo header is
> >>> the final destination address. This means that if you have a routing
> >>> header then it is that address that matters and not one in the IP
> >>> header. This is likely to be a pretty rare case but we should handle
> >>> it.
> >> There could be a chain of ipv6 extension headers. And the Routing
> >> Header could be in the middle of that chain. Can you confirm, if
> >> ipv6_find_hdr() function is good enough to get the routing header?
> >
> > Yes, it seems to be basically fine. Two things that come to mind are:
> > what happens if there are multiple routing headers (is this legal?)
> RFC3542 has implication "When multiple Routing headers are received
> ...". This makes me think that multiple routing headers are legal.
>
> This means that L4 checksum shouldn't be updated, if there exists at
> least one routing header, where segments_left > 0. Otherwise, we have
> to update L4 checksums.
>
That sounds right to me.
> > and ipv6_find_hdr() is in the ip6_tables module so using it will
> > create a dependency, which we don't want. We probably should move it
> > somewhere else.
> This ipv6_find_hdr() function seems to be able to handle only single
> routing header. Perhaps I will write my own function that will iterate
> over all extension headers and tell the caller, whether the
> ipv6hdr->daddr is the final destination address.
That sounds fine although if possible we should try to modify the existing
function to do what we want rather than adding a new one.
> >
> > You might want to look to see how IPv6 NAT handles this case currently.
> Either I am overlooking that IPv6 NAT code or it does not handle
> routing headers and checksums as I would have expected.
>
> The nf_nat_ipv6_csum_update() is called, for example, from
> tcp_manip_pkt(). And it seems that it always updates L4 checksums
> without taking into account routing headers. Will check it tomorrow
> morning one more time.
I agree, it looks like those functions don't handle this case. We should
still try to get it right regardless though.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
