DPDK patches and discussions
 help / color / mirror / Atom feed
From: Andrew Rybchenko <arybchenko@solarflare.com>
To: Ferruh Yigit <ferruh.yigit@intel.com>,
	Neil Horman <nhorman@tuxdriver.com>,
	John McNamara <john.mcnamara@intel.com>,
	"Marko Kovacevic" <marko.kovacevic@intel.com>,
	"John W. Linville" <linville@tuxdriver.com>,
	Allain Legacy <allain.legacy@windriver.com>,
	"Matt Peters" <matt.peters@windriver.com>,
	Ravi Kumar <ravi1.kumar@amd.com>,
	"Ajit Khaparde" <ajit.khaparde@broadcom.com>,
	Somnath Kotur <somnath.kotur@broadcom.com>,
	Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>,
	Wenzhuo Lu <wenzhuo.lu@intel.com>,
	Qi Zhang <qi.z.zhang@intel.com>,
	Xiao Wang <xiao.w.wang@intel.com>,
	Beilei Xing <beilei.xing@intel.com>,
	Konstantin Ananyev <konstantin.ananyev@intel.com>,
	Adrien Mazarguil <adrien.mazarguil@6wind.com>,
	Nelio Laranjeiro <nelio.laranjeiro@6wind.com>,
	Yongseok Koh <yskoh@mellanox.com>,
	"Tomasz Duszynski" <tdu@semihalf.com>,
	Dmitri Epshtein <dima@marvell.com>,
	"Natalie Samsonov" <nsamsono@marvell.com>,
	Jianbo Liu <jianbo.liu@arm.com>,
	"Alejandro Lucero" <alejandro.lucero@netronome.com>,
	Tetsuya Mukawa <mtetsuyah@gmail.com>,
	Santosh Shukla <santosh.shukla@caviumnetworks.com>,
	Jerin Jacob <jerin.jacob@caviumnetworks.com>,
	Rasesh Mody <rasesh.mody@cavium.com>,
	Harish Patil <harish.patil@cavium.com>,
	"Shahed Shaikh" <shahed.shaikh@cavium.com>,
	Bruce Richardson <bruce.richardson@intel.com>,
	Andrew Rybchenko <arybchenko@solarflare.com>,
	Jasvinder Singh <jasvinder.singh@intel.com>,
	Cristian Dumitrescu <cristian.dumitrescu@intel.com>,
	Matej Vido <vido@cesnet.cz>,
	Maciej Czekaj <maciej.czekaj@caviumnetworks.com>,
	Maxime Coquelin <maxime.coquelin@redhat.com>,
	Tiwei Bie <tiwei.bie@intel.com>,
	Zhihong Wang <zhihong.wang@intel.com>,
	Yong Wang <yongwang@vmware.com>,
	Thomas Monjalon <thomas@monjalon.net>
Cc: <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH] ethdev: add new offload flag to keep CRC
Date: Wed, 20 Jun 2018 10:42:50 +0300	[thread overview]
Message-ID: <9390200c-b113-5f22-99d4-21c42b5f56f3@solarflare.com> (raw)
In-Reply-To: <20180619180230.72585-1-ferruh.yigit@intel.com>

On 06/19/2018 09:02 PM, Ferruh Yigit wrote:
> DEV_RX_OFFLOAD_KEEP_CRC offload flag added. PMDs that supports keeping
> CRC should advertise this offload capability.
>
> DEV_RX_OFFLOAD_CRC_STRIP flag will remain one more release
> default behavior in PMDs are to keep the CRC until this flag removed
>
> Until DEV_RX_OFFLOAD_CRC_STRIP flag is removed:
> - Setting both KEEP_CRC & CRC_STRIP is INVALID
> - Setting only CRC_STRIP PMD should strip the CRC
> - Setting only KEEP_CRC PMD should keep the CRC
> - Not setting both PMD should keep the CRC
>
> A helper function rte_eth_dev_is_keep_crc() has been added to be able to
> change the no flag behavior with minimal changes in PMDs.
>
> The PMDs that doesn't report the DEV_RX_OFFLOAD_KEEP_CRC offload can
> remove rte_eth_dev_is_keep_crc() checks next release, related code
> commented to help the maintenance task.
>
> And DEV_RX_OFFLOAD_CRC_STRIP has been added to virtual drivers since
> they don't use CRC at all, when an application requires this offload
> virtual PMDs should not return error.
>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---

<...>

> diff --git a/lib/librte_ethdev/rte_ethdev_driver.h b/lib/librte_ethdev/rte_ethdev_driver.h
> index c9c825e3f..09a42f8c2 100644
> --- a/lib/librte_ethdev/rte_ethdev_driver.h
> +++ b/lib/librte_ethdev/rte_ethdev_driver.h
> @@ -325,6 +325,26 @@ typedef int (*ethdev_uninit_t)(struct rte_eth_dev *ethdev);
>   int __rte_experimental
>   rte_eth_dev_destroy(struct rte_eth_dev *ethdev, ethdev_uninit_t ethdev_uninit);
>   
> +/**
> + * PMD helper function to check if keeping CRC is requested
> + *
> + * @param rx_offloads
> + *   offloads variable
> + *
> + * @return
> + *   Return positive if keeping CRC is requested,
> + *   zero if stripping CRC is requested
> + */
> +static inline int
> +rte_eth_dev_is_keep_crc(uint64_t rx_offloads)
> +{
> +	if (rx_offloads & DEV_RX_OFFLOAD_CRC_STRIP)
> +		return 0;
> +
> +	/* no KEEP_CRC or CRC_STRIP offload flags means keep CRC */
> +	return 1;
> +}
> +
>   #ifdef __cplusplus
>   }
>   #endif

A couple of control questions about the function:
  - shouldn't __rte_experimental be used?
  - if the function remains in the future, it will be a bit asymmetric 
vs other
    offload flags. Right now it is clear why the function is introduced, but
    it is the question if the function should remain or go away in the 
future
    (as far as I know no other offload flag has similar function to check).

above things are really minor, ethdev and net/sfc
Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>

  reply	other threads:[~2018-06-20  7:43 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-08 22:57 [dpdk-dev] [RFC] " Ferruh Yigit
2018-06-09 10:11 ` Andrew Rybchenko
2018-06-11  9:18   ` Ferruh Yigit
2018-06-11 15:25 ` Stephen Hemminger
2018-06-19 12:54   ` Ferruh Yigit
2018-06-19 18:02 ` [dpdk-dev] [PATCH] " Ferruh Yigit
2018-06-20  7:42   ` Andrew Rybchenko [this message]
2018-06-20 17:24     ` Ferruh Yigit
2018-06-20 17:39       ` Andrew Rybchenko
2018-06-20 18:12         ` Ferruh Yigit
2018-06-20 21:16           ` Andrew Rybchenko
2018-06-20 10:54   ` Legacy, Allain
2018-06-20 13:44   ` Shahaf Shuler
2018-06-20 16:12     ` Ferruh Yigit
2018-06-21  7:53       ` Shahaf Shuler
2018-06-21 13:14   ` [dpdk-dev] [PATCH v2] " Ferruh Yigit
2018-06-28 23:46     ` Thomas Monjalon
2018-06-29 12:41     ` [dpdk-dev] [PATCH v3] " Ferruh Yigit
2018-06-29 11:57       ` Thomas Monjalon
2018-06-29 16:33         ` 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=9390200c-b113-5f22-99d4-21c42b5f56f3@solarflare.com \
    --to=arybchenko@solarflare.com \
    --cc=adrien.mazarguil@6wind.com \
    --cc=ajit.khaparde@broadcom.com \
    --cc=alejandro.lucero@netronome.com \
    --cc=allain.legacy@windriver.com \
    --cc=beilei.xing@intel.com \
    --cc=bruce.richardson@intel.com \
    --cc=cristian.dumitrescu@intel.com \
    --cc=dev@dpdk.org \
    --cc=dima@marvell.com \
    --cc=ferruh.yigit@intel.com \
    --cc=harish.patil@cavium.com \
    --cc=jasvinder.singh@intel.com \
    --cc=jerin.jacob@caviumnetworks.com \
    --cc=jianbo.liu@arm.com \
    --cc=john.mcnamara@intel.com \
    --cc=konstantin.ananyev@intel.com \
    --cc=linville@tuxdriver.com \
    --cc=maciej.czekaj@caviumnetworks.com \
    --cc=marko.kovacevic@intel.com \
    --cc=matt.peters@windriver.com \
    --cc=maxime.coquelin@redhat.com \
    --cc=mtetsuyah@gmail.com \
    --cc=nelio.laranjeiro@6wind.com \
    --cc=nhorman@tuxdriver.com \
    --cc=nsamsono@marvell.com \
    --cc=qi.z.zhang@intel.com \
    --cc=rahul.lakkireddy@chelsio.com \
    --cc=rasesh.mody@cavium.com \
    --cc=ravi1.kumar@amd.com \
    --cc=santosh.shukla@caviumnetworks.com \
    --cc=shahed.shaikh@cavium.com \
    --cc=somnath.kotur@broadcom.com \
    --cc=tdu@semihalf.com \
    --cc=thomas@monjalon.net \
    --cc=tiwei.bie@intel.com \
    --cc=vido@cesnet.cz \
    --cc=wenzhuo.lu@intel.com \
    --cc=xiao.w.wang@intel.com \
    --cc=yongwang@vmware.com \
    --cc=yskoh@mellanox.com \
    --cc=zhihong.wang@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).