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

Russell Bryant russell at ovn.org
Tue Nov 19 22:18:47 UTC 2019


On Tue, Nov 19, 2019 at 4:44 PM Han Zhou <zhouhan at gmail.com> wrote:
>
>
>
> 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.

Interesting - and it works the same even though there are different
arguments to arp{} or nd_ns{} in each logical flow?

>
>>
>> > >
>> > >
>> > > 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



-- 
Russell Bryant


More information about the dev mailing list