Hi Numan, Can we backport this change to 20.03 please ? I did locally, and should be a clean merge.
The OVN packages from Ubuntu LTS 20.04 (focal) which is used upstream to test OpenStack is based on 20.03 and this patch fixes an issue we have with one tempest test [0] [0] https://bugs.launchpad.net/tempest/+bug/1728886 Cheers, Lucas On Fri, Mar 20, 2020 at 8:58 AM Numan Siddique <[email protected]> wrote: > > On Thu, Mar 19, 2020 at 9:51 PM Han Zhou <[email protected]> wrote: > > > > On Thu, Mar 19, 2020 at 5:27 AM <[email protected]> wrote: > > > > > > From: Numan Siddique <[email protected]> > > > > > > If a logical port has 'unknown' address, it means it can send and receive > > > packet with any IP and MAC and generally port security is not set for > > > such logical ports. If an lport has addresses set to - ["MAC1 IP1", > > unknown], > > > right now we add arp responder flows for IP1 and respond MAC1 in the arp > > > response. But it's possible that the VIF of the logical port can use the > > IP1 > > > with a different MAC. This patch supports this usecase. When another > > logical port > > > sends ARP request for IP1, the VIF of the logical port will anyway > > respond. > > > > > > Reported-by: Maciej Józefczyk <[email protected]> > > > Signed-off-by: Numan Siddique <[email protected]> > > > --- > > > northd/ovn-northd.8.xml | 5 +++-- > > > northd/ovn-northd.c | 13 ++++++++----- > > > tests/ovn.at | 16 ++++++++++++---- > > > 3 files changed, 23 insertions(+), 11 deletions(-) > > > > > > diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml > > > index 9b44720d1..7d03cbc83 100644 > > > --- a/northd/ovn-northd.8.xml > > > +++ b/northd/ovn-northd.8.xml > > > @@ -699,8 +699,9 @@ output; > > > > > > <p> > > > These flows are omitted for logical ports (other than router > > ports or > > > - <code>localport</code> ports) that are down and for logical > > ports of > > > - type <code>virtual</code>. > > > + <code>localport</code> ports) that are down, for logical ports > > of > > > + type <code>virtual</code> and for logical ports with 'unknown' > > > + address set. > > > </p> > > > </li> > > > > > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > > > index 4f94680b5..f648d2ea7 100644 > > > --- a/northd/ovn-northd.c > > > +++ b/northd/ovn-northd.c > > > @@ -1152,7 +1152,7 @@ struct ovn_port { > > > > > > bool derived; /* Indicates whether this is an additional port > > > * derived from nbsp or nbrp. */ > > > - > > > + bool has_unknown; /* If the addresses have 'unknown' defined. */ > > > /* The port's peer: > > > * > > > * - A switch port S of type "router" has a router port R as a > > peer, > > > @@ -2059,8 +2059,11 @@ join_logical_ports(struct northd_context *ctx, > > > op->lsp_addrs > > > = xmalloc(sizeof *op->lsp_addrs * nbsp->n_addresses); > > > for (size_t j = 0; j < nbsp->n_addresses; j++) { > > > - if (!strcmp(nbsp->addresses[j], "unknown") > > > - || !strcmp(nbsp->addresses[j], "router")) { > > > + if (!strcmp(nbsp->addresses[j], "unknown")) { > > > + op->has_unknown = true; > > > + continue; > > > + } > > > + if (!strcmp(nbsp->addresses[j], "router")) { > > > continue; > > > } > > > if (is_dynamic_lsp_address(nbsp->addresses[j])) { > > > @@ -6127,7 +6130,7 @@ build_lswitch_flows(struct hmap *datapaths, struct > > hmap *ports, > > > } else { > > > /* > > > * Add ARP/ND reply flows if either the > > > - * - port is up or > > > + * - port is up and it doesn't have 'unknown' address > > defined or > > > * - port type is router or > > > * - port type is localport > > > */ > > > @@ -6136,7 +6139,7 @@ build_lswitch_flows(struct hmap *datapaths, struct > > hmap *ports, > > > continue; > > > } > > > > > > - if (lsp_is_external(op->nbsp)) { > > > + if (lsp_is_external(op->nbsp) || op->has_unknown) { > > > continue; > > > } > > > > > > diff --git a/tests/ovn.at b/tests/ovn.at > > > index 8cdbad743..1b6073ff0 100644 > > > --- a/tests/ovn.at > > > +++ b/tests/ovn.at > > > @@ -1758,11 +1758,13 @@ for is in 1 2 3; do > > > sip=`ip_to_hex 192 168 0 $is$js` > > > tip=`ip_to_hex 192 168 0 $id$jd` > > > tip_unknown=`ip_to_hex 11 11 11 11` > > > + reply_ha=; > > > if test $d != $s; then > > > - reply_ha=f000000000$d > > > - else > > > - reply_ha= > > > + if test $jd != 1; then > > > + reply_ha=f000000000$d > > > + fi > > > fi > > > + > > > test_arp $s f000000000$s $sip $tip $reply_ha > > #9 > > > test_arp $s f000000000$s $sip $tip_unknown > > #10 > > > > > > @@ -2199,7 +2201,13 @@ for s in 1 2 3; do > > > sip=192.168.0.$s > > > tip=192.168.0.$d > > > tip_unknown=11.11.11.11 > > > - if test $d != $s; then reply_ha=f0:00:00:00:00:0$d; else > > reply_ha=; fi > > > + reply_ha=; > > > + if test $d != $s; then > > > + if test $d != 1; then > > > + reply_ha=f0:00:00:00:00:0$d; > > > + fi > > > + fi > > > + > > > test_arp $s f0:00:00:00:00:0$s $sip $tip $reply_ha > > #9 > > > test_arp $s f0:00:00:00:00:0$s $sip $tip_unknown > > #10 > > > > > > -- > > > 2.24.1 > > > > > > _______________________________________________ > > > dev mailing list > > > [email protected] > > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > > Acked-by: Han Zhou <[email protected]> > > Thanks. I applied this patch to master. > > Numan > > > _______________________________________________ > > dev mailing list > > [email protected] > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
