> > > >> >>> This is achieved by extending >>> the syntax for "options:nat-addresses" in the southbound database, >>> allowing the condition 'is_chassis_resident("LPORT_NAME")' to be >>> appended >>> after the MAC and IP addresses. This condition is automatically inserted >>> by ovn-northd when the northbound "options:nat-addresses" is set to >>> "router" and the peer is a distributed gateway port. >>> >>> A separate patch will be required to support gratuitous ARP for >>> distributed NAT rules that specify logical_port and external_mac. Since >>> the MAC address differs and the logical port often resides on a different >>> chassis from the redirect-chassis, these addresses cannot be included in >>> the same "nat-addresses" string as for centralized NAT rules. >>> >>> Signed-off-by: Mickey Spiegel <mickeys....@gmail.com> >>> --- >>> ovn/controller/pinctrl.c | 104 ++++++++++++++++++++++++++++++ >>> ++++++++++++++--- >>> ovn/lib/ovn-util.c | 38 ++++++++++++++--- >>> ovn/lib/ovn-util.h | 2 + >>> ovn/northd/ovn-northd.c | 52 +++++++++++++++++------- >>> ovn/ovn-nb.xml | 33 ++++++++++++--- >>> ovn/ovn-sb.xml | 31 ++++++++++---- >>> tests/ovn.at | 70 +++++++++++++++++++++++++++++++ >>> 7 files changed, 289 insertions(+), 41 deletions(-) >>> >>> diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c >>> index b342189..08af792 100644 >>> --- a/ovn/controller/pinctrl.c >>> +++ b/ovn/controller/pinctrl.c >>> @@ -37,6 +37,7 @@ >>> #include "lib/dhcp.h" >>> #include "ovn-controller.h" >>> #include "ovn/actions.h" >>> +#include "ovn/lex.h" >>> #include "ovn/lib/logical-fields.h" >>> #include "ovn/lib/ovn-dhcp.h" >>> #include "ovn/lib/ovn-util.h" >>> @@ -1049,7 +1050,8 @@ send_garp_update(const struct sbrec_port_binding >>> *binding_rec, >>> >>> volatile struct garp_data *garp = NULL; >>> /* Update GARP for NAT IP if it exists. */ >>> - if (!strcmp(binding_rec->type, "l3gateway")) { >>> + if (!strcmp(binding_rec->type, "l3gateway") >>> + || !strcmp(binding_rec->type, "patch")) { >>> >> A comment above on why we should also look at "patch" will be useful. >> > > Replace the comment above with something along the following lines? > /* Update GARP for NAT IP if it exists. Consider port bindings with type > * "l3gateway" for logical switch ports attached to gateway routers, and > * port bindings with type "patch" for logical switch ports attached to > * distributed gateway ports. */ >
Looks good. > > >> >>> struct lport_addresses *laddrs = NULL; >>> laddrs = shash_find_data(nat_addresses, >>> binding_rec->logical_port); >>> if (!laddrs) { >>> @@ -1202,24 +1204,101 @@ get_localnet_vifs_l3gwports(const struct >>> ovsrec_bridge *br_int, >>> >>> const struct local_datapath *ld; >>> HMAP_FOR_EACH (ld, hmap_node, local_datapaths) { >>> - if (!ld->has_local_l3gateway) { >>> + if (!ld->localnet_port) { >>> continue; >>> } >>> >>> for (size_t i = 0; i < ld->ldatapath->n_lports; i++) { >>> const struct sbrec_port_binding *pb = >>> ld->ldatapath->lports[i]; >>> - if (!strcmp(pb->type, "l3gateway") >>> - /* && it's on this chassis */) { >>> + if ((ld->has_local_l3gateway && !strcmp(pb->type, >>> "l3gateway")) >>> + || !strcmp(pb->type, "patch")) { >>> >> A comment above on why we are considering "patch" will be useful. >> > > Something along the lines? > /* Consider port bindings of type "l3gateway" that connect to gateway > routers, > * and port bindings of type "patch" since they might connect to > distributed > * gateway ports with NAT addresses. */ > Looks good > > >> Does local_datapaths include every patch port or is it only those that >> have l2 distributed gateway port instantiated on it? >> > > Local datapaths is constructed in ovn/controller/binding.c. Start with > every local port (VIF, l3gateway, l2gateway, chassisredirect) and then find > all connected datapaths by walking patch ports. > Okay. > diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml >>> index 46a25f6..1f0b003 100644 >>> --- a/ovn/ovn-nb.xml >>> +++ b/ovn/ovn-nb.xml >>> @@ -244,8 +244,7 @@ >>> <column name="options" key="nat-addresses"> >>> <p> >>> This is used to send gratuitous ARPs for SNAT and DNAT IP >>> - addresses via <code>localnet</code> and is valid for only L3 >>> - gateway ports. >>> + addresses via <code>localnet</code>. >>> >> >> localnet switch port is usually a separate logical_switch_port that is >> attached to the same switch. And this logical siwtch port is usually the >> one that is connected to the router, right? I think if we clarify it above, >> it will cause less confusion. >> > > This is used to send gratuitous ARPs for SNAT and DNAT IP > addresses via the <code>localnet</code> port that is attached > to the same logical switch. This option is specified on a logical > switch port that is connected to a gateway router, or a logical > switch port that is connected to a distributed gateway port on > a logical router. > Sounds good. > > >> >> >>> </p> >>> >>> <p> >>> @@ -264,6 +263,13 @@ >>> </p> >>> >>> <p> >>> + This form of <ref column="options" >>> key="nat-addresses"/> is >>> + valid for L3 gateway ports, and for logical switch ports >>> + where <ref column="options" key="router-port"/> is the >>> name >>> + of a distributed gateway port. >>> + </p> >>> + >>> >> The above is a little confusing. I think if it read as below, it will be >> more clear? >> >> This form of options:nat-addresses is valid for for logical switch >> ports where >> options:router-port is the port of a gateway router or the name of a >> distributed gateway port. >> > > That is better. > > >> >> >>> + <p> >>> Supported only in OVN 2.8 and later. Earlier versions >>> required >>> NAT addresses to be manually synchronized. >>> </p> >>> @@ -271,10 +277,17 @@ >>> >>> <dt><code>Ethernet address followed by one or more IPv4 >>> addresses</code></dt> >>> <dd> >>> - Example: <code>80:fa:5b:06:72:b7 158.36.44.22 >>> - 158.36.44.24</code>. This would result in generation of >>> - gratuitous ARPs for IP addresses 158.36.44.22 and >>> 158.36.44.24 >>> - with a MAC address of 80:fa:5b:06:72:b7. >>> + <p> >>> + Example: <code>80:fa:5b:06:72:b7 158.36.44.22 >>> + 158.36.44.24</code>. This would result in generation of >>> + gratuitous ARPs for IP addresses 158.36.44.22 and >>> 158.36.44.24 >>> + with a MAC address of 80:fa:5b:06:72:b7. >>> + </p> >>> + >>> + <p> >>> + This form of <ref column="options" >>> key="nat-addresses"/> is >>> + only valid for L3 gateway ports. >>> >> >> s/is only valid for L3 gateway ports/is only valid for a port of a >> gateway router/ is more clear? >> > > No opinion, so I can go with this. > > >> >>> + </p> >>> </dd> >>> </dl> >>> </column> >>> @@ -1166,6 +1179,14 @@ >>> peer logical switch's destination lookup flow on the >>> <code>redirect-chassis</code>. >>> </p> >>> + >>> + <p> >>> + When this option is specified and it is desired to generate >>> + gratuitous ARPs for NAT addresses, then the peer logical >>> switch >>> + port's <ref column="options" key="nat-addresses" >>> + table="Logical_Switch_Port"/> should be set to >>> + <code>router</code>. >>> + </p> >>> </column> >>> </group> >>> >>> diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml >>> index 4e95c80..2df45e8 100644 >>> --- a/ovn/ovn-sb.xml >>> +++ b/ovn/ovn-sb.xml >>> @@ -1862,6 +1862,20 @@ tcp.flags = RST; >>> ports must have reversed <ref column="logical_port"/> and >>> <code>peer</code> values. >>> </column> >>> + >>> + <column name="options" key="nat-addresses"> >>> + MAC address of the <code>patch</code> port followed by a list of >>> + SNAT and DNAT external IP addresses, followed by >>> + <code>is_chassis_resident("<var>lport</var>")</code>, where >>> + <var>lport</var> is the name of a logical port on the same >>> chassis >>> + where the corresponding NAT rules are applied. This is used to >>> + send gratuitous ARPs for SNAT and DNAT external IP addresses via >>> + <code>localnet</code>, from the chassis where <var>lport</var> >>> + resides. Example: <code>80:fa:5b:06:72:b7 158.36.44.22 >>> + 158.36.44.24</code>. This would result in generation of >>> + gratuitous ARPs for IP addresses 158.36.44.22 and 158.36.44.24 >>> + with a MAC address of 80:fa:5b:06:72:b7. >>> >> >> The example above does not include "is_chassis_resident". Can you add >> that? >> > > Yes, I can add that. > > >> >> >>> + </column> >>> </group> >>> >>> <group title="L3 Gateway Options"> >>> @@ -1883,15 +1897,14 @@ tcp.flags = RST; >>> The <code>chassis</code> in which the port resides. >>> </column> >>> >>> - <column name="options" key="nat-addresses"> >>> - MAC address of the <code>l3gateway</code> port followed by a >>> list of >>> - SNAT and DNAT IP addresses. This is used to send gratuitous >>> ARPs for >>> - SNAT and DNAT IP addresses via <code>localnet</code> and is >>> valid for >>> - only L3 gateway ports. Example: <code>80:fa:5b:06:72:b7 >>> 158.36.44.22 >>> - 158.36.44.24</code>. This would result in generation of >>> gratuitous >>> - ARPs for IP addresses 158.36.44.22 and 158.36.44.24 with a MAC >>> - address of 80:fa:5b:06:72:b7. >>> - </column> >>> + <column name="options" key="nat-addresses"> >>> + MAC address of the <code>l3gateway</code> port followed by a >>> list of >>> + SNAT and DNAT external IP addresses. This is used to send >>> gratuitous >>> + ARPs for SNAT and DNAT external IP addresses via >>> <code>localnet</code>. >>> + Example: <code>80:fa:5b:06:72:b7 158.36.44.22 >>> 158.36.44.24</code>. >>> + This would result in generation of gratuitous ARPs for IP >>> addresses >>> + 158.36.44.22 and 158.36.44.24 with a MAC address of >>> 80:fa:5b:06:72:b7. >>> + </column> >>> </group> >>> >>> <group title="Localnet Options"> >>> diff --git a/tests/ovn.at b/tests/ovn.at >>> index bbbec90..0915f7a 100644 >>> --- a/tests/ovn.at >>> +++ b/tests/ovn.at >>> @@ -6660,3 +6660,73 @@ OVN_CHECK_PACKETS([hv2/vif1-tx.pcap], >>> [hv2-vif1.expected]) >>> OVN_CLEANUP([hv1],[hv2],[hv3]) >>> >>> AT_CLEANUP >>> + >>> +AT_SETUP([ovn -- send gratuitous arp for NAT rules on distributed >>> router]) >>> +AT_SKIP_IF([test $HAVE_PYTHON = no]) >>> +ovn_start >>> +# Create logical switch >>> +ovn-nbctl ls-add ls0 >>> +# Create gateway router >>> +ovn-nbctl create Logical_Router name=lr0 >>> +# Add router port to gateway router >>> +ovn-nbctl lrp-add lr0 lrp0 f0:00:00:00:00:01 192.168.0.1/24 \ >>> + -- set Logical_Router_Port lrp0 options:redirect-chassis="hv2" >>> +ovn-nbctl lsp-add ls0 lrp0-rp -- set Logical_Switch_Port lrp0-rp \ >>> + type=router options:router-port=lrp0-rp addresses="router" >>> >> >> A fix above for 'options:router-port' to point to the router port. >> > > Should I create the patch to fix the other two places with this > problem, or will you do that? > Please go ahead and do it. _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev