From: Shahaf Shuler <shahafs@mellanox.com>
To: Thomas Monjalon <thomas@monjalon.net>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
"Ananyev, Konstantin" <konstantin.ananyev@intel.com>
Subject: Re: [dpdk-dev] [RFC PATCH 4/4] ethdev: add helpers to move to the new offloads API
Date: Thu, 24 Aug 2017 07:12:28 +0000 [thread overview]
Message-ID: <VI1PR05MB3149D68BF5782E4BCE582CE8C39A0@VI1PR05MB3149.eurprd05.prod.outlook.com> (raw)
In-Reply-To: <5969739.W1oj9hpjs5@xps>
Thursday, August 24, 2017 1:06 AM, Thomas Monjalon:
> 23/08/2017 15:13, Shahaf Shuler:
> > Wednesday, August 23, 2017 3:29 PM, Ananyev, Konstantin:
> > > From: Shahaf Shuler
> > > > In order to enable PMDs to support only one of the APIs, and
> > > > applications to avoid branching according to the underlying device
> > > > a copy functions to/from the old/new APIs were added.
>
> Looks a good intent.
> I would prefer the word "convert" instead of "copy".
>
> > > > int
> > > > rte_eth_rx_queue_setup(uint8_t port_id, uint16_t rx_queue_id,
> [...]
> > > > + } else if ((!(dev->data->dev_flags & RTE_ETH_DEV_RXQ_OFFLOAD))
> &&
> > > > + (dev->data->dev_conf.rxmode.ignore == 1)) {
> > > > + int ret;
> > > > + struct rte_eth_rxmode rxmode;
> > > > +
> > > > + rte_eth_copy_rxq_offloads(&rxmode, rx_conf);
> > > > + if (memcmp(&rxmode, &dev->data->dev_conf.rxmode,
> > > > + sizeof(rxmode))) {
> > > > + /*
> > > > + * device which work with rxmode offloads API
> requires
> > > > + * a re-configuration in order to apply the new
> offloads
> > > > + * configuration.
> > > > + */
> > > > + dev->data->dev_conf.rxmode = rxmode;
> > > > + ret = rte_eth_dev_configure(port_id,
> > > > + dev->data->nb_rx_queues,
> > > > + dev->data->nb_tx_queues,
> > > > + &dev->data->dev_conf);
> > >
> > > Hmm, and why we would need to reconfigure our device in the middle
> > > of rx queue setup?
> >
> > The reason is the old Rx offloads API is configured on device configure.
> > This if section is for applications which already moved to the new
> > offload API however the underlying PMD still uses the old one.
>
> Isn't it risky to re-run configure here?
> We could also declare this case as an error.
>
> I think applications which have migrated to the new API, could use the
> convert functions themselves before calling configure to support not
> migrated PMDs.
> The cons of my solution are:
> - discourage apps to migrate before all PMDs have migrated
> - expose a temporary function to convert API I propose it anyway because
> there is always someone to like bad ideas ;)
Yes. I tried to make it as simple as possible for application to move to the new API.
Defining it as error flow, will enforce the application to check the PMD offload mode and branch accordingly. The conversion functions are a good helpers, yet the code remains complex due to the different cases with the different PMDs.
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?
next prev parent reply other threads:[~2017-08-24 7:12 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 [this message]
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
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=VI1PR05MB3149D68BF5782E4BCE582CE8C39A0@VI1PR05MB3149.eurprd05.prod.outlook.com \
--to=shahafs@mellanox.com \
--cc=dev@dpdk.org \
--cc=konstantin.ananyev@intel.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).