> On 11/07/2018 12:40, Ian Stokes wrote: > > On 7/10/2018 12:06 PM, Tiago Lam wrote: > >> When configuring a mempool, in netdev_dpdk_mempool_configure(), the > >> result of a call to dpdk_buf_size() is being used as the MTU. This, > >> however, is not the MTU but a ROUND_UP aligned number based on the > >> MTU, which could lead to the reuse of mempools even when the real > >> MTUs between different ports differ but somehow round up to the same > >> mbuf size. > >> > >> To support multi-segment mbufs, which is coming in later commits, > >> this distinction is important. For multi-segment mbufs the mbuf size > >> is always the same (multiple mbufs are chained together to hold the > >> packet's data), which means that using the mbuf size as the mtu would > >> lead to reusing always the same mempool, independent of the MTU set. > >> > >> To fix this, two fields are now passed to dpdk_mp_create(), the mtu > >> and the mbuf_size, thus creating a distinction between the two. The > >> latter is used for telling rte_pktmbuf_pool_create() the size of each > >> mbuf, while the former is used for tracking purposes in struct > >> dpdk_mp and as an input to create a unique hash for the mempool's name. > >> > >> Signed-off-by: Tiago Lam <[email protected]> > > > > Hi Tiago, > > > > I don't think these changes are acceptable as they break from the > > expected behavior of the shared mempool model. > > > > The reason we introduced the shared mempool model was that it was used > > from OVS 2.6 -> 2.9. > > > > A user with the same deployment moving between releases would not have > > to re-calculate the memory they dimension for OVS DPDK. > > > > With these changes however they may have to. > > > > Consider the following example: > > > > A user adds a port with mtu 1500 and a second port with mtu 1800. > > Assuming both ports are on the same socket, the same mempool would be > > re-used for both ports in the current shared mempool model. > > > However with this change, separate mempools would have to be created > > as the MTU is different, potentially requiring an increase in the > > memory provisioned by the user. It may not be possible for users to > > upgrade from OVS 2.9 to 2.10 without re-dimension their memory with > this. > > > > Hi Ian, > > Thanks for the context here. > > It's a valid point. I did consider this, but wasn't aware that users / > deployments were also taking this round up into account. In that case, > this could certainly cause problems during upgrade. > > > We had a similar issue to this in OVS 2.9 release with the per port > > memory model, it was flagged as a blocking factor for Red Hat and > > Ericsson upgrades which led to the removal of the per port model in 2.9. > > > > One possible solution here is that multiseg is supported for the per > > port memory model only? > > > > It seems we can adapt this in a different way. I was trying to reuse the > "mbuf_size" for both cases; In the per-port and shared mempool case, where > it is being used as the MTU, and in the multi-segment case where it is > being used for telling rte_pktmbuf_pool_create() the size of each mbuf. > Hence why I thought it would be better to differentiate between the real > MTU and the "mbuf_size". Instead, we can calculate two "mbuf_sizes" - the > one the shared and per-port mempool models use for tracking the MTU > changes, and in case multi-segment mbufs are in use, the "mbuf_size" for > when creating the mempool. This should suffice. > > I'll update the series with this; This patch 01/15 will probably be > dropped entirely, and patch 12/15 will likely be the one taking the > changes mentioned above. > > I'll also update patches 05/15 and 11/15 to fix the Travis CI. > > Thanks, > Tiago.
That sounds ok, will review in the next revision. Thanks Ian _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
