>From: Aaron Conole [mailto:acon...@redhat.com]
>Sent: Friday, June 9, 2017 8:26 PM
>To: Kavanagh, Mark B <mark.b.kavan...@intel.com>
>Cc: ovs-dev@openvswitch.org; Varghese, Vipin <vipin.vargh...@intel.com>
>Subject: Re: [ovs-dev] [PATCH] netdev-dpdk: use rte_eth_dev_set_mtu
>
>Hi Mark,
>
>Mark Kavanagh <mark.b.kavan...@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.vargh...@intel.com>
>> Signed-off-by: Mark Kavanagh <mark.b.kavan...@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
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to