On Thu, Feb 17, 2022 at 12:34 PM Lorenzo Bianconi
<[email protected]> wrote:
>
> Introduce nat-only-router flag for nat-addresses option in order to
> advertise in GARPs packets all SNAT/DNAT configured in the connected
> logical router but skip all configured load_balancers.
>
> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2054394
> Signed-off-by: Lorenzo Bianconi <[email protected]>
Hi Lorenzo,
This patch needs a rebase. Looking into the linked BZ, it seems like
an issue from ovn-kubernetes point of view.
Can you please update the commit message with the issue faced. Maybe
this can be a candidate for the backport.
> ---
> northd/northd.c | 53 ++++++++++++++++++++++++++++---------------------
> ovn-nb.xml | 21 ++++++++++++++++++++
> tests/ovn.at | 38 +++++++++++++++++++++++++++++++++++
> 3 files changed, 89 insertions(+), 23 deletions(-)
>
> diff --git a/northd/northd.c b/northd/northd.c
> index 4f9c10648..e6483658e 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -1641,13 +1641,13 @@ destroy_routable_addresses(struct
> ovn_port_routable_addresses *ra)
> }
>
> static char **get_nat_addresses(const struct ovn_port *op, size_t *n,
> - bool routable_only);
> + bool routable_only, bool nat_only);
>
> static void
> assign_routable_addresses(struct ovn_port *op)
> {
> size_t n;
> - char **nats = get_nat_addresses(op, &n, true);
> + char **nats = get_nat_addresses(op, &n, true, false);
>
> if (!nats) {
> return;
> @@ -2711,7 +2711,8 @@ join_logical_ports(struct northd_input *input_data,
> * The caller must free each of the n returned strings with free(),
> * and must free the returned array when it is no longer needed. */
> static char **
> -get_nat_addresses(const struct ovn_port *op, size_t *n, bool routable_only)
> +get_nat_addresses(const struct ovn_port *op, size_t *n, bool routable_only,
> + bool nat_only)
The argument 'nat_only' is used more as a negation below. So I'd
suggest changing the name
as - "include_lb_ips" or a better name if you've in mind.
Otherwise the patch LGTM.
Thanks
Numan
> {
> size_t n_nats = 0;
> struct eth_addr mac;
> @@ -2791,24 +2792,26 @@ get_nat_addresses(const struct ovn_port *op, size_t
> *n, bool routable_only)
> }
> }
>
> - const char *ip_address;
> - if (routable_only) {
> - SSET_FOR_EACH (ip_address, &op->od->lb_ips_v4_routable) {
> - ds_put_format(&c_addresses, " %s", ip_address);
> - central_ip_address = true;
> - }
> - SSET_FOR_EACH (ip_address, &op->od->lb_ips_v6_routable) {
> - ds_put_format(&c_addresses, " %s", ip_address);
> - central_ip_address = true;
> - }
> - } else {
> - SSET_FOR_EACH (ip_address, &op->od->lb_ips_v4) {
> - ds_put_format(&c_addresses, " %s", ip_address);
> - central_ip_address = true;
> - }
> - SSET_FOR_EACH (ip_address, &op->od->lb_ips_v6) {
> - ds_put_format(&c_addresses, " %s", ip_address);
> - central_ip_address = true;
> + if (!nat_only) {
> + const char *ip_address;
> + if (routable_only) {
> + SSET_FOR_EACH (ip_address, &op->od->lb_ips_v4_routable) {
> + ds_put_format(&c_addresses, " %s", ip_address);
> + central_ip_address = true;
> + }
> + SSET_FOR_EACH (ip_address, &op->od->lb_ips_v6_routable) {
> + ds_put_format(&c_addresses, " %s", ip_address);
> + central_ip_address = true;
> + }
> + } else {
> + SSET_FOR_EACH (ip_address, &op->od->lb_ips_v4) {
> + ds_put_format(&c_addresses, " %s", ip_address);
> + central_ip_address = true;
> + }
> + SSET_FOR_EACH (ip_address, &op->od->lb_ips_v6) {
> + ds_put_format(&c_addresses, " %s", ip_address);
> + central_ip_address = true;
> + }
> }
> }
>
> @@ -3375,10 +3378,14 @@ ovn_port_update_sbrec(struct northd_input *input_data,
> char **nats = NULL;
> bool l3dgw_ports = op->peer && op->peer->od &&
> op->peer->od->n_l3dgw_ports;
> - if (nat_addresses && !strcmp(nat_addresses, "router")) {
> + if (nat_addresses &&
> + (!strcmp(nat_addresses, "router") ||
> + !strcmp(nat_addresses, "nat-only-router"))) {
> if (op->peer && op->peer->od
> && (chassis || op->peer->od->n_l3dgw_ports)) {
> - nats = get_nat_addresses(op->peer, &n_nats, false);
> + nats = get_nat_addresses(op->peer, &n_nats, false,
> + !strcmp(nat_addresses,
> + "nat-only-router"));
> }
> /* Only accept manual specification of ethernet address
> * followed by IPv4 addresses on type "l3gateway" ports. */
> diff --git a/ovn-nb.xml b/ovn-nb.xml
> index e8aa8b863..e5947ac63 100644
> --- a/ovn-nb.xml
> +++ b/ovn-nb.xml
> @@ -913,6 +913,27 @@
> </p>
> </dd>
>
> + <dt><code>nat-only-router</code></dt>
> + <dd>
> + <p>
> + Gratuitous ARPs will be sent for all SNAT and DNAT external
> IP
> + addresses defined on the <ref column="options"
> + key="router-port"/>'s logical router, using the
> + <ref column="options" key="router-port"/>'s MAC address.
> + </p>
> +
> + <p>
> + This form of <ref column="options" key="nat-addresses"/> is
> + valid for logical switch ports where <ref column="options"
> + key="router-port"/> is the name of a port on a gateway
> router,
> + or the name of a distributed gateway port.
> + </p>
> +
> + <p>
> + Supported only in OVN 2.21.12.1 and later.
> + </p>
> + </dd>
> +
> <dt><code>Ethernet address followed by one or more IPv4
> addresses</code></dt>
> <dd>
> <p>
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 184594831..b80ab803f 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -8651,6 +8651,44 @@
> expected="fffffffffffff0000000000108060001080006040001f00000000001c0a80003000000
> echo $expected >> expout
> AT_CHECK([sort packets], [0], [expout])
>
> +# Temporarily remove nat-addresses option to avoid race conditions
> +# due to GARP backoff
> +ovn-nbctl lsp-set-options lrp0-rp router-port=lrp0 nat-addresses=""
> +
> +reset_pcap_file() {
> + local iface=$1
> + local pcap_file=$2
> + ovs-vsctl -- set Interface $iface options:tx_pcap=dummy-tx.pcap \
> +options:rxq_pcap=dummy-rx.pcap
> + rm -f ${pcap_file}*.pcap
> + ovs-vsctl -- set Interface $iface options:tx_pcap=${pcap_file}-tx.pcap \
> +options:rxq_pcap=${pcap_file}-rx.pcap
> +}
> +
> +as hv1 reset_pcap_file snoopvif hv1/snoopvif
> +
> +# Re-add nat-addresses option
> +ovn-nbctl lsp-set-options lrp0-rp router-port=lrp0
> nat-addresses="nat-only-router"
> +
> +# Wait for packets to be received.
> +OVS_WAIT_UNTIL([test `wc -c < "hv1/snoopvif-tx.pcap"` -ge 250])
> +trim_zeros() {
> + sed 's/\(00\)\{1,\}$//'
> +}
> +
> +$PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/snoopvif-tx.pcap |
> trim_zeros > packets
> +g0="fffffffffffff0000000000108060001080006040001f00000000001c0a80001000000000000c0a80001"
> +echo $g0 > expout
> +g1="fffffffffffff0000000000108060001080006040001f00000000001c0a80002000000000000c0a80002"
> +echo $g1 >> expout
> +
> +grep $g0 packets | head -1 > exp
> +grep $g1 packets | head -1 >> exp
> +AT_CHECK([cat exp], [0], [expout])
> +
> +g3="fffffffffffff0000000000108060001080006040001f00000000001c0a80003000000000000c0a80003"
> +AT_CHECK([grep -q $g3 packets], [1])
> +
> OVN_CLEANUP([hv1])
>
> AT_CLEANUP
> --
> 2.35.1
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
