On 12 Nov 2021, at 18:49, Mike Pattrick wrote:
> Currently ingress policing uses the basic classifier to apply traffic
> control filters if hardware offload is not enabled, else it uses
> matchall. This change enables fallback onto the matchall classifier for
> cases when the kernel is not built with basic support and hardware
> offload is not in use. Basic is still selected first.
>
> The system tests are modified to allow either basic or matchall
> classification on the ingestion filter, and to allow either 10000 or
> 10240 packets for the packet burst filter. 10000 is accurate for kernel
> 5.14 and the most recent iproute2, however, 10240 is left for
> compatibility with older kernels.
>
> Signed-off-by: Mike Pattrick <mkp at redhat.com>
> ---
> lib/netdev-linux.c | 28 +++++++++++++++++++---------
> tests/system-offloads-traffic.at | 20 +++++++++-----------
> 2 files changed, 28 insertions(+), 20 deletions(-)
>
> diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
> index 97bd21be4..e7d26de83 100644
> --- a/lib/netdev-linux.c
> +++ b/lib/netdev-linux.c
> @@ -2776,8 +2776,7 @@ netdev_linux_set_policing(struct netdev *netdev_,
> uint32_t kbits_rate,
> error = tc_add_matchall_policer(netdev_, kbits_rate, kbits_burst,
> kpkts_rate, kpkts_burst);
> }
> - ovs_mutex_unlock(&netdev->mutex);
> - return error;
> + goto out;
> }
>
> error = get_ifindex(netdev_, &ifindex);
> @@ -2794,6 +2793,8 @@ netdev_linux_set_policing(struct netdev *netdev_,
> uint32_t kbits_rate,
> }
>
> if (kbits_rate || kpkts_rate) {
> + const char * cls_name = "basic";
Changes look good to me, however, there should be no space after the *.
Maybe the maintainer can fix this on commit?
Acked-by: Eelco Chaudron <echau...@redhat.com>
> +
> error = tc_add_del_qdisc(ifindex, true, 0, TC_INGRESS);
> if (error) {
> VLOG_WARN_RL(&rl, "%s: adding policing qdisc failed: %s",
> @@ -2803,19 +2804,28 @@ netdev_linux_set_policing(struct netdev *netdev_,
> uint32_t kbits_rate,
>
> error = tc_add_policer(netdev_, kbits_rate, kbits_burst,
> kpkts_rate, kpkts_burst);
> + if (error == ENOENT) {
> + cls_name = "matchall";
> + /* This error is returned when the basic classifier is missing.
> + * Fall back to the matchall classifier. */
> + 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));
> + VLOG_WARN_RL(&rl, "%s: adding cls_%s policing action failed: %s",
> + netdev_name, cls_name, ovs_strerror(error));
> goto out;
> }
> }
>
> - netdev->kbits_rate = kbits_rate;
> - netdev->kbits_burst = kbits_burst;
> - netdev->kpkts_rate = kpkts_rate;
> - netdev->kpkts_burst = kpkts_burst;
> -
> out:
> + if (!error) {
> + netdev->kbits_rate = kbits_rate;
> + netdev->kbits_burst = kbits_burst;
> + netdev->kpkts_rate = kpkts_rate;
> + netdev->kpkts_burst = kpkts_burst;
> + }
> +
> if (!error || error == ENODEV) {
> netdev->netdev_policing_error = error;
> netdev->cache_valid |= VALID_POLICING;
> diff --git a/tests/system-offloads-traffic.at
> b/tests/system-offloads-traffic.at
> index be63057bb..80bc1dd5c 100644
> --- a/tests/system-offloads-traffic.at
> +++ b/tests/system-offloads-traffic.at
> @@ -89,10 +89,8 @@ AT_CHECK([tc -o -s -d filter show dev ovs-p0 ingress |
> [0],[dnl
> rate 100Kbit burst 1280b
> ])
> -AT_CHECK([tc -s -d filter show dev ovs-p0 ingress | grep basic |
> - sed -n 's/.*\(basic\).*/\1/; T; p; q'], [0], [dnl
> -basic
> -])
> +AT_CHECK([tc -s -d filter show dev ovs-p0 ingress |
> + egrep "basic|matchall" > /dev/null], [0])
> OVS_TRAFFIC_VSWITCHD_STOP
> AT_CLEANUP
>
> @@ -135,14 +133,13 @@ AT_CHECK([ovs-vsctl --columns=other_config list open],
> [0], [dnl
> other_config : {hw-offload="false"}
> ])
> AT_CHECK([tc -o -s -d filter show dev ovs-p0 ingress |
> - sed -n 's/.*\(pkts_rate [[0-9]]*[[a-zA-Z]]* pkts_burst
> [[0-9]]*[[a-zA-Z]]*\).*/\1/; T; p; q'],
> + sed -n 's/.*\(pkts_rate [[0-9]]*[[a-zA-Z]]* pkts_burst
> [[0-9]]*[[a-zA-Z]]*\).*/\1/; T; p; q' |
> + sed -e 's/10240/10000/'],
> [0],[dnl
> -pkts_rate 100000 pkts_burst 10240
> +pkts_rate 100000 pkts_burst 10000
> ])
> AT_CHECK([tc -s -d filter show dev ovs-p0 ingress |
> - sed -n 's/.*\(basic\).*/\1/; T; p; q'], [0], [dnl
> -basic
> -])
> + egrep "basic|matchall" > /dev/null], [0])
> OVS_TRAFFIC_VSWITCHD_STOP
> AT_CLEANUP
>
> @@ -160,9 +157,10 @@ AT_CHECK([ovs-vsctl --columns=other_config list open],
> [0], [dnl
> other_config : {hw-offload="true"}
> ])
> AT_CHECK([tc -o -s -d filter show dev ovs-p0 ingress |
> - sed -n 's/.*\(pkts_rate [[0-9]]*[[a-zA-Z]]* pkts_burst
> [[0-9]]*[[a-zA-Z]]*\).*/\1/; T; p; q'],
> + sed -n 's/.*\(pkts_rate [[0-9]]*[[a-zA-Z]]* pkts_burst
> [[0-9]]*[[a-zA-Z]]*\).*/\1/; T; p; q' |
> + sed -e 's/10240/10000/'],
> [0],[dnl
> -pkts_rate 100000 pkts_burst 10240
> +pkts_rate 100000 pkts_burst 10000
> ])
> AT_CHECK([tc -s -d filter show dev ovs-p0 ingress |
> sed -n 's/.*\(matchall\).*/\1/; T; p; q'], [0], [dnl
> --
> 2.27.0
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
