Mike Pattrick <m...@redhat.com> writes: > On Thu, Jun 5, 2025 at 10:57 AM Aaron Conole <acon...@redhat.com> wrote: >> >> Mike Pattrick via dev <ovs-dev@openvswitch.org> writes: >> >> > Currently conntrack will refuse to extract metadata from fragmented >> > IPv4 packets. Usually the fragments would be processed by the ipf >> > module, but this isn't the case for ICMP related packets. The current >> > handling will result in these being incorrectly processed. >> > >> > This patch checks for a frag offset instead of just frag flags, which is >> > similar to how conntrack handles fragments in the kernel. >> > >> > Reported-at: https://issues.redhat.com/browse/FDP-136 >> > Reported-by: Ales Musil <amu...@redhat.com> >> > Signed-off-by: Mike Pattrick <m...@redhat.com> >> > --- >> > lib/conntrack.c | 2 +- >> > tests/ofproto-dpif.at | 32 ++++++++++++++++++++++++++++++++ >> > 2 files changed, 33 insertions(+), 1 deletion(-) >> > >> > diff --git a/lib/conntrack.c b/lib/conntrack.c >> > index fe1ecc159..b02174ec4 100644 >> > --- a/lib/conntrack.c >> > +++ b/lib/conntrack.c >> > @@ -1705,7 +1705,7 @@ extract_l3_ipv4(struct conn_key *key, const void >> > *data, size_t size, >> > return false; >> > } >> > >> > - if (IP_IS_FRAGMENT(ip->ip_frag_off)) { >> > + if (ip->ip_frag_off & htons(IP_FRAG_OFF_MASK)) { >> >> Stylistic, but we may want to have something like the following >> (untested code) in lib/packets.h:: >> >> #define IP_IS_FIRST_FRAG(ip_frag_off) \ >> (((ip_frag_off) & htons(IP_MORE_FRAGMENTS | IP_FRAG_OFF_MASK)) == \ >> htons(IP_MORE_FRAGMENTS)) >> >> And the update this block to say: >> >> if (!IP_IS_FIRST_FRAG(ip->ip_frag_off)) > > I agree, looks better. > >> >> Because the first frag should be allowed to be processed to extract >> these details. >> >> It may also be a good idea to update extract_l4() to check that we are >> not the first frag and exit there as well (since the l4 data won't be >> available - it can be considered an early shunt). > > Is it possible for a frag to get to extract_l4 without hitting the > check in extract_l3 first?
At present, not that I'm aware. It's fine to also not do it :) > -M > >> >> > return false; >> > } >> > >> > diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at >> > index 3d84618b3..7930b3f29 100644 >> > --- a/tests/ofproto-dpif.at >> > +++ b/tests/ofproto-dpif.at >> > @@ -11597,6 +11597,38 @@ >> > udp,orig=(src=10.1.1.2,dst=10.1.1.1,sport=4,dport=3),reply=(src=10.1.1.1,dst=10. >> > OVS_VSWITCHD_STOP >> > AT_CLEANUP >> > >> > +AT_SETUP([ofproto-dpif - conntrack - related nat]) >> > +OVS_VSWITCHD_START >> > + >> > +add_of_ports --pcap br0 1 2 >> > + >> > +AT_DATA([flows.txt], [dnl >> > +table=0,priority=100,arp,action=normal >> > +table=0,priority=10,ip,in_port=1,udp,action=ct(commit,table=1,nat(dst=1.2.3.4:10000-10000)) >> > +table=0,priority=10,ip,in_port=2,action=ct(nat,table=1) >> > +table=0,priority=1,action=drop >> > +table=1,priority=10,in_port=1,ct_state=+trk,action=2 >> > +table=1,priority=10,in_port=2,ct_state=+trk,action=1 >> > +table=1,priority=1,action=drop >> > +]) >> > + >> > +AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) >> > + >> > +dnl 1. Send and UDP packet to port 5555. >> > +packet1=c6f94ecb72dbe64c473528c9080045000021317040004011b138ac100001ac100002a28e15b3000d20966369616f0a >> > +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 >> > packet=$packet1 actions=resubmit(,0)"]) >> > + >> > +dnl 2. Send an fragmented ICMP related reply from 1.2.3.4:10000. >> > +packet2=e64c473528c9c6f94ecb72db080045c000382e87000040019b6701020304ac1000010303ad2d000000004500001c317020004011794aac10000101020304a28e2710000d8623 >> > +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=2 >> > packet=$packet2 actions=resubmit(,0)"]) >> > + >> > +dnl 3. Check that the inner packet is translated. >> > +packet3=e64c473528c9c6f94ecb72db080045c000382e8700004001f35aac100002ac1000010303553a000000004500001c317020004011d13dac100001ac100002a28e15b3000def73 >> > +OVS_WAIT_UNTIL([ovs-pcap p1-tx.pcap | grep -q "$packet3"]) >> > + >> > +OVS_VSWITCHD_STOP >> > +AT_CLEANUP >> > + >> > AT_SETUP([ofproto-dpif - conntrack - ipv6]) >> > OVS_VSWITCHD_START >> _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
