[ovs-dev] [PATCH v5 3/6] ovn: extend expr symbols for ND

[Numan Siddique]

On Fri, May 5, 2017 at 5:21 AM, Ben Pfaff <blp at ovn.org> wrote:

> On Thu, May 04, 2017 at 08:43:26PM +0530, nusiddiq at redhat.com wrote:
> > From: Zong Kai LI <zealokii at gmail.com>
> >
> > This patch updates symbols in logical-fields for ND, such as "nd",
> > "nd.target", "nd.sll", "nd.tll", to describe more clear about
> "icmp6.type"
> > predicate.
> > It adds new symbols:
> >  - "nd_rs" stands for Router Solicitation message,
> >  - "nd_ra" stands for Router Advertisement message,
> >
> > Signed-off-by: Zongkai LI <zealokii at gmail.com>
> > Signed-off-by: Numan Siddique <nusiddiq at redhat.com>
> > ---
> >  ovn/lib/logical-fields.c | 18 ++++++++++++++----
> >  ovn/ovn-sb.xml           |  4 +++-
> >  tests/ovn.at             |  2 +-
> >  3 files changed, 18 insertions(+), 6 deletions(-)
> >
> > diff --git a/ovn/lib/logical-fields.c b/ovn/lib/logical-fields.c
> > index 26e336f..f8837f2 100644
> > --- a/ovn/lib/logical-fields.c
> > +++ b/ovn/lib/logical-fields.c
> > @@ -178,14 +178,24 @@ ovn_init_symtab(struct shash *symtab)
> >      expr_symtab_add_field(symtab, "arp.tha", MFF_ARP_THA, "arp", false);
> >
> >      expr_symtab_add_predicate(symtab, "nd",
> > -              "icmp6.type == {135, 136} && icmp6.code == 0 && ip.ttl ==
> 255");
> > +              "icmp6.type == {133, 134, 135, 136} "
> > +              "&& icmp6.code == 0 && ip.ttl == 255");
> > +    expr_symtab_add_predicate(symtab, "nd_rs",
> > +              "icmp6.type == 133 && icmp6.code == 0 && ip.ttl == 255");
> > +    expr_symtab_add_predicate(symtab, "nd_ra",
> > +              "icmp6.type == 134 && icmp6.code == 0 && ip.ttl == 255");
> >      expr_symtab_add_predicate(symtab, "nd_ns",
> >                "icmp6.type == 135 && icmp6.code == 0 && ip.ttl == 255");
> >      expr_symtab_add_predicate(symtab, "nd_na",
> >                "icmp6.type == 136 && icmp6.code == 0 && ip.ttl == 255");
> > -    expr_symtab_add_field(symtab, "nd.target", MFF_ND_TARGET, "nd",
> false);
> > -    expr_symtab_add_field(symtab, "nd.sll", MFF_ND_SLL, "nd_ns", false);
> > -    expr_symtab_add_field(symtab, "nd.tll", MFF_ND_TLL, "nd_na", false);
> > +    expr_symtab_add_field(symtab, "nd.target", MFF_ND_TARGET,
> > +              "icmp6.type == {135, 136} "
> > +              "&& icmp6.code == 0 && ip.ttl == 255", false);
> > +    expr_symtab_add_field(symtab, "nd.sll", MFF_ND_SLL,
> > +              "icmp6.type == {133, 134, 135} "
> > +              "&& icmp6.code == 0 && ip.ttl == 255", false);
> > +    expr_symtab_add_field(symtab, "nd.tll", MFF_ND_TLL,
> > +              "icmp6.type == 136 && icmp6.code == 0 && ip.ttl == 255",
> false);
>
> This patch changes the meaning of existing logical flow tables: flows
> that previously matched on just "nd" previously matched only neighbor
> solicitation and neighbor announcement packets, but after this change
> they also match two new kinds of neighbor discovery packets.  ovn-northd
> does actually use flows that match on just "nd", so I am concerned that
> this could have undesirable effects.  Have you looked at the existing
> flow tables and determined how this will change their behavior?
>

You are right.  ovn-northd uses "nd" in 3 places - [1], [2] and [3]

For [1], the match should be for neighbor solicitation packets.
For [2] and [3], the match can also be extended to neighbor advertisement
packets.

I will update this patch to use "nd_ns" for [1]. Please let me know if you
have any concerns with this.


[1] -
https://github.com/openvswitch/ovs/blob/master/ovn/northd/ovn-northd.c#L2142
[2] -
https://github.com/openvswitch/ovs/blob/master/ovn/northd/ovn-northd.c#L2635
[3] -
https://github.com/openvswitch/ovs/blob/master/ovn/northd/ovn-northd.c#L2881


Thanks
Numan


> Thanks,
>
> Ben.
>