[ovs-dev] [PATCH v3] netdev-dpdk: use rte_eth_dev_set_mtu

Darrell Ball dball at vmware.com
Tue Jul 11 01:03:19 UTC 2017



On 7/10/17, 2:44 AM, "Kavanagh, Mark B" <mark.b.kavanagh at intel.com> wrote:

    >From: Darrell Ball [mailto:dball at vmware.com]
    >Sent: Saturday, July 8, 2017 10:34 PM
    >To: Kavanagh, Mark B <mark.b.kavanagh at intel.com>; Chandran, Sugesh
    ><sugesh.chandran at intel.com>; ovs-dev at openvswitch.org; Varghese, Vipin
    ><vipin.varghese at intel.com>; aconole at redhat.com
    >Subject: Re: [ovs-dev] [PATCH v3] netdev-dpdk: use rte_eth_dev_set_mtu
    >
    >So, this required a 16.11.2 bug fix.
    >
    >Are we sure this will work for all/most NICs/drivers ?
    
    I can't say tbh Darrell, as I can only test with the NICs that are available to me (namely, Niantic and Fortville).
    
    However, AFAIA, this issue is unique to Niantic, since that H/W permits dynamic modification of MTU without stopping its PMD; for most NICs (including Fortville), a port's PMD must be stopped before modifying its MTU, and restarted thereafter.

I understand

    
    >In previous alluding to error cases, did calling rte_eth_dev_set_mtu() block
    >jumbo packets
    >that would otherwise be allowed ?
    
    For ixgbe, a call to rte_eth_dev_set_mtu(some_jumbo_frame_size) would return EINVAL, which means that the MTU for the NIC's ports could never be changed beyond the standard MTU.
    
    With this patch, that behavior is avoided, and the MTU can be set appropriately.


The question that I was getting at is since we are now removing the 
jumbo_frame flag workaround with this patch, can there be a regression where we cannot 
forward jumbo frames because rte_eth_dev_set_mtu() fails due to still another bug in some
cases ?
If the answer is yes, does it make sense to keep the jumbo_frame flag workaround as well,
‘’assuming it is compatible” with rte_eth_dev_set_mtu(), to handle unknown corner cases ?

    
    >
    >
    >
    >On 7/7/17, 3:18 AM, "ovs-dev-bounces at openvswitch.org on behalf of Kavanagh,
    >Mark B" <ovs-dev-bounces at openvswitch.org on behalf of
    >mark.b.kavanagh at intel.com> wrote:
    >
    >    >From: Chandran, Sugesh
    >    >Sent: Friday, July 7, 2017 10:44 AM
    >    >To: Kavanagh, Mark B <mark.b.kavanagh at intel.com>; ovs-
    >dev at openvswitch.org;
    >    >Varghese, Vipin <vipin.varghese at intel.com>; aconole at redhat.com
    >    >Subject: RE: [ovs-dev] [PATCH v3] netdev-dpdk: use rte_eth_dev_set_mtu
    >    >
    >    >Hi Mark,
    >    >Thank you for the information.
    >    >Please find my comments below.
    >    >
    >    >Regards
    >    >_Sugesh
    >    >
    >    >
    >    >> -----Original Message-----
    >    >> From: Kavanagh, Mark B
    >    >> Sent: Wednesday, July 5, 2017 10:20 AM
    >    >> To: Chandran, Sugesh <sugesh.chandran at intel.com>; ovs-
    >    >> dev at openvswitch.org; Varghese, Vipin <vipin.varghese at intel.com>;
    >    >> aconole at redhat.com
    >    >> Subject: RE: [ovs-dev] [PATCH v3] netdev-dpdk: use rte_eth_dev_set_mtu
    >    >>
    >    >>
    >    >>
    >    >> >From: Chandran, Sugesh
    >    >> >Sent: Monday, July 3, 2017 4:36 PM
    >    >> >To: Kavanagh, Mark B <mark.b.kavanagh at intel.com>;
    >    >> >ovs-dev at openvswitch.org; Varghese, Vipin <vipin.varghese at intel.com>;
    >    >> >aconole at redhat.com
    >    >> >Subject: RE: [ovs-dev] [PATCH v3] netdev-dpdk: use rte_eth_dev_set_mtu
    >    >> >
    >    >> >Hi Mark,
    >    >> >
    >    >> >Regards
    >    >> >_Sugesh
    >    >> >
    >    >> >
    >    >> >> -----Original Message-----
    >    >> >> From: ovs-dev-bounces at openvswitch.org [mailto:ovs-dev-
    >    >> >> bounces at openvswitch.org] On Behalf Of Mark Kavanagh
    >    >> >> Sent: Wednesday, June 28, 2017 3:52 PM
    >    >> >> To: ovs-dev at openvswitch.org; Varghese, Vipin
    >    >> >> <vipin.varghese at intel.com>; aconole at redhat.com
    >    >> >> Subject: [ovs-dev] [PATCH v3] 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: Aaron Conole <aconole at redhat.com>
    >    >> >> Reported-by: Vipin Varghese <vipin.varghese at intel.com>
    >    >> >> Reviewed-by: Aaron Conole <aconole at redhat.com>
    >    >> >> Signed-off-by: Mark Kavanagh <mark.b.kavanagh at intel.com>
    >    >> >> ---
    >    >> >>
    >    >> >> v3->v2:
    >    >> >>     - enable scatter_rx explicitly for jumbo MTU
    >    >> >>
    >    >> >> v2->v1:
    >    >> >>     - add 'reported-by' tag for Aaron Conole
    >    >> >>         - change VLOG_INFO to VLOG_ERR
    >    >> >>
    >    >> >>  lib/netdev-dpdk.c | 16 +++++++++++-----
    >    >> >>  1 file changed, 11 insertions(+), 5 deletions(-)
    >    >> >>
    >    >> >> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index
    >    >> >> bba4de3..671d585
    >    >> >> 100644
    >    >> >> --- a/lib/netdev-dpdk.c
    >    >> >> +++ b/lib/netdev-dpdk.c
    >    >> >> @@ -650,13 +650,12 @@ dpdk_eth_dev_queue_setup(struct
    >    >> netdev_dpdk
    >    >> >> *dev, int n_rxq, int n_txq)
    >    >> >>      int i;
    >    >> >>      struct rte_eth_conf conf = port_conf;
    >    >> >>
    >    >> >> +    /* For some NICs (e.g. Niantic), scatter_rx mode needs to be
    >    >explicitly
    >    >> >> +     * enabled. */
    >    >> >>      if (dev->mtu > ETHER_MTU) {
    >    >> >> -        conf.rxmode.jumbo_frame = 1;
    >    >> >> -        conf.rxmode.max_rx_pkt_len = dev->max_packet_len;
    >    >> >> -    } else {
    >    >> >> -        conf.rxmode.jumbo_frame = 0;
    >    >> >> -        conf.rxmode.max_rx_pkt_len = 0;
    >    >> >> +        conf.rxmode.enable_scatter = 1;
    >    >> >>      }
    >    >> >> +
    >    >> >>      conf.rxmode.hw_ip_checksum = (dev->hw_ol_features &
    >    >> >>                                    NETDEV_RX_CHECKSUM_OFFLOAD) != 0;
    >    >> >>      /* A device may report more queues than it makes available
    >(this
    >    >> >> has @@
    >    >> >> -676,6 +675,13 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk
    >    >> *dev,
    >    >> >> int n_rxq, int n_txq)
    >    >> >>              break;
    >    >> >>          }
    >    >> >>
    >    >> >> +        diag = rte_eth_dev_set_mtu(dev->port_id, dev->mtu);
    >    >> >> +        if (diag) {
    >    >> >> +            VLOG_ERR("Interface %s MTU (%d) setup error: %s",
    >    >> >> +                    dev->up.name, dev->mtu, rte_strerror(-diag));
    >    >> >[Sugesh] I am still getting this error when I try to test this feature
    >    >> >on a Niantic cards.
    >    >>
    >    >> Hi Sugesh,
    >    >>
    >    >> What version of DPDK are you using?
    >    >>
    >    >> I'm guessing that it's 16.11.0 - please try testing with v16.11.2,
    >since
    >    >this
    >    >> contains a bugfix for the ixgbe set_mtu function that is required for
    >this
    >    >> patch.
    >    >>
    >    >[Sugesh] Tested with v16.11.2.  It worked fine in my test setup and you
    >can
    >    >add
    >    >'Tested by'
    >    >And the changes are looks ok for me too.
    >
    >    Thanks Sugesh - shall I add 'Tested-by' and 'Acked-by' tags for you in
    >that case?
    >
    >    Cheers,
    >    Mark
    >
    >    >
    >    >> In any event, I've been in contact with DPDK colleagues wrt the
    >inconsistent
    >    >> behavior between ixgbe and i40e drivers that warrant this change;
    >they've
    >    >> pushed a patch to address same:
    >    >> https://urldefense.proofpoint.com/v2/url?u=http-
    >3A__www.dpdk.org_dev_patchwork_patch_26329_&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&
    >r=BVhFA09CGX7JQ5Ih-uZnsw&m=QX4imB5BrTr10-juIo_0ZIyAaupqmoSCyRkg3ad-
    >540&s=6Qns7wBUtQJEcoBZe5fmi3Tt6wudCganhTjDjvExes8&e= .
    >    >> I haven't had a chance to test this patch yet, but if it works, it may
    >not
    >    >be
    >    >> necessary to set scatter_rx mode in OvS.
    >    >>
    >    >> Thanks,
    >    >> Mark
    >    >>
    >    >>
    >    >> >
    >    >> >$ sudo ./utilities/ovs-vsctl set Interface dpdk0 mtu_request=1500 <--
    >    >> >This looks ok $ sudo ./utilities/ovs-vsctl set Interface dpdk0
    >    >> >mtu_request=5000 2017-07-
    >    >> 03T15:26:28Z|00073|netdev_dpdk|ERR|Interface
    >    >> >dpdk0 MTU (5000) setup
    >    >> >error: Invalid argument
    >    >> >2017-07-03T15:26:28Z|00074|netdev_dpdk|ERR|Interface dpdk0(rxq:1
    >    >> txq:2)
    >    >> >configure error: Invalid argument
    >    >> >2017-07-03T15:26:28Z|00075|dpif_netdev|ERR|Failed to set interface
    >    >> >dpdk0 new configuration
    >    >> >2017-07-03T15:26:28Z|00076|ofproto|WARN|br0: cannot get STP status on
    >    >> >nonexistent port 1
    >    >> >2017-07-03T15:26:28Z|00077|ofproto|WARN|br0: cannot get RSTP status
    >    >> on
    >    >> >nonexistent port 1
    >    >> sugeshch at silpixa00389820:~/repo/ovs_dpdk/ovs_dpdk$
    >    >> >2017-07-
    >    >> >03T15:26:32Z|00078|ofproto|WARN|br0: cannot get STP stats on
    >    >> >nonexistent port
    >    >> >1
    >    >> >2017-07-03T15:26:37Z|00079|ofproto|WARN|br0: cannot get STP stats on
    >    >> >nonexistent port 1
    >    >> >2017-07-03T15:26:42Z|00080|ofproto|WARN|br0: cannot get STP stats on
    >    >> >nonexistent port 1
    >    >> >
    >    >> >I assume just the scatter_flag may not sufficient to enable
    >jumbo_frames??
    >    >> >Because in my testing I noticed the behavior is same that reported by
    >    >> >Ian in the v2 patch. No traffic is getting forwarded through the
    >ports.
    >    >> >
    >    >> >
    >    >> >> +            break;
    >    >> >> +        }
    >    >> >> +
    >    >> >>          for (i = 0; i < n_txq; i++) {
    >    >> >>              diag = rte_eth_tx_queue_setup(dev->port_id, i, dev-
    >    >>txq_size,
    >    >> >>                                            dev->socket_id, NULL);
    >    >> >> --
    >    >> >> 1.9.3
    >    >> >>
    >    >> >> _______________________________________________
    >    >> >> dev mailing list
    >    >> >> dev at openvswitch.org
    >    >> >> https://urldefense.proofpoint.com/v2/url?u=https-
    >3A__mail.openvswitch.org_mailman_listinfo_ovs-
    >2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-
    >uZnsw&m=QX4imB5BrTr10-juIo_0ZIyAaupqmoSCyRkg3ad-540&s=hrqvPrPvbz7fNIQ2uSHjDH0-
    >8bybqH21vcc86SOLmLk&e=
    >    _______________________________________________
    >    dev mailing list
    >    dev at openvswitch.org
    >    https://urldefense.proofpoint.com/v2/url?u=https-
    >3A__mail.openvswitch.org_mailman_listinfo_ovs-
    >2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-
    >uZnsw&m=QX4imB5BrTr10-juIo_0ZIyAaupqmoSCyRkg3ad-540&s=hrqvPrPvbz7fNIQ2uSHjDH0-
    >8bybqH21vcc86SOLmLk&e=
    >
    
    



More information about the dev mailing list