| Message ID | 5618fd3c-4a65-4e71-7f13-24ac8c3c4e4e@oracle.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [ovs-dev,v2] ovn-northd.c: Add proxy ARP support to OVN | expand |
Hi, Just wondering how to move this along. It's been in the queue a while now. 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. Thanks Brendan On 04/06/2021 15:51, Brendan Doyle wrote: > From 07ecd5d00f82658e094132102575d8d576161a6b 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 | 44 ++++++++++++++++++++++ > ovn-nb.xml | 9 +++++ > tests/ovn.at | 103 > ++++++++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 156 insertions(+) > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > index 0e5092a..9b686d9 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,49 @@ 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; > + int i = 0; > + > + ips = xstrdup(arp_proxy); > + rest = ips; > + > + /* > + * Match rule on all proxy ARP IPs. > + */ > + ds_clear(match); > + ds_put_cstr(match, "arp.op == 1 && ("); > + while ((ip = strtok_r(rest,",", &rest))) { > + if (i++ > 0) { > + ds_put_cstr(match, " || "); > + }; > + ds_put_format(match, "arp.tpa == %s", ip); > + } > + ds_put_cstr(match, ")"); > + > + 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_); > + 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..4befe4a 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 > +])
On Mon, Jun 14, 2021 at 5:19 AM Brendan Doyle <brendan.doyle@oracle.com> wrote: > > Hi, > > Just wondering how to move this along. It's been in the queue a while now. > 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, Sorry for the late reply. For some reason the patch doesn't apply to me. Please see below for a few comments. > > > Thanks > > Brendan > > On 04/06/2021 15:51, Brendan Doyle wrote: > > From 07ecd5d00f82658e094132102575d8d576161a6b 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 | 44 ++++++++++++++++++++++ > > ovn-nb.xml | 9 +++++ > > tests/ovn.at | 103 > > ++++++++++++++++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 156 insertions(+) > > > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > > index 0e5092a..9b686d9 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,49 @@ 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"); In my opinion instead of defining these proxy IPs in the logical switch port options, it is better to define them in the router port as generally a router would handle proxy arps (correct me if I'm wrong here). I'd suggest to add a new column "proxy_ips" (or something more appropriate) in the Logical_Router_Port and this column would point to an address set. CMS can define the list of IPs in the address set. ovn-northd can add a logical flow like match = "inport == <router_port> && arp.tpa == $proxy_ip, action = {....} This would result in just one logical flow. CMS would do something like addr_set_uuid=$(ovn-nbctl create address_set name=proxy_arps addresses="10.0.0.4, 10.0.0.5") ovn-nbctl set logical_router_port <router_port> proxy_ip=$addr_set_uuid What do you think ? Thanks Numan > > + if (arp_proxy && op->peer) { > > + char *ips, *ip, *rest; > > + int i = 0; > > + > > + ips = xstrdup(arp_proxy); > > + rest = ips; > > + > > + /* > > + * Match rule on all proxy ARP IPs. > > + */ > > + ds_clear(match); > > + ds_put_cstr(match, "arp.op == 1 && ("); > > + while ((ip = strtok_r(rest,",", &rest))) { > > + if (i++ > 0) { > > + ds_put_cstr(match, " || "); > > + }; > > + ds_put_format(match, "arp.tpa == %s", ip); > > + } > > + ds_put_cstr(match, ")"); > > + > > + 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_); > > + 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..4befe4a 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@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
On 14/06/2021 21:17, Numan Siddique wrote: > On Mon, Jun 14, 2021 at 5:19 AM Brendan Doyle <brendan.doyle@oracle.com> wrote: >> Hi, >> >> Just wondering how to move this along. It's been in the queue a while now. >> 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, > > Sorry for the late reply. For some reason the patch doesn't apply to me. Sorry I'm not sure what you mean, the patch did not apply, it works when I apply to my repo? > Please see below for a few comments. > > >> >> Thanks >> >> Brendan >> >> On 04/06/2021 15:51, Brendan Doyle wrote: >>> From 07ecd5d00f82658e094132102575d8d576161a6b 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 | 44 ++++++++++++++++++++++ >>> ovn-nb.xml | 9 +++++ >>> tests/ovn.at | 103 >>> ++++++++++++++++++++++++++++++++++++++++++++++++++++ >>> 3 files changed, 156 insertions(+) >>> >>> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c >>> index 0e5092a..9b686d9 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,49 @@ 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"); > In my opinion instead of defining these proxy IPs in the logical > switch port options, > it is better to define them in the router port as generally a router > would handle proxy arps (correct me if I'm wrong here). > > I'd suggest to add a new column "proxy_ips" (or something more appropriate) > in the Logical_Router_Port and this column would point to an address set. > > CMS can define the list of IPs in the address set. ovn-northd can add > a logical flow like > > match = "inport == <router_port> && arp.tpa == $proxy_ip, action = {....} > This would result in just one logical flow. > > CMS would do something like > > addr_set_uuid=$(ovn-nbctl create address_set name=proxy_arps > addresses="10.0.0.4, 10.0.0.5") > ovn-nbctl set logical_router_port <router_port> proxy_ip=$addr_set_uuid > > What do you think ? In both cases only one OVS flows is created (after I made the changes) suggested by Dan. I can see the point with respect to proxy ARP and routers, in physical equipment proxy ARP is done by routers. But I think doing the proxy ARP on the LRP of the LS is consistent with the use of the "options : nat-addresses" entry in the Logical_Switch_Port TABLE . The patch makes it clear that this feature is associated with a router, It is after all in the "Options for router ports". I think it is also more efficient in that we'd respond to the ARP request earlier in the datapath flows. The suggestion you make is a lot more involved, touching much more code and changes the DB schema, so I'm hesitant to go down that path. Brendan. > > Thanks > Numan > >>> + if (arp_proxy && op->peer) { >>> + char *ips, *ip, *rest; >>> + int i = 0; >>> + >>> + ips = xstrdup(arp_proxy); >>> + rest = ips; >>> + >>> + /* >>> + * Match rule on all proxy ARP IPs. >>> + */ >>> + ds_clear(match); >>> + ds_put_cstr(match, "arp.op == 1 && ("); >>> + while ((ip = strtok_r(rest,",", &rest))) { >>> + if (i++ > 0) { >>> + ds_put_cstr(match, " || "); >>> + }; >>> + ds_put_format(match, "arp.tpa == %s", ip); >>> + } >>> + ds_put_cstr(match, ")"); >>> + >>> + 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_); >>> + 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..4befe4a 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@openvswitch.org >> https://urldefense.com/v3/__https://mail.openvswitch.org/mailman/listinfo/ovs-dev__;!!GqivPVa7Brio!LVM6snHeQdP5rdldKTqBZu_C7h5JiR3HZtfLAmyQisBZUJ2TMpQCtj8JY49M3ZRRqmU$
On Tue, Jun 15, 2021 at 7:50 AM Brendan Doyle <brendan.doyle@oracle.com> wrote: > > > > On 14/06/2021 21:17, Numan Siddique wrote: > > On Mon, Jun 14, 2021 at 5:19 AM Brendan Doyle <brendan.doyle@oracle.com> wrote: > >> Hi, > >> > >> Just wondering how to move this along. It's been in the queue a while now. > >> 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, > > > > Sorry for the late reply. For some reason the patch doesn't apply to me. > Sorry I'm not sure what you mean, the patch did not apply, it works when > I apply > to my repo? > > Please see below for a few comments. > > > > > >> > >> Thanks > >> > >> Brendan > >> > >> On 04/06/2021 15:51, Brendan Doyle wrote: > >>> From 07ecd5d00f82658e094132102575d8d576161a6b 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 | 44 ++++++++++++++++++++++ > >>> ovn-nb.xml | 9 +++++ > >>> tests/ovn.at | 103 > >>> ++++++++++++++++++++++++++++++++++++++++++++++++++++ > >>> 3 files changed, 156 insertions(+) > >>> > >>> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > >>> index 0e5092a..9b686d9 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,49 @@ 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"); > > In my opinion instead of defining these proxy IPs in the logical > > switch port options, > > it is better to define them in the router port as generally a router > > would handle proxy arps (correct me if I'm wrong here). > > > > I'd suggest to add a new column "proxy_ips" (or something more appropriate) > > in the Logical_Router_Port and this column would point to an address set. > > > > CMS can define the list of IPs in the address set. ovn-northd can add > > a logical flow like > > > > match = "inport == <router_port> && arp.tpa == $proxy_ip, action = {....} > > This would result in just one logical flow. > > > > CMS would do something like > > > > addr_set_uuid=$(ovn-nbctl create address_set name=proxy_arps > > addresses="10.0.0.4, 10.0.0.5") > > ovn-nbctl set logical_router_port <router_port> proxy_ip=$addr_set_uuid > > > > What do you think ? > > In both cases only one OVS flows is created (after I made the changes) > suggested by Dan. > I can see the point with respect to proxy ARP and routers, in physical > equipment proxy ARP is done > by routers. But I think doing the proxy ARP on the LRP of the LS is > consistent with the use of > the "options : nat-addresses" entry in the Logical_Switch_Port TABLE . > The patch makes it clear > that this feature is associated with a router, It is after all in the > "Options for router ports". I think > it is also more efficient in that we'd respond to the ARP request > earlier in the datapath flows. Even if the proxy arps are associated with a router port, the flows can still be added in the logical switch pipeline of the peer to avoid the extra hop from logical switch pipeline to router pipeline. In my opinion, it is better that the proxy arps are associated with a router port rather than a logical switch peer port. Also having a separate column makes more sense to me than defining a string of IP addresses in the options smap. I defer to other reviewers if they are fine with your patch. Thanks Numan > > The suggestion you make is a lot more involved, touching much more code > and changes the DB > schema, so I'm hesitant to go down that path. > > Brendan. > > > > > Thanks > > Numan > > > >>> + if (arp_proxy && op->peer) { > >>> + char *ips, *ip, *rest; > >>> + int i = 0; > >>> + > >>> + ips = xstrdup(arp_proxy); > >>> + rest = ips; > >>> + > >>> + /* > >>> + * Match rule on all proxy ARP IPs. > >>> + */ > >>> + ds_clear(match); > >>> + ds_put_cstr(match, "arp.op == 1 && ("); > >>> + while ((ip = strtok_r(rest,",", &rest))) { > >>> + if (i++ > 0) { > >>> + ds_put_cstr(match, " || "); > >>> + }; > >>> + ds_put_format(match, "arp.tpa == %s", ip); > >>> + } > >>> + ds_put_cstr(match, ")"); > >>> + > >>> + 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_); > >>> + 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..4befe4a 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@openvswitch.org > >> https://urldefense.com/v3/__https://mail.openvswitch.org/mailman/listinfo/ovs-dev__;!!GqivPVa7Brio!LVM6snHeQdP5rdldKTqBZu_C7h5JiR3HZtfLAmyQisBZUJ2TMpQCtj8JY49M3ZRRqmU$ > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
On 15/06/2021 13:26, Numan Siddique wrote: > On Tue, Jun 15, 2021 at 7:50 AM Brendan Doyle <brendan.doyle@oracle.com> wrote: >> >> >> On 14/06/2021 21:17, Numan Siddique wrote: >>> On Mon, Jun 14, 2021 at 5:19 AM Brendan Doyle <brendan.doyle@oracle.com> wrote: >>>> Hi, >>>> >>>> Just wondering how to move this along. It's been in the queue a while now. >>>> 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, >>> >>> Sorry for the late reply. For some reason the patch doesn't apply to me. >> Sorry I'm not sure what you mean, the patch did not apply, it works when >> I apply >> to my repo? >>> Please see below for a few comments. >>> >>> >>>> Thanks >>>> >>>> Brendan >>>> >>>> On 04/06/2021 15:51, Brendan Doyle wrote: >>>>> From 07ecd5d00f82658e094132102575d8d576161a6b 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 | 44 ++++++++++++++++++++++ >>>>> ovn-nb.xml | 9 +++++ >>>>> tests/ovn.at | 103 >>>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++ >>>>> 3 files changed, 156 insertions(+) >>>>> >>>>> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c >>>>> index 0e5092a..9b686d9 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,49 @@ 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"); >>> In my opinion instead of defining these proxy IPs in the logical >>> switch port options, >>> it is better to define them in the router port as generally a router >>> would handle proxy arps (correct me if I'm wrong here). >>> >>> I'd suggest to add a new column "proxy_ips" (or something more appropriate) >>> in the Logical_Router_Port and this column would point to an address set. >>> >>> CMS can define the list of IPs in the address set. ovn-northd can add >>> a logical flow like >>> >>> match = "inport == <router_port> && arp.tpa == $proxy_ip, action = {....} >>> This would result in just one logical flow. >>> >>> CMS would do something like >>> >>> addr_set_uuid=$(ovn-nbctl create address_set name=proxy_arps >>> addresses="10.0.0.4, 10.0.0.5") >>> ovn-nbctl set logical_router_port <router_port> proxy_ip=$addr_set_uuid >>> >>> What do you think ? >> In both cases only one OVS flows is created (after I made the changes) >> suggested by Dan. >> I can see the point with respect to proxy ARP and routers, in physical >> equipment proxy ARP is done >> by routers. But I think doing the proxy ARP on the LRP of the LS is >> consistent with the use of >> the "options : nat-addresses" entry in the Logical_Switch_Port TABLE . >> The patch makes it clear >> that this feature is associated with a router, It is after all in the >> "Options for router ports". I think >> it is also more efficient in that we'd respond to the ARP request >> earlier in the datapath flows. > Even if the proxy arps are associated with a router port, the flows can still be > added in the logical switch pipeline of the peer to avoid the extra hop from > logical switch pipeline to router pipeline. > > In my opinion, it is better that the proxy arps are associated with a > router port > rather than a logical switch peer port. Also having a separate column makes > more sense to me than defining a string of IP addresses in the options smap. I see your point, but "options : nat-addresses" is a list of IPs on a router peer port so it is at least consistent with that. Like "options : nat-addresses" I don't expect it to be a large list. > I defer to other reviewers if they are fine with your patch. Ok thanks. > Thanks > Numan > >> The suggestion you make is a lot more involved, touching much more code >> and changes the DB >> schema, so I'm hesitant to go down that path. >> >> Brendan. >> >>> Thanks >>> Numan >>> >>>>> + if (arp_proxy && op->peer) { >>>>> + char *ips, *ip, *rest; >>>>> + int i = 0; >>>>> + >>>>> + ips = xstrdup(arp_proxy); >>>>> + rest = ips; >>>>> + >>>>> + /* >>>>> + * Match rule on all proxy ARP IPs. >>>>> + */ >>>>> + ds_clear(match); >>>>> + ds_put_cstr(match, "arp.op == 1 && ("); >>>>> + while ((ip = strtok_r(rest,",", &rest))) { >>>>> + if (i++ > 0) { >>>>> + ds_put_cstr(match, " || "); >>>>> + }; >>>>> + ds_put_format(match, "arp.tpa == %s", ip); >>>>> + } >>>>> + ds_put_cstr(match, ")"); >>>>> + >>>>> + 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_); >>>>> + 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..4befe4a 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@openvswitch.org >>>> https://urldefense.com/v3/__https://mail.openvswitch.org/mailman/listinfo/ovs-dev__;!!GqivPVa7Brio!LVM6snHeQdP5rdldKTqBZu_C7h5JiR3HZtfLAmyQisBZUJ2TMpQCtj8JY49M3ZRRqmU$ >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> https://urldefense.com/v3/__https://mail.openvswitch.org/mailman/listinfo/ovs-dev__;!!GqivPVa7Brio!NrcJS3mjIV9x8Gh00JLaF0SagZXDLIzbYhxTxeL6cZuW1gbERdi7pe3ht80s-40RSQw$
On Tue, Jun 15, 2021 at 8:34 AM Brendan Doyle <brendan.doyle@oracle.com> wrote: > > > > On 15/06/2021 13:26, Numan Siddique wrote: > > On Tue, Jun 15, 2021 at 7:50 AM Brendan Doyle <brendan.doyle@oracle.com> wrote: > >> > >> > >> On 14/06/2021 21:17, Numan Siddique wrote: > >>> On Mon, Jun 14, 2021 at 5:19 AM Brendan Doyle <brendan.doyle@oracle.com> wrote: > >>>> Hi, > >>>> > >>>> Just wondering how to move this along. It's been in the queue a while now. > >>>> 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, > >>> > >>> Sorry for the late reply. For some reason the patch doesn't apply to me. > >> Sorry I'm not sure what you mean, the patch did not apply, it works when > >> I apply > >> to my repo? > >>> Please see below for a few comments. > >>> > >>> > >>>> Thanks > >>>> > >>>> Brendan > >>>> > >>>> On 04/06/2021 15:51, Brendan Doyle wrote: > >>>>> From 07ecd5d00f82658e094132102575d8d576161a6b 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 | 44 ++++++++++++++++++++++ > >>>>> ovn-nb.xml | 9 +++++ > >>>>> tests/ovn.at | 103 > >>>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++ > >>>>> 3 files changed, 156 insertions(+) > >>>>> > >>>>> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > >>>>> index 0e5092a..9b686d9 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,49 @@ 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"); > >>> In my opinion instead of defining these proxy IPs in the logical > >>> switch port options, > >>> it is better to define them in the router port as generally a router > >>> would handle proxy arps (correct me if I'm wrong here). > >>> > >>> I'd suggest to add a new column "proxy_ips" (or something more appropriate) > >>> in the Logical_Router_Port and this column would point to an address set. > >>> > >>> CMS can define the list of IPs in the address set. ovn-northd can add > >>> a logical flow like > >>> > >>> match = "inport == <router_port> && arp.tpa == $proxy_ip, action = {....} > >>> This would result in just one logical flow. > >>> > >>> CMS would do something like > >>> > >>> addr_set_uuid=$(ovn-nbctl create address_set name=proxy_arps > >>> addresses="10.0.0.4, 10.0.0.5") > >>> ovn-nbctl set logical_router_port <router_port> proxy_ip=$addr_set_uuid > >>> > >>> What do you think ? > >> In both cases only one OVS flows is created (after I made the changes) > >> suggested by Dan. > >> I can see the point with respect to proxy ARP and routers, in physical > >> equipment proxy ARP is done > >> by routers. But I think doing the proxy ARP on the LRP of the LS is > >> consistent with the use of > >> the "options : nat-addresses" entry in the Logical_Switch_Port TABLE . > >> The patch makes it clear > >> that this feature is associated with a router, It is after all in the > >> "Options for router ports". I think > >> it is also more efficient in that we'd respond to the ARP request > >> earlier in the datapath flows. > > Even if the proxy arps are associated with a router port, the flows can still be > > added in the logical switch pipeline of the peer to avoid the extra hop from > > logical switch pipeline to router pipeline. > > > > In my opinion, it is better that the proxy arps are associated with a > > router port > > rather than a logical switch peer port. Also having a separate column makes > > more sense to me than defining a string of IP addresses in the options smap. > I see your point, but "options : nat-addresses" is a list of IPs on a > router peer > port so it is at least consistent with that. Like "options : > nat-addresses" I don't > expect it to be a large list. Ok. I'd have no objections if other reviewers are fine. Please see below for one comment. Thanks Numan > > I defer to other reviewers if they are fine with your patch. > > Ok thanks. > > Thanks > > Numan > > > >> The suggestion you make is a lot more involved, touching much more code > >> and changes the DB > >> schema, so I'm hesitant to go down that path. > >> > >> Brendan. > >> > >>> Thanks > >>> Numan > >>> > >>>>> + if (arp_proxy && op->peer) { > >>>>> + char *ips, *ip, *rest; > >>>>> + int i = 0; > >>>>> + > >>>>> + ips = xstrdup(arp_proxy); > >>>>> + rest = ips; > >>>>> + > >>>>> + /* > >>>>> + * Match rule on all proxy ARP IPs. > >>>>> + */ > >>>>> + ds_clear(match); > >>>>> + ds_put_cstr(match, "arp.op == 1 && ("); > >>>>> + while ((ip = strtok_r(rest,",", &rest))) { > >>>>> + if (i++ > 0) { > >>>>> + ds_put_cstr(match, " || "); > >>>>> + }; > >>>>> + ds_put_format(match, "arp.tpa == %s", ip); > >>>>> + } > >>>>> + ds_put_cstr(match, ")"); Instead of the above while loop please make use of the util function - extract_ip_address() > >>>>> + > >>>>> + 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_); > >>>>> + 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..4befe4a 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@openvswitch.org > >>>> https://urldefense.com/v3/__https://mail.openvswitch.org/mailman/listinfo/ovs-dev__;!!GqivPVa7Brio!LVM6snHeQdP5rdldKTqBZu_C7h5JiR3HZtfLAmyQisBZUJ2TMpQCtj8JY49M3ZRRqmU$ > >> _______________________________________________ > >> dev mailing list > >> dev@openvswitch.org > >> https://urldefense.com/v3/__https://mail.openvswitch.org/mailman/listinfo/ovs-dev__;!!GqivPVa7Brio!NrcJS3mjIV9x8Gh00JLaF0SagZXDLIzbYhxTxeL6cZuW1gbERdi7pe3ht80s-40RSQw$ > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
On 15/06/2021 14:33, Numan Siddique wrote: > On Tue, Jun 15, 2021 at 8:34 AM Brendan Doyle <brendan.doyle@oracle.com> wrote: >> >> >> On 15/06/2021 13:26, Numan Siddique wrote: >>> On Tue, Jun 15, 2021 at 7:50 AM Brendan Doyle <brendan.doyle@oracle.com> wrote: >>>> >>>> On 14/06/2021 21:17, Numan Siddique wrote: >>>>> On Mon, Jun 14, 2021 at 5:19 AM Brendan Doyle <brendan.doyle@oracle.com> wrote: >>>>>> Hi, >>>>>> >>>>>> Just wondering how to move this along. It's been in the queue a while now. >>>>>> 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, >>>>> >>>>> Sorry for the late reply. For some reason the patch doesn't apply to me. >>>> Sorry I'm not sure what you mean, the patch did not apply, it works when >>>> I apply >>>> to my repo? >>>>> Please see below for a few comments. >>>>> >>>>> >>>>>> Thanks >>>>>> >>>>>> Brendan >>>>>> >>>>>> On 04/06/2021 15:51, Brendan Doyle wrote: >>>>>>> From 07ecd5d00f82658e094132102575d8d576161a6b 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 | 44 ++++++++++++++++++++++ >>>>>>> ovn-nb.xml | 9 +++++ >>>>>>> tests/ovn.at | 103 >>>>>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++ >>>>>>> 3 files changed, 156 insertions(+) >>>>>>> >>>>>>> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c >>>>>>> index 0e5092a..9b686d9 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,49 @@ 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"); >>>>> In my opinion instead of defining these proxy IPs in the logical >>>>> switch port options, >>>>> it is better to define them in the router port as generally a router >>>>> would handle proxy arps (correct me if I'm wrong here). >>>>> >>>>> I'd suggest to add a new column "proxy_ips" (or something more appropriate) >>>>> in the Logical_Router_Port and this column would point to an address set. >>>>> >>>>> CMS can define the list of IPs in the address set. ovn-northd can add >>>>> a logical flow like >>>>> >>>>> match = "inport == <router_port> && arp.tpa == $proxy_ip, action = {....} >>>>> This would result in just one logical flow. >>>>> >>>>> CMS would do something like >>>>> >>>>> addr_set_uuid=$(ovn-nbctl create address_set name=proxy_arps >>>>> addresses="10.0.0.4, 10.0.0.5") >>>>> ovn-nbctl set logical_router_port <router_port> proxy_ip=$addr_set_uuid >>>>> >>>>> What do you think ? >>>> In both cases only one OVS flows is created (after I made the changes) >>>> suggested by Dan. >>>> I can see the point with respect to proxy ARP and routers, in physical >>>> equipment proxy ARP is done >>>> by routers. But I think doing the proxy ARP on the LRP of the LS is >>>> consistent with the use of >>>> the "options : nat-addresses" entry in the Logical_Switch_Port TABLE . >>>> The patch makes it clear >>>> that this feature is associated with a router, It is after all in the >>>> "Options for router ports". I think >>>> it is also more efficient in that we'd respond to the ARP request >>>> earlier in the datapath flows. >>> Even if the proxy arps are associated with a router port, the flows can still be >>> added in the logical switch pipeline of the peer to avoid the extra hop from >>> logical switch pipeline to router pipeline. >>> >>> In my opinion, it is better that the proxy arps are associated with a >>> router port >>> rather than a logical switch peer port. Also having a separate column makes >>> more sense to me than defining a string of IP addresses in the options smap. >> I see your point, but "options : nat-addresses" is a list of IPs on a >> router peer >> port so it is at least consistent with that. Like "options : >> nat-addresses" I don't >> expect it to be a large list. > Ok. I'd have no objections if other reviewers are fine. > > Please see below for one comment. > > Thanks > Numan > >>> I defer to other reviewers if they are fine with your patch. >> Ok thanks. >>> Thanks >>> Numan >>> >>>> The suggestion you make is a lot more involved, touching much more code >>>> and changes the DB >>>> schema, so I'm hesitant to go down that path. >>>> >>>> Brendan. >>>> >>>>> Thanks >>>>> Numan >>>>> >>>>>>> + if (arp_proxy && op->peer) { >>>>>>> + char *ips, *ip, *rest; >>>>>>> + int i = 0; >>>>>>> + >>>>>>> + ips = xstrdup(arp_proxy); >>>>>>> + rest = ips; >>>>>>> + >>>>>>> + /* >>>>>>> + * Match rule on all proxy ARP IPs. >>>>>>> + */ >>>>>>> + ds_clear(match); >>>>>>> + ds_put_cstr(match, "arp.op == 1 && ("); >>>>>>> + while ((ip = strtok_r(rest,",", &rest))) { >>>>>>> + if (i++ > 0) { >>>>>>> + ds_put_cstr(match, " || "); >>>>>>> + }; >>>>>>> + ds_put_format(match, "arp.tpa == %s", ip); >>>>>>> + } >>>>>>> + ds_put_cstr(match, ")"); > Instead of the above while loop please make use of the util function - > extract_ip_address() > Sure will look in to it in a bit, just have to finish something else first. Thanks >>>>>>> + >>>>>>> + 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_); >>>>>>> + 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..4befe4a 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@openvswitch.org >>>>>> https://urldefense.com/v3/__https://mail.openvswitch.org/mailman/listinfo/ovs-dev__;!!GqivPVa7Brio!LVM6snHeQdP5rdldKTqBZu_C7h5JiR3HZtfLAmyQisBZUJ2TMpQCtj8JY49M3ZRRqmU$ >>>> _______________________________________________ >>>> dev mailing list >>>> dev@openvswitch.org >>>> https://urldefense.com/v3/__https://mail.openvswitch.org/mailman/listinfo/ovs-dev__;!!GqivPVa7Brio!NrcJS3mjIV9x8Gh00JLaF0SagZXDLIzbYhxTxeL6cZuW1gbERdi7pe3ht80s-40RSQw$ >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> https://urldefense.com/v3/__https://mail.openvswitch.org/mailman/listinfo/ovs-dev__;!!GqivPVa7Brio!M1gGAG59qK2tuFqHQceuZtUEkfxgv3YLMmZOtNl97ZvcwkDz3bIAz-s9NgmJuQyJi48$
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index 0e5092a..9b686d9 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,49 @@ 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; + int i = 0; + + ips = xstrdup(arp_proxy); + rest = ips; + + /* + * Match rule on all proxy ARP IPs. + */ + ds_clear(match); + ds_put_cstr(match, "arp.op == 1 && ("); + while ((ip = strtok_r(rest,",", &rest))) { + if (i++ > 0) { + ds_put_cstr(match, " || "); + }; + ds_put_format(match, "arp.tpa == %s", ip); + } + ds_put_cstr(match, ")"); + + 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_); + 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..4befe4a 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 +])