Skip to content

Commit

Permalink
northd: Allow need frag to be SNATed
Browse files Browse the repository at this point in the history
Considering following topology:
client - sw0 - lrp0 - lr - lrp1 - sw1 - server
sw0 in subnet 192.168.0.0/24
sw1 in subnet 172.168.0.0/24
SNAT configured for sw0 subnet
gateway_mtu=1400 configured for lrp0

If we send UDP traffic from client to server
and server responds with packet bigger than 1400
the following sequence will happen:

1) Packet is coming into lr via lrp1
2) unSNAT
3) Routing, the outport will be set to lrp0
4) Check for packet larger will fail
5) We will generate ICMP need frag

However, the last step is wrong from the server
perspective. The ICMP message will have IP source
address = lrp1 IP address. Which means that SNAT won't
happen because the source is not within the sw0 subnet,
but the inner packet has sw0 subnet address, because it
was unSNATted. This results in server ignoring the ICMP
message because server never sent any packet to the
sw0 subnet.

To fix this issue use outport IP address as source instead
of the inport one for the ICMP error message. This will
lead to SNAT for the packet which will result in correct
addresses on the sw1 side.

Reported-at: https://issues.redhat.com/browse/FDP-39
Signed-off-by: Ales Musil <amusil@redhat.com>
Co-authored-by: Ilya Maximets <i.maximets@ovn.org>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
(cherry picked from commit 0e49f49)
  • Loading branch information
almusil authored and dceara committed Oct 10, 2023
1 parent b30f7c9 commit 674690b
Show file tree
Hide file tree
Showing 4 changed files with 210 additions and 43 deletions.
30 changes: 22 additions & 8 deletions northd/northd.c
Original file line number Diff line number Diff line change
Expand Up @@ -12019,7 +12019,15 @@ build_icmperr_pkt_big_flows(struct ovn_port *op, int mtu, struct hmap *lflows,
outport->json_key)
: NULL;

if (op->lrp_networks.ipv4_addrs) {
char *ip4_src = NULL;

if (outport && outport->lrp_networks.ipv4_addrs) {
ip4_src = outport->lrp_networks.ipv4_addrs[0].addr_s;
} else if (op->lrp_networks.ipv4_addrs) {
ip4_src = op->lrp_networks.ipv4_addrs[0].addr_s;
}

if (ip4_src) {
ds_clear(match);
ds_put_format(match, "inport == %s && %sip4 && "REGBIT_PKT_LARGER
" && "REGBIT_EGRESS_LOOPBACK" == 0", op->json_key,
Expand All @@ -12039,9 +12047,8 @@ build_icmperr_pkt_big_flows(struct ovn_port *op, int mtu, struct hmap *lflows,
"icmp4.code = 4; /* Frag Needed and DF was Set. */ "
"icmp4.frag_mtu = %d; "
"next(pipeline=ingress, table=%d); };",
op->lrp_networks.ea_s,
op->lrp_networks.ipv4_addrs[0].addr_s,
mtu, ovn_stage_get_table(S_ROUTER_IN_ADMISSION));
op->lrp_networks.ea_s, ip4_src, mtu,
ovn_stage_get_table(S_ROUTER_IN_ADMISSION));
ovn_lflow_add_with_hint__(lflows, op->od, stage, 150,
ds_cstr(match), ds_cstr(actions),
NULL,
Expand All @@ -12052,7 +12059,15 @@ build_icmperr_pkt_big_flows(struct ovn_port *op, int mtu, struct hmap *lflows,
&op->nbrp->header_);
}

if (op->lrp_networks.ipv6_addrs) {
char *ip6_src = NULL;

if (outport && outport->lrp_networks.ipv6_addrs) {
ip6_src = outport->lrp_networks.ipv6_addrs[0].addr_s;
} else if (op->lrp_networks.ipv6_addrs) {
ip6_src = op->lrp_networks.ipv6_addrs[0].addr_s;
}

if (ip6_src) {
ds_clear(match);
ds_put_format(match, "inport == %s && %sip6 && "REGBIT_PKT_LARGER
" && "REGBIT_EGRESS_LOOPBACK" == 0", op->json_key,
Expand All @@ -12072,9 +12087,8 @@ build_icmperr_pkt_big_flows(struct ovn_port *op, int mtu, struct hmap *lflows,
"icmp6.code = 0; "
"icmp6.frag_mtu = %d; "
"next(pipeline=ingress, table=%d); };",
op->lrp_networks.ea_s,
op->lrp_networks.ipv6_addrs[0].addr_s,
mtu, ovn_stage_get_table(S_ROUTER_IN_ADMISSION));
op->lrp_networks.ea_s, ip6_src, mtu,
ovn_stage_get_table(S_ROUTER_IN_ADMISSION));
ovn_lflow_add_with_hint__(lflows, op->od, stage, 150,
ds_cstr(match), ds_cstr(actions),
NULL,
Expand Down

0 comments on commit 674690b

Please sign in to comment.