[ovs-dev] [PATCH v7 4/4] Replace router internal MAC with gateway MAC for reply packets
Mark Michelson
mmichels at redhat.com
Thu Aug 2 20:47:47 UTC 2018
On 08/01/2018 08:16 AM, vkommadi at redhat.com wrote:
> From: venkata anil <vkommadi at redhat.com>
>
> Previous patches in the series doesn't address issue 1 explained in [1]
> i.e
> 1) removal of router gateway port MAC address on external switches
> after expiring of aging time.
> 2) then external switches unable to learn the gateway MAC as
> reply packets carry router internal port MAC address as source
>
> To fix this, router on gateway node will use router gateway MAC address
> instead of router internal port MAC address as source for reply packets,
> so that external switches can learn gateway MAC address.
> This is done only for reply packets from router gateway to tenant VLAN
> switch ports.
> Later before delivering the packet to the port, ovn-controller will
> replace the gateway MAC with router internal port MAC in table 33.
>
> [1] //mail.openvswitch.org/pipermail/ovs-dev/2018-July/349803.html
>
> Reported-by: Miguel Angel Ajo <majopela at redhat.com>
> Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2018-July/349803.html
> Signed-off-by: Venkata Anil <vkommadi at redhat.com>
> ---
>
> v6->v7:
> * Added this patch
>
>
> ovn/controller/physical.c | 60 ++++++++++++++++++++++++++++++++++++++++++---
> ovn/northd/ovn-northd.8.xml | 10 ++++++++
> ovn/northd/ovn-northd.c | 29 ++++++++++++++++++++++
> ovn/ovn-architecture.7.xml | 4 ++-
> 4 files changed, 99 insertions(+), 4 deletions(-)
>
> diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c
> index f269a1d..1f41f59 100644
> --- a/ovn/controller/physical.c
> +++ b/ovn/controller/physical.c
> @@ -190,7 +190,9 @@ get_zone_ids(const struct sbrec_port_binding *binding,
> static void
> put_local_common_flows(uint32_t dp_key, uint32_t port_key,
> bool nested_container, const struct zone_ids *zone_ids,
> - struct ofpbuf *ofpacts_p, struct hmap *flow_table)
> + struct ofpbuf *ofpacts_p, struct hmap *flow_table,
> + struct local_datapath *ld,
> + const struct hmap *local_datapaths)
> {
> struct match match;
>
> @@ -221,11 +223,63 @@ put_local_common_flows(uint32_t dp_key, uint32_t port_key,
> }
> }
>
> + struct ofpbuf *clone = NULL;
> + clone = ofpbuf_clone(ofpacts_p);
> +
I don't understand the use of the cloned ofpbuf here. You could just
ofpbuf_clear(ofpacts_p) in each iteration of the for loop below and
reuse ofpacts_p where you've used clone below.
> /* Resubmit to table 34. */
> put_resubmit(OFTABLE_CHECK_LOOPBACK, ofpacts_p);
> ofctrl_add_flow(flow_table, OFTABLE_LOCAL_OUTPUT, 100, 0,
> &match, ofpacts_p);
>
> + /* For a reply packet from gateway with VLAN switch port as destination
> + * (excluding localnet_port and external VLAN networks), gateway router
> + * will use gateway MAC address as source MAC instead of router internal
> + * port MAC, so that external switches can learn gateway MAC address.
> + * Here (before packet is given to the port) we replace router gateway
> + * MAC address with router internal port MAC. */
> + if (ld->localnet_port && (port_key != ld->localnet_port->tunnel_key)) {
> + for (int i = 0; i < ld->n_peer_dps; i++) {
> + struct local_datapath *peer_ldp = get_local_datapath(
> + local_datapaths, ld->peer_dps[i]->peer_dp->tunnel_key);
> + const struct sbrec_port_binding *crp;
> + crp = peer_ldp->chassisredirect_port;
> + if (!crp) {
> + continue;
> + }
> +
> + if (strcmp(smap_get(&crp->options, "distributed-port"),
> + ld->peer_dps[i]->peer->logical_port) &&
> + (port_key != ld->peer_dps[i]->patch->tunnel_key)) {
> + for (int j = 0; j < crp->n_mac; j++) {
> + struct lport_addresses laddrs;
> + if (!extract_lsp_addresses(crp->mac[j], &laddrs)) {
> + continue;
> + }
> + match_set_dl_src(&match, laddrs.ea);
> + destroy_lport_addresses(&laddrs);
> + break;
> + }
> + for (int j = 0; j < ld->peer_dps[i]->peer->n_mac; j++) {
> + struct lport_addresses laddrs;
> + uint64_t mac64;
> + if (!extract_lsp_addresses(
> + ld->peer_dps[i]->peer->mac[j], &laddrs)) {
> + continue;
> + }
> + mac64 = eth_addr_to_uint64(laddrs.ea);
> + put_load(mac64,
> + MFF_ETH_SRC, 0, 48, clone);
> + destroy_lport_addresses(&laddrs);
> + break;
> + }
> + put_resubmit(OFTABLE_CHECK_LOOPBACK, clone);
> + ofctrl_add_flow(flow_table, OFTABLE_LOCAL_OUTPUT, 150, 0,
> + &match, clone);
> + }
> + }
> + }
> + ofpbuf_delete(clone);
> +
> /* Table 34, Priority 100.
> * =======================
> *
> @@ -330,7 +384,7 @@ consider_port_binding(struct ovsdb_idl_index *sbrec_chassis_by_name,
>
> struct zone_ids binding_zones = get_zone_ids(binding, ct_zones);
> put_local_common_flows(dp_key, port_key, false, &binding_zones,
> - ofpacts_p, flow_table);
> + ofpacts_p, flow_table, ld, local_datapaths);
>
> match_init_catchall(&match);
> ofpbuf_clear(ofpacts_p);
> @@ -531,7 +585,7 @@ consider_port_binding(struct ovsdb_idl_index *sbrec_chassis_by_name,
>
> struct zone_ids zone_ids = get_zone_ids(binding, ct_zones);
> put_local_common_flows(dp_key, port_key, nested_container, &zone_ids,
> - ofpacts_p, flow_table);
> + ofpacts_p, flow_table, ld, local_datapaths);
>
> /* Table 0, Priority 200, 150 and 100.
> * ==============================
> diff --git a/ovn/northd/ovn-northd.8.xml b/ovn/northd/ovn-northd.8.xml
> index 8fa5272..876c121 100644
> --- a/ovn/northd/ovn-northd.8.xml
> +++ b/ovn/northd/ovn-northd.8.xml
> @@ -2013,6 +2013,16 @@ next;
> </li>
>
> <li>
> + A priority-100 logical flow with match
> + <code>inport == <var>GW</var> &&
> + flags.rcv_from_vlan == 1</code> has actions
> + <code>eth.dst = <var>E</var>; next;</code>, where
> + <var>GW</var> is the logical router distributed gateway
> + port and <var>E</var> is the MAC address of router
> + distributed gateway port.
> + </li>
> +
> + <li>
> For each NAT rule in the OVN Northbound database that can
> be handled in a distributed manner, a priority-100 logical
> flow with match <code>ip4.src == <var>B</var> &&
> diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
> index bcf0b66..d012bb8 100644
> --- a/ovn/northd/ovn-northd.c
> +++ b/ovn/northd/ovn-northd.c
> @@ -4419,6 +4419,15 @@ add_route(struct hmap *lflows, const struct ovn_port *op,
> } else {
> ds_put_format(&actions, "ip%s.dst", is_ipv4 ? "4" : "6");
> }
> +
> + if (op->peer && op->peer->od->localnet_port &&
> + op->od->l3dgw_port && op->od->l3redirect_port &&
> + (op != op->od->l3redirect_port) &&
> + (op != op->od->l3dgw_port)) {
> + ds_put_format(&match, " && is_chassis_resident(%s)",
> + op->od->l3redirect_port->json_key);
> + ds_put_format(&actions, "; flags.rcv_from_vlan = 1");
> + }
> ds_put_format(&actions, "; "
> "%sreg1 = %s; "
> "eth.src = %s; "
> @@ -6131,6 +6140,26 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
> op->lrp_networks.ipv6_addrs[i].network_s,
> op->lrp_networks.ipv6_addrs[i].plen, NULL, NULL);
> }
> +
> + /* For a reply packet from gateway with VLAN switch port as
> + * destination, replace router internal port MAC with router gateway
> + * MAC address, so that external switches can learn gateway MAC
> + * address. Later before delivering the packet to the port,
> + * controller will replace the gateway MAC with router internal port
> + * MAC in table 33. */
> + if (op->od->l3dgw_port && (op == op->od->l3dgw_port) &&
> + op->od->l3redirect_port) {
> + ds_clear(&actions);
> + ds_clear(&match);
> + ds_put_format(&match, "inport == %s", op->json_key);
> + ds_put_format(&match, " && flags.rcv_from_vlan == 1");
> + ds_put_format(&match, " && is_chassis_resident(%s)",
> + op->od->l3redirect_port->json_key);
> + ds_put_format(&actions,
> + "eth.src = %s; next;", op->lrp_networks.ea_s);
> + ovn_lflow_add(lflows, op->od, S_ROUTER_IN_GW_REDIRECT, 100,
> + ds_cstr(&match), ds_cstr(&actions));
> + }
> }
>
> /* Convert the static routes to flows. */
> diff --git a/ovn/ovn-architecture.7.xml b/ovn/ovn-architecture.7.xml
> index ad2101c..0de41d2 100644
> --- a/ovn/ovn-architecture.7.xml
> +++ b/ovn/ovn-architecture.7.xml
> @@ -1067,7 +1067,9 @@
>
> <p>
> Flows in table 33 resemble those in table 32 but for logical ports that
> - reside locally rather than remotely. For unicast logical output ports
> + reside locally rather than remotely. If these are VLAN ports and
> + packet has router gateway port MAC address as source, replace it with
> + router internal port MAC address. For unicast logical output ports
> on the local hypervisor, the actions just resubmit to table 34. For
> multicast output ports that include one or more logical ports on the
> local hypervisor, for each such logical port <var>P</var>, the actions
>
More information about the dev
mailing list