On 22/09/2020 20:47, Numan Siddique wrote:
On Wed, Sep 16, 2020 at 10:42 PM <anton.iva...@cambridgegreys.com <mailto:anton.iva...@cambridgegreys.com>> wrote: From: Anton Ivanov <anton.iva...@cambridgegreys.com <mailto:anton.iva...@cambridgegreys.com>> This moves various rules which deny local traffic which should not be forwarded such as ND, RA, ARP, etc into a separate function. Signed-off-by: Anton Ivanov <anton.iva...@cambridgegreys.com <mailto:anton.iva...@cambridgegreys.com>> --- northd/ovn-northd.c | 200 +++++++++++++++++++++++--------------------- 1 file changed, 105 insertions(+), 95 deletions(-) diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index db14909fc..32f081d9d 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -8688,6 +8688,12 @@ build_egress_delivery_flows_for_lrouter_port( struct ovn_port *op, struct hmap *lflows, struct ds *match, struct ds *actions); +/* Filter rules for various local traffic which should not be forwarded + * by default */ +static void +build_misc_local_traffic_drop_flows_for_lrouter( + struct ovn_datapath *od, struct hmap *lflows); + static void build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, struct hmap *lflows, struct shash *meter_groups, @@ -8720,101 +8726,7 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, } HMAP_FOR_EACH (od, key_node, datapaths) { - if (!od->nbr) { - continue; - } - - /* L3 admission control: drop multicast and broadcast source, localhost - * source or destination, and zero network source or destination - * (priority 100). */ - ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 100, - "ip4.src_mcast ||" - "ip4.src == 255.255.255.255 || " - "ip4.src == 127.0.0.0/8 <http://127.0.0.0/8> || " - "ip4.dst == 127.0.0.0/8 <http://127.0.0.0/8> || " - "ip4.src == 0.0.0.0/8 <http://0.0.0.0/8> || " - "ip4.dst == 0.0.0.0/8 <http://0.0.0.0/8>", - "drop;"); - - /* Priority-90-92 flows handle ARP requests and ND packets. Most are - * per logical port but DNAT addresses can be handled per datapath - * for non gateway router ports. - */ - struct sset snat_ips = SSET_INITIALIZER(&snat_ips); - for (int i = 0; i < od->nbr->n_nat; i++) { - struct ovn_nat *nat_entry = &od->nat_entries[i]; - const struct nbrec_nat *nat = nat_entry->nb; - - /* Skip entries we failed to parse. */ - if (!nat_entry_is_valid(nat_entry)) { - continue; - } - - struct lport_addresses *ext_addrs = &nat_entry->ext_addrs; - char *ext_addr = nat_entry_is_v6(nat_entry) ? - ext_addrs->ipv6_addrs[0].addr_s : - ext_addrs->ipv4_addrs[0].addr_s; - - if (!strcmp(nat->type, "snat")) { - if (!sset_add(&snat_ips, ext_addr)) { - continue; - } - } - - /* Priority 91 and 92 flows are added for each gateway router - * port to handle the special cases. In case we get the packet - * on a regular port, just reply with the port's ETH address. - */ - if (nat_entry_is_v6(nat_entry)) { - build_lrouter_nd_flow(od, NULL, "nd_na", - ext_addrs->ipv6_addrs[0].addr_s, - ext_addrs->ipv6_addrs[0].sn_addr_s, - REG_INPORT_ETH_ADDR, NULL, false, 90, - &nat->header_, lflows); - } else { - build_lrouter_arp_flow(od, NULL, - ext_addrs->ipv4_addrs[0].addr_s, - REG_INPORT_ETH_ADDR, NULL, false, 90, - &nat->header_, lflows); - } - } - sset_destroy(&snat_ips); Hi Anton, The above for loop which adds logical flows - build_lrouter_arp_flow/build_lrouter_nd_flow do not fall into the newly added function - build_misc_local_traffic_drop_flows_for_lrouter() So I excluded this part of the code from build_misc_local_traffic_drop_flows_for_lrouter() and applied this patch to master. I also applied patch 3 and patch 4 of the series, as they were straightforward to apply. There are some conflicts in patch 2 and it needs a thorough review. Can you please resolve the conflicts and submit another version for the remaining 2 patches ?
Thanks, we are getting there. I will redo them in the next version. IMHO, there is enough "material" now to introduce the parallel processing part with future patches moving things from the existing build_lrouter_flows() and build_lswitch_flows(). I can try that as an RFC. A.
Thanks Numan - - /* Drop ARP packets (priority 85). ARP request packets for router's own - * IPs are handled with priority-90 flows. - * Drop IPv6 ND packets (priority 85). ND NA packets for router's own - * IPs are handled with priority-90 flows. - */ - ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 85, - "arp || nd", "drop;"); - - /* Allow IPv6 multicast traffic that's supposed to reach the - * router pipeline (e.g., router solicitations). - */ - ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 84, "nd_rs || nd_ra", - "next;"); - - /* Drop other reserved multicast. */ - ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 83, - "ip6.mcast_rsvd", "drop;"); - - /* Allow other multicast if relay enabled (priority 82). */ - ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 82, - "ip4.mcast || ip6.mcast", - od->mcast_info.rtr.relay ? "next;" : "drop;"); - - /* Drop Ethernet local broadcast. By definition this traffic should - * not be forwarded.*/ - ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 50, - "eth.bcast", "drop;"); - - /* TTL discard */ - ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 30, - "ip4 && ip.ttl == {0, 1}", "drop;"); - - /* Pass other traffic not already handled to the next table for - * routing. */ - ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 0, "1", "next;"); + build_misc_local_traffic_drop_flows_for_lrouter(od, lflows); } /* Logical router ingress table 3: IP Input for IPv4. */ @@ -11281,6 +11193,104 @@ build_egress_delivery_flows_for_lrouter_port( } +static void +build_misc_local_traffic_drop_flows_for_lrouter( + struct ovn_datapath *od, struct hmap *lflows) +{ + if (od->nbr) { + /* L3 admission control: drop multicast and broadcast source, localhost + * source or destination, and zero network source or destination + * (priority 100). */ + ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 100, + "ip4.src_mcast ||" + "ip4.src == 255.255.255.255 || " + "ip4.src == 127.0.0.0/8 <http://127.0.0.0/8> || " + "ip4.dst == 127.0.0.0/8 <http://127.0.0.0/8> || " + "ip4.src == 0.0.0.0/8 <http://0.0.0.0/8> || " + "ip4.dst == 0.0.0.0/8 <http://0.0.0.0/8>", + "drop;"); + + /* Priority-90-92 flows handle ARP requests and ND packets. Most are + * per logical port but DNAT addresses can be handled per datapath + * for non gateway router ports. + */ + struct sset snat_ips = SSET_INITIALIZER(&snat_ips); + for (int i = 0; i < od->nbr->n_nat; i++) { + struct ovn_nat *nat_entry = &od->nat_entries[i]; + const struct nbrec_nat *nat = nat_entry->nb; + + /* Skip entries we failed to parse. */ + if (!nat_entry_is_valid(nat_entry)) { + continue; + } + + struct lport_addresses *ext_addrs = &nat_entry->ext_addrs; + char *ext_addr = nat_entry_is_v6(nat_entry) ? + ext_addrs->ipv6_addrs[0].addr_s : + ext_addrs->ipv4_addrs[0].addr_s; + + if (!strcmp(nat->type, "snat")) { + if (!sset_add(&snat_ips, ext_addr)) { + continue; + } + } + + /* Priority 91 and 92 flows are added for each gateway router + * port to handle the special cases. In case we get the packet + * on a regular port, just reply with the port's ETH address. + */ + if (nat_entry_is_v6(nat_entry)) { + build_lrouter_nd_flow(od, NULL, "nd_na", + ext_addrs->ipv6_addrs[0].addr_s, + ext_addrs->ipv6_addrs[0].sn_addr_s, + REG_INPORT_ETH_ADDR, NULL, false, 90, + &nat->header_, lflows); + } else { + build_lrouter_arp_flow(od, NULL, + ext_addrs->ipv4_addrs[0].addr_s, + REG_INPORT_ETH_ADDR, NULL, false, 90, + &nat->header_, lflows); + } + } + sset_destroy(&snat_ips); + + /* Drop ARP packets (priority 85). ARP request packets for router's own + * IPs are handled with priority-90 flows. + * Drop IPv6 ND packets (priority 85). ND NA packets for router's own + * IPs are handled with priority-90 flows. + */ + ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 85, + "arp || nd", "drop;"); + + /* Allow IPv6 multicast traffic that's supposed to reach the + * router pipeline (e.g., router solicitations). + */ + ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 84, "nd_rs || nd_ra", + "next;"); + + /* Drop other reserved multicast. */ + ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 83, + "ip6.mcast_rsvd", "drop;"); + + /* Allow other multicast if relay enabled (priority 82). */ + ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 82, + "ip4.mcast || ip6.mcast", + od->mcast_info.rtr.relay ? "next;" : "drop;"); + + /* Drop Ethernet local broadcast. By definition this traffic should + * not be forwarded.*/ + ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 50, + "eth.bcast", "drop;"); + + /* TTL discard */ + ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 30, + "ip4 && ip.ttl == {0, 1}", "drop;"); + + /* Pass other traffic not already handled to the next table for + * routing. */ + ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 0, "1", "next;"); + } +} /* Updates the Logical_Flow and Multicast_Group tables in the OVN_SB database, * constructing their contents based on the OVN_NB database. */ static void-- 2.20.1_______________________________________________ dev mailing list d...@openvswitch.org <mailto:d...@openvswitch.org> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
-- Anton R. Ivanov Cambridgegreys Limited. Registered in England. Company Number 10273661 https://www.cambridgegreys.com/ _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
