| Message ID | 20191119012712.344975-1-russell@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,ovn] northd: Match IPv4 or IPv6 for MAC resolution | expand |
On Tue, Nov 19, 2019 at 7:04 AM Russell Bryant <russell@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@ovn.org> Acked-by: Numan Siddique <numans@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 > > > 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@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On Tue, Nov 19, 2019 at 6:33 AM Numan Siddique <numans@ovn.org> wrote: > > On Tue, Nov 19, 2019 at 7:04 AM Russell Bryant <russell@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@ovn.org> > > Acked-by: Numan Siddique <numans@ovn.org> Thanks! I applied this to master. > > > --- > > 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 > > > > > > > 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@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >
On Tue, Nov 19, 2019 at 3:33 AM Numan Siddique <numans@ovn.org> wrote: > > On Tue, Nov 19, 2019 at 7:04 AM Russell Bryant <russell@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@ovn.org> > > Acked-by: Numan Siddique <numans@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. > > > > > > 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@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
On Tue, Nov 19, 2019 at 1:38 PM Han Zhou <zhouhan@gmail.com> wrote: > > > On Tue, Nov 19, 2019 at 3:33 AM Numan Siddique <numans@ovn.org> wrote: > > > > On Tue, Nov 19, 2019 at 7:04 AM Russell Bryant <russell@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@ovn.org> > > > > Acked-by: Numan Siddique <numans@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@openvswitch.org > > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On Tue, Nov 19, 2019 at 4:44 PM Han Zhou <zhouhan@gmail.com> wrote: > > > > On Tue, Nov 19, 2019 at 1:38 PM Han Zhou <zhouhan@gmail.com> wrote: >> >> >> >> On Tue, Nov 19, 2019 at 3:33 AM Numan Siddique <numans@ovn.org> wrote: >> > >> > On Tue, Nov 19, 2019 at 7:04 AM Russell Bryant <russell@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@ovn.org> >> > >> > Acked-by: Numan Siddique <numans@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@openvswitch.org >> > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> > > >> > _______________________________________________ >> > dev mailing list >> > dev@openvswitch.org >> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
On Tue, Nov 19, 2019 at 2:19 PM Russell Bryant <russell@ovn.org> wrote: > On Tue, Nov 19, 2019 at 4:44 PM Han Zhou <zhouhan@gmail.com> wrote: > > > > > > > > On Tue, Nov 19, 2019 at 1:38 PM Han Zhou <zhouhan@gmail.com> wrote: > >> > >> > >> > >> On Tue, Nov 19, 2019 at 3:33 AM Numan Siddique <numans@ovn.org> wrote: > >> > > >> > On Tue, Nov 19, 2019 at 7:04 AM Russell Bryant <russell@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@ovn.org> > >> > > >> > Acked-by: Numan Siddique <numans@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? > > They are two different lflows (since the actions are different), and translated in two different OVS flows. During translation by ovn-controller, when parsing the actions, the ip4/ip6 is specified as prerequisite, and then the prerequisite is added as a match condition, too. Please see: https://github.com/ovn-org/ovn/blob/master/lib/actions.c#L1169 https://github.com/ovn-org/ovn/blob/master/lib/actions.c#L1211 > > > >> > >> > > > >> > > > >> > > 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@openvswitch.org > >> > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >> > > > >> > _______________________________________________ > >> > dev mailing list > >> > dev@openvswitch.org > >> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > > -- > Russell Bryant >
On Tue, Nov 19, 2019 at 5:32 PM Han Zhou <zhouhan@gmail.com> wrote: > > > On Tue, Nov 19, 2019 at 2:19 PM Russell Bryant <russell@ovn.org> wrote: > >> On Tue, Nov 19, 2019 at 4:44 PM Han Zhou <zhouhan@gmail.com> wrote: >> > >> > >> > >> > On Tue, Nov 19, 2019 at 1:38 PM Han Zhou <zhouhan@gmail.com> wrote: >> >> >> >> >> >> >> >> On Tue, Nov 19, 2019 at 3:33 AM Numan Siddique <numans@ovn.org> wrote: >> >> > >> >> > On Tue, Nov 19, 2019 at 7:04 AM Russell Bryant <russell@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@ovn.org> >> >> > >> >> > Acked-by: Numan Siddique <numans@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? >> >> They are two different lflows (since the actions are different), and > translated in two different OVS flows. During translation by > ovn-controller, when parsing the actions, the ip4/ip6 is specified as > prerequisite, and then the prerequisite is added as a match condition, too. > Please see: > https://github.com/ovn-org/ovn/blob/master/lib/actions.c#L1169 > https://github.com/ovn-org/ovn/blob/master/lib/actions.c#L1211 > Ah ha! That explains it. Thank you. :-) > > >> > >> >> >> >> > > >> >> > > >> >> > > 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@openvswitch.org >> >> > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> >> > > >> >> > _______________________________________________ >> >> > dev mailing list >> >> > dev@openvswitch.org >> >> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> >> >> >> -- >> Russell Bryant >> >
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; "
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@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?