| Message ID | 20240328165608.273344-1-pvalerio@redhat.com |
|---|---|
| State | Accepted |
| Commit | b5e6829254cc74ed100392dd22e9db3c93b2974e |
| Delegated to: | Ilya Maximets |
| Headers | show |
| Series | [ovs-dev,v2] conntrack: Do not use icmp reverse helper for icmpv6. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/intel-ovs-compilation | success | test: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
On 3/28/24 17:56, Paolo Valerio wrote: > In the flush tuple code path, while populating the conn_key, > reverse_icmp_type() gets called for both icmp and icmpv6 cases, > while, depending on the proto, its respective helper should be > called, instead. > > The above leads to an abort: > > [...] > __GI_abort () at abort.c:79 > reverse_icmp_type (type=128 '\200') at lib/conntrack.c:1795 > tuple_to_conn_key (...) at lib/conntrack.c:2590 > in conntrack_flush_tuple (...) at lib/conntrack.c:2787 > in dpif_netdev_ct_flush (...) at lib/dpif-netdev.c:9618 > ct_dpif_flush_tuple (...) at lib/ct-dpif.c:331 > ct_dpif_flush (...) at lib/ct-dpif.c:361 > dpctl_flush_conntrack (...) at lib/dpctl.c:1797 > [...] > > Fix it by calling reverse_icmp6_type() when needed. > Furthermore, self tests have been modified in order to exercise and > check this behavior. > > Fixes: 271e48a0e244 ("conntrack: Support conntrack flush by ct 5-tuple") > Reported-at: https://issues.redhat.com/browse/FDP-447 > Signed-off-by: Paolo Valerio <pvalerio@redhat.com> > --- > v2 (Ilya): > - stripped down backtrace > - aligned ternary > --- > lib/conntrack.c | 4 +++- > tests/system-traffic.at | 10 +++++++++- > 2 files changed, 12 insertions(+), 2 deletions(-) Thanks! Applied and backported down to 2.17. Best regards, Ilya Maximets.
diff --git a/lib/conntrack.c b/lib/conntrack.c index 5786424f6..7e3ed0ee0 100644 --- a/lib/conntrack.c +++ b/lib/conntrack.c @@ -2586,7 +2586,9 @@ tuple_to_conn_key(const struct ct_dpif_tuple *tuple, uint16_t zone, key->src.icmp_type = tuple->icmp_type; key->src.icmp_code = tuple->icmp_code; key->dst.icmp_id = tuple->icmp_id; - key->dst.icmp_type = reverse_icmp_type(tuple->icmp_type); + key->dst.icmp_type = (tuple->ip_proto == IPPROTO_ICMP) + ? reverse_icmp_type(tuple->icmp_type) + : reverse_icmp6_type(tuple->icmp_type); key->dst.icmp_code = tuple->icmp_code; } else { key->src.port = tuple->src_port; diff --git a/tests/system-traffic.at b/tests/system-traffic.at index 2d12d558e..87de0692a 100644 --- a/tests/system-traffic.at +++ b/tests/system-traffic.at @@ -3103,7 +3103,10 @@ AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2)], [0], [dnl icmp,orig=(src=10.1.1.1,dst=10.1.1.2,id=<cleared>,type=8,code=0),reply=(src=10.1.1.2,dst=10.1.1.1,id=<cleared>,type=0,code=0) ]) -AT_CHECK([ovs-appctl dpctl/flush-conntrack]) +AT_CHECK([ovs-appctl dpctl/flush-conntrack 'ct_nw_src=10.1.1.1,ct_nw_dst=10.1.1.2']) + +AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2)], [0], [dnl +]) dnl Pings from ns1->ns0 should fail. NS_CHECK_EXEC([at_ns1], [ping -q -c 3 -i 0.3 -w 2 10.1.1.1 | FORMAT_PING], [0], [dnl @@ -3244,6 +3247,11 @@ AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(fc00::2)], [0], [dnl icmpv6,orig=(src=fc00::1,dst=fc00::2,id=<cleared>,type=128,code=0),reply=(src=fc00::2,dst=fc00::1,id=<cleared>,type=129,code=0) ]) +AT_CHECK([ovs-appctl dpctl/flush-conntrack 'ct_ipv6_src=fc00::1,ct_ipv6_dst=fc00::2']) + +AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(fc00::2)], [0], [dnl +]) + OVS_TRAFFIC_VSWITCHD_STOP AT_CLEANUP
In the flush tuple code path, while populating the conn_key, reverse_icmp_type() gets called for both icmp and icmpv6 cases, while, depending on the proto, its respective helper should be called, instead. The above leads to an abort: [...] __GI_abort () at abort.c:79 reverse_icmp_type (type=128 '\200') at lib/conntrack.c:1795 tuple_to_conn_key (...) at lib/conntrack.c:2590 in conntrack_flush_tuple (...) at lib/conntrack.c:2787 in dpif_netdev_ct_flush (...) at lib/dpif-netdev.c:9618 ct_dpif_flush_tuple (...) at lib/ct-dpif.c:331 ct_dpif_flush (...) at lib/ct-dpif.c:361 dpctl_flush_conntrack (...) at lib/dpctl.c:1797 [...] Fix it by calling reverse_icmp6_type() when needed. Furthermore, self tests have been modified in order to exercise and check this behavior. Fixes: 271e48a0e244 ("conntrack: Support conntrack flush by ct 5-tuple") Reported-at: https://issues.redhat.com/browse/FDP-447 Signed-off-by: Paolo Valerio <pvalerio@redhat.com> --- v2 (Ilya): - stripped down backtrace - aligned ternary --- lib/conntrack.c | 4 +++- tests/system-traffic.at | 10 +++++++++- 2 files changed, 12 insertions(+), 2 deletions(-)