[ovs-dev] [PATCH ovn] northd: do not centralized traffic for unclaimed virtual ports

[Numan Siddique]

On Fri, Jun 4, 2021 at 12:55 PM Lorenzo Bianconi
<lorenzo.bianconi at redhat.com> wrote:
>
> Add a rule to drop traffic from a distributed NAT if the virtual
> port has not claimed yet becaused otherwise the traffic will be
> centralized misconfiguring the TOR switch.
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1952961
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi at redhat.com>
> ---
>  northd/ovn-northd.c | 23 ++++++++++++++++++-----
>  tests/ovn.at        | 26 ++++++++++++++++++++++++++
>  2 files changed, 44 insertions(+), 5 deletions(-)


Hi Lorenzo,  this would require changes in ovn nb documentation
and the ddlog part is missing.

Thanks
Numan

>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 9652ce252..539b8f8b0 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -11666,6 +11666,7 @@ lrouter_check_nat_entry(struct ovn_datapath *od, const struct nbrec_nat *nat,
>  static void
>  build_lrouter_nat_defrag_and_lb(struct ovn_datapath *od,
>                                  struct hmap *lflows,
> +                                struct hmap *ports,
>                                  struct shash *meter_groups,
>                                  struct hmap *lbs,
>                                  struct ds *match, struct ds *actions)
> @@ -11773,10 +11774,21 @@ build_lrouter_nat_defrag_and_lb(struct ovn_datapath *od,
>              ds_clear(match);
>              ds_clear(actions);
>              ds_put_format(match,
> -                          "ip%s.src == %s && outport == %s && "
> -                          "is_chassis_resident(\"%s\")",
> +                          "ip%s.src == %s && outport == %s",
>                            is_v6 ? "6" : "4", nat->logical_ip,
> -                          od->l3dgw_port->json_key, nat->logical_port);
> +                          od->l3dgw_port->json_key);
> +            /* Add a rule to drop traffic from a distributed NAT if
> +             * the virtual port has not claimed yet becaused otherwise
> +             * the traffic will be centralized misconfiguring the TOR switch.
> +             */
> +            struct ovn_port *op = ovn_port_find(ports, nat->logical_port);
> +            if (op && op->nbsp && !strcmp(op->nbsp->type, "virtual")) {
> +                ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_GW_REDIRECT,
> +                                        80, ds_cstr(match), "drop;",
> +                                        &nat->header_);
> +            }
> +            ds_put_format(match, " && is_chassis_resident(\"%s\")",
> +                          nat->logical_port);
>              ds_put_format(actions, "eth.src = %s; %s = %s; next;",
>                            nat->external_mac,
>                            is_v6 ? REG_SRC_IPV6 : REG_SRC_IPV4,
> @@ -11935,8 +11947,9 @@ build_lswitch_and_lrouter_iterate_by_od(struct ovn_datapath *od,
>                                          &lsi->actions);
>      build_misc_local_traffic_drop_flows_for_lrouter(od, lsi->lflows);
>      build_lrouter_arp_nd_for_datapath(od, lsi->lflows);
> -    build_lrouter_nat_defrag_and_lb(od, lsi->lflows, lsi->meter_groups,
> -                                    lsi->lbs, &lsi->match, &lsi->actions);
> +    build_lrouter_nat_defrag_and_lb(od, lsi->lflows, lsi->ports,
> +                                    lsi->meter_groups, lsi->lbs, &lsi->match,
> +                                    &lsi->actions);
>  }
>
>  /* Helper function to combine all lflow generation which is iterated by port.
> diff --git a/tests/ovn.at b/tests/ovn.at
> index f26894ce4..7731c915e 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -17165,6 +17165,16 @@ send_arp_reply() {
>      as hv$hv ovs-appctl netdev-dummy/receive hv${hv}-vif$inport $request
>  }
>
> +send_icmp_packet() {
> +    local inport=$1 hv=$2 eth_src=$3 eth_dst=$4 ipv4_src=$5 ipv4_dst=$6 ip_chksum=$7 data=$8
> +    shift 8
> +
> +    local ip_ttl=ff
> +    local ip_len=001c
> +    local packet=${eth_dst}${eth_src}08004500${ip_len}00004000${ip_ttl}01${ip_chksum}${ipv4_src}${ipv4_dst}${data}
> +    as hv$hv ovs-appctl netdev-dummy/receive hv${hv}-vif$inport $packet
> +}
> +
>  net_add n1
>
>  sim_add hv1
> @@ -17377,6 +17387,22 @@ logical_port=sw0-vir) = x])
>  wait_row_count nb:Logical_Switch_Port 1 up=false name=sw0-vir
>
>  check ovn-nbctl --wait=hv sync
> +
> +# verify the traffic from virtual port is discarded if the port is not claimed
> +AT_CHECK([grep lr_in_gw_redirect lr0-flows2 | grep "ip4.src == 10.0.0.10"], [0], [dnl
> +  table=17(lr_in_gw_redirect  ), priority=100  , match=(ip4.src == 10.0.0.10 && outport == "lr0-public" && is_chassis_resident("sw0-vir")), action=(eth.src = 10:54:00:00:00:10; reg1 = 172.168.0.50; next;)
> +  table=17(lr_in_gw_redirect  ), priority=80   , match=(ip4.src == 10.0.0.10 && outport == "lr0-public"), action=(drop;)
> +])
> +
> +eth_src=505400000003
> +eth_dst=00000000ff01
> +ip_src=$(ip_to_hex 10 0 0 10)
> +ip_dst=$(ip_to_hex 172 168 0 101)
> +send_icmp_packet 1 1 $eth_src $eth_dst $ip_src $ip_dst c4c9 0000000000000000000000
> +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int | awk '/table=25, n_packets=1, n_bytes=45/{print $7" "$8}'],[0],[dnl
> +priority=80,ip,reg15=0x3,metadata=0x3,nw_src=10.0.0.10 actions=drop
> +])
> +
>  # hv1 should remove the flow for the ACL with is_chassis_redirect check for sw0-vir.
>  check_virtual_offlows_not_present hv1
>
> --
> 2.31.1
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>