On Mon, Mar 24, 2025 at 9:42 AM Aditya Mehakare <[email protected]> wrote:
> This commit adds COPP support for DHCPv4 Relay. > > Acked-by: Naveen Yerramneni <[email protected]> > Signed-off-by: Aditya Mehakare <[email protected]> > --- > v2: Handle intermittent test failure. > --- > Hi Aditya, thank you for the patch, I have some minor comments down below. lib/copp.c | 1 + > lib/copp.h | 1 + > northd/northd.c | 20 +++++--- > ovn-nb.xml | 4 ++ > tests/ovn-northd.at | 2 +- > tests/ovn.at | 120 ++++++++++++++++++++++++++++++++++++++++++++ > 6 files changed, 141 insertions(+), 7 deletions(-) > > diff --git a/lib/copp.c b/lib/copp.c > index 11dd9029d..8ba9ab7f4 100644 > --- a/lib/copp.c > +++ b/lib/copp.c > @@ -40,6 +40,7 @@ static char *copp_proto_names[COPP_PROTO_MAX] = { > [COPP_REJECT] = "reject", > [COPP_SVC_MONITOR] = "svc-monitor", > [COPP_BFD] = "bfd", > + [COPP_DHCPV4_RELAY] = "dhcpv4-relay", > }; > > static const char * > diff --git a/lib/copp.h b/lib/copp.h > index b99737220..3a7d1ccc5 100644 > --- a/lib/copp.h > +++ b/lib/copp.h > @@ -38,6 +38,7 @@ enum copp_proto { > COPP_BFD, > COPP_REJECT, > COPP_SVC_MONITOR, > + COPP_DHCPV4_RELAY, > COPP_PROTO_MAX, > COPP_PROTO_INVALID = COPP_PROTO_MAX, > }; > diff --git a/northd/northd.c b/northd/northd.c > index 1d3e132d4..19cddd6c6 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -15098,7 +15098,8 @@ static void > build_dhcp_relay_flows_for_lrouter_port(struct ovn_port *op, > struct lflow_table *lflows, > struct ds *match, struct ds > *actions, > - struct lflow_ref *lflow_ref) > + struct lflow_ref *lflow_ref, > + const struct shash *meter_groups) > { > if (!op->nbrp || !op->nbrp->dhcp_relay || > !op->lrp_networks.n_ipv4_addrs) { > return; > @@ -15147,8 +15148,11 @@ build_dhcp_relay_flows_for_lrouter_port(struct > ovn_port *op, > "next; /* DHCP_RELAY_REQ */", > op->lrp_networks.ipv4_addrs[0].addr_s, server_ip_str); > > - ovn_lflow_add_with_hint(lflows, op->od, S_ROUTER_IN_IP_INPUT, 110, > - ds_cstr(match), ds_cstr(actions), > + ovn_lflow_add_with_hint__(lflows, op->od, S_ROUTER_IN_IP_INPUT, 110, > + ds_cstr(match), ds_cstr(actions), NULL, > + copp_meter_get(COPP_DHCPV4_RELAY, > + op->od->nbr->copp, > + meter_groups), > nit: The arguments are not properly aligned. &op->nbrp->header_, lflow_ref); > > ds_clear(match); > @@ -15209,9 +15213,12 @@ build_dhcp_relay_flows_for_lrouter_port(struct > ovn_port *op, > " = dhcp_relay_resp_chk(%s, %s); next; /* DHCP_RELAY_RESP */", > op->lrp_networks.ipv4_addrs[0].addr_s, server_ip_str); > > - ovn_lflow_add_with_hint(lflows, op->od, > S_ROUTER_IN_DHCP_RELAY_RESP_CHK, > + ovn_lflow_add_with_hint__(lflows, op->od, > S_ROUTER_IN_DHCP_RELAY_RESP_CHK, > 100, > - ds_cstr(match), ds_cstr(actions), > + ds_cstr(match), ds_cstr(actions), NULL, > + copp_meter_get(COPP_DHCPV4_RELAY, > + op->od->nbr->copp, > + meter_groups), > nit: Same here. &op->nbrp->header_, lflow_ref); > > > @@ -17394,7 +17401,8 @@ build_lswitch_and_lrouter_iterate_by_lrp(struct > ovn_port *op, > build_dhcpv6_reply_flows_for_lrouter_port(op, lsi->lflows, > &lsi->match, > op->lflow_ref); > build_dhcp_relay_flows_for_lrouter_port(op, lsi->lflows, &lsi->match, > - &lsi->actions, op->lflow_ref); > + &lsi->actions, op->lflow_ref, > + lsi->meter_groups); > build_ipv6_input_flows_for_lrouter_port(op, lsi->lflows, > &lsi->match, &lsi->actions, > lsi->meter_groups, > diff --git a/ovn-nb.xml b/ovn-nb.xml > index ff5f2f249..b72be9e59 100644 > --- a/ovn-nb.xml > +++ b/ovn-nb.xml > @@ -640,6 +640,10 @@ > Rate limiting meter for packets that are arriving to service > monitor MAC address. > </column> > + <column name="meters" key="dhcpv4-relay"> > + Rate limiting meter for DHCPv4 relay packets (request/response) when > + DHCPv4 Relay functionality is enabled. > + </column> > <column name="external_ids"> > See <em>External IDs</em> at the beginning of this document. > </column> > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index 419130aa8..55605c443 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -4203,7 +4203,7 @@ AT_CHECK([ovn-sbctl list logical_flow | grep > trigger_event -A 2 | grep -q meter0 > > # let's try to add an usupported protocol "dhcp" > AT_CHECK([ovn-nbctl --wait=hv copp-add copp5 dhcp meter1],[1],[],[dnl > -ovn-nbctl: Invalid control protocol. Allowed values: arp, arp-resolve, > dhcpv4-opts, dhcpv6-opts, dns, event-elb, icmp4-error, icmp6-error, igmp, > nd-na, nd-ns, nd-ns-resolve, nd-ra-opts, tcp-reset, bfd, reject, > svc-monitor. > +ovn-nbctl: Invalid control protocol. Allowed values: arp, arp-resolve, > dhcpv4-opts, dhcpv6-opts, dns, event-elb, icmp4-error, icmp6-error, igmp, > nd-na, nd-ns, nd-ns-resolve, nd-ra-opts, tcp-reset, bfd, reject, > svc-monitor, dhcpv4-relay. > ]) > > #Let's try to add a valid protocol to an unknown datapath > diff --git a/tests/ovn.at b/tests/ovn.at > index 714286596..235f324ed 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -40171,6 +40171,126 @@ n_packets=3 > > sid=`ip_to_hex 172.16.1.1` > > +# ==================================================== > +# Test Copp for DHCP request/response packets > + > +AT_CHECK([ovn-nbctl --wait=hv meter-add dhcp_relay_meter drop 10 pktps 5]) > +AT_CHECK([ovn-nbctl --wait=hv copp-add copp_dhcp_relay dhcpv4-relay > dhcp_relay_meter]) > +AT_CHECK([ovn-nbctl --wait=hv lr-copp-add copp_dhcp_relay lr0]) > nit: All of those could be replaced with "check". + > +AT_CHECK([as hv1 ovs-appctl -t ovn-controller meter-table-list], [0], [dnl > +dhcp_relay_meter: 1 > +]) > + > +# Print some information that may help debugging. > +ovs-ofctl dump-flows br-int > pflows_dhcp_relay_copp > +ovn-sbctl find logical_flow controller_meter=dhcp_relay_meter > > lflows_dhcp_relay_copp > + > +## Verify packets within defined limit are not dropped. > + > +# Send DHCP discovery. > +src_mac="505400000010" > +src_ip=`ip_to_hex 0.0.0.0` > +dst_mac="ffffffffffff" > +dst_ip=`ip_to_hex 255.255.255.255` > +yiaddr=`ip_to_hex 0.0.0.0` > +giaddr=`ip_to_hex 0.0.0.0` > +sid=$src_ip > This is a great opportunity to use scapy for the send_dhcp_packet. + > +for i in {1..3} > +do > + send_dhcp_packet $src_mac $src_ip $dst_mac $dst_ip 01 01 $yiaddr > $giaddr $sid vif0 > +done > + > +echo "Meter stats:" > +as hv1 ovs-ofctl -O OpenFlow15 meter-stats br-int > +meter_stat=$(as hv1 ovs-ofctl -O OpenFlow15 meter-stats br-int) > +drop_cnt=$(echo $meter_stat | tail -n1 | sed -n > 's/.*packet_count:\([[0-9]]\+\).*/\1/p') > +AT_CHECK([ test $drop_cnt -eq 0 ], [0], []) > nit: Extra space around the test, also there is no need for the second and third argument. + > +# Send DHCP offer. > +src_mac="50540000001f" > +src_ip=`ip_to_hex 172.16.1.1` > +dst_mac="505400000002" > +dst_ip=`ip_to_hex 192.168.1.1` > +yiaddr=`ip_to_hex 192.168.1.10` > +giaddr=`ip_to_hex 192.168.1.1` > +sid=$src_ip > + > +sleep 1 > +for i in {1..3} > +do > + send_dhcp_packet $src_mac $src_ip $dst_mac $dst_ip 02 02 $yiaddr > $giaddr $sid ext0 > +done > + > +echo "Meter stats:" > +as hv1 ovs-ofctl -O OpenFlow15 meter-stats br-int > +meter_stat=$(as hv1 ovs-ofctl -O OpenFlow15 meter-stats br-int) > +drop_cnt=$(echo $meter_stat | tail -n1 | sed -n > 's/.*packet_count:\([[0-9]]\+\).*/\1/p') > +AT_CHECK([ test $drop_cnt -eq 0 ], [0], []) nit: Same here. > + > +## Verify packets exceeding rate limiting are dropped. > + > +# Send DHCP discovery. > +src_mac="505400000010" > +src_ip=`ip_to_hex 0.0.0.0` > +dst_mac="ffffffffffff" > +dst_ip=`ip_to_hex 255.255.255.255` > +yiaddr=`ip_to_hex 0.0.0.0` > +giaddr=`ip_to_hex 0.0.0.0` > +sid=$src_ip > + > +sleep 1 > +start_time=$(date +%s%3N) > +for i in {1..15} > +do > + send_dhcp_packet $src_mac $src_ip $dst_mac $dst_ip 01 01 $yiaddr > $giaddr $sid vif0 > +done > +end_time=$(date +%s%3N) > + > +# On particularly slow or overloaded systems, the transmission rate may > +# be lower than the configured meter rate. To prevent false test > +# failures, we check the transmission duration, and if it's > +# greater than expected time, just skip the test. > +d_ms=$(expr $end_time - $start_time) > +echo "Discover transmission duration ms: $d_ms" > +AT_SKIP_IF([test $d_ms -gt 1000]) > + > +echo "Meter stats:" > +as hv1 ovs-ofctl -O OpenFlow15 meter-stats br-int > +meter_stat=$(as hv1 ovs-ofctl -O OpenFlow15 meter-stats br-int) > +drop_cnt=$(echo $meter_stat | tail -n1 | sed -n > 's/.*packet_count:\([[0-9]]\+\).*/\1/p') > +AT_CHECK([ test $drop_cnt -gt 1 ], [0], []) > nit: Same here. > + > +# Send DHCP offer. > +src_mac="50540000001f" > +src_ip=`ip_to_hex 172.16.1.1` > +dst_mac="505400000002" > +dst_ip=`ip_to_hex 192.168.1.1` > +yiaddr=`ip_to_hex 192.168.1.10` > +giaddr=`ip_to_hex 192.168.1.1` > +sid=$src_ip > + > +sleep 1 > +start_time=$(date +%s%3N) > +for i in {1..15} > +do > + send_dhcp_packet $src_mac $src_ip $dst_mac $dst_ip 02 02 $yiaddr > $giaddr $sid ext0 > +done > +end_time=$(date +%s%3N) > + > +d_ms=$(expr $end_time - $start_time) > +echo "Offer transmission duration ms: $d_ms" > +AT_SKIP_IF([test $d_ms -gt 1000]) > + > +echo "Meter stats:" > +as hv1 ovs-ofctl -O OpenFlow15 meter-stats br-int > +prev_cnt=$drop_cnt > +meter_stat=$(as hv1 ovs-ofctl -O OpenFlow15 meter-stats br-int) > +drop_cnt=$(echo $meter_stat | tail -n1 | sed -n > 's/.*packet_count:\([[0-9]]\+\).*/\1/p') > +drop_delta=$(expr $drop_cnt - $prev_cnt) > +AT_CHECK([ test $drop_delta -gt 1 ], [0], []) > nit: Same here. > + > OVN_CLEANUP([hv1 > /WARN\|DHCP_RELAY/d > ]) > -- > 2.43.5 > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > Thanks, Ales _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
