On Wed, Sep 16, 2020 at 10:42 PM <anton.iva...@cambridgegreys.com> wrote:
> From: Anton Ivanov <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> > --- > 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 || " > - "ip4.dst == 127.0.0.0/8 || " > - "ip4.src == 0.0.0.0/8 || " > - "ip4.dst == 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 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 || " > + "ip4.dst == 127.0.0.0/8 || " > + "ip4.src == 0.0.0.0/8 || " > + "ip4.dst == 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 > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev