On Mon, Jan 22, 2024 at 3:55 PM Dumitru Ceara <[email protected]> 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. > > Reported-at: > https://mail.openvswitch.org/pipermail/ovs-dev/2024-January/410847.html > Reported-at: https://issues.redhat.com/browse/FDP-301 > Fixes: 60bdc8ea78d7 ("NAT: Provide port range in input") > Signed-off-by: Dumitru Ceara <[email protected]> > --- > lib/actions.c | 16 ++++++++++++++-- > tests/ovn.at | 8 ++++---- > 2 files changed, 18 insertions(+), 6 deletions(-) > > diff --git a/lib/actions.c b/lib/actions.c > index 38cf4642d6..fdc0529de6 100644 > --- a/lib/actions.c > +++ b/lib/actions.c > @@ -1131,8 +1131,20 @@ encode_ct_nat(const struct ovnact_ct_nat *cn, > } > > if (cn->port_range.exists) { > - nat->range.proto.min = cn->port_range.port_lo; > - nat->range.proto.max = cn->port_range.port_hi; > + nat->range.proto.min = cn->port_range.port_lo; > + nat->range.proto.max = cn->port_range.port_hi; > + > + /* Explicitly set the port selection algorithm to "random". > Otherwise > + * it's up to the datapath to choose how to select the port and > that > + * might create unexpected behavior changes when the datapath > defaults > + * change. > + * > + * NOTE: for the userspace datapath the "random" function doesn't > + * really generate random ports, it uses "hash" under the hood: > + * https://issues.redhat.com/browse/FDP-269. */ > + if (nat->range.proto.min && nat->range.proto.max) { > + nat->flags |= NX_NAT_F_PROTO_RANDOM; > + } > } > > ofpacts->header = ofpbuf_push_uninit(ofpacts, nat_offset); > diff --git a/tests/ovn.at b/tests/ovn.at > index 8cc4c311c1..e8cef83cb5 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -1368,7 +1368,7 @@ ct_dnat(fd11::2); > has prereqs ip > ct_dnat(192.168.1.2, 1-3000); > formats as ct_dnat(192.168.1.2,1-3000); > - encodes as > ct(commit,table=19,zone=NXM_NX_REG11[0..15],nat(dst=192.168.1.2:1-3000)) > + encodes as > ct(commit,table=19,zone=NXM_NX_REG11[0..15],nat(dst=192.168.1.2:1 > -3000,random)) > has prereqs ip > > ct_dnat(192.168.1.2, 192.168.1.3); > @@ -1402,7 +1402,7 @@ ct_dnat_in_czone(fd11::2); > has prereqs ip > ct_dnat_in_czone(192.168.1.2, 1-3000); > formats as ct_dnat_in_czone(192.168.1.2,1-3000); > - encodes as > ct(commit,table=19,zone=NXM_NX_REG11[0..15],nat(dst=192.168.1.2:1-3000)) > + encodes as > ct(commit,table=19,zone=NXM_NX_REG11[0..15],nat(dst=192.168.1.2:1 > -3000,random)) > has prereqs ip > > ct_dnat_in_czone(192.168.1.2, 192.168.1.3); > @@ -1436,7 +1436,7 @@ ct_snat(fd11::2); > has prereqs ip > ct_snat(192.168.1.2, 1-3000); > formats as ct_snat(192.168.1.2,1-3000); > - encodes as > ct(commit,table=19,zone=NXM_NX_REG12[0..15],nat(src=192.168.1.2:1-3000)) > + encodes as > ct(commit,table=19,zone=NXM_NX_REG12[0..15],nat(src=192.168.1.2:1 > -3000,random)) > has prereqs ip > > ct_snat(192.168.1.2, 192.168.1.3); > @@ -1470,7 +1470,7 @@ ct_snat_in_czone(fd11::2); > has prereqs ip > ct_snat_in_czone(192.168.1.2, 1-3000); > formats as ct_snat_in_czone(192.168.1.2,1-3000); > - encodes as > ct(commit,table=19,zone=NXM_NX_REG11[0..15],nat(src=192.168.1.2:1-3000)) > + encodes as > ct(commit,table=19,zone=NXM_NX_REG11[0..15],nat(src=192.168.1.2:1 > -3000,random)) > has prereqs ip > > ct_snat_in_czone(192.168.1.2, 192.168.1.3); > -- > 2.39.3 > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > Looks good to me, thanks. Acked-by: Ales Musil <[email protected]> -- Ales Musil Senior Software Engineer - OVN Core Red Hat EMEA <https://www.redhat.com> [email protected] <https://red.ht/sig> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
