On Thu, Jun 9, 2016 at 5:32 AM, Ben Pfaff <b...@ovn.org> wrote:
> On Wed, Jun 08, 2016 at 03:28:58PM +0800, Zong Kai LI wrote:
> > This patch adds some lflows for 'na' action to support ND versus ARP.
> >
> > For ovn-northd, it will generate lflows per each IPv6 address on
> > echo lport, with lport mac and IPv6 addresss, with 'na' action.
> > e.g. match=(ip6 && nd && icmp6.type == 135 &&
> > nd.target == fde3:f657:aac1:0:f816:3eff:fe13:8198),
> > action=(na{fa:16:3e:13:81:98; reg0 = 0x1; outport = inport;
> > inport = ""; output;};)
> > And new lflows will be set in tabel ls_in_arp_nd_rsp, which is renamed
> > from previous ls_in_arp_rsp.
> >
> > Setting reg0 = 0x1 to mention that such kind of NA packets are replied
> > by ovn-controller, and for these packets, dont do conntrack on them.
> > Also modfiy current table 32 and table 48, to make these packets
> > output directly.
> >
> > Signed-off-by: Zong Kai LI <zealo...@gmail.com>
>
> I don't understand why it is necessary to have special-case code in
> ovn-controller physical_run() for neighbor advertisements. Nothing
> similar is needed for ARP. It would be much better to avoid special
> cases. Can you explain? At any rate, ovn-controller should definitely
> not have any knowledge of what purpose the logical flows use registers
> for.
>
> This adds a Linux-specific header file to physical.c, but that should
> not be necessary.
>
> None of these casts should be necessary:
> > + match_set_nw_proto(&match, (uint8_t)IPPROTO_ICMPV6);
> > + match_set_icmp_type(&match, (uint8_t)ND_NEIGHBOR_ADVERT);
> > + match_set_reg(&match, 0, (uint32_t)1);
>
> In ovn-northd, it seems like a really bad idea to use substrings
> searches on ACLs as a basis for making decisions.
>
> Thanks,
>
> Ben.
>
Hi.Ben.
--- Explanation for special modification for ND in physical.c---
Yes, there are something different from ARP for ND.
For logical egress pipeline, before my modification:
- for ARP packets, in OVN table 48(ls_out_pre_acl), they will match lflow
"match=(1), action=(next;)", while for ND packets will match lflow
"match=(ip), action=(ct_next;)"
- and the same to following tables, the flows ARP packets matched will
not have conntrack actions.
- but for NA packets generated/replied by ovn-controller, they are new to
conntrack, and will not get correct processed, and dropped by ovs-vswitch
later with warning message like "Failed to acquire udpif_key corresponding
to unexpected flow (Invalid argument)".
So for NA packets generated/replied by ovn-controller, I want to use reg0
to mark those packets, and make them directly outputted to table 64(the
output table) from table 48, not go through conntrack by tables after table
48.
So for OVN table ls_out_pre_acl, we will have a lflow with match and action
such as "match=(ip6 && icmp6 && icmp6.type == 136 && reg0 == 0x1),
action=(output;)".
It's OK for actions for ND to set reg0 in OVN
table ls_in_arp_nd_rsp(renamed from ls_in_arp_rsp).
But OVN table 34(OFTABLE_DROP_LOOPBACK) will reset regX(X=0,1,2) to 0. So I
try to modify table 32 to skip that and directly output ND packets replied
by ovn-controller to table 48.
--- End of explanation---
If above explanation make sense for you, then in physical.c, I think at
least <netinet/icmp6.h> is needed, ND_NEIGHBOR_ADVERT will need it.
Yes, the casts you mentioned are unnecessary, I will remove them.
In ovn-northd, I will try to find a better way to do ACLs searching.
Thanks for your time and review, and have a nice day! :)
Best regards,
Zong Kai, LI
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
