[dpdk-users] A minor suggestion to the comment of the rte_eth_tx_burst() of dpdk-16.04

[dpdk-users] A minor suggestion to the comment of the rte_eth_tx_burst() of dpdk-16.04

[Wu, Xiaoban]

Dear DPDK Users,


I have a very small suggestion to the comment of the rte_eth_tx_burst(). It is not a big deal to the experienced users, but I think this will make the new users know how to use this function more clearly and see the big picture easily.


The comment of "tx_pkts" says:

 * @param tx_pkts
 *   The address of an array of *nb_pkts* pointers to *rte_mbuf* structures
 *   which contain the output packets.


I am a beginner of DPDK. If I see this comment I would probably naively use rte_zmalloc() to construct such pointer array, for each pointer I use rte_malloc() to allocate memory, and finally fill the memory starting from the position (pointed by the macro rte_pktmbuf_mtod()) with real packet content(header and payload).


But I just find that the rte_eth_tx_burst() will call the rte_pktmbuf_free() to free the struct rte_mbuf. In default, "CONFIG_RTE_LIBRTE_MBUF_DEBUG=n", this means it will skip the __rte_mbuf_sanity_check(), and will finally get into rte_mempool_put(). Since above construction process skip the construction of the field "struct rte_mempool", it means the "struct rte_mempool" field is null in each packet "struct rte_mbuf"(of course, all the fields are zero). Hence it will finally cause segmentation fault.


Thus, I have a very small suggestion, please add something like "It is recommended that the tx_pkts is constructed by rte_pktmbuf_pool_create() combined with rte_mempool_get_bulk(), if not please make sure the fields of the struct rte_mbuf are correctly configured."


Thank you,

Xiaoban

Re: [dpdk-users] A minor suggestion to the comment of the rte_eth_tx_burst() of dpdk-16.04

[Thomas Monjalon]

2016-06-10 19:09, Wu, Xiaoban:
> Dear DPDK Users,
> 
> 
> I have a very small suggestion to the comment of the rte_eth_tx_burst(). It is not a big deal to the experienced users, but I think this will make the new users know how to use this function more clearly and see the big picture easily.
> 
> 
> The comment of "tx_pkts" says:
> 
>  * @param tx_pkts
>  *   The address of an array of *nb_pkts* pointers to *rte_mbuf* structures
>  *   which contain the output packets.
> 
> 
> I am a beginner of DPDK. If I see this comment I would probably naively use rte_zmalloc() to construct such pointer array, for each pointer I use rte_malloc() to allocate memory, and finally fill the memory starting from the position (pointed by the macro rte_pktmbuf_mtod()) with real packet content(header and payload).
> 
> 
> But I just find that the rte_eth_tx_burst() will call the rte_pktmbuf_free() to free the struct rte_mbuf. In default, "CONFIG_RTE_LIBRTE_MBUF_DEBUG=n", this means it will skip the __rte_mbuf_sanity_check(), and will finally get into rte_mempool_put(). Since above construction process skip the construction of the field "struct rte_mempool", it means the "struct rte_mempool" field is null in each packet "struct rte_mbuf"(of course, all the fields are zero). Hence it will finally cause segmentation fault.
> 
> 
> Thus, I have a very small suggestion, please add something like "It is recommended that the tx_pkts is constructed by rte_pktmbuf_pool_create() combined with rte_mempool_get_bulk(), if not please make sure the fields of the struct rte_mbuf are correctly configured."

Good suggestion.
Olivier do you agree?

Re: [dpdk-users] A minor suggestion to the comment of the rte_eth_tx_burst() of dpdk-16.04

[Olivier Matz]

Hi,

On 06/10/2016 11:41 PM, Thomas Monjalon wrote:
> 2016-06-10 19:09, Wu, Xiaoban:
>> Dear DPDK Users,
>> 
>> 
>> I have a very small suggestion to the comment of the
>> rte_eth_tx_burst(). It is not a big deal to the experienced users,
>> but I think this will make the new users know how to use this
>> function more clearly and see the big picture easily.
>> 
>> 
>> The comment of "tx_pkts" says:
>> 
>> * @param tx_pkts *   The address of an array of *nb_pkts* pointers
>> to *rte_mbuf* structures *   which contain the output packets.
>> 
>> 
>> I am a beginner of DPDK. If I see this comment I would probably
>> naively use rte_zmalloc() to construct such pointer array, for each
>> pointer I use rte_malloc() to allocate memory, and finally fill the
>> memory starting from the position (pointed by the macro
>> rte_pktmbuf_mtod()) with real packet content(header and payload).
>> 
>> 
>> But I just find that the rte_eth_tx_burst() will call the
>> rte_pktmbuf_free() to free the struct rte_mbuf. In default,
>> "CONFIG_RTE_LIBRTE_MBUF_DEBUG=n", this means it will skip the
>> __rte_mbuf_sanity_check(), and will finally get into
>> rte_mempool_put(). Since above construction process skip the
>> construction of the field "struct rte_mempool", it means the
>> "struct rte_mempool" field is null in each packet "struct
>> rte_mbuf"(of course, all the fields are zero). Hence it will
>> finally cause segmentation fault.
>> 
>> 
>> Thus, I have a very small suggestion, please add something like "It
>> is recommended that the tx_pkts is constructed by
>> rte_pktmbuf_pool_create() combined with rte_mempool_get_bulk(), if
>> not please make sure the fields of the struct rte_mbuf are
>> correctly configured."
> 
> Good suggestion. Olivier do you agree?

Yes, I'll send a patch to dev@dpdk.org to enhance the API doc.
Thanks Xiaoban for reporting.

Regards,
Olivier