On 29 Sep 2026, at 10:34, David Marchand wrote:
> Move this code to a new netdev-dpdk-common.c file for reuse.
> Add separate wrappers for eth and vhost netdevs (in preparation of the
> separation later).
Thanks David, the patch looks good to me in general. Some style issues below.
//Eelco
> diff --git a/lib/netdev-dpdk-common.c b/lib/netdev-dpdk-common.c
[...]
> + * failure, except where otherwise noted. All of them must be provided,
> except
> + * where otherwise noted.
> + */
The closing */ could go at the end of the previous line in
all instances here. Same comment as on the previous patch
regarding consistent style across the file.
[...]
> + struct rte_mbuf **pkts, int pkt_cnt,
> + bool should_steal)
> +{
> + int i = 0;
> + int cnt = 0;
> + struct rte_mbuf *pkt = NULL;
> + uint64_t current_time = rte_rdtsc();
Reverse christmas tree for the local declarations, now
that the code is being moved?
[...]
> +dpdk_qos_ingress_policer_construct(uint32_t rate, uint32_t burst)
> +{
> + struct dpdk_qos_ingress_policer *policer = NULL;
> + uint64_t rate_bytes;
> + uint64_t burst_bytes;
Swap these two for reverse christmas tree.
> + int err = 0;
[...]
> + /* Destroy any existing ingress policer for the device if one exists */
Period at the end of comment.
[...]
> + if (policer) {
> + ovsrcu_postpone__((void (*)(void *))
> dpdk_qos_ingress_policer_destruct,
> + policer);
Could this use ovsrcu_postpone() directly? The types
match: dpdk_qos_ingress_policer_destruct() takes a
'struct dpdk_qos_ingress_policer *' and 'policer' is
the same type.
[...]
> diff --git a/lib/netdev-dpdk-common.h b/lib/netdev-dpdk-common.h
[...]
> +void dpdk_qos_ingress_policer_destruct(struct dpdk_qos_ingress_policer *);
> +int dpdk_qos_ingress_policer_run(struct dpdk_qos_ingress_policer *policer,
Per the coding style, parameter names can be omitted
from prototypes when the type already makes the purpose
clear. For instance 'policer' here, 'common' below, and
similar struct pointer parameters throughout this move.
Also true for earlier patches.
> + struct rte_mbuf **pkts, int pkt_cnt,
> + bool should_steal);
[...]
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
[...]
> + nb_rx = dpdk_qos_ingress_policer_run(policer,
> (struct rte_mbuf **) batch->packets,
> nb_rx, true);
The continuation lines still have the old alignment from
ingress_policer_run(). Both call sites need re-aligning
to the opening parenthesis of
dpdk_qos_ingress_policer_run().
> qos_drops -= nb_rx;
[...]
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
