[ovs-dev] [PATCH v2 ovn 1/5] ovn-northd: Document OVS register usage in logical flows.
Dumitru Ceara
dceara at redhat.com
Thu Jul 2 07:24:31 UTC 2020
On 7/1/20 8:57 PM, Han Zhou wrote:
>
>
> On Tue, Jun 30, 2020 at 2:41 PM Dumitru Ceara <dceara at redhat.com
> <mailto:dceara at redhat.com>> wrote:
>>
>> Also, use macros instead of bare references to register names.
>>
>> Signed-off-by: Dumitru Ceara <dceara at redhat.com
> <mailto:dceara at redhat.com>>
>> ---
>> northd/ovn-northd.c | 166
> ++++++++++++++++++++++++++++++++++++---------------
>> 1 file changed, 118 insertions(+), 48 deletions(-)
>>
>> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
>> index ae1b85a..1a47f5f 100644
>> --- a/northd/ovn-northd.c
>> +++ b/northd/ovn-northd.c
>> @@ -227,8 +227,63 @@ enum ovn_stage {
>> #define REG_ECMP_GROUP_ID "reg8[0..15]"
>> #define REG_ECMP_MEMBER_ID "reg8[16..31]"
>>
>> +/* Registers used for routing. */
>> +#define REG_NEXT_HOP_IPV4 "reg0"
>> +#define REG_NEXT_HOP_IPV6 "xxreg0"
>> +#define REG_SRC_IPV4 "reg1"
>> +#define REG_SRC_IPV6 "xxreg1"
>> +
>> #define FLAGBIT_NOT_VXLAN "flags[1] == 0"
>>
>> +/*
>> + * OVS register usage:
>> + *
>> + * Logical Switch pipeline:
>> + * +----------+-------------------------------------+
>> + * | R0 | REGBIT_{CONNTRACK/DHCP/DNS/HAIRPIN} |
>> + * +----------+-------------------------------------+
>> + * | R1 - R15 | UNUSED |
>> + * +----------+-------------------------------------+
>> + *
>> + * Logical Router pipeline:
>> + * +-----+--------------------------+---+-------------+
>> + * | R0 | REGBIT_ND_RA_OPTS_RESULT | | |
>> + * | | IPv4-NEXT-HOP | | |
>> + * +-----+--------------------------+ | |
>> + * | R1 | IPv4-SRC-IP for ARP-REQ | | |
>> + * +-----+--------------------------+ | |
>> + * | R2 | UNUSED | X | |
>> + * +-----+--------------------------+ X | |
>> + * | R3 | UNUSED | R | IPv6 |
>> + * +-----+--------------------------+ E | NEXT-HOP |
>> + * | R4 | UNUSED | G | |
>> + * +-----+--------------------------+ 0 | |
>> + * | R5 | UNUSED | | |
>> + * +-----+--------------------------+ | |
>> + * | R6 | UNUSED | | |
>> + * +-----+--------------------------+ | |
>> + * | R7 | UNUSED | | |
>> + * +-----+--------------------------+---+-------------+
>> + * | R8 | ECMP_GROUP_ID | | |
>> + * | | ECMP_MEMBER_ID | | |
>> + * +-----+--------------------------+ | |
>> + * | R9 | UNUSED | | |
>> + * +-----+--------------------------+ | |
>> + * | R10 | UNUSED | X | |
>> + * +-----+--------------------------+ X | |
>> + * | R11 | UNUSED | R | IPv6-SRC-IP |
>> + * +-----+--------------------------+ E | for NS |
>> + * | R12 | UNUSED | G | |
>> + * +-----+--------------------------+ 1 | |
>> + * | R13 | UNUSED | | |
>> + * +-----+--------------------------+ | |
>> + * | R14 | UNUSED | | |
>> + * +-----+--------------------------+ | |
>> + * | R15 | UNUSED | | |
>> + * +-----+--------------------------+---+-------------+
>> + *
>> + */
>
> Thanks for clearly documenting register usage in the table format.
> However, I think here is a mistake. The xxreg registers are 128-bit
> each, and should overlay only 4 regular registers. E.g. xxreg0 should
> span R0 - R3, xxreg1 should span R4 - R7.
Oops, my bad.. I had started with documenting all registers but then
decided that I shouldn't include R10-R15 (logical fields) but I messed
up the diagram. Thanks for spotting this, I'll fix it in v3. Same
applies for patch 2/5 of the series.
>
> With this addressed:
> Acked-by: Han Zhou <hzhou at ovn.org <mailto:hzhou at ovn.org>>
Thanks,
Dumitru
>> +
>> /* Returns an "enum ovn_stage" built from the arguments. */
>> static enum ovn_stage
>> ovn_stage_build(enum ovn_datapath_type dp_type, enum ovn_pipeline
> pipeline,
>> @@ -7127,15 +7182,15 @@ build_routing_policy_flow(struct hmap *lflows,
> struct ovn_datapath *od,
>> ds_put_format(&actions, "pkt.mark = %u; ", pkt_mark);
>> }
>> bool is_ipv4 = strchr(rule->nexthop, '.') ? true : false;
>> - ds_put_format(&actions, "%sreg0 = %s; "
>> - "%sreg1 = %s; "
>> + ds_put_format(&actions, "%s = %s; "
>> + "%s = %s; "
>> "eth.src = %s; "
>> "outport = %s; "
>> "flags.loopback = 1; "
>> "next;",
>> - is_ipv4 ? "" : "xx",
>> + is_ipv4 ? REG_NEXT_HOP_IPV4 : REG_NEXT_HOP_IPV6,
>> rule->nexthop,
>> - is_ipv4 ? "" : "xx",
>> + is_ipv4 ? REG_SRC_IPV4 : REG_SRC_IPV6,
>> lrp_addr_s,
>> out_port->lrp_networks.ea_s,
>> out_port->json_key);
>> @@ -7525,14 +7580,14 @@ build_ecmp_route_flow(struct hmap *lflows,
> struct ovn_datapath *od,
>> REG_ECMP_MEMBER_ID" == %"PRIu16,
>> eg->id, er->id);
>> ds_clear(&actions);
>> - ds_put_format(&actions, "%sreg0 = %s; "
>> - "%sreg1 = %s; "
>> + ds_put_format(&actions, "%s = %s; "
>> + "%s = %s; "
>> "eth.src = %s; "
>> "outport = %s; "
>> "next;",
>> - is_ipv4 ? "" : "xx",
>> + is_ipv4 ? REG_NEXT_HOP_IPV4 : REG_NEXT_HOP_IPV6,
>> route->nexthop,
>> - is_ipv4 ? "" : "xx",
>> + is_ipv4 ? REG_SRC_IPV4 : REG_SRC_IPV6,
>> lrp_addr_s,
>> out_port->lrp_networks.ea_s,
>> out_port->json_key);
>> @@ -7567,8 +7622,8 @@ add_route(struct hmap *lflows, const struct
> ovn_port *op,
>> &match, &priority);
>>
>> struct ds actions = DS_EMPTY_INITIALIZER;
>> - ds_put_format(&actions, "ip.ttl--; "REG_ECMP_GROUP_ID" = 0;
> %sreg0 = ",
>> - is_ipv4 ? "" : "xx");
>> + ds_put_format(&actions, "ip.ttl--; "REG_ECMP_GROUP_ID" = 0; %s = ",
>> + is_ipv4 ? REG_NEXT_HOP_IPV4 : REG_NEXT_HOP_IPV6);
>>
>> if (gateway) {
>> ds_put_cstr(&actions, gateway);
>> @@ -7576,12 +7631,12 @@ add_route(struct hmap *lflows, const struct
> ovn_port *op,
>> ds_put_format(&actions, "ip%s.dst", is_ipv4 ? "4" : "6");
>> }
>> ds_put_format(&actions, "; "
>> - "%sreg1 = %s; "
>> + "%s = %s; "
>> "eth.src = %s; "
>> "outport = %s; "
>> "flags.loopback = 1; "
>> "next;",
>> - is_ipv4 ? "" : "xx",
>> + is_ipv4 ? REG_SRC_IPV4 : REG_SRC_IPV6,
>> lrp_addr_s,
>> op->lrp_networks.ea_s,
>> op->json_key);
>> @@ -9052,7 +9107,8 @@ build_lrouter_flows(struct hmap *datapaths,
> struct hmap *ports,
>> ds_put_format(
>> &match, "outport == %s && %s == %s",
>> od->l3dgw_port->json_key,
>> - is_v6 ? "xxreg0" : "reg0", nat->external_ip);
>> + is_v6 ? REG_NEXT_HOP_IPV6 : REG_NEXT_HOP_IPV4,
>> + nat->external_ip);
>> ds_clear(&actions);
>> ds_put_format(
>> &actions, "eth.dst = %s; next;",
>> @@ -9214,9 +9270,10 @@ build_lrouter_flows(struct hmap *datapaths,
> struct hmap *ports,
>> * router, add flows that are specific to a NAT rule. These
>> * flows indicate the presence of an applicable NAT rule that
>> * can be applied in a distributed manner.
>> - * In particulr reg1 and eth.src are set to NAT external
> IP and
>> - * NAT external mac so the ARP request generated in the
> following
>> - * stage is sent out with proper IP/MAC src addresses
>> + * In particulr REG_SRC_IPV4/REG_SRC_IPV6 and eth.src are
> set to
>> + * NAT external IP and NAT external mac so the ARP request
>> + * generated in the following stage is sent out with
> proper IP/MAC
>> + * src addresses.
>> */
>> if (distributed) {
>> ds_clear(&match);
>> @@ -9226,8 +9283,9 @@ build_lrouter_flows(struct hmap *datapaths,
> struct hmap *ports,
>> "is_chassis_resident(\"%s\")",
>> is_v6 ? "6" : "4", nat->logical_ip,
>> od->l3dgw_port->json_key,
> nat->logical_port);
>> - ds_put_format(&actions, "eth.src = %s; %sreg1 = %s;
> next;",
>> - nat->external_mac, is_v6 ? "xx" : "",
>> + ds_put_format(&actions, "eth.src = %s; %s = %s; next;",
>> + nat->external_mac,
>> + is_v6 ? REG_SRC_IPV6 : REG_SRC_IPV4,
>> nat->external_ip);
>> ovn_lflow_add_with_hint(lflows, od,
> S_ROUTER_IN_GW_REDIRECT,
>> 100, ds_cstr(&match),
>> @@ -9549,14 +9607,15 @@ build_lrouter_flows(struct hmap *datapaths,
> struct hmap *ports,
>> *
>> * For regular routes without ECMP, table IP_ROUTING sets outport
> to the
>> * correct output port, eth.src to the output port's MAC address, and
>> - * '[xx]reg0' to the next-hop IP address (leaving 'ip[46].dst', the
>> - * packet’s final destination, unchanged), and advances to the
> next table.
>> + * REG_NEXT_HOP_IPV4/REG_NEXT_HOP_IPV6 to the next-hop IP address
>> + * (leaving 'ip[46].dst', the packet’s final destination,
> unchanged), and
>> + * advances to the next table.
>> *
>> * For ECMP routes, i.e. multiple routes with same policy and
> prefix, table
>> * IP_ROUTING remembers ECMP group id and selects a member id,
> and advances
>> - * to table IP_ROUTING_ECMP, which sets outport, eth.src and
> '[xx]reg0' for
>> - * the selected ECMP member.
>> - * */
>> + * to table IP_ROUTING_ECMP, which sets outport, eth.src and
>> + * REG_NEXT_HOP_IPV4/REG_NEXT_HOP_IPV6 for the selected ECMP member.
>> + */
>> HMAP_FOR_EACH (op, key_node, ports) {
>> if (!op->nbrp) {
>> continue;
>> @@ -9693,8 +9752,8 @@ build_lrouter_flows(struct hmap *datapaths,
> struct hmap *ports,
>> * permitted/denied/rerouted to the address in the rule's nexthop.
>> * This table sets outport to the correct out_port,
>> * eth.src to the output port's MAC address,
>> - * and '[xx]reg0' to the next-hop IP address (leaving
>> - * 'ip[46].dst', the packet’s final destination, unchanged), and
>> + * and REG_NEXT_HOP_IPV4/REG_NEXT_HOP_IPV6 to the next-hop IP address
>> + * (leaving 'ip[46].dst', the packet’s final destination,
> unchanged), and
>> * advances to the next table for ARP/ND resolution. */
>> HMAP_FOR_EACH (od, key_node, datapaths) {
>> if (!od->nbr) {
>> @@ -9731,9 +9790,11 @@ build_lrouter_flows(struct hmap *datapaths,
> struct hmap *ports,
>> /* Local router ingress table ARP_RESOLVE: ARP Resolution.
>> *
>> * Any unicast packet that reaches this table is an IP packet whose
>> - * next-hop IP address is in reg0. (ip4.dst is the final
> destination.)
>> - * This table resolves the IP address in reg0 into an output port in
>> - * outport and an Ethernet address in eth.dst.
>> + * next-hop IP address is in REG_NEXT_HOP_IPV4/REG_NEXT_HOP_IPV6
>> + * (ip4.dst/ipv6.dst is the final destination).
>> + * This table resolves the IP address in
>> + * REG_NEXT_HOP_IPV4/REG_NEXT_HOP_IPV6 into an output port in
> outport and
>> + * an Ethernet address in eth.dst.
>> */
>> HMAP_FOR_EACH (op, key_node, ports) {
>> if (op->nbsp && !lsp_is_enabled(op->nbsp)) {
>> @@ -9742,17 +9803,18 @@ build_lrouter_flows(struct hmap *datapaths,
> struct hmap *ports,
>>
>> if (op->nbrp) {
>> /* This is a logical router port. If next-hop IP address in
>> - * '[xx]reg0' matches IP address of this router port, then
>> - * the packet is intended to eventually be sent to this
>> - * logical port. Set the destination mac address using this
>> - * port's mac address.
>> + * REG_NEXT_HOP_IPV4/REG_NEXT_HOP_IPV6 matches IP address
> of this
>> + * router port, then the packet is intended to eventually
> be sent
>> + * to this logical port. Set the destination mac address
> using
>> + * this port's mac address.
>> *
>> * The packet is still in peer's logical pipeline. So the
> match
>> * should be on peer's outport. */
>> if (op->peer && op->nbrp->peer) {
>> if (op->lrp_networks.n_ipv4_addrs) {
>> ds_clear(&match);
>> - ds_put_format(&match, "outport == %s && reg0 == ",
>> + ds_put_format(&match, "outport == %s && "
>> + REG_NEXT_HOP_IPV4 "== ",
>> op->peer->json_key);
>> op_put_v4_networks(&match, op, false);
>>
>> @@ -9767,7 +9829,8 @@ build_lrouter_flows(struct hmap *datapaths,
> struct hmap *ports,
>>
>> if (op->lrp_networks.n_ipv6_addrs) {
>> ds_clear(&match);
>> - ds_put_format(&match, "outport == %s && xxreg0 == ",
>> + ds_put_format(&match, "outport == %s && "
>> + REG_NEXT_HOP_IPV6 " == ",
>> op->peer->json_key);
>> op_put_v6_networks(&match, op);
>>
>> @@ -9838,7 +9901,8 @@ build_lrouter_flows(struct hmap *datapaths,
> struct hmap *ports,
>> }
>>
>> ds_clear(&match);
>> - ds_put_format(&match, "outport == %s && reg0
> == %s",
>> + ds_put_format(&match, "outport == %s && "
>> + REG_NEXT_HOP_IPV4 " == %s",
>> peer->json_key, ip_s);
>>
>> ds_clear(&actions);
>> @@ -9874,7 +9938,8 @@ build_lrouter_flows(struct hmap *datapaths,
> struct hmap *ports,
>> }
>>
>> ds_clear(&match);
>> - ds_put_format(&match, "outport == %s &&
> xxreg0 == %s",
>> + ds_put_format(&match, "outport == %s && "
>> + REG_NEXT_HOP_IPV6 " == %s",
>> peer->json_key, ip_s);
>>
>> ds_clear(&actions);
>> @@ -9925,8 +9990,9 @@ build_lrouter_flows(struct hmap *datapaths,
> struct hmap *ports,
>>
>> if (find_lrp_member_ip(peer, vip)) {
>> ds_clear(&match);
>> - ds_put_format(&match, "outport == %s && reg0
> == %s",
>> - peer->json_key, vip);
>> + ds_put_format(&match, "outport == %s && "
>> + REG_NEXT_HOP_IPV4 " == %s",
>> + peer->json_key, vip);
>>
>> ds_clear(&actions);
>> ds_put_format(&actions,
>> @@ -9971,8 +10037,9 @@ build_lrouter_flows(struct hmap *datapaths,
> struct hmap *ports,
>> }
>>
>> ds_clear(&match);
>> - ds_put_format(&match, "outport == %s && reg0
> == %s",
>> - peer->json_key, vip);
>> + ds_put_format(&match, "outport == %s && "
>> + REG_NEXT_HOP_IPV4 " == %s",
>> + peer->json_key, vip);
>>
>> ds_clear(&actions);
>> ds_put_format(&actions, "eth.dst = %s;
> next;", ea_s);
>> @@ -10026,7 +10093,8 @@ build_lrouter_flows(struct hmap *datapaths,
> struct hmap *ports,
>>
>> if (router_port->lrp_networks.n_ipv4_addrs) {
>> ds_clear(&match);
>> - ds_put_format(&match, "outport == %s && reg0 == ",
>> + ds_put_format(&match, "outport == %s && "
>> + REG_NEXT_HOP_IPV4 " == ",
>> peer->json_key);
>> op_put_v4_networks(&match, router_port, false);
>>
>> @@ -10041,7 +10109,8 @@ build_lrouter_flows(struct hmap *datapaths,
> struct hmap *ports,
>>
>> if (router_port->lrp_networks.n_ipv6_addrs) {
>> ds_clear(&match);
>> - ds_put_format(&match, "outport == %s && xxreg0 == ",
>> + ds_put_format(&match, "outport == %s && "
>> + REG_NEXT_HOP_IPV6 " == ",
>> peer->json_key);
>> op_put_v6_networks(&match, router_port);
>>
>> @@ -10063,10 +10132,10 @@ build_lrouter_flows(struct hmap *datapaths,
> struct hmap *ports,
>> }
>>
>> ovn_lflow_add(lflows, od, S_ROUTER_IN_ARP_RESOLVE, 0, "ip4",
>> - "get_arp(outport, reg0); next;");
>> + "get_arp(outport, " REG_NEXT_HOP_IPV4 "); next;");
>>
>> ovn_lflow_add(lflows, od, S_ROUTER_IN_ARP_RESOLVE, 0, "ip6",
>> - "get_nd(outport, xxreg0); next;");
>> + "get_nd(outport, " REG_NEXT_HOP_IPV6 "); next;");
>> }
>>
>> /* Local router ingress table CHK_PKT_LEN: Check packet length.
>> @@ -10212,7 +10281,8 @@ build_lrouter_flows(struct hmap *datapaths,
> struct hmap *ports,
>>
>> ds_clear(&match);
>> ds_put_format(&match, "eth.dst == 00:00:00:00:00:00 && "
>> - "ip6 && xxreg0 == %s", route->nexthop);
>> + "ip6 && " REG_NEXT_HOP_IPV6 " == %s",
>> + route->nexthop);
>> struct in6_addr sn_addr;
>> struct eth_addr eth_dst;
>> in6_addr_solicited_node(&sn_addr, &gw_ip6);
>> @@ -10240,15 +10310,15 @@ build_lrouter_flows(struct hmap *datapaths,
> struct hmap *ports,
>> "eth.dst == 00:00:00:00:00:00 && ip4",
>> "arp { "
>> "eth.dst = ff:ff:ff:ff:ff:ff; "
>> - "arp.spa = reg1; "
>> - "arp.tpa = reg0; "
>> + "arp.spa = " REG_SRC_IPV4 "; "
>> + "arp.tpa = " REG_NEXT_HOP_IPV4 "; "
>> "arp.op = 1; " /* ARP request */
>> "output; "
>> "};");
>> ovn_lflow_add(lflows, od, S_ROUTER_IN_ARP_REQUEST, 100,
>> "eth.dst == 00:00:00:00:00:00 && ip6",
>> "nd_ns { "
>> - "nd.target = xxreg0; "
>> + "nd.target = " REG_NEXT_HOP_IPV6 "; "
>> "output; "
>> "};");
>> ovn_lflow_add(lflows, od, S_ROUTER_IN_ARP_REQUEST, 0, "1",
> "output;");
>>
>> _______________________________________________
>> dev mailing list
>> dev at openvswitch.org <mailto:dev at openvswitch.org>
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
More information about the dev
mailing list