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

Numan Siddique numans at ovn.org
Sun Jun 27 17:19:34 UTC 2021


On Sun, Jun 27, 2021 at 8:46 AM Brendan Doyle <brendan.doyle at oracle.com> wrote:
>
> Hi Numan,
>
> I did a fresh clone and recreated/resubmitted the patch  [PATCH ovn v4]
> ovn-northd.c: Add proxy ARP support to OVN.

v4 still doesn't apply.  Maybe you can share your github private
branch with the patch ?

Thanks
Numan

>
> I tried to create a PR, but I got an error when trying to create a
> remote branch from which to
> generate the PR:
> > git push --set-upstream origin proxy_arp:proxy_arp
> > Username for 'https://github.com': BrendanDoyle1
> > Password for 'https://BrendanDoyle1@github.com':
> > remote: Permission to ovn-org/ovn.git denied to BrendanDoyle1.
> > fatal: unable to access 'https://github.com/ovn-org/ovn/': The
> > requested URL returned error: 403
> If the v4 does not work I'll have to wait for my goit/GitHub experts to
> come in on Monday.
>
> Brendan.
>
> On 25/06/2021 22:22, Numan Siddique wrote:
> > On Thu, Jun 24, 2021 at 5:14 AM Brendan Doyle <brendan.doyle at oracle.com> wrote:
> >> Hi,
> >>
> >> Just wondering how to move this along. It's been in the queue a while now.
> >> I've made the requested changes, this is v3 of the patch.
> >>
> >> The product I'm working on requires this feature and If I can't get it
> >> upstream
> >> I'll need to look at other options.
> > Hi Brendan,
> >
> > I just can't apply your patch cleanly.  The same issue I faced with
> > your previous version.
> > Normally when patch is submitted, the ovsrobot applies the patch on
> > top of the master
> > and runs the tests.  It generally creates a branch with the patch series number.
> >
> > In your case, the series number is 249668
> > (https://urldefense.com/v3/__https://patchwork.ozlabs.org/project/ovn/list/?series=249668__;!!ACWV5N9M2RV99hQ!fmNb3A9Qn2_9U4wjyZ1fVFxqrPOwLDHJ8uBbL-0zKNDDHDNqE-xORpdhqZdjs1rnJo4$ )
> > and the branch is here -
> > https://urldefense.com/v3/__https://github.com/ovsrobot/ovn/commits/series_249668__;!!ACWV5N9M2RV99hQ!fmNb3A9Qn2_9U4wjyZ1fVFxqrPOwLDHJ8uBbL-0zKNDDHDNqE-xORpdhqZdjS24tNa4$  and I don't see
> > your patch applied cleanly.  So I'm pretty sure something is wrong
> > with your patch format.
> >
> > Either you can resend the patch or send a github PR.
> >
> > Thanks
> > Numan
> >
> >>
> >> Thanks
> >>
> >> Brendan
> >>
> >> On 18/06/2021 14:53, Brendan Doyle wrote:
> >>>  From 634fd88b726700b30cb76203ca45f1e9c041368a Mon Sep 17 00:00:00 2001
> >>> From: Brendan Doyle <brendan.doyle at 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 at oracle.com>
> >>> ---
> >>>   northd/ovn-northd.c |  46 +++++++++++++++++++++++
> >>>   ovn-nb.xml          |   9 +++++
> >>>   tests/ovn.at        | 103
> >>> ++++++++++++++++++++++++++++++++++++++++++++++++++++
> >>>   3 files changed, 158 insertions(+)
> >>>
> >>> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> >>> index 0e5092a..2bc6f28 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,51 @@ 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) {
> >>> +            struct lport_addresses proxy_arp_addrs;
> >>> +            int i = 0;
> >>> +
> >>> +            if (extract_ip_addresses(arp_proxy, &proxy_arp_addrs)) {
> >>> +                /*
> >>> +                 * Match rule on all proxy ARP IPs.
> >>> +                 */
> >>> +                ds_clear(match);
> >>> +                ds_put_cstr(match, "arp.op == 1 && (");
> >>> +
> >>> +                for (i = 0; i < proxy_arp_addrs.n_ipv4_addrs; i++) {
> >>> +                    if (i > 0) {
> >>> +                        ds_put_cstr(match, " || ");
> >>> +                    }
> >>> +                    ds_put_format(match, "arp.tpa == %s",
> >>> +                        proxy_arp_addrs.ipv4_addrs[i].addr_s);
> >>> +                }
> >>> +
> >>> +                ds_put_cstr(match, ")");
> >>> +                destroy_lport_addresses(&proxy_arp_addrs);
> >>> +
> >>> +                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; "
> >>> +                    "outport = inport; "
> >>> +                    "flags.loopback = 1; "
> >>> +                    "output;",
> >>> +                    op->peer->lrp_networks.ea_s,
> >>> +                    op->peer->lrp_networks.ea_s);
> >>> +
> >>> +                ovn_lflow_add_with_hint(lflows, op->od,
> >>> S_SWITCH_IN_ARP_ND_RSP,
> >>> +                    50, ds_cstr(match), ds_cstr(actions),
> >>> &op->nbsp->header_);
> >>> +            }
> >>> +        }
> >>>       }
> >>>   }
> >>>
> >>> diff --git a/ovn-nb.xml b/ovn-nb.xml
> >>> index 02fd216..e4a8114 100644
> >>> --- a/ovn-nb.xml
> >>> +++ b/ovn-nb.xml
> >>> @@ -848,6 +848,15 @@
> >>>               </dd>
> >>>             </dl>
> >>>           </column>
> >>> +
> >>> +        <column name="options" key="arp_proxy">
> >>> +          Optional. A 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..ffbb594 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
> >>> +1
> >>> +])
> >>> +
> >>> +# 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
> >>> +])
> >> _______________________________________________
> >> dev mailing list
> >> dev at openvswitch.org
> >> https://urldefense.com/v3/__https://mail.openvswitch.org/mailman/listinfo/ovs-dev__;!!ACWV5N9M2RV99hQ!fmNb3A9Qn2_9U4wjyZ1fVFxqrPOwLDHJ8uBbL-0zKNDDHDNqE-xORpdhqZdjcC4Z6IU$
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev


More information about the dev mailing list