[ovs-dev] [PATCH ovn] northd: Match IPv4 or IPv6 for MAC resolution

Han Zhou zhouhan at gmail.com
Tue Nov 19 21:44:35 UTC 2019


On Tue, Nov 19, 2019 at 1:38 PM Han Zhou <zhouhan at gmail.com> wrote:

>
>
> On Tue, Nov 19, 2019 at 3:33 AM Numan Siddique <numans at ovn.org> wrote:
> >
> > On Tue, Nov 19, 2019 at 7:04 AM Russell Bryant <russell at ovn.org> wrote:
> > >
> > > While debugging some problems in a cluster using ovn-kubernetes, I
> > > noticed that we're creating two conflicting logical flows.  These two
> > > flows only matched on the destination MAC address.  It was not
> > > deterministic whether you'd hit the IPv4 (ARP) or IPv6 (NS) version.
> > >
> > > This change adds an ip4 or ip6 match to each flow as appropriate.
> > >
> > > Signed-off-by: Russell Bryant <russell at ovn.org>
> >
> > Acked-by: Numan Siddique <numans at ovn.org>
> >
> > > ---
> > >  northd/ovn-northd.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > --- NOTE ---
> > >
> > > I've only tested this by running "make check" and "make check-kernel"
> so
> > > far, and all tests still pass.
> > >
> > > If I'm reading this code right, I'm really surprised this hasn't come
> up
> > > sooner?  I guess we also don't have adequate test coverage for these
> > > flows?
> >
> > Thanks for the patch.  Yeah we don't have much coverage here.
> > We should add system tests for this.
> >
> > Numan
> >
>
> I noticed this when I was testing ddlog which couldn't handle this well
> initially but later fixed. I thought it was a problem, too, but then
> figured out it is actually handled by ovn-controller when translating to
> open-flows. The condition ip4/ip6 is added during the translation
> automatically.
>
> This just explains why "this hasn't come up sooner", but the patch LGTM.
It is better to add the condition in logical flows.


> > >
> > >
> > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> > > index 41e97f841..f0ab43b27 100644
> > > --- a/northd/ovn-northd.c
> > > +++ b/northd/ovn-northd.c
> > > @@ -9319,7 +9319,7 @@ build_lrouter_flows(struct hmap *datapaths,
> struct hmap *ports,
> > >          }
> > >
> > >          ovn_lflow_add(lflows, od, S_ROUTER_IN_ARP_REQUEST, 100,
> > > -                      "eth.dst == 00:00:00:00:00:00",
> > > +                      "eth.dst == 00:00:00:00:00:00 && ip4",
> > >                        "arp { "
> > >                        "eth.dst = ff:ff:ff:ff:ff:ff; "
> > >                        "arp.spa = reg1; "
> > > @@ -9328,7 +9328,7 @@ build_lrouter_flows(struct hmap *datapaths,
> struct hmap *ports,
> > >                        "output; "
> > >                        "};");
> > >          ovn_lflow_add(lflows, od, S_ROUTER_IN_ARP_REQUEST, 100,
> > > -                      "eth.dst == 00:00:00:00:00:00",
> > > +                      "eth.dst == 00:00:00:00:00:00 && ip6",
> > >                        "nd_ns { "
> > >                        "nd.target = xxreg0; "
> > >                        "output; "
> > > --
> > > 2.23.0
> > >
> > > _______________________________________________
> > > 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