[ovs-dev] [PATCH v4 3/7] netdev: Remove unused may_steal.

Ilya Maximets i.maximets at samsung.com
Mon Oct 9 06:49:25 UTC 2017


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.


More information about the dev mailing list