[ovs-dev] [PATCH] vswitchd: Ingress policing to use matchall instead of basic
Michael Pattrick
mkp at redhat.com
Thu Oct 14 19:44:24 UTC 2021
Currently ingress policing uses the basic classifier to apply traffic
control filters.
However, cls_basic was removed from the upcoming RHEL9. Basic is probably
not the proper classifier to use when the more accurage matchall
classifier is available and already in use in the code base. Switching
from basic to matchall allows us to remove a function.
I've also modified tests to detect when ingress policing fails to apply.
Signed-off-by: Michael Pattrick <mkp at redhat.com>
---
lib/netdev-linux.c | 85 +++++++++-------------------------------------
tests/ovs-vsctl.at | 24 ++++++-------
2 files changed, 28 insertions(+), 81 deletions(-)
diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index 97bd21be4..d64b88512 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -486,10 +486,6 @@ static struct tcmsg *netdev_linux_tc_make_request(const struct netdev *,
unsigned int flags,
struct ofpbuf *);
-static int tc_add_policer(struct netdev *, uint32_t kbits_rate,
- uint32_t kbits_burst, uint32_t kpkts_rate,
- uint32_t kpkts_burst);
-
static int tc_parse_qdisc(const struct ofpbuf *, const char **kind,
struct nlattr **options);
static int tc_parse_class(const struct ofpbuf *, unsigned int *queue_id,
@@ -2662,6 +2658,20 @@ nl_msg_put_act_police(struct ofpbuf *request, struct tc_police police,
}
}
+/* Adds a policer to 'netdev' with a rate of 'kbits_rate' and a burst size
+ * of 'kbits_burst', with a rate of 'kpkts_rate' and a burst size of
+ * 'kpkts_burst'.
+ *
+ * This function is equivalent to running:
+ * /sbin/tc filter add dev <devname> parent ffff: protocol all prio 49
+ * matchall police rate <kbits_rate>kbit burst <kbits_burst>k
+ * mtu 65535 drop
+ *
+ * The configuration and stats may be seen with the following command:
+ * /sbin/tc -s filter show dev <devname> parent ffff:
+ *
+ * Returns 0 if successful, otherwise a positive errno value.
+ */
static int
tc_add_matchall_policer(struct netdev *netdev, uint32_t kbits_rate,
uint32_t kbits_burst, uint32_t kpkts_rate,
@@ -2801,8 +2811,8 @@ netdev_linux_set_policing(struct netdev *netdev_, uint32_t kbits_rate,
goto out;
}
- error = tc_add_policer(netdev_, kbits_rate, kbits_burst,
- kpkts_rate, kpkts_burst);
+ error = tc_add_matchall_policer(netdev_, kbits_rate, kbits_burst,
+ kpkts_rate, kpkts_burst);
if (error){
VLOG_WARN_RL(&rl, "%s: adding policing action failed: %s",
netdev_name, ovs_strerror(error));
@@ -5589,69 +5599,6 @@ netdev_linux_tc_make_request(const struct netdev *netdev, int type,
return tc_make_request(ifindex, type, flags, request);
}
-/* Adds a policer to 'netdev' with a rate of 'kbits_rate' and a burst size
- * of 'kbits_burst', with a rate of 'kpkts_rate' and a burst size of
- * 'kpkts_burst'.
- *
- * This function is equivalent to running:
- * /sbin/tc filter add dev <devname> parent ffff: protocol all prio 49
- * basic police rate <kbits_rate>kbit burst <kbits_burst>k
- * mtu 65535 drop
- *
- * The configuration and stats may be seen with the following command:
- * /sbin/tc -s filter show dev <devname> parent ffff:
- *
- * Returns 0 if successful, otherwise a positive errno value.
- */
-static int
-tc_add_policer(struct netdev *netdev, uint32_t kbits_rate,
- uint32_t kbits_burst, uint32_t kpkts_rate,
- uint32_t kpkts_burst)
-{
- size_t basic_offset, police_offset;
- struct tc_police tc_police;
- struct ofpbuf request;
- struct tcmsg *tcmsg;
- int error;
- int mtu = 65535;
-
- memset(&tc_police, 0, sizeof tc_police);
- tc_police.action = TC_POLICE_SHOT;
- tc_police.mtu = mtu;
- tc_fill_rate(&tc_police.rate, ((uint64_t) kbits_rate * 1000)/8, mtu);
-
- /* The following appears wrong in one way: In networking a kilobit is
- * usually 1000 bits but this uses 1024 bits.
- *
- * However if you "fix" those problems then "tc filter show ..." shows
- * "125000b", meaning 125,000 bits, when OVS configures it for 1000 kbit ==
- * 1,000,000 bits, whereas this actually ends up doing the right thing from
- * tc's point of view. Whatever. */
- tc_police.burst = tc_bytes_to_ticks(
- tc_police.rate.rate, MIN(UINT32_MAX / 1024, kbits_burst) * 1024 / 8);
- tcmsg = netdev_linux_tc_make_request(netdev, RTM_NEWTFILTER,
- NLM_F_EXCL | NLM_F_CREATE, &request);
- if (!tcmsg) {
- return ENODEV;
- }
- tcmsg->tcm_parent = tc_make_handle(0xffff, 0);
- tcmsg->tcm_info = tc_make_handle(49,
- (OVS_FORCE uint16_t) htons(ETH_P_ALL));
- nl_msg_put_string(&request, TCA_KIND, "basic");
- basic_offset = nl_msg_start_nested(&request, TCA_OPTIONS);
- police_offset = nl_msg_start_nested(&request, TCA_BASIC_ACT);
- nl_msg_put_act_police(&request, tc_police, kpkts_rate, kpkts_burst);
- nl_msg_end_nested(&request, police_offset);
- nl_msg_end_nested(&request, basic_offset);
-
- error = tc_transact(&request, NULL);
- if (error) {
- return error;
- }
-
- return 0;
-}
-
static void
read_psched(void)
{
diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
index d6cd2c084..193bfc5c7 100644
--- a/tests/ovs-vsctl.at
+++ b/tests/ovs-vsctl.at
@@ -1670,22 +1670,22 @@ AT_BANNER([set ingress policing test])
AT_SETUP([set ingress_policing_rate and ingress_policing_burst])
AT_KEYWORDS([ingress_policing])
-OVS_VSCTL_SETUP
+AT_CHECK([ip link add a1 type veth])
+AT_CHECK([ip link set a1 up])
+on_exit 'ip link del a1'
+OVS_VSWITCHD_START([add-port br0 a1])
AT_CHECK([RUN_OVS_VSCTL_TOGETHER(
- [add-br a],
- [add-port a a1],
[set interface a1 ingress_policing_rate=100],
[set interface a1 ingress_policing_burst=10],
[--columns=ingress_policing_burst,ingress_policing_rate list interface a1])],
[0],
[
-
-
ingress_policing_burst: 10
ingress_policing_rate: 100
])
-OVS_VSCTL_CLEANUP
+AT_CHECK([grep "adding policing action failed" ovs-vswitchd.log], [1], [], [])
+OVS_VSWITCHD_STOP
AT_CLEANUP
dnl ----------------------------------------------------------------------
@@ -1693,20 +1693,20 @@ AT_BANNER([set ingress policing(kpkts) test])
AT_SETUP([set ingress_policing_kpkts_rate and ingress_policing_kpkts_burst])
AT_KEYWORDS([ingress_policing_kpkts])
-OVS_VSCTL_SETUP
+AT_CHECK([ip link add a1 type veth])
+AT_CHECK([ip link set a1 up])
+on_exit 'ip link del a1'
+OVS_VSWITCHD_START([add-port br0 a1])
AT_CHECK([RUN_OVS_VSCTL_TOGETHER(
- [add-br a],
- [add-port a a1],
[set interface a1 ingress_policing_kpkts_rate=100],
[set interface a1 ingress_policing_kpkts_burst=10],
[--columns=ingress_policing_kpkts_burst,ingress_policing_kpkts_rate list interface a1])],
[0],
[
-
-
ingress_policing_kpkts_burst: 10
ingress_policing_kpkts_rate: 100
])
-OVS_VSCTL_CLEANUP
+AT_CHECK([grep "adding policing action failed" ovs-vswitchd.log], [1], [], [])
+OVS_VSWITCHD_STOP
AT_CLEANUP
--
2.31.1
More information about the dev
mailing list