On 1/29/24 15:33, Xavier Simonart wrote: > Hi Dumitru > > Thanks for the patch. > > On Tue, Jan 23, 2024 at 10:44 PM Mark Michelson <mmich...@redhat.com> wrote: > >> Hi Dumitru, >> >> Acked-by: Mark Michelson <mmich...@redhat.com> >> >> I think this change is good. I looked through the documentation for the >> "external_port_range" column in the NAT table. This change appears to >> actually make the documentation *more* accurate rather than to introduce >> any potentially undesirable behavior changes. >> >> What I find more telling is that the only tests you had to update were >> the actions tests. We apparently don't have any system tests that use >> NAT with an external port range. That's not great :( >>
That's not great indeed, we should try to improve that in the future. >> On 1/22/24 09:54, Dumitru Ceara wrote: >>> This is to avoid unexpected behavior changes due to the underlying >>> datapath (e.g., kernel) changing defaults. If we don't explicitly >>> request a port selection algorithm, OVS leaves it up to the >>> datapath to decide how to do the port selection. Currently that means >>> that source port allocation is not random if the original source port >>> fits in the requested range. >> > > This looks good to me. > In most cases, a source port within range was not translated w/ existing > behavior > > ovn-nbctl --portrange lr-nat-add rtr snat 66.66.66.66 42.42.42.0/24 > 10000-20000 > > 43.43.43.2:15000 -> 42.42.42.2:8080 => 66.66.66.66:15000 -> > 42.42.42.2:80 # no PAT as within range > > But it was still translated in some cases: > > ovn-nbctl --portrange lr-nat-add rtr snat 66.66.66.66 42.42.42.0/24 > 10000-20000 > > 43.43.43.2:40000 -> 42.42.42.2:8080 => 66.66.66.66:15000 -> > 42.42.42.2:80 # PAT. dst port happens to be 15000 > > 43.43.43.2:15000 -> 42.42.42.2:8080 => 66.66.66.66:XXX -> > 42.42.42.2:80 # PAT to XXX != 15000 as 15000 already used. > > In other words, I do not see how one could rely on existing (pre-patch) > behavior (no PAT if the source port is within the range) > as it is not guaranteed. > > That makes sense, thanks for trying this out! Thanks for the reviews, Mark, Ales and Xavier! I pushed this to main and backported it to all stable branches down to 22.03. Regards, Dumitru _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
