[ovs-dev] netdev-dpdk: use rte_eth_dev_set_mtu

DPDK provides an API to set the MTU of compatible physical devices -
rte_eth_dev_set_mtu(). Prior to DPDK v16.07 however, this API was not
implemented in some DPDK PMDs (i40e, specifically). To allow the use
of jumbo frames with affected NICs in OvS-DPDK, MTU configuration was
achieved by setting the jumbo frame flag, and corresponding maximum
permitted Rx frame size, in an rte_eth_conf structure for the NIC
port, and subsequently invoking rte_eth_dev_configure() with that
configuration.

However, that method does not set the MTU field of the underlying DPDK
structure (rte_eth_dev) for the corresponding physical device;
consequently, rte_eth_dev_get_mtu() reports the incorrect MTU for an
OvS-DPDK phy device with non-standard MTU.

Resolve this issue by invoking rte_eth_dev_set_mtu() when setting up
or modifying the MTU of a DPDK phy port.

Fixes: 0072e93 ("netdev-dpdk: add support for jumbo frames")
Reported-by: Vipin Varghese <vipin.varghese@intel.com>
Signed-off-by: Mark Kavanagh <mark.b.kavanagh@intel.com>
---
 lib/netdev-dpdk.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

Hi Mark,

Mark Kavanagh <mark.b.kavanagh@intel.com> writes:

> DPDK provides an API to set the MTU of compatible physical devices -
> rte_eth_dev_set_mtu(). Prior to DPDK v16.07 however, this API was not
> implemented in some DPDK PMDs (i40e, specifically). To allow the use
> of jumbo frames with affected NICs in OvS-DPDK, MTU configuration was
> achieved by setting the jumbo frame flag, and corresponding maximum
> permitted Rx frame size, in an rte_eth_conf structure for the NIC
> port, and subsequently invoking rte_eth_dev_configure() with that
> configuration.
>
> However, that method does not set the MTU field of the underlying DPDK
> structure (rte_eth_dev) for the corresponding physical device;
> consequently, rte_eth_dev_get_mtu() reports the incorrect MTU for an
> OvS-DPDK phy device with non-standard MTU.
>
> Resolve this issue by invoking rte_eth_dev_set_mtu() when setting up
> or modifying the MTU of a DPDK phy port.
>
> Fixes: 0072e93 ("netdev-dpdk: add support for jumbo frames")
> Reported-by: Vipin Varghese <vipin.varghese@intel.com>
> Signed-off-by: Mark Kavanagh <mark.b.kavanagh@intel.com>
> ---

I think I should also get a reported-by tag for this ;) :

https://mail.openvswitch.org/pipermail/ovs-dev/2016-January/308828.html

I should have probably not claimed that it wasn't a show-stopper.  I
also don't see the comment I requested you to add that would warn us to
revisit this in the future :(

I will carve out some time to apply and test this.  Thanks Mark.

-Aaron

>From: Aaron Conole [mailto:aconole@redhat.com]
>Sent: Friday, June 9, 2017 8:26 PM
>To: Kavanagh, Mark B <mark.b.kavanagh@intel.com>
>Cc: ovs-dev@openvswitch.org; Varghese, Vipin <vipin.varghese@intel.com>
>Subject: Re: [ovs-dev] [PATCH] netdev-dpdk: use rte_eth_dev_set_mtu
>
>Hi Mark,
>
>Mark Kavanagh <mark.b.kavanagh@intel.com> writes:
>
>> DPDK provides an API to set the MTU of compatible physical devices -
>> rte_eth_dev_set_mtu(). Prior to DPDK v16.07 however, this API was not
>> implemented in some DPDK PMDs (i40e, specifically). To allow the use
>> of jumbo frames with affected NICs in OvS-DPDK, MTU configuration was
>> achieved by setting the jumbo frame flag, and corresponding maximum
>> permitted Rx frame size, in an rte_eth_conf structure for the NIC
>> port, and subsequently invoking rte_eth_dev_configure() with that
>> configuration.
>>
>> However, that method does not set the MTU field of the underlying DPDK
>> structure (rte_eth_dev) for the corresponding physical device;
>> consequently, rte_eth_dev_get_mtu() reports the incorrect MTU for an
>> OvS-DPDK phy device with non-standard MTU.
>>
>> Resolve this issue by invoking rte_eth_dev_set_mtu() when setting up
>> or modifying the MTU of a DPDK phy port.
>>
>> Fixes: 0072e93 ("netdev-dpdk: add support for jumbo frames")
>> Reported-by: Vipin Varghese <vipin.varghese@intel.com>
>> Signed-off-by: Mark Kavanagh <mark.b.kavanagh@intel.com>
>> ---
>
>I think I should also get a reported-by tag for this ;) :

Hey Aaron,

You definitely have a much better memory than me ;)

I pushed this patch in response to a request from Vipin, who flagged an issue he encountered with rte_eth_dev_get_mtu() last week.
You're completely right though, you did point it out way back in January of last year - apologies for the oversight, I'll add a reported-by tag for you in v2.

>
>https://mail.openvswitch.org/pipermail/ovs-dev/2016-January/308828.html
>
>I should have probably not claimed that it wasn't a show-stopper.  I
>also don't see the comment I requested you to add that would warn us to
>revisit this in the future :(

I did add that comment in V3 of the patch, but removed it again in V4: https://mail.openvswitch.org/pipermail/ovs-dev/2016-February/309599.html.

In V4, the implementation of dpdk_buf_size changed completely:

	== V3 ==
	static uint32_t
	dpdk_buf_size(struct netdev_dpdk *netdev, int frame_len)
	{
	    struct rte_eth_dev_info info;
	    uint32_t buf_size;
	    /* XXX: This is a workaround for DPDK v2.2, and needs to be refactored with a
	     * future DPDK release. */
	    uint32_t len = frame_len + (2 * DPDK_VLAN_TAG_LEN);
	
	    if(netdev->type == DPDK_DEV_ETH) {
	        rte_eth_dev_info_get(netdev->port_id, &info);
	        buf_size = (info.min_rx_bufsize == 0) ?
	                   NETDEV_DPDK_DEFAULT_RX_BUFSIZE :
	                   info.min_rx_bufsize;
	    } else {
	        buf_size = NETDEV_DPDK_DEFAULT_RX_BUFSIZE;
	    }
	
	    if(len % buf_size) {
	        len = buf_size * ((len/buf_size) + 1);
	    }
	
	    return len;
	}
	
	== V4 ==
	static uint32_t
	dpdk_buf_size(int mtu)
	{
	    return ROUND_UP((MTU_TO_MAX_FRAME_LEN(mtu) + RTE_PKTMBUF_HEADROOM),
		                     NETDEV_DPDK_MBUF_ALIGN);
	}

In replacing the function body, I omitted to include the comment, which is on me - sorry about that... :( 

Thanks,
Mark

>
>I will carve out some time to apply and test this.  Thanks Mark.
>
>-Aaron