From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
To: "Yigit, Ferruh" <ferruh.yigit@intel.com>,
Shahaf Shuler <shahafs@mellanox.com>,
Thomas Monjalon <thomas@monjalon.net>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [RFC PATCH 4/4] ethdev: add helpers to move to the new offloads API
Date: Wed, 30 Aug 2017 14:15:34 +0000 [thread overview]
Message-ID: <2601191342CEEE43887BDE71AB9772584F23E2EC@IRSMSX103.ger.corp.intel.com> (raw)
In-Reply-To: <59831a36-6565-9c4f-2d8e-8bdb096246e3@intel.com>
> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Wednesday, August 30, 2017 1:43 PM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Shahaf Shuler <shahafs@mellanox.com>; Thomas Monjalon
> <thomas@monjalon.net>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [RFC PATCH 4/4] ethdev: add helpers to move to the new offloads API
>
> On 8/30/2017 11:16 AM, Ananyev, Konstantin wrote:
> > Hi Ferruh,
> >
> >> -----Original Message-----
> >> From: Yigit, Ferruh
> >> Sent: Wednesday, August 30, 2017 8:51 AM
> >> To: Shahaf Shuler <shahafs@mellanox.com>; Thomas Monjalon <thomas@monjalon.net>; Ananyev, Konstantin
> >> <konstantin.ananyev@intel.com>
> >> Cc: dev@dpdk.org
> >> Subject: Re: [dpdk-dev] [RFC PATCH 4/4] ethdev: add helpers to move to the new offloads API
> >>
> >> On 8/30/2017 7:30 AM, Shahaf Shuler wrote:
> >>> Tuesday, August 29, 2017 3:55 PM, Ferruh Yigit:
> >>>>>> Considering the re-configuration is risky, and without other ideas I will
> >>>> need to fall back to the error flow case.
> >>>>>> Are we OK with that?
> >>>>>
> >>>>> I think we can take the risk of keeping this call to
> >>>>> rte_eth_dev_configure() in the middle of rte_eth_rx_queue_setup().
> >>>>> In theory it should be acceptable.
> >>>>> If we merge it soon, it can be better tested with every drivers.
> >>>>
> >>>> I doubt about taking that risk. Some driver does HW configuration via
> >>>> configure() and combination of start/stop, setup_queue and configure can
> >>>> be complex.
> >>>>
> >>>> I am for generating error for this case.
> >>>>
> >>>> Generating error also can be good motivation for PMDs to adapt new
> >>>> method.
> >>>
> >>> Adding Ananyev suggestion from other thread:
> >>> For tx_prepare() work, we used the following approach:
> >>> 1. submitted patch with changes in rte_ethdev and PMDs we are familiar with (Intel ones).
> >>> For other PMDs - patch contained just minimal changes to make it build cleanly.
> >>> 2. Asked other PMD maintainers to review rte_ethdev changes and provide a proper patch
> >>> for the PMD they own.
> >>
> >> tx_prepare() is a little different, since it was not clear if all PMDs
> >> needs updating that is why asked to PMD owners, and the ones requires
> >> updating already has been updated with ethdev patch. Here we know all
> >> PMDs need updating, and they need proper time in advance.
> >>
> >>>
> >>> So I am OK with both suggestions. Meaning:
> >>> 1. Define the case were application use the new offloads API with PMD which supports the old one as an error.
> >>> 2. apply patches to ethdev with the above behavior.
> >>>
> >>> Just to emphasize, it means that PMDs which won't move to the new API by the end of 17.11 will not be able to run with any of the
> >> examples and application on DPDK tree (and also with other applications which moved to the new API), as I plan to submit patches which
> >> convert them all to the new API.
> >>
> >> I think it is good idea to update samples/apps to new method, but this
> >> can be short notice for PMD owners.
> >>
> >> Can we wait one more release to update samples/apps, to give time for
> >> PMDs to be updated, since old applications will work with new PMDs
> >> (thanks to your helpers), I believe this won't be a problem.
> >
> > I am not sure what is your suggestion here?
> > Support both flavors of PMD API for 17.11?
>
> Support both with an exception, when application uses new method but PMD
> uses old one, throw an error (because of above discussed technical issue).
>
> This lets existing applications run without problem, and pushes PMDs to
> adapt new method.
>
> Your suggestion to have only new method and convert all PMDs to new
> method is good, but I am not sure if this is realistic for this release,
> since different PMDs have different pace for updates.
>
> ethdev updates can be done in this release, with the PMDs that already
> changed to new method. The sample/app modifications Shahaf mentioned can
> be done in the beginning of the next release, and this gives time to
> remaining PMDs until end of next release. What do you think?
If it is just a timing concern - can we probably request PMD maintainers at least to:
- review the proposed new API and object if they have any concerns
- provide an estimation - how long it would take to adopt a new one
let say in a week time?
Then we could make a final decision - do things in one go, or prolong the pain for 2 releases.
Konstantin
>
> > Konstantin
> >
> >>
> >>>
> >>> Any objection to this approach?
> >>>
> >>>
> >
next prev parent reply other threads:[~2017-08-30 14:15 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-07 10:54 [dpdk-dev] [RFC PATCH 0/4] ethdev " Shahaf Shuler
2017-08-07 10:54 ` [dpdk-dev] [RFC PATCH 1/4] ethdev: rename Rx and Tx configuration structs Shahaf Shuler
2017-08-23 21:39 ` Thomas Monjalon
2017-08-07 10:54 ` [dpdk-dev] [RFC PATCH 2/4] ethdev: introduce Rx queue offloads API Shahaf Shuler
2017-08-23 12:21 ` Ananyev, Konstantin
2017-08-23 13:06 ` Shahaf Shuler
2017-08-23 21:48 ` Thomas Monjalon
2017-08-29 12:50 ` Ferruh Yigit
2017-08-30 6:22 ` Shahaf Shuler
2017-08-29 13:11 ` Ferruh Yigit
2017-08-07 10:54 ` [dpdk-dev] [RFC PATCH 3/4] ethdev: introduce Tx " Shahaf Shuler
2017-08-07 10:54 ` [dpdk-dev] [RFC PATCH 4/4] ethdev: add helpers to move to the new " Shahaf Shuler
2017-08-23 12:28 ` Ananyev, Konstantin
2017-08-23 13:13 ` Shahaf Shuler
2017-08-23 22:06 ` Thomas Monjalon
2017-08-24 7:12 ` Shahaf Shuler
2017-08-25 13:26 ` Thomas Monjalon
2017-08-29 12:55 ` Ferruh Yigit
2017-08-30 6:30 ` Shahaf Shuler
2017-08-30 7:50 ` Ferruh Yigit
2017-08-30 10:16 ` Ananyev, Konstantin
2017-08-30 12:42 ` Ferruh Yigit
2017-08-30 13:25 ` Thomas Monjalon
2017-08-30 14:15 ` Ananyev, Konstantin [this message]
2017-08-28 14:12 ` Ananyev, Konstantin
2017-08-29 6:26 ` Shahaf Shuler
2017-08-29 9:43 ` Ananyev, Konstantin
2017-08-23 6:39 ` [dpdk-dev] [RFC PATCH 0/4] ethdev " Shahaf Shuler
2017-08-23 22:16 ` Thomas Monjalon
2017-08-25 10:31 ` Jerin Jacob
2017-08-27 6:05 ` Shahaf Shuler
2017-08-28 5:00 ` Jerin Jacob
2017-08-28 10:57 ` Thomas Monjalon
2017-09-05 7:07 ` Jerin Jacob
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2601191342CEEE43887BDE71AB9772584F23E2EC@IRSMSX103.ger.corp.intel.com \
--to=konstantin.ananyev@intel.com \
--cc=dev@dpdk.org \
--cc=ferruh.yigit@intel.com \
--cc=shahafs@mellanox.com \
--cc=thomas@monjalon.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).