[ovs-dev] [PATCH ovn] ovn-northd: Don't add arp responder flows for lports with 'unknown' address.
Lucas Alvares Gomes
lucasagomes at gmail.com
Tue Feb 16 13:25:35 UTC 2021
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 <numans at ovn.org> wrote:
>
> On Thu, Mar 19, 2020 at 9:51 PM Han Zhou <zhouhan at gmail.com> wrote:
> >
> > On Thu, Mar 19, 2020 at 5:27 AM <numans at ovn.org> wrote:
> > >
> > > From: Numan Siddique <numans at ovn.org>
> > >
> > > 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 <mjozefcz at redhat.com>
> > > Signed-off-by: Numan Siddique <numans at ovn.org>
> > > ---
> > > 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
> > > dev at openvswitch.org
> > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
> > Acked-by: Han Zhou <hzhou at ovn.org>
>
> Thanks. I applied this patch to master.
>
> Numan
>
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
More information about the dev
mailing list