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

Darrell Ball dball at vmware.com
Wed Jul 12 16:47:31 UTC 2017



On 7/12/17, 1:04 AM, "Kavanagh, Mark B" <mark.b.kavanagh at intel.com> wrote:

    >From: Darrell Ball [mailto:dball at vmware.com]
    >Sent: Tuesday, July 11, 2017 5:03 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
    >
    >
    >
    >On 7/11/17, 8:44 AM, "Kavanagh, Mark B" <mark.b.kavanagh at intel.com> wrote:
    >
    >    >From: Darrell Ball [mailto:dball at vmware.com]
    >    >Sent: Tuesday, July 11, 2017 2:03 AM
    >    >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
    >    >
    >    >
    >    >
    >    >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 ?
    >
    >    Sure, I definitely understand your concerns Darrel - unfortunately, I
    >can't definitively answer yes. However, since OvS now supports DPDK v16.11.2,
    >I would hope that the stability of the latter would provide some level of
    >assurance.
    >
    >    >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 ?
    >
    >    If we keep the workaround, it essentially negates the need for the
    >netdev_set_mtu function, since jumbo frame mode and the NIC port's max rx
    >packet length are set by rte_eth_dev_configure. The downside of this approach
    >is that it doesn't update the MTU for the Ethernet device itself; as a result,
    >any call to rte_eth_dev_get_mtu will always return 1500, irrespective of the
    >MTU that was set using the existing method.
    >
    >Let us just go ahead with the patch and fix with any potential fallout/bugs,
    >as it is the right approach
    >anyways.
    
    Thanks Darrell - much appreciated.
    
    I'm, spinning a v4 to add 'acked-by' and 'tested-by' tags for Sugesh - shall I add an 'acked-by' for you also?
    
    Thanks again,
    Mark
    

Pls add my Acked-by

Thanks Darrell


    
    
    >
    >    Thanks again,
    >    Mark
    >
    >    >
    >    >
    >    >    >
    >    >    >
    >    >    >
    >    >    >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=uilaK90D4TOVoH58JNXRg
    >Q
    >    >&
    >    >    >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