Re: [ovs-dev] [PATCH ovn 2/3] nb: Add support for match and priority in NAT.

2024-05-28 Thread Ales Musil
On Tue, May 28, 2024 at 9:52 PM Mark Michelson  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 
> > ---
> >   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
> > 
> >   
> >
> > +
> > +  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  > + db="OVN_Southbound"/> column in the OVN
> > +  Southbound database's  db="OVN_Southbound"/>
> > +  table.  This allows for more fine-grained control over the NAT
> rule.
> > +
> > +
> > +
> > +  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 match is defined.
> > +
> > +
> >   
> > 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_PORTLOGICAL_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:03lp0
> > -dnat_and_snat  fd01::1
>fd11::2
> > -dnat_and_snat  fd01::2
>fd11::3 00:00:00:01:02:03lp0
> > -snat   30.0.0.1
> 192.168.1.0/24
> > -snat   fd01::1
>fd11::/64
> > +TYPE GATEWAY_PORT  MATCH
>  EXTERNAL_IPEXTERNAL_PORTLOGICAL_IP  EXTERNAL_MAC
>LOGICAL_PORT
> > +dnat 30.0.0.1
>   192.168.1.2
> > +dnat fd01::1
> 

Re: [ovs-dev] [PATCH ovn 2/3] nb: Add support for match and priority in NAT.

2024-05-28 Thread Mark Michelson

Thanks Ales, I have just one question about this.

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?


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 
---
  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

  
  
+

+  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  column in the OVN
+  Southbound database's 
+  table.  This allows for more fine-grained control over the NAT rule.
+
+
+
+  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 match is defined.
+
+
  
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_IPEXTERNAL_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:03lp0
-dnat_and_snat  fd01::1 
fd11::2
-dnat_and_snat  fd01::2 
fd11::3 00:00:00:01:02:03lp0
-snat   30.0.0.1
192.168.1.0/24
-snat   fd01::1 
fd11::/64
+TYPE GATEWAY_PORT  MATCH EXTERNAL_IP   
 EXTERNAL_PORTLOGICAL_IP  EXTERNAL_MAC LOGICAL_PORT
+dnat 30.0.0.1  
  192.168.1.2
+dnat fd01::1   
  fd11::2
+dnat_and_snat30.0.0.1  
  192.168.1.2
+dnat_and_snat30.0.0.2  
  192.168.1.3 00:00:00:01:02:03lp0
+dnat_and_snatfd01::1   
  fd11::2
+dnat_and_snatfd01::2   
  

Re: [ovs-dev] [PATCH ovn 2/3] nb: Add support for match and priority in NAT.

2024-05-03 Thread 0-day Robot
Bleep bloop.  Greetings Ales Musil, I am a robot and I have tried out your 
patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Line is 408 characters long (recommended limit is 79)
#374 FILE: utilities/ovn-nbctl.8.xml:1178:
  [--may-exist] [--stateless] 
[--gateway-port=GATEWAY_PORT] [--portrange] 
[--match=MATCH] 
[--priority=PRIORITY] lr-nat-add 
router type external_ip logical_ip 
[logical_port external_mac] 
[external_port_range]

Lines checked: 720, Warnings: 1, Errors: 0


Please check this out.  If you feel there has been an error, please email 
acon...@redhat.com

Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev