| Message ID | 20250605174134.1149214-1-mkp@redhat.com |
|---|---|
| State | Accepted |
| Commit | 614029aac052946b14ba73ea6e942688900506a3 |
| Delegated to: | aaron conole |
| Headers | show |
| Series | [ovs-dev,v2] conntrack: Allow inner NAT of related fragments. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/cirrus-robot | success | cirrus build: passed |
| ovsrobot/github-robot-_Build_and_Test | fail | github build: failed |
| ovsrobot/github-robot-_Build_and_Test | fail | github build: failed |
On Thu, Jun 5, 2025 at 1:41 PM Mike Pattrick <mkp@redhat.com> wrote: > > 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 <amusil@redhat.com> > Fixes: a489b16854b5 ("conntrack: New userspace connection tracker.") > Signed-off-by: Mike Pattrick <mkp@redhat.com> > --- Recheck-request: github-robot
Mike Pattrick <mkp@redhat.com> 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 <amusil@redhat.com> > Fixes: a489b16854b5 ("conntrack: New userspace connection tracker.") > Signed-off-by: Mike Pattrick <mkp@redhat.com> > --- I did manually restart the github run because it failed again (one flaky test and also the distclean race). That was successful.
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 <amusil@redhat.com> > Fixes: a489b16854b5 ("conntrack: New userspace connection tracker.") > Signed-off-by: Mike Pattrick <mkp@redhat.com> > --- Thanks, applied and backported.
diff --git a/lib/conntrack.c b/lib/conntrack.c index fe1ecc159..c9b186af4 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_IS_LATER_FRAG(ip->ip_frag_off)) { return false; } diff --git a/lib/packets.h b/lib/packets.h index a102f8163..d687fa999 100644 --- a/lib/packets.h +++ b/lib/packets.h @@ -736,6 +736,8 @@ IP_ECN_is_ce(uint8_t dsfield) #define IP_FRAG_OFF_MASK 0x1fff /* Fragment offset. */ #define IP_IS_FRAGMENT(ip_frag_off) \ ((ip_frag_off) & htons(IP_MORE_FRAGMENTS | IP_FRAG_OFF_MASK)) +#define IP_IS_LATER_FRAG(ip_frag_off) \ + ((ip_frag_off) & htons(IP_FRAG_OFF_MASK)) #define IP_HEADER_LEN 20 struct ip_header { 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
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 <amusil@redhat.com> Fixes: a489b16854b5 ("conntrack: New userspace connection tracker.") Signed-off-by: Mike Pattrick <mkp@redhat.com> --- v2: Change to a macro, --- lib/conntrack.c | 2 +- lib/packets.h | 2 ++ tests/ofproto-dpif.at | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 35 insertions(+), 1 deletion(-)