On Mon, Feb 06, 2023 at 12:46:10PM +0100, Ales Musil wrote: > The inner header was not handled properly. > Simplify the code which allows proper handling > of the inner headers. > > Reported-at: https://bugzilla.redhat.com/2137754 > Signed-off-by: Ales Musil <[email protected]>
Nice clean-up too :) Reviewed-by: Simon Horman <[email protected]> > diff --git a/lib/conntrack.c b/lib/conntrack.c > index 550b2be9b..3162924ca 100644 > --- a/lib/conntrack.c > +++ b/lib/conntrack.c ... > static void > -reverse_nat_packet(struct dp_packet *pkt, const struct conn *conn) > +nat_inner_packet(struct dp_packet *pkt, struct conn_key *key, > + uint16_t nat_action) > { > char *tail = dp_packet_tail(pkt); > uint16_t pad = dp_packet_l2_pad_size(pkt); > @@ -875,98 +827,77 @@ reverse_nat_packet(struct dp_packet *pkt, const struct > conn *conn) > uint16_t orig_l3_ofs = pkt->l3_ofs; > uint16_t orig_l4_ofs = pkt->l4_ofs; > > - if (conn->key.dl_type == htons(ETH_TYPE_IP)) { > - struct ip_header *nh = dp_packet_l3(pkt); > - struct icmp_header *icmp = dp_packet_l4(pkt); > - struct ip_header *inner_l3 = (struct ip_header *) (icmp + 1); > - /* This call is already verified to succeed during the code path from > - * 'conn_key_extract()' which calls 'extract_l4_icmp()'. */ > - extract_l3_ipv4(&inner_key, inner_l3, tail - ((char *)inner_l3) - > pad, > + void *l3 = dp_packet_l3(pkt); > + void *l4 = dp_packet_l4(pkt); > + void *inner_l3; > + /* These calls are already verified to succeed during the code path from > + * 'conn_key_extract()' which calls > + * 'extract_l4_icmp()'/'extract_l4_icmp6()'. */ > + if (key->dl_type == htons(ETH_TYPE_IP)) { > + inner_l3 = (char *) l4 + sizeof(struct icmp_header); > + extract_l3_ipv4(&inner_key, inner_l3, tail - ((char *) inner_l3) - > pad, > &inner_l4, false); > - pkt->l3_ofs += (char *) inner_l3 - (char *) nh; > - pkt->l4_ofs += inner_l4 - (char *) icmp; > + } else { > + inner_l3 = (char *) l4 + sizeof(struct icmp6_data_header); > + extract_l3_ipv6(&inner_key, inner_l3, tail - ((char *) inner_l3) - > pad, > + &inner_l4); > + } > + pkt->l3_ofs += (char *) inner_l3 - (char *) l3; > + pkt->l4_ofs += inner_l4 - (char *) l4; > > - if (conn->nat_action & NAT_ACTION_SRC) { > - packet_set_ipv4_addr(pkt, &inner_l3->ip_src, > - conn->key.src.addr.ipv4); > - } else if (conn->nat_action & NAT_ACTION_DST) { > - packet_set_ipv4_addr(pkt, &inner_l3->ip_dst, > - conn->key.dst.addr.ipv4); > - } > + /* Reverse the key for inner packet. */ > + struct conn_key rev_key = *key; > + conn_key_reverse(&rev_key); > + > + pat_packet(pkt, &rev_key); > + > + if (key->dl_type == htons(ETH_TYPE_IP)) { > + nat_packet_ipv4(pkt, &rev_key, nat_action); > > - reverse_pat_packet(pkt, conn); > + struct icmp_header *icmp = (struct icmp_header *) l4; > icmp->icmp_csum = 0; nit: not for this patch, but is the above line necessary? > icmp->icmp_csum = csum(icmp, tail - (char *) icmp - pad); > } else { > - struct ovs_16aligned_ip6_hdr *nh6 = dp_packet_l3(pkt); > - struct icmp6_data_header *icmp6 = dp_packet_l4(pkt); > - struct ovs_16aligned_ip6_hdr *inner_l3_6 = > - (struct ovs_16aligned_ip6_hdr *) (icmp6 + 1); > - /* This call is already verified to succeed during the code path from > - * 'conn_key_extract()' which calls 'extract_l4_icmp6()'. */ > - extract_l3_ipv6(&inner_key, inner_l3_6, > - tail - ((char *)inner_l3_6) - pad, > - &inner_l4); > - pkt->l3_ofs += (char *) inner_l3_6 - (char *) nh6; > - pkt->l4_ofs += inner_l4 - (char *) icmp6; > - > - if (conn->nat_action & NAT_ACTION_SRC) { > - packet_set_ipv6_addr(pkt, conn->key.nw_proto, > - inner_l3_6->ip6_src.be32, > - &conn->key.src.addr.ipv6, true); > - } else if (conn->nat_action & NAT_ACTION_DST) { > - packet_set_ipv6_addr(pkt, conn->key.nw_proto, > - inner_l3_6->ip6_dst.be32, > - &conn->key.dst.addr.ipv6, true); > - } > - reverse_pat_packet(pkt, conn); > + nat_packet_ipv6(pkt, &rev_key, nat_action); > + > + struct icmp6_data_header *icmp6 = (struct icmp6_data_header *) l4; > icmp6->icmp6_base.icmp6_cksum = 0; ditto. > - icmp6->icmp6_base.icmp6_cksum = packet_csum_upperlayer6(nh6, icmp6, > - IPPROTO_ICMPV6, tail - (char *) icmp6 - pad); > + icmp6->icmp6_base.icmp6_cksum = > + packet_csum_upperlayer6(l3, icmp6, IPPROTO_ICMPV6, > + tail - (char *) icmp6 - pad); > } > + > pkt->l3_ofs = orig_l3_ofs; > pkt->l4_ofs = orig_l4_ofs; > } _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
