On Sun, May 29, 2022 at 10:21 AM Salem Sol via dev <[email protected]> wrote: > > In case of modifying an IPv6 packet src/dst address the checksum should be > recalculated only for the last frag. Currently it's done for all frags, > leading to incorrect reassembled packet checksum. > Fix it by adding a new flag to recalculate the checksum only for last frag. > > Fixes: bc7a5acdff08 ("datapath: add ipv6 'set' action") > Signed-off-by: Salem Sol <[email protected]> > --- > lib/packets.c | 22 ++++++++++++++++++++-- > tests/system-traffic.at | 35 +++++++++++++++++++++++++++++++++++ > 2 files changed, 55 insertions(+), 2 deletions(-) > https://github.com/salemsol/ovs/actions/runs/2404335827 > > diff --git a/lib/packets.c b/lib/packets.c > index d0fba8176..70b0e6ad2 100644 > --- a/lib/packets.c > +++ b/lib/packets.c > @@ -1148,6 +1148,20 @@ packet_set_ipv4_addr(struct dp_packet *packet, > put_16aligned_be32(addr, new_addr); > } > > +static bool > +packet_is_last_ipv6_frag(struct dp_packet *packet) > +{ > + const struct ovs_16aligned_ip6_frag *frag_hdr; > + const struct ovs_16aligned_ip6_hdr *nh; > + uint8_t *data = dp_packet_l3(packet); > + > + nh = ALIGNED_CAST(struct ovs_16aligned_ip6_hdr *, data);
Nit: is the above line needed? Otherwise, looks good to me. Acked-by: Mike Pattrick <[email protected]> Cheers, M > + data += sizeof *nh; > + frag_hdr = ALIGNED_CAST(struct ovs_16aligned_ip6_frag *, data); > + return (frag_hdr->ip6f_offlg & IP6F_OFF_MASK) && > + !(frag_hdr->ip6f_offlg & IP6F_MORE_FRAG); > +} > + > /* Returns true, if packet contains at least one routing header where > * segements_left > 0. > * > @@ -1334,17 +1348,21 @@ packet_set_ipv6(struct dp_packet *packet, const > struct in6_addr *src, > { > struct ovs_16aligned_ip6_hdr *nh = dp_packet_l3(packet); > uint8_t proto = 0; > + bool recalc_csum; > bool rh_present; > > rh_present = packet_rh_present(packet, &proto); > + recalc_csum = nh->ip6_nxt == IPPROTO_FRAGMENT ? > + packet_is_last_ipv6_frag(packet) : true; > > if (memcmp(&nh->ip6_src, src, sizeof(ovs_be32[4]))) { > - packet_set_ipv6_addr(packet, proto, nh->ip6_src.be32, src, true); > + packet_set_ipv6_addr(packet, proto, nh->ip6_src.be32, > + src, recalc_csum); > } > > if (memcmp(&nh->ip6_dst, dst, sizeof(ovs_be32[4]))) { > packet_set_ipv6_addr(packet, proto, nh->ip6_dst.be32, dst, > - !rh_present); > + !rh_present && recalc_csum); > } > > packet_set_ipv6_tc(&nh->ip6_flow, key_tc); > diff --git a/tests/system-traffic.at b/tests/system-traffic.at > index 239105e89..16ba42d84 100644 > --- a/tests/system-traffic.at > +++ b/tests/system-traffic.at > @@ -192,6 +192,41 @@ NS_CHECK_EXEC([at_ns0], [ping6 -s 3200 -q -c 3 -i 0.3 -w > 2 fc00:1::2 | FORMAT_PI > OVS_TRAFFIC_VSWITCHD_STOP > AT_CLEANUP > > +AT_SETUP([datapath - ping6 between two ports with header modify]) > +OVS_TRAFFIC_VSWITCHD_START() > + > +AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"]) > + > +ADD_NAMESPACES(at_ns0, at_ns1) > + > +ADD_VETH(p0, at_ns0, br0, "fc00::1/96", e4:11:22:33:44:55) > +ADD_VETH(p1, at_ns1, br0, "fc00::2/96", e4:11:22:33:44:54) > +NS_CHECK_EXEC([at_ns0], [ip -6 neigh add fc00::3 lladdr e4:11:22:33:44:54 > dev p0]) > +NS_CHECK_EXEC([at_ns0], [ip -6 neigh add fc00::2 lladdr e4:11:22:33:44:54 > dev p0]) > +NS_CHECK_EXEC([at_ns0], [ip -6 neigh add fc00::1 lladdr e4:11:22:33:44:55 > dev p0]) > + > +dnl Linux seems to take a little time to get its IPv6 stack in order. Without > +dnl waiting, we get occasional failures due to the following error: > +dnl "connect: Cannot assign requested address" > +OVS_WAIT_UNTIL([ip netns exec at_ns0 ping6 -c 1 fc00::2]) > + > +AT_CHECK([ovs-ofctl del-flows -OOpenFlow15 br0], [], [stdout], [stderr]) > +AT_CHECK([ovs-ofctl add-flow -OOpenFlow15 br0 > in_port=ovs-p0,ipv6,ipv6_dst=fc00::3,ipv6_src=fc00::1,actions=set_field:fc00::2-\>ipv6_dst,ovs-p1], > [], [stdout], [stderr]) > +AT_CHECK([ovs-ofctl add-flow -OOpenFlow15 br0 > in_port=ovs-p1,ipv6,ipv6_dst=fc00::1,ipv6_src=fc00::2,actions=set_field:fc00::3-\>ipv6_src,ovs-p0], > [], [stdout], [stderr]) > + > +NS_CHECK_EXEC([at_ns0], [ping6 -q -c 3 -i 0.3 -w 2 fc00::3 | FORMAT_PING], > [0], [dnl > +3 packets transmitted, 3 received, 0% packet loss, time 0ms > +]) > +NS_CHECK_EXEC([at_ns0], [ping6 -s 1600 -q -c 3 -i 0.3 -w 2 fc00::3 | > FORMAT_PING], [0], [dnl > +3 packets transmitted, 3 received, 0% packet loss, time 0ms > +]) > +NS_CHECK_EXEC([at_ns0], [ping6 -s 3200 -q -c 3 -i 0.3 -w 2 fc00::3 | > FORMAT_PING], [0], [dnl > +3 packets transmitted, 3 received, 0% packet loss, time 0ms > +]) > + > +OVS_TRAFFIC_VSWITCHD_STOP > +AT_CLEANUP > + > AT_SETUP([datapath - ping over bond]) > OVS_TRAFFIC_VSWITCHD_START() > > -- > 2.21.0 > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
