On Tue, May 28, 2024 at 9:52 PM Mark Michelson <[email protected]> wrote:

> Thanks Ales, I have just one question about this.
>
>
Hi Mark,

thank you for the review.


>
> I noticed that you have updated the output of `ovn-nbctl lr-nat-list` to
> include the match if it is configured. I'm curious if there is any
> utility in printing the priority as well?
>
>
I was thinking about that, however the output seems to be pretty crowded as
is, so I'm not really sure whether it is a good idea. If others disagree I
can include it.


>
> On 5/3/24 03:26, Ales Musil wrote:
> > Add support for match and priority in NAT table. This allows to define
> > NAT that has extra match condition to have more fine-grained control
> > over the final NAT rule application. At the same time it allows for
> > NAT rules that would be considered as duplicates otherwise e.g.
> > multiple SNATs with same logical IP, but different external IP. Also,
> > when the match is specified allow addition of priority to order the
> > NAT rule valuation as needed.
> >
> > Signed-off-by: Ales Musil <[email protected]>
> > ---
> >   ovn-nb.ovsschema          |   8 +-
> >   ovn-nb.xml                |  15 +++
> >   tests/ovn-nbctl.at        | 220 +++++++++++++++++++++++++++-----------
> >   utilities/ovn-nbctl.8.xml |  14 ++-
> >   utilities/ovn-nbctl.c     | 189 ++++++++++++++++++++------------
> >   5 files changed, 307 insertions(+), 139 deletions(-)
> >
> > diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema
> > index 10ce50b25..e3c4aff9d 100644
> > --- a/ovn-nb.ovsschema
> > +++ b/ovn-nb.ovsschema
> > @@ -1,7 +1,7 @@
> >   {
> >       "name": "OVN_Northbound",
> > -    "version": "7.3.1",
> > -    "cksum": "3899022625 35372",
> > +    "version": "7.4.0",
> > +    "cksum": "1908497390 35615",
> >       "tables": {
> >           "NB_Global": {
> >               "columns": {
> > @@ -524,6 +524,10 @@
> >                                        "refType": "weak"},
> >                                "min": 0,
> >                                "max": 1}},
> > +                "priority": {"type": {"key": {"type": "integer",
> > +                                              "minInteger": 0,
> > +                                              "maxInteger": 32767}}},
> > +                "match": {"type": "string"},
> >                   "options": {"type": {"key": "string", "value":
> "string",
> >                                        "min": 0, "max": "unlimited"}},
> >                   "external_ids": {
> > diff --git a/ovn-nb.xml b/ovn-nb.xml
> > index 5cb6ba640..fbad5f124 100644
> > --- a/ovn-nb.xml
> > +++ b/ovn-nb.xml
> > @@ -3924,6 +3924,21 @@ or
> >         </p>
> >       </column>
> >
> > +    <column name="match">
> > +      The packets that the NAT rules should match, in addition to the
> match
> > +      that is created based on the NAT type, in the same expression
> > +      language used for the <ref column="match" table="Logical_Flow"
> > +                                 db="OVN_Southbound"/> column in the OVN
> > +      Southbound database's <ref table="Logical_Flow"
> db="OVN_Southbound"/>
> > +      table.  This allows for more fine-grained control over the NAT
> rule.
> > +    </column>
> > +
> > +    <column name="priority">
> > +      The NAT rule's priority.  Rules with numerically higher priority
> > +      take precedence over those with lower.  The priority is taken into
> > +      account only if the <code>match</code> is defined.
> > +    </column>
> > +
> >       <column name="options" key="stateless">
> >         Indicates if a dnat_and_snat rule should lead to connection
> >         tracking state or not.
> > diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
> > index 5248e6c76..19c83a4a5 100644
> > --- a/tests/ovn-nbctl.at
> > +++ b/tests/ovn-nbctl.at
> > @@ -625,15 +625,15 @@ AT_CHECK([ovn-nbctl lr-nat-add lr0 dnat_and_snat
> fd01::1 fd11::2])
> >   AT_CHECK([ovn-nbctl lr-nat-add lr0 dnat_and_snat 30.0.0.2 192.168.1.3
> lp0 00:00:00:01:02:03])
> >   AT_CHECK([ovn-nbctl lr-nat-add lr0 dnat_and_snat fd01::2 fd11::3 lp0
> 00:00:00:01:02:03])
> >   AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl
> > -TYPE             GATEWAY_PORT          EXTERNAL_IP
> EXTERNAL_PORT    LOGICAL_IP          EXTERNAL_MAC         LOGICAL_PORT
> > -dnat                                   30.0.0.1
>     192.168.1.2
> > -dnat                                   fd01::1
>    fd11::2
> > -dnat_and_snat                          30.0.0.1
>     192.168.1.2
> > -dnat_and_snat                          30.0.0.2
>     192.168.1.3         00:00:00:01:02:03    lp0
> > -dnat_and_snat                          fd01::1
>    fd11::2
> > -dnat_and_snat                          fd01::2
>    fd11::3             00:00:00:01:02:03    lp0
> > -snat                                   30.0.0.1
>     192.168.1.0/24
> > -snat                                   fd01::1
>    fd11::/64
> > +TYPE             GATEWAY_PORT          MATCH
>  EXTERNAL_IP        EXTERNAL_PORT    LOGICAL_IP          EXTERNAL_MAC
>    LOGICAL_PORT
> > +dnat                                                         30.0.0.1
>                           192.168.1.2
> > +dnat                                                         fd01::1
>                          fd11::2
> > +dnat_and_snat                                                30.0.0.1
>                           192.168.1.2
> > +dnat_and_snat                                                30.0.0.2
>                           192.168.1.3         00:00:00:01:02:03    lp0
> > +dnat_and_snat                                                fd01::1
>                          fd11::2
> > +dnat_and_snat                                                fd01::2
>                          fd11::3             00:00:00:01:02:03    lp0
> > +snat                                                         30.0.0.1
>                           192.168.1.0/24
> > +snat                                                         fd01::1
>                          fd11::/64
> >   ])
> >   AT_CHECK([ovn-nbctl lr-nat-add lr0 snat 30.0.0.1 192.168.1.0/24],
> [1], [],
> >   [ovn-nbctl: 30.0.0.1, 192.168.1.0/24: a NAT with this external_ip and
> logical_ip already exists
> > @@ -661,28 +661,28 @@ AT_CHECK([ovn-nbctl lr-nat-add lr0 dnat_and_snat
> 30.0.0.1 192.168.1.3], [1], [],
> >   ])
> >   AT_CHECK([ovn-nbctl --may-exist lr-nat-add lr0 dnat_and_snat 30.0.0.2
> 192.168.1.3 lp0 00:00:00:04:05:06])
> >   AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl
> > -TYPE             GATEWAY_PORT          EXTERNAL_IP
> EXTERNAL_PORT    LOGICAL_IP          EXTERNAL_MAC         LOGICAL_PORT
> > -dnat                                   30.0.0.1
>     192.168.1.2
> > -dnat                                   fd01::1
>    fd11::2
> > -dnat_and_snat                          30.0.0.1
>     192.168.1.2
> > -dnat_and_snat                          30.0.0.2
>     192.168.1.3         00:00:00:04:05:06    lp0
> > -dnat_and_snat                          fd01::1
>    fd11::2
> > -dnat_and_snat                          fd01::2
>    fd11::3             00:00:00:01:02:03    lp0
> > -snat                                   30.0.0.1
>     192.168.1.0/24
> > -snat                                   fd01::1
>    fd11::/64
> > +TYPE             GATEWAY_PORT          MATCH
>  EXTERNAL_IP        EXTERNAL_PORT    LOGICAL_IP          EXTERNAL_MAC
>    LOGICAL_PORT
> > +dnat                                                         30.0.0.1
>                           192.168.1.2
> > +dnat                                                         fd01::1
>                          fd11::2
> > +dnat_and_snat                                                30.0.0.1
>                           192.168.1.2
> > +dnat_and_snat                                                30.0.0.2
>                           192.168.1.3         00:00:00:04:05:06    lp0
> > +dnat_and_snat                                                fd01::1
>                          fd11::2
> > +dnat_and_snat                                                fd01::2
>                          fd11::3             00:00:00:01:02:03    lp0
> > +snat                                                         30.0.0.1
>                           192.168.1.0/24
> > +snat                                                         fd01::1
>                          fd11::/64
> >   ])
> >   AT_CHECK([ovn-nbctl --may-exist lr-nat-add lr0 dnat_and_snat 30.0.0.2
> 192.168.1.3])
> >   AT_CHECK([ovn-nbctl --may-exist lr-nat-add lr0 dnat_and_snat fd01::2
> fd11::3])
> >   AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl
> > -TYPE             GATEWAY_PORT          EXTERNAL_IP
> EXTERNAL_PORT    LOGICAL_IP          EXTERNAL_MAC         LOGICAL_PORT
> > -dnat                                   30.0.0.1
>     192.168.1.2
> > -dnat                                   fd01::1
>    fd11::2
> > -dnat_and_snat                          30.0.0.1
>     192.168.1.2
> > -dnat_and_snat                          30.0.0.2
>     192.168.1.3
> > -dnat_and_snat                          fd01::1
>    fd11::2
> > -dnat_and_snat                          fd01::2
>    fd11::3
> > -snat                                   30.0.0.1
>     192.168.1.0/24
> > -snat                                   fd01::1
>    fd11::/64
> > +TYPE             GATEWAY_PORT          MATCH
>  EXTERNAL_IP        EXTERNAL_PORT    LOGICAL_IP          EXTERNAL_MAC
>    LOGICAL_PORT
> > +dnat                                                         30.0.0.1
>                           192.168.1.2
> > +dnat                                                         fd01::1
>                          fd11::2
> > +dnat_and_snat                                                30.0.0.1
>                           192.168.1.2
> > +dnat_and_snat                                                30.0.0.2
>                           192.168.1.3
> > +dnat_and_snat                                                fd01::1
>                          fd11::2
> > +dnat_and_snat                                                fd01::2
>                          fd11::3
> > +snat                                                         30.0.0.1
>                           192.168.1.0/24
> > +snat                                                         fd01::1
>                          fd11::/64
> >   ])
> >
> >   check_row_count nb:NAT 0 options:stateless=true
> > @@ -720,26 +720,26 @@ AT_CHECK([ovn-nbctl lr-nat-del lr0 dnat_and_snat
> 30.0.0.3])
> >   AT_CHECK([ovn-nbctl lr-nat-del lr0 dnat_and_snat 30.0.0.1])
> >   AT_CHECK([ovn-nbctl lr-nat-del lr0 dnat_and_snat fd01::1])
> >   AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl
> > -TYPE             GATEWAY_PORT          EXTERNAL_IP
> EXTERNAL_PORT    LOGICAL_IP          EXTERNAL_MAC         LOGICAL_PORT
> > -dnat                                   30.0.0.1
>     192.168.1.2
> > -dnat                                   fd01::1
>    fd11::2
> > -dnat_and_snat                          30.0.0.2
>     192.168.1.3
> > -dnat_and_snat                          40.0.0.2
>     192.168.1.4
> > -dnat_and_snat                          fd01::2
>    fd11::3
> > -snat                                   30.0.0.1
>     192.168.1.0/24
> > -snat                                   40.0.0.3
>     192.168.1.6
> > -snat                                   fd01::1
>    fd11::/64
> > +TYPE             GATEWAY_PORT          MATCH
>  EXTERNAL_IP        EXTERNAL_PORT    LOGICAL_IP          EXTERNAL_MAC
>    LOGICAL_PORT
> > +dnat                                                         30.0.0.1
>                           192.168.1.2
> > +dnat                                                         fd01::1
>                          fd11::2
> > +dnat_and_snat                                                30.0.0.2
>                           192.168.1.3
> > +dnat_and_snat                                                40.0.0.2
>                           192.168.1.4
> > +dnat_and_snat                                                fd01::2
>                          fd11::3
> > +snat                                                         30.0.0.1
>                           192.168.1.0/24
> > +snat                                                         40.0.0.3
>                           192.168.1.6
> > +snat                                                         fd01::1
>                          fd11::/64
> >   ])
> >
> >   AT_CHECK([ovn-nbctl lr-nat-del lr0 dnat])
> >   AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl
> > -TYPE             GATEWAY_PORT          EXTERNAL_IP
> EXTERNAL_PORT    LOGICAL_IP          EXTERNAL_MAC         LOGICAL_PORT
> > -dnat_and_snat                          30.0.0.2
>     192.168.1.3
> > -dnat_and_snat                          40.0.0.2
>     192.168.1.4
> > -dnat_and_snat                          fd01::2
>    fd11::3
> > -snat                                   30.0.0.1
>     192.168.1.0/24
> > -snat                                   40.0.0.3
>     192.168.1.6
> > -snat                                   fd01::1
>    fd11::/64
> > +TYPE             GATEWAY_PORT          MATCH
>  EXTERNAL_IP        EXTERNAL_PORT    LOGICAL_IP          EXTERNAL_MAC
>    LOGICAL_PORT
> > +dnat_and_snat                                                30.0.0.2
>                           192.168.1.3
> > +dnat_and_snat                                                40.0.0.2
>                           192.168.1.4
> > +dnat_and_snat                                                fd01::2
>                          fd11::3
> > +snat                                                         30.0.0.1
>                           192.168.1.0/24
> > +snat                                                         40.0.0.3
>                           192.168.1.6
> > +snat                                                         fd01::1
>                          fd11::/64
> >   ])
> >
> >   AT_CHECK([ovn-nbctl lr-nat-del lr0])
> > @@ -810,12 +810,12 @@ AT_CHECK([ovn-nbctl show lr0 | grep -C2 'external
> port(s): "1"' | uuidfilt], [0]
> >   ])
> >
> >   AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl
> > -TYPE             GATEWAY_PORT          EXTERNAL_IP
> EXTERNAL_PORT    LOGICAL_IP          EXTERNAL_MAC         LOGICAL_PORT
> > -dnat                                   40.0.0.4           1-3000
>    192.168.1.7
> > -dnat                                   40.0.0.5           1
>     192.168.1.10
> > -dnat_and_snat                          40.0.0.5           1-3000
>    192.168.1.8
> > -dnat_and_snat                          40.0.0.6           1-3000
>    192.168.1.9         00:00:00:04:05:06    lp0
> > -snat                                   40.0.0.3           21-65535
>    192.168.1.6
> > +TYPE             GATEWAY_PORT          MATCH
>  EXTERNAL_IP        EXTERNAL_PORT    LOGICAL_IP          EXTERNAL_MAC
>    LOGICAL_PORT
> > +dnat                                                         40.0.0.4
>          1-3000           192.168.1.7
> > +dnat                                                         40.0.0.5
>          1                192.168.1.10
> > +dnat_and_snat                                                40.0.0.5
>          1-3000           192.168.1.8
> > +dnat_and_snat                                                40.0.0.6
>          1-3000           192.168.1.9         00:00:00:04:05:06    lp0
> > +snat                                                         40.0.0.3
>          21-65535         192.168.1.6
> >   ])
> >
> >   AT_CHECK([ovn-nbctl lr-nat-del lr0])
> > @@ -895,39 +895,39 @@ AT_CHECK([ovn-nbctl --gateway-port=lrp00
> lr-nat-add lr0 dnat 30.0.0.10 20.0.0.11
> >   AT_CHECK([ovn-nbctl --gateway-port=lrp01 lr-nat-add lr0 dnat 30.0.0.10
> 20.0.0.10])
> >
> >   AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl
> > -TYPE             GATEWAY_PORT          EXTERNAL_IP
> EXTERNAL_PORT    LOGICAL_IP          EXTERNAL_MAC         LOGICAL_PORT
> > -dnat             lrp00                 30.0.0.10
>    20.0.0.10
> > -dnat             lrp01                 30.0.0.10
>    20.0.0.10
> > -snat                                   172.64.1.10
>    20.0.0.10
> > -snat             lrp00                 172.64.0.10
>    20.0.0.10
> > +TYPE             GATEWAY_PORT          MATCH
>  EXTERNAL_IP        EXTERNAL_PORT    LOGICAL_IP          EXTERNAL_MAC
>    LOGICAL_PORT
> > +dnat             lrp00                                       30.0.0.10
>                          20.0.0.10
> > +dnat             lrp01                                       30.0.0.10
>                          20.0.0.10
> > +snat
>  172.64.1.10                         20.0.0.10
> > +snat             lrp00
>  172.64.0.10                         20.0.0.10
> >   ])
> >
> >   AT_CHECK([ovn-nbctl lr-nat-del lr0 dnat 30.0.0.10])
> >   AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl
> > -TYPE             GATEWAY_PORT          EXTERNAL_IP
> EXTERNAL_PORT    LOGICAL_IP          EXTERNAL_MAC         LOGICAL_PORT
> > -snat                                   172.64.1.10
>    20.0.0.10
> > -snat             lrp00                 172.64.0.10
>    20.0.0.10
> > +TYPE             GATEWAY_PORT          MATCH
>  EXTERNAL_IP        EXTERNAL_PORT    LOGICAL_IP          EXTERNAL_MAC
>    LOGICAL_PORT
> > +snat
>  172.64.1.10                         20.0.0.10
> > +snat             lrp00
>  172.64.0.10                         20.0.0.10
> >   ])
> >
> >   AT_CHECK([ovn-nbctl --gateway-port=lrp00 lr-nat-add lr0 snat 30.0.0.10
> 20.0.0.20])
> >   AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl
> > -TYPE             GATEWAY_PORT          EXTERNAL_IP
> EXTERNAL_PORT    LOGICAL_IP          EXTERNAL_MAC         LOGICAL_PORT
> > -snat                                   172.64.1.10
>    20.0.0.10
> > -snat             lrp00                 172.64.0.10
>    20.0.0.10
> > -snat             lrp00                 30.0.0.10
>    20.0.0.20
> > +TYPE             GATEWAY_PORT          MATCH
>  EXTERNAL_IP        EXTERNAL_PORT    LOGICAL_IP          EXTERNAL_MAC
>    LOGICAL_PORT
> > +snat
>  172.64.1.10                         20.0.0.10
> > +snat             lrp00
>  172.64.0.10                         20.0.0.10
> > +snat             lrp00                                       30.0.0.10
>                          20.0.0.20
> >   ])
> >   AT_CHECK([ovn-nbctl lr-nat-del lr0 dnat lrp11], [1], [],
> >   [ovn-nbctl: lrp11: port name not found
> >   ])
> >   AT_CHECK([ovn-nbctl lr-nat-del lr0 snat lrp00])
> >   AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl
> > -TYPE             GATEWAY_PORT          EXTERNAL_IP
> EXTERNAL_PORT    LOGICAL_IP          EXTERNAL_MAC         LOGICAL_PORT
> > -snat                                   172.64.1.10
>    20.0.0.10
> > +TYPE             GATEWAY_PORT          MATCH
>  EXTERNAL_IP        EXTERNAL_PORT    LOGICAL_IP          EXTERNAL_MAC
>    LOGICAL_PORT
> > +snat
>  172.64.1.10                         20.0.0.10
> >   ])
> >   AT_CHECK([ovn-nbctl lr-nat-del lr0 snat lrp01])
> >   AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl
> > -TYPE             GATEWAY_PORT          EXTERNAL_IP
> EXTERNAL_PORT    LOGICAL_IP          EXTERNAL_MAC         LOGICAL_PORT
> > -snat                                   172.64.1.10
>    20.0.0.10
> > +TYPE             GATEWAY_PORT          MATCH
>  EXTERNAL_IP        EXTERNAL_PORT    LOGICAL_IP          EXTERNAL_MAC
>    LOGICAL_PORT
> > +snat
>  172.64.1.10                         20.0.0.10
> >   ])
> >
> >   AT_CHECK([ovn-nbctl lr-nat-del lr0 snat 20.0.0 lrp01], [1], [],
> > @@ -938,6 +938,96 @@ AT_CHECK([ovn-nbctl lr-nat-del lr0 snat 20.0.0.10
> lrp01], [1], [],
> >   ])
> >   AT_CHECK([ovn-nbctl --if-exists lr-nat-del lr0 snat 20.0.0.10 lrp01])
> >   AT_CHECK([ovn-nbctl lr-nat-del lr0 snat])
> > +
> > +AT_CHECK([ovn-nbctl lr-nat-del lr0])
> > +AT_CHECK([ovn-nbctl lrp-del-gateway-chassis lrp00 chassis1])
> > +AT_CHECK([ovn-nbctl lrp-del-gateway-chassis lrp01 chassis2])
> > +
> > +AT_CHECK([ovn-nbctl lr-nat-add lr0 snat 10.0.0.1 192.168.0.0/24])
> > +AT_CHECK([ovn-nbctl --match="tcp" lr-nat-add lr0 snat 10.0.0.2
> 192.168.0.0/24])
> > +AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl
> > +TYPE             GATEWAY_PORT          MATCH
>  EXTERNAL_IP        EXTERNAL_PORT    LOGICAL_IP          EXTERNAL_MAC
>    LOGICAL_PORT
> > +snat                                                         10.0.0.1
>                           192.168.0.0/24
> > +snat                                   tcp                   10.0.0.2
>                           192.168.0.0/24
> > +])
> > +
> > +AT_CHECK([ovn-nbctl --match="tcp" lr-nat-add lr0 snat 10.0.0.3
> 192.168.0.0/24], [1], [],
> > +[ovn-nbctl: a NAT with this type (snat), logical_ip (192.168.0.0/24)
> and match "tcp" already exists
> > +])
> > +AT_CHECK([ovn-nbctl --match="tcp" lr-nat-add lr0 snat 10.0.0.2
> 192.168.0.0/24], [1], [],
> > +[ovn-nbctl: 10.0.0.2, 192.168.0.0/24: a NAT with this external_ip and
> logical_ip already exists
> > +])
> > +AT_CHECK([ovn-nbctl --may-exist --priority=5 --match="udp" lr-nat-add
> lr0 snat 10.0.0.2 192.168.0.0/24])
> > +AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl
> > +TYPE             GATEWAY_PORT          MATCH
>  EXTERNAL_IP        EXTERNAL_PORT    LOGICAL_IP          EXTERNAL_MAC
>    LOGICAL_PORT
> > +snat                                                         10.0.0.1
>                           192.168.0.0/24
> > +snat                                   udp                   10.0.0.2
>                           192.168.0.0/24
> > +])
> > +
> > +AT_CHECK([ovn-nbctl --match="udp" lr-nat-del lr0 snat 192.168.0.0/24])
> > +AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl
> > +TYPE             GATEWAY_PORT          MATCH
>  EXTERNAL_IP        EXTERNAL_PORT    LOGICAL_IP          EXTERNAL_MAC
>    LOGICAL_PORT
> > +snat                                                         10.0.0.1
>                           192.168.0.0/24
> > +])
> > +
> > +AT_CHECK([ovn-nbctl lr-nat-del lr0 snat 192.168.0.0/24])
> > +AT_CHECK([ovn-nbctl lr-nat-list lr0], [0])
> > +
> > +AT_CHECK([ovn-nbctl lr-nat-add lr0 dnat 10.0.0.1 192.168.0.1])
> > +AT_CHECK([ovn-nbctl --match="tcp" lr-nat-add lr0 dnat 10.0.0.1
> 192.168.0.2])
> > +AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl
> > +TYPE             GATEWAY_PORT          MATCH
>  EXTERNAL_IP        EXTERNAL_PORT    LOGICAL_IP          EXTERNAL_MAC
>    LOGICAL_PORT
> > +dnat                                                         10.0.0.1
>                           192.168.0.1
> > +dnat                                   tcp                   10.0.0.1
>                           192.168.0.2
> > +])
> > +
> > +AT_CHECK([ovn-nbctl --match="tcp" lr-nat-add lr0 dnat 10.0.0.1
> 192.168.0.3], [1], [],
> > +[ovn-nbctl: a NAT with this type (dnat), external_ip (10.0.0.1) and
> match "tcp" already exists
> > +])
> > +AT_CHECK([ovn-nbctl --match="tcp" lr-nat-add lr0 dnat 10.0.0.1
> 192.168.0.2], [1], [],
> > +[ovn-nbctl: 10.0.0.1, 192.168.0.2: a NAT with this external_ip and
> logical_ip already exists
> > +])
> > +AT_CHECK([ovn-nbctl --may-exist --priority=10 --match="udp" lr-nat-add
> lr0 dnat 10.0.0.1 192.168.0.2])
> > +AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl
> > +TYPE             GATEWAY_PORT          MATCH
>  EXTERNAL_IP        EXTERNAL_PORT    LOGICAL_IP          EXTERNAL_MAC
>    LOGICAL_PORT
> > +dnat                                                         10.0.0.1
>                           192.168.0.1
> > +dnat                                   udp                   10.0.0.1
>                           192.168.0.2
> > +])
> > +
> > +AT_CHECK([ovn-nbctl --match="udp" lr-nat-del lr0 dnat 10.0.0.1])
> > +AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl
> > +TYPE             GATEWAY_PORT          MATCH
>  EXTERNAL_IP        EXTERNAL_PORT    LOGICAL_IP          EXTERNAL_MAC
>    LOGICAL_PORT
> > +dnat                                                         10.0.0.1
>                           192.168.0.1
> > +])
> > +
> > +AT_CHECK([ovn-nbctl lr-nat-del lr0 dnat 10.0.0.1])
> > +AT_CHECK([ovn-nbctl lr-nat-list lr0], [0])
> > +
> > +AT_CHECK([ovn-nbctl lr-nat-add lr0 dnat_and_snat 10.0.0.1 192.168.0.1])
> > +AT_CHECK([ovn-nbctl --match="tcp" lr-nat-add lr0 dnat_and_snat 10.0.0.1
> 192.168.0.2])
> > +AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl
> > +TYPE             GATEWAY_PORT          MATCH
>  EXTERNAL_IP        EXTERNAL_PORT    LOGICAL_IP          EXTERNAL_MAC
>    LOGICAL_PORT
> > +dnat_and_snat                                                10.0.0.1
>                           192.168.0.1
> > +dnat_and_snat                          tcp                   10.0.0.1
>                           192.168.0.2
> > +])
> > +
> > +AT_CHECK([ovn-nbctl --match="tcp" lr-nat-add lr0 dnat_and_snat 10.0.0.1
> 192.168.0.3], [1], [],
> > +[ovn-nbctl: a NAT with this type (dnat_and_snat), external_ip
> (10.0.0.1) and match "tcp" already exists
> > +])
> > +AT_CHECK([ovn-nbctl --match="tcp" lr-nat-add lr0 dnat_and_snat 10.0.0.1
> 192.168.0.2], [1], [],
> > +[ovn-nbctl: 10.0.0.1, 192.168.0.2: a NAT with this external_ip and
> logical_ip already exists
> > +])
> > +AT_CHECK([ovn-nbctl --may-exist --priority=10 --match="udp" lr-nat-add
> lr0 dnat_and_snat 10.0.0.1 192.168.0.2])
> > +AT_CHECK([ovn-nbctl lr-nat-list lr0], [0], [dnl
> > +TYPE             GATEWAY_PORT          MATCH
>  EXTERNAL_IP        EXTERNAL_PORT    LOGICAL_IP          EXTERNAL_MAC
>    LOGICAL_PORT
> > +dnat_and_snat                                                10.0.0.1
>                           192.168.0.1
> > +dnat_and_snat                          udp                   10.0.0.1
>                           192.168.0.2
> > +])
> > +
> > +AT_CHECK([ovn-nbctl --priority=5 lr-nat-add lr0 snat 10.0.0.2
> 192.168.0.0/24], [1], [],
> > +[ovn-nbctl: priority can be set only with --match option.
> > +])
> > +
> >   ])
> >
> >   dnl
> ---------------------------------------------------------------------
> > diff --git a/utilities/ovn-nbctl.8.xml b/utilities/ovn-nbctl.8.xml
> > index ea2b201a5..9e31d1cf0 100644
> > --- a/utilities/ovn-nbctl.8.xml
> > +++ b/utilities/ovn-nbctl.8.xml
> > @@ -1175,7 +1175,7 @@
> >       <h2>NAT Commands</h2>
> >
> >       <dl>
> > -      <dt>[<code>--may-exist</code>] [<code>--stateless</code>]
> [<code>--gateway-port</code>=<var>GATEWAY_PORT</var>] [--portrange]
> <code>lr-nat-add</code> <var>router</var> <var>type</var>
> <var>external_ip</var> <var>logical_ip</var> [<var>logical_port</var>
> <var>external_mac</var>] [<var>external_port_range</var>]</dt>
> > +      <dt>[<code>--may-exist</code>] [<code>--stateless</code>]
> [<code>--gateway-port</code>=<var>GATEWAY_PORT</var>] [--portrange]
> [<code>--match</code>=<var>MATCH</var>]
> [<code>--priority</code>=<var>PRIORITY</var>] <code>lr-nat-add</code>
> <var>router</var> <var>type</var> <var>external_ip</var>
> <var>logical_ip</var> [<var>logical_port</var> <var>external_mac</var>]
> [<var>external_port_range</var>]</dt>
> >         <dd>
> >           <p>
> >             Adds the specified NAT to <var>router</var>.
> > @@ -1224,6 +1224,18 @@
> >             <code>external_port_range</code> is <code>1-65535</code>.
> >           </p>
> >
> > +        <p>
> > +          The <code>--match</code> allows to specify the extra match
> condition.
> > +          The extra match is for more fine-grained control over the NAT
> rule.
> > +        </p>
> > +
> > +        <p>
> > +          The <code>--priority</code> option allows to specify order of
> NAT
> > +          rule evaluation. Priority must be between <code>0</code> and
> > +          <code>32767</code>, inclusive and can be only specified
> together
> > +          with <code>--match</code>.
> > +        </p>
> > +
> >           <p>
> >             When <var>type</var> is <code>dnat</code>, the externally
> >             visible IP address <var>external_ip</var> is DNATted to the
> > diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
> > index 618f3a18b..da31c45f1 100644
> > --- a/utilities/ovn-nbctl.c
> > +++ b/utilities/ovn-nbctl.c
> > @@ -4981,6 +4981,8 @@ nbctl_pre_lr_nat_add(struct ctl_context *ctx)
> >       ovsdb_idl_add_column(ctx->idl, &nbrec_nat_col_logical_port);
> >       ovsdb_idl_add_column(ctx->idl, &nbrec_nat_col_external_mac);
> >       ovsdb_idl_add_column(ctx->idl, &nbrec_nat_col_gateway_port);
> > +    ovsdb_idl_add_column(ctx->idl, &nbrec_nat_col_match);
> > +    ovsdb_idl_add_column(ctx->idl, &nbrec_nat_col_priority);
> >       ovsdb_idl_add_column(ctx->idl, &nbrec_nat_col_options);
> >
> >       ovsdb_idl_add_column(ctx->idl,
> &nbrec_logical_router_port_col_name);
> > @@ -5177,6 +5179,21 @@ nbctl_lr_nat_add(struct ctl_context *ctx)
> >           }
> >       }
> >
> > +    int64_t priority = 0;
> > +    const char *match = shash_find_data(&ctx->options, "--match");
> > +    const char *priority_opt = shash_find_data(&ctx->options,
> "--priority");
> > +
> > +    if (!match && priority_opt) {
> > +        ctl_error(ctx, "priority can be set only with --match option.");
> > +        goto cleanup;
> > +    } else if (match && priority_opt) {
> > +        error = parse_priority(priority_opt, &priority);
> > +        if (error) {
> > +            ctx->error = error;
> > +            goto cleanup;
> > +        }
> > +    }
> > +
> >       for (size_t i = 0; i < lr->n_nat; i++) {
> >           const struct nbrec_nat *nat = lr->nat[i];
> >
> > @@ -5207,6 +5224,10 @@ nbctl_lr_nat_add(struct ctl_context *ctx)
> >                               nbrec_nat_verify_external_mac(nat);
> >                               nbrec_nat_set_logical_port(nat,
> logical_port);
> >                               nbrec_nat_set_external_mac(nat,
> external_mac);
> > +                            if (match) {
> > +                                nbrec_nat_set_match(nat, match);
> > +                                nbrec_nat_set_priority(nat, priority);
> > +                            }
> >                               should_return = true;
> >                           } else {
> >                               ctl_error(ctx, "%s, %s: a NAT with this "
> > @@ -5215,12 +5236,19 @@ nbctl_lr_nat_add(struct ctl_context *ctx)
> >                                         new_logical_ip);
> >                               should_return = true;
> >                           }
> > -                } else {
> > -                    ctl_error(ctx, "a NAT with this type (%s), %s (%s) "
> > -                              "already exists",
> > -                              nat_type,
> > -                              is_snat ? "logical_ip" : "external_ip",
> > -                              is_snat ? new_logical_ip :
> new_external_ip);
> > +                } else if (!match || !strcmp(nat->match, match)) {
> > +                    if (match) {
> > +                        ctl_error(ctx, "a NAT with this type (%s), %s
> (%s) "
> > +                                   "and match \"%s\" already exists",
> nat_type,
> > +                                  is_snat ? "logical_ip" :
> "external_ip",
> > +                                  is_snat ? new_logical_ip :
> new_external_ip,
> > +                                  match);
> > +                    } else {
> > +                        ctl_error(ctx, "a NAT with this type (%s), %s
> (%s) "
> > +                                       "already exists", nat_type,
> > +                                  is_snat ? "logical_ip" :
> "external_ip",
> > +                                  is_snat ? new_logical_ip :
> new_external_ip);
> > +                    }
> >                       should_return = true;
> >                   }
> >               }
> > @@ -5273,6 +5301,11 @@ nbctl_lr_nat_add(struct ctl_context *ctx)
> >       }
> >       nbrec_nat_set_options(nat, &nat_options);
> >
> > +    if (match) {
> > +        nbrec_nat_set_match(nat, match);
> > +        nbrec_nat_set_priority(nat, priority);
> > +    }
> > +
> >       smap_destroy(&nat_options);
> >
> >       /* Insert the NAT into the logical router. */
> > @@ -5293,30 +5326,79 @@ nbctl_pre_lr_nat_del(struct ctl_context *ctx)
> >       ovsdb_idl_add_column(ctx->idl, &nbrec_nat_col_logical_ip);
> >       ovsdb_idl_add_column(ctx->idl, &nbrec_nat_col_external_ip);
> >       ovsdb_idl_add_column(ctx->idl, &nbrec_nat_col_gateway_port);
> > +    ovsdb_idl_add_column(ctx->idl, &nbrec_nat_col_match);
> >
> >       ovsdb_idl_add_column(ctx->idl,
> &nbrec_logical_router_port_col_name);
> >   }
> >
> > +static size_t
> > +lr_nat_del_matching(const struct nbrec_logical_router *lr, const char
> *type,
> > +                    const char *ip,
> > +                    const struct nbrec_logical_router_port *dgw_port,
> > +                    const char *match, bool is_snat)
> > +{
> > +    size_t n_deleted = 0;
> > +
> > +    for (size_t i = 0; i < lr->n_nat; i++) {
> > +        struct nbrec_nat *nat = lr->nat[i];
> > +        char *old_ip = normalize_prefix_str(is_snat
> > +                                            ? nat->logical_ip
> > +                                            : nat->external_ip);
> > +        if (!old_ip) {
> > +            continue;
> > +        }
> > +
> > +        bool delete = true;
> > +
> > +        if (type && strcmp(type, nat->type)) {
> > +            delete = false;
> > +        }
> > +
> > +        if (ip && strcmp(ip, old_ip)) {
> > +            delete = false;
> > +        }
> > +
> > +        if (dgw_port && nat->gateway_port != dgw_port) {
> > +            delete = false;
> > +        }
> > +
> > +        if (match && strcmp(match, nat->match)) {
> > +            delete = false;
> > +        }
> > +
> > +        if (delete) {
> > +            nbrec_logical_router_update_nat_delvalue(lr, nat);
> > +            n_deleted++;
> > +        }
> > +
> > +        free(old_ip);
> > +    }
> > +
> > +    return n_deleted;
> > +}
> > +
> >   static void
> >   nbctl_lr_nat_del(struct ctl_context *ctx)
> >   {
> >       const struct nbrec_logical_router *lr = NULL;
> >       bool must_exist = !shash_find(&ctx->options, "--if-exists");
> > +    const char *match = shash_find_data(&ctx->options, "--match");
> >       char *error = lr_by_name_or_uuid(ctx, ctx->argv[1], true, &lr);
> >       if (error) {
> >           ctx->error = error;
> >           return;
> >       }
> >
> > -    if (ctx->argc == 2) {
> > -        /* If type, external_ip and logical_ip are not specified, delete
> > -         * all NATs. */
> > +    if (ctx->argc == 2 && !match) {
> > +        /* If type, external_ip, logical_ip and match are not specified,
> > +         * delete all NATs. */
> >           nbrec_logical_router_verify_nat(lr);
> >           nbrec_logical_router_set_nat(lr, NULL, 0);
> >           return;
> >       }
> >
> >       const char *nat_type = ctx->argv[2];
> > +    int is_snat = !strcmp("snat", nat_type);
> >       if (strcmp(nat_type, "dnat") && strcmp(nat_type, "snat")
> >               && strcmp(nat_type, "dnat_and_snat")) {
> >           ctl_error(ctx, "%s: type must be one of \"dnat\", \"snat\" and
> "
> > @@ -5326,16 +5408,11 @@ nbctl_lr_nat_del(struct ctl_context *ctx)
> >
> >       if (ctx->argc == 3) {
> >           /*Deletes all NATs with the specified type. */
> > -        for (size_t i = 0; i < lr->n_nat; i++) {
> > -            if (!strcmp(nat_type, lr->nat[i]->type)) {
> > -                nbrec_logical_router_update_nat_delvalue(lr,
> lr->nat[i]);
> > -            }
> > -        }
> > +        lr_nat_del_matching(lr, nat_type, NULL, NULL, match, is_snat);
> >           return;
> >       }
> >
> >       char *nat_ip = normalize_prefix_str(ctx->argv[3]);
> > -    int is_snat = !strcmp("snat", nat_type);
> >       const struct nbrec_logical_router_port *dgw_port = NULL;
> >
> >       if (ctx->argc == 4) {
> > @@ -5346,32 +5423,9 @@ nbctl_lr_nat_del(struct ctl_context *ctx)
> >                   ctx->error = error;
> >                   return;
> >               }
> > -
> > -            /* Deletes all NATs matching the type and gateway_port
> > -             * specified. */
> > -            for (size_t i = 0; i < lr->n_nat; i++) {
> > -                if (!strcmp(nat_type, lr->nat[i]->type) &&
> > -                    lr->nat[i]->gateway_port == dgw_port) {
> > -                    nbrec_logical_router_update_nat_delvalue(lr,
> lr->nat[i]);
> > -                }
> > -            }
> > -            return;
> >           }
> >
> > -        /* Remove NAT rules matching the type and IP (based on type). */
> > -        for (size_t i = 0; i < lr->n_nat; i++) {
> > -            struct nbrec_nat *nat = lr->nat[i];
> > -            char *old_ip = normalize_prefix_str(is_snat
> > -                                                ? nat->logical_ip
> > -                                                : nat->external_ip);
> > -            if (!old_ip) {
> > -                continue;
> > -            }
> > -            if (!strcmp(nat_type, nat->type) && !strcmp(nat_ip,
> old_ip)) {
> > -                nbrec_logical_router_update_nat_delvalue(lr, nat);
> > -            }
> > -            free(old_ip);
> > -        }
> > +        lr_nat_del_matching(lr, nat_type, nat_ip, dgw_port, match,
> is_snat);
> >           goto cleanup;
> >       }
> >
> > @@ -5386,32 +5440,22 @@ nbctl_lr_nat_del(struct ctl_context *ctx)
> >           goto cleanup;
> >       }
> >
> > -    /* Remove matching NAT. */
> > -    for (size_t i = 0; i < lr->n_nat; i++) {
> > -        struct nbrec_nat *nat = lr->nat[i];
> > -        bool should_return = false;
> > -        char *old_ip = normalize_prefix_str(is_snat
> > -                                            ? nat->logical_ip
> > -                                            : nat->external_ip);
> > -        if (!old_ip) {
> > -            continue;
> > -        }
> > -        if (!strcmp(nat_type, nat->type) && !strcmp(nat_ip, old_ip) &&
> > -            nat->gateway_port == dgw_port) {
> > -            nbrec_logical_router_update_nat_delvalue(lr, nat);
> > -            should_return = true;
> > -        }
> > -        free(old_ip);
> > -        if (should_return) {
> > -            goto cleanup;
> > +    size_t n_deleted = lr_nat_del_matching(lr, nat_type, nat_ip,
> dgw_port,
> > +                                           match, is_snat);
> > +    if (must_exist && !n_deleted) {
> > +        struct ds ds = DS_EMPTY_INITIALIZER;
> > +        ds_put_format(&ds, "no matching NAT with the type (%s), %s
> (%s)",
> > +                      nat_type, is_snat ? "logical_ip" : "external_ip",
> > +                      nat_ip);
> > +        if (match) {
> > +            ds_put_format(&ds, ", gateway_port (%s) and match (%s)",
> > +                          ctx->argv[4], match);
> > +        } else {
> > +            ds_put_format(&ds, " and gateway_port (%s)", ctx->argv[4]);
> >           }
> > -    }
> >
> > -    if (must_exist) {
> > -        ctl_error(ctx, "no matching NAT with the type (%s), %s (%s) and
> "
> > -                  "gateway_port (%s)", nat_type,
> > -                  is_snat ? "logical_ip" : "external_ip", nat_ip,
> > -                  ctx->argv[4]);
> > +        ctx->error = xstrdup(ds_cstr_ro(&ds));
> > +        ds_destroy(&ds);
> >       }
> >
> >   cleanup:
> > @@ -5431,6 +5475,7 @@ nbctl_pre_lr_nat_list(struct ctl_context *ctx)
> >       ovsdb_idl_add_column(ctx->idl, &nbrec_nat_col_external_mac);
> >       ovsdb_idl_add_column(ctx->idl, &nbrec_nat_col_logical_port);
> >       ovsdb_idl_add_column(ctx->idl, &nbrec_nat_col_gateway_port);
> > +    ovsdb_idl_add_column(ctx->idl, &nbrec_nat_col_match);
> >
> >       ovsdb_idl_add_column(ctx->idl,
> &nbrec_logical_router_port_col_name);
> >   }
> > @@ -5448,11 +5493,12 @@ nbctl_lr_nat_list(struct ctl_context *ctx)
> >       struct smap lr_nats = SMAP_INITIALIZER(&lr_nats);
> >       for (size_t i = 0; i < lr->n_nat; i++) {
> >           const struct nbrec_nat *nat = lr->nat[i];
> > -        char *key = xasprintf("%-17.13s%-22.18s%s",
> > +        char *key = xasprintf("%-17.13s%-22.18s%-22.18s%-19.15s",
> >                                 nat->type,
> >                                 nat->gateway_port
> >                                 ? nat->gateway_port->name
> >                                 : "",
> > +                              nat->match,
> >                                 nat->external_ip);
> >           if (nat->external_mac && nat->logical_port) {
> >               smap_add_format(&lr_nats, key,
> > @@ -5472,13 +5518,13 @@ nbctl_lr_nat_list(struct ctl_context *ctx)
> >       const struct smap_node **nodes = smap_sort(&lr_nats);
> >       if (nodes) {
> >           ds_put_format(&ctx->output,
> > -                "%-17.13s%-22.18s%-19.15s%-17.13s%-20.16s%-21.17s%s\n",
> > -                "TYPE", "GATEWAY_PORT", "EXTERNAL_IP", "EXTERNAL_PORT",
> > -                "LOGICAL_IP", "EXTERNAL_MAC", "LOGICAL_PORT");
> > +
> "%-17.13s%-22.18s%-22.18s%-19.15s%-17.13s%-20.16s%-21.17s%s\n",
> > +                "TYPE", "GATEWAY_PORT", "MATCH", "EXTERNAL_IP",
> > +                "EXTERNAL_PORT", "LOGICAL_IP", "EXTERNAL_MAC",
> "LOGICAL_PORT");
> >           for (size_t i = 0; i < smap_count(&lr_nats); i++) {
> >               const struct smap_node *node = nodes[i];
> > -            ds_put_format(&ctx->output, "%-58.54s%s\n",
> > -                    node->key, node->value);
> > +            ds_put_format(&ctx->output, "%-80.86s%s\n",
> > +                          node->key, node->value);
> >           }
> >           free(nodes);
> >       }
> > @@ -8022,9 +8068,10 @@ static const struct ctl_command_syntax
> nbctl_commands[] = {
> >         "[LOGICAL_PORT EXTERNAL_MAC] [EXTERNAL_PORT_RANGE]",
> >         nbctl_pre_lr_nat_add, nbctl_lr_nat_add,
> >         NULL, "--may-exist,--stateless,--portrange,--add-route,"
> > -      "--gateway-port=", RW },
> > +      "--gateway-port=,--priority=,--match=", RW },
> >       { "lr-nat-del", 1, 4, "ROUTER [TYPE [IP] [GATEWAY_PORT]]",
> > -      nbctl_pre_lr_nat_del, nbctl_lr_nat_del, NULL, "--if-exists", RW },
> > +      nbctl_pre_lr_nat_del, nbctl_lr_nat_del, NULL,
> > +      "--if-exists,--match=", RW },
> >       { "lr-nat-list", 1, 1, "ROUTER", nbctl_pre_lr_nat_list,
> >         nbctl_lr_nat_list, NULL, "", RO },
> >       { "lr-nat-update-ext-ip", 4, 4, "ROUTER TYPE IP ADDRESS_SET",
>
>
Thanks,
Ales

-- 

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

Reply via email to