> On 27/11/2018 13:57, Stokes, Ian wrote: > >> Commit dfaf00e started using the result of dpdk_buf_size() to > >> calculate the available size on each mbuf, as opposed to using the > >> previous MBUF_SIZE macro. However, this was calculating the mbuf size > >> by adding up the MTU with RTE_PKTMBUF_HEADROOM and only then aligning > >> to NETDEV_DPDK_MBUF_ALIGN. Instead, the accounting for the > >> RTE_PKTMBUF_HEADROOM should only happen after alignment, as per below. > >> > >> Before alignment: > >> ROUNDUP(MTU(1500) + RTE_PKTMBUF_HEADROOM(128), 1024) = 2048 > >> > >> After aligment: > >> ROUNDUP(MTU(1500), 1024) + 128 = 2176 > >> > >> This might seem insignificant, however, it might have performance > >> implications in DPDK, where each mbuf is expected to have 2k + > >> RTE_PKTMBUF_HEADROOM of available space. This is because not only > >> some NICs have course grained alignments of 1k, they will also take > >> RTE_PKTMBUF_HEADROOM bytes from the overall available space in an > >> mbuf when setting up their Rx requirements. Thus, only the "After > alignment" > >> case above would guarantee a 2k of available room, as the "Before > >> alignment" would report only 1920B. > >> > >> Some extra information can be found at: > >> https://mails.dpdk.org/archives/dev/2018-November/119219.html > >> > >> Note: This has been found by Ian Stokes while going through some > >> af_packet checks. > >> > > > > Hi Tiago,, thanks for this, just a query below. > > > >> Reported-by: Ian Stokes <ian.sto...@intel.com> > >> Fixes: dfaf00e ("netdev-dpdk: fix mbuf sizing") > >> Signed-off-by: Tiago Lam <tiago....@intel.com> > >> --- > >> Documentation/topics/dpdk/memory.rst | 20 ++++++++++---------- > >> lib/netdev-dpdk.c | 6 ++++-- > >> 2 files changed, 14 insertions(+), 12 deletions(-) > >> > >> diff --git a/Documentation/topics/dpdk/memory.rst > >> b/Documentation/topics/dpdk/memory.rst > >> index c9b739f..3c4ee17 100644 > >> --- a/Documentation/topics/dpdk/memory.rst > >> +++ b/Documentation/topics/dpdk/memory.rst > >> @@ -107,8 +107,8 @@ Example 1 > >> > >> MTU = 1500 Bytes > >> Number of mbufs = 262144 > >> - Mbuf size = 2752 Bytes > >> - Memory required = 262144 * 2752 = 721 MB > >> + Mbuf size = 2880 Bytes > >> + Memory required = 262144 * 2880 = 755 MB > > > > These measurements don't deem to take the header and trailer into > account for total size when calculating the memory requirements? Is there > any particular reason? > > > > total_elt_sz = mp->header_size + mp->elt_size + mp->trailer_size; > > > > I would expect above to be 262144 * 3008 = 788 MB. > > Thanks for looking into it, Ian. > > I hadn't considered that. > > But I guess you're right, technically, since that will end up being the > total size of each mempool element. However, the size of each mbuf is > still the 2880B above. > > Looking into commit 31154f95 (before the first change to this file was > applied), I can see that the size of each mbuf would be 2944B (for a 1500B > MTU), where each mempool element would have a size of 3008B (the extra 64B > is the mp->header_size above). And we are using the 3008B instead of the > 2944B as the mbuf size. Was this on purpose to reflect the total memory > required?
Yes, this is on purpose to account for the total memory requirements for a given deployment as memory for the header would also have to be provisioned for. > > Just wondering though, I can add the same logic anyway, which would change > the values to 2880B + 64B = 2944B. I think above should be 3008 again, looking at the v1 of this patch it would break down as follows total_elt_sz = mp->header_size + mp->elt_size + mp->trailer_size; 3008 = 64 + 2880 + 64 I would think the trailer should be accounted for as well as the header. Ian > > Tiago. _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev