[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