On 1 Jul 2023, at 16:40, [email protected] wrote:
> From: Lin Huang <[email protected]>
>
> Now srtcm_policer will free pkts, if packets are exceed rate limit.
> This patch change srtcm_policer not to free pkts, just count dropped packets.
>
Thanks for this patch Lin, however, this patch is wrongly freeing packets.
Your code assumes packets get dropped in order, but this is not the case. The
srtcm_policer_run_single_packet() function can drop the first packet, pass the
second, and drop the third.
Cheers,
Eelco
> Signed-off-by: Lin Huang <[email protected]>
> ---
> lib/netdev-dpdk.c | 30 +++++++++++++++---------------
> 1 file changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 63dac689e..2f7f74395 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -2399,8 +2399,7 @@ netdev_dpdk_srtcm_policer_pkt_handle(struct
> rte_meter_srtcm *meter,
> static int
> srtcm_policer_run_single_packet(struct rte_meter_srtcm *meter,
> struct rte_meter_srtcm_profile *profile,
> - struct rte_mbuf **pkts, int pkt_cnt,
> - bool should_steal)
> + struct rte_mbuf **pkts, int pkt_cnt)
> {
> int i = 0;
> int cnt = 0;
> @@ -2410,16 +2409,9 @@ srtcm_policer_run_single_packet(struct rte_meter_srtcm
> *meter,
> for (i = 0; i < pkt_cnt; i++) {
> pkt = pkts[i];
> /* Handle current packet */
> - if (netdev_dpdk_srtcm_policer_pkt_handle(meter, profile,
> - pkt, current_time)) {
> - if (cnt != i) {
> - pkts[cnt] = pkt;
> - }
> + if (!netdev_dpdk_srtcm_policer_pkt_handle(meter, profile,
> + pkt, current_time)) {
> cnt++;
> - } else {
> - if (should_steal) {
> - rte_pktmbuf_free(pkt);
> - }
> }
> }
>
> @@ -2435,10 +2427,14 @@ ingress_policer_run(struct ingress_policer *policer,
> struct rte_mbuf **pkts,
> rte_spinlock_lock(&policer->policer_lock);
> cnt = srtcm_policer_run_single_packet(&policer->in_policer,
> &policer->in_prof,
> - pkts, pkt_cnt, should_steal);
> + pkts, pkt_cnt);
> rte_spinlock_unlock(&policer->policer_lock);
>
> - return cnt;
> + if (should_steal && cnt) {
> + rte_pktmbuf_free_bulk(&pkts[pkt_cnt - cnt], cnt);
> + }
> +
> + return pkt_cnt - cnt;
> }
>
> static bool
> @@ -4976,9 +4972,13 @@ egress_policer_run(struct qos_conf *conf, struct
> rte_mbuf **pkts, int pkt_cnt,
>
> cnt = srtcm_policer_run_single_packet(&policer->egress_meter,
> &policer->egress_prof, pkts,
> - pkt_cnt, should_steal);
> + pkt_cnt);
>
> - return cnt;
> + if (should_steal && cnt) {
> + rte_pktmbuf_free_bulk(&pkts[pkt_cnt - cnt], cnt);
> + }
> +
> + return pkt_cnt - cnt;
> }
>
> static const struct dpdk_qos_ops egress_policer_ops = {
> --
> 2.39.1
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
