On 09.10.2017 09:16, Ilya Maximets wrote:
> On 08.10.2017 12:32, Gao Zhenyu wrote:
>> Hi llya,
>>
>> Thanks for working it. Your patch tried to eliminate the may_steal in dpdk
>> qos, because may_steal handled on dpif-netdev layer and always true.
>> But in function dpdk_do_tx_copy, it set the may_steal to false, because
>> the packet buffer were not allocated by dpdk side so cannot released by
>> netdev_dpdk_policer_run(). Otherwise it may hit coredump. Did you test that
>> scenario?
>>
>> Thanks
>> Zhenyu Gao
>
> Good catch. Thanks.
>
> Following incremental can be used to fix the issue:
> ----------------------------------------------------------------------------
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 300a0ae..3352ae2 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -1538,7 +1538,9 @@ netdev_dpdk_policer_run(struct rte_meter_srtcm *meter,
> }
> cnt++;
> } else {
> - rte_pktmbuf_free(pkt);
> + /* In case of calling from 'dpdk_do_tx_copy' 'pkt' could be not
> + * a DPDK allocated mbuf. */
> + dp_packet_delete((struct dp_packet *) pkt);
> }
> }
>
> ----------------------------------------------------------------------------
>
>
> Best regards, Ilya Maximets.
>
Sorry, one more change is required to avoid double free:
----------------------------------------------------------------------------
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 3352ae2..bb8bbf3 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -1851,6 +1851,7 @@ dpdk_do_tx_copy(struct netdev *netdev, int qid, struct
dp_packet_batch *batch)
cnt = netdev_dpdk_qos_run(dev, (struct rte_mbuf **) batch->packets,
batch_cnt);
dropped += batch_cnt - cnt;
+ batch->count = cnt;
}
for (uint32_t i = 0; i < cnt; i++) {
----------------------------------------------------------------------------
OTOH, 'may_steal' in QoS is a different entity. This patch is intended to
remove 'may_steal' from netdev API, but this is an internal to netdev-dpdk
variable. So, we can keep it in this patch set and use patch from v3.
Beside that, I think, some cleanup is needed here anyway.
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
