> On Oct 9, 2015, at 9:21 PM, Ben Pfaff <b...@nicira.com> wrote:
>
> ---
> ovn/TODO | 6 -
From ovn/TODO:
-=-=-=-=-=-=-=-=-
** IP to MAC binding
Somehow it has to be possible for an L3 logical router to map from an
IP address to an Ethernet address. This can happen statically or
dynamically. Probably both cases need to be supported eventually.
-=-=-=-=-=-=-=-=-
With this patch, I think this part of TODO could be updated, since static is
supported.
> /* The 'key' comes from nb->header_.uuid or sb->external_ids:logical-switch.
> */
> struct ovn_datapath {
> struct hmap_node key_node; /* Index on 'key'. */
> - struct uuid key; /* nb->header_.uuid. */
> + struct uuid key; /* (nbs/nbr)->header_.uuid. */
I think the comment describing this structure also needs an update.
> - if (!smap_get_uuid(&sb->external_ids, "logical-switch", &key)) {
> + if (!smap_get_uuid(&sb->external_ids, "logical-switch", &key) &&
> + !smap_get_uuid(&sb->external_ids, "logical-router", &key)) {
> ovsdb_idl_txn_add_comment(ctx->ovnsb_txn,
> "deleting Datapath_Binding "UUID_FMT"
> that "
> "lacks external-ids:logical-switch",
I think this comment can use an update to include "logical-router". There's a
warning a few lines down with a similar issue.
> struct ovn_port {
> struct hmap_node key_node; /* Index on 'key'. */
> - const char *key; /* nb->name and sb->logical_port */
> + char *key; /* nb->name and sb->logical_port */
I think this can be either "nbs" or "nbr", not "nb".
> + /* Logical router port data. */
> + ovs_be32 ip, mask; /* 192.168.10.123/24. */
> + ovs_be32 network; /* 192.168.10.0. */
> + ovs_be32 bcast; /* 192.168.10.255. */
It's probably obvious, but it may be worth pointing out that these are example
values if the system were supplied an IP/mask of "192.168.10.123/24".
> static struct ovn_port *
> ovn_port_create(struct hmap *ports, const char *key,
> - const struct nbrec_logical_port *nb,
> + const struct nbrec_logical_port *nbs,
> + const struct nbrec_logical_router_port *nbr,
Do you think it's worth renaming the table Logical_Port to Logical_Switch_Port?
> + ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_ROUTING,
> + count_1bits(ntohl(mask)), match, ds_cstr(&actions));
It might be worth mentioning the priorities are based on the netmask to achieve
LPM.
>
> + /* ARP reply. These flows reply to ARP requests for the router's own
> + * IP address. */
> + match = xasprintf(
> + "inport == %s && arp.tha == "ETH_ADDR_FMT" && arp.op == 1",
> + op->json_key, ETH_ADDR_ARGS(op->mac));
As Han mentioned, this should be the "arp.tpa" instead of "tha".
> + char *actions = xasprintf(
> + "eth.dst = eth.src; "
> + "eth.src = "ETH_ADDR_FMT"; "
Shouldn't we be setting "eth.type"?
> + "inport = \"\"; /* Allow sending out inport. */ "
I assume you tested this, and ovn-controller handles this properly.
> + <group title="OVN_Northbound Relationship">
> + <p>
> + Each row in <ref table="Datapath_Binding"/> is associated with some
> + logical datapath. <code>ovn-northd</code> uses these key to track
> the
> + association of a logical datapath with concepts in the <ref
> + db="OVN_Northbound"/> database.
s/key/keys/
This is awesome! Can't wait to try it all together!
Acked-by: Justin Pettit <jpet...@nicira.com>
--Justin
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
