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? -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
