DPDK patches and discussions
 help / color / mirror / Atom feed
From: Shahaf Shuler <shahafs@mellanox.com>
To: Andrew Rybchenko <arybchenko@solarflare.com>,
	Ferruh Yigit <ferruh.yigit@intel.com>,
	Wei Zhao <wei.zhao1@intel.com>, "dev@dpdk.org" <dev@dpdk.org>
Cc: "stable@dpdk.org" <stable@dpdk.org>, Wenzhuo Lu <wenzhuo.lu@intel.com>
Subject: Re: [dpdk-dev] [PATCH] net/ixgbe: remove hardcoded CRC STRIP config from ixgbe
Date: Sun, 12 Aug 2018 08:46:56 +0000	[thread overview]
Message-ID: <DB7PR05MB442693076A5CAB0E11E06407C33A0@DB7PR05MB4426.eurprd05.prod.outlook.com> (raw)
In-Reply-To: <0f570014-6212-43d8-6947-fdbc22f1b7b4@solarflare.com>

Sunday, August 12, 2018 10:53 AM, Andrew Rybchenko:
> Subject: Re: [PATCH] net/ixgbe: remove hardcoded CRC STRIP config from
> ixgbe
> 
> On 12.08.2018 09:28, Shahaf Shuler wrote:
> > Thursday, August 9, 2018 11:32 AM, Ferruh Yigit:
> >> Subject: Re: [PATCH] net/ixgbe: remove hardcoded CRC STRIP config
> >> from ixgbe
> >>
> >> On 7/24/2018 3:36 AM, Wei Zhao wrote:
> >>> There is CRC related ifdefs for ixgbe:
> >>> CONFIG_RTE_LIBRTE_IXGBE_PF_DISABLE_STRIP_CRC=n
> >>> It is used in VF drivers ixgbevf_dev_configure() functions.
> >>> VF cannot change the CRC strip behavior and based on what PF
> >>> configured it needs to response proper to user
> >>> ixgbevf_dev_configure() request. Right now what PF set is defined by
> >>> above config options but this method is too static.
> >>>
> >>> Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
> >>> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> >> <...>
> >>> @@ -334,6 +334,7 @@ struct rte_eth_rxmode {
> >>>   	 * structure are allowed to be set.
> >>>   	 */
> >>>   	uint64_t offloads;
> >>> +	uint64_t offloads_disable;
> >> Do we need a disable flag in ethdev, an offload not enabled is
> >> disabled by default isn't it. This conflicts with offloads flag and makes is
> confusing.
> > +1.
> >
> > **all** the offloads are disabled by default.
> >
> >> For igb/e1000 VF case, VF driver can't change what PF set and VF
> >> driver can't learn the PF setting dynamically, so this information
> >> needs to be passed to VF driver by application/user.
> >> Currently this information passed by compile time config option, my
> >> suggestion was using devargs.
> >>
> >> In your implementation testpmd parameter added to get this
> >> information and pass to driver, but this means all applications needs
> >> to do this, instead adding this support to driver looks better to me.
> 
> I think we should add fixed offloads to dev_info. I.e. if offlload is supported,
> it could be marked as fixed (i.e. always enabled).
> If offload is not supported, it is always disabled (and cannot/should not be
> marked as fixed).
> May be the right name for it is not "fixed", but "always_enabled".

I think it will over complicate applications. Those limitation should be expressed as part of the "limitation" section of the corresponding PMD guide. 

PMDs with such limitation can also put some warning message to notify or even fail the device configuration if the needed permanent offload is not set by the application. 

> Also it should be persistent. It should not be allowed in above ixgbe case to
> change the offload state on PF if there are other users (drivers attached).
> Otherwise, we need mechanism to notify apps about these changes -
> overcomplicated.

  reply	other threads:[~2018-08-12  8:46 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-24  2:36 Wei Zhao
2018-08-02  2:27 ` Zhang, Qi Z
2018-08-09  1:51   ` Zhao1, Wei
2018-08-09  8:31 ` Ferruh Yigit
2018-08-12  6:28   ` Shahaf Shuler
2018-08-12  7:52     ` Andrew Rybchenko
2018-08-12  8:46       ` Shahaf Shuler [this message]
2018-08-13 15:10         ` Ferruh Yigit

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=DB7PR05MB442693076A5CAB0E11E06407C33A0@DB7PR05MB4426.eurprd05.prod.outlook.com \
    --to=shahafs@mellanox.com \
    --cc=arybchenko@solarflare.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=stable@dpdk.org \
    --cc=wei.zhao1@intel.com \
    --cc=wenzhuo.lu@intel.com \
    /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).