diff mbox series

[ovs-dev] ovn-northd.c: Add proxy ARP support to OVN

Message ID fe4a6405-6a53-5d53-22fc-c5b922f9ab4a@oracle.com
State Changes Requested
Headers show
Series [ovs-dev] ovn-northd.c: Add proxy ARP support to OVN | expand

Commit Message

Brendan Doyle May 31, 2021, 9:06 a.m. UTC
From a9d3140845175edb7644b2d0d82a95bd6cf94662 Mon Sep 17 00:00:00 2001
From: Brendan Doyle <brendan.doyle@oracle.com>
Date: Fri, 28 May 2021 10:01:17 -0700
Subject: [PATCH ovn] ovn-northd.c: Add proxy ARP support to OVN

This patch provides the ability to configure proxy ARP IPs on a Logical
Switch Router port. The IPs are added as Options for router ports. This
provides a useful feature where traffic for a service must be sent to an
address in a logical network address space, but the service is provided
in a different network. For example an NFS service is provide to Logical
networks at an address in their Logical network space, but the NFS
server resides in a physical network. A Logical switch Router port can
be configured to respond to ARP requests sent to the service "Logical
address", the Logical Router/Gateway can then be configured to forward
the traffic to the underlay/physical network.

Signed-off-by: Brendan Doyle <brendan.doyle@oracle.com>
---
  northd/ovn-northd.c |  38 +++++++++++++++++++
  ovn-nb.xml          |   9 +++++
  tests/ovn.at        | 103 
++++++++++++++++++++++++++++++++++++++++++++++++++++
  3 files changed, 150 insertions(+)

+echo $expected | ovstest test-ovn expr-to-packets > expected
+
+OVN_CHECK_PACKETS([pa-hv/vif1-tx.pcap], [expected])
+
+OVN_CLEANUP([pa-hv])
+AT_CLEANUP
+])
--
1.8.3.1

Comments

Brendan Doyle June 2, 2021, 8:26 a.m. UTC | #1
Hi,

Just wondering what the process is here, I submitted this patch a while 
ago, and I see quite a few
other patches have been submitted since. So wondering what happens after 
a patch is submitted,
how do I know if it has been accepted/rejected/need more work, and how 
does it make it into the
repo if accepted. This does not seem to be documented in:

Submitting Patches — Open Virtual Network (OVN) 21.03.0 documentation 
<https://docs.ovn.org/en/stable/internals/contributing/submitting-patches.html>

Thanks

Brendan

On 31/05/2021 10:06, Brendan Doyle wrote:
> From a9d3140845175edb7644b2d0d82a95bd6cf94662 Mon Sep 17 00:00:00 2001
> From: Brendan Doyle <brendan.doyle@oracle.com>
> Date: Fri, 28 May 2021 10:01:17 -0700
> Subject: [PATCH ovn] ovn-northd.c: Add proxy ARP support to OVN
>
> This patch provides the ability to configure proxy ARP IPs on a Logical
> Switch Router port. The IPs are added as Options for router ports. This
> provides a useful feature where traffic for a service must be sent to an
> address in a logical network address space, but the service is provided
> in a different network. For example an NFS service is provide to Logical
> networks at an address in their Logical network space, but the NFS
> server resides in a physical network. A Logical switch Router port can
> be configured to respond to ARP requests sent to the service "Logical
> address", the Logical Router/Gateway can then be configured to forward
> the traffic to the underlay/physical network.
>
> Signed-off-by: Brendan Doyle <brendan.doyle@oracle.com>
> ---
>  northd/ovn-northd.c |  38 +++++++++++++++++++
>  ovn-nb.xml          |   9 +++++
>  tests/ovn.at        | 103 
> ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 150 insertions(+)
>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 0e5092a..a377e83 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -6943,6 +6943,7 @@ build_lswitch_arp_nd_responder_known_ips(struct 
> ovn_port *op,
>                                           struct ds *match)
>  {
>      if (op->nbsp) {
> +        const char *arp_proxy;
>          if (!strcmp(op->nbsp->type, "virtual")) {
>              /* Handle
>               *  - GARPs for virtual ip which belongs to a logical port
> @@ -7096,6 +7097,43 @@ build_lswitch_arp_nd_responder_known_ips(struct 
> ovn_port *op,
>                  }
>              }
>          }
> +
> +        /*
> +         * Add responses for ARP proxies.
> +         */
> +        arp_proxy = smap_get(&op->nbsp->options,"arp_proxy");
> +        if (arp_proxy && op->peer) {
> +            char *ips, *ip, *rest;
> +
> +            ips = xstrdup(arp_proxy);
> +            rest = ips;
> +
> +            while ((ip = strtok_r(rest,",", &rest))) {
> +                ds_clear(match);
> +                ds_put_format(match, "arp.tpa == %s && arp.op == 1", 
> ip);
> +
> +                ds_clear(actions);
> +                ds_put_format(actions,
> +                    "eth.dst = eth.src; "
> +                    "eth.src = %s; "
> +                    "arp.op = 2; /* ARP reply */ "
> +                    "arp.tha = arp.sha; "
> +                    "arp.sha = %s; "
> +                    "arp.tpa = arp.spa; "
> +                    "arp.spa = %s; "
> +                    "outport = inport; "
> +                    "flags.loopback = 1; "
> +                    "output;",
> +                    op->peer->lrp_networks.ea_s,
> +                    op->peer->lrp_networks.ea_s,
> +                    ip);
> +
> +                ovn_lflow_add_with_hint(lflows, op->od, 
> S_SWITCH_IN_ARP_ND_RSP,
> +                    50, ds_cstr(match), ds_cstr(actions),
> +                    &op->nbsp->header_);
> +            }
> +            free(ips);
> +        }
>      }
>  }
>
> diff --git a/ovn-nb.xml b/ovn-nb.xml
> index 02fd216..4b6c183 100644
> --- a/ovn-nb.xml
> +++ b/ovn-nb.xml
> @@ -848,6 +848,15 @@
>              </dd>
>            </dl>
>          </column>
> +
> +        <column name="options" key="arp_proxy">
> +          Optional. A comma separated list IPv4 addresses that this
> +          logical switch <code>router</code> port will reply to ARP 
> requests.
> +          Example: <code>169.254.239.254,169.254.239.2</code>. The
> +          <ref column="options" key="router-port"/>'s logical router 
> should
> +          have a route to forward packets sent to configured proxy 
> ARP IPs to
> +          an appropriate destination.
> +        </column>
>        </group>
>
>        <group title="Options for localnet ports">
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 2c3c36d..c675cc9 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -26527,3 +26527,106 @@ AT_CHECK([test $(ovn-appctl -t 
> ovn-controller coverage/read-counter lflow_run) =
>  OVN_CLEANUP([hv1])
>  AT_CLEANUP
>  ])
> +
> +OVN_FOR_EACH_NORTHD([
> +AT_SETUP([ovn -- proxy-arp: 1 HVs, 1 LSs, 1 lport/LS, 1 LR])
> +AT_KEYWORDS([proxy-arp])
> +ovn_start
> +
> +# Logical network:
> +# One LR - lr1 has switch ls1 (192.16.1.0/24) connected to it,
> +# and and one HV with IP 192.16.1.6.
> +
> +ovn-nbctl lr-add lr1
> +ovn-nbctl ls-add ls1
> +
> +# Connect ls1 to lr1
> +ovn-nbctl lrp-add lr1 ls1 00:00:00:01:02:f1 192.16.1.1/24
> +ovn-nbctl lsp-add ls1 rp-ls1 -- set Logical_Switch_Port rp-ls1 \
> +    type=router options:router-port=ls1 addresses=\"00:00:00:01:02:f1\"
> +
> +# Create logical port ls1-lp1 in ls1
> +ovn-nbctl lsp-add ls1 ls1-lp1 \
> +-- lsp-set-addresses ls1-lp1 "00:00:00:01:02:03 192.16.1.6"
> +
> +
> +# Create one hypervisor and create OVS ports corresponding to logical 
> ports.
> +net_add n1
> +
> +sim_add pa-hv
> +as pa-hv
> +ovs-vsctl add-br br-phys
> +ovn_attach n1 br-phys 192.16.0.1
> +
> +# Note: tx/rx are with respect to the LS port, so
> +# tx on switch port is HV rx, etc.
> +ovs-vsctl -- add-port br-int vif1 -- \
> +    set interface vif1 external-ids:iface-id=ls1-lp1 \
> +    options:tx_pcap=pa-hv/vif1-tx.pcap \
> +    options:rxq_pcap=pa-hv/vif1-rx.pcap \
> +    ofport-request=1
> +
> +# And proxy ARP flows for 69.254.239.254 and 169.254.239.2
> +# and check that SB flows have been added.
> +ovn-nbctl --wait=hv add Logical_Switch_Port rp-ls1 \
> +options arp_proxy='"169.254.239.254,169.254.239.2"'
> +ovn-sbctl dump-flows > sbflows
> +AT_CAPTURE_FILE([sbflows])
> +
> +AT_CHECK([ovn-sbctl dump-flows | grep ls_in_arp_rsp | grep 
> "169.254.239.2" | wc -l], [0], [dnl
> +2
> +])
> +
> +# Remove and check that the flows have been removed
> +ovn-nbctl --wait=hv remove Logical_Switch_Port rp-ls1 options 
> arp_proxy='"169.254.239.254,169.254.239.2"'
> +
> +AT_CHECK([ovn-sbctl dump-flows | grep ls_in_arp_rsp | grep 
> "169.254.239.2" | wc -l], [0], [dnl
> +0
> +])
> +
> +# Add the flows back send arp request and check we see an ARP response
> +ovn-nbctl --wait=hv add Logical_Switch_Port rp-ls1 \
> +options arp_proxy='"169.254.239.254,169.254.239.2"'
> +
> +ls1_p1_mac=00:00:00:01:02:03
> +ls1_p1_ip=192.16.1.6
> +
> +ls1_ro_mac=00:00:00:01:02:f1
> +ls1_ro_ip=192.168.1.1
> +
> +proxy_ip1=169.254.239.254
> +proxy_ip2=169.254.239.2
> +
> +bcast_mac=ff:ff:ff:ff:ff:ff
> +
> +# Send ARP request for 169.254.239.254
> +packet="inport==\"ls1-lp1\" && eth.src==$ls1_p1_mac && 
> eth.dst==$bcast_mac &&
> +       arp.op==1 && arp.sha==$ls1_p1_mac && arp.spa==$ls1_p1_ip &&
> +       arp.tha==$bcast_mac && arp.tpa==$proxy_ip1"
> +
> +as pa-hv ovs-appctl -t ovn-controller inject-pkt "$packet"
> +
> +ovs-ofctl dump-flows br-int| grep 169.254.239.254 | grep priority=50 
> > debug1
> +AT_CAPTURE_FILE([debug1])
> +
> +
> +# Check if packet hit the ARP reply ovs flow
> +AT_CHECK([ovs-ofctl dump-flows br-int | \
> +    grep "169.254.239.254" | \
> +    grep "priority=50" | \
> +    grep "arp_op=1" | \
> +    grep "n_packets=1" | wc -l], [0], [dnl
> +1
> +])
> +
> +# Check that the HV gets an ARP reply
> +expected="eth.src==$ls1_ro_mac && eth.dst==$ls1_p1_mac &&
> +       arp.op==2 && arp.sha==$ls1_ro_mac && arp.spa==$proxy_ip1 &&
> +       arp.tha==$ls1_p1_mac && arp.tpa==$ls1_p1_ip"
> +echo $expected | ovstest test-ovn expr-to-packets > expected
> +
> +OVN_CHECK_PACKETS([pa-hv/vif1-tx.pcap], [expected])
> +
> +OVN_CLEANUP([pa-hv])
> +AT_CLEANUP
> +])
> -- 
> 1.8.3.1
>
Dan Williams June 2, 2021, 2:42 p.m. UTC | #2
On Wed, 2021-06-02 at 09:26 +0100, Brendan Doyle wrote:
> Hi,
> 
> Just wondering what the process is here, I submitted this patch a while
> ago, and I see quite a few
> other patches have been submitted since. So wondering what happens
> after 
> a patch is submitted,
> how do I know if it has been accepted/rejected/need more work, and how 
> does it make it into the
> repo if accepted. This does not seem to be documented in:

Thanks for the patch; now that it's posted the patch is waiting for
review. You can track overall status here:


http://patchwork.ozlabs.org/project/ovn/list/?series=246447

but you'll also get replies to the patch with review comments. Reviews
may take a bit of time as the patch volume is fairly large, and
everyone is busy. But don't worry! It's on the list.

> 
> Submitting Patches — Open Virtual Network (OVN) 21.03.0 documentation
> < 
> https://docs.ovn.org/en/stable/internals/contributing/submitting-patches.html
> >

Good point, there should probably be a "What's next?" section.

---

One thing though, the patch seems to be linewrapped and won't apply
cleanly. Any chance you could fix that up by setting your email client
to "preformatted" before pasting the patch into the mail, or using git-
send-email?

Thanks,
Dan

> 
> Thanks
> 
> Brendan
> 
> On 31/05/2021 10:06, Brendan Doyle wrote:
> > From a9d3140845175edb7644b2d0d82a95bd6cf94662 Mon Sep 17 00:00:00
> > 2001
> > From: Brendan Doyle <brendan.doyle@oracle.com>
> > Date: Fri, 28 May 2021 10:01:17 -0700
> > Subject: [PATCH ovn] ovn-northd.c: Add proxy ARP support to OVN
> > 
> > This patch provides the ability to configure proxy ARP IPs on a
> > Logical
> > Switch Router port. The IPs are added as Options for router ports.
> > This
> > provides a useful feature where traffic for a service must be sent
> > to an
> > address in a logical network address space, but the service is
> > provided
> > in a different network. For example an NFS service is provide to
> > Logical
> > networks at an address in their Logical network space, but the NFS
> > server resides in a physical network. A Logical switch Router port
> > can
> > be configured to respond to ARP requests sent to the service
> > "Logical
> > address", the Logical Router/Gateway can then be configured to
> > forward
> > the traffic to the underlay/physical network.
> > 
> > Signed-off-by: Brendan Doyle <brendan.doyle@oracle.com>
> > ---
> >  northd/ovn-northd.c |  38 +++++++++++++++++++
> >  ovn-nb.xml          |   9 +++++
> >  tests/ovn.at        | 103 
> > ++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 150 insertions(+)
> > 
> > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> > index 0e5092a..a377e83 100644
> > --- a/northd/ovn-northd.c
> > +++ b/northd/ovn-northd.c
> > @@ -6943,6 +6943,7 @@
> > build_lswitch_arp_nd_responder_known_ips(struct 
> > ovn_port *op,
> >                                           struct ds *match)
> >  {
> >      if (op->nbsp) {
> > +        const char *arp_proxy;
> >          if (!strcmp(op->nbsp->type, "virtual")) {
> >              /* Handle
> >               *  - GARPs for virtual ip which belongs to a logical
> > port
> > @@ -7096,6 +7097,43 @@
> > build_lswitch_arp_nd_responder_known_ips(struct 
> > ovn_port *op,
> >                  }
> >              }
> >          }
> > +
> > +        /*
> > +         * Add responses for ARP proxies.
> > +         */
> > +        arp_proxy = smap_get(&op->nbsp->options,"arp_proxy");
> > +        if (arp_proxy && op->peer) {
> > +            char *ips, *ip, *rest;
> > +
> > +            ips = xstrdup(arp_proxy);
> > +            rest = ips;
> > +
> > +            while ((ip = strtok_r(rest,",", &rest))) {
> > +                ds_clear(match);
> > +                ds_put_format(match, "arp.tpa == %s && arp.op ==
> > 1", 
> > ip);
> > +
> > +                ds_clear(actions);
> > +                ds_put_format(actions,
> > +                    "eth.dst = eth.src; "
> > +                    "eth.src = %s; "
> > +                    "arp.op = 2; /* ARP reply */ "
> > +                    "arp.tha = arp.sha; "
> > +                    "arp.sha = %s; "
> > +                    "arp.tpa = arp.spa; "
> > +                    "arp.spa = %s; "
> > +                    "outport = inport; "
> > +                    "flags.loopback = 1; "
> > +                    "output;",
> > +                    op->peer->lrp_networks.ea_s,
> > +                    op->peer->lrp_networks.ea_s,
> > +                    ip);
> > +
> > +                ovn_lflow_add_with_hint(lflows, op->od, 
> > S_SWITCH_IN_ARP_ND_RSP,
> > +                    50, ds_cstr(match), ds_cstr(actions),
> > +                    &op->nbsp->header_);
> > +            }
> > +            free(ips);
> > +        }
> >      }
> >  }
> > 
> > diff --git a/ovn-nb.xml b/ovn-nb.xml
> > index 02fd216..4b6c183 100644
> > --- a/ovn-nb.xml
> > +++ b/ovn-nb.xml
> > @@ -848,6 +848,15 @@
> >              </dd>
> >            </dl>
> >          </column>
> > +
> > +        <column name="options" key="arp_proxy">
> > +          Optional. A comma separated list IPv4 addresses that
> > this
> > +          logical switch <code>router</code> port will reply to
> > ARP 
> > requests.
> > +          Example: <code>169.254.239.254,169.254.239.2</code>. The
> > +          <ref column="options" key="router-port"/>'s logical
> > router 
> > should
> > +          have a route to forward packets sent to configured proxy
> > ARP IPs to
> > +          an appropriate destination.
> > +        </column>
> >        </group>
> > 
> >        <group title="Options for localnet ports">
> > diff --git a/tests/ovn.at b/tests/ovn.at
> > index 2c3c36d..c675cc9 100644
> > --- a/tests/ovn.at
> > +++ b/tests/ovn.at
> > @@ -26527,3 +26527,106 @@ AT_CHECK([test $(ovn-appctl -t 
> > ovn-controller coverage/read-counter lflow_run) =
> >  OVN_CLEANUP([hv1])
> >  AT_CLEANUP
> >  ])
> > +
> > +OVN_FOR_EACH_NORTHD([
> > +AT_SETUP([ovn -- proxy-arp: 1 HVs, 1 LSs, 1 lport/LS, 1 LR])
> > +AT_KEYWORDS([proxy-arp])
> > +ovn_start
> > +
> > +# Logical network:
> > +# One LR - lr1 has switch ls1 (192.16.1.0/24) connected to it,
> > +# and and one HV with IP 192.16.1.6.
> > +
> > +ovn-nbctl lr-add lr1
> > +ovn-nbctl ls-add ls1
> > +
> > +# Connect ls1 to lr1
> > +ovn-nbctl lrp-add lr1 ls1 00:00:00:01:02:f1 192.16.1.1/24
> > +ovn-nbctl lsp-add ls1 rp-ls1 -- set Logical_Switch_Port rp-ls1 \
> > +    type=router options:router-port=ls1
> > addresses=\"00:00:00:01:02:f1\"
> > +
> > +# Create logical port ls1-lp1 in ls1
> > +ovn-nbctl lsp-add ls1 ls1-lp1 \
> > +-- lsp-set-addresses ls1-lp1 "00:00:00:01:02:03 192.16.1.6"
> > +
> > +
> > +# Create one hypervisor and create OVS ports corresponding to
> > logical 
> > ports.
> > +net_add n1
> > +
> > +sim_add pa-hv
> > +as pa-hv
> > +ovs-vsctl add-br br-phys
> > +ovn_attach n1 br-phys 192.16.0.1
> > +
> > +# Note: tx/rx are with respect to the LS port, so
> > +# tx on switch port is HV rx, etc.
> > +ovs-vsctl -- add-port br-int vif1 -- \
> > +    set interface vif1 external-ids:iface-id=ls1-lp1 \
> > +    options:tx_pcap=pa-hv/vif1-tx.pcap \
> > +    options:rxq_pcap=pa-hv/vif1-rx.pcap \
> > +    ofport-request=1
> > +
> > +# And proxy ARP flows for 69.254.239.254 and 169.254.239.2
> > +# and check that SB flows have been added.
> > +ovn-nbctl --wait=hv add Logical_Switch_Port rp-ls1 \
> > +options arp_proxy='"169.254.239.254,169.254.239.2"'
> > +ovn-sbctl dump-flows > sbflows
> > +AT_CAPTURE_FILE([sbflows])
> > +
> > +AT_CHECK([ovn-sbctl dump-flows | grep ls_in_arp_rsp | grep 
> > "169.254.239.2" | wc -l], [0], [dnl
> > +2
> > +])
> > +
> > +# Remove and check that the flows have been removed
> > +ovn-nbctl --wait=hv remove Logical_Switch_Port rp-ls1 options 
> > arp_proxy='"169.254.239.254,169.254.239.2"'
> > +
> > +AT_CHECK([ovn-sbctl dump-flows | grep ls_in_arp_rsp | grep 
> > "169.254.239.2" | wc -l], [0], [dnl
> > +0
> > +])
> > +
> > +# Add the flows back send arp request and check we see an ARP
> > response
> > +ovn-nbctl --wait=hv add Logical_Switch_Port rp-ls1 \
> > +options arp_proxy='"169.254.239.254,169.254.239.2"'
> > +
> > +ls1_p1_mac=00:00:00:01:02:03
> > +ls1_p1_ip=192.16.1.6
> > +
> > +ls1_ro_mac=00:00:00:01:02:f1
> > +ls1_ro_ip=192.168.1.1
> > +
> > +proxy_ip1=169.254.239.254
> > +proxy_ip2=169.254.239.2
> > +
> > +bcast_mac=ff:ff:ff:ff:ff:ff
> > +
> > +# Send ARP request for 169.254.239.254
> > +packet="inport==\"ls1-lp1\" && eth.src==$ls1_p1_mac && 
> > eth.dst==$bcast_mac &&
> > +       arp.op==1 && arp.sha==$ls1_p1_mac && arp.spa==$ls1_p1_ip &&
> > +       arp.tha==$bcast_mac && arp.tpa==$proxy_ip1"
> > +
> > +as pa-hv ovs-appctl -t ovn-controller inject-pkt "$packet"
> > +
> > +ovs-ofctl dump-flows br-int| grep 169.254.239.254 | grep
> > priority=50 
> > > debug1
> > +AT_CAPTURE_FILE([debug1])
> > +
> > +
> > +# Check if packet hit the ARP reply ovs flow
> > +AT_CHECK([ovs-ofctl dump-flows br-int | \
> > +    grep "169.254.239.254" | \
> > +    grep "priority=50" | \
> > +    grep "arp_op=1" | \
> > +    grep "n_packets=1" | wc -l], [0], [dnl
> > +1
> > +])
> > +
> > +# Check that the HV gets an ARP reply
> > +expected="eth.src==$ls1_ro_mac && eth.dst==$ls1_p1_mac &&
> > +       arp.op==2 && arp.sha==$ls1_ro_mac && arp.spa==$proxy_ip1 &&
> > +       arp.tha==$ls1_p1_mac && arp.tpa==$ls1_p1_ip"
> > +echo $expected | ovstest test-ovn expr-to-packets > expected
> > +
> > +OVN_CHECK_PACKETS([pa-hv/vif1-tx.pcap], [expected])
> > +
> > +OVN_CLEANUP([pa-hv])
> > +AT_CLEANUP
> > +])
> > -- 
> > 1.8.3.1
> > 
> 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Brendan Doyle June 2, 2021, 3:43 p.m. UTC | #3
On 02/06/2021 15:42, Dan Williams wrote:
> On Wed, 2021-06-02 at 09:26 +0100, Brendan Doyle wrote:
>> Hi,
>>
>> Just wondering what the process is here, I submitted this patch a while
>> ago, and I see quite a few
>> other patches have been submitted since. So wondering what happens
>> after
>> a patch is submitted,
>> how do I know if it has been accepted/rejected/need more work, and how
>> does it make it into the
>> repo if accepted. This does not seem to be documented in:
> Thanks for the patch; now that it's posted the patch is waiting for
> review. You can track overall status here:
>
>
> https://urldefense.com/v3/__http://patchwork.ozlabs.org/project/ovn/list/?series=246447__;!!GqivPVa7Brio!NAT2CA0Mr3YQ0gASU9iSe8-ql0xQnnn3q7gvr8E25QYgrAvVWAE0QvPelMH9KXmj-pM$
Great, thanks
>
> but you'll also get replies to the patch with review comments. Reviews
> may take a bit of time as the patch volume is fairly large, and
> everyone is busy. But don't worry! It's on the list.
NP - I know the feeling :)
>
>> Submitting Patches — Open Virtual Network (OVN) 21.03.0 documentation
>> <
>> https://urldefense.com/v3/__https://docs.ovn.org/en/stable/internals/contributing/submitting-patches.html__;!!GqivPVa7Brio!NAT2CA0Mr3YQ0gASU9iSe8-ql0xQnnn3q7gvr8E25QYgrAvVWAE0QvPelMH9OLJt62c$
> Good point, there should probably be a "What's next?" section.
>
> ---
>
> One thing though, the patch seems to be linewrapped and won't apply
> cleanly. Any chance you could fix that up by setting your email client
> to "preformatted" before pasting the patch into the mail, or using git-
> send-email?
Oh, sorry I did run the patch checker before pasting into email, I'll 
see what I can do.
Then just resubmit?

> Thanks,
> Dan
>
>> Thanks
>>
>> Brendan
>>
>> On 31/05/2021 10:06, Brendan Doyle wrote:
>>>  From a9d3140845175edb7644b2d0d82a95bd6cf94662 Mon Sep 17 00:00:00
>>> 2001
>>> From: Brendan Doyle <brendan.doyle@oracle.com>
>>> Date: Fri, 28 May 2021 10:01:17 -0700
>>> Subject: [PATCH ovn] ovn-northd.c: Add proxy ARP support to OVN
>>>
>>> This patch provides the ability to configure proxy ARP IPs on a
>>> Logical
>>> Switch Router port. The IPs are added as Options for router ports.
>>> This
>>> provides a useful feature where traffic for a service must be sent
>>> to an
>>> address in a logical network address space, but the service is
>>> provided
>>> in a different network. For example an NFS service is provide to
>>> Logical
>>> networks at an address in their Logical network space, but the NFS
>>> server resides in a physical network. A Logical switch Router port
>>> can
>>> be configured to respond to ARP requests sent to the service
>>> "Logical
>>> address", the Logical Router/Gateway can then be configured to
>>> forward
>>> the traffic to the underlay/physical network.
>>>
>>> Signed-off-by: Brendan Doyle <brendan.doyle@oracle.com>
>>> ---
>>>   northd/ovn-northd.c |  38 +++++++++++++++++++
>>>   ovn-nb.xml          |   9 +++++
>>>   tests/ovn.at        | 103
>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>   3 files changed, 150 insertions(+)
>>>
>>> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
>>> index 0e5092a..a377e83 100644
>>> --- a/northd/ovn-northd.c
>>> +++ b/northd/ovn-northd.c
>>> @@ -6943,6 +6943,7 @@
>>> build_lswitch_arp_nd_responder_known_ips(struct
>>> ovn_port *op,
>>>                                            struct ds *match)
>>>   {
>>>       if (op->nbsp) {
>>> +        const char *arp_proxy;
>>>           if (!strcmp(op->nbsp->type, "virtual")) {
>>>               /* Handle
>>>                *  - GARPs for virtual ip which belongs to a logical
>>> port
>>> @@ -7096,6 +7097,43 @@
>>> build_lswitch_arp_nd_responder_known_ips(struct
>>> ovn_port *op,
>>>                   }
>>>               }
>>>           }
>>> +
>>> +        /*
>>> +         * Add responses for ARP proxies.
>>> +         */
>>> +        arp_proxy = smap_get(&op->nbsp->options,"arp_proxy");
>>> +        if (arp_proxy && op->peer) {
>>> +            char *ips, *ip, *rest;
>>> +
>>> +            ips = xstrdup(arp_proxy);
>>> +            rest = ips;
>>> +
>>> +            while ((ip = strtok_r(rest,",", &rest))) {
>>> +                ds_clear(match);
>>> +                ds_put_format(match, "arp.tpa == %s && arp.op ==
>>> 1",
>>> ip);
>>> +
>>> +                ds_clear(actions);
>>> +                ds_put_format(actions,
>>> +                    "eth.dst = eth.src; "
>>> +                    "eth.src = %s; "
>>> +                    "arp.op = 2; /* ARP reply */ "
>>> +                    "arp.tha = arp.sha; "
>>> +                    "arp.sha = %s; "
>>> +                    "arp.tpa = arp.spa; "
>>> +                    "arp.spa = %s; "
>>> +                    "outport = inport; "
>>> +                    "flags.loopback = 1; "
>>> +                    "output;",
>>> +                    op->peer->lrp_networks.ea_s,
>>> +                    op->peer->lrp_networks.ea_s,
>>> +                    ip);
>>> +
>>> +                ovn_lflow_add_with_hint(lflows, op->od,
>>> S_SWITCH_IN_ARP_ND_RSP,
>>> +                    50, ds_cstr(match), ds_cstr(actions),
>>> +                    &op->nbsp->header_);
>>> +            }
>>> +            free(ips);
>>> +        }
>>>       }
>>>   }
>>>
>>> diff --git a/ovn-nb.xml b/ovn-nb.xml
>>> index 02fd216..4b6c183 100644
>>> --- a/ovn-nb.xml
>>> +++ b/ovn-nb.xml
>>> @@ -848,6 +848,15 @@
>>>               </dd>
>>>             </dl>
>>>           </column>
>>> +
>>> +        <column name="options" key="arp_proxy">
>>> +          Optional. A comma separated list IPv4 addresses that
>>> this
>>> +          logical switch <code>router</code> port will reply to
>>> ARP
>>> requests.
>>> +          Example: <code>169.254.239.254,169.254.239.2</code>. The
>>> +          <ref column="options" key="router-port"/>'s logical
>>> router
>>> should
>>> +          have a route to forward packets sent to configured proxy
>>> ARP IPs to
>>> +          an appropriate destination.
>>> +        </column>
>>>         </group>
>>>
>>>         <group title="Options for localnet ports">
>>> diff --git a/tests/ovn.at b/tests/ovn.at
>>> index 2c3c36d..c675cc9 100644
>>> --- a/tests/ovn.at
>>> +++ b/tests/ovn.at
>>> @@ -26527,3 +26527,106 @@ AT_CHECK([test $(ovn-appctl -t
>>> ovn-controller coverage/read-counter lflow_run) =
>>>   OVN_CLEANUP([hv1])
>>>   AT_CLEANUP
>>>   ])
>>> +
>>> +OVN_FOR_EACH_NORTHD([
>>> +AT_SETUP([ovn -- proxy-arp: 1 HVs, 1 LSs, 1 lport/LS, 1 LR])
>>> +AT_KEYWORDS([proxy-arp])
>>> +ovn_start
>>> +
>>> +# Logical network:
>>> +# One LR - lr1 has switch ls1 (192.16.1.0/24) connected to it,
>>> +# and and one HV with IP 192.16.1.6.
>>> +
>>> +ovn-nbctl lr-add lr1
>>> +ovn-nbctl ls-add ls1
>>> +
>>> +# Connect ls1 to lr1
>>> +ovn-nbctl lrp-add lr1 ls1 00:00:00:01:02:f1 192.16.1.1/24
>>> +ovn-nbctl lsp-add ls1 rp-ls1 -- set Logical_Switch_Port rp-ls1 \
>>> +    type=router options:router-port=ls1
>>> addresses=\"00:00:00:01:02:f1\"
>>> +
>>> +# Create logical port ls1-lp1 in ls1
>>> +ovn-nbctl lsp-add ls1 ls1-lp1 \
>>> +-- lsp-set-addresses ls1-lp1 "00:00:00:01:02:03 192.16.1.6"
>>> +
>>> +
>>> +# Create one hypervisor and create OVS ports corresponding to
>>> logical
>>> ports.
>>> +net_add n1
>>> +
>>> +sim_add pa-hv
>>> +as pa-hv
>>> +ovs-vsctl add-br br-phys
>>> +ovn_attach n1 br-phys 192.16.0.1
>>> +
>>> +# Note: tx/rx are with respect to the LS port, so
>>> +# tx on switch port is HV rx, etc.
>>> +ovs-vsctl -- add-port br-int vif1 -- \
>>> +    set interface vif1 external-ids:iface-id=ls1-lp1 \
>>> +    options:tx_pcap=pa-hv/vif1-tx.pcap \
>>> +    options:rxq_pcap=pa-hv/vif1-rx.pcap \
>>> +    ofport-request=1
>>> +
>>> +# And proxy ARP flows for 69.254.239.254 and 169.254.239.2
>>> +# and check that SB flows have been added.
>>> +ovn-nbctl --wait=hv add Logical_Switch_Port rp-ls1 \
>>> +options arp_proxy='"169.254.239.254,169.254.239.2"'
>>> +ovn-sbctl dump-flows > sbflows
>>> +AT_CAPTURE_FILE([sbflows])
>>> +
>>> +AT_CHECK([ovn-sbctl dump-flows | grep ls_in_arp_rsp | grep
>>> "169.254.239.2" | wc -l], [0], [dnl
>>> +2
>>> +])
>>> +
>>> +# Remove and check that the flows have been removed
>>> +ovn-nbctl --wait=hv remove Logical_Switch_Port rp-ls1 options
>>> arp_proxy='"169.254.239.254,169.254.239.2"'
>>> +
>>> +AT_CHECK([ovn-sbctl dump-flows | grep ls_in_arp_rsp | grep
>>> "169.254.239.2" | wc -l], [0], [dnl
>>> +0
>>> +])
>>> +
>>> +# Add the flows back send arp request and check we see an ARP
>>> response
>>> +ovn-nbctl --wait=hv add Logical_Switch_Port rp-ls1 \
>>> +options arp_proxy='"169.254.239.254,169.254.239.2"'
>>> +
>>> +ls1_p1_mac=00:00:00:01:02:03
>>> +ls1_p1_ip=192.16.1.6
>>> +
>>> +ls1_ro_mac=00:00:00:01:02:f1
>>> +ls1_ro_ip=192.168.1.1
>>> +
>>> +proxy_ip1=169.254.239.254
>>> +proxy_ip2=169.254.239.2
>>> +
>>> +bcast_mac=ff:ff:ff:ff:ff:ff
>>> +
>>> +# Send ARP request for 169.254.239.254
>>> +packet="inport==\"ls1-lp1\" && eth.src==$ls1_p1_mac &&
>>> eth.dst==$bcast_mac &&
>>> +       arp.op==1 && arp.sha==$ls1_p1_mac && arp.spa==$ls1_p1_ip &&
>>> +       arp.tha==$bcast_mac && arp.tpa==$proxy_ip1"
>>> +
>>> +as pa-hv ovs-appctl -t ovn-controller inject-pkt "$packet"
>>> +
>>> +ovs-ofctl dump-flows br-int| grep 169.254.239.254 | grep
>>> priority=50
>>>> debug1
>>> +AT_CAPTURE_FILE([debug1])
>>> +
>>> +
>>> +# Check if packet hit the ARP reply ovs flow
>>> +AT_CHECK([ovs-ofctl dump-flows br-int | \
>>> +    grep "169.254.239.254" | \
>>> +    grep "priority=50" | \
>>> +    grep "arp_op=1" | \
>>> +    grep "n_packets=1" | wc -l], [0], [dnl
>>> +1
>>> +])
>>> +
>>> +# Check that the HV gets an ARP reply
>>> +expected="eth.src==$ls1_ro_mac && eth.dst==$ls1_p1_mac &&
>>> +       arp.op==2 && arp.sha==$ls1_ro_mac && arp.spa==$proxy_ip1 &&
>>> +       arp.tha==$ls1_p1_mac && arp.tpa==$ls1_p1_ip"
>>> +echo $expected | ovstest test-ovn expr-to-packets > expected
>>> +
>>> +OVN_CHECK_PACKETS([pa-hv/vif1-tx.pcap], [expected])
>>> +
>>> +OVN_CLEANUP([pa-hv])
>>> +AT_CLEANUP
>>> +])
>>> -- 
>>> 1.8.3.1
>>>
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>> https://urldefense.com/v3/__https://mail.openvswitch.org/mailman/listinfo/ovs-dev__;!!GqivPVa7Brio!NAT2CA0Mr3YQ0gASU9iSe8-ql0xQnnn3q7gvr8E25QYgrAvVWAE0QvPelMH9-EMNduE$
>
diff mbox series

Patch

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 0e5092a..a377e83 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -6943,6 +6943,7 @@  build_lswitch_arp_nd_responder_known_ips(struct 
ovn_port *op,
                                           struct ds *match)
  {
      if (op->nbsp) {
+        const char *arp_proxy;
          if (!strcmp(op->nbsp->type, "virtual")) {
              /* Handle
               *  - GARPs for virtual ip which belongs to a logical port
@@ -7096,6 +7097,43 @@  build_lswitch_arp_nd_responder_known_ips(struct 
ovn_port *op,
                  }
              }
          }
+
+        /*
+         * Add responses for ARP proxies.
+         */
+        arp_proxy = smap_get(&op->nbsp->options,"arp_proxy");
+        if (arp_proxy && op->peer) {
+            char *ips, *ip, *rest;
+
+            ips = xstrdup(arp_proxy);
+            rest = ips;
+
+            while ((ip = strtok_r(rest,",", &rest))) {
+                ds_clear(match);
+                ds_put_format(match, "arp.tpa == %s && arp.op == 1", ip);
+
+                ds_clear(actions);
+                ds_put_format(actions,
+                    "eth.dst = eth.src; "
+                    "eth.src = %s; "
+                    "arp.op = 2; /* ARP reply */ "
+                    "arp.tha = arp.sha; "
+                    "arp.sha = %s; "
+                    "arp.tpa = arp.spa; "
+                    "arp.spa = %s; "
+                    "outport = inport; "
+                    "flags.loopback = 1; "
+                    "output;",
+                    op->peer->lrp_networks.ea_s,
+                    op->peer->lrp_networks.ea_s,
+                    ip);
+
+                ovn_lflow_add_with_hint(lflows, op->od, 
S_SWITCH_IN_ARP_ND_RSP,
+                    50, ds_cstr(match), ds_cstr(actions),
+                    &op->nbsp->header_);
+            }
+            free(ips);
+        }
      }
  }

diff --git a/ovn-nb.xml b/ovn-nb.xml
index 02fd216..4b6c183 100644
--- a/ovn-nb.xml
+++ b/ovn-nb.xml
@@ -848,6 +848,15 @@ 
              </dd>
            </dl>
          </column>
+
+        <column name="options" key="arp_proxy">
+          Optional. A comma separated list IPv4 addresses that this
+          logical switch <code>router</code> port will reply to ARP 
requests.
+          Example: <code>169.254.239.254,169.254.239.2</code>. The
+          <ref column="options" key="router-port"/>'s logical router should
+          have a route to forward packets sent to configured proxy ARP 
IPs to
+          an appropriate destination.
+        </column>
        </group>

        <group title="Options for localnet ports">
diff --git a/tests/ovn.at b/tests/ovn.at
index 2c3c36d..c675cc9 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -26527,3 +26527,106 @@  AT_CHECK([test $(ovn-appctl -t ovn-controller 
coverage/read-counter lflow_run) =
  OVN_CLEANUP([hv1])
  AT_CLEANUP
  ])
+
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([ovn -- proxy-arp: 1 HVs, 1 LSs, 1 lport/LS, 1 LR])
+AT_KEYWORDS([proxy-arp])
+ovn_start
+
+# Logical network:
+# One LR - lr1 has switch ls1 (192.16.1.0/24) connected to it,
+# and and one HV with IP 192.16.1.6.
+
+ovn-nbctl lr-add lr1
+ovn-nbctl ls-add ls1
+
+# Connect ls1 to lr1
+ovn-nbctl lrp-add lr1 ls1 00:00:00:01:02:f1 192.16.1.1/24
+ovn-nbctl lsp-add ls1 rp-ls1 -- set Logical_Switch_Port rp-ls1 \
+    type=router options:router-port=ls1 addresses=\"00:00:00:01:02:f1\"
+
+# Create logical port ls1-lp1 in ls1
+ovn-nbctl lsp-add ls1 ls1-lp1 \
+-- lsp-set-addresses ls1-lp1 "00:00:00:01:02:03 192.16.1.6"
+
+
+# Create one hypervisor and create OVS ports corresponding to logical 
ports.
+net_add n1
+
+sim_add pa-hv
+as pa-hv
+ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.16.0.1
+
+# Note: tx/rx are with respect to the LS port, so
+# tx on switch port is HV rx, etc.
+ovs-vsctl -- add-port br-int vif1 -- \
+    set interface vif1 external-ids:iface-id=ls1-lp1 \
+    options:tx_pcap=pa-hv/vif1-tx.pcap \
+    options:rxq_pcap=pa-hv/vif1-rx.pcap \
+    ofport-request=1
+
+# And proxy ARP flows for 69.254.239.254 and 169.254.239.2
+# and check that SB flows have been added.
+ovn-nbctl --wait=hv add Logical_Switch_Port rp-ls1 \
+options arp_proxy='"169.254.239.254,169.254.239.2"'
+ovn-sbctl dump-flows > sbflows
+AT_CAPTURE_FILE([sbflows])
+
+AT_CHECK([ovn-sbctl dump-flows | grep ls_in_arp_rsp | grep 
"169.254.239.2" | wc -l], [0], [dnl
+2
+])
+
+# Remove and check that the flows have been removed
+ovn-nbctl --wait=hv remove Logical_Switch_Port rp-ls1 options 
arp_proxy='"169.254.239.254,169.254.239.2"'
+
+AT_CHECK([ovn-sbctl dump-flows | grep ls_in_arp_rsp | grep 
"169.254.239.2" | wc -l], [0], [dnl
+0
+])
+
+# Add the flows back send arp request and check we see an ARP response
+ovn-nbctl --wait=hv add Logical_Switch_Port rp-ls1 \
+options arp_proxy='"169.254.239.254,169.254.239.2"'
+
+ls1_p1_mac=00:00:00:01:02:03
+ls1_p1_ip=192.16.1.6
+
+ls1_ro_mac=00:00:00:01:02:f1
+ls1_ro_ip=192.168.1.1
+
+proxy_ip1=169.254.239.254
+proxy_ip2=169.254.239.2
+
+bcast_mac=ff:ff:ff:ff:ff:ff
+
+# Send ARP request for 169.254.239.254
+packet="inport==\"ls1-lp1\" && eth.src==$ls1_p1_mac && 
eth.dst==$bcast_mac &&
+       arp.op==1 && arp.sha==$ls1_p1_mac && arp.spa==$ls1_p1_ip &&
+       arp.tha==$bcast_mac && arp.tpa==$proxy_ip1"
+
+as pa-hv ovs-appctl -t ovn-controller inject-pkt "$packet"
+
+ovs-ofctl dump-flows br-int| grep 169.254.239.254 | grep priority=50 > 
debug1
+AT_CAPTURE_FILE([debug1])
+
+
+# Check if packet hit the ARP reply ovs flow
+AT_CHECK([ovs-ofctl dump-flows br-int | \
+    grep "169.254.239.254" | \
+    grep "priority=50" | \
+    grep "arp_op=1" | \
+    grep "n_packets=1" | wc -l], [0], [dnl
+1
+])
+
+# Check that the HV gets an ARP reply
+expected="eth.src==$ls1_ro_mac && eth.dst==$ls1_p1_mac &&
+       arp.op==2 && arp.sha==$ls1_ro_mac && arp.spa==$proxy_ip1 &&
+       arp.tha==$ls1_p1_mac && arp.tpa==$ls1_p1_ip"