[ovs-dev] [PATCH v3 ovn] northd: do not centralized traffic for unclaimed virtual ports
Mark Michelson
mmichels at redhat.com
Thu Jul 22 20:54:05 UTC 2021
Hi Lorenzo,
In the commit message:
s/centralized/centralize/
There's no reason for you to upload a new version of the patch with this
change. Whoever merges this can edit the commit message.
Acked-by: Mark Michelson <mmichels at redhat.com>
On 7/22/21 2:08 PM, Lorenzo Bianconi 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
>
> Acked-by: Dumitru Ceara <dceara at redhat.com>
> Co-authored-by: Numan Siddique <numans at ovn.org>
> Signed-off-by: Numan Siddique <numans at ovn.org>
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi at redhat.com>
> ---
> Changes since v2:
> - rebase on top of ovn master
>
> Changes since v1:
> - add northd documentation
> - add DDlog support (numan)
> ---
> northd/ovn-northd.8.xml | 7 +++++++
> northd/ovn-northd.c | 22 +++++++++++++++++-----
> northd/ovn_northd.dl | 15 +++++++++++++++
> tests/ovn.at | 26 ++++++++++++++++++++++++++
> 4 files changed, 65 insertions(+), 5 deletions(-)
>
> diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> index 6599ba194..99a19f853 100644
> --- a/northd/ovn-northd.8.xml
> +++ b/northd/ovn-northd.8.xml
> @@ -3784,6 +3784,13 @@ icmp6 {
> external ip and <var>D</var> is NAT external mac.
> </li>
>
> + <li>
> + For each NAT rule in the OVN Northbound database that can
> + be handled in a distributed manner, a priority-80 logical flow
> + with drop action if the NAT logical port is a virtual port not
> + claimed by any chassis yet.
> + </li>
> +
> <li>
> A priority-50 logical flow with match
> <code>outport == <var>GW</var></code> has actions
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index af3e0bf87..1058c1c26 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -12045,7 +12045,8 @@ lrouter_check_nat_entry(struct ovn_datapath *od, const struct nbrec_nat *nat,
> /* NAT, Defrag and load balancing. */
> static void
> build_lrouter_nat_defrag_and_lb(struct ovn_datapath *od, struct hmap *lflows,
> - struct ds *match, struct ds *actions)
> + struct hmap *ports, struct ds *match,
> + struct ds *actions)
> {
> if (!od->nbr) {
> return;
> @@ -12168,10 +12169,21 @@ build_lrouter_nat_defrag_and_lb(struct ovn_datapath *od, struct hmap *lflows,
> 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,
> @@ -12309,7 +12321,7 @@ 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->match,
> + build_lrouter_nat_defrag_and_lb(od, lsi->lflows, lsi->ports, &lsi->match,
> &lsi->actions);
> }
>
> diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl
> index bae34a6a1..ab33a139e 100644
> --- a/northd/ovn_northd.dl
> +++ b/northd/ovn_northd.dl
> @@ -5807,6 +5807,10 @@ for (rp in &RouterPort(.router = &Router{._uuid = lr_uuid, .options = lr_options
> }
> }
>
> +relation VirtualLogicalPort(logical_port: Option<string>)
> +VirtualLogicalPort(Some{logical_port}) :-
> + lsp in &nb::Logical_Switch_Port(.name = logical_port, .__type = "virtual").
> +
> /* NAT rules are only valid on Gateway routers and routers with
> * l3dgw_port (router has a port with "redirect-chassis"
> * specified). */
> @@ -6151,6 +6155,17 @@ for (r in &Router(._uuid = lr_uuid,
> .actions = actions,
> .external_ids = stage_hint(nat.nat._uuid));
>
> + for (VirtualLogicalPort(nat.nat.logical_port)) {
> + Some{var gwport} = l3dgw_port in
> + Flow(.logical_datapath = lr_uuid,
> + .stage = s_ROUTER_IN_GW_REDIRECT(),
> + .priority = 80,
> + .__match = "${ipX}.src == ${nat.nat.logical_ip} && "
> + "outport == ${json_string_escape(gwport.name)}",
> + .actions = "drop;",
> + .external_ids = stage_hint(nat.nat._uuid))
> + };
> +
> /* Egress Loopback table: For NAT on a distributed router.
> * If packets in the egress pipeline on the distributed
> * gateway port have ip.dst matching a NAT external IP, then
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 777409144..eaf344be9 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -17397,6 +17397,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
> @@ -17611,6 +17621,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
>
>
More information about the dev
mailing list