[dpdk-dev] doc: announce ethdev CRC strip flag deprecation

Message ID 20180320112631.107105-1-ferruh.yigit@intel.com (mailing list archive)
State Superseded, archived
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail apply issues

Commit Message

Ferruh Yigit March 20, 2018, 11:26 a.m. UTC
  Make CRC stripping default behavior, deprecate flag
DEV_RX_OFFLOAD_CRC_STRIP.

Introduce a new flag to let applications request keep CRC.

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 doc/guides/rel_notes/deprecation.rst | 6 ++++++
 1 file changed, 6 insertions(+)
  

Comments

Thomas Monjalon March 20, 2018, 11:35 a.m. UTC | #1
20/03/2018 12:26, Ferruh Yigit:
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> +* ethdev: Make CRC stript default behavior without any flag required and add a

s/stript/stripping/

> +  new offload flag to let application request for keeping CRC if PMD reports
> +  capability for it.
> +  ``DEV_RX_OFFLOAD_CRC_STRIP`` flag will be removed.
> +  ``DEV_RX_OFFLOAD_KEETP_CRC`` will be added.

s/KEETP/KEEP/

I think we should introduce the new flag without removing the old one
for one release.
Setting both flags would be an error.
Setting no flag would mean stripping.
So the CRC_STRIP flag would be just ignored by PMDs.

Opinions?
  
Ferruh Yigit March 20, 2018, 5:23 p.m. UTC | #2
On 3/20/2018 11:35 AM, Thomas Monjalon wrote:
> 20/03/2018 12:26, Ferruh Yigit:
>> --- a/doc/guides/rel_notes/deprecation.rst
>> +++ b/doc/guides/rel_notes/deprecation.rst
>> +* ethdev: Make CRC stript default behavior without any flag required and add a
> 
> s/stript/stripping/
> 
>> +  new offload flag to let application request for keeping CRC if PMD reports
>> +  capability for it.
>> +  ``DEV_RX_OFFLOAD_CRC_STRIP`` flag will be removed.
>> +  ``DEV_RX_OFFLOAD_KEETP_CRC`` will be added.
> 
> s/KEETP/KEEP/
> 
> I think we should introduce the new flag without removing the old one
> for one release.
> Setting both flags would be an error.
> Setting no flag would mean stripping.
> So the CRC_STRIP flag would be just ignored by PMDs.
> 
> Opinions?

Introducing KEEP_CRC in this release is OK, since no PMD announces this
capability, it should be safe.

Since many PMD's offloading patches are not applied yet, deciding on "Setting no
flag would mean stripping" helps to get this correct at first place for them.
Only this is ABI break, but I am not aware of any PMD that implements not
stripping CRC case so I hope this is acceptable.

If there is no objection, I will update notice to only remove flag in next release.
  

Patch

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index b40c57f28..6e1266b20 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -147,6 +147,12 @@  Deprecation Notices
   successful. This modification will only impact the PMDs, not the
   applications.
 
+* ethdev: Make CRC stript default behavior without any flag required and add a
+  new offload flag to let application request for keeping CRC if PMD reports
+  capability for it.
+  ``DEV_RX_OFFLOAD_CRC_STRIP`` flag will be removed.
+  ``DEV_RX_OFFLOAD_KEETP_CRC`` will be added.
+
 * i40e: The default flexible payload configuration which extracts the first 16
   bytes of the payload for RSS will be deprecated starting from 18.02. If
   required the previous behavior can be configured using existing flow