Hi Darrell,

On Friday 09 June 2017 04:50 AM, Darrell Ball wrote:

>
> On 5/31/17, 6:08 AM, "[email protected] on behalf of Santosh 
> Shukla" <[email protected] on behalf of 
> [email protected]> wrote:
>
>     Some pmd driver(e.g: vNIC thunderx PMD) want mbuf_size to be multiple of
>     cache_line_size. With out this fix, Netdev-dpdk initialization would
>     simply fail for those drivers.
>     
>     Signed-off-by: Santosh Shukla <[email protected]>
>     ---
>     - Tested arm64/ThunderX platform for vNIC pmd,
>     - Topology: phy-phy and phy-vm
>     - Tested x86 platform for XL710/i40e pmd.
>     
>      lib/netdev-dpdk.c | 7 +++++--
>      1 file changed, 5 insertions(+), 2 deletions(-)
>     
>     diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>     index 9941f884f..1952a5cec 100644
>     --- a/lib/netdev-dpdk.c
>     +++ b/lib/netdev-dpdk.c
>     @@ -76,9 +76,12 @@ static struct vlog_rate_limit rl = 
> VLOG_RATE_LIMIT_INIT(5, 20);
>      #define MTU_TO_MAX_FRAME_LEN(mtu)   ((mtu) + ETHER_HDR_MAX_LEN)
>      #define FRAME_LEN_TO_MTU(frame_len) ((frame_len)                    \
>                                           - ETHER_HDR_LEN - ETHER_CRC_LEN)
>     -#define MBUF_SIZE(mtu)              (MTU_TO_MAX_FRAME_LEN(mtu)      \
>     +#define _MBUF_SIZE(mtu)             (MTU_TO_MAX_FRAME_LEN(mtu)      \
>                                           + sizeof(struct dp_packet)     \
>                                           + RTE_PKTMBUF_HEADROOM)
>     +#define MBUF_SIZE(mtu)              ROUND_UP((_MBUF_SIZE(mtu)),     \
>     +                                             RTE_CACHE_LINE_SIZE)
>
> Why not just add ROUND_UP to the existing MBUF_SIZE macro; I think it is
> more straightforward and there are probably enough ‘MTU’ macros
> already.

ok, Will do in v2.

>
>     +
>      #define NETDEV_DPDK_MBUF_ALIGN      1024
>      #define NETDEV_DPDK_MAX_PKT_LEN     9728
>      
>     @@ -495,7 +498,7 @@ dpdk_mp_create(int socket_id, int mtu)
>                                                MP_CACHE_SZ,
>                                                sizeof (struct dp_packet)
>                                                       - sizeof (struct 
> rte_mbuf),
>     -                                          MBUF_SIZE(mtu)
>     +                                          MBUF_SIZE(dmp->mtu)
>
> This change does not seem necessary and distracts from the point
> of the patch.

Ok, Will stick to 'mtu' instead of dmp->mtu. Although, higher up in function mtu
assigned to dmp->mtu, so it made sense to me to use 'dmp->mtu'. But I agree
with your argument. Perhaps should go as separate cleanup patch.

Will do in v2.

Thanks for review feedback.

>                                                       - sizeof(struct 
> dp_packet),
>                                                socket_id);
>              if (dmp->mp) {
>     -- 
>     2.11.0
>     
>     _______________________________________________
>     dev mailing list
>     [email protected]
>     
> https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=1tNyJJHbYGc7-BZ-RPxJoTCjeqEwQs-1EhjxLywTsfo&s=oGUJD3e3CuDW6phStK-eGR-rFu7ldsm3VeSL3Zd7vcg&e=
>  
>     
>

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to