> On 29/04/2021 18:04, Lorenzo Bianconi wrote: > > From: Dumitru Ceara <[email protected]> > > > > Change the ovn-northd implementation to set the new 'controller_meter' > > field for flows that need to punt packets to ovn-controller. > > > > Protocol packets for which CoPP is enforced when sending packets to > > ovn-controller (if configured): > > - ARP > > - ND_NS > > - ND_NA > > - ND_RA > > - DNS > > - IGMP > > - packets that require ARP resolution before forwarding > > - packets that require ND_NS before forwarding > > - packets that need to be replied to with ICMP Errors > > - packets that need to be replied to with TCP RST > > - packets that need to be replied to with DHCP_OPTS > > - BFD > > > Add reject? > > Do you need to add event-elb?
ack, I will fix it in v2 > > > Co-authored-by: Lorenzo Bianconi <[email protected]> > > Signed-off-by: Lorenzo Bianconi <[email protected]> > > Signed-off-by: Dumitru Ceara <[email protected]> > > --- > > lib/copp.c | 1 + > > lib/copp.h | 1 + > > northd/ovn-northd.c | 451 ++++++++++++++++++++++++-------------- > > ovn-nb.xml | 3 + > > tests/atlocal.in | 3 + > > tests/system-ovn.at | 119 ++++++++++ > > utilities/ovn-nbctl.8.xml | 1 + > > 7 files changed, 417 insertions(+), 162 deletions(-) > > > > diff --git a/lib/copp.c b/lib/copp.c > > index ac53a1094..7713046e5 100644 > > --- a/lib/copp.c > > +++ b/lib/copp.c > > @@ -37,6 +37,7 @@ static char *copp_proto_names[COPP_PROTO_MAX] = { > > [COPP_ND_NS_RESOLVE] = "nd-ns-resolve", > > [COPP_ND_RA_OPTS] = "nd-ra-opts", > > [COPP_TCP_RESET] = "tcp-reset", > > + [COPP_REJECT] = "reject", > > [COPP_BFD] = "bfd", > > }; > > [...] > > Why do you add the same flow twice but one with a meter instead of > modifying the flow above? it is a bug :) thx for spotting it. I will fix in v2 > > > const char *dns_action = "eth.dst <-> eth.src; ip4.src <-> > > ip4.dst; " > > "udp.dst = udp.src; udp.src = 53; outport = inport; " > > "flags.loopback = 1; output;"; > > @@ -7244,7 +7280,8 @@ build_lswitch_external_port(struct ovn_port *op, > > static void > > build_lswitch_destination_lookup_bmcast(struct ovn_datapath *od, > > struct hmap *lflows, > > - struct ds *actions) > > + struct ds *actions, > > + struct shash *meter_groups) > > { > > if (od->nbs) { > > > > @@ -7265,12 +7302,16 @@ build_lswitch_destination_lookup_bmcast(struct > > ovn_datapath *od, > > } > > ds_put_cstr(actions, "igmp;"); > > /* Punt IGMP traffic to controller. */ > > - ovn_lflow_add_unique(lflows, od, S_SWITCH_IN_L2_LKUP, 100, > > - "ip4 && ip.proto == 2", ds_cstr(actions)); > > + ovn_lflow_add_unique__(lflows, od, S_SWITCH_IN_L2_LKUP, 100, > > + "ip4 && ip.proto == 2", > > ds_cstr(actions), > > + copp_meter_get(COPP_IGMP, od->nbs->copp, > > + meter_groups)); > > > > /* Punt MLD traffic to controller. */ > > - ovn_lflow_add_unique(lflows, od, S_SWITCH_IN_L2_LKUP, 100, > > - "mldv1 || mldv2", ds_cstr(actions)); > > + ovn_lflow_add_unique__(lflows, od, S_SWITCH_IN_L2_LKUP, 100, > > + "mldv1 || mldv2", ds_cstr(actions), > > + copp_meter_get(COPP_IGMP, od->nbs->copp, > > + meter_groups)); > > > > /* Flood all IP multicast traffic destined to 224.0.0.X to all > > * ports - RFC 4541, section 2.1.2, item 2. > > @@ -8650,23 +8691,29 @@ add_router_lb_flow(struct hmap *lflows, struct > > ovn_datapath *od, > > struct ds *match, struct ds *actions, int priority, > > enum lb_snat_type snat_type, struct ovn_lb_vip *lb_vip, > > const char *proto, struct nbrec_load_balancer *lb, > > - struct shash *meter_groups, struct sset *nat_entries) > > + struct shash *meter_groups, struct sset *nat_entries, > > + bool reject) > > { > > build_empty_lb_event_flow(od, lflows, lb_vip, lb, S_ROUTER_IN_DNAT, > > meter_groups); > > > > + const char *meter = NULL; > > + if (reject) { > > + meter = copp_meter_get(COPP_REJECT, od->nbr->copp, meter_groups); > > + } > > /* A match and actions for new connections. */ > > char *new_match = xasprintf("ct.new && %s", ds_cstr(match)); > > if (snat_type == FORCE_SNAT || snat_type == SKIP_SNAT) { > > char *new_actions = xasprintf("flags.%s_snat_for_lb = 1; %s", > > snat_type == SKIP_SNAT ? "skip" : "force", > > ds_cstr(actions)); > > - ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_DNAT, priority, > > - new_match, new_actions, &lb->header_); > > + ovn_lflow_add_with_hint__(lflows, od, S_ROUTER_IN_DNAT, priority, > > + new_match, new_actions, meter, > > &lb->header_); > > free(new_actions); > > } else { > > - ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_DNAT, priority, > > - new_match, ds_cstr(actions), &lb->header_); > > + ovn_lflow_add_with_hint__(lflows, od, S_ROUTER_IN_DNAT, priority, > > + new_match, ds_cstr(actions), meter, > > + &lb->header_); > > } > > > > /* A match and actions for established connections. */ > > @@ -8792,8 +8839,8 @@ build_lrouter_lb_flows(struct hmap *lflows, struct > > ovn_datapath *od, > > struct ovn_lb_vip *lb_vip = &lb->vips[j]; > > struct ovn_northd_lb_vip *lb_vip_nb = &lb->vips_nb[j]; > > ds_clear(actions); > > - build_lb_vip_actions(lb_vip, lb_vip_nb, actions, > > - lb->selection_fields, false); > > + bool reject = build_lb_vip_actions(lb_vip, lb_vip_nb, actions, > > + lb->selection_fields, > > false); > > > > if (!sset_contains(&all_ips, lb_vip->vip_str)) { > > sset_add(&all_ips, lb_vip->vip_str); > > @@ -8858,7 +8905,7 @@ build_lrouter_lb_flows(struct hmap *lflows, struct > > ovn_datapath *od, > > } > > add_router_lb_flow(lflows, od, match, actions, prio, > > snat_type, lb_vip, proto, nb_lb, > > - meter_groups, nat_entries); > > + meter_groups, nat_entries, reject); > > } > > } > > sset_destroy(&all_ips); > > @@ -9097,7 +9144,7 @@ build_lrouter_nd_flow(struct ovn_datapath *od, struct > > ovn_port *op, > > const char *sn_ip_address, const char *eth_addr, > > struct ds *extra_match, bool drop, uint16_t priority, > > const struct ovsdb_idl_row *hint, > > - struct hmap *lflows) > > + struct hmap *lflows, struct shash *meter_groups) > > { > > struct ds match = DS_EMPTY_INITIALIZER; > > struct ds actions = DS_EMPTY_INITIALIZER; > > @@ -9119,6 +9166,8 @@ build_lrouter_nd_flow(struct ovn_datapath *od, struct > > ovn_port *op, > > > > if (drop) { > > ds_put_format(&actions, "drop;"); > > + ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_IP_INPUT, priority, > > + ds_cstr(&match), ds_cstr(&actions), hint); > > } else { > > ds_put_format(&actions, > > "%s { " > > @@ -9135,11 +9184,13 @@ build_lrouter_nd_flow(struct ovn_datapath *od, > > struct ovn_port *op, > > ip_address, > > ip_address, > > eth_addr); > > + ovn_lflow_add_with_hint__(lflows, od, S_ROUTER_IN_IP_INPUT, > > priority, > > + ds_cstr(&match), ds_cstr(&actions), > > + copp_meter_get(COPP_ND_NA, od->nbr->copp, > > + meter_groups), > > + hint); > > } > > > > - ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_IP_INPUT, priority, > > - ds_cstr(&match), ds_cstr(&actions), hint); > > - > > ds_destroy(&match); > > ds_destroy(&actions); > > } > > @@ -9147,7 +9198,8 @@ build_lrouter_nd_flow(struct ovn_datapath *od, struct > > ovn_port *op, > > static void > > build_lrouter_nat_arp_nd_flow(struct ovn_datapath *od, > > struct ovn_nat *nat_entry, > > - struct hmap *lflows) > > + struct hmap *lflows, > > + struct shash *meter_groups) > > { [...] > > +ADD_NAMESPACES(server) > > +NS_CHECK_EXEC([server], [ip link set dev lo up]) > > +ADD_VETH(s1, server, br-ext, "172.16.1.50/24", "f0:00:00:01:02:05", \ > > + "172.16.1.1") > > +NS_CHECK_EXEC([server], [ip addr add 1000::b/64 dev s1]) > > Why do you add this IPv6 address here? it is not necessary, I will remove it. > > > + > > +AT_CHECK([ovs-vsctl set Open_vSwitch . > > external-ids:ovn-bridge-mappings=phynet:br-ext]) > > +check ovn-nbctl lsp-add public public1 \ > > + -- lsp-set-addresses public1 unknown \ > > + -- lsp-set-type public1 localnet \ > > + -- lsp-set-options public1 network_name=phynet > > + > > +NS_CHECK_EXEC([sw01], [tcpdump -nni sw01 icmp -Q in > reject.pcap &]) > > +check ovn-nbctl meter-add acl-meter drop 1 pktps 0 > > +check ovn-nbctl --wait=hv ls-copp-add sw0 reject acl-meter > > +check ovn-nbctl acl-add sw0 from-lport 1002 'inport == "sw01" && ip && > > udp' reject > > + > > +AT_CHECK([ovn-nbctl ls-copp-list sw0], [0], [dnl > > +reject: acl-meter > > +]) > > + > > +ip netns exec sw01 scapy -H <<-EOF > > +p = IP(src="192.168.1.2", dst="192.168.1.1")/ UDP(dport = 12345) / > > Raw(b"X"*64) > > +send (p, iface='sw01', loop = 0, verbose = 0, count = 20) > > +EOF > > + > > +sleep 2> +kill $(pidof tcpdump) > > + > > +# 1pps + 1 burst size > > +OVS_WAIT_UNTIL([ > > + n_reject=$(grep unreachable reject.pcap | wc -l) > > + test "${n_reject}" = "2" > > +]) > > + > > +NS_CHECK_EXEC([server], [tcpdump -nni s1 arp[[24:4]]=0xac100164 > arp.pcap > > &]) > > +check ovn-nbctl meter-add arp-meter drop 1 pktps 0 > > +check ovn-nbctl --wait=hv lr-copp-add R1 arp-resolve arp-meter > > +AT_CHECK([ovn-nbctl lr-copp-list R1], [0], [dnl > > +arp-resolve: arp-meter > > +]) > > + > > +ip netns exec sw01 scapy -H <<-EOF > > +p = IP(src="192.168.1.2", dst="172.16.1.100")/ TCP(dport = 80, flags="S") > > / Raw(b"X"*64) > > +send (p, iface='sw01', loop = 0, verbose = 0, count = 100) > > +EOF > > + > > +sleep 2 > > +kill $(pidof tcpdump) > > + > > +# 1pps + 1 burst size > > +OVS_WAIT_UNTIL([ > > + n_arp=$(grep ARP arp.pcap | wc -l) > > + test "${n_arp}" = "2" > > +]) > > + > > +kill $(pidof ovn-controller) > > + > > +as ovn-sb > > +OVS_APP_EXIT_AND_WAIT([ovsdb-server]) > > + > > +as ovn-nb > > +OVS_APP_EXIT_AND_WAIT([ovsdb-server]) > > + > > +as northd > > +OVS_APP_EXIT_AND_WAIT([ovn-northd]) > > + > > +as > > +OVS_TRAFFIC_VSWITCHD_STOP(["/.*error receiving.*/d > > +/.*terminating with signal 15.*/d"]) > > +AT_CLEANUP > > Did you test this with the meters disabled? Does it behave as expected? yes, but I will cover it in the test case. Regards, Lorenzo > > > +]) > > diff --git a/utilities/ovn-nbctl.8.xml b/utilities/ovn-nbctl.8.xml > > index 6c95e8104..5f5f71015 100644 > > --- a/utilities/ovn-nbctl.8.xml > > +++ b/utilities/ovn-nbctl.8.xml > > @@ -1151,6 +1151,7 @@ > > <li>packets that need to be replied to with ICMP Errors</li> > > <li>packets that need to be replied to with TCP RST</li> > > <li>packets that need to be replied to with DHCP_OPTS</li> > > + <li>packets that trigger a reject action</li> > > <li>BFD</li> > > </ul> > > </p> > > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
